-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[llvm] Fix assertion error where we didn't check fixed point types. #80757
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
Conversation
@llvm/pr-subscribers-debuginfo Author: None (PiJoules) ChangesFull diff: https://github.com/llvm/llvm-project/pull/80757.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index eb2d992c7e75e..f00ff1565c665 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -224,12 +224,15 @@ bool DebugHandlerBase::isUnsignedDIType(const DIType *Ty) {
Encoding == dwarf::DW_ATE_float || Encoding == dwarf::DW_ATE_UTF ||
Encoding == dwarf::DW_ATE_boolean ||
Encoding == dwarf::DW_ATE_complex_float ||
+ Encoding == dwarf::DW_ATE_signed_fixed ||
+ Encoding == dwarf::DW_ATE_unsigned_fixed ||
(Ty->getTag() == dwarf::DW_TAG_unspecified_type &&
Ty->getName() == "decltype(nullptr)")) &&
"Unsupported encoding");
return Encoding == dwarf::DW_ATE_unsigned ||
Encoding == dwarf::DW_ATE_unsigned_char ||
Encoding == dwarf::DW_ATE_UTF || Encoding == dwarf::DW_ATE_boolean ||
+ Encoding == llvm::dwarf::DW_ATE_unsigned_fixed ||
Ty->getTag() == dwarf::DW_TAG_unspecified_type;
}
|
faf7461
to
b15940e
Compare
Test? |
b15940e
to
de5967f
Compare
Added one to test the crash doesn't happen |
llvm/test/DebugInfo/fixed-point.ll
Outdated
@@ -0,0 +1,26 @@ | |||
;; This fixes https://github.com/llvm/llvm-project/issues/81555 | |||
;; Note this isn't correct dwarf but we just need to assert this doesn't crash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this isn't correct, should we instead reject it in the IR Verifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Assuming I'm using the verify correctly, it looks like this dwarf actually is valid.
de5967f
to
58fb25d
Compare
ping |
it breaks at least four builders
there is the same for win builds too. |
…types." (#82285) These changes break the `LLVM::fixed-point.ll` test some targets what fails the builds for those targets (more details #80757 (comment)) The problem wasn't fixed for few days, Reverts #80757
Sorry for the breakages and thanks for the revert. I probably shouldn't have pushed right before an extended weekend :/ |
This fixes #81555