Skip to content

Conversation

@andjo403
Copy link
Contributor

no changes in llvm-opt-benchmark as this fold is guarded by the option EnableKnowledgeRetention.

proof: https://alive2.llvm.org/ce/z/EyAUA4

@andjo403 andjo403 requested a review from nikic as a code owner January 14, 2025 19:05
@andjo403 andjo403 requested review from dtcxzyw and removed request for nikic January 14, 2025 19:05
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jan 14, 2025
@andjo403 andjo403 requested a review from nikic January 14, 2025 19:06
@llvmbot
Copy link
Member

llvmbot commented Jan 14, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Andreas Jonson (andjo403)

Changes

no changes in llvm-opt-benchmark as this fold is guarded by the option EnableKnowledgeRetention.

proof: https://alive2.llvm.org/ce/z/EyAUA4


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+6-5)
  • (modified) llvm/test/Transforms/InstCombine/assume.ll (+12-7)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index dd5a4ba5a4724a..4972b987c95608 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3253,12 +3253,13 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     // call void @llvm.assume(i1 %D)
     // into
     // call void @llvm.assume(i1 true) [ "align"(i32* [[A]], i64  Constant + 1)]
-    uint64_t AlignMask;
+    uint64_t AlignMask = 1;
     if (EnableKnowledgeRetention &&
-        match(IIOperand,
-              m_SpecificICmp(ICmpInst::ICMP_EQ,
-                             m_And(m_Value(A), m_ConstantInt(AlignMask)),
-                             m_Zero()))) {
+        (match(IIOperand,
+               m_SpecificICmp(ICmpInst::ICMP_EQ,
+                              m_And(m_Value(A), m_ConstantInt(AlignMask)),
+                              m_Zero())) ||
+         match(IIOperand, m_Not(m_Trunc(m_Value(A)))))) {
       if (isPowerOf2_64(AlignMask + 1)) {
         uint64_t Offset = 0;
         match(A, m_Add(m_Value(A), m_ConstantInt(Offset)));
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index 2d7bc49b6dcaee..c21f8457e82d14 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -35,13 +35,18 @@ define i32 @foo1(ptr %a) #0 {
 }
 
 define i32 @align_assume_trunc_cond(ptr %a) #0 {
-; CHECK-LABEL: @align_assume_trunc_cond(
-; CHECK-NEXT:    [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; CHECK-NEXT:    [[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
-; CHECK-NEXT:    [[TRUNC:%.*]] = trunc i64 [[PTRINT]] to i1
-; CHECK-NEXT:    [[MASKCOND:%.*]] = xor i1 [[TRUNC]], true
-; CHECK-NEXT:    tail call void @llvm.assume(i1 [[MASKCOND]])
-; CHECK-NEXT:    ret i32 [[T0]]
+; DEFAULT-LABEL: @align_assume_trunc_cond(
+; DEFAULT-NEXT:    [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
+; DEFAULT-NEXT:    [[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
+; DEFAULT-NEXT:    [[TRUNC:%.*]] = trunc i64 [[PTRINT]] to i1
+; DEFAULT-NEXT:    [[MASKCOND:%.*]] = xor i1 [[TRUNC]], true
+; DEFAULT-NEXT:    tail call void @llvm.assume(i1 [[MASKCOND]])
+; DEFAULT-NEXT:    ret i32 [[T0]]
+;
+; BUNDLES-LABEL: @align_assume_trunc_cond(
+; BUNDLES-NEXT:    [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
+; BUNDLES-NEXT:    call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 2) ]
+; BUNDLES-NEXT:    ret i32 [[T0]]
 ;
   %t0 = load i32, ptr %a, align 4
   %ptrint = ptrtoint ptr %a to i64

m_SpecificICmp(ICmpInst::ICMP_EQ,
m_And(m_Value(A), m_ConstantInt(AlignMask)),
m_Zero())) ||
match(IIOperand, m_Not(m_Trunc(m_Value(A)))))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think trunc match needs to be first. A failed pattern match is not guranteed to not modify the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can change the order but it is not possible to make a test that will fail as the m_and sub pattern will not execute unless it is an icmp instruction and in that case it can not also match the n_not pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case I think you're right that it won't be an issue either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LG

@andjo403 andjo403 merged commit 2570e35 into llvm:main Jan 15, 2025
8 checks passed
@andjo403 andjo403 deleted the truncInAssumeAlign branch January 15, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants