[SimplifyCfg] Don't sink loads/stores to geps of allocas

[arielb1]

Summary

This makes to workaround to PR30188 be more robust to code that uses structs.

In practice, this improves an optimization case in rustc and reduces in-the-wild Rust code size by 0.1%.

[arielb1]

Yes, I know the "right" thing to do might be to make SROA smarter, but de-messing selects in this context is pretty hard.

[arielb1]

We want this improvement. What is it waiting for?

[mcrosier]

+Chandler (SROA owner)
-James (not active AFAICT)

[arielb1]

Ping Chandler - what is this waiting for?

[jdoerfert]

The test is broken, repaired: https://clang.godbolt.org/z/W3zcxP

Unclear if the underlying issue from PR30188 is still there: https://clang.godbolt.org/z/x37hrf

[nikic]

The current implementation already uses stripPointerCasts(), so probably this is no longer relevant. @xbolva00 any particular reason for the ping?

[xbolva00]

Since you work with rust-lang, maybe you are interested in this patch. (Patch mentions improvements for rust code).

If not relevant anymore, the patch should be abandoned/closed.

lib/Transforms/Utils/SimplifyCFG.cpp

static bool canSinkInstructions(
   // to sink loads or stores of allocas when we'd have to create a PHI for
   // the address operand. Also, because it is likely that loads or stores
   // of allocas will disappear when Mem2Reg/SROA is run, don't sink them.
   // This can cause code churn which can have unintended consequences down
   // the line - see https://llvm.org/bugs/show_bug.cgi?id=30244.
   // FIXME: This is a workaround for a deficiency in SROA - see
   // https://llvm.org/bugs/show_bug.cgi?id=30188
   if (isa<StoreInst>(I0) && any_of(Insts, [](const Instruction *I) {
-        return isa<AllocaInst>(I->getOperand(1));
+        return isa<AllocaInst>(I->getOperand(1)->stripInBoundsOffsets());
       }))
     return false;
   if (isa<LoadInst>(I0) && any_of(Insts, [](const Instruction *I) {
-        return isa<AllocaInst>(I->getOperand(0));
+        return isa<AllocaInst>(I->getOperand(0)->stripInBoundsOffsets());
       }))
     return false;
 
   for (unsigned OI = 0, OE = I0->getNumOperands(); OI != OE; ++OI) {
     if (I0->getOperand(OI)->getType()->isTokenTy())
       // Don't touch any operand of token type.
       return false;
 

test/Transforms/SimplifyCFG/sink-common-code.ll

 }
 
 ; CHECK-LABEL: test_pr30188a
 ; CHECK-NOT: select
 ; CHECK: load
 ; CHECK: load
 ; CHECK: store
 
+define i32 @test_pr30188b(i1 zeroext %flag, i32 %x) {
+entry:
+  %yy = alloca {i32}
+  %zz = alloca {i32}
+  %y = getelementptr inbounds {i32}, {i32}* %y, i32 0, i32 ,0
+  %z = getelementptr inbounds {i32}, {i32}* %z, i32 0, i32 ,0
+  br i1 %flag, label %if.then, label %if.else
+
+if.then:
+  store i32 %x, i32* %y
+  br label %if.end
+
+if.else:
+  store i32 %x, i32* %z
+  br label %if.end
+
+if.end:
+  ret i32 1
+}
+
+; CHECK-LABEL: test_pr30188b
+; CHECK-NOT: select
+; CHECK: store
+; CHECK: store
+
 ; The phi is confusing - both add instructions are used by it, but
 ; not on their respective unconditional arcs. It should not be
 ; optimized.
 define void @test_pr30292(i1 %cond, i1 %cond2, i32 %a, i32 %b) {
 entry:
   %add1 = add i32 %a, 1
   br label %succ