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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125264