[PATCH #2a/2] strub: indirect volatile parms in wrappers

[Alexandre Oliva <oliva@adacore.com>]

[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