[PATCH] D137705: [AMDGPU] Add DAG Combine for right-shift carry add to uaddo

Pierre van Houtryve via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 00:15:22 PST 2022


Pierre-vh added a comment.

In D137705#3935192 <https://reviews.llvm.org/D137705#3935192>, @arsenm wrote:

> In D137705#3933081 <https://reviews.llvm.org/D137705#3933081>, @Pierre-vh wrote:
>
>> In D137705#3932390 <https://reviews.llvm.org/D137705#3932390>, @arsenm wrote:
>>
>>> Apparently there is already an in flight version of this at D106139 <https://reviews.llvm.org/D106139>
>>
>> Right, I think the reviewers there asked to move it to InstCombine, but I think we also want it in the backend, right?
>
> Combines in the backend are primarily for patterns that arise as the result of legalization. Looking at this again, I'm inclined to have it in instcombine primarily.
>
>> Perhaps it should be kept as a target-specific combine for now? It can always be moved later if needed
>
> It definitely should be done generically

So this stack of diff should go and an instcombine implementation done instead?
D107552 <https://reviews.llvm.org/D107552> was also in-flight for this. Should I take it over?
There was some discussion about it being a less understandable canonical form though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137705



More information about the llvm-commits mailing list