[PATCH] D138766: [InstCombine] If loading from small alloca, load whole alloca and perform variable extraction

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

>> Does the 2x register limit mean we are also creating a double-width load? Are we relying on later passes/codegen to narrow that?
>
> We are indeed creating double-width load,
> which is generally going to survive until Codegen.
> Alternatively, we could produce non-canonical IR here,
> by performing legalization ourselves.
> Would that be better?

I'm not sure it makes much difference to subsequent IR transforms, but creating a known illegal load seems like a (scary) precedent for target-independent canonicalization.

>> If we're trying to justify this target-independently, then lets use a less familiar target to avoid reaching the conclusion that the transform is generally good.
>> I have very little idea about what is happening here with RISCV64:
>> https://godbolt.org/z/vEcMP6P4x
>
> Please clarify, would be be less burdensome to first proceed with
> a single-width change, and then discuss relaxing it to double-width?

Yes, the single-width seems less scary, but we're still really stretching to call this a canonicalization. Does transforming later in IR miss some other optimizations that you want to happen?
For example, I'm still not sure what is happening here:
https://godbolt.org/z/Tcq8af83j

Have I messed up the target specification in some way? Assuming the 2nd version is worse, how is a target expected to reverse it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138766