| Message ID | ory1arg49o.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [strub] improve handling of indirected volatile parms [PR112938] | expand |
On Sat, Mar 9, 2024 at 10:10 AM Alexandre Oliva <oliva@adacore.com> wrote: > > > The earlier patch for PR112938 arranged for volatile parms to be made > indirect in internal strub wrapped bodies. > > The first problem that remained, more evident, was that the indirected > parameter remained volatile, despite the indirection, but it wasn't > regimplified, so indirecting it was malformed gimple. > > Regimplifying turned out not to be needed. The best course of action > was to drop the volatility from the by-reference parm, that was being > unexpectedly inherited from the original volatile parm. > > That exposed another problem: the dereferences would then lose their > volatile status, so we had to bring volatile back to them. > > Regstrapped on x86_64-linux-gnu. Ok to install? OK > > for gcc/ChangeLog > > PR middle-end/112938 > * ipa-strub.cc (pass_ipa_strub::execute): Drop volatility from > indirected parm. > (maybe_make_indirect): Restore volatility in dereferences. > > for gcc/testsuite/ChangeLog > > PR middle-end/112938 > * g++.dg/strub-internal-pr112938.cc: New. > --- > gcc/ipa-strub.cc | 7 +++++++ > gcc/testsuite/g++.dg/strub-internal-pr112938.cc | 12 ++++++++++++ > 2 files changed, 19 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/strub-internal-pr112938.cc > > diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc > index dff94222351ad..8fa7bdf530023 100644 > --- a/gcc/ipa-strub.cc > +++ b/gcc/ipa-strub.cc > @@ -1940,6 +1940,9 @@ maybe_make_indirect (indirect_parms_t &indirect_parms, tree op, int *rec) > TREE_TYPE (TREE_TYPE (op)), > op, > build_int_cst (TREE_TYPE (op), 0)); > + if (TYPE_VOLATILE (TREE_TYPE (TREE_TYPE (op))) > + && !TREE_THIS_VOLATILE (ret)) > + TREE_SIDE_EFFECTS (ret) = TREE_THIS_VOLATILE (ret) = 1; > return ret; > } > } > @@ -2894,6 +2897,10 @@ pass_ipa_strub::execute (function *) > probably drop the TREE_ADDRESSABLE and keep the TRUE. */ > tree ref_type = build_ref_type_for (nparm); > > + if (TREE_THIS_VOLATILE (nparm) > + && TYPE_VOLATILE (TREE_TYPE (nparm)) > + && !TYPE_VOLATILE (ref_type)) > + TREE_SIDE_EFFECTS (nparm) = TREE_THIS_VOLATILE (nparm) = 0; > DECL_ARG_TYPE (nparm) = TREE_TYPE (nparm) = ref_type; > relayout_decl (nparm); > TREE_ADDRESSABLE (nparm) = 0; > diff --git a/gcc/testsuite/g++.dg/strub-internal-pr112938.cc b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc > new file mode 100644 > index 0000000000000..5a74becc2697e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fdump-tree-optimized -O2" } */ > +/* { dg-require-effective-target strub } */ > + > +bool __attribute__ ((__strub__ ("internal"))) > +f(bool i, volatile bool j) > +{ > + return (i ^ j) == j; > +} > + > +/* Check for two dereferences of the indirected volatile j parm. */ > +/* { dg-final { scan-tree-dump-times {={v} \*j_[0-9][0-9]*(D)} 2 "optimized" } } */ > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity > Excluding neuro-others for not behaving ""normal"" is *not* inclusive
On Mar 11, 2024, Richard Biener <richard.guenther@gmail.com> wrote: > On Sat, Mar 9, 2024 at 10:10 AM Alexandre Oliva <oliva@adacore.com> wrote: >> >> >> The earlier patch for PR112938 arranged for volatile parms to be made >> indirect in internal strub wrapped bodies. >> >> The first problem that remained, more evident, was that the indirected >> parameter remained volatile, despite the indirection, but it wasn't >> regimplified, so indirecting it was malformed gimple. >> >> Regimplifying turned out not to be needed. The best course of action >> was to drop the volatility from the by-reference parm, that was being >> unexpectedly inherited from the original volatile parm. >> >> That exposed another problem: the dereferences would then lose their >> volatile status, so we had to bring volatile back to them. >> >> Regstrapped on x86_64-linux-gnu. Ok to install? > OK Thanks, sorry, I dropped the ball on this one. I'm going to put it in momentarily. It had been approved for gcc 14, before it branched off; should I install it there as well? >> >> for gcc/ChangeLog >> >> PR middle-end/112938 >> * ipa-strub.cc (pass_ipa_strub::execute): Drop volatility from >> indirected parm. >> (maybe_make_indirect): Restore volatility in dereferences. >> >> for gcc/testsuite/ChangeLog >> >> PR middle-end/112938 >> * g++.dg/strub-internal-pr112938.cc: New.
On Apr 16, 2024, Alexandre Oliva <oliva@adacore.com> wrote: > I'm going to put it in momentarily. It had been approved for gcc 14, > before it branched off; should I install it there as well? Ermh, nevermind, I'm not sure how I got the idea that we'd already branched, but I was absolutely sure that this was the case. Doh! :-)
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index dff94222351ad..8fa7bdf530023 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -1940,6 +1940,9 @@ maybe_make_indirect (indirect_parms_t &indirect_parms, tree op, int *rec) TREE_TYPE (TREE_TYPE (op)), op, build_int_cst (TREE_TYPE (op), 0)); + if (TYPE_VOLATILE (TREE_TYPE (TREE_TYPE (op))) + && !TREE_THIS_VOLATILE (ret)) + TREE_SIDE_EFFECTS (ret) = TREE_THIS_VOLATILE (ret) = 1; return ret; } } @@ -2894,6 +2897,10 @@ pass_ipa_strub::execute (function *) probably drop the TREE_ADDRESSABLE and keep the TRUE. */ tree ref_type = build_ref_type_for (nparm); + if (TREE_THIS_VOLATILE (nparm) + && TYPE_VOLATILE (TREE_TYPE (nparm)) + && !TYPE_VOLATILE (ref_type)) + TREE_SIDE_EFFECTS (nparm) = TREE_THIS_VOLATILE (nparm) = 0; DECL_ARG_TYPE (nparm) = TREE_TYPE (nparm) = ref_type; relayout_decl (nparm); TREE_ADDRESSABLE (nparm) = 0; diff --git a/gcc/testsuite/g++.dg/strub-internal-pr112938.cc b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc new file mode 100644 index 0000000000000..5a74becc2697e --- /dev/null +++ b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-fdump-tree-optimized -O2" } */ +/* { dg-require-effective-target strub } */ + +bool __attribute__ ((__strub__ ("internal"))) +f(bool i, volatile bool j) +{ + return (i ^ j) == j; +} + +/* Check for two dereferences of the indirected volatile j parm. */ +/* { dg-final { scan-tree-dump-times {={v} \*j_[0-9][0-9]*(D)} 2 "optimized" } } */