[PATCH] D112389: [SCEV] Move SCEVLostPoisonFlags() check into SCEVExpander

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D112389#3084958 <https://reviews.llvm.org/D112389#3084958>, @reames wrote:

> So, LGTM to the code motion, but I think this code is badly incomplete.
>
> If I understand it correctly, the intent is to prevent SCEVExpander from reusing an IR instruction with flags stronger than those implied by the SCEV.  This is a laudable goal, but the code completely ignores folding of the binop during SCEV construction.

Right. I agree that the handling is incomplete, though it might be tricky to come up with cases where it fails in practice.

> I see two correct approaches:
>
> 1. Default to returning true (meaning can't reuse) unless we can prove flag match.  This is somewhat the inverse of the current code.
> 2. drop poison generating flags from the IR instruction
>
> We could, of course, use some combination of both.
>
> Any chance you'd be willing to follow up here?  If not, I probably will.

I don't plan to look into this one myself -- I only ran into this while looking at invalidation.


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

https://reviews.llvm.org/D112389