[arm] Convert gcc.target/arm/stl-cond.c into an RTL test

Message ID 5A5F333A.5020604@foss.arm.com
State New
Headers show
Series
  • [arm] Convert gcc.target/arm/stl-cond.c into an RTL test
Related show

Commit Message

Kyrill Tkachov Jan. 17, 2018, 11:27 a.m.
Hi all,

This is an awkward testsuite failure. The original bug was that we were failing to put out
the conditional code in the conditional form of the STL instruction (oops!).
So we wanted to output STLNE, but instead output STL.
The testacase relies on if-conversion to conditionalise the insn for STL.
However, ever since r251643 the expansion of a non-relaxed atomic store
always includes a compiler barrier. That blocks if-conversion in all cases.

So there's no easy to get to a conditional STL instruction from a C program.
But we do want to test for the original bug fix that if the RTL insn for STL is conditionalised
it should output the conditional code.

The solution in this patch is to convert the test into an RTL test with the COND_EXEC form
of the STL insn and scan the assembly output there.
This seems to work fine, and gives us an opportunity to create a gcc.dg/rtl/arm directory
in the RTL tests.

This now makes the gcc.target/arm/stl-cond.c disappear (as the test is deleted) and
the new test in gcc.dg/rtl/arm/stl-cond.c passes.

Committing to trunk.
Thanks,
Kyrill

2018-01-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.dg/rtl/arm/stl-cond.c: New test.
     * gcc.target/arm/stl-cond.c: Delete.

Comments

Christophe Lyon Jan. 18, 2018, 9:25 a.m. | #1
Hi Kyrill,


On 17 January 2018 at 12:27, Kyrill  Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,

>

> This is an awkward testsuite failure. The original bug was that we were

> failing to put out

> the conditional code in the conditional form of the STL instruction (oops!).

> So we wanted to output STLNE, but instead output STL.

> The testacase relies on if-conversion to conditionalise the insn for STL.

> However, ever since r251643 the expansion of a non-relaxed atomic store

> always includes a compiler barrier. That blocks if-conversion in all cases.

>

> So there's no easy to get to a conditional STL instruction from a C program.

> But we do want to test for the original bug fix that if the RTL insn for STL

> is conditionalised

> it should output the conditional code.

>

> The solution in this patch is to convert the test into an RTL test with the

> COND_EXEC form

> of the STL insn and scan the assembly output there.

> This seems to work fine, and gives us an opportunity to create a

> gcc.dg/rtl/arm directory

> in the RTL tests.

>

> This now makes the gcc.target/arm/stl-cond.c disappear (as the test is

> deleted) and

> the new test in gcc.dg/rtl/arm/stl-cond.c passes.

>

> Committing to trunk.

> Thanks,

> Kyrill

>

> 2018-01-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     * gcc.dg/rtl/arm/stl-cond.c: New test.

>     * gcc.target/arm/stl-cond.c: Delete.


I've noticed that the new test is unsupported on armeb, is this intentional?

Christophe
Kyrill Tkachov Jan. 18, 2018, 9:28 a.m. | #2
Hi Christophe,

On 18/01/18 09:25, Christophe Lyon wrote:
> Hi Kyrill,

>

>

> On 17 January 2018 at 12:27, Kyrill  Tkachov

> <kyrylo.tkachov@foss.arm.com> wrote:

>> Hi all,

>>

>> This is an awkward testsuite failure. The original bug was that we were

>> failing to put out

>> the conditional code in the conditional form of the STL instruction (oops!).

>> So we wanted to output STLNE, but instead output STL.

>> The testacase relies on if-conversion to conditionalise the insn for STL.

>> However, ever since r251643 the expansion of a non-relaxed atomic store

>> always includes a compiler barrier. That blocks if-conversion in all cases.

>>

>> So there's no easy to get to a conditional STL instruction from a C program.

>> But we do want to test for the original bug fix that if the RTL insn for STL

>> is conditionalised

>> it should output the conditional code.

>>

>> The solution in this patch is to convert the test into an RTL test with the

>> COND_EXEC form

>> of the STL insn and scan the assembly output there.

>> This seems to work fine, and gives us an opportunity to create a

>> gcc.dg/rtl/arm directory

>> in the RTL tests.

>>

>> This now makes the gcc.target/arm/stl-cond.c disappear (as the test is

>> deleted) and

>> the new test in gcc.dg/rtl/arm/stl-cond.c passes.

>>

>> Committing to trunk.

>> Thanks,

>> Kyrill

>>

>> 2018-01-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>>

>>      * gcc.dg/rtl/arm/stl-cond.c: New test.

>>      * gcc.target/arm/stl-cond.c: Delete.

> I've noticed that the new test is unsupported on armeb, is this intentional?


I think it can PASS on armeb. I guess it's just a matter of changing the target in
/* { dg-do compile { target arm-*-* } } */
to arm*-*-*.

Such a patch is pre-approved.
Kyrill

> Christophe
Christophe Lyon Jan. 18, 2018, 4:15 p.m. | #3
On 18 January 2018 at 10:28, Kyrill  Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Christophe,

>

>

> On 18/01/18 09:25, Christophe Lyon wrote:

>>

>> Hi Kyrill,

>>

>>

>> On 17 January 2018 at 12:27, Kyrill  Tkachov

>> <kyrylo.tkachov@foss.arm.com> wrote:

>>>

>>> Hi all,

>>>

>>> This is an awkward testsuite failure. The original bug was that we were

>>> failing to put out

>>> the conditional code in the conditional form of the STL instruction

>>> (oops!).

