diff mbox

MIPS: fix failing branch range checks for micromips

Message ID 0DA23CC379F5F945ACB41CF394B98277210EDC9C@LEMAIL01.le.imgtec.org
State New
Headers show

Commit Message

Andrew Bennett July 3, 2015, 11:50 p.m. UTC 
Hi,

The current branch range tests assume that the MIPS branch instructions 
have a 16 bit branch offset which is shifted by 2.  Unfortunately for microMIPS 
this offset is shifted by 1 which reduces the branch range and is causing the 
branch-[2,4,6,10,12].c tests to fail.  
 
The following patch fixes this issue by firstly adding a new macro to branch-helper.h
which outputs the correct number of nops to describe the maximum positive range
of a 16 bit micromips branch offset (assuming the branch instruction has a delay slot).
Secondly it breaks-up the branch-[2,4,6,10,12].c files into mips tests (which have 
-mno-micromips added to them) and micromips tests (which use the new macro).

I have tested this on the mips-mti-elf target using mips32r2/{-mno-micromips/-mmicromips}
test options and there are no new regressions.
	  
There is a follow-up patch that I will be working on that will correctly update the other 
branch tests to correctly test out of range branch behaviour for micromips.  Currently these 
are passing because the mips branch range offset is large enough.  These offsets will 
need to be reduced for micromips to verify the compiler is calculating branch ranges correctly.

The ChangeLog and patch are below.

Ok to commit?


Many thanks,



Andrew



testsuite/
	* gcc.target/mips/branch-2.c: Add -mno-micromips to dg-options.
	* gcc.target/mips/branch-4.c: Ditto.
	* gcc.target/mips/branch-6.c: Ditto. 
	* gcc.target/mips/branch-8.c: Ditto.
	* gcc.target/mips/branch-10.c: Ditto.
	* gcc.target/mips/branch-12.c: Ditto.
	* gcc.target/mips/branch-umips-2.c: New file.
	* gcc.target/mips/branch-umips-4.c: New file.
	* gcc.target/mips/branch-umips-6.c: New file. 
	* gcc.target/mips/branch-umips-8.c: New file.
	* gcc.target/mips/branch-umips-10.c: New file.
	* gcc.target/mips/branch-umips-12.c: New file.
	* gcc.target/mips/branch-helper.h (OCCUPY_0xfffc): New define.

Comments

Moore, Catherine July 6, 2015, 11:22 p.m. UTC | #1 
Hi Andrew,

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Andrew Bennett
> Sent: Friday, July 03, 2015 7:50 PM
> Subject: [PATCH] MIPS: fix failing branch range checks for micromips
> 
> diff --git a/gcc/testsuite/gcc.target/mips/branch-10.c
> b/gcc/testsuite/gcc.target/mips/branch-10.c
> index e2b1b5f..00569b0 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-10.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-10.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-mshared -mabi=n32" } */
> +/* { dg-options "-mshared -mabi=n32 -mno-micromips" } */
>  /* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
>  /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
> 

Like the other patch, the -mno-micromips should be removed from dg-options and NOCOMPRESS used in place of NOMIPS16.
This comment applies to all of the branch-*.c tests.

> 
> diff --git a/gcc/testsuite/gcc.target/mips/branch-helper.h
> b/gcc/testsuite/gcc.target/mips/branch-helper.h
> index 85399be..bc4a31f 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-helper.h
> +++ b/gcc/testsuite/gcc.target/mips/branch-helper.h
> @@ -33,5 +33,23 @@
>         D2 ("nop") "\n\t" \
>         D1 ("nop"))
> 
> +/* Emit something that is 0xfffc bytes long, which is the largest
> +   permissible range for micromips forward branches when branches

s/micromips/microMIPS/

> +   have delay slots.  */
> +#define OCCUPY_0xfffc \
> +  asm (D13 ("nop32") "\n\t" \
> +       D12 ("nop32") "\n\t" \
> +       D11 ("nop32") "\n\t" \
> +       D10 ("nop32") "\n\t" \
> +       D9 ("nop32") "\n\t" \
> +       D8 ("nop32") "\n\t" \
> +       D7 ("nop32") "\n\t" \
> +       D6 ("nop32") "\n\t" \
> +       D5 ("nop32") "\n\t" \
> +       D4 ("nop32") "\n\t" \
> +       D3 ("nop32") "\n\t" \
> +       D2 ("nop32") "\n\t" \
> +       D1 ("nop32") "\n\t" \
> +       D0 ("nop32"))
>  /* Likewise emit something that is 0x1fffc bytes long.  */  #define
> OCCUPY_0x1fffc do { asm ("nop"); OCCUPY_0x1fff8; } while (0)

