[LV] Interleaved access vectorization: fix computing new alias info

[anemet]

Summary

As a new access is generated spanning across multiple fields we need to
propagate alias info from all the fields to form the most generic alias info.

rdar://35602528

[anemet]

@hfinkel, @mkuper, could you guys look at this please. We are experiencing a serious miscompile due to this bug. Thanks!

[mkuper]

lib/Transforms/Vectorize/LoopVectorize.cpp

952 ↗

(On Diff #124785)

InnerLoopVectorizer::addMetadata() also calls addNewMetadata(), while this doesn't.
Is that intentional?

[anemet]

lib/Transforms/Vectorize/LoopVectorize.cpp

952 ↗

(On Diff #124785)

Yes, it is intentional. addNewMetadata adds noalias metadata based on the inserted memchecks. We need a way to aggregate that information from several accesses as well.

I can add a FIXME that we don't propagate this information currently. This is conservatively correct.

[mkuper]

LGTM

lib/Transforms/Vectorize/LoopVectorize.cpp

952 ↗

(On Diff #124785)

The FIXME would probably be good.

llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp

for (auto I : Members)
         return I.first - SmallestKey;
 
     llvm_unreachable("InterleaveGroup contains no such member");
   }
 
   Instruction *getInsertPos() const { return InsertPos; }
   void setInsertPos(Instruction *Inst) { InsertPos = Inst; }
 
+  /// Add metadata (e.g. alias info) from the instructions in this group to \p
+  /// NewInst.
+  ///
+  /// FIXME: this function currently does not add noalias metadata a'la
+  /// addNewMedata.  To do that we need to compute the intersection of the
+  /// noalias info from all members.
+  void addMetadata(Instruction *NewInst) const {
+    SmallVector<Value *, 4> VL;
+    std::transform(Members.begin(), Members.end(), std::back_inserter(VL),
+                   [](std::pair<int, Instruction *> p) { return p.second; });
+    propagateMetadata(NewInst, VL);
+  }
+
 private:
   unsigned Factor; // Interleave Factor.
   bool Reverse;
   unsigned Align;
   DenseMap<int, Instruction *> Members;
   int SmallestKey = 0;
   int LargestKey = 0;
 
void InnerLoopVectorizer::vectorizeInterleaveGroup(Instruction *Instr) {
 
   // Vectorize the interleaved load group.
   if (isa<LoadInst>(Instr)) {
     // For each unroll part, create a wide load for the group.
     SmallVector<Value *, 2> NewLoads;
     for (unsigned Part = 0; Part < UF; Part++) {
       auto *NewLoad = Builder.CreateAlignedLoad(
           NewPtrs[Part], Group->getAlignment(), "wide.vec");
-      addMetadata(NewLoad, Instr);
+      Group->addMetadata(NewLoad);
       NewLoads.push_back(NewLoad);
     }
 
     // For each member in the group, shuffle out the appropriate data from the
     // wide loads.
     for (unsigned I = 0; I < InterleaveFactor; ++I) {
       Instruction *Member = Group->getMember(I);
 
for (unsigned Part = 0; Part < UF; Part++) {
 
     // Interleave the elements in the wide vector.
     Constant *IMask = createInterleaveMask(Builder, VF, InterleaveFactor);
     Value *IVec = Builder.CreateShuffleVector(WideVec, UndefVec, IMask,
                                               "interleaved.vec");
 
     Instruction *NewStoreInstr =
         Builder.CreateAlignedStore(IVec, NewPtrs[Part], Group->getAlignment());
-    addMetadata(NewStoreInstr, Instr);
+
+    Group->addMetadata(NewStoreInstr);
   }
 }
 
 void InnerLoopVectorizer::vectorizeMemoryInstruction(Instruction *Instr,
                                                      VectorParts *BlockInMask) {
   // Attempt to issue a wide load.
   LoadInst *LI = dyn_cast<LoadInst>(Instr);
   StoreInst *SI = dyn_cast<StoreInst>(Instr);

llvm/trunk/test/Transforms/LoopVectorize/interleaved-accesses-alias.ll

+; RUN: opt -S -loop-vectorize -force-vector-width=2 -force-vector-interleave=1 -enable-interleaved-mem-accesses=true < %s | FileCheck %s
+
+; When merging two stores with interleaved access vectorization, make sure we
+; propagate the alias information from all scalar stores to form the most
+; generic alias info.
+
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+target triple = "arm64-apple-ios5.0.0"
+
+%struct.Vec4r = type { double, double, double, double }
+%struct.Vec2r = type { double, double }
+
+define void @foobar(%struct.Vec4r* nocapture readonly %p, i32 %i)
+{
+entry:
+  %cp = alloca [20 x %struct.Vec2r], align 8
+  %0 = bitcast [20 x %struct.Vec2r]* %cp to i8*
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  %arraydecay = getelementptr inbounds [20 x %struct.Vec2r], [20 x %struct.Vec2r]* %cp, i64 0, i64 0
+  call void @g(%struct.Vec2r* nonnull %arraydecay) #4
+  ret void
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %x = getelementptr inbounds %struct.Vec4r, %struct.Vec4r* %p, i64 %indvars.iv, i32 0
+  %1 = load double, double* %x, align 8, !tbaa !3
+  %mul = fmul double %1, 2.000000e+00
+  %x4 = getelementptr inbounds [20 x %struct.Vec2r], [20 x %struct.Vec2r]* %cp, i64 0, i64 %indvars.iv, i32 0
+
+; The new store should alias any double rather than one of the fields of Vec2r.
+; CHECK: store <4 x double> {{.*}} !tbaa ![[STORE_TBAA:[0-9]+]]
+; CHECK-DAG: ![[DOUBLE_TBAA:[0-9]+]] = !{!"double", !{{[0-9+]}}, i64 0}
+; CHECK-DAG: ![[STORE_TBAA]] = !{![[DOUBLE_TBAA]], ![[DOUBLE_TBAA]], i64 0}
+  store double %mul, double* %x4, align 8, !tbaa !8
+  %y = getelementptr inbounds %struct.Vec4r, %struct.Vec4r* %p, i64 %indvars.iv, i32 1
+  %2 = load double, double* %y, align 8, !tbaa !10
+  %mul7 = fmul double %2, 3.000000e+00
+  %y10 = getelementptr inbounds [20 x %struct.Vec2r], [20 x %struct.Vec2r]* %cp, i64 0, i64 %indvars.iv, i32 1
+  store double %mul7, double* %y10, align 8, !tbaa !11
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 4
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}
+
+declare void @g(%struct.Vec2r*)
+
+!llvm.module.flags = !{!0, !1}
+!llvm.ident = !{!2}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"PIC Level", i32 2}
+!2 = !{!"clang version 6.0.0 (trunk 319007) (llvm/trunk 319324)"}
+!3 = !{!4, !5, i64 0}
+!4 = !{!"Vec4r", !5, i64 0, !5, i64 8, !5, i64 16, !5, i64 24}
+!5 = !{!"double", !6, i64 0}
+!6 = !{!"omnipotent char", !7, i64 0}
+!7 = !{!"Simple C/C++ TBAA"}
+!8 = !{!9, !5, i64 0}
+!9 = !{!"Vec2r", !5, i64 0, !5, i64 8}
+!10 = !{!4, !5, i64 8}
+!11 = !{!9, !5, i64 8}