From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by sourceware.org (Postfix) with ESMTPS id 95A193858C50 for ; Mon, 21 Mar 2022 07:58:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 95A193858C50 Received: by mail-ej1-x62a.google.com with SMTP id qa43so28025498ejc.12 for ; Mon, 21 Mar 2022 00:58:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hIHNaF86v1oi7g9hIXBofSsTgMn7OLWm7tdyIV0noHY=; b=y9BDdBY0db8KT9SGh11lHBZC+tYc+WOhsd5lWflzj1kPuHBv9hsXN9D3tURdQAfLCB QZBkj76X3u6aDkVjQhupbi0eB+GQi2e2WDYFLQ43u4XsFP2NVKdZKb/SxX9bTzjtNx7r MD1uoedLFs3D23xOsh8SYbhytCb6Jx8fLwCNpDK9Nc7P+KNY2+V0veHHBrRNJNoxzAaY LyOSr+gBXLXspsmkVImxfunYu6VXepCgfB52z1ofLHEXp+lKvNZTCnyiCxpbxowrsVtt 9+LB69BwjBtBYef+PMWVGC9mnKAOBB1T/BHAWaHpGSPk6eeY18lpDx6fZVcuQA1awxZl xn6g== X-Gm-Message-State: AOAM533qF0oMT0V4Xnj8L8SlPg9Cc6z3Szm+JBm3Xdfp0klYJbYIcMe/ s5GMF/lGrgOV+1J63ftQo0cUXFTn7bBAZMj0EDY= X-Google-Smtp-Source: ABdhPJwr41jon2NtUpwQaRnWEJjZAFVoQNzEGP5dknIK+kS6MBZMauWS2DhjeyePCgsyhzSUbeBE9aWdk9GakMefqIQ= X-Received: by 2002:a17:907:da0:b0:6df:d4a4:9d0f with SMTP id go32-20020a1709070da000b006dfd4a49d0fmr9703877ejc.407.1647849495336; Mon, 21 Mar 2022 00:58:15 -0700 (PDT) MIME-Version: 1.0 References: <20220222135513.GA1591@delia.home> <2efa0fec-aea3-3b9e-61f2-7f0f332668e7@suse.de> In-Reply-To: <2efa0fec-aea3-3b9e-61f2-7f0f332668e7@suse.de> From: Richard Biener Date: Mon, 21 Mar 2022 08:58:04 +0100 Message-ID: Subject: Re: [PING^2][PATCH][final] Handle compiler-generated asm insn To: Tom de Vries , Alexandre Oliva Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Mar 2022 07:58:18 -0000 On Thu, Mar 17, 2022 at 4:10 PM Tom de Vries via Gcc-patches wrote: > > On 3/9/22 13:50, Tom de Vries wrote: > > On 2/22/22 14:55, Tom de Vries wrote: > >> Hi, > >> > >> For the nvptx port, with -mptx-comment we have in pr53465.s: > >> ... > >> // #APP > >> // 9 "gcc/testsuite/gcc.c-torture/execute/pr53465.c" 1 > >> // Start: Added by -minit-regs=3: > >> // #NO_APP > >> mov.u32 %r26, 0; > >> // #APP > >> // 9 "gcc/testsuite/gcc.c-torture/execute/pr53465.c" 1 > >> // End: Added by -minit-regs=3: > >> // #NO_APP > >> ... > >> > >> The comments where generated using the compiler-generated equivalent of: > >> ... > >> asm ("// Comment"); > >> ... > >> but both the printed location and the NO_APP/APP are unnecessary for a > >> compiler-generated asm insn. > >> > >> Fix this by handling ASM_INPUT_SOURCE_LOCATION == UNKNOWN_LOCATION in > >> final_scan_insn_1, such what we simply get: > >> ... > >> // Start: Added by -minit-regs=3: > >> mov.u32 %r26, 0; > >> // End: Added by -minit-regs=3: > >> ... > >> > >> Tested on nvptx. > >> > >> OK for trunk? > >> > > > > Ping^2. > > Tobias just reported an ICE in PR104968, and this patch fixes it. > > I'd like to known whether this patch is acceptable for stage 4 or not. > > If not, I need to fix PR104968 in a different way. Say, disable > -mcomment by default, or trying harder to propagate source info on > outlined functions. Usually targets use UNSPECs to emit compiler-generated "asm" instructions. I think an unknown location is a reasonable but not the best way to identify 'compiler-generated', we might lose the location through optimization. (why does it not use the INSN_LOCATION?) Rather than a location I'd use sth like DECL_ARTIFICIAL to disable 'user-mangling', do we have something like that for ASM or an insn in general? If not maybe there's an unused bit on ASMs we can enable this way. IIRC some of the Ada hardening GIMPLE passes also emit ASMs that could 'benefit' from this. Richard. > Thanks, > - Tom > > >> [final] Handle compiler-generated asm insn > >> > >> gcc/ChangeLog: > >> > >> 2022-02-21 Tom de Vries > >> > >> PR rtl-optimization/104596 > >> * config/nvptx/nvptx.cc (gen_comment): Use gen_rtx_ASM_INPUT instead > >> of gen_rtx_ASM_INPUT_loc. > >> * final.cc (final_scan_insn_1): Handle > >> ASM_INPUT_SOURCE_LOCATION == UNKNOWN_LOCATION. > >> > >> --- > >> gcc/config/nvptx/nvptx.cc | 3 +-- > >> gcc/final.cc | 17 +++++++++++------ > >> 2 files changed, 12 insertions(+), 8 deletions(-) > >> > >> diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc > >> index 858789e6df7..4124c597f24 100644 > >> --- a/gcc/config/nvptx/nvptx.cc > >> +++ b/gcc/config/nvptx/nvptx.cc > >> @@ -5381,8 +5381,7 @@ gen_comment (const char *s) > >> size_t len = strlen (ASM_COMMENT_START) + strlen (sep) + strlen > >> (s) + 1; > >> char *comment = (char *) alloca (len); > >> snprintf (comment, len, "%s%s%s", ASM_COMMENT_START, sep, s); > >> - return gen_rtx_ASM_INPUT_loc (VOIDmode, ggc_strdup (comment), > >> - cfun->function_start_locus); > >> + return gen_rtx_ASM_INPUT (VOIDmode, ggc_strdup (comment)); > >> } > >> /* Initialize all declared regs at function entry. > >> diff --git a/gcc/final.cc b/gcc/final.cc > >> index a9868861bd2..e6443ef7a4f 100644 > >> --- a/gcc/final.cc > >> +++ b/gcc/final.cc > >> @@ -2642,15 +2642,20 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, > >> int optimize_p ATTRIBUTE_UNUSED, > >> if (string[0]) > >> { > >> expanded_location loc; > >> + bool unknown_loc_p > >> + = ASM_INPUT_SOURCE_LOCATION (body) == UNKNOWN_LOCATION; > >> - app_enable (); > >> - loc = expand_location (ASM_INPUT_SOURCE_LOCATION (body)); > >> - if (*loc.file && loc.line) > >> - fprintf (asm_out_file, "%s %i \"%s\" 1\n", > >> - ASM_COMMENT_START, loc.line, loc.file); > >> + if (!unknown_loc_p) > >> + { > >> + app_enable (); > >> + loc = expand_location (ASM_INPUT_SOURCE_LOCATION (body)); > >> + if (*loc.file && loc.line) > >> + fprintf (asm_out_file, "%s %i \"%s\" 1\n", > >> + ASM_COMMENT_START, loc.line, loc.file); > >> + } > >> fprintf (asm_out_file, "\t%s\n", string); > >> #if HAVE_AS_LINE_ZERO > >> - if (*loc.file && loc.line) > >> + if (!unknown_loc_p && loc.file && *loc.file && loc.line) > >> fprintf (asm_out_file, "%s 0 \"\" 2\n", ASM_COMMENT_START); > >> #endif > >> }