[llvm] r284239 - [DAG] add folds for negated shifted sign bit

[Sanjay Patel via llvm-commits]

Thanks, Eli!
Hopefully fixed with:
https://reviews.llvm.org/rL284268

Still looking for a test case.

On Fri, Oct 14, 2016 at 12:04 PM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 10/14/2016 7:26 AM, Sanjay Patel via llvm-commits wrote:
>
>> Author: spatel
>> Date: Fri Oct 14 09:26:47 2016
>> New Revision: 284239
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=284239&view=rev
>> Log:
>> [DAG] add folds for negated shifted sign bit
>>
>> The same folds exist in InstCombine already.
>>
>> This came up as part of:
>> https://reviews.llvm.org/D25485
>>
>>
>> Modified:
>>      llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>      llvm/trunk/test/CodeGen/X86/negate-shift.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/
>> SelectionDAG/DAGCombiner.cpp?rev=284239&r1=284238&r2=284239&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri Oct 14
>> 09:26:47 2016
>> @@ -1954,6 +1954,19 @@ SDValue DAGCombiner::visitSUB(SDNode *N)
>>                          DAG.getConstant(-N1C->getAPIntValue(), DL, VT));
>>     }
>>   +  // Right-shifting everything out but the sign bit followed by
>> negation is the
>> +  // same as flipping arithmetic/logical shift type without the negation:
>> +  // -(X >>u 31) -> (X >>s 31)
>> +  // -(X >>s 31) -> (X >>u 31)
>> +  if (isNullConstantOrNullSplatConstant(N0) &&
>> +      (N1->getOpcode() == ISD::SRA || N1->getOpcode() == ISD::SRL)) {
>> +    ConstantSDNode *ShiftAmt = isConstOrConstSplat(N1.getOperand(1));
>> +    if (ShiftAmt && ShiftAmt->getZExtValue() == VT.getScalarSizeInBits()
>> - 1) {
>> +      auto NewOpcode = N1->getOpcode() == ISD::SRA ? ISD::SRL :ISD::SRA;
>> +      return DAG.getNode(NewOpcode, DL, VT, N1.getOperand(0),
>> N1.getOperand(1));
>> +    }
>> +  }
>>
>
> You need to check whether the new node is legal.  It's possible to have a
> target where SRL is legal, but SRA isn't.
>
> Not sure what targets would actually trigger this issue off the top of my
> head, but they probably exist. SSE2 is missing an arithmetic shift right
> for <2 x i64>, but you can't trigger the issue there because it doesn't use
> ISD::SRL for vectors after legalization.
>
> -Eli
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161014/69a08782/attachment.html>