On Dec 11, 2023, Alexandre Oliva <ol...@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
