[llvm] r237645 - DAGCombiner: Factor common pattern into isOneConstant() function. NFC

[qcolombet]

> On May 19, 2015, at 10:42 AM, Matthias Braun <matze at braunis.de> wrote:
> 
> 
>> On May 19, 2015, at 9:59 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> Hi Matthias,
>> 
>> 
>>> On May 18, 2015, at 5:25 PM, Matthias Braun <matze at braunis.de> wrote:
>>> 
>>> Author: matze
>>> Date: Mon May 18 19:25:21 2015
>>> New Revision: 237645
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=237645&view=rev
>>> Log:
>>> DAGCombiner: Factor common pattern into isOneConstant() function. NFC
>>> 
>>> Modified:
>>>  llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>> 
>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=237645&r1=237644&r2=237645&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon May 18 19:25:21 2015
>>> @@ -1590,6 +1590,11 @@ static bool isAllOnesConstant(SDValue V)
>>> return Const != nullptr && Const->isAllOnesValue();
>>> }
>>> 
>>> +static bool isOneConstant(SDValue V) {
>>> +  ConstantSDNode *Const = dyn_cast<ConstantSDNode>(V);
>>> +  return Const != nullptr && Const->isOne();
>>> +}
>>> +
>>> SDValue DAGCombiner::visitADD(SDNode *N) {
>>> SDValue N0 = N->getOperand(0);
>>> SDValue N1 = N->getOperand(1);
>>> @@ -1722,13 +1727,12 @@ SDValue DAGCombiner::visitADD(SDNode *N)
>>> 
>>> if (N1.getOpcode() == ISD::AND) {
>>>   SDValue AndOp0 = N1.getOperand(0);
>>> -    ConstantSDNode *AndOp1 = dyn_cast<ConstantSDNode>(N1->getOperand(1));
>>>   unsigned NumSignBits = DAG.ComputeNumSignBits(AndOp0);
>>>   unsigned DestBits = VT.getScalarType().getSizeInBits();
>>> 
>>>   // (add z, (and (sbbl x, x), 1)) -> (sub z, (sbbl x, x))
>>>   // and similar xforms where the inner op is either ~0 or 0.
>>> -    if (NumSignBits == DestBits && AndOp1 && AndOp1->isOne()) {
>>> +    if (NumSignBits == DestBits && isOneConstant(N1->getOperand(1))) {
>>>     SDLoc DL(N);
>>>     return DAG.getNode(ISD::SUB, DL, VT, N->getOperand(0), AndOp0);
>>>   }
>>> @@ -2123,7 +2127,7 @@ SDValue DAGCombiner::visitSDIV(SDNode *N
>>> if (N0C && N1C && !N1C->isNullValue())
>>>   return DAG.FoldConstantArithmetic(ISD::SDIV, SDLoc(N), VT, N0C, N1C);
>>> // fold (sdiv X, 1) -> X
>>> -  if (N1C && N1C->getAPIntValue() == 1LL)
>>> +  if (N1C && N1C->isOne())
>> 
>> Simply N1C->isOneConstant()?
> 
> Unfortunately no, at this (and several other places) the code doesn't just check for ConstantSDNodes but also for ConstSplats and sets N1C accordingly so exchanging with isOneConstant(X) changes the semantics. I didn't want to introduce a isOneContantOrSplat function for a handful of places.

Thanks for the clarification.
I think that is debatable but definitely for another clean-up :).

Q.
> 
> - Matthias