Teach computeKnownBits to use new align attribute/metadata

[apilipenko]

Summary

Teach computeKnownBits to use align attribute on return values and align metadata on loads. This fixes the following bug: https://llvm.org/bugs/show_bug.cgi?id=25068

[reames]

LGTM w/comments addressed and one part separated into a follow on change as noted inline.

lib/Analysis/ValueTracking.cpp

1412

The changes to this function LGTM w/appropriate test cases.

2981

I'm not sure the GlobalVariable code is equivalent to the other case. I would prefer you separate the refactoring to use getAlignment into a separate patch. It seems slightly higher risk.

(My concern comes from the difference between BaseAlign and Align. I haven't looked at these carefully and I'm worried that your getAlignment is computing Align not BaseAlign.)

test/Transforms/InstCombine/assume-redundant.ll

52

Please add versions of this test for both load w/metadata and allocas.

lib/Analysis/ValueTracking.cpp

if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I->getOperand(0))) {
                               Depth, Q);
           break;
         }
       }
     }
   }
 }
 
-static unsigned getAlignment(Value *V, const DataLayout &DL) {
+static unsigned getAlignment(const Value *V, const DataLayout &DL) {
   unsigned Align = 0;
   if (auto *GO = dyn_cast<GlobalObject>(V)) {
     Align = GO->getAlignment();
     if (Align == 0) {
       if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
         Type *ObjectType = GVar->getType()->getElementType();
         if (ObjectType->isSized()) {
           // If the object is defined in the current Module, we'll be giving
           // it the preferred alignment. Otherwise, we have to assume that it
           // may only have the minimum ABI alignment.
           if (GVar->isStrongDefinitionForLinker())
             Align = DL.getPreferredAlignment(GVar);
           else
             Align = DL.getABITypeAlignment(ObjectType);
         }
       }
     }
-  } else if (Argument *A = dyn_cast<Argument>(V)) {
+  } else if (const Argument *A = dyn_cast<Argument>(V)) {
     Align = A->getType()->isPointerTy() ? A->getParamAlignment() : 0;
 
     if (!Align && A->hasStructRetAttr()) {
       // An sret parameter has at least the ABI alignment of the return type.
       Type *EltTy = cast<PointerType>(A->getType())->getElementType();
       if (EltTy->isSized())
         Align = DL.getABITypeAlignment(EltTy);
     }
+  } else if (const AllocaInst *AI = dyn_cast<AllocaInst>(V))
+    Align = AI->getAlignment();
+  else if (auto CS = ImmutableCallSite(V))
+    Align = CS.getAttributes().getParamAlignment(AttributeSet::ReturnIndex);
+  else if (const LoadInst *LI = dyn_cast<LoadInst>(V))
+    if (MDNode *MD = LI->getMetadata(LLVMContext::MD_align)) {
+      ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
+      Align = CI->getLimitedValue();
     }
+
   return Align;
 }
 
 /// Determine which bits of V are known to be either zero or one and return
 /// them in the KnownZero/KnownOne bit sets.
 ///
 /// NOTE: we cannot consider 'undef' to be "IsZero" here.  The problem is that
 /// we cannot optimize based on the assumption that it is zero without changing
if (!Ty->isSized())
     return false;
   
   APInt Offset(DL.getTypeStoreSizeInBits(VTy), 0);
   return isDereferenceableFromAttribute(V, Offset, Ty, DL, CtxI, DT, TLI);
 }
 
 static bool isAligned(const Value *Base, APInt Offset, unsigned Align,
                       const DataLayout &DL) {
-  APInt BaseAlign(Offset.getBitWidth(), 0);
-  if (const AllocaInst *AI = dyn_cast<AllocaInst>(Base))
-    BaseAlign = AI->getAlignment();
-  else if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(Base))
-    BaseAlign = GV->getAlignment();
-  else if (const Argument *A = dyn_cast<Argument>(Base))
-    BaseAlign = A->getParamAlignment();
-  else if (auto CS = ImmutableCallSite(Base))
-    BaseAlign = CS.getAttributes().getParamAlignment(AttributeSet::ReturnIndex);
-  else if (const LoadInst *LI = dyn_cast<LoadInst>(Base))
-    if (MDNode *MD = LI->getMetadata(LLVMContext::MD_align)) {
-      ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
-      BaseAlign = CI->getLimitedValue();
-    }
+  APInt BaseAlign(Offset.getBitWidth(), getAlignment(Base, DL));
 
   if (!BaseAlign) {
     Type *Ty = Base->getType()->getPointerElementType();
     BaseAlign = DL.getABITypeAlignment(Ty);
   }
 
   APInt Alignment(Offset.getBitWidth(), Align);
 

test/Transforms/InstCombine/assume-redundant.ll

for.body:                                         ; preds = %for.body, %entry
   %indvars.iv.next.1 = add nuw nsw i64 %indvars.iv.next, 1
   %exitcond.1 = icmp eq i64 %indvars.iv.next, 1599
   br i1 %exitcond.1, label %for.end, label %for.body
 
 for.end:                                          ; preds = %for.body
   ret void
 }
 
+declare align 8 i8* @get()
+
+; Check that redundant align assume is removed
+; CHECK-LABEL: @test
+; CHECK-NOT: call void @llvm.assume
+define void @test() {
+  %p = call align 8 i8* @get()
+  %ptrint = ptrtoint i8* %p to i64
+  %maskedptr = and i64 %ptrint, 7
+  %maskcond = icmp eq i64 %maskedptr, 0
+  call void @llvm.assume(i1 %maskcond)
+  ret void
+}
+
 ; Function Attrs: nounwind
 declare void @llvm.assume(i1) #1
 
 attributes #0 = { nounwind uwtable }
 attributes #1 = { nounwind }