[PATCH] D125264: [ConstraintElimination] Simplify ssub(a, b) if a s>=b && a s>=0.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 428672.
fhahn added a comment.



In D125264#3503308 <https://reviews.llvm.org/D125264#3503308>, @spatel wrote:

> In D125264#3501823 <https://reviews.llvm.org/D125264#3501823>, @nikic wrote:
>
>> In D125264#3501812 <https://reviews.llvm.org/D125264#3501812>, @fhahn wrote:
>>
>>> In D125264#3501773 <https://reviews.llvm.org/D125264#3501773>, @nikic wrote:
>>>
>>>> Would it be sufficient to only replace the `m_ExtractValue<1>`? I believe that's what we usually do. (Would also work if only the extractvalue is dominated.)
>>>
>>> And leave the removal of the actual intrinsic call to instcombine (or some other existing transform)? If that's what we do elsewhere I think it should work, the current placement in the regular optimization pipeline means we run instcombine shortly after ConstraintElimiantion.
>>
>> Yeah, that's what I had in mind. Though checking now, it looks like both CVP (https://github.com/llvm/llvm-project/blob/72831a592edf1bdcca15354181867079a17d4f76/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp#L572) and SimplifyIndVars (https://github.com/llvm/llvm-project/blob/72831a592edf1bdcca15354181867079a17d4f76/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp#L423) do replace the with.overflow intrinsic itself, so maybe it's fine as is.
>
> Those examples also do eraseFromParent(). I don't know if there's any official guideline for the behavior, but cleaning up dead values here makes it less likely that things will break if some other pass gets inserted in the pipeline between this and the next round of instcombine.

That's a good point, thanks! I updated the code to remove both the extracts and the intrinsic, if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125264

Files:
  llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
  llvm/test/Transforms/ConstraintElimination/ssub-with-overflow.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125264.428672.patch
Type: text/x-patch
Size: 6713 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220511/87c2c019/attachment.bin>