[RISCV][llvm-exegesis] Add missing operand frm for FCVT_D_W

[tclin914]

We encountered the index of operand out of bounds crash because FCVT_D_W lacks frm operand.

[llvmbot]

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Jim Lin (tclin914)

Changes

We encountered the index of operand out of bounds crash because FCVT_D_W lacks frm operand.

---

Full diff: https://github.com/llvm/llvm-project/pull/149989.diff

1 Files Affected:

• (modified) llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp (+4-2)

diff --git a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
index 676479b3d5792..d54df2b5dc0ef 100644
--- a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
@@ -651,8 +651,10 @@ static std::vector<MCInst> loadFP64RegBits32(const MCSubtargetInfo &STI,
   }
 
   std::vector<MCInst> Instrs = loadIntReg(STI, ScratchIntReg, Bits);
-  Instrs.push_back(
-      MCInstBuilder(RISCV::FCVT_D_W).addReg(Reg).addReg(ScratchIntReg));
+  Instrs.push_back(MCInstBuilder(RISCV::FCVT_D_W)
+                       .addReg(Reg)
+                       .addReg(ScratchIntReg)
+                       .addImm(7));
   return Instrs;
 }
 

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Jim Lin (tclin914)

Changes

We encountered the index of operand out of bounds crash because FCVT_D_W lacks frm operand.

---

Full diff: https://github.com/llvm/llvm-project/pull/149989.diff

1 Files Affected:

• (modified) llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp (+4-2)

diff --git a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
index 676479b3d5792..d54df2b5dc0ef 100644
--- a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
@@ -651,8 +651,10 @@ static std::vector<MCInst> loadFP64RegBits32(const MCSubtargetInfo &STI,
   }
 
   std::vector<MCInst> Instrs = loadIntReg(STI, ScratchIntReg, Bits);
-  Instrs.push_back(
-      MCInstBuilder(RISCV::FCVT_D_W).addReg(Reg).addReg(ScratchIntReg));
+  Instrs.push_back(MCInstBuilder(RISCV::FCVT_D_W)
+                       .addReg(Reg)
+                       .addReg(ScratchIntReg)
+                       .addImm(7));
   return Instrs;
 }
 

[boomanaiden154]

It looks like before this produced a malformed instruction?

I would guess that the verifier should be catching an issue like this, which means that there are two reasons (assuming I'm understanding things correctly) this wasn't caught earlier:

1. There isn't test coverage in exegesis that is actually exercising this code path and running the verifier over it. In that case we should add some.
2. The verifier isn't catching this. That would be weird though given this is a simple operand count mismatch and should be trivial to catch...

[mshockwave]

could you add a test?

[tclin914]

It looks like before this produced a malformed instruction?

I would guess that the verifier should be catching an issue like this, which means that there are two reasons (assuming I'm understanding things correctly) this wasn't caught earlier:

1. There isn't test coverage in exegesis that is actually exercising this code path and running the verifier over it. In that case we should add some.
2. The verifier isn't catching this. That would be weird though given this is a simple operand count mismatch and should be trivial to catch...

Thanks for the suggestion. I've added a test case that shows how the register setup code snippet looks to ensure the code path is verified.

[tclin914]

could you add a test?

Done. Thanks.

[boomanaiden154]

LGTM. Please wait for someone with actual RISCV experience to approve before landing though.

[dybv-sc]

LGTM

[topperc]

LGTM

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-ubuntu running on as-builder-4 while building llvm at step 7 "test-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/8442

Here is the relevant piece of the build log for the reference

Step 7 (test-check-all) failure: Test just built components: check-all completed (failure)
******************** TEST 'LLVM :: TableGen/RuntimeLibcallEmitter.td' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/llvm-tblgen -gen-runtime-libcalls -I /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/../../include /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td | /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/FileCheck /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td # RUN: at line 1
+ /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/llvm-tblgen -gen-runtime-libcalls -I /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/../../include /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td
+ /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/FileCheck /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td
/home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td:98:16: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT: sqrtl_f80 = 7, // sqrtl
               ^
<stdin>:32:23: note: scanning from here
 calloc = 6, // calloc
                      ^
<stdin>:34:2: note: possible intended match here
 sqrtl_f80 = 8, // sqrtl
 ^

Input file: <stdin>
Check file: /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           .
           .
           .
          27:  ___memcpy = 1, // ___memcpy 
          28:  ___memset = 2, // ___memset 
          29:  __ashlsi3 = 3, // __ashlsi3 
          30:  __lshrdi3 = 4, // __lshrdi3 
          31:  bzero = 5, // bzero 
          32:  calloc = 6, // calloc 
next:98'0                           X error: no match found
          33:  sqrtl_f128 = 7, // sqrtl 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
          34:  sqrtl_f80 = 8, // sqrtl 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
next:98'1      ?                        possible intended match
          35:  NumLibcallImpls = 9 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~
          36: }; 
next:98'0     ~~~
          37: } // End namespace RTLIB 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
          38: } // End namespace llvm 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~
          39: #endif 
next:98'0     ~~~~~~~
...