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

[Friedman, Eli via llvm-commits]

On 11/9/2016 11:16 AM, Davide Italiano via llvm-commits wrote:
> On Wed, Nov 9, 2016 at 9:49 AM, Daniel Berlin via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> 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)
>>
> Uh, thanks. Out of curiosity, do you happen to know why undef wasn't
> represented as a lattice value from the beginning (and instead we end
> up trying values that match up?) Is there a particular reason (e.g.
> efficiency)? I can't think of one.
> --
This is very old code; see r17044, r18776 and r32715.  I doubt Chris 
remembers the details (but CC'ing him in case he does).  I haven't 
really considered it before, but as far as I know, there isn't any 
particular reason we can't treat undef like a constant rather than 
"unknown".  Constant folding knows how to fold operations involving 
undef correctly.  You probably need to special-case branches and PHI 
nodes with undef operands.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project