[PATCH] D136158: [AArch64] Adjust operand sequence for Add+Sub to combine more inline shift

chenglin.bi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 24 23:03:03 PDT 2022


bcl5980 added a comment.

In D136158#3880624 <https://reviews.llvm.org/D136158#3880624>, @dmgreen wrote:

> In D136158#3878631 <https://reviews.llvm.org/D136158#3878631>, @bcl5980 wrote:
>
>> In D136158#3878602 <https://reviews.llvm.org/D136158#3878602>, @dmgreen wrote:
>>
>>> Have you tried testing this? I think it will get stuck in DAG combines.
>>>
>>> Otherwise the idea sounds good. Have you considered doing it for GlobalISel, or in a way that GlobalISel would also benefit?
>>
>> I haven't found any stuck in DAG combine local and it looks all of the premerge test is also passed:
>> https://reviews.llvm.org/harbormaster/build/291788/
>>
>> Which combination do you think will cause the dead loop in DAG combine with this patch? I can try to create a test for that.
>
> Yes - the llvm regression tests are very much a subset of all the patterns that can come up in practice. I'm not sure what this is running into. Perhaps try and bootstrap or compiling the llvm-test-suite?

Or can we do it on Machine IR(AArch64MIPeepholeOpt) to avoid dead loop and also GISel can get benefit?


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

https://reviews.llvm.org/D136158



More information about the llvm-commits mailing list