[RISCV]Enable isIntDivCheap when attribute is minsize

[liaolucy]

Summary

Don't expand divisions by constants when attribute is minsize.

[craig.topper]

llvm/test/CodeGen/RISCV/div_minsize.ll

9

RV32 and RV64 aren't in your RUN lines

34

This is overly indented

[liaolucy]

address comments. update testcase

[craig.topper]

This code in ReplaceNodeResults needs to call isIntDivCheap so that we promote div by constant when isIntDivCheap returns false. That will fix testsize2 and testsize4 to use divw and divuw for RV64IM.

case ISD::SDIV:                                                                
case ISD::UDIV:                                                                
case ISD::UREM: {                                                              
  MVT VT = N->getSimpleValueType(0);                                           
  assert((VT == MVT::i8 || VT == MVT::i16 || VT == MVT::i32) &&                
         Subtarget.is64Bit() && Subtarget.hasStdExtM() &&                      
         "Unexpected custom legalisation");                                    
  // Don't promote division/remainder by constant since we should expand those 
  // to multiply by magic constant.                                            
  // FIXME: What if the expansion is disabled for minsize.                     
  if (N->getOperand(1).getOpcode() == ISD::Constant)                           
    return;

[liaolucy]

address comments. fix testcases to use divw and divuw for RV64IM.

[barannikov88]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12541

There is MF->getFunction().hasOptSize() which also checks for Attribute::OptimizeForSize.
Just a suggestion.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12541

Every other target that does this only does it for MinSize. Are you suggesting we do this for -Os as well?

[barannikov88]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12541

That's right.
According to the comment near hasOptSize, it checks for Os and Oz, one of which is "optimize for size" and the other is "optimize for minimum size".
Has the other attribute been introduced after other targets implemented the hook?..

[barannikov88]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12541

Please ignore my comment, I mixed up OptimizeForSize and MinSize. Returning false only for MinSize sounds reasonable.

[craig.topper]

LGTM

[• pcwang-thead]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12540

The reason why other targets make vector an exception is because there is no vector integer division as the comments said, so I think it is OK for RVV since there are vector integer divide instructions. While for P extension, it could be an exception.

[liaolucy]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12540

Yes, that is why I added a TODO.

llvm/lib/Target/RISCV/RISCVISelLowering.h

bool shouldProduceAndByConstByHoistingConstFromShiftsLHSOfAnd(
       SelectionDAG &DAG) const override;
   bool shouldSinkOperands(Instruction *I,
                           SmallVectorImpl<Use *> &Ops) const override;
   bool shouldScalarizeBinop(SDValue VecOp) const override;
   bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const override;
   bool isFPImmLegal(const APFloat &Imm, EVT VT,
                     bool ForCodeSize) const override;
 
+  bool isIntDivCheap(EVT VT, AttributeList Attr) const override;
+
   bool softPromoteHalfType() const override { return true; }
 
   /// Return the register type for a given MVT, ensuring vectors are treated
   /// as a series of gpr sized integers.
   MVT getRegisterTypeForCallingConv(LLVMContext &Context, CallingConv::ID CC,
                                     EVT VT) const override;
 
   /// Return the number of registers for a given MVT, ensuring vectors are

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

void RISCVTargetLowering::ReplaceNodeResults(SDNode *N,
   case ISD::UDIV:
   case ISD::UREM: {
     MVT VT = N->getSimpleValueType(0);
     assert((VT == MVT::i8 || VT == MVT::i16 || VT == MVT::i32) &&
            Subtarget.is64Bit() && Subtarget.hasStdExtM() &&
            "Unexpected custom legalisation");
     // Don't promote division/remainder by constant since we should expand those
     // to multiply by magic constant.
-    // FIXME: What if the expansion is disabled for minsize.
-    if (N->getOperand(1).getOpcode() == ISD::Constant)
+    AttributeList Attr = DAG.getMachineFunction().getFunction().getAttributes();
+    if (N->getOperand(1).getOpcode() == ISD::Constant &&
+        !isIntDivCheap(N->getValueType(0), Attr))
       return;
 
     // If the input is i32, use ANY_EXTEND since the W instructions don't read
     // the upper 32 bits. For other types we need to sign or zero extend
     // based on the opcode.
     unsigned ExtOpc = ISD::ANY_EXTEND;
     if (VT != MVT::i32)
       ExtOpc = N->getOpcode() == ISD::SDIV ? ISD::SIGN_EXTEND
RISCVTargetLowering::BuildSDIVPow2(SDNode *N, const APInt &Divisor,
   // negate the result.
   if (Divisor.isNonNegative())
     return SRA;
 
   Created.push_back(SRA.getNode());
   return DAG.getNode(ISD::SUB, DL, VT, DAG.getConstant(0, DL, VT), SRA);
 }
 
+bool RISCVTargetLowering::isIntDivCheap(EVT VT, AttributeList Attr) const {
+  // When aggressively optimizing for code size, we prefer to use a div
+  // instruction, as it is usually smaller than the alternative sequence.
+  // TODO: Add vector division?
+  bool OptSize = Attr.hasFnAttr(Attribute::MinSize);
+  return OptSize && !VT.isVector();
+}
+
 #define GET_REGISTER_MATCHER
 #include "RISCVGenAsmMatcher.inc"
 
 Register
 RISCVTargetLowering::getRegisterByName(const char *RegName, LLT VT,
                                        const MachineFunction &MF) const {
   Register Reg = MatchRegisterAltName(RegName);
   if (Reg == RISCV::NoRegister)

llvm/test/CodeGen/RISCV/div_minsize.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+m -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IM %s
+; RUN: llc -mtriple=riscv64 -mattr=+m -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IM %s
+
+define i32 @testsize1(i32 %x) minsize nounwind {
+; RV32IM-LABEL: testsize1:
+; RV32IM:       # %bb.0: # %entry
+; RV32IM-NEXT:    li a1, 32
+; RV32IM-NEXT:    div a0, a0, a1
+; RV32IM-NEXT:    ret
+;
+; RV64IM-LABEL: testsize1:
+; RV64IM:       # %bb.0: # %entry
+; RV64IM-NEXT:    li a1, 32
+; RV64IM-NEXT:    divw a0, a0, a1
+; RV64IM-NEXT:    ret
+entry:
+  %div = sdiv i32 %x, 32
+  ret i32 %div
+}
+
+define i32 @testsize2(i32 %x) minsize nounwind {
+; RV32IM-LABEL: testsize2:
+; RV32IM:       # %bb.0: # %entry
+; RV32IM-NEXT:    li a1, 33
+; RV32IM-NEXT:    div a0, a0, a1
+; RV32IM-NEXT:    ret
+;
+; RV64IM-LABEL: testsize2:
+; RV64IM:       # %bb.0: # %entry
+; RV64IM-NEXT:    li a1, 33
+; RV64IM-NEXT:    divw a0, a0, a1
+; RV64IM-NEXT:    ret
+entry:
+  %div = sdiv i32 %x, 33
+  ret i32 %div
+}
+
+define i32 @testsize3(i32 %x) minsize nounwind {
+; RV32IM-LABEL: testsize3:
+; RV32IM:       # %bb.0: # %entry
+; RV32IM-NEXT:    srli a0, a0, 5
+; RV32IM-NEXT:    ret
+;
+; RV64IM-LABEL: testsize3:
+; RV64IM:       # %bb.0: # %entry
+; RV64IM-NEXT:    srliw a0, a0, 5
+; RV64IM-NEXT:    ret
+entry:
+  %div = udiv i32 %x, 32
+  ret i32 %div
+}
+
+define i32 @testsize4(i32 %x) minsize nounwind {
+; RV32IM-LABEL: testsize4:
+; RV32IM:       # %bb.0:
+; RV32IM-NEXT:    li a1, 33
+; RV32IM-NEXT:    divu a0, a0, a1
+; RV32IM-NEXT:    ret
+;
+; RV64IM-LABEL: testsize4:
+; RV64IM:       # %bb.0:
+; RV64IM-NEXT:    li a1, 33
+; RV64IM-NEXT:    divuw a0, a0, a1
+; RV64IM-NEXT:    ret
+  %div = udiv i32 %x, 33
+  ret i32 %div
+}
+