Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIR][CIRGen] Change SignBitOp result type to !cir.bool #1187

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

PikachuHyA
Copy link
Collaborator

No description provided.

@PikachuHyA
Copy link
Collaborator Author

When I added support for __builtin_signbit, I found that returning !cir.bool from SignBitOp caused a crash during the lowering process to LLVM IR. This issue occurred because a builtin.unrealized_conversion_cast appeared during lowering and could not be eliminated. However, when I changed the return value of SignBitOp to !s32i, the lowering to LLVM IR proceeded without any issues.

error with return !cir.bool

$./bin/clang -fclangir -c ../clang/test/CIR/CodeGen/builtin-signbit.c -Xclang -emit-llvm -o t.ll
loc(fused["../clang/test/CIR/CodeGen/builtin-signbit.c":13:33, "../clang/test/CIR/CodeGen/builtin-signbit.c":13:34]): error: LLVM Translation failed for operation: builtin.unrealized_conversion_cast
fatal error: error in backend: Lowering from LLVMIR dialect to llvm IR failed!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -fclangir -c ../clang/test/CIR/CodeGen/builtin-signbit.c -Xclang -emit-llvm -o t.ll
1.      <eof> parser at end of file
 #0 0x00000000020911e8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (./bin/clang+0x20911e8)
 #1 0x000000000208ecde llvm::sys::RunSignalHandlers() (./bin/clang+0x208ecde)
 #2 0x0000000002090579 llvm::sys::CleanupOnSignal(unsigned long) (./bin/clang+0x2090579)
 #3 0x00000000020092d7 (anonymous namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) CrashRecoveryContext.cpp:0:0
 #4 0x000000000200926f llvm::CrashRecoveryContext::HandleExit(int) (./bin/clang+0x200926f)
 #5 0x000000000208b2f7 llvm::sys::Process::Exit(int, bool) (./bin/clang+0x208b2f7)
 #6 0x0000000000eb7db6 (./bin/clang+0xeb7db6)
 #7 0x000000000200f7e6 llvm::report_fatal_error(llvm::Twine const&, bool) (./bin/clang+0x200f7e6)
 #8 0x000000000200f6c6 (./bin/clang+0x200f6c6)
 #9 0x000000000336f4b2 (./bin/clang+0x336f4b2)
#10 0x0000000003345b12 cir::CIRGenConsumer::HandleTranslationUnit(clang::ASTContext&) (./bin/clang+0x3345b12)
#11 0x0000000005521379 clang::ParseAST(clang::Sema&, bool, bool) (./bin/clang+0x5521379)
#12 0x0000000002b05cbf clang::FrontendAction::Execute() (./bin/clang+0x2b05cbf)
#13 0x0000000002a7659d clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (./bin/clang+0x2a7659d)
#14 0x0000000002beb9de clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (./bin/clang+0x2beb9de)
#15 0x0000000000eb78e8 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (./bin/clang+0xeb78e8)
#16 0x0000000000eb3e3e ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#17 0x00000000028c8379 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::$_0>(long) Job.cpp:0:0
#18 0x0000000002009206 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (./bin/clang+0x2009206)
#19 0x00000000028c7a42 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (./bin/clang+0x28c7a42)
#20 0x0000000002886cec clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (./bin/clang+0x2886cec)
#21 0x0000000002886ff7 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (./bin/clang+0x2886ff7)
#22 0x00000000028a4408 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (./bin/clang+0x28a4408)
#23 0x0000000000eb345c clang_main(int, char**, llvm::ToolContext const&) (./bin/clang+0xeb345c)
#24 0x0000000000ec2eb7 main (./bin/clang+0xec2eb7)
#25 0x00007f04f4bff192 __libc_start_main (/lib64/libc.so.6+0x28192)
#26 0x0000000000eb1a3e _start (./bin/clang+0xeb1a3e)

@PikachuHyA
Copy link
Collaborator Author

I think the error is related to #480

@Lancern
Copy link
Member

Lancern commented Nov 30, 2024

It crashes because the result type of the zext instruction generated by the lowering of cir.signbit does not match what !cir.bool lowers to:

auto converted = rewriter.create<mlir::LLVM::ZExtOp>(
op.getLoc(), mlir::IntegerType::get(rewriter.getContext(), 32),
cmpResult);

You could update it to:

auto converted = rewriter.create<mlir::LLVM::ZExtOp>(
    op.getLoc(), getTypeConverter()->convertType(op.getType()),
    cmpResult);

And it should be OK then.

@PikachuHyA PikachuHyA force-pushed the change_signbit_return_bool branch from 8f257ca to 6420143 Compare December 2, 2024 07:34
@PikachuHyA PikachuHyA force-pushed the change_signbit_return_bool branch from 6420143 to 81f6ee1 Compare December 2, 2024 08:42
@PikachuHyA
Copy link
Collaborator Author

Thank you very much, @orbiri and @Lancern .

The SignBitOp now returns a boolean value.

@bcardosolopes Please review this patch.

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bcardosolopes bcardosolopes changed the title [CIR][CIRGen]: Change SignBitOp result type to !cir.bool [CIR][CIRGen] Change SignBitOp result type to !cir.bool Dec 2, 2024
@bcardosolopes bcardosolopes merged commit aa7b5c6 into llvm:main Dec 2, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants