[ValueTracking] Add tracking of the alignment assume bundle

[scui]

Summary

This patch is to add the support of the value tracking of the alignment assume bundle.

[etiotto]

Look OK to me but you should also add other reviewers.

[scui]

Anyone has any comments for this patch? Any comments or suggestions are appreciated.

[efriedma]

What does the third operand of an align bundle mean? It doesn't seem to be documented in LangRef.

[jdoerfert]

llvm/lib/Analysis/ValueTracking.cpp

737

You don't want to look for the bundle yourself. Use

SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::Align};
if (RetainedKnowledge RK = getKnowledgeValidInContext(V, AttrKinds, CtxI, DT, AC)) {
  ...
}

instead to account for all the interesting cases, e.g., multiple align bundles. Also, use braces for non trivial conditionals. I don't think the return is what you want here, other deductions might still apply. Also, I think we have alignment bundle -> Align information in the AlignmentFromAssumptions.cpp pass. We should create a helper function so this is less magic.

[scui]

In D88669#2328334, @efriedma wrote:

What does the third operand of an align bundle mean? It doesn't seem to be documented in LangRef.

You are right. My understanding is that align bundle of (p, 32, 24) assumes (p+24) is 32-byte aligned. - typo corrected.

[jdoerfert]

In D88669#2328607, @scui wrote:

In D88669#2328334, @efriedma wrote:

What does the third operand of an align bundle mean? It doesn't seem to be documented in LangRef.

You are right. My understanding is that align bundle of (p, 32, 24) assumes (p-24) is 32-byte aligned.

That's correct. It matches the old llvm.assume alignment encoding via instructions.
I mentioned before that we should probably just create a gep with the offset instead of keeping it as a third operand but as long as we do we should have a helper to extract it from the bundle.

[scui]

Rewrite the code using getKnowledgeValidInContext. The code is much cleaner than the previous version, but the result computed is less accurate if the size of the alignment bundle is three. I'll leave the improvement for the future, together with the improvement of value tracking of GEP (an example is that, with the future improvement, we can fold away the branch "br i1 %3, ...") in f2 of the added case assume-align.ll).

[scui]

llvm/lib/Analysis/ValueTracking.cpp

737

You don't want to look for the bundle yourself. Use

SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::Align};
if (RetainedKnowledge RK = getKnowledgeValidInContext(V, AttrKinds, CtxI, DT, AC)) {
  ...
}

instead to account for all the interesting cases, e.g., multiple align bundles. Also, use braces for non trivial conditionals. I don't think the return is what you want here, other deductions might still apply. Also, I think we have alignment bundle -> Align information in the AlignmentFromAssumptions.cpp pass. We should create a helper function so this is less magic.

Thanks for the comments and suggestions. I have rewrite the code using getKnowledgeValidInContext. Please take a look. Thanks

[jdoerfert]

Some nits you should address, otherwise LGTM.

In D88669#2330433, @scui wrote:

Rewrite the code using getKnowledgeValidInContext. The code is much cleaner than the previous version, but the result computed is less accurate if the size of the alignment bundle is three. I'll leave the improvement for the future, together with the improvement of value tracking of GEP (an example is that, with the future improvement, we can fold away the branch "br i1 %3, ...") in f2 of the added case assume-align.ll).

FWIW, we should either not encode the offset but use a GEP, or make getKnowledgeValidInContext aware. Either should be a follow up.

llvm/lib/Analysis/ValueTracking.cpp

681

I don't think the parenthesis part in the comment should be here.

llvm/test/Transforms/InstCombine/assume-align.ll

48

Instead of the comment above, add a TODO/FIXME here describing the two potential solutions to this problem.

[jdoerfert]

Reverse ping. More comments (nits) and notice that we should merge this, D89830 reimplemented it in the meantime, see my comment there too.

llvm/lib/Analysis/ValueTracking.cpp

676

No need for the vector, replace it below with {Attribute::Alignment}.

In D89830 @arichardson uses Log2_32 instead of countTrailingZeros. I think that makes sense since we have a power of 2.

[scui]

Updated to address review comments: I adopted the use of Log2_32 from D89830, removed the parenthesis comment, and added a TODO comment in the test case.

