[PATCH] D66882: [DAGCombiner] Match (add X, X) as (shl X, 1) when detecting rotate.

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix marked an inline comment as done.
deadalnix added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6145
   SDValue NewShiftNode = DAG.getConstant(NeededShiftAmt, DL, ShiftVT);
   return DAG.getNode(Opcode, DL, ResVT, OppShiftLHS, NewShiftNode);
 }
----------------
lebedev.ri wrote:
> deadalnix wrote:
> > This is where the shift node is built.
> Err, this is actually exactly what i'm saying :)
> I'm simply wondering if the current implementation, i.e.
> ```
>   // Value and Type of the shift.
>   SDValue OppShiftLHS = OppShift.getOperand(0);
>   EVT ShiftedVT = OppShiftLHS.getValueType();
> 
>   // Amount of the existing shift.
>   ConstantSDNode *OppShiftCst = isConstOrConstSplat(OppShift.getOperand(1));
> 
>   // (add v v) -> (shl v 1)
>   if (OppShift.getOpcode() == ISD::SRL && OppShiftCst &&
>       ExtractFrom.getOpcode() == ISD::ADD &&
>       ExtractFrom.getOperand(0) == ExtractFrom.getOperand(1) &&
>       ExtractFrom.getOperand(0) == OppShiftLHS &&
>       OppShiftCst->getAPIntValue() == ShiftedVT.getScalarSizeInBits() - 1)
>     return DAG.getNode(ISD::SHL, DL, ShiftedVT, OppShiftLHS,
>                        DAG.getShiftAmountConstant(1, ShiftedVT, DL));
> ```
> is the simplest solution here, or the code that is located later in the function
> can be refactored to handle this new `add %x, %x` case too, just like it already
> handles div/rem nodes?
The code is written in a style where it bail when something is wrong. As a result it either need to be rewritten, or new patterns need to be added early on.

Sorry for not understanding what you were hinting at before.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66882/new/

https://reviews.llvm.org/D66882