[AArch64] Remove AArch64TargetLowering::BuildSDIVPow2 override

[RKSimon]

Summary

Now that D45806 etc has landed we've greatly improved the generic SDIV by constant handling in DAGCombiner.

While investigating how to support non-uniform vectors for TargetLowering::BuildSDIV (PR36545) it was noticed that removing the AArch64 override and using the generic path appears to improve codegen, but I'd like confirmation from arm specialists.

[efriedma]

The codegen was changed intentionally; see https://reviews.llvm.org/D4438 .

[RKSimon]

In D49174#1158894, @efriedma wrote:

The codegen was changed intentionally; see https://reviews.llvm.org/D4438 .

So should I abandon this?

[efriedma]

Yes, unless you have some evidence the previous benchmarking was wrong.

[RKSimon]

llvm-mca says something similar - abandoning

lib/Target/AArch64/AArch64ISelLowering.h

private:
   SDValue LowerVECREDUCE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerATOMIC_LOAD_SUB(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerATOMIC_LOAD_AND(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerDYNAMIC_STACKALLOC(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerWindowsDYNAMIC_STACKALLOC(SDValue Op, SDValue Chain,
                                          SDValue &Size,
                                          SelectionDAG &DAG) const;
 
-  SDValue BuildSDIVPow2(SDNode *N, const APInt &Divisor, SelectionDAG &DAG,
-                        std::vector<SDNode *> *Created) const override;
   SDValue getSqrtEstimate(SDValue Operand, SelectionDAG &DAG, int Enabled,
                           int &ExtraSteps, bool &UseOneConst,
                           bool Reciprocal) const override;
   SDValue getRecipEstimate(SDValue Operand, SelectionDAG &DAG, int Enabled,
                            int &ExtraSteps) const override;
   unsigned combineRepeatedFPDivisors() const override;
 
   ConstraintType getConstraintType(StringRef Constraint) const override;

lib/Target/AArch64/AArch64ISelLowering.cpp

if (DCI.isBeforeLegalizeOps())
     return SDValue();
 
   if (SDValue Cmp = foldVectorXorShiftIntoCmp(N, DAG, Subtarget))
     return Cmp;
 
   return performIntegerAbsCombine(N, DAG);
 }
 
-SDValue
-AArch64TargetLowering::BuildSDIVPow2(SDNode *N, const APInt &Divisor,
-                                     SelectionDAG &DAG,
-                                     std::vector<SDNode *> *Created) const {
-  AttributeList Attr = DAG.getMachineFunction().getFunction().getAttributes();
-  if (isIntDivCheap(N->getValueType(0), Attr))
-    return SDValue(N,0); // Lower SDIV as SDIV
-
-  // fold (sdiv X, pow2)
-  EVT VT = N->getValueType(0);
-  if ((VT != MVT::i32 && VT != MVT::i64) ||
-      !(Divisor.isPowerOf2() || (-Divisor).isPowerOf2()))
-    return SDValue();
-
-  SDLoc DL(N);
-  SDValue N0 = N->getOperand(0);
-  unsigned Lg2 = Divisor.countTrailingZeros();
-  SDValue Zero = DAG.getConstant(0, DL, VT);
-  SDValue Pow2MinusOne = DAG.getConstant((1ULL << Lg2) - 1, DL, VT);
-
-  // Add (N0 < 0) ? Pow2 - 1 : 0;
-  SDValue CCVal;
-  SDValue Cmp = getAArch64Cmp(N0, Zero, ISD::SETLT, CCVal, DAG, DL);
-  SDValue Add = DAG.getNode(ISD::ADD, DL, VT, N0, Pow2MinusOne);
-  SDValue CSel = DAG.getNode(AArch64ISD::CSEL, DL, VT, Add, N0, CCVal, Cmp);
-
-  if (Created) {
-    Created->push_back(Cmp.getNode());
-    Created->push_back(Add.getNode());
-    Created->push_back(CSel.getNode());
-  }
-
-  // Divide by pow2.
-  SDValue SRA =
-      DAG.getNode(ISD::SRA, DL, VT, CSel, DAG.getConstant(Lg2, DL, MVT::i64));
-
-  // If we're dividing by a positive value, we're done.  Otherwise, we must
-  // negate the result.
-  if (Divisor.isNonNegative())
-    return SRA;
-
-  if (Created)
-    Created->push_back(SRA.getNode());
-  return DAG.getNode(ISD::SUB, DL, VT, DAG.getConstant(0, DL, VT), SRA);
-}
-
 static SDValue performMulCombine(SDNode *N, SelectionDAG &DAG,
                                  TargetLowering::DAGCombinerInfo &DCI,
                                  const AArch64Subtarget *Subtarget) {
   if (DCI.isBeforeLegalizeOps())
     return SDValue();
 
   // The below optimizations require a constant RHS.
   if (!isa<ConstantSDNode>(N->getOperand(1)))

test/CodeGen/AArch64/fast-isel-sdiv.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64-linux-gnu                               -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,ISEL
 ; RUN: llc -mtriple=aarch64-linux-gnu -fast-isel -fast-isel-abort=1 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,FAST
 
 define i32 @sdiv_i32_exact(i32 %a) {
 ; CHECK-LABEL: sdiv_i32_exact:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    asr w0, w0, #3
 ; CHECK-NEXT:    ret
   %1 = sdiv exact i32 %a, 8
   ret i32 %1
 }
 
 define i32 @sdiv_i32_pos(i32 %a) {
-; CHECK-LABEL: sdiv_i32_pos:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add w8, w0, #7 // =7
-; CHECK-NEXT:    cmp w0, #0 // =0
-; CHECK-NEXT:    csel w8, w8, w0, lt
-; CHECK-NEXT:    asr w0, w8, #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: sdiv_i32_pos:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr w8, w0, #31
+; ISEL-NEXT:    add w8, w0, w8, lsr #29
+; ISEL-NEXT:    asr w0, w8, #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: sdiv_i32_pos:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add w8, w0, #7 // =7
+; FAST-NEXT:    cmp w0, #0 // =0
+; FAST-NEXT:    csel w8, w8, w0, lt
+; FAST-NEXT:    asr w0, w8, #3
+; FAST-NEXT:    ret
   %1 = sdiv i32 %a, 8
   ret i32 %1
 }
 
 define i32 @sdiv_i32_neg(i32 %a) {
-; CHECK-LABEL: sdiv_i32_neg:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add w8, w0, #7 // =7
-; CHECK-NEXT:    cmp w0, #0 // =0
-; CHECK-NEXT:    csel w8, w8, w0, lt
-; CHECK-NEXT:    neg w0, w8, asr #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: sdiv_i32_neg:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr w8, w0, #31
+; ISEL-NEXT:    add w8, w0, w8, lsr #29
+; ISEL-NEXT:    neg w0, w8, asr #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: sdiv_i32_neg:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add w8, w0, #7 // =7
+; FAST-NEXT:    cmp w0, #0 // =0
+; FAST-NEXT:    csel w8, w8, w0, lt
+; FAST-NEXT:    neg w0, w8, asr #3
+; FAST-NEXT:    ret
   %1 = sdiv i32 %a, -8
   ret i32 %1
 }
 
 define i64 @sdiv_i64_exact(i64 %a) {
 ; CHECK-LABEL: sdiv_i64_exact:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    asr x0, x0, #4
 ; CHECK-NEXT:    ret
   %1 = sdiv exact i64 %a, 16
   ret i64 %1
 }
 
 define i64 @sdiv_i64_pos(i64 %a) {
-; CHECK-LABEL: sdiv_i64_pos:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add x8, x0, #15 // =15
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    asr x0, x8, #4
-; CHECK-NEXT:    ret
+; ISEL-LABEL: sdiv_i64_pos:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #60
+; ISEL-NEXT:    asr x0, x8, #4
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: sdiv_i64_pos:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add x8, x0, #15 // =15
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    asr x0, x8, #4
+; FAST-NEXT:    ret
   %1 = sdiv i64 %a, 16
   ret i64 %1
 }
 
 define i64 @sdiv_i64_neg(i64 %a) {
-; CHECK-LABEL: sdiv_i64_neg:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add x8, x0, #15 // =15
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    neg x0, x8, asr #4
-; CHECK-NEXT:    ret
+; ISEL-LABEL: sdiv_i64_neg:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #60
+; ISEL-NEXT:    neg x0, x8, asr #4
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: sdiv_i64_neg:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add x8, x0, #15 // =15
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    neg x0, x8, asr #4
+; FAST-NEXT:    ret
   %1 = sdiv i64 %a, -16
   ret i64 %1
 }

test/CodeGen/AArch64/sdivpow2.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64-linux-gnu -fast-isel=0 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,ISEL
 ; RUN: llc -mtriple=aarch64-linux-gnu -fast-isel=1 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,FAST
 
 define i32 @test1(i32 %x) {
-; CHECK-LABEL: test1:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add w8, w0, #7 // =7
-; CHECK-NEXT:    cmp w0, #0 // =0
-; CHECK-NEXT:    csel w8, w8, w0, lt
-; CHECK-NEXT:    asr w0, w8, #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test1:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr w8, w0, #31
+; ISEL-NEXT:    add w8, w0, w8, lsr #29
+; ISEL-NEXT:    asr w0, w8, #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test1:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add w8, w0, #7 // =7
+; FAST-NEXT:    cmp w0, #0 // =0
+; FAST-NEXT:    csel w8, w8, w0, lt
+; FAST-NEXT:    asr w0, w8, #3
+; FAST-NEXT:    ret
   %div = sdiv i32 %x, 8
   ret i32 %div
 }
 
 define i32 @test2(i32 %x) {
-; CHECK-LABEL: test2:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add w8, w0, #7 // =7
-; CHECK-NEXT:    cmp w0, #0 // =0
-; CHECK-NEXT:    csel w8, w8, w0, lt
-; CHECK-NEXT:    neg w0, w8, asr #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test2:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr w8, w0, #31
+; ISEL-NEXT:    add w8, w0, w8, lsr #29
+; ISEL-NEXT:    neg w0, w8, asr #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test2:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add w8, w0, #7 // =7
+; FAST-NEXT:    cmp w0, #0 // =0
+; FAST-NEXT:    csel w8, w8, w0, lt
+; FAST-NEXT:    neg w0, w8, asr #3
+; FAST-NEXT:    ret
   %div = sdiv i32 %x, -8
   ret i32 %div
 }
 
 define i32 @test3(i32 %x) {
-; CHECK-LABEL: test3:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add w8, w0, #31 // =31
-; CHECK-NEXT:    cmp w0, #0 // =0
-; CHECK-NEXT:    csel w8, w8, w0, lt
-; CHECK-NEXT:    asr w0, w8, #5
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test3:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr w8, w0, #31
+; ISEL-NEXT:    add w8, w0, w8, lsr #27
+; ISEL-NEXT:    asr w0, w8, #5
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test3:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add w8, w0, #31 // =31
+; FAST-NEXT:    cmp w0, #0 // =0
+; FAST-NEXT:    csel w8, w8, w0, lt
+; FAST-NEXT:    asr w0, w8, #5
+; FAST-NEXT:    ret
   %div = sdiv i32 %x, 32
   ret i32 %div
 }
 
 define i64 @test4(i64 %x) {
-; CHECK-LABEL: test4:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add x8, x0, #7 // =7
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    asr x0, x8, #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test4:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #61
+; ISEL-NEXT:    asr x0, x8, #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test4:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add x8, x0, #7 // =7
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    asr x0, x8, #3
+; FAST-NEXT:    ret
   %div = sdiv i64 %x, 8
   ret i64 %div
 }
 
 define i64 @test5(i64 %x) {
-; CHECK-LABEL: test5:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add x8, x0, #7 // =7
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    neg x0, x8, asr #3
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test5:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #61
+; ISEL-NEXT:    neg x0, x8, asr #3
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test5:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add x8, x0, #7 // =7
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    neg x0, x8, asr #3
+; FAST-NEXT:    ret
   %div = sdiv i64 %x, -8
   ret i64 %div
 }
 
 define i64 @test6(i64 %x) {
-; CHECK-LABEL: test6:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    add x8, x0, #63 // =63
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    asr x0, x8, #6
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test6:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #58
+; ISEL-NEXT:    asr x0, x8, #6
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test6:
+; FAST:       // %bb.0:
+; FAST-NEXT:    add x8, x0, #63 // =63
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    asr x0, x8, #6
+; FAST-NEXT:    ret
   %div = sdiv i64 %x, 64
   ret i64 %div
 }
 
 define i64 @test7(i64 %x) {
-; CHECK-LABEL: test7:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    orr x8, xzr, #0xffffffffffff
-; CHECK-NEXT:    add x8, x0, x8
-; CHECK-NEXT:    cmp x0, #0 // =0
-; CHECK-NEXT:    csel x8, x8, x0, lt
-; CHECK-NEXT:    asr x0, x8, #48
-; CHECK-NEXT:    ret
+; ISEL-LABEL: test7:
+; ISEL:       // %bb.0:
+; ISEL-NEXT:    asr x8, x0, #63
+; ISEL-NEXT:    add x8, x0, x8, lsr #16
+; ISEL-NEXT:    asr x0, x8, #48
+; ISEL-NEXT:    ret
+;
+; FAST-LABEL: test7:
+; FAST:       // %bb.0:
+; FAST-NEXT:    orr x8, xzr, #0xffffffffffff
+; FAST-NEXT:    add x8, x0, x8
+; FAST-NEXT:    cmp x0, #0 // =0
+; FAST-NEXT:    csel x8, x8, x0, lt
+; FAST-NEXT:    asr x0, x8, #48
+; FAST-NEXT:    ret
   %div = sdiv i64 %x, 281474976710656
   ret i64 %div
 }