[PATCH] D106139: [DAGCombine] Combine srX of add that intends to get the carry as uaddo

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 28 07:21:45 PDT 2021


dmgreen added a reviewer: spatel.
dmgreen added a comment.

> In D106139#2909632 <https://reviews.llvm.org/D106139#2909632>, @lebedev.ri wrote:
>
>> In D106139#2909346 <https://reviews.llvm.org/D106139#2909346>, @abinavpp wrote:
>>
>>> @lebedev.ri Can you confirm the revision you require in this?
>>
>>
>>
>>> Is there an issue with the code in this patch and/or do you want us to move this to inst-combine?
>>> I still agree with @arsenm that this sort of thing might need to be done in both the places, in which case I don't see any issue in the order in which we push for it.
>>
>> I agree that we might need to do this in both the places, emphasis is mine,
>> but mainly this should be done in middle-end, and afterwards the back-end fold
>> should be motivated by an end-to-end test showing the missed optimization.

Do we usually create uadd.with.overflow in instcombine? I thought we moved all of that to CodeGenPrep as is was causing regressions otherwise.
(I mean creating uadd.with.overflow from instructions that didn't start out as different add.with.overflow, generating them from plain llvm instructions. That might have been problems with the particular transform, but from what I remember instcombine was creating cross-basic-block overflow values, that the backend then couldn't handle very well. I'm not sure that was the exact reason for moving it though, I just remember codesize regressions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106139



More information about the llvm-commits mailing list