[InstSimplify] fold LoadInst for uniformaly initialized constant global variables

[khei4]

Summary

Fold loadInst for uniformly initialized global arrays.
Goal proof: https://alive2.llvm.org/ce/z/oZtVby
motivated by https://github.com/rust-lang/rust/issues/107208

[khei4]

add tests

[nikic]

Looks basically fine to me.

llvm/lib/Analysis/InstructionSimplify.cpp

6590

nit: regardless of?

6593

nit: Add a newline after this.

llvm/test/Transforms/InstSimplify/load.ll

53 ↗

(On Diff #497984)

I would add an additional test that shows that this can look through multiple GEPs and doesn't depend on the GEP source element type ([4 x i32]) either.

%gep1 = getelementptr inbounds i8, ptr @constzeroarray, i64 %idx1
%gep = getelementptr inbouds i8, ptr %gep1, i64 %idx2

[khei4]

Fix comments and add test cases.

[nikic]

I think something went wrong with the patch upload, it seems like only part of the test diff is there?

llvm/test/Transforms/InstSimplify/load.ll

64 ↗

(On Diff #498137)

Using i64 0 doesn't really make sense, because it is a no-op. The instruction will be removed before we get to the fold. It should use a variable index as well.

It's also fine to reuse constzeroarray rather than constzeroarrayi8 here -- in fact, that shows that we don't need a matching stride between the getelementptr and the globals declaration. (We do need it for the non-zero initializer case.)

87 ↗

(On Diff #498137)

Same here, getelementptr with zero index is a no-op.

[khei4]

I rebased commits and created a new diff. Maybe the cause is I combine the test commit and functional commit in the wrong way. I'm going to separate them next time.

[nikic]

LGTM. Could you please share the Name <email> to use for the commit?

[khei4]

Thanks a lot for review! Please use Kohei Asano <[e-mail]>.

llvm/test/Transforms/InstSimplify/load.ll

64 ↗

(On Diff #498137)

Using i64 0 doesn't really make sense, because it is a no-op. The instruction will be removed before we get to the fold. It should use a variable index as well.

It's also fine to reuse constzeroarray rather than constzeroarrayi8 here -- in fact, that shows that we don't need a matching stride between the getelementptr and the globals declaration. (We do need it for the non-zero initializer case.)

Thanks. Used other variable indexes and reused constzeroarray for the second test case.

53 ↗

(On Diff #497984)

Seems reasonable. Thanks for the review!

[khei4]

I noticed mistakenly reverted the review fix. Rebase.

llvm/lib/Analysis/InstructionSimplify.cpp

if (auto *PtrOpC = dyn_cast<Constant>(PtrOp))
     return ConstantFoldLoadFromConstPtr(PtrOpC, LI->getType(), Q.DL);
 
   // We can only fold the load if it is from a constant global with definitive
   // initializer. Skip expensive logic if this is not the case.
   auto *GV = dyn_cast<GlobalVariable>(getUnderlyingObject(PtrOp));
   if (!GV || !GV->isConstant() || !GV->hasDefinitiveInitializer())
     return nullptr;
 
+  // If GlobalVariable's initializer is uniform, then return the constant
+  // regardless its offset.
+  if (Constant *C =
+          ConstantFoldLoadFromUniformValue(GV->getInitializer(), LI->getType()))
+    return C;
   // Try to convert operand into a constant by stripping offsets while looking
   // through invariant.group intrinsics.
   APInt Offset(Q.DL.getIndexTypeSizeInBits(PtrOp->getType()), 0);
   PtrOp = PtrOp->stripAndAccumulateConstantOffsets(
       Q.DL, Offset, /* AllowNonInbounts */ true,
       /* AllowInvariantGroup */ true);
   if (PtrOp == GV) {
     // Index size may have changed due to address space casts.