Hi!

On 2025-05-23T17:01:31+0200, Richard Biener <rguent...@suse.de> wrote:
> Am 23.05.2025 um 16:49 schrieb Thomas Schwinge <tschwi...@baylibre.com>:
>> This fell out of me looking into PR119835.  This doesn't resolve the 
>> underlying
>> issue, but instead of failing GIMPLE semantics verification just by chance in
>> the 'GIMPLE pass: nrv' context, it makes the issue observable generally.
>> (... thereby regressing a small number of offloading test cases where host 
>> vs.
>> offload compilers disagree on 'aggregate_value_p' for functions that return
>> aggregate types.)
>> 
>> This cross-references just the three places in the code that I ran into;
>> likely there are more?
>> 
>> No regressions for powerpc64le-unknown-linux-gnu, x86_64-pc-linux-gnu 
>> bootstrap
>> and 'make check' (without offloading configured).
>
> I think this is a step in the wrong direction in absence of quoting the wrong 
> thing that happens downstream when we violate this (an assert does not 
> qualify).  ESP. When at the same time we allow the actual thing returned to 
> be a register (aka SSA name)

ACK; you certainly understand GIMPLE and RTL expansion semantics better
than I do.  ;-)

You're also implicitly telling me that 
"'GIMPLE_RETURN' vs. 'RESULT_DECL' if 'aggregate_value_p'" isn't actually
a GIMPLE semantics invariant, thanks.  I conclude that in case that this
"invariant" is violated, that's not a problem for RTL expansion of
'GIMPLE_RETURN', which is then handled like all the other cases where
"we are not returning the current function's RESULT_DECL".

I'm not sure whether just disabling the 'assert' in
'gcc/tree-nrv.cc:pass_nrv::execute' is conceptually right (or may
potentially drive that pass into an inconsistent state), and as we of
course intend to eventually fix this issue properly (thanks for your
ideas in PR119835!), so for now, I propose to simply
"Disable 'pass_nrv' for offloading compilation [PR119835]", see attached.
Any comments before I push that?


Grüße
 Thomas


>From d94bb0a7f45ef102c4d44fe1a1eedd1eef041c21 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwi...@baylibre.com>
Date: Tue, 27 May 2025 16:02:05 +0200
Subject: [PATCH] Disable 'pass_nrv' for offloading compilation [PR119835]

... to avoid running into ICEs per PR119835, until that's resolved properly.

	PR middle-end/119835
	gcc/
	* tree-nrv.cc (pass_nrv::gate) [ACCEL_COMPILER]: 'return false;'.
	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c:
	'#pragma GCC optimize "-fno-inline"'.
	* testsuite/libgomp.c-c++-common/target-abi-struct-1.c: New.
	* testsuite/libgomp.c-c++-common/target-abi-struct-1-O0.c: Adjust.
---
 gcc/tree-nrv.cc                                        | 10 +++++++++-
 .../libgomp.c-c++-common/target-abi-struct-1-O0.c      |  2 +-
 .../libgomp.c-c++-common/target-abi-struct-1.c         |  1 +
 .../testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c |  6 +++++-
 4 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1.c

diff --git a/gcc/tree-nrv.cc b/gcc/tree-nrv.cc
index 180ce39de4c..e78e4b7e56b 100644
--- a/gcc/tree-nrv.cc
+++ b/gcc/tree-nrv.cc
@@ -125,7 +125,15 @@ public:
   {}
 
   /* opt_pass methods: */
-  bool gate (function *) final override { return optimize > 0; }
+  bool gate (function *) final override
+  {
+#ifdef ACCEL_COMPILER
+    /* PR119835 */
+    return false;
+#else
+    return optimize > 0;
+#endif
+  }
 
   unsigned int execute (function *) final override;
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1-O0.c b/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1-O0.c
index 35ec75d648d..9bf949a1f06 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1-O0.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1-O0.c
@@ -1,3 +1,3 @@
 /* { dg-additional-options -O0 } */
 
-#include "../libgomp.oacc-c-c++-common/abi-struct-1.c"
+#include "target-abi-struct-1.c"
diff --git a/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1.c b/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1.c
new file mode 100644
index 00000000000..d9268af55cf
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c-c++-common/target-abi-struct-1.c
@@ -0,0 +1 @@
+#include "../libgomp.oacc-c-c++-common/abi-struct-1.c"
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c
index 80786555fe2..4b541711f36 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/abi-struct-1.c
@@ -1,6 +1,10 @@
 /* Inspired by 'gcc.target/nvptx/abi-struct-arg.c', 'gcc.target/nvptx/abi-struct-ret.c'.  */
 
-/* See also '../libgomp.c-c++-common/target-abi-struct-1-O0.c'.  */
+/* See also '../libgomp.c-c++-common/target-abi-struct-1.c'.  */
+
+/* To exercise PR119835 (if optimizations enabled): disable inlining, so that
+   GIMPLE passes still see the functions that return aggregate types.  */
+#pragma GCC optimize "-fno-inline"
 
 typedef struct {} empty;  /* See 'gcc/doc/extend.texi', "Empty Structures".  */
 typedef struct {char a;} schar;
-- 
2.34.1

Reply via email to