[PATCH] D7174: [AArch64] Fix problems in handling generic MSR/MRS instructions

James Molloy james.molloy at arm.com
Mon Jul 6 02:36:30 PDT 2015


jmolloy accepted this revision.

This revision is now accepted and ready to land.

Hi Petr,

This generally looks good to me. Apart from the nits in the comments below, generally:

- The tablegen+tests changes need to be split from the ARM changes and applied in two separate patches.
- The spelling correction needs to be split out and committed as its own (trivial, needs no code review) patch.

Cheers,

James


================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:1084
@@ +1083,3 @@
+      << "return MCDisassembler::Fail; }\n";
+  }
+  else {
----------------
Merge lines 1084 and 1085 together.

================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:1899
@@ +1898,3 @@
+      RecordVal *HasCompleteDecoderVal = TypeRecord->getValue("hasCompleteDecoder");
+      BitInit *HasCompleteDecoderBit = HasCompleteDecoderVal ?
+        dyn_cast<BitInit>(HasCompleteDecoderVal->getValue()) : nullptr;
----------------
You can use dyn_cast_or_null here:

    BitInit *BI = dyn_cast_or_null<BitInit>(TypeRecord->getValue("hasCompleteDecoder")); 
    bool HasCompleteDecoder = BI ? BI->getValue() : true;

================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:1976
@@ -1909,2 +1975,3 @@
 
-    OperandInfo OpInfo(Decoder);
+    RecordVal *HasCompleteDecoderVal = TypeRecord->getValue("hasCompleteDecoder");
+    BitInit *HasCompleteDecoderBit = HasCompleteDecoderVal ?
----------------
Same comment as in line 1899.

================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:2179
@@ -2100,2 +2178,3 @@
      << "                   << \", using decoder \" << DecodeIdx << \"\\n\" );\n"
-     << "      DEBUG(dbgs() << \"----- DECODE SUCCESSFUL -----\\n\");\n"
+     << "      DEBUG(dbgs() << \"----- DECODE COMPLETE -----\\n\");\n"
+     << "      return S;\n"
----------------
Why change SUCCESSFUL to COMPLETE here?


http://reviews.llvm.org/D7174







More information about the llvm-commits mailing list