[PATCH] D26432: [SCCP] Force unknown branches to overdefined

[Daniel Berlin via llvm-commits]

Actually, digging further into it,  I don't understand how this fixes it at
all, and your test does nothing because it's all CHECK's :-)

The example given in the bug report is or true, undef

ResolveUndefs already does the right thing for this, and does not consider
the result undef.

The propagation order makes it impossible for that to happen, because in
reality, undef is a set of lattice values but not represented in the
lattice, and resolveUndefs is trying to figure out the lattice values that
work.

That's ... never going to work, as you've discovered (and eli suspects on
the bug report).

I suspect pretty much every part of markForcedConstant is vulnerable to
this undef different circumstances.

Realistically, undef needs to be an actual lattice value lower than unknown

That way, or undef, true comes out true during *solving*.
The parts of resolveundefs handling this need to be made part of the meet
operation

This handles the case where undef must be constrained (IE or undef, true).

After solving is finished, anything left as undef can then be filled in
unconstrained, because you are guaranteed it is not forced to a certain
value (or that your solver sucks :P)

(note that in the freeze/poison proposal, this should just work because the
freezes will be different ssa names)






On Tue, Nov 8, 2016 at 4:50 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

> So, it looks like what happens here is you expect undef to be a
> constrainable value.
>
> That is, for "or true, undef", you expect it to be true, despite the undef
> being there.
>
> Adding an undef expert to say whether this expectation is reasonable.
>
> (IE whether undef is allowed to have super-magical possibilities, such
> that the compiler can pick a value that the or returns false).
>
> Otherwise, FWIW, it seems simple to fix "right" (just constant fold it
> using the symbolic operands and if it returns true/false, use that)
>
>
>
>
> On Tue, Nov 8, 2016 at 4:39 PM, Davide Italiano via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> davide added a comment.
>>
>> This fixes the miscompile reported in PR30448. I tried  on many internal
>> tests and bootstrapping clang, and it works without problems. We lose some
>> optimization power, but it seems that the pattern is rare enough so we
>> shouldn't really worry.
>>
>>
>> https://reviews.llvm.org/D26432
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161109/df5da489/attachment.html>