[PATCH] ARM: Improve handling of the Thumb2 M-class MSR instruction

Petr Pavlu petr.pavlu at arm.com
Fri Aug 29 03:48:39 PDT 2014


Hi Renato,

Thank you for looking at this patch. Responses to your comments inlined.

Petr

================
Comment at: lib/Target/ARM/ARMInstrThumb2.td:4023
@@ -4022,2 +4022,3 @@
   let Inst{19-16} = 0b1111;
   let Inst{7-0} = mask;
+
----------------
rengolin wrote:
> I know it was there before, but this "mask" could also be called "SYSm" for consistency
Makes sense. I will attach a new patch. I also noticed that line "let Inst{19-16} = 0b1111;" is not necessary because the information is already provided in "let Inst{31-12} = 0b11110011111011111000;" so the new patch removes it.

================
Comment at: lib/Target/ARM/ARMInstrThumb2.td:4025
@@ +4024,3 @@
+
+  let Unpredictable{20-16} = 0b11111;
+  let Unpredictable{13} = 0b1;
----------------
rengolin wrote:
> Isn't this the same as saying:
> 
>     let Unpredictable{20} = 0b1
> 
It does not seem to be same. With "let Unpredictable{20-16} = 0b11111;":
```
$ ./bin/llvm-mc --disassemble -triple=thumbv7em
0xe0 0xf3 0x00 0x80
        .text
<stdin>:1:1: warning: potentially undefined instruction encoding
0xe0 0xf3 0x00 0x80
^
        mrs     r0, apsr
```

With "let Unpredictable{20} = 0b1;":
```
$ ./bin/llvm-mc --disassemble -triple=thumbv7em
0xe0 0xf3 0x00 0x80
        .text
<stdin>:1:1: warning: invalid instruction encoding
0xe0 0xf3 0x00 0x80
^
        lsls    r3, r6, #3
<stdin>:1:16: warning: invalid instruction encoding
0xe0 0xf3 0x00 0x80
               ^
```

The first output is the expected one.

================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:817
@@ +816,3 @@
+    // For writes, handle extended mask bits if the DSP extension is present.
+    if (Opcode == ARM::t2MSR_M && (FeatureBits & ARM::FeatureDSPThumb2)) {
+      switch (SYSm) {
----------------
rengolin wrote:
> Was the t2MRS_M a typo? I'm surprised it worked at all...
It was not a typo. The patch structures the code in a way that this condition is now inverted (MRS -> MSR).

http://reviews.llvm.org/D4979






More information about the llvm-commits mailing list