PR34603 Fix by Early-CSE extension

[evstupac]

Summary

This is an alternative patch that solves PR34603:
https://bugs.llvm.org/show_bug.cgi?id=34603

Even if it is fixed by https://reviews.llvm.org/D38566 the patch Early-CSE extension could be useful for some cases.

[• dberlin]

I'll repeat what i said on the bug
FWIW: Given the choice, i would much rather fix the underlying problem then patch various passes to work around some small subset of it (here, load of selected addresses)
This is underway right now, actually.

I would be more amenable to the latter approach if there was data that we could get, say, 90% of the gain of changing the canonical form by doing so.

I have doubts this is the case, but if you've got data that says it is, ...

lib/Transforms/Scalar/EarlyCSE.cpp

if (MemInst.isValid() && MemInst.isLoad()) {
             Inst->replaceAllUsesWith(Op);
           removeMSSA(Inst);
           Inst->eraseFromParent();
           Changed = true;
           ++NumCSELoad;
           continue;
         }
       }
+      // Special replace for a load with selected address:
+      //   a = select p, a1, a2
+      //   b = load a
+      //
+      // if there are existing loads from (or store to) "a1" and "a2":
+      //   b1 = load a1 (store b1 to a1)
+      //   b2 = load a2 (store b2 to a2)
+      //
+      // The load "b = load a" could be replaced with just:
+      //   b = select p, b1, b2
+
+      Value *IP = MemInst.getPointerOperand();
+      if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(IP)) {
+        if (SelectInst *IS = dyn_cast<SelectInst>(GEP->getOperand(0))) {
+          Instruction *IP1 = GEP->clone();
+          Instruction *IP2 = GEP->clone();
+          IP1->setOperand(0, IS->getOperand(1));
+          IP2->setOperand(0, IS->getOperand(2));
+          Value *VPP1 = AvailableValues.lookup(IP1);
+          Value *VPP2 = AvailableValues.lookup(IP2);
+          IP1->deleteValue();
+          IP2->deleteValue();
+          if (VPP1 && VPP2 && IS->hasOneUse()) {
+            LoadValue InVal1 = AvailableLoads.lookup(VPP1);
+            LoadValue InVal2 = AvailableLoads.lookup(VPP2);
+            if (InVal1.DefInst && InVal2.DefInst &&
+                InVal1.MatchingId == MemInst.getMatchingId() &&
+                InVal2.MatchingId == MemInst.getMatchingId() &&
+                !MemInst.isVolatile() && MemInst.isUnordered() &&
+                InVal1.IsAtomic >= MemInst.isAtomic() &&
+                InVal2.IsAtomic >= MemInst.isAtomic() &&
+                ((InVal1.IsInvariant && InVal2.IsInvariant) ||
+                 MemInst.isInvariantLoad() ||
+                 (isSameMemGeneration(InVal1.Generation, CurrentGeneration,
+                                      InVal1.DefInst, Inst) &&
+                  isSameMemGeneration(InVal2.Generation, CurrentGeneration,
+                                      InVal2.DefInst, Inst)))) {
+              Value *Op1 = getOrCreateResult(InVal1.DefInst, Inst->getType());
+              Value *Op2 = getOrCreateResult(InVal2.DefInst, Inst->getType());
+              if (Op1 && Op2) {
+                Value *VV = SelectInst::Create(IS->getOperand(0),
+                                               Op1, Op2, "", Inst);
+                DEBUG(dbgs() << "EarlyCSE CSE LOAD: " << *Inst
+                             << "  to: " << *VV << '\n');
+                if (!Inst->use_empty())
+                  Inst->replaceAllUsesWith(VV);
+                removeMSSA(Inst);
+                Inst->eraseFromParent();
+                Changed = true;
+                ++NumCSELoad;
+                continue;
+              }
+            }
+          }
+        }
+      }
 
       // Otherwise, remember that we have this instruction.
       AvailableLoads.insert(
           MemInst.getPointerOperand(),
           LoadValue(Inst, CurrentGeneration, MemInst.getMatchingId(),
                     MemInst.isAtomic(), MemInst.isInvariantLoad()));
       LastStore = nullptr;
       continue;

test/Transforms/EarlyCSE/select_load.ll

+; RUN: opt < %s -S -early-cse | FileCheck %s
+
+; Check that load (with selected address) is deleted
+
+; CHECK-LABEL: @min(
+; CHECK: load i32
+; CHECK: load i32
+; CHECK-NOT: load i32
+
+; Function Attrs: norecurse nounwind readonly uwtable
+define i32 @min(i32* nocapture readonly, i32* nocapture readonly, i32) local_unnamed_addr {
+  %4 = sext i32 %2 to i64
+  %5 = getelementptr inbounds i32, i32* %0, i64 %4
+  %6 = load i32, i32* %5, align 4
+  %7 = getelementptr inbounds i32, i32* %1, i64 %4
+  %8 = load i32, i32* %7, align 4
+  %9 = icmp slt i32 %6, %8
+  %10 = select i1 %9, i32* %0, i32* %1
+  %11 = getelementptr inbounds i32, i32* %10, i64 %4
+  %12 = load i32, i32* %11, align 4
+  ret i32 %12
+}