ucb-bar / esp-llvm

UCB-BAR fork of LLVM! NOT UPSTREAM RISCV LLVM
Other
123 stars 55 forks source link

Assembling "j 0x10" fails with "error: instruction requires: ", Mask == 32 #23

Open neuschaefer opened 8 years ago

neuschaefer commented 8 years ago

Hi,

I did the following:

$ echo 'j 0x10' | build1/bin/llvm-mc -triple riscv
<stdin>:1:1: error: instruction requires: (, 1<<5)
j 0x10
^
    .text

With the following (ugly) patch to help debugging:

diff --git a/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index cc198b1..2a390c8 100644
--- a/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -822,8 +822,11 @@ bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
     unsigned Mask = 1;
     for (unsigned I = 0; I < sizeof(ErrorInfo) * 8 - 1; ++I) {
       if (ErrorInfo & Mask) {
-        Msg += " ";
+        Msg += " (";
         Msg += getSubtargetFeatureName(ErrorInfo & Mask);
+        Msg += ", 1<<";
+        Msg += '0' + I;
+        Msg += ")";
       }
       Mask <<= 1;
     }

Somehow getSubtargetFeatureName(32); returns an empty string.

jordypotman commented 8 years ago

You also need to specify the CPU. The following works for me:

$ echo 'j 0x10' | llvm-mc -triple riscv -mcpu=RV64I
    .text
    j   16

The reason that getSubtargetFeatureName(32); returns an empty string is that the AssemblerPredicate definitions in RISCVInstrInfo.td do not set the feature name. The following patch fixes that:

diff --git a/lib/Target/RISCV/RISCVInstrInfo.td b/lib/Target/RISCV/RISCVInstrInfo.td
index 2a69abc..5ba7eb4 100644
--- a/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/lib/Target/RISCV/RISCVInstrInfo.td
@@ -12,17 +12,17 @@
  */

  def IsRV32 :    Predicate<"Subtarget.isRV32()">,
-                 AssemblerPredicate<"FeatureRV32">;
+                 AssemblerPredicate<"FeatureRV32", "RV32">;
  def IsRV64 :    Predicate<"Subtarget.isRV64()">,
-                 AssemblerPredicate<"FeatureRV64">;
+                 AssemblerPredicate<"FeatureRV64", "RV64">;
  def HasM   :    Predicate<"Subtarget.hasM()">,
-                 AssemblerPredicate<"FeatureM">;
+                 AssemblerPredicate<"FeatureM", "M">;
  def HasF   :    Predicate<"Subtarget.hasF()">,
-                 AssemblerPredicate<"FeatureF">;
+                 AssemblerPredicate<"FeatureF", "F">;
  def HasD   :    Predicate<"Subtarget.hasD()">,
-                 AssemblerPredicate<"FeatureD">;
+                 AssemblerPredicate<"FeatureD", "D">;
  def HasA   :    Predicate<"Subtarget.hasA()">,
-                 AssemblerPredicate<"FeatureA">;
+                 AssemblerPredicate<"FeatureA", "A">;

There is also an issue in RISCVMCTargetDesc.cpp that might be related to this. The MCInstrInfo for the RISCV64 target is not registered. The following patch fixes that:

diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp b/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
index 4cac6c0..8f1c26b 100644
--- a/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
+++ b/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
@@ -143,7 +143,7 @@ extern "C" void LLVMInitializeRISCVTargetMC() {
   // Register the MCInstrInfo.
   TargetRegistry::RegisterMCInstrInfo(TheRISCVTarget,
                                       createRISCVMCInstrInfo);
-  TargetRegistry::RegisterMCInstrInfo(TheRISCVTarget,
+  TargetRegistry::RegisterMCInstrInfo(TheRISCV64Target,
                                       createRISCVMCInstrInfo);

My riscv-llvm tree is currently quite out of date so the line numbers in the above patches might not be correct.

neuschaefer commented 8 years ago

Thanks for taking a look. Your patch works (as far as I can see).