[Inline] Fix noalias addition on simplified instructions (PR50589)

[nikic]

Summary

When adding noalias/alias.scope metadata, we analyze the instructions of the original callee, and then place metadata on the corresponding inlined instructions in the caller as provided by VMap. However, this assumes that this actually a clone of the instruction, rather than the result of simplification. If simplification occurred, the instruction that VMap points to may not have any relationship as far as ModRef behavior is concerned.

Fix this by tracking simplified instructions during cloning and then only processing instructions that have not been simplified. This is done with an additional map form original to cloned instruction, into which we only insert if no simplification is performed. The mapping in VMap can then be compared to this map. If they're the same, the instruction hasn't been simplified. (I originally wanted to only track a set of simplified instructions, but that wouldn't work if the instruction only gets simplified afterwards, e.g. based on rewritten phis.)

Fixes https://bugs.llvm.org/show_bug.cgi?id=50589.

[jeroen.dobbelaere]

llvm/test/Transforms/Inline/pr50589.ll

21

Use '--match-full-lines' to ensure that the !noalias metadata is missing ? Or else maybe a CHECK-NOT: !noalias might also do the trick.

[nikic]

Add end of line match.

llvm/test/Transforms/Inline/pr50589.ll

21

It looks like --match-full-lines isn't compatible with update_test_checks output, so I added an explicit {{$}} to these two lines.

[jeroen.dobbelaere]

lgtm. Thanks !

llvm/include/llvm/Transforms/Utils/Cloning.h

struct ClonedCodeInfo {
   /// are in the entry block but are not a constant size.
   bool ContainsDynamicAllocas = false;
 
   /// All cloned call sites that have operand bundles attached are appended to
   /// this vector.  This vector may contain nulls or undefs if some of the
   /// originally inserted callsites were DCE'ed after they were cloned.
   std::vector<WeakTrackingVH> OperandBundleCallSites;
 
+  /// Like VMap, but maps only unsimplified instructions. Values in the map
+  /// may be dangling, it is only intended to be used via isSimplified(), to
+  /// check whether the main VMap mapping involves simplification or not.
+  DenseMap<const Value *, const Value *> OrigVMap;
+
   ClonedCodeInfo() = default;
+
+  bool isSimplified(const Value *From, const Value *To) const {
+    return OrigVMap.lookup(From) != To;
+  }
 };
 
 /// Return a copy of the specified basic block, but without
 /// embedding the block into a particular function.  The block returned is an
 /// exact copy of the specified basic block, without any remapping having been
 /// performed.  Because of this, this is only suitable for applications where
 /// the basic block will be inserted into the same function that it was cloned
 /// from (loop unrolling would use this, for example).

llvm/lib/Transforms/Utils/CloneFunction.cpp

for (BasicBlock::const_iterator II = StartingInst, IE = --BB->end(); II != IE;
     }
 
     if (II->hasName())
       NewInst->setName(II->getName() + NameSuffix);
     VMap[&*II] = NewInst; // Add instruction map to value.
     NewBB->getInstList().push_back(NewInst);
     hasCalls |= (isa<CallInst>(II) && !isa<DbgInfoIntrinsic>(II));
 
-    if (CodeInfo)
+    if (CodeInfo) {
+      CodeInfo->OrigVMap[&*II] = NewInst;
       if (auto *CB = dyn_cast<CallBase>(&*II))
         if (CB->hasOperandBundles())
           CodeInfo->OperandBundleCallSites.push_back(NewInst);
+    }
 
     if (const AllocaInst *AI = dyn_cast<AllocaInst>(II)) {
       if (isa<ConstantInt>(AI->getArraySize()))
         hasStaticAllocas = true;
       else
         hasDynamicAllocas = true;
     }
   }
