Checks
Commit Message
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.
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(-)
Comments
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
@@ -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
@@ -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" } } */