diff --git
> a/gcc/testsuite/gcc.target/mips/branch-umips-10.c
> b/gcc/testsuite/gcc.target/mips/branch-umips-10.c

I see that you are naming these tests after the original branch-<number> tests that they were derived from.
I think it would be better to keep all of the microMIPS tests named umips-???.    I don't think preserving the original number is important.

Thanks,
Catherine
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/mips/branch-10.c b/gcc/testsuite/gcc.target/mips/branch-10.c
index e2b1b5f..00569b0 100644
--- a/gcc/testsuite/gcc.target/mips/branch-10.c
+++ b/gcc/testsuite/gcc.target/mips/branch-10.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=n32" } */
+/* { dg-options "-mshared -mabi=n32 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 
diff --git a/gcc/testsuite/gcc.target/mips/branch-12.c b/gcc/testsuite/gcc.target/mips/branch-12.c
index 4aef160..7d7580b 100644
--- a/gcc/testsuite/gcc.target/mips/branch-12.c
+++ b/gcc/testsuite/gcc.target/mips/branch-12.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=64" } */
+/* { dg-options "-mshared -mabi=64 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 
diff --git a/gcc/testsuite/gcc.target/mips/branch-2.c b/gcc/testsuite/gcc.target/mips/branch-2.c
index 6409c4c..241e885 100644
--- a/gcc/testsuite/gcc.target/mips/branch-2.c
+++ b/gcc/testsuite/gcc.target/mips/branch-2.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=32" } */
+/* { dg-options "-mshared -mabi=32 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$25|\\\$28|cpload)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 /* { dg-final { scan-assembler-not "\\.cprestore" } } */
diff --git a/gcc/testsuite/gcc.target/mips/branch-4.c b/gcc/testsuite/gcc.target/mips/branch-4.c
index 31e4909..923e6d4 100644
--- a/gcc/testsuite/gcc.target/mips/branch-4.c
+++ b/gcc/testsuite/gcc.target/mips/branch-4.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=n32" } */
+/* { dg-options "-mshared -mabi=n32 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$25|\\\$28|%gp_rel|%got)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 
diff --git a/gcc/testsuite/gcc.target/mips/branch-6.c b/gcc/testsuite/gcc.target/mips/branch-6.c
index 77e0340..2c75ab1 100644
--- a/gcc/testsuite/gcc.target/mips/branch-6.c
+++ b/gcc/testsuite/gcc.target/mips/branch-6.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=64" } */
+/* { dg-options "-mshared -mabi=64 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$25|\\\$28|%gp_rel|%got)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 
diff --git a/gcc/testsuite/gcc.target/mips/branch-8.c b/gcc/testsuite/gcc.target/mips/branch-8.c
index ba5f954..85df6b8 100644
--- a/gcc/testsuite/gcc.target/mips/branch-8.c
+++ b/gcc/testsuite/gcc.target/mips/branch-8.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-mshared -mabi=32" } */
+/* { dg-options "-mshared -mabi=32 -mno-micromips" } */
 /* { dg-final { scan-assembler-not "(\\\$28|cpload|cprestore)" } } */
 /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
 
diff --git a/gcc/testsuite/gcc.target/mips/branch-helper.h b/gcc/testsuite/gcc.target/mips/branch-helper.h
index 85399be..bc4a31f 100644
--- a/gcc/testsuite/gcc.target/mips/branch-helper.h
+++ b/gcc/testsuite/gcc.target/mips/branch-helper.h
@@ -33,5 +33,23 @@ 
        D2 ("nop") "\n\t" \
        D1 ("nop"))
 