[scui]

In D88669#2345905, @jdoerfert wrote:

Reverse ping. More comments (nits) and notice that we should merge this, D89830 reimplemented it in the meantime, see my comment there too.

Thanks for the comments. I don’t have commit access. If we commit this first, I will need your help to commit for me (“Shimin Cui <[e-mail]>”). Otherwise @arichardson can merge this with D89830 to commit both.

llvm/lib/Analysis/ValueTracking.cpp

static void computeKnownBitsFromAssume(const Value *V, KnownBits &Known,
                                        unsigned Depth, const Query &Q) {
   // Use of assumptions is context-sensitive. If we don't have a context, we
   // cannot use them!
   if (!Q.AC || !Q.CxtI)
     return;
 
   unsigned BitWidth = Known.getBitWidth();
 
+  // Refine Known set if the pointer alignment is set by assume bundles (only
+  // minimum alignment is set if the size of the bundle is three).
+  if (Q.CxtI && V->getType()->isPointerTy()) {
+    SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::Alignment};
+    if (RetainedKnowledge RK =
+            getKnowledgeValidInContext(V, AttrKinds, Q.CxtI, Q.DT, Q.AC)) {
+      Known.Zero.setLowBits(countTrailingZeros(RK.ArgValue));
+    }
+  }
+
   // Note that the patterns below need to be kept in sync with the code
   // in AssumptionCache::updateAffectedValues.
 
   for (auto &AssumeVH : Q.AC->assumptionsFor(V)) {
     if (!AssumeVH)
       continue;
     CallInst *I = cast<CallInst>(AssumeVH);
     assert(I->getParent()->getParent() == Q.CxtI->getParent()->getParent() &&
for (auto &AssumeVH : Q.AC->assumptionsFor(V)) {
     uint64_t C;
     switch (Cmp->getPredicate()) {
     default:
       break;
     case ICmpInst::ICMP_EQ:
       // assume(v = a)
       if (match(Cmp, m_c_ICmp(Pred, m_V, m_Value(A))) &&
           isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
         KnownBits RHSKnown =
             computeKnownBits(A, Depth+1, Query(Q, I)).anyextOrTrunc(BitWidth);
         Known.Zero |= RHSKnown.Zero;
         Known.One  |= RHSKnown.One;
       // assume(v & b = a)
       } else if (match(Cmp,
                        m_c_ICmp(Pred, m_c_And(m_V, m_Value(B)), m_Value(A))) &&
                  isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
         KnownBits RHSKnown =

llvm/test/Analysis/BasicAA/featuretest.ll

 ; NO_ASSUME-NEXT:    [[C1:%.*]] = getelementptr i32, i32* [[A:%.*]], i64 1
 ; NO_ASSUME-NEXT:    [[C:%.*]] = bitcast i32* [[C1]] to i8*
 ; NO_ASSUME-NEXT:    store i8 42, i8* [[C]], align 1
 ; NO_ASSUME-NEXT:    ret i32 0
 ;
 ; USE_ASSUME-LABEL: @gep_distance_test3(
 ; USE_ASSUME-NEXT:    [[C1:%.*]] = getelementptr i32, i32* [[A:%.*]], i64 1
 ; USE_ASSUME-NEXT:    [[C:%.*]] = bitcast i32* [[C1]] to i8*
-; USE_ASSUME-NEXT:    store i8 42, i8* [[C]], align 1
+; USE_ASSUME-NEXT:    store i8 42, i8* [[C]], align 4
 ; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "dereferenceable"(i32* [[A]], i64 4), "nonnull"(i32* [[A]]), "align"(i32* [[A]], i64 4) ]
 ; USE_ASSUME-NEXT:    ret i32 0
 ;
   %X = load i32, i32* %A
   %B = bitcast i32* %A to i8*
   %C = getelementptr i8, i8* %B, i64 4
   store i8 42, i8* %C
   %Y = load i32, i32* %A

llvm/test/Analysis/ValueTracking/assume-queries-counter.ll

 ; REQUIRES: asserts
 
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine --debug-counter=assume-queries-counter-skip=0,assume-queries-counter-count=1 -S | FileCheck %s --check-prefixes=SAME,COUNTER1
-; RUN: opt < %s -instcombine --debug-counter=assume-queries-counter-skip=1,assume-queries-counter-count=2 -S | FileCheck %s --check-prefixes=SAME,COUNTER2
-; RUN: opt < %s -instcombine --debug-counter=assume-queries-counter-skip=2,assume-queries-counter-count=1 -S | FileCheck %s --check-prefixes=SAME,COUNTER3
+; RUN: opt < %s -instcombine --debug-counter=assume-queries-counter-skip=1,assume-queries-counter-count=4 -S | FileCheck %s --check-prefixes=SAME,COUNTER2
+; RUN: opt < %s -instcombine --debug-counter=assume-queries-counter-skip=6,assume-queries-counter-count=1 -S | FileCheck %s --check-prefixes=SAME,COUNTER3
 
 declare i1 @get_val()
 declare void @llvm.assume(i1)
 
 define dso_local i1 @test1(i32* readonly %0) {
 ; COUNTER1-LABEL: @test1(
 ; COUNTER1-NEXT:    call void @llvm.assume(i1 true) [ "nonnull"(i32* [[TMP0:%.*]]) ]
 ; COUNTER1-NEXT:    ret i1 false

llvm/test/Transforms/InstCombine/assume-align.ll

+; RUN: opt -S -passes=instcombine,simplify-cfg < %s 2>&1 | FileCheck %s
+
+declare void @llvm.assume(i1 noundef)
+
+define void @f1(i8* %a) {
+; CHECK-LABEL: @f1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[PTR:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i64 4
+; CHECK-NEXT:    [[TMP0:%.*]] = ptrtoint i8* [[PTR]] to i64
+; CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 3
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 0
+; CHECK-NEXT:    br i1 [[TMP2]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "align"(i8* [[PTR]], i64 4) ]
+; CHECK-NEXT:    [[TMP3:%.*]] = bitcast i8* [[PTR]] to i32*
+; CHECK-NEXT:    store i32 4, i32* [[TMP3]], align 4
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %ptr = getelementptr inbounds i8, i8* %a, i64 4
+  %0 = ptrtoint i8* %ptr to i64
+  %1 = and i64 %0, 3
+  %2 = icmp eq i64 %1, 0
+  br i1 %2, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  call void @llvm.assume(i1 true) [ "align"(i8* %ptr, i64 4) ]
+  %3 = ptrtoint i8* %ptr to i64
+  %4 = and i64 %3, 3
+  %5 = icmp eq i64 %4, 0
+  br i1 %5, label %if.then1, label %if.else1
+
+if.then1:                                         ; preds = %if.then
+  %6 = bitcast i8* %ptr to i32*
+  store i32 4, i32* %6, align 4
+  br label %if.end
+
+if.else1:                                         ; preds = %if.then
+  store i8 1, i8* %ptr, align 1
+  br label %if.end
+
+if.end:                                           ; preds = %if.then1, %if.else1, %entry
+  ret void
+}
+
+define void @f2(i8* %a) {
+; CHECK-LABEL: @f2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "align"(i8* [[A:%.*]], i64 32, i32 24) ]
+; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds i8, i8* [[A]], i64 8
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[TMP0]] to i64
+; CHECK-NEXT:    [[TMP2:%.*]] = and i64 [[TMP1]], 8
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp eq i64 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[TMP3]], label [[IF_THEN1:%.*]], label [[IF_ELSE1:%.*]]
+;
+entry:
+  call void @llvm.assume(i1 true) [ "align"(i8* %a, i64 32, i32 24) ]
+  %0 = getelementptr inbounds i8, i8* %a, i64 8
+  %1 = ptrtoint i8* %0 to i64
+  %2 = and i64 %1, 15
+  %3 = icmp eq i64 %2, 0
+  br i1 %3, label %if.then, label %if.else
+
+if.then:                                          ; preds = %entry
+  %4 = bitcast i8* %0 to i64*
+  store i64 16, i64* %4, align 4
+  br label %if.end
+
+if.else:                                          ; preds = %entry
+  store i8 1, i8* %0, align 1
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  ret void
+}
+