>>> So we wanted to output STLNE, but instead output STL.

>>> The testacase relies on if-conversion to conditionalise the insn for STL.

>>> However, ever since r251643 the expansion of a non-relaxed atomic store

>>> always includes a compiler barrier. That blocks if-conversion in all

>>> cases.

>>>

>>> So there's no easy to get to a conditional STL instruction from a C

>>> program.

>>> But we do want to test for the original bug fix that if the RTL insn for

>>> STL

>>> is conditionalised

>>> it should output the conditional code.

>>>

>>> The solution in this patch is to convert the test into an RTL test with

>>> the

>>> COND_EXEC form

>>> of the STL insn and scan the assembly output there.

>>> This seems to work fine, and gives us an opportunity to create a

>>> gcc.dg/rtl/arm directory

>>> in the RTL tests.

>>>

>>> This now makes the gcc.target/arm/stl-cond.c disappear (as the test is

>>> deleted) and

>>> the new test in gcc.dg/rtl/arm/stl-cond.c passes.

>>>

>>> Committing to trunk.

>>> Thanks,

>>> Kyrill

>>>

>>> 2018-01-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>>>

>>>      * gcc.dg/rtl/arm/stl-cond.c: New test.

>>>      * gcc.target/arm/stl-cond.c: Delete.

>>

>> I've noticed that the new test is unsupported on armeb, is this

>> intentional?

>

>

> I think it can PASS on armeb. I guess it's just a matter of changing the

> target in

> /* { dg-do compile { target arm-*-* } } */

> to arm*-*-*.

>

> Such a patch is pre-approved.

> Kyrill

>


OK, done as r256851 after confirming that is does pass on armeb.

Christophe

>> Christophe

>

>

Patch

diff --git a/gcc/testsuite/gcc.dg/rtl/arm/stl-cond.c b/gcc/testsuite/gcc.dg/rtl/arm/stl-cond.c
new file mode 100644
index 0000000000000000000000000000000000000000..e2bc610e1faf4012d06764d78d7853d2237c7b01
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/rtl/arm/stl-cond.c
@@ -0,0 +1,61 @@ 
+/* { dg-do compile { target arm-*-* } } */
+/* { dg-require-effective-target arm_arm_ok } */
+/* { dg-require-effective-target arm_arch_v8a_ok } */
+/* { dg-options "-O2 -marm" } */
+/* { dg-add-options arm_arch_v8a } */
+
+/* We want to test that the STL instruction gets the conditional
+   suffix when under a COND_EXEC.  However, COND_EXEC is very hard to
+   generate from C code because the atomic_store expansion adds a compiler
+   barrier before the insn, preventing if-conversion.  So test the output
+   here with a hand-crafted COND_EXEC wrapped around an STL.  */
+
+void __RTL (startwith ("final")) foo (int *a, int b)
+{
+(function "foo"
+  (param "a"
+    (DECL_RTL (reg/v:SI r0))
+    (DECL_RTL_INCOMING (reg:SI r0))
+  )
+  (param "b"
+    (DECL_RTL (reg/v:SI r1))
+    (DECL_RTL_INCOMING (reg:SI r1))
+  )
+  (insn-chain
+    (block 2
+	(edge-from entry (flags "FALLTHRU"))
+	(cnote 5 [bb 2] NOTE_INSN_BASIC_BLOCK)
+
+  (insn:TI 7 (parallel [
+	(set (reg:CC cc)
+	     (compare:CC (reg:SI r1)
+			 (const_int 0)))
+	(set (reg/v:SI r1)
+	     (reg:SI r1 ))
+        ])  ;; {*movsi_compare0}
+     (nil))
+
+  ;; A conditional atomic store-release: STLNE for Armv8-A.
+  (insn 10 (cond_exec (ne (reg:CC cc)
+	   (const_int 0))
+	(set (mem/v:SI (reg/v/f:SI r0) [-1  S4 A32])
+		(unspec_volatile:SI [
+		(reg/v:SI r1)
+		(const_int 3)
+		] VUNSPEC_STL))) ;; {*p atomic_storesi}
+	(expr_list:REG_DEAD (reg:CC cc)
+	(expr_list:REG_DEAD (reg/v:SI r1)
+	(expr_list:REG_DEAD (reg/v/f:SI r0)
+		(nil)))))
+      (edge-to exit (flags "FALLTHRU"))
+    ) ;; block 2
+  ) ;; insn-chain
+  (crtl
+    (return_rtx
+      (reg/i:SI r0)
+    ) ;; return_rtx
+  ) ;; crtl
+) ;; function
+}
+
+/* { dg-final { scan-assembler "stlne" } } */
diff --git a/gcc/testsuite/gcc.target/arm/stl-cond.c b/gcc/testsuite/gcc.target/arm/stl-cond.c
deleted file mode 100644
index de14bb580b82eaf8ca0a3e6e11f842c4baf5c756..0000000000000000000000000000000000000000
--- a/gcc/testsuite/gcc.target/arm/stl-cond.c
+++ /dev/null
@@ -1,19 +0,0 @@ 
-/* { dg-do compile } */
-/* { dg-require-effective-target arm_arm_ok } */ 
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-options "-O2 -marm" } */
-/* { dg-add-options arm_arch_v8a } */
-
-struct backtrace_state
-{
-  int threaded;
-  int lock_alloc;
-};
-
-void foo (struct backtrace_state *state)
-{
-  if (state->threaded)
-    __sync_lock_release (&state->lock_alloc);
-}
-
-/* { dg-final { scan-assembler "stlne" } } */