+/* Emit something that is 0xfffc bytes long, which is the largest
+   permissible range for micromips forward branches when branches
+   have delay slots.  */
+#define OCCUPY_0xfffc \
+  asm (D13 ("nop32") "\n\t" \
+       D12 ("nop32") "\n\t" \
+       D11 ("nop32") "\n\t" \
+       D10 ("nop32") "\n\t" \
+       D9 ("nop32") "\n\t" \
+       D8 ("nop32") "\n\t" \
+       D7 ("nop32") "\n\t" \
+       D6 ("nop32") "\n\t" \
+       D5 ("nop32") "\n\t" \
+       D4 ("nop32") "\n\t" \
+       D3 ("nop32") "\n\t" \
+       D2 ("nop32") "\n\t" \
+       D1 ("nop32") "\n\t" \
+       D0 ("nop32"))
 /* Likewise emit something that is 0x1fffc bytes long.  */
 #define OCCUPY_0x1fffc do { asm ("nop"); OCCUPY_0x1fff8; } while (0)
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-10.c b/gcc/testsuite/gcc.target/mips/branch-umips-10.c
new file mode 100644
index 0000000..e84b462
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-10.c
@@ -0,0 +1,13 @@ 
+/* { dg-options "-mshared -mabi=n32 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
+/* { dg-final { scan-assembler-not "\tjrc?\t\\\$1\n" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (int (*bar) (void), int *x)
+{
+  *x = bar ();
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-12.c b/gcc/testsuite/gcc.target/mips/branch-umips-12.c
new file mode 100644
index 0000000..6704aad
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-12.c
@@ -0,0 +1,13 @@ 
+/* { dg-options "-mshared -mabi=64 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
+/* { dg-final { scan-assembler-not "\tjrc?\t\\\$1\n" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (int (*bar) (void), int *x)
+{
+  *x = bar ();
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-2.c b/gcc/testsuite/gcc.target/mips/branch-umips-2.c
new file mode 100644
index 0000000..d124f91
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-2.c
@@ -0,0 +1,13 @@ 
+/* { dg-options "-mshared -mabi=32 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$25|\\\$28|cpload)" } } */
+/* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
+/* { dg-final { scan-assembler-not "\\.cprestore" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (volatile int *x)
+{
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-4.c b/gcc/testsuite/gcc.target/mips/branch-umips-4.c
new file mode 100644
index 0000000..294805a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-4.c
@@ -0,0 +1,12 @@ 
+/* { dg-options "-mshared -mabi=n32 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$25|\\\$28|%gp_rel|%got)" } } */
+/* { dg-final { scan-assembler-not "\tjrc?\t\\\$1\n" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (volatile int *x)
+{
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-6.c b/gcc/testsuite/gcc.target/mips/branch-umips-6.c
new file mode 100644
index 0000000..ab02e88
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-6.c
@@ -0,0 +1,12 @@ 
+/* { dg-options "-mshared -mabi=64 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$25|\\\$28|%gp_rel|%got)" } } */
+/* { dg-final { scan-assembler-not "\tjrc?\t\\\$1\n" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (volatile int *x)
+{
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}
diff --git a/gcc/testsuite/gcc.target/mips/branch-umips-8.c b/gcc/testsuite/gcc.target/mips/branch-umips-8.c
new file mode 100644
index 0000000..5050669
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/branch-umips-8.c
@@ -0,0 +1,13 @@ 
+/* { dg-options "-mshared -mabi=32 -mmicromips" } */
+/* { dg-final { scan-assembler-not "(\\\$28|cpload|cprestore)" } } */
+/* { dg-final { scan-assembler-not "\tjrc?\t\\\$1\n" } } */
+
+#include "branch-helper.h"
+
+NOMIPS16 void
+foo (int (*bar) (void), int *x)
+{
+  *x = bar ();
+  if (__builtin_expect (*x == 0, 1))
+    OCCUPY_0xfffc;
+}