[GVN] Support load of pointer-select to value-select conversion.

This patch extends the available-value logic to detect loads
of pointer-selects that can be replaced by a value select.

For example, consider the code below:

  loop:
    %sel.phi = phi i32* [ %start, %ph ], [ %sel, %ph ]
    %l = load %ptr
    %l.sel = load %sel.phi
    %sel = select cond, %ptr, %sel.phi
    ...

  exit:
    %res = load %sel
    use(%res)

The load of the pointer phi can be replaced by a load of the start value
outside the loop and a new phi/select chain based on the loaded values,
as illustrated below

    %l.start = load %start
  loop:
    sel.phi.prom = phi i32 [ %l.start, %ph ], [ %sel.prom, %ph ]
    %l = load %ptr
    %sel.prom = select cond, %l, %sel.phi.prom
    ...
  exit:
    use(%sel.prom)

This is a first step towards alllowing vectorizing loops using common libc++
library functions, like std::min_element (https://clang.godbolt.org/z/6czGzzqbs)

    #include <vector>
    #include <algorithm>

    int foo(const std::vector<int> &V) {
        return *std::min_element(V.begin(), V.end());
    }

Reviewed By: reames

Differential Revision: https://reviews.llvm.org/D118143

Author: fhahn

llvm/lib/Transforms/Scalar/GVN.cpp

@@ -182,12 +182,14 @@ struct llvm::gvn::AvailableValue {
     SimpleVal, // A simple offsetted value that is accessed.
     LoadVal,   // A value produced by a load.
     MemIntrin, // A memory intrinsic which is loaded from.
-    UndefVal   // A UndefValue representing a value from dead block (which
+    UndefVal,  // A UndefValue representing a value from dead block (which
                // is not yet physically removed from the CFG).
+    SelectVal, // A pointer select which is loaded from and for which the load
+               // can be replace by a value select.
   };
 
   /// V - The value that is live out of the block.
-  PointerIntPair<Value *, 2, ValType> Val;
+  PointerIntPair<Value *, 3, ValType> Val;
 
   /// Offset - The byte offset in Val that is interesting for the load query.
   unsigned Offset = 0;
@@ -224,10 +226,19 @@ struct llvm::gvn::AvailableValue {
     return Res;
   }
 
+  static AvailableValue getSelect(SelectInst *Sel) {
+    AvailableValue Res;
+    Res.Val.setPointer(Sel);
+    Res.Val.setInt(SelectVal);
+    Res.Offset = 0;
+    return Res;
+  }
+
   bool isSimpleValue() const { return Val.getInt() == SimpleVal; }
   bool isCoercedLoadValue() const { return Val.getInt() == LoadVal; }
   bool isMemIntrinValue() const { return Val.getInt() == MemIntrin; }
   bool isUndefValue() const { return Val.getInt() == UndefVal; }
+  bool isSelectValue() const { return Val.getInt() == SelectVal; }
 
   Value *getSimpleValue() const {
     assert(isSimpleValue() && "Wrong accessor");
@@ -244,6 +255,11 @@ struct llvm::gvn::AvailableValue {
     return cast<MemIntrinsic>(Val.getPointer());
   }
 
+  SelectInst *getSelectValue() const {
+    assert(isSelectValue() && "Wrong accessor");
+    return cast<SelectInst>(Val.getPointer());
+  }
+
   /// Emit code at the specified insertion point to adjust the value defined
   /// here to the specified type. This handles various coercion cases.
   Value *MaterializeAdjustedValue(LoadInst *Load, Instruction *InsertPt,
@@ -275,6 +291,10 @@ struct llvm::gvn::AvailableValueInBlock {
     return get(BB, AvailableValue::getUndef());
   }
 
+  static AvailableValueInBlock getSelect(BasicBlock *BB, SelectInst *Sel) {
+    return get(BB, AvailableValue::getSelect(Sel));
+  }
+
   /// Emit code at the end of this block to adjust the value defined here to
   /// the specified type. This handles various coercion cases.
   Value *MaterializeAdjustedValue(LoadInst *Load, GVNPass &gvn) const {
@@ -897,6 +917,16 @@ ConstructSSAForLoadSet(LoadInst *Load,
   return SSAUpdate.GetValueInMiddleOfBlock(Load->getParent());
 }
 
+static LoadInst *findDominatingLoad(Value *Ptr, 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))
+      return LI;
+  }
+  return nullptr;
+}
+
 Value *AvailableValue::MaterializeAdjustedValue(LoadInst *Load,
                                                 Instruction *InsertPt,
                                                 GVNPass &gvn) const {
@@ -937,6 +967,17 @@ Value *AvailableValue::MaterializeAdjustedValue(LoadInst *Load,
                       << "  " << *getMemIntrinValue() << '\n'
                       << *Res << '\n'
                       << "\n\n\n");
+  } 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());
+    assert(L1 && L2 &&
+           "must be able to obtain dominating loads for both value operands of "
+           "the select");
+    Res = SelectInst::Create(Sel->getCondition(), L1, L2, "", Sel);
   } else {
     llvm_unreachable("Should not materialize value from dead block");
   }
