[llvm-dev] funnel shift, select, and poison

[Nuno Lopes via llvm-dev]

You are right: select in SDAG has to be poison-blocking as well,  
otherwise the current lowering from IR's select to SDAG's select would  
be wrong. Which makes the select->or transformation incorrect at SDAG  
level as well.
I guess until recently people believed that poison in SDAG wasn't much  
of a problem (myself included). I was convinced otherwise with the  
test cases that Juneyoung found a few weeks ago:  
https://bugs.llvm.org/show_bug.cgi?id=40657

MI doesn't have poison AFAIK, so at least we are safe there, I hope  
(not sure about undef, though).

I don't know if there was a thorough discussion on the performance  
benefits of having poison in SDAG, though. Is it that important? are  
there loop-related optimizations that require nsw information?
If so, then we should pay the price and fix SDAG. (and extend Alive to  
verify SDAG as well -- ok, I'm hanging myself here, but..). If not,  
maybe getting rid of poison in SDAG could be a good thing.
There's also undef, which I don't know exactly what's the semantics,  
but AFAIR is as bad as the IR's.

Nuno


Quoting Sanjay Patel via llvm-dev <llvm-dev at lists.llvm.org>:

> I don't object to deferring the optimization, but let me check my poison
> understanding...
>   select i1 %cond, i1 true, i1 %x --> or i1 %cond, %x
>
> 1. 'select' is a poison-blocking operation, but 'or' is
> non-poison-blocking, so we are propagating a potentially poisonous %x with
> this transform.
> 2. We will fix the problem in IR by removing this transform in IR
> 3. The backend (SDAG) has that same transform.
> 4. SDAG has poison because we propagate the wrapping/exact flags to DAG
> nodes.
> 5. Are we just sweeping the bug under the rug? Nobody cares because SDAG is
> undocumented, so anything goes down there?
>
>
> On Tue, Feb 26, 2019 at 2:06 PM John Regehr via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> > Transforms/InstCombine/select.ll
>> > ================================
>> > define i1 @trueval_is_true(i1 %C, i1 %X) {
>> >   %R = select i1 %C, i1 1, i1 %X
>> >   ret i1 %R
>> > }
>> > =>
>> > define i1 @trueval_is_true(i1 %C, i1 %X) {
>> >   %R = or i1 %C, %X
>> >   ret i1 %R
>> > }
>> > ERROR: Target is more poisonous than source (when %C = #x1 & %X = poison)
>> >
>> > (there are many variations of these select->arithmetic transformations)
>>
>> This particular little family of transformations can be reliably done by
>> all of the backends I looked at, so disabling them at the IR level
>> should be OK. See a bit more discussion here:
>>
>> > https://bugs.llvm.org/show_bug.cgi?id=40768
>>
>> John