Skip to content

Commit

Permalink
[WGSL] Merge constant rewriting into the type checker
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=262387
rdar://116247645

Reviewed by Mike Wyrzykowski.

Originally I was trying to keep type checking and constant rewriting as separate
passes, as I think it's nice to have small passes with a single responsibility,
but I don't think it's possible in this case. Constant rewriting depends on type
checking, in order to make sure that the expressions that are being evaluated are
valid, and type checking depends depends on constant evaluation, for things like
`array<T, constexpr>`. While merging the two phases, I also simplified a couple
of things:
- Remove the constant duplication/insertion, which was currently implement as part
  of the global rewriter (which wasn't a great design to begin with). Instead, we
  now just serialize all the constants inline. If that proves to be a problem, we
  can always fallback to the previous approach, but probably implemented in a more
  appropriate place anyway.
- Stopped replacing AST nodes: instead, we just save the constant value in all Expression
  nodes and check it during serialization. This greatly simplified the code.

* Source/WebGPU/WGSL/AST/ASTExpression.h:
(WGSL::AST::Expression::constantValue const):
(WGSL::AST::Expression::setConstantValue):
* Source/WebGPU/WGSL/AST/ASTVisitor.cpp:
(WGSL::AST::extractInteger): Deleted.
* Source/WebGPU/WGSL/AST/ASTVisitor.h:
* Source/WebGPU/WGSL/ConstantFunctions.h:
(WGSL::constantVector):
* Source/WebGPU/WGSL/ConstantRewriter.cpp: Removed.
* Source/WebGPU/WGSL/ConstantRewriter.h: Removed.
* Source/WebGPU/WGSL/ConstantValue.h:
(WGSL::ConstantValue::toInt const):
(WGSL::ConstantValue::toDouble const):
* Source/WebGPU/WGSL/EntryPointRewriter.cpp:
(WGSL::EntryPointRewriter::EntryPointRewriter):
* Source/WebGPU/WGSL/GlobalVariableRewriter.cpp:
(WGSL::RewriteGlobalVariables::collectGlobals):
(WGSL::RewriteGlobalVariables::insertStructs):
(WGSL::RewriteGlobalVariables::insertParameters):
(WGSL::RewriteGlobalVariables::readVariable):
* Source/WebGPU/WGSL/Metal/MetalFunctionWriter.cpp:
(WGSL::Metal::FunctionDefinitionWriter::visit):
(WGSL::Metal::FunctionDefinitionWriter::serializeConstant):
* Source/WebGPU/WGSL/TypeCheck.cpp:
(WGSL::TypeChecker::TypeChecker):
(WGSL::TypeChecker::visitVariable):
(WGSL::TypeChecker::visit):
(WGSL::TypeChecker::introduceType):
(WGSL::TypeChecker::introduceValue):
(WGSL::TypeChecker::setConstantValue):
* Source/WebGPU/WGSL/WGSL.cpp:
(WGSL::staticCheck):
* Source/WebGPU/WGSL/tests/valid/global-constant-vector.wgsl:
* Source/WebGPU/WGSL/tests/valid/global-used-by-callee.wgsl:
* Source/WebGPU/WGSL/tests/valid/local-constant-vector.wgsl:
* Source/WebGPU/WGSL/tests/valid/packing.wgsl:
* Source/WebGPU/WebGPU.xcodeproj/project.pbxproj:

Canonical link: https://commits.webkit.org/268726@main
  • Loading branch information
tadeuzagallo committed Oct 2, 2023
1 parent 26433d6 commit d042e66
Show file tree
Hide file tree
Showing 17 changed files with 279 additions and 563 deletions.
5 changes: 5 additions & 0 deletions Source/WebGPU/WGSL/AST/ASTExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "ASTBuilder.h"
#include "ASTNode.h"
#include "ConstantValue.h"
#include <wtf/ReferenceWrapperVector.h>

namespace WGSL {
Expand Down Expand Up @@ -54,13 +55,17 @@ class Expression : public Node {

const Type* inferredType() const { return m_inferredType; }

const std::optional<ConstantValue>& constantValue() const { return m_constantValue; }
void setConstantValue(ConstantValue value) { m_constantValue = value; }

protected:
Expression(SourceSpan span)
: Node(span)
{ }

private:
const Type* m_inferredType { nullptr };
std::optional<ConstantValue> m_constantValue { std::nullopt };
};

} // namespace AST
Expand Down
15 changes: 0 additions & 15 deletions Source/WebGPU/WGSL/AST/ASTVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,19 +527,4 @@ void Visitor::visit(VariableQualifier&)
{
}

std::optional<unsigned> extractInteger(const AST::Expression& expression)
{
switch (expression.kind()) {
case AST::NodeKind::AbstractIntegerLiteral:
return { static_cast<unsigned>(downcast<AST::AbstractIntegerLiteral>(expression).value()) };
case AST::NodeKind::Unsigned32Literal:
return { static_cast<unsigned>(downcast<AST::Unsigned32Literal>(expression).value()) };
case AST::NodeKind::Signed32Literal:
return { static_cast<unsigned>(downcast<AST::Signed32Literal>(expression).value()) };
default:
// FIXME: handle constants and overrides
return std::nullopt;
}
}

} // namespace WGSL::AST
2 changes: 0 additions & 2 deletions Source/WebGPU/WGSL/AST/ASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,5 @@ class Visitor {
Result<void> m_expectedError;
};

std::optional<unsigned> extractInteger(const AST::Expression&);

} // namespace AST
} // namespace WGSL
4 changes: 2 additions & 2 deletions Source/WebGPU/WGSL/ConstantFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ static ConstantValue constantVector(AST::CallExpression& call, const FixedVector
auto argumentCount = arguments.size();

if (!argumentCount) {
ASSERT(std::holds_alternative<Types::Vector>(*call.inferredType()));
return zeroValue(call.inferredType());
ASSERT(std::holds_alternative<Types::Vector>(*call.target().inferredType()));
return zeroValue(call.target().inferredType());
}

if (argumentCount == 1 && !std::holds_alternative<ConstantVector>(arguments[0])) {
Expand Down
Loading

0 comments on commit d042e66

Please sign in to comment.