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