Bug 198198 – [WHLSL] Make sure we properly emit code for "&*x"

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Saam Barati]

...

[Saam Barati]

int foo(thread Bar* bar) {
    return bar->x;
}

turns into

int foo(thread Bar* bar) {
   return *operator&.x(&*bar);
}

the "&*bar" is suspect.

[Myles C. Maxfield]

Yeah, the parser turns bar->x into (*bar).x, then the property resolver reduces the dot expression via the ander. The ander requires a pointer, so we get one by calling &. The checker detects that (*bar) is an lvalue, so we think it's legal to say &(*bar)

[Saam Barati]

Actually, after Robin and Myles thought about it, it seems like it's correct. "*x" is an lvalue if x is an lvalue.

I guess we sort of need to treat "*x" like C++ treats references. Not sure how far this will extend in terms of implementation. But there's a chance all we need is an AST rewrite rule since we can't variables with reference types.

[Saam Barati]

As Myles suggested, a good test would be
&*p == p => true

[Myles C. Maxfield]

One way to do this would be a pass that cancels out adjacent & and * operators. But that may not be sufficient.

[Myles C. Maxfield]

For &*x, the emitted Metal code right now will be

auto temp = *x;
auto temp2 = &temp;

Which is wrong. Perhaps we could modify the Metal codegen to instead emit

auto& temp = *x;
auto temp2 = &temp;

[Myles C. Maxfield]

If we take this approach, it may actually solve https://bugs.webkit.org/show_bug.cgi?id=197448.

*x = 4;

Would get compiled to

auto& temp = *x;
temp = 4;

Which I think is correct

[Saam Barati]

(In reply to Myles C. Maxfield from comment #7)
> If we take this approach, it may actually solve
> https://bugs.webkit.org/show_bug.cgi?id=197448.
> 
> *x = 4;
> 
> Would get compiled to
> 
> auto& temp = *x;
> temp = 4;
> 
> Which I think is correct

Looks like it’d be correct. Though we’d need to make sure to selectively emit references since I’m assuming “auto& temp = 42” is invalid metal

[Saam Barati]

(In reply to Saam Barati from comment #8)
> (In reply to Myles C. Maxfield from comment #7)
> > If we take this approach, it may actually solve
> > https://bugs.webkit.org/show_bug.cgi?id=197448.
> > 
> > *x = 4;
> > 
> > Would get compiled to
> > 
> > auto& temp = *x;
> > temp = 4;
> > 
> > Which I think is correct
> 
> Looks like it’d be correct. Though we’d need to make sure to selectively
> emit references since I’m assuming “auto& temp = 42” is invalid metal

Oooh. I think you mean just for dereference nodes!

[Saam Barati]

Created attachment 371046 [details]
patch

[Myles C. Maxfield]

*** Bug 197448 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 371046 [details]
patch

Clearing flags on attachment: 371046

Committed r245973: <https://trac.webkit.org/changeset/245973>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51308436>