[PATCH] [InstCombine] call SimplifyICmpInst with correct context

[David Majnemer]

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
context.


>
> 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
>
> EMAIL PREFERENCES
>   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>