Message ID | 5A5F333A.5020604@foss.arm.com |
---|---|
State | New |
Headers | show |
Series | [arm] Convert gcc.target/arm/stl-cond.c into an RTL test | expand |
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
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
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 > >
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" } } */