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

[Petr Pavlu]

Hi James,

Thank you for the review. I will split the changes into three separate patches as you suggested before committing (typo fix, TableGen, AArch64 part).
Other comments are inlined.

Thanks,
Petr


================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:1899
@@ +1898,3 @@
+      RecordVal *HasCompleteDecoderVal = TypeRecord->getValue("hasCompleteDecoder");
+      BitInit *HasCompleteDecoderBit = HasCompleteDecoderVal ?
+        dyn_cast<BitInit>(HasCompleteDecoderVal->getValue()) : nullptr;
----------------
jmolloy wrote:
> You can use dyn_cast_or_null here:
> 
>     BitInit *BI = dyn_cast_or_null<BitInit>(TypeRecord->getValue("hasCompleteDecoder")); 
>     bool HasCompleteDecoder = BI ? BI->getValue() : true;
This would need
```
BitInit *BI = dyn_cast_or_null<BitInit>(TypeRecord->getValue("hasCompleteDecoder")->getValue());
```
but `TypeRecord->getValue("hasCompleteDecoder")` can be `nullptr` so that cannot be used.

================
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"
----------------
jmolloy wrote:
> Why change SUCCESSFUL to COMPLETE here?
I believe the original intention of this message was to give information that the "filter decoding" part in FixedLenDecoder was successful. This is ok but the bitpattern then gets passed by FixedLenDecoder to a specific decoder method, it can reject the bitpattern and so the decoding as a whole fails.

This created a bit confusing output from `llvm-mc -disassemble -debug` when the input bitpattern was invalid. It would contain 'DECODE SUCCESSFUL' followed by 'invalid instruction encoding':
```
$ echo "0x1f 0x40 0x00 0xd5" | llvm-mc -triple=aarch64 -show-inst -show-encoding -disassemble -debug
[...]
31211: OPC_CheckField(19, 1, 0, 5): FieldValue = 0, ExpectedValue = 0: PASS
31217: OPC_Decode: opcode 1283, using decoder 220
----- DECODE SUCCESSFUL -----
<stdin>:1:1: warning: invalid instruction encoding
0x1f 0x40 0x00 0xd5
^
```

The patch changed this to printing 'DECODE COMPLETE' when FixedLenDecoder fully finished the decoding (including running any decoding methods).

However, it might be better to simply remove this message because it does not provide much information (`OPC_Decode` implies end of decoding similarly to `OPC_Fail`) and additionally extend the `OPC_Decode` debug message with a result returned from the decoding method. I will update the patch.


http://reviews.llvm.org/D7174