[arm] Follow up for asm-flags vs thumb1

Message ID 18352f16-98fa-2e07-1274-473253a7217d@linaro.org
State New
Headers show
Series
  • [arm] Follow up for asm-flags vs thumb1
Related show

Commit Message

Richard Henderson Nov. 14, 2019, 5:42 p.m.
What I committed today does in fact ICE for thumb1, as you suspected.

I'm currently testing the following vs

  arm-sim/
  arm-sim/-mthumb
  arm-sim/-mcpu=cortex-a15/-mthumb.

which, with the default cpu for arm-elf-eabi, should test all of arm, thumb1,
thumb2.

I'm not thrilled about the ifdef in aarch-common.c, but I don't see a different
way to catch this case for arm and still compile for aarch64.

Ideas?

Particularly ones that work with __attribute__((target("thumb")))?  Which, now
that I've thought about it I really should be testing...


r~
gcc/
	* config/arm/aarch-common.c (arm_md_asm_adjust): Sorry
	for asm flags in thumb1 mode.
	* config/arm/arm-c.c (arm_cpu_builtins): Do not define
	__GCC_ASM_FLAG_OUTPUTS__ in thumb1 mode.
	* doc/extend.texi (FlagOutputOperands): Document thumb1 restriction.

gcc/testsuite/
	* gcc.target/arm/asm-flag-1.c: Skip if arm_thumb1.
	* gcc.target/arm/asm-flag-3.c: Skip if arm_thumb1.
	* gcc.target/arm/asm-flag-5.c: Skip if arm_thumb1.
	* gcc.target/arm/asm-flag-6.c: Skip if arm_thumb1.

Comments

Richard Earnshaw (lists) Nov. 15, 2019, 2:26 p.m. | #1
On 14/11/2019 17:42, Richard Henderson wrote:
> What I committed today does in fact ICE for thumb1, as you suspected.

> 

> I'm currently testing the following vs

> 

>    arm-sim/

>    arm-sim/-mthumb

>    arm-sim/-mcpu=cortex-a15/-mthumb.

> 

> which, with the default cpu for arm-elf-eabi, should test all of arm, thumb1,

> thumb2.

> 

> I'm not thrilled about the ifdef in aarch-common.c, but I don't see a different

> way to catch this case for arm and still compile for aarch64.

> 

> Ideas?

> 

> Particularly ones that work with __attribute__((target("thumb")))?  Which, now

> that I've thought about it I really should be testing...

> 

> 

> r~

> 


+#ifdef TARGET_THUMB1
+      if (TARGET_THUMB1)

Could you use one of the standard HAVE_<insn> patterns?

+
+  if (!TARGET_THUMB1)
+    builtin_define ("__GCC_ASM_FLAG_OUTPUTS__");

I think you should use def_or_undef_macro() here, so that if the user 
switches to thumb using the target attribute extensions the macro will 
become undefined for the duration of that switch.

R.

Patch

diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index 760ef6c9c0a..6f3db3838ba 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -544,6 +544,15 @@  arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> &/*inputs*/,
       if (strncmp (con, "=@cc", 4) != 0)
 	continue;
       con += 4;
+
+#ifdef TARGET_THUMB1
+      if (TARGET_THUMB1)
+	{
+	  sorry ("asm flags not supported in thumb1 mode");
+	  break;
+	}
+#endif
+
       if (strchr (con, ',') != NULL)
 	{
 	  error ("alternatives not allowed in %<asm%> flag output");
diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c
index c4485ce7af1..865c448d531 100644
--- a/gcc/config/arm/arm-c.c
+++ b/gcc/config/arm/arm-c.c
@@ -122,7 +122,9 @@  arm_cpu_builtins (struct cpp_reader* pfile)
   if (arm_arch_notm)
     builtin_define ("__ARM_ARCH_ISA_ARM");
   builtin_define ("__APCS_32__");
-  builtin_define ("__GCC_ASM_FLAG_OUTPUTS__");
+
+  if (!TARGET_THUMB1)
+    builtin_define ("__GCC_ASM_FLAG_OUTPUTS__");
 
   def_or_undef_macro (pfile, "__thumb__", TARGET_THUMB);
   def_or_undef_macro (pfile, "__thumb2__", TARGET_THUMB2);
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 1c8ae0d5cd3..62a98e939c8 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9810,6 +9810,8 @@  signed greater than
 signed less than equal
 @end table
 
+The flag output constraints are not supported in thumb1 mode.
+
 @item x86 family
 The flag output constraints for the x86 family are of the form
 @samp{=@@cc@var{cond}} where @var{cond} is one of the standard
diff --git a/gcc/testsuite/gcc.target/arm/asm-flag-1.c b/gcc/testsuite/gcc.target/arm/asm-flag-1.c
index 9707ebfcebb..97104d3ac73 100644
--- a/gcc/testsuite/gcc.target/arm/asm-flag-1.c
+++ b/gcc/testsuite/gcc.target/arm/asm-flag-1.c
@@ -1,6 +1,7 @@ 
 /* Test the valid @cc<cc> asm flag outputs.  */
 /* { dg-do compile } */
 /* { dg-options "-O" } */
+/* { dg-skip-if "" { arm_thumb1 } } */
 
 #ifndef __GCC_ASM_FLAG_OUTPUTS__
 #error "missing preprocessor define"
diff --git a/gcc/testsuite/gcc.target/arm/asm-flag-3.c b/gcc/testsuite/gcc.target/arm/asm-flag-3.c
index e84e3431277..e2d616051cc 100644
--- a/gcc/testsuite/gcc.target/arm/asm-flag-3.c
+++ b/gcc/testsuite/gcc.target/arm/asm-flag-3.c
@@ -1,6 +1,7 @@ 
 /* Test some of the valid @cc<cc> asm flag outputs.  */
 /* { dg-do compile } */
 /* { dg-options "-O" } */
+/* { dg-skip-if "" { arm_thumb1 } } */
 
 #define DO(C) \
 void f##C(void) { char x; asm("" : "=@cc"#C(x)); if (!x) asm(""); asm(""); }
diff --git a/gcc/testsuite/gcc.target/arm/asm-flag-5.c b/gcc/testsuite/gcc.target/arm/asm-flag-5.c
index 4d4394e1478..9a8ff586c29 100644
--- a/gcc/testsuite/gcc.target/arm/asm-flag-5.c
+++ b/gcc/testsuite/gcc.target/arm/asm-flag-5.c
@@ -1,6 +1,7 @@ 
 /* Test error conditions of asm flag outputs.  */
 /* { dg-do compile } */
 /* { dg-options "" } */
+/* { dg-skip-if "" { arm_thumb1 } } */
 
 void f_B(void) { _Bool x; asm("" : "=@cccc"(x)); }
 void f_c(void) { char x; asm("" : "=@cccc"(x)); }
diff --git a/gcc/testsuite/gcc.target/arm/asm-flag-6.c b/gcc/testsuite/gcc.target/arm/asm-flag-6.c
index 09174e04ae6..d862db4e106 100644
--- a/gcc/testsuite/gcc.target/arm/asm-flag-6.c
+++ b/gcc/testsuite/gcc.target/arm/asm-flag-6.c
@@ -1,5 +1,6 @@ 
 /* Executable testcase for 'output flags.'  */
 /* { dg-do run } */
+/* { dg-skip-if "" { arm_thumb1 } } */
 
 int test_bits (long nzcv)
 {