[DAGCombiner] Add target hook function to decide folding (mul (add x, c1), c2)

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

What about all the optimizations we have for mul by constant using SHXADD? Won't this miss out on those?

[benshi001]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

I do not think it will affect the optimization with SHxADD.

For the ones such as
(mul 11/13/25/41/73/37/21), (mul (3/5/9)*power_of_2), (mul power_of_2 + (2/4/8)), those are pure mul without add involved, so they won't be affected.

For the rules such as
(x + y * 4) -> (SH2ADD y, x)
(x + y * 20) -> (SH2ADD (SH2ADD x, x), y)
.......

My patch still generate better code.

c1 + x * 72 (c1 is a non-simm12 constant)

before current patch

lui Ry, higher-bits of c1
addi Ry, Ry, lower-12-bits of c1
sh3add Rz, x, x
sh3add Rz, Rz, Ry

after my patch

addi Ry, x, c1/72
sh3add Ry, Ry, Ry
sll Ry, Ry, 3

I will add those cases to the test file.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

Isn’t it possible for the MUL you create here to have 11/13/25/41/73/37/21 as a constant that should use SHXADD?

[benshi001]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

Yes, it is possible. And for mul with 11/13/25/41/73/37/21 which use shxadd, my change still generates better asm. I will add in the tests.

[benshi001]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

Sorry, your concern is right. I can not handle mul with 11/13/25/41/73/37/21.

[benshi001]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

537 ↗

