[llvm] cf18ec4 - [GVN] Check load type in select PRE

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-03-14T12:46:54+01:00
New Revision: cf18ec445dcef8061fb523817ab5c36d1d08ceca

URL: https://github.com/llvm/llvm-project/commit/cf18ec445dcef8061fb523817ab5c36d1d08ceca
DIFF: https://github.com/llvm/llvm-project/commit/cf18ec445dcef8061fb523817ab5c36d1d08ceca.diff

LOG: [GVN] Check load type in select PRE

This is no longer implicitly guaranteed with opaque pointers.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/GVN.cpp
    llvm/test/Transforms/GVN/opaque-ptr.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index b71fcacb83eee..b3c19eeedf4f0 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -922,11 +922,12 @@ ConstructSSAForLoadSet(LoadInst *Load,
   return SSAUpdate.GetValueInMiddleOfBlock(Load->getParent());
 }
 
-static LoadInst *findDominatingLoad(Value *Ptr, SelectInst *Sel,
+static LoadInst *findDominatingLoad(Value *Ptr, Type *LoadTy, SelectInst *Sel,
                                     DominatorTree &DT) {
   for (Value *U : Ptr->users()) {
     auto *LI = dyn_cast<LoadInst>(U);
-    if (LI && LI->getParent() == Sel->getParent() && DT.dominates(LI, Sel))
+    if (LI && LI->getType() == LoadTy && LI->getParent() == Sel->getParent() &&
+        DT.dominates(LI, Sel))
       return LI;
   }
   return nullptr;
@@ -975,10 +976,10 @@ Value *AvailableValue::MaterializeAdjustedValue(LoadInst *Load,
   } else if (isSelectValue()) {
     // Introduce a new value select for a load from an eligible pointer select.
     SelectInst *Sel = getSelectValue();
-    LoadInst *L1 =
-        findDominatingLoad(Sel->getOperand(1), Sel, gvn.getDominatorTree());
-    LoadInst *L2 =
-        findDominatingLoad(Sel->getOperand(2), Sel, gvn.getDominatorTree());
+    LoadInst *L1 = findDominatingLoad(Sel->getOperand(1), LoadTy, Sel,
+                                      gvn.getDominatorTree());
+    LoadInst *L2 = findDominatingLoad(Sel->getOperand(2), LoadTy, Sel,
+                                      gvn.getDominatorTree());
     assert(L1 && L2 &&
            "must be able to obtain dominating loads for both value operands of "
            "the select");
@@ -1078,14 +1079,15 @@ static void reportMayClobberedLoad(LoadInst *Load, MemDepResult DepInfo,
 /// clobber the loads.
 static Optional<AvailableValue>
 tryToConvertLoadOfPtrSelect(BasicBlock *DepBB, BasicBlock::iterator End,
-                            Value *Address, DominatorTree &DT, AAResults *AA) {
+                            Value *Address, Type *LoadTy, DominatorTree &DT,
+                            AAResults *AA) {
 
   auto *Sel = dyn_cast_or_null<SelectInst>(Address);
   if (!Sel || DepBB != Sel->getParent())
     return None;
 
-  LoadInst *L1 = findDominatingLoad(Sel->getOperand(1), Sel, DT);
-  LoadInst *L2 = findDominatingLoad(Sel->getOperand(2), Sel, DT);
+  LoadInst *L1 = findDominatingLoad(Sel->getOperand(1), LoadTy, Sel, DT);
+  LoadInst *L2 = findDominatingLoad(Sel->getOperand(2), LoadTy, Sel, DT);
   if (!L1 || !L2)
     return None;
 
@@ -1108,8 +1110,8 @@ bool GVNPass::AnalyzeLoadAvailability(LoadInst *Load, MemDepResult DepInfo,
   if (!DepInfo.isDef() && !DepInfo.isClobber()) {
     assert(isa<SelectInst>(Address));
     if (auto R = tryToConvertLoadOfPtrSelect(
-            Load->getParent(), Load->getIterator(), Address, getDominatorTree(),
-            getAliasAnalysis())) {
+            Load->getParent(), Load->getIterator(), Address, Load->getType(),
+            getDominatorTree(), getAliasAnalysis())) {
       Res = *R;
       return true;
     }
@@ -1274,9 +1276,9 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load, LoadDepVect &Deps,
     Value *Address = Deps[i].getAddress();
 
     if (!DepInfo.isDef() && !DepInfo.isClobber()) {
-      if (auto R = tryToConvertLoadOfPtrSelect(DepBB, DepBB->end(), Address,
-                                               getDominatorTree(),
-                                               getAliasAnalysis())) {
+      if (auto R = tryToConvertLoadOfPtrSelect(
+              DepBB, DepBB->end(), Address, Load->getType(), getDominatorTree(),
+              getAliasAnalysis())) {
         ValuesPerBlock.push_back(
             AvailableValueInBlock::get(DepBB, std::move(*R)));
         continue;

diff  --git a/llvm/test/Transforms/GVN/opaque-ptr.ll b/llvm/test/Transforms/GVN/opaque-ptr.ll
index 617290d333272..ffc14d9a96af3 100644
--- a/llvm/test/Transforms/GVN/opaque-ptr.ll
+++ b/llvm/test/Transforms/GVN/opaque-ptr.ll
@@ -103,3 +103,37 @@ join:
   %v = load i32, ptr %gep
   ret i32 %v
 }
+
+define i32 @select_pre(ptr %px, ptr %py) {
+; CHECK-LABEL: @select_pre(
+; CHECK-NEXT:    [[T2:%.*]] = load i32, ptr [[PY:%.*]], align 4
+; CHECK-NEXT:    [[T3:%.*]] = load i32, ptr [[PX:%.*]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[T2]], [[T3]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[CMP]], i32 [[T3]], i32 [[T2]]
+; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[CMP]], ptr [[PX]], ptr [[PY]]
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %t2 = load i32, ptr %py, align 4
+  %t3 = load i32, ptr %px, align 4
+  %cmp = icmp slt i32 %t2, %t3
+  %select = select i1 %cmp, ptr %px, ptr %py
+  %r = load i32, ptr %select, align 4
+  ret i32 %r
+}
+
+define i64 @select_pre_
diff erent_types(ptr %px, ptr %py) {
+; CHECK-LABEL: @select_pre_
diff erent_types(
+; CHECK-NEXT:    [[T2:%.*]] = load i32, ptr [[PY:%.*]], align 4
+; CHECK-NEXT:    [[T3:%.*]] = load i32, ptr [[PX:%.*]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[T2]], [[T3]]
+; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[CMP]], ptr [[PX]], ptr [[PY]]
+; CHECK-NEXT:    [[R:%.*]] = load i64, ptr [[SELECT]], align 4
+; CHECK-NEXT:    ret i64 [[R]]
+;
+  %t2 = load i32, ptr %py, align 4
+  %t3 = load i32, ptr %px, align 4
+  %cmp = icmp slt i32 %t2, %t3
+  %select = select i1 %cmp, ptr %px, ptr %py
+  %r = load i64, ptr %select, align 4
+  ret i64 %r
+}