| Message ID | orbkau6bvu.fsf_-_@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [#2a/2] | expand |
On Wed, Dec 13, 2023 at 4:05 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Dec 12, 2023, Richard Biener <richard.guenther@gmail.com> wrote: > > > On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva <oliva@adacore.com> wrote: > > >> DECL_NOT_GIMPLE_REG_P (arg) = 0; > > > I wonder why you clear this at all? > > That code seems to be inherited from expand_thunk. > ISTR that flag was not negated when I started the strub implementation, > back in gcc-10. > > >> + convert in separate statements. ??? Should > >> + we drop volatile from the wrapper > >> + instead? */ > > > volatile on function parameters are indeed odd beasts. You could > > also force volatile arguments to be passed indirectly. > > Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of > #1/2, now a cleanup that IMHO would still be desirable. > > > strub: indirect volatile parms in wrappers > > Arrange for strub internal wrappers to pass volatile arguments by > reference to the wrapped bodies. OK. > > for gcc/ChangeLog > > PR middle-end/112938 > * ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args > by reference to internal strub wrapped bodies. > > for gcc/testsuite/ChangeLog > > PR middle-end/112938 > * gcc.dg/strub-internal-volatile.c: Check indirection of > volatile args. > --- > gcc/ipa-strub.cc | 19 +++++++++---------- > gcc/testsuite/gcc.dg/strub-internal-volatile.c | 5 +++++ > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc > index 45294b0b46bcb..943bb60996fc1 100644 > --- a/gcc/ipa-strub.cc > +++ b/gcc/ipa-strub.cc > @@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *) > parm = DECL_CHAIN (parm), > nparm = DECL_CHAIN (nparm), > nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE) > - if (!(0 /* DECL_BY_REFERENCE (narg) */ > - || is_gimple_reg_type (TREE_TYPE (nparm)) > - || VECTOR_TYPE_P (TREE_TYPE (nparm)) > - || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE > - || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) > - && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) > - <= 4 * UNITS_PER_WORD)))) > + if (TREE_THIS_VOLATILE (parm) > + || !(0 /* DECL_BY_REFERENCE (narg) */ > + || is_gimple_reg_type (TREE_TYPE (nparm)) > + || VECTOR_TYPE_P (TREE_TYPE (nparm)) > + || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE > + || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) > + && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) > + <= 4 * UNITS_PER_WORD)))) > { > /* No point in indirecting pointer types. Presumably they > won't ever pass the size-based test above, but check the > @@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *) > { > tree tmp = arg; > /* If ARG is e.g. volatile, we must copy and > - convert in separate statements. ??? Should > - we drop volatile from the wrapper > - instead? */ > + convert in separate statements. */ > if (!is_gimple_val (arg)) > { > tmp = create_tmp_reg (TYPE_MAIN_VARIANT > diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c > index cdfca67616bc8..227406af245cc 100644 > --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c > +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c > @@ -1,4 +1,5 @@ > /* { dg-do compile } */ > +/* { dg-options "-fdump-ipa-strub" } */ > /* { dg-require-effective-target strub } */ > > void __attribute__ ((strub("internal"))) > @@ -8,3 +9,7 @@ f(volatile short) { > void g(void) { > f(0); > } > + > +/* We make volatile parms indirect in the wrapped f. */ > +/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */ > +/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */ > > > -- > 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
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 45294b0b46bcb..943bb60996fc1 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *) parm = DECL_CHAIN (parm), nparm = DECL_CHAIN (nparm), nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE) - if (!(0 /* DECL_BY_REFERENCE (narg) */ - || is_gimple_reg_type (TREE_TYPE (nparm)) - || VECTOR_TYPE_P (TREE_TYPE (nparm)) - || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE - || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - <= 4 * UNITS_PER_WORD)))) + if (TREE_THIS_VOLATILE (parm) + || !(0 /* DECL_BY_REFERENCE (narg) */ + || is_gimple_reg_type (TREE_TYPE (nparm)) + || VECTOR_TYPE_P (TREE_TYPE (nparm)) + || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE + || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + <= 4 * UNITS_PER_WORD)))) { /* No point in indirecting pointer types. Presumably they won't ever pass the size-based test above, but check the @@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *) { tree tmp = arg; /* If ARG is e.g. volatile, we must copy and - convert in separate statements. ??? Should - we drop volatile from the wrapper - instead? */ + convert in separate statements. */ if (!is_gimple_val (arg)) { tmp = create_tmp_reg (TYPE_MAIN_VARIANT diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c index cdfca67616bc8..227406af245cc 100644 --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-options "-fdump-ipa-strub" } */ /* { dg-require-effective-target strub } */ void __attribute__ ((strub("internal"))) @@ -8,3 +9,7 @@ f(volatile short) { void g(void) { f(0); } + +/* We make volatile parms indirect in the wrapped f. */ +/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */ +/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */