Re: [Patch][gas][arm] Diagnose unpredictable mrrc, mrrc2 etc instructions

[Tamar Christina <Tamar dot Christina at arm dot com>]

Hi Richard,

I've updated the patch with the new mask.

Thanks,
Tamar


From: Richard Earnshaw (lists) <Richard.Earnshaw@arm.com>
Sent: Tuesday, August 16, 2016 4:19 PM
To: Tamar Christina; binutils@sourceware.org
Cc: James Greenhalgh
Subject: Re: [Patch][gas][arm] Diagnose unpredictable mrrc, mrrc2 etc instructions
    
On 16/08/16 15:12, Richard Earnshaw (lists) wrote:
> On 16/08/16 14:25, Tamar Christina wrote:
>> Hi All,
>>
>> When instructions such as MRRC are used with the same destination registers
>> then the result is unpredictable but no warning was issued to the user.
>>
>> This patch adds a warning for this pattern to the instructions:
>>
>> * MRRC
>> * MRRC2
>>
>>
>> Added new test for message and ran tests for gas for arm-none-linux-gnueabi:
>>
>> Ok for trunk?
>>
>> Thanks,
>> Tamar
>>
>> gas/
>> 2016-08-09  Tamar Christina  <tamar.christina@arm.com>
>>
>>       * config/tc-arm.c (do_co_reg2c): Added constraint.
>>
>> gas/testsuite/
>> 2016-08-09  Tamar Christina  <tamar.christina@arm.com>
>>
>>       * gas/arm/dest-unpredictable.s: New.
>>       * gas/arm/dest-unpredictable.l: New.
>>       * gas/arm/dest-unpredictable.d: New.
>>
>>
>> binutils_review6.patch
>>
>>
>> :100644 100644 73d0531... 9d0aeae... M       gas/config/tc-arm.c
>> :000000 100644 0000000... 129d08c... A       gas/testsuite/gas/arm/dest-unpredictable.d
>> :000000 100644 0000000... 7c17c25... A       gas/testsuite/gas/arm/dest-unpredictable.l
>> :000000 100644 0000000... fae22be... A       gas/testsuite/gas/arm/dest-unpredictable.s
>>
>> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
>> index 73d0531..9d0aeae 100644
>> --- a/gas/config/tc-arm.c
>> +++ b/gas/config/tc-arm.c
>> @@ -8691,6 +8691,14 @@ do_co_reg2c (void)
>>        constraint (Rn == REG_PC, BAD_PC);
>>      }
>>  
>> +  /* Only check the MRRC{2} variants.  */
>> +  if ((inst.instruction & 0x0C500000) == 0x0C500000)
> 
> I think the mask should be 0x0CF00000 - you do care that bits 21 and 23
> are both zero.
> 
> OK with that change.
> 

As just discussed off-list.  That should of course be 0x0FF00000, since
you also care about bits 24 and 25 being zero.

So take that as the pre-approved change...

Sorry for the confusion.

R.

> R.
> 
>> +    {
>> +       /* If Rd == Rn, error that the operation is
>> +      unpredictable (example MRRC p3,#1,r1,r1,c4).  */
>> +       constraint (Rd == Rn, BAD_OVERLAP);
>> +    }
>> +
>>    inst.instruction |= inst.operands[0].reg << 8;
>>    inst.instruction |= inst.operands[1].imm << 4;
>>    inst.instruction |= Rd << 12;
>> diff --git a/gas/testsuite/gas/arm/dest-unpredictable.d b/gas/testsuite/gas/arm/dest-unpredictable.d
>> new file mode 100644
>> index 0000000..129d08c
>> --- /dev/null
>> +++ b/gas/testsuite/gas/arm/dest-unpredictable.d
>> @@ -0,0 +1,2 @@
>> +# name: Unpredictable MRRC and MRRC2 instructions. - ARM
>> +# error-output: dest-unpredictable.l
>> diff --git a/gas/testsuite/gas/arm/dest-unpredictable.l b/gas/testsuite/gas/arm/dest-unpredictable.l
>> new file mode 100644
>> index 0000000..7c17c25
>> --- /dev/null
>> +++ b/gas/testsuite/gas/arm/dest-unpredictable.l
>> @@ -0,0 +1,5 @@
>> +[^:]*: Assembler messages:
>> +[^:]*:6: Error: registers may not be the same -- `mrrc p0,#1,r1,r1,c4'
>> +[^:]*:7: Error: registers may not be the same -- `mrrc2 p0,#1,r1,r1,c4'
>> +[^:]*:20: Error: registers may not be the same -- `mrrc p0,#1,r1,r1,c4'
>> +[^:]*:21: Error: registers may not be the same -- `mrrc2 p0,#1,r1,r1,c4'
>> diff --git a/gas/testsuite/gas/arm/dest-unpredictable.s b/gas/testsuite/gas/arm/dest-unpredictable.s
>> new file mode 100644
>> index 0000000..fae22be
>> --- /dev/null
>> +++ b/gas/testsuite/gas/arm/dest-unpredictable.s
>> @@ -0,0 +1,29 @@
>> +.syntax unified
>> +
>> +.arm
>> +
>> +@ warnings
>> +mrrc  p0,#1,r1,r1,c4                @ unpredictable
>> +mrrc2 p0,#1,r1,r1,c4                @ ditto
>> +
>> +@ normal
>> +mrrc  p0,#1,r1,r2,c4                @ predictable
>> +mrrc2 p0,#1,r1,r2,c4                @ ditto
>> +mcrr  p0,#1,r1,r2,c4                @ ditto
>> +mcrr2 p0,#1,r1,r2,c4                @ ditto
>> +mcrr  p0,#1,r1,r1,c4                @ ditto
>> +mcrr2 p0,#1,r1,r1,c4                @ ditto
>> +
>> +.thumb
>> +
>> +@ warnings
>> +mrrc  p0,#1,r1,r1,c4                @ unpredictable
>> +mrrc2 p0,#1,r1,r1,c4                @ ditto
>> +
>> +@ normal
>> +mrrc  p0,#1,r1,r2,c4                @ predictable
>> +mrrc2 p0,#1,r1,r2,c4                @ ditto
>> +mcrr  p0,#1,r1,r2,c4                @ ditto
>> +mcrr2 p0,#1,r1,r2,c4                @ ditto
>> +mcrr  p0,#1,r1,r1,c4                @ ditto
>> +mcrr2 p0,#1,r1,r1,c4                @ ditto
>>
> 

:100644 100644 73d0531... 0000000... M	gas/config/tc-arm.c
:000000 100644 0000000... 129d08c... A	gas/testsuite/gas/arm/dest-unpredictable.d
:000000 100644 0000000... 7c17c25... A	gas/testsuite/gas/arm/dest-unpredictable.l
:000000 100644 0000000... fae22be... A	gas/testsuite/gas/arm/dest-unpredictable.s

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 73d0531..944f3c2 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -8691,6 +8691,14 @@ do_co_reg2c (void)
       constraint (Rn == REG_PC, BAD_PC);
     }
 
+  /* Only check the MRRC{2} variants.  */
+  if ((inst.instruction & 0x0FF00000) == 0x0C500000)
+    {
+       /* If Rd == Rn, error that the operation is
+	  unpredictable (example MRRC p3,#1,r1,r1,c4).  */
+       constraint (Rd == Rn, BAD_OVERLAP);
+    }
+
   inst.instruction |= inst.operands[0].reg << 8;
   inst.instruction |= inst.operands[1].imm << 4;
   inst.instruction |= Rd << 12;
diff --git a/gas/testsuite/gas/arm/dest-unpredictable.d b/gas/testsuite/gas/arm/dest-unpredictable.d
new file mode 100644
index 0000000..129d08c
--- /dev/null
+++ b/gas/testsuite/gas/arm/dest-unpredictable.d
@@ -0,0 +1,2 @@
+# name: Unpredictable MRRC and MRRC2 instructions. - ARM
+# error-output: dest-unpredictable.l
diff --git a/gas/testsuite/gas/arm/dest-unpredictable.l b/gas/testsuite/gas/arm/dest-unpredictable.l
new file mode 100644
index 0000000..7c17c25
--- /dev/null
+++ b/gas/testsuite/gas/arm/dest-unpredictable.l
@@ -0,0 +1,5 @@
+[^:]*: Assembler messages:
+[^:]*:6: Error: registers may not be the same -- `mrrc p0,#1,r1,r1,c4'
+[^:]*:7: Error: registers may not be the same -- `mrrc2 p0,#1,r1,r1,c4'
+[^:]*:20: Error: registers may not be the same -- `mrrc p0,#1,r1,r1,c4'
+[^:]*:21: Error: registers may not be the same -- `mrrc2 p0,#1,r1,r1,c4'
diff --git a/gas/testsuite/gas/arm/dest-unpredictable.s b/gas/testsuite/gas/arm/dest-unpredictable.s
new file mode 100644
index 0000000..fae22be
--- /dev/null
+++ b/gas/testsuite/gas/arm/dest-unpredictable.s
@@ -0,0 +1,29 @@
+.syntax unified
+
+.arm
+
+@ warnings
+mrrc  p0,#1,r1,r1,c4		@ unpredictable
+mrrc2 p0,#1,r1,r1,c4		@ ditto
+
+@ normal
+mrrc  p0,#1,r1,r2,c4		@ predictable
+mrrc2 p0,#1,r1,r2,c4		@ ditto
+mcrr  p0,#1,r1,r2,c4		@ ditto
+mcrr2 p0,#1,r1,r2,c4		@ ditto
+mcrr  p0,#1,r1,r1,c4		@ ditto
+mcrr2 p0,#1,r1,r1,c4		@ ditto
+
+.thumb
+
+@ warnings
+mrrc  p0,#1,r1,r1,c4		@ unpredictable
+mrrc2 p0,#1,r1,r1,c4		@ ditto
+
+@ normal
+mrrc  p0,#1,r1,r2,c4		@ predictable
+mrrc2 p0,#1,r1,r2,c4		@ ditto
+mcrr  p0,#1,r1,r2,c4		@ ditto
+mcrr2 p0,#1,r1,r2,c4		@ ditto
+mcrr  p0,#1,r1,r1,c4		@ ditto
+mcrr2 p0,#1,r1,r1,c4		@ ditto