[PATCH] D116468: [AArch64] Combine ADD/SUB instructions when they contain a 24-bit immediate.

Fri Jan 7 16:09:22 PST 2022

red1bluelost added a comment.

In D116468#3224907 <https://reviews.llvm.org/D116468#3224907>, @dmgreen wrote:

> In D116468#3224507 <https://reviews.llvm.org/D116468#3224507>, @benshi001 wrote:
>
>> I am quite sorry for that. Though `make check-all` passed and everything was good on amd64, the clang built into aarch64 ELF and running on aarch64-linux crashed. And I have no aarch64 host machine to debug that (although I guess it should be a minor bug).
>
> Oh I see. Sorry for not seeing that message. I hadn't realized it was reverted.
>
> I have no strong opinion whether this is best done as an AArch64MIPeephole or in MachineCombine. They are probably both fine for this kind of thing. Whichever @red1bluelost thinks will be easiest to extend to SUBS instructions that write nzcv sounds OK to me, if the goal here is to address https://github.com/llvm/llvm-project/issues/51482.

Either method (Peephole or MachineCombine) would be easy to extend for the SUBS instructions. I think the Peephole approach would be better, it seems more encapsulated than my approach.

What is the status of D111034 <https://reviews.llvm.org/D111034> @benshi001, is it likely to be committed or is that bug still holding it up? The buildbot logs don't appear anymore so I can't see where that bug happened. Could that bug also be present with my MachineCombine patch?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116468