[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