[PATCH] D144184: [InstSimplify] fold LoadInst for uniformaly initialized constant global variables

Kohei Asano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 18 17:08:12 PST 2023


khei4 added a comment.

@nikic Thank you! Please use `Kohei Asano <kk.asano.luxy at gmail.com>`.



================
Comment at: llvm/test/Transforms/InstSimplify/load.ll:64
+;
+  %gep1 = getelementptr inbounds i8, ptr @constzeroarrayi8, i64 0
+  %gep = getelementptr inbounds i8, ptr %gep1, i64 %idx
----------------
nikic wrote:
> 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.)
> 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.



================
Comment at: llvm/test/Transforms/InstSimplify/load.ll:53
+  ret i32 %load
+}
+
----------------
nikic wrote:
> 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
> ```
Seems reasonable. Thanks for the review!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144184/new/

https://reviews.llvm.org/D144184



More information about the llvm-commits mailing list