[PATCH] [mips] Add names and tests for the hardware registers

Daniel Sanders daniel.sanders at imgtec.com
Tue Nov 4 02:47:10 PST 2014


LGTM with a nit, hwr_ulr added, and the FIXME comments about the .set directives.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1760
@@ +1759,3 @@
+           .Case("hwr_ccres", 3)
+           .Default(-1);
+
----------------
Could you add 'hwr_ulr' ($29) too? GAS doesn't accept it at the moment, but it's in the spec.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2255
@@ -2225,1 +2254,3 @@
+
+
   Index = matchFPURegisterName(Identifier);
----------------
Nit: Multiple blank lines

================
Comment at: test/MC/Mips/mips-hwr-register-names.s:6-9
@@ +5,6 @@
+        .set noat
+        # CHECK:      .set    push
+        # CHECK-NEXT: .set    mips32r2
+        # CHECK-NEXT: rdhwr   $4, $hwr_cpunum
+        # CHECK-NEXT: .set    pop             # encoding: [0x7c,0x04,0x00,0x3b]
+        rdhwr     $a0,$hwr_cpunum
----------------
Could you add a FIXME comment about the .set lines?

Eventually, I'd like the instruction printer for rdhwr to stop printing the .set directives around it (this doesn't need to be done in this patch). It makes some sense for CodeGen to do emit them but not for the assembler.

================
Comment at: test/MC/Mips/mips32r2/valid.s:140-143
@@ -139,3 +139,6 @@
         pref      1, 8($5)             # CHECK: pref 1, 8($5)           # encoding: [0xcc,0xa1,0x00,0x08]
-        rdhwr     $sp,$11              
+        rdhwr     $sp,$11              # CHECK:      .set  push
+                                       # CHECK-NEXT: .set  mips32r2
+                                       # CHECK-NEXT: rdhwr $sp, $11
+                                       # CHECK-NEXT: .set  pop          # encoding: [0x7c,0x1d,0x58,0x3b]
         rotr      $1,15                # CHECK: rotr $1, $1, 15         # encoding: [0x00,0x21,0x0b,0xc2]
----------------
I'd like to remove the checks for the .set directives but the encoding is on the '.set pop' line so we'd have to keep that for the moment. It would look strange to have a .set pop without a .set push so let's leave the .set directives in the test for now.

Could you add a FIXME comment about them?

http://reviews.llvm.org/D5763






More information about the llvm-commits mailing list