[RISCV][NFC] Simplify immediate materialisation

[benshi001]

Summary

1. Make alias: (slli.uw x, 0) -> (zexti32 x).
2. Use (zexti32 x) instead of (zext.w x) to simplify building the instruction sequence of immediate materialisation.

[benshi001]

According to the spec of the B extension, (slli.uw x, 0) can fully replace the (zext.w x).

Using slli.uw can simpify the logic, which reduce an extra else branch in several places of the RISCV backend.

[craig.topper]

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

[benshi001]

In D114060#3136735, @craig.topper wrote:

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

The immediate materialisation already become more tricky with SHxADD/BCLRI/BSETI/SLLIUW, most assembly sequances are not obvious, so is it necessary to keep immediate materialisation assembly readable ?

[craig.topper]

In D114060#3136742, @benshi001 wrote:

In D114060#3136735, @craig.topper wrote:

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

The immediate materialisation already become more tricky with SHxADD/BCLRI/BSETI/SLLIUW, most assembly sequances are not obvious, so is it necessary to keep immediate materialisation assembly readable ?

I'm not arguing the readability of the whole immediate materialization itself. That you have to be able to read like any other instructions. My concern is that this is the only time the compiler will emit a slli.uw with a zero shift amount and there's no alias for it. It might be surprising the first time someone sees it.

[benshi001]

In D114060#3136769, @craig.topper wrote:

In D114060#3136742, @benshi001 wrote:

In D114060#3136735, @craig.topper wrote:

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

The immediate materialisation already become more tricky with SHxADD/BCLRI/BSETI/SLLIUW, most assembly sequances are not obvious, so is it necessary to keep immediate materialisation assembly readable ?

I'm not arguing the readability of the whole immediate materialization itself. That you have to be able to read like any other instructions. My concern is that this is the only time the compiler will emit a slli.uw with a zero shift amount and there's no alias for it. It might be surprising the first time someone sees it.

According to the spec of the B extension, "zext.w" has already been aliased to "add.uw x, zero", so how about making a new alias like "zext32 x" to "slli.uw t, 0" ?

[benshi001]

In D114060#3136804, @benshi001 wrote:

In D114060#3136769, @craig.topper wrote:

In D114060#3136742, @benshi001 wrote:

In D114060#3136735, @craig.topper wrote:

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

The immediate materialisation already become more tricky with SHxADD/BCLRI/BSETI/SLLIUW, most assembly sequances are not obvious, so is it necessary to keep immediate materialisation assembly readable ?

I'm not arguing the readability of the whole immediate materialization itself. That you have to be able to read like any other instructions. My concern is that this is the only time the compiler will emit a slli.uw with a zero shift amount and there's no alias for it. It might be surprising the first time someone sees it.

According to the spec of the B extension, "zext.w" has already been aliased to "add.uw x, zero", so how about making a new alias like "zext32 x" to "slli.uw t, 0" ?

I have added alias (slli.uw x, 0) -> (zexti32 x), not sure the name is proper. But I think (slli.uw x, 0) should have an alias.

[jrtc27]

Don’t make up your own non-standard aliases; I get annoyed when binutils does the same and we have to copy them

[benshi001]

In D114060#3136769, @craig.topper wrote:

In D114060#3136742, @benshi001 wrote:

In D114060#3136735, @craig.topper wrote:

Is the compiler simplicity worth making the assembly harder to read since it won't print zext.w anymore?

The immediate materialisation already become more tricky with SHxADD/BCLRI/BSETI/SLLIUW, most assembly sequances are not obvious, so is it necessary to keep immediate materialisation assembly readable ?

I'm not arguing the readability of the whole immediate materialization itself. That you have to be able to read like any other instructions. My concern is that this is the only time the compiler will emit a slli.uw with a zero shift amount and there's no alias for it. It might be surprising the first time someone sees it.

