[PATCH] [InstCombine] call SimplifyICmpInst with correct context

David Majnemer david.majnemer at gmail.com
Wed Jun 24 22:23:08 PDT 2015

On Wed, Jun 24, 2015 at 10:00 PM, Jingyue Wu <jingyue at google.com> wrote:

> And just to be clear, I agree with you that it's a potential issue that
> `ComputeSignBit` is unable to say "I don't have a context". But my concern
> is, even if we fix that issue, not passing the context instruction to
> `SimplifyICmpInst` at all in `InstCombine` may overkill some useful
> optimizations.

I see these as two separable issues: a correctness issue and a performance
issue.  My thinking was that we should solve the correctness issue by being
conservative at the API boundary and regain lost performance by providing
the context instruction.  My fear is not that your change to
SimplifyICmpInst is incorrect but that all the other callers of the various
SimplifyXYZInst API have latent bugs because they aren't passing in a

> Btw, I noticed that `SimplifyDemandedBits` indeed pass `UserI` as the
> context instruction (
> http://llvm.org/docs/doxygen/html/InstCombineSimplifyDemanded_8cpp_source.html#l00075),
> which seems aligned with the approach in my patch.
> http://reviews.llvm.org/D10695
>   http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150624/a4c1ace8/attachment.html>

More information about the llvm-commits mailing list