diff mbox series

[#2/2] strub: drop volatile from wrapper args [PR112938]

Message ID orh6ko6sr3.fsf@lxoliva.fsfla.org
State New
Headers show
Series [#1/2] strub: handle volatile promoted args in internal strub [PR112938] | expand

Commit Message

Alexandre Oliva Dec. 12, 2023, 2:48 a.m. UTC 
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 mbox series

Patch

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" } } */