[webkit-reviews] review granted: [Bug 193360] [WHLSL] Add native function synthesis passes : [Attachment 358932] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 193360: [WHLSL] Add native function synthesis passes
https://bugs.webkit.org/show_bug.cgi?id=193360

Attachment 358932: Patch

https://bugs.webkit.org/attachment.cgi?id=358932&action=review




--- Comment #5 from Dean Jackson <dino at apple.com> ---
Comment on attachment 358932
  --> https://bugs.webkit.org/attachment.cgi?id=358932
Patch

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

> Source/WebCore/ChangeLog:21
> +	   - CheckDuplicateFunctions which makes sure the same function isn't
defined twice
> +	   - Intrinsics, which remembers all of the native types so they can be
referred to by the
> +	     rest of the compiler
> +	   - RecursiveTypeChecker which makes sure types don't refer to
themselves
> +	   - SynthesizeArrayOperatorLength which creates operator.length()
functions for arrays
> +	   - SynthesizeConstructors which creates copy constructors and default
constructors for all
> +	     types
> +	   - SynthesizeEnumerationFunctions which provides cast operators
between enum types and their
> +	     base types
> +	   - SynthesizeStructureAccessors which provides getters, setters, and
anders for each member
> +	     of a struct

While it would be annoying, I wish these were in separate patches.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLIntrinsics.cpp:61
> +    : m_textureTypeNames { String("Texture1D",
String::ConstructFromLiteral),
> +	   String("RWTexture1D", String::ConstructFromLiteral),
> +	   String("Texture1DArray", String::ConstructFromLiteral),
> +	   String("RWTexture1DArray", String::ConstructFromLiteral),
> +	   String("Texture2D", String::ConstructFromLiteral),
> +	   String("RWTexture2D", String::ConstructFromLiteral),
> +	   String("Texture2DArray", String::ConstructFromLiteral),
> +	   String("RWTexture2DArray", String::ConstructFromLiteral),
> +	   String("Texture3D", String::ConstructFromLiteral),
> +	   String("RWTexture3D", String::ConstructFromLiteral),
> +	   String("TextureCube",  String::ConstructFromLiteral) }
> +    , m_textureInnerTypeNames { String("uchar",
String::ConstructFromLiteral),
> +	   String("ushort", String::ConstructFromLiteral),
> +	   String("uint", String::ConstructFromLiteral),
> +	   String("char", String::ConstructFromLiteral),
> +	   String("short", String::ConstructFromLiteral),
> +	   String("int", String::ConstructFromLiteral),
> +	   String("half", String::ConstructFromLiteral),
> +	   String("float", String::ConstructFromLiteral) }
> +    , m_depthTextureInnerTypes { String("half",
String::ConstructFromLiteral),
> +	   String("float", String::ConstructFromLiteral) }

You can use "foo"_s for all these.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLIntrinsics.cpp:78
> +    if (nativeTypeDeclaration.name() == "void")

Maybe we should have static consts for these strings, rather than using the raw
text?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLIntrinsics.h:135
> +    Vector<String> m_textureTypeNames;
> +    Vector<String> m_textureInnerTypeNames;

Do these ever change? Can they be static?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLIntrinsics.h:139
> +    Vector<String> m_depthTextureInnerTypes;

Ditto.