* [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938]
@ 2023-12-12 2:02 Alexandre Oliva
2023-12-12 2:48 ` [PATCH #2/2] strub: drop volatile from wrapper args [PR112938] Alexandre Oliva
2023-12-12 11:46 ` [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Richard Biener
0 siblings, 2 replies; 6+ messages in thread
From: Alexandre Oliva @ 2023-12-12 2:02 UTC (permalink / raw)
To: gcc-patches
When generating code for an internal strub wrapper, don't clear the
DECL_NOT_GIMPLE_REG_P flag of volatile args, and gimplify them both
before and after any conversion.
While at that, move variable TMP into narrower scopes so that it's
more trivial to track where ARG lives.
Regstrapped on x86_64-linux-gnu. Ok to install?
(there's a #2/2 followup coming up that addresses the ??? comment added
herein)
for gcc/ChangeLog
PR middle-end/112938
* ipa-strub.cc (pass_ipa_strub::execute): Handle promoted
volatile args in internal strub. Simplify.
for gcc/testsuite/ChangeLog
PR middle-end/112938
* gcc.dg/strub-internal-volatile.c: New.
---
gcc/ipa-strub.cc | 29 +++++++++++++++++-------
gcc/testsuite/gcc.dg/strub-internal-volatile.c | 10 ++++++++
2 files changed, 31 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/strub-internal-volatile.c
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
index 8ec6824e8a802..45294b0b46bcb 100644
--- a/gcc/ipa-strub.cc
+++ b/gcc/ipa-strub.cc
@@ -3203,7 +3203,6 @@ pass_ipa_strub::execute (function *)
i++, arg = DECL_CHAIN (arg), nparm = DECL_CHAIN (nparm))
{
tree save_arg = arg;
- tree tmp = arg;
/* Arrange to pass indirectly the parms, if we decided to do
so, and revert its type in the wrapper. */
@@ -3211,10 +3210,9 @@ pass_ipa_strub::execute (function *)
{
tree ref_type = TREE_TYPE (nparm);
TREE_ADDRESSABLE (arg) = true;
- tree addr = build1 (ADDR_EXPR, ref_type, arg);
- tmp = arg = addr;
+ arg = build1 (ADDR_EXPR, ref_type, arg);
}
- else
+ else if (!TREE_THIS_VOLATILE (arg))
DECL_NOT_GIMPLE_REG_P (arg) = 0;
/* Convert the argument back to the type used by the calling
@@ -3223,16 +3221,31 @@ pass_ipa_strub::execute (function *)
double to be passed on unchanged to the wrapped
function. */
if (TREE_TYPE (nparm) != DECL_ARG_TYPE (nparm))
- arg = fold_convert (DECL_ARG_TYPE (nparm), arg);
+ {
+ 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? */
+ if (!is_gimple_val (arg))
+ {
+ tmp = create_tmp_reg (TYPE_MAIN_VARIANT
+ (TREE_TYPE (arg)), "arg");
+ gimple *stmt = gimple_build_assign (tmp, arg);
+ gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
+ }
+ arg = fold_convert (DECL_ARG_TYPE (nparm), tmp);
+ }
if (!is_gimple_val (arg))
{
- tmp = create_tmp_reg (TYPE_MAIN_VARIANT
- (TREE_TYPE (arg)), "arg");
+ tree tmp = create_tmp_reg (TYPE_MAIN_VARIANT
+ (TREE_TYPE (arg)), "arg");
gimple *stmt = gimple_build_assign (tmp, arg);
gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
+ arg = tmp;
}
- vargs.quick_push (tmp);
+ vargs.quick_push (arg);
arg = save_arg;
}
/* These strub arguments are adjusted later. */
diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
new file mode 100644
index 0000000000000..cdfca67616bc8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target strub } */
+
+void __attribute__ ((strub("internal")))
+f(volatile short) {
+}
+
+void g(void) {
+ f(0);
+}
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH #2/2] strub: drop volatile from wrapper args [PR112938]
2023-12-12 2:02 [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Alexandre Oliva
@ 2023-12-12 2:48 ` Alexandre Oliva
2023-12-12 11:46 ` [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Alexandre Oliva @ 2023-12-12 2:48 UTC (permalink / raw)
To: gcc-patches
On Dec 11, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
> (there's a #2/2 followup coming up that addresses the ??? comment added
> herein)
Here it is. Also regstrapped on x86_64-linux-gnu, along with the
previous patch (that had also been regstrapped by itself). I think this
would be a desirable thing to do (maybe also with TYPE_QUAL_ATOMIC), but
I'm a little worried about modifying the types of args of the original
function decl, the one that is about to become a wrapper. This would be
visible at least in debug information. OTOH, keeping the volatile in
the wrapper would serve no useful purpose whatsoever, it would likely
just make it slower, and such top-level qualifiers really only apply
within the function body, which the wrapper isn't. Thoughts? Ok to
install?
Drop volatile from argument types in internal strub wrappers that are
not made indirect. Their volatility is only relevant within the body
of the function, and that body is moved to the wrapped function.
for gcc/ChangeLog
PR middle-end/112938
* ipa-strub.cc (pass_ipa_strub::execute): Drop volatile from
internal strub wrapper args.
for gcc/testsuite/ChangeLog
PR middle-end/112938
* gcc.dg/strub-internal-volatile.c: Check for dropped volatile
in wrapper.
---
gcc/ipa-strub.cc | 14 +++++++++++---
gcc/testsuite/gcc.dg/strub-internal-volatile.c | 5 +++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
index 45294b0b46bcb..bab20c386bb01 100644
--- a/gcc/ipa-strub.cc
+++ b/gcc/ipa-strub.cc
@@ -2922,6 +2922,16 @@ pass_ipa_strub::execute (function *)
if (nparmt)
adjust_ftype++;
}
+ else if (TREE_THIS_VOLATILE (parm))
+ {
+ /* Drop volatile from wrapper's arguments, they're just
+ temporaries copied to the wrapped function. ??? Should
+ we drop TYPE_QUAL_ATOMIC as well? */
+ TREE_TYPE (parm) = build_qualified_type (TREE_TYPE (parm),
+ TYPE_QUALS (TREE_TYPE (parm))
+ & ~TYPE_QUAL_VOLATILE);
+ TREE_THIS_VOLATILE (parm) = 0;
+ }
/* Also adjust the wrapped function type, if needed. */
if (adjust_ftype)
@@ -3224,9 +3234,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..0ffa98d799d32 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 drop volatile from the wrapper, and keep it in the wrapped f, so
+ the count remains 1. */
+/* { dg-final { scan-ipa-dump-times "volatile" 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938]
2023-12-12 2:02 [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Alexandre Oliva
2023-12-12 2:48 ` [PATCH #2/2] strub: drop volatile from wrapper args [PR112938] Alexandre Oliva
@ 2023-12-12 11:46 ` Richard Biener
2023-12-13 3:04 ` [PATCH #2a/2] Alexandre Oliva
2023-12-13 3:06 ` [PATCH #2a/2] strub: indirect volatile parms in wrappers Alexandre Oliva
1 sibling, 2 replies; 6+ messages in thread
From: Richard Biener @ 2023-12-12 11:46 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> When generating code for an internal strub wrapper, don't clear the
> DECL_NOT_GIMPLE_REG_P flag of volatile args, and gimplify them both
> before and after any conversion.
>
> While at that, move variable TMP into narrower scopes so that it's
> more trivial to track where ARG lives.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
>
> (there's a #2/2 followup coming up that addresses the ??? comment added
> herein)
>
>
> for gcc/ChangeLog
>
> PR middle-end/112938
> * ipa-strub.cc (pass_ipa_strub::execute): Handle promoted
> volatile args in internal strub. Simplify.
>
> for gcc/testsuite/ChangeLog
>
> PR middle-end/112938
> * gcc.dg/strub-internal-volatile.c: New.
> ---
> gcc/ipa-strub.cc | 29 +++++++++++++++++-------
> gcc/testsuite/gcc.dg/strub-internal-volatile.c | 10 ++++++++
> 2 files changed, 31 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/strub-internal-volatile.c
>
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 8ec6824e8a802..45294b0b46bcb 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -3203,7 +3203,6 @@ pass_ipa_strub::execute (function *)
> i++, arg = DECL_CHAIN (arg), nparm = DECL_CHAIN (nparm))
> {
> tree save_arg = arg;
> - tree tmp = arg;
>
> /* Arrange to pass indirectly the parms, if we decided to do
> so, and revert its type in the wrapper. */
> @@ -3211,10 +3210,9 @@ pass_ipa_strub::execute (function *)
> {
> tree ref_type = TREE_TYPE (nparm);
> TREE_ADDRESSABLE (arg) = true;
> - tree addr = build1 (ADDR_EXPR, ref_type, arg);
> - tmp = arg = addr;
> + arg = build1 (ADDR_EXPR, ref_type, arg);
> }
> - else
> + else if (!TREE_THIS_VOLATILE (arg))
> DECL_NOT_GIMPLE_REG_P (arg) = 0;
I wonder why you clear this at all? The next update_address_taken
will take care of this if possible.
>
> /* Convert the argument back to the type used by the calling
> @@ -3223,16 +3221,31 @@ pass_ipa_strub::execute (function *)
> double to be passed on unchanged to the wrapped
> function. */
> if (TREE_TYPE (nparm) != DECL_ARG_TYPE (nparm))
> - arg = fold_convert (DECL_ARG_TYPE (nparm), arg);
> + {
> + 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? */
volatile on function parameters are indeed odd beasts. You could
also force volatile arguments to be passed indirectly. I think for
GIMPLE thunks we do it as you now do here.
> + if (!is_gimple_val (arg))
> + {
> + tmp = create_tmp_reg (TYPE_MAIN_VARIANT
> + (TREE_TYPE (arg)), "arg");
> + gimple *stmt = gimple_build_assign (tmp, arg);
> + gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
> + }
> + arg = fold_convert (DECL_ARG_TYPE (nparm), tmp);
> + }
>
> if (!is_gimple_val (arg))
> {
> - tmp = create_tmp_reg (TYPE_MAIN_VARIANT
> - (TREE_TYPE (arg)), "arg");
> + tree tmp = create_tmp_reg (TYPE_MAIN_VARIANT
> + (TREE_TYPE (arg)), "arg");
> gimple *stmt = gimple_build_assign (tmp, arg);
> gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
> + arg = tmp;
> }
> - vargs.quick_push (tmp);
> + vargs.quick_push (arg);
> arg = save_arg;
> }
> /* These strub arguments are adjusted later. */
> diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> new file mode 100644
> index 0000000000000..cdfca67616bc8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target strub } */
> +
> +void __attribute__ ((strub("internal")))
> +f(volatile short) {
> +}
> +
> +void g(void) {
> + f(0);
> +}
>
> --
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH #2a/2]
2023-12-12 11:46 ` [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Richard Biener
@ 2023-12-13 3:04 ` Alexandre Oliva
2023-12-13 8:52 ` Richard Biener
2023-12-13 3:06 ` [PATCH #2a/2] strub: indirect volatile parms in wrappers Alexandre Oliva
1 sibling, 1 reply; 6+ messages in thread
From: Alexandre Oliva @ 2023-12-13 3:04 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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(-)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH #2a/2] strub: indirect volatile parms in wrappers
2023-12-12 11:46 ` [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Richard Biener
2023-12-13 3:04 ` [PATCH #2a/2] Alexandre Oliva
@ 2023-12-13 3:06 ` Alexandre Oliva
1 sibling, 0 replies; 6+ messages in thread
From: Alexandre Oliva @ 2023-12-13 3:06 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[sorry that the previous, unfinished post got through]
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.
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(-)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH #2a/2]
2023-12-13 3:04 ` [PATCH #2a/2] Alexandre Oliva
@ 2023-12-13 8:52 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2023-12-13 8:52 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-13 8:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-12 2:02 [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Alexandre Oliva
2023-12-12 2:48 ` [PATCH #2/2] strub: drop volatile from wrapper args [PR112938] Alexandre Oliva
2023-12-12 11:46 ` [PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938] Richard Biener
2023-12-13 3:04 ` [PATCH #2a/2] Alexandre Oliva
2023-12-13 8:52 ` Richard Biener
2023-12-13 3:06 ` [PATCH #2a/2] strub: indirect volatile parms in wrappers Alexandre Oliva
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).