[PATCH] D127392: [InstCombine] Combine consecutive loads which are being merged to form a wider load.

[Biplob Mishra via Phabricator via llvm-commits]

bipmis added a comment.

In D127392#3607415 <https://reviews.llvm.org/D127392#3607415>, @nikic wrote:

> The main question I see here is whether this needs to be TTI based or not -- if yes, then it also shouldn't be in InstCombine. I think there's two reason why we might want TTI:
>
> - Do we want to create unaligned loads? Creating them is legal, but if the target does not support fast unaligned loads, the backend will break them up again. Should we only perform this optimization if `TTI.allowsMisalignedMemoryAccesses` reports a fast unaligned access?
> - Do we want to create loads with illegal sizes? For example, if we have a 64-bit target, so we want to combine two `i64` loads into an `i128` load, even though it will later be broken up again? (At least for the current implementation where both loads must have the same size, the question of whether to allow `i24` loads or similar does not come up.)

Thanks for the review. I have made most of the other suggested changes and can post a patch for the same. It would also handle some more test cases.

Right I think we need to be sure on this particular path. If we want a TTI based should Aggressive Instcombine be a better choice. We also see some use scenarios of TTI in InstCombine but is our case a vaild use case scenario. 
Next patch would be in InstCombine for this reason.
I will also test the patch up with extended tests to see if we are seeing any performance issues and if the backend breaking up happens ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127392