Skip to content

[CodeGen] Fix lower constant intrinsics for dead code #102442

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

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

aengelke
Copy link
Contributor

@aengelke aengelke commented Aug 8, 2024

lowerConstantIntrinsics does an RPO traveral, which doesn't reach dead blocks. Remove the assertion that all intrinsics are lowered, because some intrinsics might remain.

lowerConstantIntrinsics does an RPO traveral, which doesn't reach dead
blocks. Remove the assertion that all intrinsics are lowered, because
some intrinsics might remain.
@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Alexis Engelke (aengelke)

Changes

lowerConstantIntrinsics does an RPO traveral, which doesn't reach dead blocks. Remove the assertion that all intrinsics are lowered, because some intrinsics might remain.


Full diff: https://github.com/llvm/llvm-project/pull/102442.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp (+1-1)
  • (added) llvm/test/Transforms/PreISelIntrinsicLowering/constant-intrinscs-dead-code.ll (+31)
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 0d3dd650b8ee68..0df90f402aaf48 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -349,8 +349,8 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
       Changed |= forEachCall(F, [&](CallInst *CI) {
         Function *Parent = CI->getParent()->getParent();
         TargetLibraryInfo &TLI = LookupTLI(*Parent);
+        // Intrinsics in unreachable code are not lowered.
         bool Changed = lowerConstantIntrinsics(*Parent, TLI, /*DT=*/nullptr);
-        assert(Changed && "lowerConstantIntrinsics did not lower intrinsic");
         return Changed;
       });
       break;
diff --git a/llvm/test/Transforms/PreISelIntrinsicLowering/constant-intrinscs-dead-code.ll b/llvm/test/Transforms/PreISelIntrinsicLowering/constant-intrinscs-dead-code.ll
new file mode 100644
index 00000000000000..35de92372cb9fc
--- /dev/null
+++ b/llvm/test/Transforms/PreISelIntrinsicLowering/constant-intrinscs-dead-code.ll
@@ -0,0 +1,31 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S < %s -passes=pre-isel-intrinsic-lowering | FileCheck %s
+
+define void @test_dead() {
+; CHECK-LABEL: define void @test_dead() {
+; CHECK-NEXT:    ret void
+; CHECK:       [[DEAD:.*]]:
+; CHECK-NEXT:    [[X:%.*]] = call i32 @llvm.objectsize.i32.p21(ptr addrspace(21) null, i1 false, i1 false, i1 false)
+; CHECK-NEXT:    br label %[[DEAD]]
+;
+  ret void
+
+dead:
+  %x = call i32 @llvm.objectsize.i32.p21(ptr addrspace(21) null, i1 false, i1 false, i1 false)
+  br label %dead
+}
+
+define i32 @test_two() {
+; CHECK-LABEL: define i32 @test_two() {
+; CHECK-NEXT:    ret i32 -1
+; CHECK:       [[DEAD:.*]]:
+; CHECK-NEXT:    [[X:%.*]] = call i32 @llvm.objectsize.i32.p21(ptr addrspace(21) null, i1 false, i1 false, i1 false)
+; CHECK-NEXT:    br label %[[DEAD]]
+;
+  %a = call i32 @llvm.objectsize.i32.p21(ptr addrspace(21) null, i1 false, i1 false, i1 false)
+  ret i32 %a
+
+dead:
+  %x = call i32 @llvm.objectsize.i32.p21(ptr addrspace(21) null, i1 false, i1 false, i1 false)
+  br label %dead
+}

Copy link
Collaborator

@bjope bjope left a comment

Choose a reason for hiding this comment

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

LG. thanks!

@aengelke aengelke merged commit 5313d2e into llvm:main Aug 8, 2024
9 checks passed
@aengelke aengelke deleted the preisel-fix branch August 8, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants