[PATCH] D144445: (WIP)[ConstantFold][InstSimplify] folding load for constant global all-element-equal arrays and structs

Wed Feb 22 00:24:16 PST 2023

khei4 added a comment.

Thank you for the review!

In D144445#4141534 <https://reviews.llvm.org/D144445#4141534>, @nikic wrote:

> Please take a look at IR produced by rustc: https://godbolt.org/z/c88Wadrbn
>
> Note that rustc always represents constant globals in raw representation: The two lookup tables here are `<{ [2 x i8] c"\01\01" }>` and `<{ [4 x i8] c"\01\00\01\00" }>`. Especially note that the latter is not `[i16 1, i16 1]` or similar, it's all represented as bytes.

Cool! I failed to notice the alignment, I'll fix them. Thank you.

> Note that the kimwitu++ regression is due to an increase in code size: http://llvm-compile-time-tracker.com/compare.php?from=38ed416693c29761e8a934144102def74b4b47b3&to=cddf9d1ed0bde41b392352bf2391d065e2d27c35&stat=size-text This is pretty normal -- if code size changes, there is usually corresponding compile-time change.

I didn't notice size changes too. Thank you for pointing that out!

================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:770

+Constant *llvm::ConstantFoldLoadFromAllEqAggregate(Constant *C, Type *Ty) {
+
----------------
nikic wrote:
> junparser wrote:
> > khei4 wrote:
> > > junparser wrote:
> > > > What you need maybe something like Constant::getSplatValue with proper handle with constant struct and array
> > > @junparser
> > > Thank you for the review! 
> > > Yeah, we can say so, but `Splat` seems like from [[ https://lists.llvm.org/pipermail/llvm-dev/2017-May/112758.html | conventions around vector type ]] 
> > > , so if implemented as a method, it might be better to separate them.
> > > As you see, architecturally this element's equality checking might be general enough to implement as a Constant method. Is it better?
> > Since this is complement for ConstantFoldLoadFromUniformValue, so it is better to do inside ConstantFoldLoadFromUniformValue.
> This should not be part of ConstantFoldLoadFromUniformValue.
Ok. (FYI: `ConstantFoldLoadFromUniformValue ` seems to require that all bits are equivalent https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Analysis/ConstantFolding.h#L167~L171)

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

https://reviews.llvm.org/D144445