nvptx 'TARGET_USE_LOCAL_THUNK_ALIAS_P', 'TARGET_SUPPORTS_ALIASES' (was: [committed][nvptx] Use .alias directive for mptx >= 6.3)

Message ID 87bke7zrw5.fsf@euler.schwinge.homeip.net
State Accepted
Headers
Series nvptx 'TARGET_USE_LOCAL_THUNK_ALIAS_P', 'TARGET_SUPPORTS_ALIASES' (was: [committed][nvptx] Use .alias directive for mptx >= 6.3) |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Thomas Schwinge Sept. 12, 2023, 9:02 a.m. UTC
  Hi!

On 2022-12-02T12:24:58+0100, I wrote:
> On 2022-03-22T14:41:46+0100, Tom de Vries via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> Starting with ptx isa version 6.3, a ptx directive .alias is available.
>> Use this directive to support symbol aliases, as far as possible.
>>
>> The alias support is off by default.  It can be turned on using a switch
>> -malias.

>> This patch causes a regression:
>> ...
>> -PASS: gcc.dg/pr60797.c  (test for errors, line 4)
>> +FAIL: gcc.dg/pr60797.c  (test for errors, line 4)
>> ...
>> The test-case is skipped for effective target alias, and both without and with
>> this patch the nvptx target is considered to not support it, so the test-case is
>> executed.  The test-case expects an error message along the lines of "alias
>> definitions not supported in this configuration", but instead we run into:
>> ...
>> gcc.dg/pr60797.c:4:12: error: foo aliased to undefined symbol
>> ...
>> This is probably due to the fact that the nvptx backend now defines macros
>> ASM_OUTPUT_DEF and ASM_OUTPUT_DEF_FROM_DECLS, so from the point of view of the
>> common part of the compiler, aliases are supported.
>
> Testing this with the new '-malias' flag not active (default), I'm seeing
> a number of more regressions:
>
>     [...]: error: alias definitions not supported in this configuration
>
>     [-PASS:-]{+FAIL:+} gcc.dg/pr56727-1.c (test for excess errors)
>
>     PASS: gcc.dg/pr84739.c  (test for warnings, line 6)
>     PASS: gcc.dg/pr84739.c  (test for warnings, line 21)
>     [-PASS:-]{+FAIL:+} gcc.dg/pr84739.c (test for excess errors)
>
>     [-PASS:-]{+FAIL:+} gcc.dg/ipa/pr81520.c (test for excess errors)
>
> ..., and then, in particular, a ton of regressions in GCC/C++ testing
> ('check-gcc-c++') due to approximately 40000 new
> "error: alias definitions not supported in this configuration"
> diagnostics.  Whoops...  (I suppose you've not tested that at all?)
>
> If the new '-malias' flag is not active, we have to maintain (restore)
> the previous state of other macros, as I'm demonstrating in the attached
> "[WIP] nvptx 'TARGET_USE_LOCAL_THUNK_ALIAS_P', 'TARGET_SUPPORTS_ALIASES'".
> This completely addresses in particular the 'check-gcc-c++' regressions,
> and does not cause any changes if the new '-malias' flag in fact is
> active (as when testing with '-mptx=6.3 -malias', for example).
>
> However, this further regresses the 'gcc.dg/pr56727-1.c' and
> 'gcc.dg/ipa/pr81520.c' test cases mentioned above:
>
>     during IPA pass: whole-program
>     [...]: internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:647
>
>     {+FAIL: gcc.dg/pr56727-1.c (internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:647)+}
>     FAIL: gcc.dg/pr56727-1.c (test for excess errors)
>
>     {+FAIL: gcc.dg/ipa/pr81520.c (internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:647)+}
>     FAIL: gcc.dg/ipa/pr81520.c (test for excess errors)
>
> Such ICEs we're not yet currently seeing elsewhere; this remains to be
> analyzed -- after all, these test cases PASSed originally.

That'll be addressed by
<https://inbox.sourceware.org/87ledgzxcl.fsf@euler.schwinge.homeip.net>
"More '#ifdef ASM_OUTPUT_DEF' -> 'if (TARGET_SUPPORTS_ALIASES)' etc.":

    [-FAIL: gcc.dg/pr56727-1.c (internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:647)-]
    [-FAIL:-]{+PASS:+} gcc.dg/pr56727-1.c (test for excess errors)

    [-FAIL:-]{+PASS:+} gcc.dg/pr60797.c  (test for errors, line 4)
    [-FAIL:-]{+PASS:+} gcc.dg/pr60797.c (test for excess errors)

    [-FAIL: gcc.dg/ipa/pr81520.c (internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:647)-]
    [-FAIL:-]{+PASS:+} gcc.dg/ipa/pr81520.c (test for excess errors)

> And, obviously, the patch needs some "copy editing".

> --- a/gcc/config/nvptx/nvptx.h
> +++ b/gcc/config/nvptx/nvptx.h
> @@ -338,6 +338,12 @@ struct GTY(()) machine_function
>  #define ASM_OUTPUT_DEF_FROM_DECLS(STREAM, NAME, VALUE)       \
>    nvptx_asm_output_def_from_decls (STREAM, NAME, VALUE)
>
> +//TODO
> +/* ..., but don't let that dummy ASM_OUTPUT_DEF definition influence other
> +   macros.  */
> +#define TARGET_USE_LOCAL_THUNK_ALIAS_P(DECL) (!((nvptx_alias == 0 || !TARGET_PTX_6_3)))
> +#define TARGET_SUPPORTS_ALIASES (!((nvptx_alias == 0 || !TARGET_PTX_6_3)))

Pushed to master branch commit 537e2cc30d0f8ba6433af52f2fef038d75d93174
"nvptx 'TARGET_USE_LOCAL_THUNK_ALIAS_P', 'TARGET_SUPPORTS_ALIASES'", see
attached.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  

Patch

From 537e2cc30d0f8ba6433af52f2fef038d75d93174 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Thu, 14 Jul 2022 22:05:17 +0200
Subject: [PATCH] nvptx 'TARGET_USE_LOCAL_THUNK_ALIAS_P',
 'TARGET_SUPPORTS_ALIASES'

This fixes up commit f8b15e177155960017ac0c5daef8780d1127f91c
"[nvptx] Use .alias directive for mptx >= 6.3", which regressed in
particular C++ test cases if the new '-malias' flag was not active.
In that case, we have to maintain (that is now, restore) the previous
state of 'TARGET_USE_LOCAL_THUNK_ALIAS_P', 'TARGET_SUPPORTS_ALIASES'.

The remaining three regressions are to be resolved via
<https://inbox.sourceware.org/87ledgzxcl.fsf@euler.schwinge.homeip.net>
"More '#ifdef ASM_OUTPUT_DEF' -> 'if (TARGET_SUPPORTS_ALIASES)' etc.".

	gcc/
	* config/nvptx/nvptx.h (TARGET_USE_LOCAL_THUNK_ALIAS_P)
	(TARGET_SUPPORTS_ALIASES): Define.
---
 gcc/config/nvptx/nvptx.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/config/nvptx/nvptx.h b/gcc/config/nvptx/nvptx.h
index 15a52344d4a..129427e5654 100644
--- a/gcc/config/nvptx/nvptx.h
+++ b/gcc/config/nvptx/nvptx.h
@@ -338,6 +338,11 @@  struct GTY(()) machine_function
 #define ASM_OUTPUT_DEF_FROM_DECLS(STREAM, NAME, VALUE)	\
   nvptx_asm_output_def_from_decls (STREAM, NAME, VALUE)
 
+/* ..., but also override other macros to avoid 'gcc/defaults.h'-initialization
+   due to that dummy 'ASM_OUTPUT_DEF'.  */
+#define TARGET_USE_LOCAL_THUNK_ALIAS_P(DECL) TARGET_SUPPORTS_ALIASES
+#define TARGET_SUPPORTS_ALIASES (nvptx_alias != 0)
+
 #define NO_DOT_IN_LABEL
 #define ASM_COMMENT_START "//"
 
-- 
2.34.1