(On Diff #365026)

I have skip those constants in the zba extension. And will figure out a better way for them in the future.

Thanks for your help.

[benshi001]

Is there any better way to implement this optimization other than ISelDagToDag?

1. It seems hard to calculate c1/c0 by writing TD mapping rules.

2. I also tried DAG transform in RISCVTargetLowering::ReplaceNodeResults, it is also not easy, since it involves more code about legalization.

[craig.topper]

In D107711#2934246, @benshi001 wrote:

Is there any better way to implement this optimization other than ISelDagToDag?

1. It seems hard to calculate c1/c0 by writing TD mapping rules.

2. I also tried DAG transform in RISCVTargetLowering::ReplaceNodeResults, it is also not easy, since it involves more code about legalization.

It should probably be a DAG combine and the transform that's turning (mul (add X, C1), C2) into (add (mul X, C2), C1 * C2) should ask the target if it is profitable, or at least call isLegalAddImmediate for C1*C2. @spatel or @lebedev.ri, what do you think?

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

18 ↗

(On Diff #365158)

#include <set>. and it should be after all llvm headers

534 ↗

(On Diff #365158)

Use std::array so you can use begin()/end() on the std::set.

Though you could sort the array and use std::binary_search and avoid the set completely.

[lebedev.ri]

In D107711#2934794, @craig.topper wrote:

In D107711#2934246, @benshi001 wrote:

Is there any better way to implement this optimization other than ISelDagToDag?

1. It seems hard to calculate c1/c0 by writing TD mapping rules.

2. I also tried DAG transform in RISCVTargetLowering::ReplaceNodeResults, it is also not easy, since it involves more code about legalization.

It should probably be a DAG combine and the transform that's turning (mul (add X, C1), C2) into (add (mul X, C2), C1 * C2) should ask the target if it is profitable, or at least call isLegalAddImmediate for C1*C2. @spatel or @lebedev.ri, what do you think?

Putting it into DAGCombine SGTM

[benshi001]

In D107711#2934794, @craig.topper wrote:

In D107711#2934246, @benshi001 wrote:

Is there any better way to implement this optimization other than ISelDagToDag?

1. It seems hard to calculate c1/c0 by writing TD mapping rules.

2. I also tried DAG transform in RISCVTargetLowering::ReplaceNodeResults, it is also not easy, since it involves more code about legalization.

It should probably be a DAG combine and the transform that's turning (mul (add X, C1), C2) into (add (mul X, C2), C1 * C2) should ask the target if it is profitable, or at least call isLegalAddImmediate for C1*C2. @spatel or @lebedev.ri, what do you think?

Thanks for your help! I have added a new target hook function isMulAddWithConstNotProfitable to let the DAGCombiner consult the target before combining (mul (add X, C1), C2).

This solution seems more clear. And it really improve most riscv's assembly code, except two of them, which I have made inline comments.

[benshi001]

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

230

This should not be a regression, since 8 bytes are saved, and the mulw should cost no more than 3 cycles since 73 is a small integer, so does for 19/25/41/73/11/13...

295

This can be further optimized to

(SLLIW (SH1ADD a0, a0, a0), 6).

I will make another patch for this optimization.

[benshi001]

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

295

The best form should be

; RV64IM-NEXT: addi a0, a0, 1000
; RV64IM-NEXT: sh1add a0, a0, a0
; RV64IM-NEXT: slliw a0, a0, 6

It can be done via new rules in RISCVInstrInfoB.td

[benshi001]

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

295

I have submitted another patch to optimize that case.

https://reviews.llvm.org/D107820

[benshi001]

In this solution, this is no need to concern the impact to optimization with SHXADD.

[jrtc27]

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

210–211

Why are RV32IM check lines, in a file whose name and path don't say bitmanip, mentioning bitmanip instructions?

[craig.topper]

llvm/include/llvm/CodeGen/TargetLowering.h

2089

Drop the "Not" and return true. Remove the ! from the caller.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9097

Use cast instead of dyn_cast and drop this assert. cast asserts internally.

[benshi001]

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

305

This regression will be fixed by D107708.

[lebedev.ri]

Comments need updating

[benshi001]

In D107711#2939527, @lebedev.ri wrote:

Comments need updating

I have updated the inline comments. Thank you.

[lebedev.ri]

In D107711#2939527, @lebedev.ri wrote:

Comments need updating

llvm/include/llvm/CodeGen/TargetLowering.h

2086

llvm/lib/Target/RISCV/RISCVISelLowering.h

467

[benshi001]

llvm/include/llvm/CodeGen/TargetLowering.h

2086

I think my origin default true is right, the default return value should not be false.

Since my hook is called as

if (AddNode.getNode()->hasOneUse() &&
    TLI.isMulAddWithConstProfitable(AddNode, ConstNode))
  return true;

So it should return default true for undetermined cases.

[benshi001]

llvm/include/llvm/CodeGen/TargetLowering.h

2086

Actually my previous version is isMulAddWithConstNotProfitable, which return default false, and return true for clear regression on specific targets.

Craig suggested me to remove the Not, and inverse the condition when calling.

The core issue is, the original DAGCombiner will do the folding if the AddNode has only one use, which will harm performance in some situation. And the solution is adding another check (along with the hasOneUse) to let the target prevent the transform if the target does think there is regresssion.

But if the target is also not sure, what default value should be ? And the hook name should have Not or should not have a Not ?

[jrtc27]

llvm/include/llvm/CodeGen/TargetLowering.h

2086

The default should be whatever makes SelectionDAG behave the same as it currently does unless the changes in behaviour turn out to be useful for the majority of targets.

TLI functions should be positive not negative; there are no hooks that have Not in them (other than when they refer to a not instruction).

I'm not sure what the issue is though? Whether you have a NotProfitable function that defaults to false or a Profitable function that defaults to true makes no semantic difference other than when the caller needs to put a ! in front of it, it's purely a stylistic issue.

[benshi001]

llvm/include/llvm/CodeGen/TargetLowering.h

2086

I will keep current form without Not, and still return default true to "make SelectionDAG behave the same as it currently does".

And I have improved my comments more clear as

/// Return true if it may be profitable to fold
/// (mul (add x, c1), c2) -> (add (mul x, c2), c1*c2), and return false
/// to prevent the folding for definite regression.
/// The target should check the cost of materializing c1, c2 and c1*c2 into
/// registers. If it is not sure about some cases, a default true
/// can be returned to let the DAGCombiner decide.

[lebedev.ri]

SGTM

[benshi001]

ping ... Can this patch be approved ? It seems there is no objection on adding a hook isMulAddWithConstProfitable

[lebedev.ri]

LGTM unless other have further comments.
Thanks.

[benshi001]

In D107711#2951596, @lebedev.ri wrote:

LGTM unless other have further comments.
Thanks.

I would like to land on Sunday evening, unless there will be other objection.

[jrtc27]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

16857

"finds no regression" doesn't make sense to me. A regression is a bug, but if the target says it's profitable then there is no regression; a regression would be if a target said it was not profitable but it in fact was, and so the emitted code got worse. So when a target returns false, it's not that it finds a regression, because nothing has happened yet.

This also feeds into a related point. When comments in DAGCombiner talk about regressions, they generally mean "this transformation should make sense on most targets, but many of them don't enable it currently because they have patterns or custom lowering that would need to be adapted to handle it otherwise they won't match important cases any more" (with the "won't match important cases" being the regression), generally marked as TODO (all but one, with the odd-one-out still saying that something should probably be improved). This is not that case, this is just asking the target whether the transformation is profitable, simply as a "does your instruction set benefit from this transformation?".

llvm/lib/Target/RISCV/RISCVISelLowering.h

464

What's the point of duplicating this doxygen comment? It's just going to get outdated, and anyone who wants to know what it does (which is pretty obvious from the name) can just look at TargetLowering.h. If we copied the documentation to every override the tree would be a mess.

[benshi001]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

16857

Thanks. I have made the comments more clear.

llvm/lib/Target/RISCV/RISCVISelLowering.h

464

Thanks. I have removed the redundant comments.

[jrtc27]

llvm/include/llvm/CodeGen/TargetLowering.h

2083

The comment's still not great, putting grammatical issues aside. A lot of it is just explaining the basics of TLI hooks, but also is overly prescriptive with what backends should do to evaluate it (and I also don't like "to avoid definite worse code generated", often TLI hooks end up being best-effort heuristics, unable to give a definitive answer, because that might require extremely expensive whole-function checks that depend on knowing what other transformations are going to be made).

I'd just go with something like (borrowing style from surrounding examples):

Return true if it may be profitable to transform
(mul (add x, c1), c2) -> (add (mul x, c2), c1*c2).
This may not be true if c1 and c2 can be represented as immediates but c1*c2 cannot, for example.

[benshi001]

llvm/include/llvm/CodeGen/TargetLowering.h

2083

Thanks, I have updated the comments according to your suggested expression.

One more issue, English is not my mother language and your are appreciated to help me fix any "grammatical issues" you mentioned. ^_^

[luismarques]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9088

Nit: IMO, it would be more intuitive to compare the other way around, swapping the operands and changing the condition to >=.

[benshi001]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9088

I can swap the operands, but why changing the condition to >=, should using > be better? Since I think this should consider i64 on rv64 and i32 on rv32.

[jrtc27]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9088

Yes, > is correct, and I agree with Luis that this is the more natural way round to express it.

[luismarques]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9088

Sorry, ignore the part about the equals :) I mixed some thoughts when writing that.

[benshi001]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9088

Thanks. The order has been changed.

llvm/include/llvm/CodeGen/TargetLowering.h

public:
   /// This may be true if the target does not directly support the
   /// multiplication operation for the specified type or the sequence of simpler
   /// ops is faster than the multiply.
   virtual bool decomposeMulByConstant(LLVMContext &Context,
                                       EVT VT, SDValue C) const {
     return false;
   }
 
+  /// Return true if it may be profitable to fold
+  /// (mul (add x, c1), c2) -> (add (mul x, c2), c1*c2), and return false
+  /// to prevent the folding for definite regression.
+  /// The target should check the cost of materializing c1, c2 and c1*c2 into
+  /// registers. If it is not sure about some cases, a default true
+  /// can be returned to let the DAGCombiner decide.
+  /// AddNode is (add x, c1), and ConstNode is c2.
+  virtual bool isMulAddWithConstProfitable(const SDValue &AddNode,
+                                           const SDValue &ConstNode) const {
+    return true;
+  }
+
   /// Return true if it is more correct/profitable to use strict FP_TO_INT
   /// conversion operations - canonicalizing the FP source value instead of
   /// converting all cases and then selecting based on value.
   /// This may be true if the target throws exceptions for out of bounds
   /// conversions or has fast FP CMOV.
   virtual bool shouldUseStrictFP_TO_INT(EVT FpVT, EVT IntVT,
                                         bool IsSigned) const {
     return false;

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

 //     (A + c1) * c3
 //     (A + c2) * c3
 // We're checking for cases where we have common "c3 * A" expressions.
 bool DAGCombiner::isMulAddWithConstProfitable(SDNode *MulNode,
                                               SDValue &AddNode,
                                               SDValue &ConstNode) {
   APInt Val;
 
-  // If the add only has one use, this would be OK to do.
-  if (AddNode.getNode()->hasOneUse())
+  // If the add only has one use and the target finds no regression, this
+  // would be OK to do.
+  if (AddNode.getNode()->hasOneUse() &&
+      TLI.isMulAddWithConstProfitable(AddNode, ConstNode))
     return true;
 
   // Walk all the users of the constant with which we're multiplying.
   for (SDNode *Use : ConstNode->uses()) {
     if (Use == MulNode) // This use is the one we're on right now. Skip it.
       continue;
 
     if (Use->getOpcode() == ISD::MUL) { // We have another multiply use.

llvm/lib/Target/RISCV/RISCVISelLowering.h

bool shouldConvertConstantLoadToIntImm(const APInt &Imm,
     return true;
   }
   bool mayBeEmittedAsTailCall(const CallInst *CI) const override;
   bool shouldConsiderGEPOffsetSplit() const override { return true; }
 
   bool decomposeMulByConstant(LLVMContext &Context, EVT VT,
                               SDValue C) const override;
 
+  /// Return true if it may be profitable to fold
+  /// (mul (add x, c1), c2) -> (add (mul x, c2), c1*c2), and return false
+  /// to prevent the folding for definite regression.
+  /// The target should check the cost of materializing c1, c2 and c1*c2 into
+  /// registers. If it is not sure about some cases, a default true
+  /// can be returned to let the DAGCombiner decide.
+  bool isMulAddWithConstProfitable(const SDValue &AddNode,
+                                   const SDValue &ConstNode) const override;
+
   TargetLowering::AtomicExpansionKind
   shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const override;
   Value *emitMaskedAtomicRMWIntrinsic(IRBuilderBase &Builder, AtomicRMWInst *AI,
                                       Value *AlignedAddr, Value *Incr,
                                       Value *Mask, Value *ShiftAmt,
                                       AtomicOrdering Ord) const override;
   TargetLowering::AtomicExpansionKind
   shouldExpandAtomicCmpXchgInIR(AtomicCmpXchgInst *CI) const override;

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

if (auto *ConstNode = dyn_cast<ConstantSDNode>(C.getNode())) {
         return true;
       }
     }
   }
 
   return false;
 }
 
+bool RISCVTargetLowering::isMulAddWithConstProfitable(
+    const SDValue &AddNode, const SDValue &ConstNode) const {
+  // Let the DAGCombiner decide for vectors.
+  EVT VT = AddNode.getValueType();
+  if (VT.isVector())
+    return true;
+
+  // Let the DAGCombiner decide for larger types.
+  if (Subtarget.getXLen() < VT.getScalarSizeInBits())
+    return true;
+
+  // It is not profitable if c1 is simm12 while c1*c2 is not.
+  ConstantSDNode *C1Node = cast<ConstantSDNode>(AddNode.getOperand(1));
+  ConstantSDNode *C2Node = cast<ConstantSDNode>(ConstNode);
+  const APInt &C1 = C1Node->getAPIntValue();
+  const APInt &C2 = C2Node->getAPIntValue();
+  if (C1.isSignedIntN(12) && !(C1 * C2).isSignedIntN(12))
+    return false;
+
+  // Default to true and let the DAGCombiner decide.
+  return true;
+}
+
 bool RISCVTargetLowering::allowsMisalignedMemoryAccesses(
     EVT VT, unsigned AddrSpace, Align Alignment, MachineMemOperand::Flags Flags,
     bool *Fast) const {
   if (!VT.isVector())
     return false;
 
   EVT ElemVT = VT.getVectorElementType();
   if (Alignment >= ElemVT.getStoreSize()) {

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

; RV64IMB-NEXT:    ret
   %tmp0 = add i64 %x, 8953
   %tmp1 = mul i64 %tmp0, 23
   ret i64 %tmp1
 }
 
 define i32 @add_mul_combine_reject_a1(i32 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_a1:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1971
 ; RV32IMB-NEXT:    addi a1, zero, 29
 ; RV32IMB-NEXT:    mul a0, a0, a1
-; RV32IMB-NEXT:    lui a1, 14
-; RV32IMB-NEXT:    addi a1, a1, -185
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_a1:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1971
 ; RV64IMB-NEXT:    addi a1, zero, 29
 ; RV64IMB-NEXT:    mul a0, a0, a1
-; RV64IMB-NEXT:    lui a1, 14
-; RV64IMB-NEXT:    addiw a1, a1, -185
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1971
   %tmp1 = mul i32 %tmp0, 29
   ret i32 %tmp1
 }
 
 define signext i32 @add_mul_combine_reject_a2(i32 signext %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_a2:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1971
 ; RV32IMB-NEXT:    addi a1, zero, 29
 ; RV32IMB-NEXT:    mul a0, a0, a1
-; RV32IMB-NEXT:    lui a1, 14
-; RV32IMB-NEXT:    addi a1, a1, -185
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_a2:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1971
 ; RV64IMB-NEXT:    addi a1, zero, 29
-; RV64IMB-NEXT:    mul a0, a0, a1
-; RV64IMB-NEXT:    lui a1, 14
-; RV64IMB-NEXT:    addiw a1, a1, -185
-; RV64IMB-NEXT:    addw a0, a0, a1
+; RV64IMB-NEXT:    mulw a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1971
   %tmp1 = mul i32 %tmp0, 29
   ret i32 %tmp1
 }
 
 define i64 @add_mul_combine_reject_a3(i64 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_a3:
 ; RV32IMB:       # %bb.0:
 ; RV32IMB-NEXT:    addi a2, zero, 29
 ; RV32IMB-NEXT:    mul a1, a1, a2
 ; RV32IMB-NEXT:    mulhu a3, a0, a2
 ; RV32IMB-NEXT:    add a1, a3, a1
 ; RV32IMB-NEXT:    mul a2, a0, a2
 ; RV32IMB-NEXT:    lui a0, 14
 ; RV32IMB-NEXT:    addi a0, a0, -185
 ; RV32IMB-NEXT:    add a0, a2, a0
 ; RV32IMB-NEXT:    sltu a2, a0, a2
 ; RV32IMB-NEXT:    add a1, a1, a2
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_a3:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1971
 ; RV64IMB-NEXT:    addi a1, zero, 29
 ; RV64IMB-NEXT:    mul a0, a0, a1
-; RV64IMB-NEXT:    lui a1, 14
-; RV64IMB-NEXT:    addiw a1, a1, -185
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i64 %x, 1971
   %tmp1 = mul i64 %tmp0, 29
   ret i64 %tmp1
 }
 
 define i32 @add_mul_combine_reject_c1(i32 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_c1:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1000
 ; RV32IMB-NEXT:    sh3add a1, a0, a0
 ; RV32IMB-NEXT:    sh3add a0, a1, a0
-; RV32IMB-NEXT:    lui a1, 18
-; RV32IMB-NEXT:    addi a1, a1, -728
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_c1:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1000
 ; RV64IMB-NEXT:    sh3add a1, a0, a0
 ; RV64IMB-NEXT:    sh3add a0, a1, a0
-; RV64IMB-NEXT:    lui a1, 18
-; RV64IMB-NEXT:    addiw a1, a1, -728
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1000
   %tmp1 = mul i32 %tmp0, 73
   ret i32 %tmp1
 }
 
 define signext i32 @add_mul_combine_reject_c2(i32 signext %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_c2:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1000
 ; RV32IMB-NEXT:    sh3add a1, a0, a0
 ; RV32IMB-NEXT:    sh3add a0, a1, a0
-; RV32IMB-NEXT:    lui a1, 18
-; RV32IMB-NEXT:    addi a1, a1, -728
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_c2:
 ; RV64IMB:       # %bb.0:
-; RV64IMB-NEXT:    sh3add a1, a0, a0
-; RV64IMB-NEXT:    sh3add a0, a1, a0
-; RV64IMB-NEXT:    lui a1, 18
-; RV64IMB-NEXT:    addiw a1, a1, -728
-; RV64IMB-NEXT:    addw a0, a0, a1
+; RV64IMB-NEXT:    addi a0, a0, 1000
+; RV64IMB-NEXT:    addi a1, zero, 73
+; RV64IMB-NEXT:    mulw a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1000
   %tmp1 = mul i32 %tmp0, 73
   ret i32 %tmp1
 }
 
 define i64 @add_mul_combine_reject_c3(i64 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_c3:
 ; RV32IMB:       # %bb.0:
 ; RV32IMB-NEXT:    addi a2, zero, 73
 ; RV32IMB-NEXT:    mul a1, a1, a2
 ; RV32IMB-NEXT:    mulhu a3, a0, a2
 ; RV32IMB-NEXT:    add a1, a3, a1
 ; RV32IMB-NEXT:    mul a2, a0, a2
 ; RV32IMB-NEXT:    lui a0, 18
 ; RV32IMB-NEXT:    addi a0, a0, -728
 ; RV32IMB-NEXT:    add a0, a2, a0
 ; RV32IMB-NEXT:    sltu a2, a0, a2
 ; RV32IMB-NEXT:    add a1, a1, a2
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_c3:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1000
 ; RV64IMB-NEXT:    sh3add a1, a0, a0
 ; RV64IMB-NEXT:    sh3add a0, a1, a0
-; RV64IMB-NEXT:    lui a1, 18
-; RV64IMB-NEXT:    addiw a1, a1, -728
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i64 %x, 1000
   %tmp1 = mul i64 %tmp0, 73
   ret i64 %tmp1
 }
 
 define i32 @add_mul_combine_reject_d1(i32 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_d1:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1000
 ; RV32IMB-NEXT:    sh1add a0, a0, a0
 ; RV32IMB-NEXT:    slli a0, a0, 6
-; RV32IMB-NEXT:    lui a1, 47
-; RV32IMB-NEXT:    addi a1, a1, -512
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_d1:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1000
 ; RV64IMB-NEXT:    sh1add a0, a0, a0
 ; RV64IMB-NEXT:    slli a0, a0, 6
-; RV64IMB-NEXT:    lui a1, 47
-; RV64IMB-NEXT:    addiw a1, a1, -512
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1000
   %tmp1 = mul i32 %tmp0, 192
   ret i32 %tmp1
 }
 
 define signext i32 @add_mul_combine_reject_d2(i32 signext %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_d2:
 ; RV32IMB:       # %bb.0:
+; RV32IMB-NEXT:    addi a0, a0, 1000
 ; RV32IMB-NEXT:    sh1add a0, a0, a0
 ; RV32IMB-NEXT:    slli a0, a0, 6
-; RV32IMB-NEXT:    lui a1, 47
-; RV32IMB-NEXT:    addi a1, a1, -512
-; RV32IMB-NEXT:    add a0, a0, a1
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_d2:
 ; RV64IMB:       # %bb.0:
-; RV64IMB-NEXT:    sh1add a0, a0, a0
-; RV64IMB-NEXT:    slli a0, a0, 6
-; RV64IMB-NEXT:    lui a1, 47
-; RV64IMB-NEXT:    addiw a1, a1, -512
-; RV64IMB-NEXT:    addw a0, a0, a1
+; RV64IMB-NEXT:    addi a0, a0, 1000
+; RV64IMB-NEXT:    addi a1, zero, 192
+; RV64IMB-NEXT:    mulw a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i32 %x, 1000
   %tmp1 = mul i32 %tmp0, 192
   ret i32 %tmp1
 }
 
 define i64 @add_mul_combine_reject_d3(i64 %x) {
 ; RV32IMB-LABEL: add_mul_combine_reject_d3:

 ; RV32IMB-NEXT:    addi a0, a0, -512
 ; RV32IMB-NEXT:    add a0, a2, a0
 ; RV32IMB-NEXT:    sltu a2, a0, a2
 ; RV32IMB-NEXT:    add a1, a1, a2
 ; RV32IMB-NEXT:    ret
 ;
 ; RV64IMB-LABEL: add_mul_combine_reject_d3:
 ; RV64IMB:       # %bb.0:
+; RV64IMB-NEXT:    addi a0, a0, 1000
 ; RV64IMB-NEXT:    sh1add a0, a0, a0
 ; RV64IMB-NEXT:    slli a0, a0, 6
-; RV64IMB-NEXT:    lui a1, 47
-; RV64IMB-NEXT:    addiw a1, a1, -512
-; RV64IMB-NEXT:    add a0, a0, a1
 ; RV64IMB-NEXT:    ret
   %tmp0 = add i64 %x, 1000
   %tmp1 = mul i64 %tmp0, 192
   ret i64 %tmp1
 }