@@ -1023,8 +1064,53 @@ static void reportMayClobberedLoad(LoadInst *Load, MemDepResult DepInfo,
   ORE->emit(R);
 }
 
+/// Check if a load from pointer-select \p Address in \p DepBB can be converted
+/// to a value select. The following conditions need to be satisfied:
+/// 1. The pointer select (\p Address) must be defined in \p DepBB.
+/// 2. Both value operands of the pointer select must be loaded in the same
+/// basic block, before the pointer select.
+/// 3. There must be no instructions between the found loads and \p End that may
+/// clobber the loads.
+static Optional<AvailableValue>
+tryToConvertLoadOfPtrSelect(BasicBlock *DepBB, BasicBlock::iterator End,
+                            Value *Address, 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);
+  if (!L1 || !L2)
+    return None;
+
+  // Ensure there are no accesses that may modify the locations referenced by
+  // either L1 or L2 between L1, L2 and the specified End iterator.
+  Instruction *EarlierLoad = L1->comesBefore(L2) ? L1 : L2;
+  MemoryLocation L1Loc = MemoryLocation::get(L1);
+  MemoryLocation L2Loc = MemoryLocation::get(L2);
+  if (any_of(make_range(EarlierLoad->getIterator(), End), [&](Instruction &I) {
+        return isModSet(AA->getModRefInfo(&I, L1Loc)) ||
+               isModSet(AA->getModRefInfo(&I, L2Loc));
+      }))
+    return None;
+
+  return AvailableValue::getSelect(Sel);
+}
+
 bool GVNPass::AnalyzeLoadAvailability(LoadInst *Load, MemDepResult DepInfo,
                                       Value *Address, AvailableValue &Res) {
+  if (!DepInfo.isDef() && !DepInfo.isClobber()) {
+    assert(isa<SelectInst>(Address));
+    if (auto R = tryToConvertLoadOfPtrSelect(
+            Load->getParent(), Load->getIterator(), Address, getDominatorTree(),
+            getAliasAnalysis())) {
+      Res = *R;
+      return true;
+    }
+    return false;
+  }
+
   assert((DepInfo.isDef() || DepInfo.isClobber()) &&
          "expected a local dependence");
   assert(Load->isUnordered() && "rules below are incorrect for ordered access");
@@ -1092,6 +1178,7 @@ bool GVNPass::AnalyzeLoadAvailability(LoadInst *Load, MemDepResult DepInfo,
         }
       }
     }
+
     // Nothing known about this clobber, have to be conservative
     LLVM_DEBUG(
         // fast print dep, using operator<< on instruction is too slow.
@@ -1176,16 +1263,23 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load, LoadDepVect &Deps,
       continue;
     }
 
-    if (!DepInfo.isDef() && !DepInfo.isClobber()) {
-      UnavailableBlocks.push_back(DepBB);
-      continue;
-    }
-
     // The address being loaded in this non-local block may not be the same as
     // the pointer operand of the load if PHI translation occurs.  Make sure
     // to consider the right address.
     Value *Address = Deps[i].getAddress();
 
