On 3/21/22 08:58, Richard Biener wrote:
On Thu, Mar 17, 2022 at 4:10 PM Tom de Vries via Gcc-patches
<gcc-patches@gcc.gnu.org> 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.
Hi,
thanks for the review.
Usually targets use UNSPECs to emit compiler-generated "asm"
instructions.
Ack. [ I could go down that route eventually, but for now I'm hoping to
implement this without having to change the port. ]
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?)
I don't know. FWIW, at the time that ASM_INPUT_SOURCE_LOCATION was
introduced (2007), there was no INSN_LOCATION yet (introduced in 2012),
only INSN_LOCATOR, my guess is that it has something to do with that.
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?
Haven't found it.
If not maybe there's an unused
bit on ASMs we can enable this way.
Done. I've used the jump flag for that.
Updated, untested patch attached.
Is this what you meant?
Thanks,
- Tom
[final] Handle compiler-generated asm insn
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:
- adding new flag ASM_INPUT_ARTIFICIAL_P
- in gen_comment:
- setting ASM_INPUT_ARTIFICIAL_P to 1
- setting ASM_INPUT_SOURCE_LOCATION to UNKNOWN_LOCATION,
- in final_scan_insn_1:
- handling ASM_INPUT_SOURCE_LOCATION == UNKNOWN_LOCATION and
ASM_INPUT_ARTIFICIAL_P
such what we simply get:
...
// Start: Added by -minit-regs=3:
mov.u32 %r26, 0;
// End: Added by -minit-regs=3:
...
Tested on nvptx.
gcc/ChangeLog:
2022-02-21 Tom de Vries <tdevr...@suse.de>
PR rtl-optimization/104596
* rtl.h (struct rtx_def): Document use of jump flag in ASM_INPUT.
(ASM_INPUT_ARTIFICIAL_P): New macro.
* config/nvptx/nvptx.cc (gen_comment): Use gen_rtx_ASM_INPUT instead
of gen_rtx_ASM_INPUT_loc. Set ASM_INPUT_ARTIFICIAL_P.
* final.cc (final_scan_insn_1): Handle
ASM_INPUT_SOURCE_LOCATION == UNKNOWN_LOCATION and
ASM_INPUT_ARTIFICIAL_P.
---
gcc/config/nvptx/nvptx.cc | 5 +++--
gcc/final.cc | 18 ++++++++++++------
gcc/rtl.h | 3 +++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc
index 87efc23bd96a..93df3f309d18 100644
--- a/gcc/config/nvptx/nvptx.cc
+++ b/gcc/config/nvptx/nvptx.cc
@@ -5442,8 +5442,9 @@ 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),
- DECL_SOURCE_LOCATION (cfun->decl));
+ rtx asm_input = gen_rtx_ASM_INPUT (VOIDmode, ggc_strdup (comment));
+ ASM_INPUT_ARTIFICIAL_P (asm_input) = 1;
+ return asm_input;
}
/* Initialize all declared regs at function entry.
diff --git a/gcc/final.cc b/gcc/final.cc
index a9868861bd2c..fee512869482 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -2642,15 +2642,21 @@ 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 (!ASM_INPUT_ARTIFICIAL_P (body))
+ app_enable ();
+ if (!unknown_loc_p)
+ {
+ 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.line)
fprintf (asm_out_file, "%s 0 \"\" 2\n", ASM_COMMENT_START);
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 9df2fab622e7..3d7cc2be45c4 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -325,6 +325,7 @@ struct GTY((desc("0"), tag("0"),
1 in a VALUE is SP_BASED_VALUE_P in cselib.cc.
1 in a SUBREG generated by LRA for reload insns.
1 in a REG if this is a static chain register.
+ 1 in an ASM_INPUT if it is compiler-generated.
Dumped as "/j" in RTL dumps. */
unsigned int jump : 1;
/* In a CODE_LABEL, part of the two-bit alternate entry field.
@@ -2592,6 +2593,8 @@ do { \
#define ASM_OPERANDS_LABEL(RTX, N) XCVECEXP (RTX, 5, N, ASM_OPERANDS)
#define ASM_OPERANDS_SOURCE_LOCATION(RTX) XCUINT (RTX, 6, ASM_OPERANDS)
#define ASM_INPUT_SOURCE_LOCATION(RTX) XCUINT (RTX, 1, ASM_INPUT)
+#define ASM_INPUT_ARTIFICIAL_P(RTX) \
+ (RTL_FLAG_CHECK1 ("ASM_INPUT_ARTIFICIAL_P", (RTX), ASM_INPUT)->jump)
/* 1 if RTX is a mem that is statically allocated in read-only memory. */
#define MEM_READONLY_P(RTX) \
