[PATCH] D138904: [AArch64] Transform shift+and to shift+shift to select more shifted register

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:689-692
 bool AArch64DAGToDAGISel::SelectShiftedRegister(SDValue N, bool AllowROR,
                                                 SDValue &Reg, SDValue &Shift) {
+  if (SelectShiftedRegisterFromAnd(N, Reg, Shift))
+    return true;
----------------
mingmingl wrote:
> Selecting `and (shl/srl/sra, x, c), mask` into `shl (srl/sra, x, c1), c2` (and folding shl) is simplifying one instruction away but could change the pipeline (latency and throughput as well) of the instruciton.
> 
> For example, arithmetic operations with shifted operand could use M pipeline [1] on neoverse n1, and M pipeline is used for all ALU operations with imm-shifted operand for cortex a57. 
> 
> I wonder if improvements are seen in some benchmarks from this patch?
> 
> [1] for neoverse n1,  M pipeline is used for {ADDS, SUBS}  with "Arithmetic, LSR/ASR/ROR shift or LSL shift > 4".
I agree that it  may not get any timing improvement except lsl < 4. I also don't believe this patch can get improvement in any real benchmarks. Actually the current shifted register select function also does not consider for the thing you mentioned. If you really worry about that I can limit the lsl constant < 4.But personally I don't want to do that.


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

https://reviews.llvm.org/D138904