+    if (!DepInfo.isDef() && !DepInfo.isClobber()) {
+      if (auto R = tryToConvertLoadOfPtrSelect(DepBB, DepBB->end(), Address,
+                                               getDominatorTree(),
+                                               getAliasAnalysis())) {
+        ValuesPerBlock.push_back(
+            AvailableValueInBlock::get(DepBB, std::move(*R)));
+        continue;
+      }
+      UnavailableBlocks.push_back(DepBB);
+      continue;
+    }
+
     AvailableValue AV;
     if (AnalyzeLoadAvailability(Load, DepInfo, Address, AV)) {
       // subtlety: because we know this was a non-local dependency, we know
@@ -1923,8 +2017,9 @@ bool GVNPass::processLoad(LoadInst *L) {
   if (Dep.isNonLocal())
     return processNonLocalLoad(L);
 
+  Value *Address = L->getPointerOperand();
   // Only handle the local case below
-  if (!Dep.isDef() && !Dep.isClobber()) {
+  if (!Dep.isDef() && !Dep.isClobber() && !isa<SelectInst>(Address)) {
     // This might be a NonFuncLocal or an Unknown
     LLVM_DEBUG(
         // fast print dep, using operator<< on instruction is too slow.
@@ -1934,7 +2029,7 @@ bool GVNPass::processLoad(LoadInst *L) {
   }
 
   AvailableValue AV;
-  if (AnalyzeLoadAvailability(L, Dep, L->getPointerOperand(), AV)) {
+  if (AnalyzeLoadAvailability(L, Dep, Address, AV)) {
     Value *AvailableValue = AV.MaterializeAdjustedValue(L, L, *this);
 
     // Replace the load!

llvm/test/Transforms/GVN/PRE/pre-load-through-select.ll

@@ -9,13 +9,15 @@ define i32 @test_pointer_phi_select_simp_1(i32* %a, i32* %b, i1 %cond)  {
 ; CHECK-NEXT:    [[L_1:%.*]] = load i32, i32* [[A:%.*]], align 4
 ; CHECK-NEXT:    [[L_2:%.*]] = load i32, i32* [[B:%.*]], align 4
 ; CHECK-NEXT:    [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
+; CHECK-NEXT:    [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
 ; CHECK-NEXT:    [[MIN_SELECT:%.*]] = select i1 [[CMP_I_I_I]], i32* [[A]], i32* [[B]]
 ; CHECK-NEXT:    br label [[EXIT:%.*]]
 ; CHECK:       else:
+; CHECK-NEXT:    [[RES_2_PRE:%.*]] = load i32, i32* [[A]], align 4
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
+; CHECK-NEXT:    [[RES_2:%.*]] = phi i32 [ [[TMP0]], [[THEN]] ], [ [[RES_2_PRE]], [[ELSE]] ]
 ; CHECK-NEXT:    [[P:%.*]] = phi i32* [ [[MIN_SELECT]], [[THEN]] ], [ [[A]], [[ELSE]] ]
-; CHECK-NEXT:    [[RES_2:%.*]] = load i32, i32* [[P]], align 4
 ; CHECK-NEXT:    ret i32 [[RES_2]]
 ;
 entry:
@@ -118,13 +120,15 @@ define i32 @test_pointer_phi_select_simp_store_noclobber(i32* %a, i32* %b, i32*
 ; CHECK-NEXT:    [[L_2:%.*]] = load i32, i32* [[B:%.*]], align 4
 ; CHECK-NEXT:    store i32 99, i32* [[C:%.*]], align 4
 ; CHECK-NEXT:    [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
+; CHECK-NEXT:    [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
 ; CHECK-NEXT:    [[MIN_SELECT:%.*]] = select i1 [[CMP_I_I_I]], i32* [[A]], i32* [[B]]
 ; CHECK-NEXT:    br label [[EXIT:%.*]]
 ; CHECK:       else:
+; CHECK-NEXT:    [[RES_2_PRE:%.*]] = load i32, i32* [[A]], align 4
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
+; CHECK-NEXT:    [[RES_2:%.*]] = phi i32 [ [[TMP0]], [[THEN]] ], [ [[RES_2_PRE]], [[ELSE]] ]
 ; CHECK-NEXT:    [[P:%.*]] = phi i32* [ [[MIN_SELECT]], [[THEN]] ], [ [[A]], [[ELSE]] ]
-; CHECK-NEXT:    [[RES_2:%.*]] = load i32, i32* [[P]], align 4
 ; CHECK-NEXT:    ret i32 [[RES_2]]
 ;
 entry:
@@ -648,9 +652,9 @@ define i32 @test_pointer_phi_select_single_block_store(i32* %a, i32* %b)  {
 ; CHECK-NEXT:    [[L_1:%.*]] = load i32, i32* [[A:%.*]], align 4
 ; CHECK-NEXT:    [[L_2:%.*]] = load i32, i32* [[B:%.*]], align 4
 ; CHECK-NEXT:    [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
+; CHECK-NEXT:    [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
 ; CHECK-NEXT:    [[MIN_SELECT:%.*]] = select i1 [[CMP_I_I_I]], i32* [[A]], i32* [[B]]
-; CHECK-NEXT:    [[RES_0:%.*]] = load i32, i32* [[MIN_SELECT]], align 4
-; CHECK-NEXT:    ret i32 [[RES_0]]
+; CHECK-NEXT:    ret i32 [[TMP0]]
 ;
 entry:
   %l.1 = load i32, i32* %a, align 4
@@ -723,3 +727,24 @@ entry:
   %res.0 = load i32, i32* %min.select, align 4
   ret i32 %res.0
 }
+
+define i32 @test_pointer_phi_select_single_block_store_after(i32* %a, i32* %b, i32* %c)  {
+; CHECK-LABEL: @test_pointer_phi_select_single_block_store_after(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[L_1:%.*]] = load i32, i32* [[A:%.*]], align 4
+; CHECK-NEXT:    [[L_2:%.*]] = load i32, i32* [[B:%.*]], align 4
+; CHECK-NEXT:    [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
+; CHECK-NEXT:    [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
+; CHECK-NEXT:    [[MIN_SELECT:%.*]] = select i1 [[CMP_I_I_I]], i32* [[A]], i32* [[B]]
+; CHECK-NEXT:    store i32 99, i32* [[C:%.*]], align 4
+; CHECK-NEXT:    ret i32 [[TMP0]]
+;
+entry:
+  %l.1 = load i32, i32* %a, align 4
+  %l.2 = load i32, i32* %b, align 4
+  %cmp.i.i.i = icmp ult i32 %l.1, %l.2
+  %min.select  = select i1 %cmp.i.i.i, i32* %a, i32* %b
+  %res.0 = load i32, i32* %min.select, align 4
+  store i32 99, i32* %c
+  ret i32 %res.0
+}