Is there a way to append a comment after the machine instruction when calling BuildMI ? Could it be better to be slli.uw a0, a0, 0 ; zero extend i32 to i64 ?

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value,
   RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Value, getSTI().getFeatureBits());
 
   MCRegister SrcReg = RISCV::X0;
   for (RISCVMatInt::Inst &Inst : Seq) {
     if (Inst.Opc == RISCV::LUI) {
       emitToStreamer(
           Out, MCInstBuilder(RISCV::LUI).addReg(DestReg).addImm(Inst.Imm));
-    } else if (Inst.Opc == RISCV::ADDUW) {
-      emitToStreamer(Out, MCInstBuilder(RISCV::ADDUW)
-                              .addReg(DestReg)
-                              .addReg(SrcReg)
-                              .addReg(RISCV::X0));
     } else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
                Inst.Opc == RISCV::SH3ADD) {
       emitToStreamer(
           Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addReg(
                    SrcReg));
     } else {
       emitToStreamer(
           Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addImm(

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp

if (Val > 0 && Res.size() > 2) {
 
     // If we have exactly 32 leading zeros and Zba, we can try using zext.w at
     // the end of the sequence.
     if (LeadingZeros == 32 && ActiveFeatures[RISCV::FeatureStdExtZba]) {
       // Try replacing upper bits with 1.
       uint64_t LeadingOnesVal = Val | maskLeadingOnes<uint64_t>(LeadingZeros);
       TmpSeq.clear();
       generateInstSeqImpl(LeadingOnesVal, ActiveFeatures, TmpSeq);
-      TmpSeq.push_back(RISCVMatInt::Inst(RISCV::ADDUW, 0));
+      TmpSeq.push_back(RISCVMatInt::Inst(RISCV::SLLIUW, 0));
 
       // Keep the new sequence if it is an improvement.
       if (TmpSeq.size() < Res.size()) {
         Res = TmpSeq;
         // A 2 instruction sequence is the best we can do.
         if (Res.size() <= 2)
           return Res;
       }

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Imm, Subtarget.getFeatureBits());
 
   SDNode *Result = nullptr;
   SDValue SrcReg = CurDAG->getRegister(RISCV::X0, XLenVT);
   for (RISCVMatInt::Inst &Inst : Seq) {
     SDValue SDImm = CurDAG->getTargetConstant(Inst.Imm, DL, XLenVT);
     if (Inst.Opc == RISCV::LUI)
       Result = CurDAG->getMachineNode(RISCV::LUI, DL, XLenVT, SDImm);
-    else if (Inst.Opc == RISCV::ADDUW)
-      Result = CurDAG->getMachineNode(RISCV::ADDUW, DL, XLenVT, SrcReg,
-                                      CurDAG->getRegister(RISCV::X0, XLenVT));
     else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
              Inst.Opc == RISCV::SH3ADD)
       Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SrcReg, SrcReg);
     else
       Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SrcReg, SDImm);
 
     // Only the first instruction has X0 as its source.
     SrcReg = SDValue(Result, 0);

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

for (RISCVMatInt::Inst &Inst : Seq) {
     // Otherwise, write the result to the temp register.
     if (++Num == Seq.size())
       Result = DstReg;
 
     if (Inst.Opc == RISCV::LUI) {
       BuildMI(MBB, MBBI, DL, get(RISCV::LUI), Result)
           .addImm(Inst.Imm)
           .setMIFlag(Flag);
-    } else if (Inst.Opc == RISCV::ADDUW) {
-      BuildMI(MBB, MBBI, DL, get(RISCV::ADDUW), Result)
-          .addReg(SrcReg, RegState::Kill)
-          .addReg(RISCV::X0)
-          .setMIFlag(Flag);
     } else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
                Inst.Opc == RISCV::SH3ADD) {
       BuildMI(MBB, MBBI, DL, get(Inst.Opc), Result)
           .addReg(SrcReg, RegState::Kill)
           .addReg(SrcReg, RegState::Kill)
           .setMIFlag(Flag);
     } else {
       BuildMI(MBB, MBBI, DL, get(Inst.Opc), Result)

llvm/lib/Target/RISCV/RISCVInstrInfoZb.td

 } // Predicates = [HasStdExtZbbOrZbp]
 
 //===----------------------------------------------------------------------===//
 // Pseudo Instructions
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasStdExtZba, IsRV64] in {
 def : InstAlias<"zext.w $rd, $rs", (ADDUW GPR:$rd, GPR:$rs, X0)>;
+def : InstAlias<"zexti32 $rd, $rs", (SLLIUW GPR:$rd, GPR:$rs, 0)>;
 }
 
 let Predicates = [HasStdExtZbp] in {
 def : InstAlias<"rev.p $rd, $rs",  (GREVI GPR:$rd, GPR:$rs, 0b00001)>;
 def : InstAlias<"rev2.n $rd, $rs", (GREVI GPR:$rd, GPR:$rs, 0b00010)>;
 def : InstAlias<"rev.n $rd, $rs",  (GREVI GPR:$rd, GPR:$rs, 0b00011)>;
 def : InstAlias<"rev4.b $rd, $rs", (GREVI GPR:$rd, GPR:$rs, 0b00100)>;
 def : InstAlias<"rev2.b $rd, $rs", (GREVI GPR:$rd, GPR:$rs, 0b00110)>;

llvm/test/CodeGen/RISCV/imm.ll

 ; RV64I-NEXT:    slli a0, a0, 12
 ; RV64I-NEXT:    addi a0, a0, -1366
 ; RV64I-NEXT:    ret
 ;
 ; RV64IZBA-LABEL: imm_2863311530:
 ; RV64IZBA:       # %bb.0:
 ; RV64IZBA-NEXT:    lui a0, 699051
 ; RV64IZBA-NEXT:    addiw a0, a0, -1366
-; RV64IZBA-NEXT:    zext.w a0, a0
+; RV64IZBA-NEXT:    zexti32 a0, a0
 ; RV64IZBA-NEXT:    ret
 ;
 ; RV64IZBS-LABEL: imm_2863311530:
 ; RV64IZBS:       # %bb.0:
 ; RV64IZBS-NEXT:    lui a0, 174763
 ; RV64IZBS-NEXT:    addiw a0, a0, -1366
 ; RV64IZBS-NEXT:    bseti a0, a0, 31
 ; RV64IZBS-NEXT:    ret

llvm/test/CodeGen/RISCV/rv64zba.ll

 ; RV64I-NEXT:    addi a0, zero, 1
 ; RV64I-NEXT:    slli a0, a0, 32
 ; RV64I-NEXT:    addi a0, a0, -2
 ; RV64I-NEXT:    ret
 ;
 ; RV64ZBA-LABEL: imm_zextw:
 ; RV64ZBA:       # %bb.0:
 ; RV64ZBA-NEXT:    addi a0, zero, -2
-; RV64ZBA-NEXT:    zext.w a0, a0
+; RV64ZBA-NEXT:    zexti32 a0, a0
 ; RV64ZBA-NEXT:    ret
   ret i64 4294967294 ; -2 in 32 bits.
 }
 
 define i64 @imm_zextw2() nounwind {
 ; RV64I-LABEL: imm_zextw2:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    lui a0, 171
 ; RV64I-NEXT:    addiw a0, a0, -1365
 ; RV64I-NEXT:    slli a0, a0, 12
 ; RV64I-NEXT:    addi a0, a0, -1366
 ; RV64I-NEXT:    ret
 ;
 ; RV64ZBA-LABEL: imm_zextw2:
 ; RV64ZBA:       # %bb.0:
 ; RV64ZBA-NEXT:    lui a0, 699051
 ; RV64ZBA-NEXT:    addiw a0, a0, -1366
-; RV64ZBA-NEXT:    zext.w a0, a0
+; RV64ZBA-NEXT:    zexti32 a0, a0
 ; RV64ZBA-NEXT:    ret
   ret i64 2863311530 ; 0xAAAAAAAA
 }
 
 define i64 @mul11(i64 %a) {
 ; RV64I-LABEL: mul11:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    addi a1, zero, 11

llvm/test/MC/RISCV/rv64zba-aliases-valid.s

 #                        aliases enabled
 # CHECK-S-OBJ-NOALIAS    Match both the .s and objdumped object output with
 #                        aliases disabled
 
 # CHECK-S-OBJ-NOALIAS: add.uw t0, t1, zero
 # CHECK-S-OBJ: zext.w t0, t1
 zext.w x5, x6
 
+# CHECK-S-OBJ-NOALIAS: slli.uw t0, t1, 0
+# CHECK-S-OBJ: zexti32 t0, t1
+zexti32 x5, x6
+
 # CHECK-S-OBJ-NOALIAS: addi t1, zero, -2
-# CHECK-S-OBJ-NOALIAS-NEXT: add.uw t1, t1, zero
+# CHECK-S-OBJ-NOALIAS-NEXT: slli.uw t1, t1, 0
 # CHECK-S-OBJ: addi t1, zero, -2
-# CHECK-S-OBJ-NEXT: zext.w t1, t1
+# CHECK-S-OBJ-NEXT: zexti32 t1, t1
 li x6, 0xfffffffe
 
 # CHECK-S-OBJ-NOALIAS: lui t2, 699051
 # CHECK-S-OBJ-NOALIAS-NEXT: addiw t2, t2, -1366
-# CHECK-S-OBJ-NOALIAS-NEXT: add.uw t2, t2, zero
+# CHECK-S-OBJ-NOALIAS-NEXT: slli.uw t2, t2, 0
 # CHECK-S-OBJ: lui t2, 699051
 # CHECK-S-OBJ-NEXT: addiw t2, t2, -1366
-# CHECK-S-OBJ-NEXT: zext.w t2, t2
+# CHECK-S-OBJ-NEXT: zexti32 t2, t2
 li x7, 0xaaaaaaaa
 
 # CHECK-S-OBJ-NOALIAS: lui t0, 768955
 # CHECK-S-OBJ-NOALIAS-NEXT: slli.uw t0, t0, 12
 # CHECK-S-OBJ-NOALIAS-NEXT: addi t0, t0, 1979
 # CHECK-S-OBJ: lui t0, 768955
 # CHECK-S-OBJ-NEXT: slli.uw t0, t0, 12
 # CHECK-S-OBJ-NEXT: addi t0, t0, 1979

llvm/test/MC/RISCV/rv64zba-valid.s

 # With Bitmanip base extension:
 # RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zba -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zba < %s \
 # RUN:     | llvm-objdump --mattr=+experimental-zba -d -r - \
 # RUN:     | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
 
-# CHECK-ASM-AND-OBJ: slli.uw t0, t1, 0
+# CHECK-ASM-AND-OBJ: slli.uw t0, t1, 1
+# CHECK-ASM: encoding: [0x9b,0x12,0x13,0x08]
+slli.uw t0, t1, 1
+# CHECK-ASM-AND-OBJ: zexti32 t0, t1
 # CHECK-ASM: encoding: [0x9b,0x12,0x03,0x08]
 slli.uw t0, t1, 0
 # CHECK-ASM-AND-OBJ: add.uw t0, t1, t2
 # CHECK-ASM: encoding: [0xbb,0x02,0x73,0x08]
 add.uw t0, t1, t2
 # CHECK-ASM-AND-OBJ: sh1add.uw t0, t1, t2
 # CHECK-ASM: encoding: [0xbb,0x22,0x73,0x20]
 sh1add.uw t0, t1, t2
 # CHECK-ASM-AND-OBJ: sh2add.uw t0, t1, t2
 # CHECK-ASM: encoding: [0xbb,0x42,0x73,0x20]
 sh2add.uw t0, t1, t2
 # CHECK-ASM-AND-OBJ: sh3add.uw t0, t1, t2
 # CHECK-ASM: encoding: [0xbb,0x62,0x73,0x20]
 sh3add.uw t0, t1, t2