[llvm] r301990 - [PowerPC, DAGCombiner] Fold a << (b % (sizeof(a) * 8)) back to a single instruction

[Friedman, Eli via llvm-commits]

On 5/3/2017 11:43 AM, Friedman, Eli wrote:
> On 5/2/2017 5:07 PM, Tim Shen via llvm-commits wrote:
>> Author: timshen
>> Date: Tue May  2 19:07:02 2017
>> New Revision: 301990
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=301990&view=rev
>> Log:
>> [PowerPC, DAGCombiner] Fold a << (b % (sizeof(a) * 8)) back to a 
>> single instruction
>>
>> Summary:
>> This is the corresponding llvm change to D28037 to ensure no performance
>> regression.
>>
>> Reviewers: bogner, kbarton, hfinkel, iteratee, echristo
>>
>> Subscribers: nemanjai, llvm-commits
>>
>> Differential Revision: https://reviews.llvm.org/D28329
>>
>> Modified:
>>      llvm/trunk/include/llvm/Target/TargetLowering.h
>>      llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>      llvm/trunk/lib/Target/PowerPC/PPCISelLowering.h
>>      llvm/trunk/test/CodeGen/PowerPC/shift_mask.ll
>>
>> Modified: llvm/trunk/include/llvm/Target/TargetLowering.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetLowering.h?rev=301990&r1=301989&r2=301990&view=diff
>> ============================================================================== 
>>
>> --- llvm/trunk/include/llvm/Target/TargetLowering.h (original)
>> +++ llvm/trunk/include/llvm/Target/TargetLowering.h Tue May  2 
>> 19:07:02 2017
>> @@ -2061,6 +2061,14 @@ public:
>>       return false;
>>     }
>>   +  // Return true if the instruction that performs a << b actually 
>> performs
>> +  // a << (b % (sizeof(a) * 8)).
>> +  virtual bool supportsModuloShift(ISD::NodeType Inst, EVT 
>> ReturnType) const {
>> +    assert((Inst == ISD::SHL || Inst == ISD::SRA || Inst == 
>> ISD::SRL) &&
>> +           "Expect a shift instruction");
>> +    return false;
>> +  }
>> +
>> //===--------------------------------------------------------------------===//
>>     // Runtime Library hooks
>>     //
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=301990&r1=301989&r2=301990&view=diff
>> ============================================================================== 
>>
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue May 2 
>> 19:07:02 2017
>> @@ -5294,6 +5294,17 @@ SDValue DAGCombiner::visitSHL(SDNode *N)
>>       }
>>     }
>>   +  // If the target supports masking y in (shl, y),
>> +  // fold (shl x, (and y, ((1 << numbits(x)) - 1))) -> (shl x, y)
>> +  if (TLI.isOperationLegal(ISD::SHL, VT) &&
>> +      TLI.supportsModuloShift(ISD::SHL, VT) && N1->getOpcode() == 
>> ISD::AND) {
>> +    if (ConstantSDNode *Mask = 
>> isConstOrConstSplat(N1->getOperand(1))) {
>> +      if (Mask->getZExtValue() == OpSizeInBits - 1) {
>> +        return DAG.getNode(ISD::SHL, SDLoc(N), VT, N0, 
>> N1->getOperand(0));
>> +      }
>> +    }
>> +  }
>> +
>>     ConstantSDNode *N1C = isConstOrConstSplat(N1);
>>       // fold (shl c1, c2) -> c1<<c2
>> @@ -5492,6 +5503,17 @@ SDValue DAGCombiner::visitSRA(SDNode *N)
>>     EVT VT = N0.getValueType();
>>     unsigned OpSizeInBits = VT.getScalarSizeInBits();
>>   +  // If the target supports masking y in (sra, y),
>> +  // fold (sra x, (and y, ((1 << numbits(x)) - 1))) -> (sra x, y)
>> +  if (TLI.isOperationLegal(ISD::SRA, VT) &&
>> +      TLI.supportsModuloShift(ISD::SRA, VT) && N1->getOpcode() == 
>> ISD::AND) {
>> +    if (ConstantSDNode *Mask = 
>> isConstOrConstSplat(N1->getOperand(1))) {
>> +      if (Mask->getZExtValue() == OpSizeInBits - 1) {
>> +        return DAG.getNode(ISD::SRA, SDLoc(N), VT, N0, 
>> N1->getOperand(0));
>> +      }
>> +    }
>> +  }
>> +
>>     // Arithmetic shifting an all-sign-bit value is a no-op.
>>     if (DAG.ComputeNumSignBits(N0) == OpSizeInBits)
>>       return N0;
>> @@ -5650,6 +5672,17 @@ SDValue DAGCombiner::visitSRL(SDNode *N)
>>     EVT VT = N0.getValueType();
>>     unsigned OpSizeInBits = VT.getScalarSizeInBits();
>>   +  // If the target supports masking y in (srl, y),
>> +  // fold (srl x, (and y, ((1 << numbits(x)) - 1))) -> (srl x, y)
>> +  if (TLI.isOperationLegal(ISD::SRL, VT) &&
>> +      TLI.supportsModuloShift(ISD::SRL, VT) && N1->getOpcode() == 
>> ISD::AND) {
>> +    if (ConstantSDNode *Mask = 
>> isConstOrConstSplat(N1->getOperand(1))) {
>> +      if (Mask->getZExtValue() == OpSizeInBits - 1) {
>> +        return DAG.getNode(ISD::SRL, SDLoc(N), VT, N0, 
>> N1->getOperand(0));
>> +      }
>> +    }
>> +  }
>
> I see three problems here:
>
> 1. It needs to be documented.
> 2. This isn't consistent with other transforms done by DAGCombine, 
> e.g. constant folding won't honor this.
> 3. I'm not sure we want to do this; are we actually getting any 
> benefit here over letting targets pattern-match the pattern? 

Ping.

-Eli

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