[LLVMdev] nsw is still logically inconsistent

[Dan Gohman]

On Dec 17, 2011, at 6:42 AM, Rafael Ávila de Espíndola wrote:

>> LICM on ToT hoists the div. Even speculatively.
>> 
>> The other thing is that div is just one example. The problem could
>> also come up with an array load with an index could be safe to
>> speculate if the index is known to be bounded. isDereferenceablePointer
>> currently doesn't know how to do this, but it'll probably get
>> smarter some day.
> 
> So, looking at just this example, it looks like semantics we assign to
> poison values have to do one of
> 
> *) Avoid the udiv being moved, since a valid removal of nsw would create
> a program that is now exposed to undefined behavior.

This could suppress legitimate optimizations. udiv is an expensive
operation on most targets, and being able to hoist it out of an
if statement inside a loop, as LICM does now, could be really
desirable in some cases.

> *) Say that use of poison values only produces other poison values, not
> undefined behavior.
> 
> The second option would give us a lot of liberty to move instruction
> around, but is a major change to what the IL looks like these days. In
> particular:
> 
> *) removing a nsw would become invalid, as it would change a poison
> value to a 0, which would cause udiv to have undefined behavior again.
> 
> *) Codegen would have to be very careful not to produce undefined
> behavior when handling instructions that potentially see poison values.
> 
> If we really want the ability to expose instructions with side effects
> to poison, maybe the best thing to do is to have poison resistant
> variants. For example, we would say that moving the udiv is invalid, but
> it could be replaced with a "udiv poison i1 1, %t5". The poison variant
> has the same definition as udiv, put produces poison when t5 is 0 (or
> poison) and therefore can be moved. The same goes for speculating a load.

udiv is just one example. In the case of an indexed load, this
would require codegen to do bounds checking, which I think is beyond
its charter here.

Dan