Bug 197448

Summary: [WHLSL] Cannot assign through pointers
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Saam Barati <saam>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: justin_fan, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 195925    
Bug Blocks: 198399    
Attachments:
Description Flags
WIP none

Description Myles C. Maxfield 2019-04-30 17:46:51 PDT
Consider the following WHLSL statement:

*x = y;

When we compile this, we naively turn this into

auto temp1 = *x;
temp1 = y;

This doesn't actually assign through the pointer, instead it just reassigns a temporary

The WHLSLFunctionWriter needs to be smart enough to understand how to emit code that assigns through a pointer. We also need to determine which situations it needs to be made smarter about.
Comment 1 Myles C. Maxfield 2019-04-30 17:58:28 PDT
It looks like after the property resolver runs, lvalues can only be either variable references or dereference expressions. Assigning to a variable reference will just work, so it looks like we only need to special case assigning to a dereference expression. Also, we shouldn't need to go digging through the AST; we can just handle the outermost dereference expression.
Comment 2 Myles C. Maxfield 2019-04-30 18:08:54 PDT
Created attachment 368638 [details]
WIP
Comment 3 Radar WebKit Bug Importer 2019-05-13 17:07:32 PDT
<rdar://problem/50746281>
Comment 4 Myles C. Maxfield 2019-05-13 19:30:47 PDT
Comment on attachment 368638 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=368638&action=review

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:495
> +        m_stack.append(rightName);

This "if" block needs a return.
Comment 5 Saam Barati 2019-05-22 16:42:47 PDT
Myles is doing the WIP here in the property resolver patch. The only question we need to answer is if there are other areas where this matters?

Maybe things to consider:
*p++, etc

These should probably be desugared though. Need to verify
Comment 6 Myles C. Maxfield 2019-05-30 22:53:30 PDT
This is needed for https://bugs.webkit.org/show_bug.cgi?id=198399.
Comment 7 Myles C. Maxfield 2019-05-30 22:54:17 PDT
void FunctionDefinitionWriter::visit(AST::DereferenceExpression& dereferenceExpression)
{
    checkErrorAndVisit(dereferenceExpression.pointer());
    auto right = m_stack.takeLast();
    auto variableName = generateNextVariableName();
    m_stringBuilder.append(makeString(AST::toString(*dereferenceExpression.typeAnnotation().leftAddressSpace()), ' ', m_typeNamer.mangledNameForType(dereferenceExpression.resolvedType()), "& ", variableName, " = *", right, ";\n"));
    m_stack.append(variableName);
}
Comment 8 Saam Barati 2019-05-31 01:57:26 PDT
(In reply to Myles C. Maxfield from comment #6)
> This is needed for https://bugs.webkit.org/show_bug.cgi?id=198399.

Maybe you were running into the "&*x" problem? I also ran into this issue while trying to test the zero filling code. I took your change and turned it into a patch with a test here:
https://bugs.webkit.org/show_bug.cgi?id=198198
Comment 9 Myles C. Maxfield 2019-05-31 09:12:13 PDT

*** This bug has been marked as a duplicate of bug 198198 ***