[PATCH] D112142: [InstCombine][ConstantFolding] Make ConstantFoldLoadThroughBitcast TypeSize-aware

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm marked 3 inline comments as done.
peterwaller-arm added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/vscale_load_bitcast.ll:6
+; CHECK-LABEL: @constprop_load_bitcast(
+; CHECK-NEXT:    ret <8 x i8> zeroinitializer
+;
----------------
paulwalker-arm wrote:
> david-arm wrote:
> > I guess this fold is happening because effectively i1 is being promoted to i8? Otherwise the minimum size of <vscale x 16 x i1> would be just 2 bytes, i.e. less than than <8 x i8>.
> This is likely due to the issue above but at the same time the test is relying on undefined behaviour.  You either want to use a smaller fixed length vector or perhaps add a `vscale_range` so the load does overlap the store.
> 
> Can you also add a negative test for the case where the load & store must remain.  I guess if you want to tick all the boxes you could also add a test where the store remains but the load is removed.
Right. I've added a negative test and fixed up the size query, thanks both for your attention to detail!

I considered your comment about vscale_range but:

1. I don't see a straightforward route to get at it without propagating the function attributes through the constant folding chain. Maybe that's the right thing to do? Or is there a way to take  Constant* and a Type*, and ask "what vscale_range is in effect?". I'm guessing not since these aren't in principle bound to functions. I don't want to fiddle with the function signature without a positive steer from someone that it's the right thing to do.

2. What is the best way to make isKnown* queries in the presence of a vscale_range attribute?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112142