This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][CodeGen] Support Zfinx codegen
ClosedPublic

Authored by sunshaoce on Apr 20 2023, 9:48 PM.

Diff Detail

Event Timeline

sunshaoce created this revision.Apr 20 2023, 9:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2023, 9:48 PM
sunshaoce requested review of this revision.Apr 20 2023, 9:48 PM

Plz update llvm/docs/RISCVUsage.rst to update the support status from Assembly Support to Support :)

sunshaoce updated this revision to Diff 515637.Apr 21 2023, 1:15 AM

Address @kito-cheng's comment

Do we need any changes for GPRF32 in RISCVInstrInfo::storeRegToStackSlot and RISCVInstrInfo::loadRegFromStackSlot?

craig.topper added inline comments.Apr 21 2023, 12:18 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4403

Is FMV_W_X_RV64 correct for Zfinx?

9106

Is FMV_X_ANYEXTW_RV64 correct for Zfinx?

15454

This is a vector related function. I don't think this change is tested in this patch

llvm/lib/Target/RISCV/RISCVInstrInfoF.td
437

Why are these CSR aliases copied?

sunshaoce marked 2 inline comments as done.Apr 23 2023, 8:06 AM

Do we need any changes for GPRF32 in RISCVInstrInfo::storeRegToStackSlot and RISCVInstrInfo::loadRegFromStackSlot?

  • I tried to add a diff, but the test file didn't change at all.
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index b41f7afc1aa5..8ea0b5d4bce4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -524,6 +524,9 @@ void RISCVInstrInfo::storeRegToStackSlot(MachineBasicBlock &MBB,
   } else if (RISCV::FPR32RegClass.hasSubClassEq(RC)) {
     Opcode = RISCV::FSW;
     IsScalableVector = false;
+  } else if (RISCV::GPRF32RegClass.hasSubClassEq(RC)) {
+    Opcode = RISCV::SW;
+    IsScalableVector = false;
   } else if (RISCV::FPR64RegClass.hasSubClassEq(RC)) {
     Opcode = RISCV::FSD;
     IsScalableVector = false;
@@ -608,6 +611,9 @@ void RISCVInstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
   } else if (RISCV::FPR32RegClass.hasSubClassEq(RC)) {
     Opcode = RISCV::FLW;
     IsScalableVector = false;
+  } else if (RISCV::GPRF32RegClass.hasSubClassEq(RC)) {
+    Opcode = RISCV::LW;
+    IsScalableVector = false;
   } else if (RISCV::FPR64RegClass.hasSubClassEq(RC)) {
     Opcode = RISCV::FLD;
     IsScalableVector = false;
  • RISCV::GPRF32RegClass uses the same registers as RISCV::GPRRegClass in practice. In RISCVInstrInfo::storeRegToStackSlot and RISCVInstrInfo::loadRegFromStackSlot, the corresponding load/store will be selected.
if (RISCV::GPRRegClass.hasSubClassEq(RC)) {
  Opcode = TRI->getRegSizeInBits(RISCV::GPRRegClass) == 32 ?
           RISCV::SW : RISCV::SD;
  IsScalableVector = false;
}
  • So is it possible that RISCVInstrInfo::storeRegToStackSlot and RISCVInstrInfo::loadRegFromStackSlot don't need to be modified?
sunshaoce updated this revision to Diff 516172.Apr 23 2023, 9:08 AM

Address @craig.topper's comments

sunshaoce added inline comments.Apr 23 2023, 9:44 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4403

We added this mapping in RISCVInstrInfoF.td: def : Pat<(riscv_fmv_w_x_rv64 GPR:$src), (FMV_W_X GPR:$src)>;

9106

We added this mapping in RISCVInstrInfoF.td: def : Pat<(riscv_fmv_x_anyextw_rv64 FPR32:$src), (FMV_X_W FPR32:$src)>;

craig.topper added inline comments.Apr 23 2023, 10:32 PM
llvm/lib/Target/RISCV/RISCVInstrInfoF.td
742

This pattern doesn't look right. A copy would not guarantee sign extension.

sunshaoce updated this revision to Diff 516346.Apr 24 2023, 3:43 AM

update RISCVInstrInfoF.td

sunshaoce added inline comments.Apr 24 2023, 3:46 AM
llvm/lib/Target/RISCV/RISCVInstrInfoF.td
586–587

def : Pat<(sext_inreg (riscv_fmv_x_anyextw_rv64 FPR32:$src), i32), (FMV_X_W FPR32:$src)>;
It seems to not work, and deleting it has no effect on the tests.

742

def : Pat<(sext_inreg (riscv_fmv_x_anyextw_rv64 GPRF32:$src), i32), (COPY GPRF32:$src)>; After deletion, it will generate the sext.w a0, a0 instruction in the bcvt_f32_to_sext_i32 test.

craig.topper added inline comments.Apr 28 2023, 10:50 AM
llvm/lib/Target/RISCV/RISCVInstrInfoF.td
644
def : Pat<(f32 (load (AddrRegImm GPR:$rs1, simm12:$imm12))),
          (COPY_TO_REGCLASS (LW GPR:$rs1, simm12:$imm12), GPRF32)>;
647
def : Pat<(store (f32 FPR32INX:$rs2), (AddrRegImm GPR:$rs1, simm12:$imm12)),
          (SW (COPY_TO_REGCLASS FPR32INX:$rs2, GPR), GPR:$rs1, simm12:$imm12)>;
660

(COPY_TO_REGCLASS GPR:$rs1, GPRF32)

661

(COPY_TO_REGCLASS FPR32INX:$rs1, GPR)

740

(COPY_TO_REGCLASS GPR:$src, GPRF32)

741

(COPY_TO_REGCLASS GPRF32:$src, GPR)

sunshaoce marked 6 inline comments as done.

Address @craig.topper's comments. Thanks!

This revision is now accepted and ready to land.May 1 2023, 9:44 PM
This revision was automatically updated to reflect the committed changes.