[PATCH] D130100: [AArch64] Combine a load into GPR followed by a copy to FPR to a load into FPR directly through MIPeepholeOpt

[Mingming Liu via Phabricator via llvm-commits]

mingmingl abandoned this revision.
mingmingl added a comment.

In D130100#3696135 <https://reviews.llvm.org/D130100#3696135>, @dmgreen wrote:

> I worry about the potential for bugs in any patch in the AArch64MIPeepholeOptimizer. And loading makes that more complex. The altered tests look like they are either intrinsics like the pmull64 that was recently changed or strange types (<32 x i1>, <16 x i12>) that don't come up a lot in practice. The global-isel change in dp1.ll is a missed fold that is probably better handled in global-isel. The irregular type tests also show it only handling straight loads, not instructions like `ld1 { v1.s }[1], [x12]`. The current model is to try and handle almost all combines during ISel so that they can all work together in one place.

I totally agree with the fact that loading/storing would make things more complex here, that global-isel should be able to handle dp1.ll; strange types (<16 x i2><32xi1>) looks more interesting but I doubt if it's a result of multiple issues.

> If this comes up in more places it might be worth it. From the examples in the tests I'm not so sure though. Do you have any other examples where this helps?

I'm convinced it's better to handle affected test cases in ISel (yet still made changes, hope to make it closer to correct);

I don't observe other examples where this helps -> problem hunting starts from pmull test case (`llvm/test/CodeGen/AArch64/aarch64-pmull2.ll` in D131047 <https://reviews.llvm.org/D131047>)

I'll abandon this patch for now; thanks for your timely feedback on the series of changes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130100