Re: clang asm goto issue small reproducer

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Dec 15, 2021, at 7:25 PM, ndesaulniers ndesaulniers@google.com wrote:

> On Wed, Dec 15, 2021 at 4:18 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
>>
>> On Wed, Dec 15, 2021 at 3:50 PM Nick Desaulniers
>> <ndesaulniers@google.com> wrote:
>> >
>> > On Wed, Dec 15, 2021 at 2:06 PM Mathieu Desnoyers
>> > <mathieu.desnoyers@efficios.com> wrote:
>> > >
>> > > Hi Nick,
>> > >
>> > > I have a small reproducer for the asm goto issue we discussed over irc. I have
>> > > axed away all code that is not the problematic function by hand. Hopefully this
>> > > won't change the overall optimization context too much.
>> >
>> > If I cut the compilation pipeline off at after "middle-end"
>> > optimizations, I can see that `this_cpu_list_pop` returns `NULL` for
>> > test.c while in test-asm.c, it's a phi node of either NULL or head
>> > (which looks correct; for the test-asm.c case).
>> >
>> > "GVNPass" seems to be converting
>> >
>> > return:                                           ; preds =
>> > %cleanup.thread19, %cleanup.thread
>> >   %retval.118 = phi %struct.percpu_list_node* [ %9, %cleanup.thread ],
>> > [ null, %cleanup.thread19 ]
>> >   ret %struct.percpu_list_node* %retval.118
>> >
>> > into:
>> >
>> > return:                                           ; preds =
>> > %cleanup.thread19, %cleanup.thread
>> >   ret %struct.percpu_list_node* null
>> >
>> > Which drops the
>> >   %9 = load %struct.percpu_list_node*, %struct.percpu_list_node**
>> > %head, align 8, !tbaa !9
>>
>> Ah, Eli pointed out that I'm chasing the wrong squirrel. From test.c we have:
>>
>> struct percpu_list_node *head;
>> load = (intptr_t *)&head;
>> ret = rseq_cmpnev_storeoffp_load(targetptr, expectnot,
>>      offset, load, cpu);
>>
>> then in rseq_cmpnev_storeoffp_load():
>> __asm__ __volatile__ goto (
>>   :
>>   :
>> [load] "m" (*load));
>>
>> Did we ever initialize head?  Isn't this equivalent to:
>>
>> int* foo (void) {
>>   int *head;
>>   int **load;
>>   load = &head;
>>   asm(""::"m"(*load):"memory");
>>   return head;
>> }
>>
>> Isn't that equivalent to:
>>
>> int* foo (void) {
>>   int *head;
>>   asm(""::"m"(head):"memory");
>>   return head;
>> }
>>
>> For which neither compiler warns for -Wuninitialized? They would if
>> head was an argument to a function call...
> 
> What's going on here...
> 
> $ cat y.c
> void x (void) {
>  int y;
>  asm(""::"r"(y));
> }
> void z (void) {
>  int y;
>  asm(""::"m"(y));
> }
> $ clang -Wuninitialized y.c -c
> y.c:3:15: warning: variable 'y' is uninitialized when used here
> [-Wuninitialized]
>  asm(""::"r"(y));
>              ^
> y.c:2:8: note: initialize the variable 'y' to silence this warning
>  int y;
>       ^
>        = 0
> 1 warning generated.
> $ gcc -Wuninitialized y.c -c
> y.c: In function ‘x’:
> y.c:3:3: warning: ‘y’ is used uninitialized [-Wuninitialized]
>    3 |   asm(""::"r"(y));
>      |   ^~~
> 
> Only 1 warning each? I expected 2.
> 
> Is there something special about "m" constraints and -Wuninitialized?

In my work-arounds for the lack of output operands with asm goto,
I've used either of those tricks across various architectures:

- "m" (y) input operand with "memory" clobber, or
- "r" (&y) input operand with "memory" clobber

to achieve writing to a memory area from assembler without having
output operands available.

It may be stretching the definitions, but those were my (perhaps
ill advised) expectations.

Thanks,

Mathieu

> 
> --
> Thanks,
> ~Nick Desaulniers

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com