Bug 193436 - [WHLSL] Implement the recursion checker
Summary: [WHLSL] Implement the recursion checker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-14 22:27 PST by Myles C. Maxfield
Modified: 2019-01-16 16:54 PST (History)
9 users (show)

See Also:


Attachments
Patch (12.01 KB, patch)
2019-01-14 22:29 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-highsierra (2.01 MB, application/zip)
2019-01-15 00:21 PST, EWS Watchlist
no flags Details
Patch (17.05 KB, patch)
2019-01-15 16:46 PST, Myles C. Maxfield
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2019-01-14 22:27:32 PST
[WHLSL] Implement the recursion checker
Comment 1 Myles C. Maxfield 2019-01-14 22:29:32 PST
Created attachment 359138 [details]
Patch
Comment 2 Myles C. Maxfield 2019-01-14 22:44:33 PST
Comment on attachment 359138 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLRecursionChecker.cpp:62
> +    void visit(AST::FunctionDefinition& functionDefinition) override
> +    {
> +        auto addResult = m_visitingSet.add(&functionDefinition);
> +        if (!addResult.isNewEntry) {
> +            setError();
> +            return;
> +        }
> +
> +        Visitor::visit(functionDefinition);
> +
> +        auto success = m_visitingSet.remove(&functionDefinition);
> +        ASSERT_UNUSED(success, success);
> +    }
> +
> +    void visit(AST::CallExpression& callExpression) override
> +    {
> +        ASSERT(callExpression.function());
> +        Visitor::visit(*callExpression.function());
> +    }

These can be private.
Comment 3 Darin Adler 2019-01-14 23:05:14 PST
Comment on attachment 359138 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLRecursionChecker.cpp:42
> +    RecursionChecker() = default;
> +
> +    virtual ~RecursionChecker() = default;

These can be omitted because this is what will be generated if you don’t write anything.
Comment 4 EWS Watchlist 2019-01-15 00:21:31 PST
Comment on attachment 359138 [details]
Patch

Attachment 359138 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10755536

New failing tests:
compositing/backing/animate-into-view.html
Comment 5 EWS Watchlist 2019-01-15 00:21:32 PST
Created attachment 359143 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 Myles C. Maxfield 2019-01-15 16:46:45 PST
Created attachment 359225 [details]
Patch
Comment 7 Saam Barati 2019-01-16 08:40:49 PST
Comment on attachment 359225 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLRecursionChecker.cpp:53
> +        auto success = m_visitingSet.remove(&functionDefinition);

Each AST node representing a call to a function “foo” points to the same memory location?
Comment 8 Myles C. Maxfield 2019-01-16 16:47:41 PST
Comment on attachment 359225 [details]
Patch

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

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLRecursionChecker.cpp:53
>> +        auto success = m_visitingSet.remove(&functionDefinition);
> 
> Each AST node representing a call to a function “foo” points to the same memory location?

Yes. Each function definition is a single object in the AST, and all calls to it have a .function() that points to it.
Comment 9 Myles C. Maxfield 2019-01-16 16:53:47 PST
Committed r240096: <https://trac.webkit.org/changeset/240096>
Comment 10 Radar WebKit Bug Importer 2019-01-16 16:54:29 PST
<rdar://problem/47335051>