[llvm-commits] CVS: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

[Chris Lattner]

On Jun 27, 2007, at 1:50 PM, Dan Gohman wrote:

>>>> I think that undef udiv intmax -> 0, no?  If not, plz update
>>>> instcombine as well.
>>>
>>> intmax udiv intmax -> 1.
>>> It seems like folding undef/X to undef isn't safe either though,  
>>> with
>>> the way it sounds like undef is intended to work. This code:
>>>
>>>   %x = udiv i32 undef, %intmax
>>>   %y = udiv i32 %x, 2
>>>
>>> will always set %y to 0. Maybe instcombine can fold the second
>>> udiv by looking through its operands, but it can't safely fold the
>>> first. The best it could do is try to fold away all of %x's uses so
>>> that %x isn't needed anymore.

Duncan pointed out that I confused myself.  If something is undef, we  
can choose to pick any specific value for the undef to pick the  
cancellation.

>> Ug, excellent point.  At this point, I'm inclined to just give up
>> folding of udiv undefs.  What do you think?
>
> udiv isn't the only one, the way this is going...
>
>   %x = mul i32 undef, 2
>   %y = srem i32 %x, 2

This is fine, we fold the mul to 0 (because the undef could be zero).

>   %x = and i32 undef, 0xffff0000
>   %y = and i32 %x,    0x0000ffff
>
> and so on for a lot of others.

For and, we fold undef to 0 (because the undef could be 0)
For or undef, X, we fold to -1, because the undef could be -1.

>>> Even simple things like undef+X don't seem to be safe to fold.
>>>
>>>   %x = undef;
>>>   if (%x >= 0)
>>>     %z = %y / (%x + 1);         // don't divide by undef!
>>
>> Fortunately, this isn't a problem.  LLVM has no copy instruction, so
>> the code is really this:
>>
>>>   if (undef >= 0)
>>>     %z = %y / (undef + 1);         // don't divide by undef!
>>
>> There is nothing that specifies the two undefs are the same value.
>> Also, in C, if you have an undefined variable, you aren't guaranteed
>> to get the same undef value each time you read the variable, so
>> transforming C into LLVM is ok :)
>
> In C, an uninitialized variable has an "indeterminate value", which is
> potentially a trap representation, which can't even be multiplied by
> zero without incurring undefined behavior. I don't know where it
> suggests that a variable with indeterminate value might be different
> on each read though.

There have been discussions about this issue on the GCC list.  I  
remember the resolution (they take the same basic approach we do),  
but I don't remember why.  I think a DR may be submitted to the C  
committee on the issue.

IIRC, the basic reason this (allowing an undefined value to have  
multiple values) bites GCC is due to regalloc.  For example, if you  
have:

   int x;
   int y;

   y = 1;
   print(x, y);
   ...

   y = 2;
   print(x, y);

Because there is no live range for x (just uses) x and y can be  
allocated to the same register.  Doing so causes the value of x to  
follow the value of y.

> LLVM does so have copy instructions. The syntax is a little odd  
> though,
> and the keyword is spelled 'bitcast' ;-).

Point taken. :)

-Chris