Update location; exclude invariant (and simple) loads from memory inst scheduler data list.

Add volatile test.

Author: mjbedy

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

@@ -21412,7 +21412,18 @@ void BoUpSLP::BlockScheduling::initScheduleData(Instruction *FromI,
            "new ScheduleData already in scheduling region");
     SD->init(SchedulingRegionID, I);
 
+    auto CanIgnoreLoad = [](const Instruction *I) {
+      const LoadInst *LI = dyn_cast<LoadInst>(I);
+      // If there is a simple load marked as invariant, we can ignore it.
+      // But, in the (unlikely) case of non-simple invariant load,
+      // we should not ignore it.
+      return LI && LI->isSimple() &&
+             LI->getMetadata(LLVMContext::MD_invariant_load);
+    };
+
     if (I->mayReadOrWriteMemory() &&
+        // Simple InvariantLoad does not depend on other memory accesses.
+        !CanIgnoreLoad(I) &&
         (!isa<IntrinsicInst>(I) ||
          (cast<IntrinsicInst>(I)->getIntrinsicID() != Intrinsic::sideeffect &&
           cast<IntrinsicInst>(I)->getIntrinsicID() !=
@@ -21623,17 +21634,6 @@ void BoUpSLP::BlockScheduling::calculateDependencies(
       }
     }
 
-    // Helper to detect loads marked with !invariant.load metadata. Such loads
-    // are defined to read from memory that never changes for the lifetime of
-    // the program; any store to the same location would be UB. Therefore we
-    // can conservatively treat an invariant load and any store as non-aliasing
-    // for scheduling/dep purposes and skip creating a dependency edge.
-    auto IsInvariantLoad = [](const Instruction *I) {
-      if (const auto *LI = dyn_cast<LoadInst>(I))
-        return LI->getMetadata(LLVMContext::MD_invariant_load) != nullptr;
-      return false;
-    };
-
     // Handle the memory dependencies (if any).
     ScheduleData *NextLoadStore = BundleMember->getNextLoadStore();
     if (!NextLoadStore)
@@ -21647,15 +21647,10 @@ void BoUpSLP::BlockScheduling::calculateDependencies(
     unsigned DistToSrc = 1;
     bool IsNonSimpleSrc = !SrcLoc.Ptr || !isSimple(SrcInst);
 
-    if (IsInvariantLoad(SrcInst))
-      return; // Invariant load cannot have memory dependencies.
-
     for (ScheduleData *DepDest = NextLoadStore; DepDest;
          DepDest = DepDest->getNextLoadStore()) {
       assert(isInSchedulingRegion(*DepDest) && "Expected to be in region");
 
-      Instruction *DestInst = DepDest->getInst();
-
       // We have two limits to reduce the complexity:
       // 1) AliasedCheckLimit: It's a small limit to reduce calls to
       //    SLP->isAliased (which is the expensive part in this loop).
@@ -21664,8 +21659,7 @@ void BoUpSLP::BlockScheduling::calculateDependencies(
       //    It's important for the loop break condition (see below) to
       //    check this limit even between two read-only instructions.
       if (DistToSrc >= MaxMemDepDistance ||
-          (!IsInvariantLoad(DestInst) && // Cannot have memory deps.
-           (SrcMayWrite || DepDest->getInst()->mayWriteToMemory()) &&
+          ((SrcMayWrite || DepDest->getInst()->mayWriteToMemory()) &&
            (IsNonSimpleSrc || NumAliased >= AliasedCheckLimit ||
             SLP->isAliased(SrcLoc, SrcInst, DepDest->getInst())))) {
 

llvm/test/Transforms/SLPVectorizer/AMDGPU/invariant-load-no-alias-store.ll

@@ -73,6 +73,45 @@ entry:
   ret void
 }
 
+define void @voltest(ptr addrspace(1) %base, ptr addrspace(1) %otherA, ptr addrspace(1) %otherB) #0 {
+; CHECK-LABEL: define void @voltest(
+; CHECK-SAME: ptr addrspace(1) [[BASE:%.*]], ptr addrspace(1) [[OTHERA:%.*]], ptr addrspace(1) [[OTHERB:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[P0:%.*]] = getelementptr half, ptr addrspace(1) [[BASE]], i32 0
+; CHECK-NEXT:    [[P1:%.*]] = getelementptr half, ptr addrspace(1) [[BASE]], i32 1
+; CHECK-NEXT:    [[A0PTR:%.*]] = getelementptr half, ptr addrspace(1) [[OTHERA]], i32 0
+; CHECK-NEXT:    [[B0PTR:%.*]] = getelementptr half, ptr addrspace(1) [[OTHERB]], i32 0
+; CHECK-NEXT:    [[A0:%.*]] = load volatile half, ptr addrspace(1) [[A0PTR]], align 2, !invariant.load [[META0]]
+; CHECK-NEXT:    [[B0:%.*]] = load volatile half, ptr addrspace(1) [[B0PTR]], align 2, !invariant.load [[META0]]
+; CHECK-NEXT:    [[ADD0:%.*]] = fadd reassoc half [[A0]], [[B0]]
+; CHECK-NEXT:    store half [[ADD0]], ptr addrspace(1) [[P0]], align 2
+; CHECK-NEXT:    [[A1PTR:%.*]] = getelementptr half, ptr addrspace(1) [[OTHERA]], i32 1
+; CHECK-NEXT:    [[B1PTR:%.*]] = getelementptr half, ptr addrspace(1) [[OTHERB]], i32 1
+; CHECK-NEXT:    [[A1:%.*]] = load volatile half, ptr addrspace(1) [[A1PTR]], align 2, !invariant.load [[META0]]
+; CHECK-NEXT:    [[B1:%.*]] = load volatile half, ptr addrspace(1) [[B1PTR]], align 2, !invariant.load [[META0]]
+; CHECK-NEXT:    [[ADD1:%.*]] = fadd reassoc half [[A1]], [[B1]]
+; CHECK-NEXT:    store half [[ADD1]], ptr addrspace(1) [[P1]], align 2
+; CHECK-NEXT:    ret void
+;
+entry:
+  %p0 = getelementptr half, ptr addrspace(1) %base, i32 0
+  %p1 = getelementptr half, ptr addrspace(1) %base, i32 1
+  ; First pair of invariant loads from otherA.
+  %A0PTR = getelementptr half, ptr addrspace(1) %otherA, i32 0
+  %B0PTR = getelementptr half, ptr addrspace(1) %otherB, i32 0
+  %A0 = load volatile half, ptr addrspace(1) %A0PTR, align 2, !invariant.load !0
+  %B0 = load volatile half, ptr addrspace(1) %B0PTR, align 2, !invariant.load !0
+  %add0 = fadd reassoc half %A0, %B0
+  store half %add0, ptr addrspace(1) %p0, align 2
+  %A1PTR = getelementptr half, ptr addrspace(1) %otherA, i32 1
+  %B1PTR = getelementptr half, ptr addrspace(1) %otherB, i32 1
+  %A1 = load volatile half, ptr addrspace(1) %A1PTR, align 2, !invariant.load !0
+  %B1 = load volatile half, ptr addrspace(1) %B1PTR, align 2, !invariant.load !0
+  %add1 = fadd reassoc half %A1, %B1
+  store half %add1, ptr addrspace(1) %p1, align 2
+  ret void
+}
+
 
 attributes #0 = { nounwind }