* [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
@ 2020-01-16 13:32 Wilco Dijkstra
2020-01-16 13:47 ` Richard Sandiford
2020-01-27 16:41 ` Segher Boessenkool
0 siblings, 2 replies; 7+ messages in thread
From: Wilco Dijkstra @ 2020-01-16 13:32 UTC (permalink / raw)
To: GCC Patches
Cc: Andrew Pinski, Kyrylo Tkachov, Richard Earnshaw, Richard Sandiford
The separate shrinkwrapping pass may insert stores in the middle
of atomics loops which can cause issues on some implementations.
Avoid this by delaying splitting of atomic patterns until after
prolog/epilog generation.
Bootstrap completed, no test regressions on AArch64.
Andrew, can you verify this fixes the failure you were getting?
ChangeLog:
2020-01-16 Wilco Dijkstra <wdijkstr@arm.com>
PR target/92692
* config/aarch64/aarch64.c (aarch64_split_compare_and_swap)
Add assert to ensure prolog has been emitted.
(aarch64_split_atomic_op): Likewise.
* config/aarch64/atomics.md (aarch64_compare_and_swap<mode>)
Use epilogue_completed rather than reload_completed.
(aarch64_atomic_exchange<mode>): Likewise.
(aarch64_atomic_<atomic_optab><mode>): Likewise.
(atomic_nand<mode>): Likewise.
(aarch64_atomic_fetch_<atomic_optab><mode>): Likewise.
(atomic_fetch_nand<mode>): Likewise.
(aarch64_atomic_<atomic_optab>_fetch<mode>): Likewise.
(atomic_nand_fetch<mode>): Likewise.
---
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ac89cc1f9c938455d33d8850d9ebfc0473cb73dc..cd9d813f2ac4990971f6435fdb28b0f94ae10309 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18375,6 +18375,9 @@ aarch64_emit_post_barrier (enum memmodel model)
void
aarch64_split_compare_and_swap (rtx operands[])
{
+ /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
+ gcc_assert (epilogue_completed);
+
rtx rval, mem, oldval, newval, scratch, x, model_rtx;
machine_mode mode;
bool is_weak;
@@ -18469,6 +18472,9 @@ void
aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
rtx value, rtx model_rtx, rtx cond)
{
+ /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
+ gcc_assert (epilogue_completed);
+
machine_mode mode = GET_MODE (mem);
machine_mode wmode = (mode == DImode ? DImode : SImode);
const enum memmodel model = memmodel_from_int (INTVAL (model_rtx));
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index c2bcabd0c3c2627b7222dcbc1af9c2e6b7ce6a76..996947799b5ef8445e9786b94e1ce62fd16e5b5c 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -56,7 +56,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
(clobber (match_scratch:SI 7 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_compare_and_swap (operands);
@@ -80,7 +80,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
(clobber (match_scratch:SI 7 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_compare_and_swap (operands);
@@ -104,7 +104,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
(clobber (match_scratch:SI 7 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_compare_and_swap (operands);
@@ -223,7 +223,7 @@ (define_insn_and_split "aarch64_atomic_exchange<mode>"
(clobber (match_scratch:SI 4 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (SET, operands[0], NULL, operands[1],
@@ -344,7 +344,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab><mode>"
(clobber (match_scratch:SI 4 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (<CODE>, NULL, operands[3], operands[0],
@@ -400,7 +400,7 @@ (define_insn_and_split "atomic_nand<mode>"
(clobber (match_scratch:SI 4 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (NOT, NULL, operands[3], operands[0],
@@ -504,7 +504,7 @@ (define_insn_and_split "aarch64_atomic_fetch_<atomic_optab><mode>"
(clobber (match_scratch:SI 5 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (<CODE>, operands[0], operands[4], operands[1],
@@ -551,7 +551,7 @@ (define_insn_and_split "atomic_fetch_nand<mode>"
(clobber (match_scratch:SI 5 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (NOT, operands[0], operands[4], operands[1],
@@ -604,7 +604,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab>_fetch<mode>"
(clobber (match_scratch:SI 4 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (<CODE>, NULL, operands[0], operands[1],
@@ -628,7 +628,7 @@ (define_insn_and_split "atomic_nand_fetch<mode>"
(clobber (match_scratch:SI 4 "=&r"))]
""
"#"
- "&& reload_completed"
+ "&& epilogue_completed"
[(const_int 0)]
{
aarch64_split_atomic_op (NOT, NULL, operands[0], operands[1],
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-16 13:32 [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692) Wilco Dijkstra
@ 2020-01-16 13:47 ` Richard Sandiford
2020-01-17 2:37 ` Andrew Pinski
2020-01-27 16:41 ` Segher Boessenkool
1 sibling, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-01-16 13:47 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Andrew Pinski, Kyrylo Tkachov, Richard Earnshaw
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> The separate shrinkwrapping pass may insert stores in the middle
> of atomics loops which can cause issues on some implementations.
> Avoid this by delaying splitting of atomic patterns until after
> prolog/epilog generation.
>
> Bootstrap completed, no test regressions on AArch64.
>
> Andrew, can you verify this fixes the failure you were getting?
>
> ChangeLog:
> 2020-01-16 Wilco Dijkstra <wdijkstr@arm.com>
>
> PR target/92692
> * config/aarch64/aarch64.c (aarch64_split_compare_and_swap)
> Add assert to ensure prolog has been emitted.
> (aarch64_split_atomic_op): Likewise.
> * config/aarch64/atomics.md (aarch64_compare_and_swap<mode>)
> Use epilogue_completed rather than reload_completed.
> (aarch64_atomic_exchange<mode>): Likewise.
> (aarch64_atomic_<atomic_optab><mode>): Likewise.
> (atomic_nand<mode>): Likewise.
> (aarch64_atomic_fetch_<atomic_optab><mode>): Likewise.
> (atomic_fetch_nand<mode>): Likewise.
> (aarch64_atomic_<atomic_optab>_fetch<mode>): Likewise.
> (atomic_nand_fetch<mode>): Likewise.
OK if Andrew confirms it works, thanks.
Richard
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index ac89cc1f9c938455d33d8850d9ebfc0473cb73dc..cd9d813f2ac4990971f6435fdb28b0f94ae10309 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -18375,6 +18375,9 @@ aarch64_emit_post_barrier (enum memmodel model)
> void
> aarch64_split_compare_and_swap (rtx operands[])
> {
> + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> + gcc_assert (epilogue_completed);
> +
> rtx rval, mem, oldval, newval, scratch, x, model_rtx;
> machine_mode mode;
> bool is_weak;
> @@ -18469,6 +18472,9 @@ void
> aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
> rtx value, rtx model_rtx, rtx cond)
> {
> + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> + gcc_assert (epilogue_completed);
> +
> machine_mode mode = GET_MODE (mem);
> machine_mode wmode = (mode == DImode ? DImode : SImode);
> const enum memmodel model = memmodel_from_int (INTVAL (model_rtx));
> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> index c2bcabd0c3c2627b7222dcbc1af9c2e6b7ce6a76..996947799b5ef8445e9786b94e1ce62fd16e5b5c 100644
> --- a/gcc/config/aarch64/atomics.md
> +++ b/gcc/config/aarch64/atomics.md
> @@ -56,7 +56,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> (clobber (match_scratch:SI 7 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_compare_and_swap (operands);
> @@ -80,7 +80,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> (clobber (match_scratch:SI 7 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_compare_and_swap (operands);
> @@ -104,7 +104,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> (clobber (match_scratch:SI 7 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_compare_and_swap (operands);
> @@ -223,7 +223,7 @@ (define_insn_and_split "aarch64_atomic_exchange<mode>"
> (clobber (match_scratch:SI 4 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (SET, operands[0], NULL, operands[1],
> @@ -344,7 +344,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab><mode>"
> (clobber (match_scratch:SI 4 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (<CODE>, NULL, operands[3], operands[0],
> @@ -400,7 +400,7 @@ (define_insn_and_split "atomic_nand<mode>"
> (clobber (match_scratch:SI 4 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (NOT, NULL, operands[3], operands[0],
> @@ -504,7 +504,7 @@ (define_insn_and_split "aarch64_atomic_fetch_<atomic_optab><mode>"
> (clobber (match_scratch:SI 5 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (<CODE>, operands[0], operands[4], operands[1],
> @@ -551,7 +551,7 @@ (define_insn_and_split "atomic_fetch_nand<mode>"
> (clobber (match_scratch:SI 5 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (NOT, operands[0], operands[4], operands[1],
> @@ -604,7 +604,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab>_fetch<mode>"
> (clobber (match_scratch:SI 4 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (<CODE>, NULL, operands[0], operands[1],
> @@ -628,7 +628,7 @@ (define_insn_and_split "atomic_nand_fetch<mode>"
> (clobber (match_scratch:SI 4 "=&r"))]
> ""
> "#"
> - "&& reload_completed"
> + "&& epilogue_completed"
> [(const_int 0)]
> {
> aarch64_split_atomic_op (NOT, NULL, operands[0], operands[1],
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-16 13:47 ` Richard Sandiford
@ 2020-01-17 2:37 ` Andrew Pinski
2020-01-17 3:16 ` Andrew Pinski
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Pinski @ 2020-01-17 2:37 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches, Andrew Pinski, Kyrylo Tkachov,
Richard Earnshaw, richard.sandiford
On Thu, Jan 16, 2020 at 5:14 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> > The separate shrinkwrapping pass may insert stores in the middle
> > of atomics loops which can cause issues on some implementations.
> > Avoid this by delaying splitting of atomic patterns until after
> > prolog/epilog generation.
> >
> > Bootstrap completed, no test regressions on AArch64.
> >
> > Andrew, can you verify this fixes the failure you were getting?
> >
> > ChangeLog:
> > 2020-01-16 Wilco Dijkstra <wdijkstr@arm.com>
> >
> > PR target/92692
> > * config/aarch64/aarch64.c (aarch64_split_compare_and_swap)
> > Add assert to ensure prolog has been emitted.
> > (aarch64_split_atomic_op): Likewise.
> > * config/aarch64/atomics.md (aarch64_compare_and_swap<mode>)
> > Use epilogue_completed rather than reload_completed.
> > (aarch64_atomic_exchange<mode>): Likewise.
> > (aarch64_atomic_<atomic_optab><mode>): Likewise.
> > (atomic_nand<mode>): Likewise.
> > (aarch64_atomic_fetch_<atomic_optab><mode>): Likewise.
> > (atomic_fetch_nand<mode>): Likewise.
> > (aarch64_atomic_<atomic_optab>_fetch<mode>): Likewise.
> > (atomic_nand_fetch<mode>): Likewise.
>
> OK if Andrew confirms it works, thanks.
Yes this fixes the issue for me.
Thanks,
Andrew
>
> Richard
>
> > ---
> >
> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > index ac89cc1f9c938455d33d8850d9ebfc0473cb73dc..cd9d813f2ac4990971f6435fdb28b0f94ae10309 100644
> > --- a/gcc/config/aarch64/aarch64.c
> > +++ b/gcc/config/aarch64/aarch64.c
> > @@ -18375,6 +18375,9 @@ aarch64_emit_post_barrier (enum memmodel model)
> > void
> > aarch64_split_compare_and_swap (rtx operands[])
> > {
> > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> > + gcc_assert (epilogue_completed);
> > +
> > rtx rval, mem, oldval, newval, scratch, x, model_rtx;
> > machine_mode mode;
> > bool is_weak;
> > @@ -18469,6 +18472,9 @@ void
> > aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
> > rtx value, rtx model_rtx, rtx cond)
> > {
> > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> > + gcc_assert (epilogue_completed);
> > +
> > machine_mode mode = GET_MODE (mem);
> > machine_mode wmode = (mode == DImode ? DImode : SImode);
> > const enum memmodel model = memmodel_from_int (INTVAL (model_rtx));
> > diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> > index c2bcabd0c3c2627b7222dcbc1af9c2e6b7ce6a76..996947799b5ef8445e9786b94e1ce62fd16e5b5c 100644
> > --- a/gcc/config/aarch64/atomics.md
> > +++ b/gcc/config/aarch64/atomics.md
> > @@ -56,7 +56,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > (clobber (match_scratch:SI 7 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_compare_and_swap (operands);
> > @@ -80,7 +80,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > (clobber (match_scratch:SI 7 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_compare_and_swap (operands);
> > @@ -104,7 +104,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > (clobber (match_scratch:SI 7 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_compare_and_swap (operands);
> > @@ -223,7 +223,7 @@ (define_insn_and_split "aarch64_atomic_exchange<mode>"
> > (clobber (match_scratch:SI 4 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (SET, operands[0], NULL, operands[1],
> > @@ -344,7 +344,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab><mode>"
> > (clobber (match_scratch:SI 4 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (<CODE>, NULL, operands[3], operands[0],
> > @@ -400,7 +400,7 @@ (define_insn_and_split "atomic_nand<mode>"
> > (clobber (match_scratch:SI 4 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (NOT, NULL, operands[3], operands[0],
> > @@ -504,7 +504,7 @@ (define_insn_and_split "aarch64_atomic_fetch_<atomic_optab><mode>"
> > (clobber (match_scratch:SI 5 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (<CODE>, operands[0], operands[4], operands[1],
> > @@ -551,7 +551,7 @@ (define_insn_and_split "atomic_fetch_nand<mode>"
> > (clobber (match_scratch:SI 5 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (NOT, operands[0], operands[4], operands[1],
> > @@ -604,7 +604,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab>_fetch<mode>"
> > (clobber (match_scratch:SI 4 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (<CODE>, NULL, operands[0], operands[1],
> > @@ -628,7 +628,7 @@ (define_insn_and_split "atomic_nand_fetch<mode>"
> > (clobber (match_scratch:SI 4 "=&r"))]
> > ""
> > "#"
> > - "&& reload_completed"
> > + "&& epilogue_completed"
> > [(const_int 0)]
> > {
> > aarch64_split_atomic_op (NOT, NULL, operands[0], operands[1],
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-17 2:37 ` Andrew Pinski
@ 2020-01-17 3:16 ` Andrew Pinski
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Pinski @ 2020-01-17 3:16 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches, Andrew Pinski, Kyrylo Tkachov,
Richard Earnshaw, richard.sandiford
On Thu, Jan 16, 2020 at 5:51 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Thu, Jan 16, 2020 at 5:14 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
> >
> > Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> > > The separate shrinkwrapping pass may insert stores in the middle
> > > of atomics loops which can cause issues on some implementations.
> > > Avoid this by delaying splitting of atomic patterns until after
> > > prolog/epilog generation.
> > >
> > > Bootstrap completed, no test regressions on AArch64.
> > >
> > > Andrew, can you verify this fixes the failure you were getting?
> > >
> > > ChangeLog:
> > > 2020-01-16 Wilco Dijkstra <wdijkstr@arm.com>
> > >
> > > PR target/92692
> > > * config/aarch64/aarch64.c (aarch64_split_compare_and_swap)
> > > Add assert to ensure prolog has been emitted.
> > > (aarch64_split_atomic_op): Likewise.
> > > * config/aarch64/atomics.md (aarch64_compare_and_swap<mode>)
> > > Use epilogue_completed rather than reload_completed.
> > > (aarch64_atomic_exchange<mode>): Likewise.
> > > (aarch64_atomic_<atomic_optab><mode>): Likewise.
> > > (atomic_nand<mode>): Likewise.
> > > (aarch64_atomic_fetch_<atomic_optab><mode>): Likewise.
> > > (atomic_fetch_nand<mode>): Likewise.
> > > (aarch64_atomic_<atomic_optab>_fetch<mode>): Likewise.
> > > (atomic_nand_fetch<mode>): Likewise.
> >
> > OK if Andrew confirms it works, thanks.
>
> Yes this fixes the issue for me.
Here is the new assembly showing it worked:
d390: f9000bf3 str x19, [sp, #16]
d394: 885ffdc8 ldaxr w8, [x14]
d398: 6b01011f cmp w8, w1
d39c: 54000061 b.ne d3a8
<pthread_rwlock_clockwrlock+0x168> // b.any
d3a0: 88137dc5 stxr w19, w5, [x14]
Thanks,
Andrew Pinski
>
> Thanks,
> Andrew
>
> >
> > Richard
> >
> > > ---
> > >
> > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > > index ac89cc1f9c938455d33d8850d9ebfc0473cb73dc..cd9d813f2ac4990971f6435fdb28b0f94ae10309 100644
> > > --- a/gcc/config/aarch64/aarch64.c
> > > +++ b/gcc/config/aarch64/aarch64.c
> > > @@ -18375,6 +18375,9 @@ aarch64_emit_post_barrier (enum memmodel model)
> > > void
> > > aarch64_split_compare_and_swap (rtx operands[])
> > > {
> > > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> > > + gcc_assert (epilogue_completed);
> > > +
> > > rtx rval, mem, oldval, newval, scratch, x, model_rtx;
> > > machine_mode mode;
> > > bool is_weak;
> > > @@ -18469,6 +18472,9 @@ void
> > > aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
> > > rtx value, rtx model_rtx, rtx cond)
> > > {
> > > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */
> > > + gcc_assert (epilogue_completed);
> > > +
> > > machine_mode mode = GET_MODE (mem);
> > > machine_mode wmode = (mode == DImode ? DImode : SImode);
> > > const enum memmodel model = memmodel_from_int (INTVAL (model_rtx));
> > > diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> > > index c2bcabd0c3c2627b7222dcbc1af9c2e6b7ce6a76..996947799b5ef8445e9786b94e1ce62fd16e5b5c 100644
> > > --- a/gcc/config/aarch64/atomics.md
> > > +++ b/gcc/config/aarch64/atomics.md
> > > @@ -56,7 +56,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > > (clobber (match_scratch:SI 7 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_compare_and_swap (operands);
> > > @@ -80,7 +80,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > > (clobber (match_scratch:SI 7 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_compare_and_swap (operands);
> > > @@ -104,7 +104,7 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
> > > (clobber (match_scratch:SI 7 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_compare_and_swap (operands);
> > > @@ -223,7 +223,7 @@ (define_insn_and_split "aarch64_atomic_exchange<mode>"
> > > (clobber (match_scratch:SI 4 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (SET, operands[0], NULL, operands[1],
> > > @@ -344,7 +344,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab><mode>"
> > > (clobber (match_scratch:SI 4 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (<CODE>, NULL, operands[3], operands[0],
> > > @@ -400,7 +400,7 @@ (define_insn_and_split "atomic_nand<mode>"
> > > (clobber (match_scratch:SI 4 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (NOT, NULL, operands[3], operands[0],
> > > @@ -504,7 +504,7 @@ (define_insn_and_split "aarch64_atomic_fetch_<atomic_optab><mode>"
> > > (clobber (match_scratch:SI 5 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (<CODE>, operands[0], operands[4], operands[1],
> > > @@ -551,7 +551,7 @@ (define_insn_and_split "atomic_fetch_nand<mode>"
> > > (clobber (match_scratch:SI 5 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (NOT, operands[0], operands[4], operands[1],
> > > @@ -604,7 +604,7 @@ (define_insn_and_split "aarch64_atomic_<atomic_optab>_fetch<mode>"
> > > (clobber (match_scratch:SI 4 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (<CODE>, NULL, operands[0], operands[1],
> > > @@ -628,7 +628,7 @@ (define_insn_and_split "atomic_nand_fetch<mode>"
> > > (clobber (match_scratch:SI 4 "=&r"))]
> > > ""
> > > "#"
> > > - "&& reload_completed"
> > > + "&& epilogue_completed"
> > > [(const_int 0)]
> > > {
> > > aarch64_split_atomic_op (NOT, NULL, operands[0], operands[1],
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-16 13:32 [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692) Wilco Dijkstra
2020-01-16 13:47 ` Richard Sandiford
@ 2020-01-27 16:41 ` Segher Boessenkool
2020-01-27 16:57 ` Wilco Dijkstra
1 sibling, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2020-01-27 16:41 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Andrew Pinski, Kyrylo Tkachov, Richard Earnshaw,
Richard Sandiford
Hi!
On Thu, Jan 16, 2020 at 12:50:14PM +0000, Wilco Dijkstra wrote:
> The separate shrinkwrapping pass may insert stores in the middle
> of atomics loops which can cause issues on some implementations.
> Avoid this by delaying splitting of atomic patterns until after
> prolog/epilog generation.
Note that this isn't specific to sws at all: there isn't anything
stopping later passes from doing this either. Is there anything that
protects us from sched2 doing similar here, for example?
Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-27 16:41 ` Segher Boessenkool
@ 2020-01-27 16:57 ` Wilco Dijkstra
2020-01-28 13:21 ` Segher Boessenkool
0 siblings, 1 reply; 7+ messages in thread
From: Wilco Dijkstra @ 2020-01-27 16:57 UTC (permalink / raw)
To: Segher Boessenkool
Cc: GCC Patches, Andrew Pinski, Kyrylo Tkachov, Richard Earnshaw,
Richard Sandiford
Hi Segher,
> On Thu, Jan 16, 2020 at 12:50:14PM +0000, Wilco Dijkstra wrote:
>> The separate shrinkwrapping pass may insert stores in the middle
>> of atomics loops which can cause issues on some implementations.
>> Avoid this by delaying splitting of atomic patterns until after
>> prolog/epilog generation.
>
> Note that this isn't specific to sws at all: there isn't anything
> stopping later passes from doing this either. Is there anything that
> protects us from sched2 doing similar here, for example?
The expansions create extra basic blocks and insert various barriers
that would stop any reasonable scheduler from doing it. And the
current scheduler is basic block based.
Wilco
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692)
2020-01-27 16:57 ` Wilco Dijkstra
@ 2020-01-28 13:21 ` Segher Boessenkool
0 siblings, 0 replies; 7+ messages in thread
From: Segher Boessenkool @ 2020-01-28 13:21 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Andrew Pinski, Kyrylo Tkachov, Richard Earnshaw,
Richard Sandiford
Hi!
On Mon, Jan 27, 2020 at 04:28:29PM +0000, Wilco Dijkstra wrote:
> > On Thu, Jan 16, 2020 at 12:50:14PM +0000, Wilco Dijkstra wrote:
> >> The separate shrinkwrapping pass may insert stores in the middle
> >> of atomics loops which can cause issues on some implementations.
> >> Avoid this by delaying splitting of atomic patterns until after
> >> prolog/epilog generation.
> >
> > Note that this isn't specific to sws at all: there isn't anything
> > stopping later passes from doing this either. Is there anything that
> > protects us from sched2 doing similar here, for example?
>
> The expansions create extra basic blocks and insert various barriers
> that would stop any reasonable scheduler from doing it. And the
> current scheduler is basic block based.
Hrm, and probably no other passes after *logue can create new store
insns (ignoring peepholes and splitters, that would be almost *asking*
for trouble ;-) ). I see. Thanks for explaining.
Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-28 12:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 13:32 [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692) Wilco Dijkstra
2020-01-16 13:47 ` Richard Sandiford
2020-01-17 2:37 ` Andrew Pinski
2020-01-17 3:16 ` Andrew Pinski
2020-01-27 16:41 ` Segher Boessenkool
2020-01-27 16:57 ` Wilco Dijkstra
2020-01-28 13:21 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).