void PruningFunctionCloner::CloneBlock(
 
   if (!TerminatorDone) {
     Instruction *NewInst = OldTI->clone();
     if (OldTI->hasName())
       NewInst->setName(OldTI->getName() + NameSuffix);
     NewBB->getInstList().push_back(NewInst);
     VMap[OldTI] = NewInst; // Add instruction map to value.
 
-    if (CodeInfo)
+    if (CodeInfo) {
+      CodeInfo->OrigVMap[OldTI] = NewInst;
       if (auto *CB = dyn_cast<CallBase>(OldTI))
         if (CB->hasOperandBundles())
           CodeInfo->OperandBundleCallSites.push_back(NewInst);
+    }
 
     // Recursively clone any reachable successor blocks.
     append_range(ToClone, successors(BB->getTerminator()));
   }
 
   if (CodeInfo) {
     CodeInfo->ContainsCalls |= hasCalls;
     CodeInfo->ContainsDynamicAllocas |= hasDynamicAllocas;

llvm/lib/Transforms/Utils/InlineFunction.cpp

void ScopedAliasMetadataDeepCloner::remap(Function::iterator FStart,
   }
 }
 
 /// If the inlined function has noalias arguments,
 /// then add new alias scopes for each noalias argument, tag the mapped noalias
 /// parameters with noalias metadata specifying the new scope, and tag all
 /// non-derived loads, stores and memory intrinsics with the new alias scopes.
 static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap,
-                                  const DataLayout &DL, AAResults *CalleeAAR) {
+                                  const DataLayout &DL, AAResults *CalleeAAR,
+                                  ClonedCodeInfo &InlinedFunctionInfo) {
   if (!EnableNoAliasConversion)
     return;
 
   const Function *CalledFunc = CB.getCalledFunction();
   SmallVector<const Argument *, 4> NoAliasArgs;
 
   for (const Argument &Arg : CalledFunc->args())
     if (CB.paramHasAttr(Arg.getArgNo(), Attribute::NoAlias) && !Arg.use_empty())
static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap,
   // instructions, add the alias scope metadata.
   for (ValueToValueMapTy::iterator VMI = VMap.begin(), VMIE = VMap.end();
        VMI != VMIE; ++VMI) {
     if (const Instruction *I = dyn_cast<Instruction>(VMI->first)) {
       if (!VMI->second)
         continue;
 
       Instruction *NI = dyn_cast<Instruction>(VMI->second);
-      if (!NI)
+      if (!NI || InlinedFunctionInfo.isSimplified(I, NI))
         continue;
 
       bool IsArgMemOnlyCall = false, IsFuncCall = false;
       SmallVector<const Value *, 2> PtrArgs;
 
       if (const LoadInst *LI = dyn_cast<LoadInst>(I))
         PtrArgs.push_back(LI->getPointerOperand());
       else if (const StoreInst *SI = dyn_cast<StoreInst>(I))
Function::iterator FirstNewBlock;
     fixupLineNumbers(Caller, FirstNewBlock, &CB,
                      CalledFunc->getSubprogram() != nullptr);
 
     // Now clone the inlined noalias scope metadata.
     SAMetadataCloner.clone();
     SAMetadataCloner.remap(FirstNewBlock, Caller->end());
 
     // Add noalias metadata if necessary.
-    AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR);
+    AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR, InlinedFunctionInfo);
 
     // Clone return attributes on the callsite into the calls within the inlined
     // function which feed into its return value.
     AddReturnAttributes(CB, VMap);
 
     // Propagate metadata on the callsite if necessary.
     PropagateCallSiteMetadata(CB, FirstNewBlock, Caller->end());
 

llvm/test/Transforms/Inline/pr50589.ll

 ; CHECK-NEXT:    store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2:%.*]], align 2
 ; CHECK-NEXT:    ret <2 x i8> [[RET]]
 ;
   %ret = call <2 x i8> @llvm.masked.load.v2i8(<2 x i8>* %ptr1, i32 1, <2 x i1> %mask, <2 x i8> %passthru)
   store <2 x i8> zeroinitializer, <2 x i8>* %ptr2
   ret <2 x i8> %ret
 }
 
-; TODO: The load should not have !noalias.
+; The load should not have !noalias.
 define void @caller1(<2 x i8>* %ptr1, <2 x i8>* %ptr2) {
 ; CHECK-LABEL: @caller1(
-; CHECK-NEXT:    [[PASSTHRU:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !noalias !0
+; CHECK-NEXT:    [[PASSTHRU:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2{{$}}
 ; CHECK-NEXT:    call void @llvm.experimental.noalias.scope.decl(metadata [[META0:![0-9]+]])
 ; CHECK-NEXT:    store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2]], align 2, !alias.scope !0
 ; CHECK-NEXT:    ret void
 ;
   %passthru = load <2 x i8>, <2 x i8>* %ptr2
   call <2 x i8> @callee1(<2 x i8>* %ptr1, <2 x i8>* %ptr2, <2 x i1> zeroinitializer, <2 x i8> %passthru)
   ret void
 }
 
 define <2 x i8> @callee2(<2 x i8>* %ptr1, <2 x i8>* noalias %ptr2, <2 x i1> %mask) {
 ; CHECK-LABEL: @callee2(
 ; CHECK-NEXT:    [[PASSTHRU:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2
 ; CHECK-NEXT:    [[RET:%.*]] = call <2 x i8> @llvm.masked.load.v2i8.p0v2i8(<2 x i8>* [[PTR1:%.*]], i32 1, <2 x i1> [[MASK:%.*]], <2 x i8> [[PASSTHRU]])
 ; CHECK-NEXT:    store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2]], align 2
 ; CHECK-NEXT:    ret <2 x i8> [[RET]]
 ;
   %passthru = load <2 x i8>, <2 x i8>* %ptr2
   %ret = call <2 x i8> @llvm.masked.load.v2i8(<2 x i8>* %ptr1, i32 1, <2 x i1> %mask, <2 x i8> %passthru)
   store <2 x i8> zeroinitializer, <2 x i8>* %ptr2
   ret <2 x i8> %ret
 }
 
-; TODO: The load should not have !noalias.
+; The load should not have !noalias.
 define void @caller2(<2 x i8>* %ptr1, <2 x i8>* %ptr2) {
 ; CHECK-LABEL: @caller2(
 ; CHECK-NEXT:    call void @llvm.experimental.noalias.scope.decl(metadata [[META3:![0-9]+]])
-; CHECK-NEXT:    [[PASSTHRU_I:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !alias.scope !3, !noalias !3
+; CHECK-NEXT:    [[PASSTHRU_I:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !alias.scope !3{{$}}
 ; CHECK-NEXT:    store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2]], align 2, !alias.scope !3
 ; CHECK-NEXT:    ret void
 ;
   call <2 x i8> @callee2(<2 x i8>* %ptr1, <2 x i8>* %ptr2, <2 x i1> zeroinitializer)
   ret void
 }
 
 declare <2 x i8> @llvm.masked.load.v2i8(<2 x i8>*, i32, <2 x i1>, <2 x i8>)