Hi!
On 2025-05-28T09:18:29+0200, Richard Biener <[email protected]> wrote:
> On Tue, 27 May 2025, Thomas Schwinge wrote:
>> "'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?
>
> I'm not sure you can disable this pass - it runs even at -O0
No, runs only for 'optimize > 0'.
(I guess you were looking at 'pass_return_slot', living in the same
file.)
> so parts
> of it might be required for correctness, since some types cannot be
> copied. Maybe RTL expansion will apply NRV if that's the case,
> irrespective of whether the flag is set, but maybe not.
>
> I think a more appropriate solution would be to simply change
> the assert as follows
> --- a/gcc/tree-nrv.cc
> +++ b/gcc/tree-nrv.cc
> @@ -171,12 +171,12 @@ pass_nrv::execute (function *fun)
>
> if (greturn *return_stmt = dyn_cast <greturn *> (stmt))
> {
> - /* In a function with an aggregate return value, the
> - gimplifier has changed all non-empty RETURN_EXPRs to
> - return the RESULT_DECL. */
> + /* In a function with an aggregate return value, if
> + there is a return that does not return RESULT_DECL
> + we cannot perform NRV optimizations. */
> ret_val = gimple_return_retval (return_stmt);
> - if (ret_val)
> - gcc_assert (ret_val == result);
> + if (ret_val && ret_val != result)
> + return 0;
> }
> else if (gimple_has_lhs (stmt)
Ah, right, in this scanning stage, no code transformations have been done
yet, so we may still 'return 0;' (..., which then effectively also
disables the pass).
But, really also lose the check for non-offloading configurations, or do
this defensive variant only '#ifdef ACCEL_COMPILER', as in the attached
"Defuse 'RESULT_DECL' check in 'pass_nrv' for offloading compilation
[PR119835]"?
Grüße
Thomas
>From 6f391a97d49072f3ff32ea397e3d70ad9103c196 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <[email protected]>
Date: Tue, 27 May 2025 16:02:05 +0200
Subject: [PATCH] Defuse 'RESULT_DECL' check in '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.
Co-authored-by: Richard Biener <[email protected]>
---
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..171d4c19f3e 100644
--- a/gcc/tree-nrv.cc
+++ b/gcc/tree-nrv.cc
@@ -176,7 +176,15 @@ pass_nrv::execute (function *fun)
return the RESULT_DECL. */
ret_val = gimple_return_retval (return_stmt);
if (ret_val)
- gcc_assert (ret_val == result);
+ {
+#ifdef ACCEL_COMPILER
+ /* PR119835 */
+ if (ret_val != result)
+ return 0;
+#else
+ gcc_assert (ret_val == result);
+#endif
+ }
}
else if (gimple_has_lhs (stmt)
&& gimple_get_lhs (stmt) == result)
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
