Bug 198059 - WHLSL: Add an AST dumper
Summary: WHLSL: Add an AST dumper
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-20 16:06 PDT by Saam Barati
Modified: 2019-05-21 20:19 PDT (History)
9 users (show)

See Also:


Attachments
WIP (39.14 KB, patch)
2019-05-20 18:36 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
WIP (39.99 KB, patch)
2019-05-20 20:17 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (44.53 KB, patch)
2019-05-21 18:03 PDT, Saam Barati
mmaxfield: review+
Details | Formatted Diff | Diff
patch for landing (44.61 KB, patch)
2019-05-21 19:35 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch for landing (44.59 KB, patch)
2019-05-21 19:37 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-05-20 16:06:46 PDT
...
Comment 1 Saam Barati 2019-05-20 18:36:00 PDT
Created attachment 370289 [details]
WIP

It's starting to print stuff
Comment 2 Saam Barati 2019-05-20 20:17:37 PDT
Created attachment 370295 [details]
WIP

It prints more things
Comment 3 Saam Barati 2019-05-21 17:45:09 PDT
This program:

```
typedef Foo = int;
typedef Foo2 = float[42];

struct M {
    int a;
    int b;
    float c;
}

[numthreads(10, 20, 30)] compute float test1() {
    return 25;
}

vertex float test2() {
    return 25;
}

vertex float test3() : SV_Position {
    bool b = 25 ? true : false;
    return 25;
}

void testLoops()
{
    float negativeFloat = -1.5;
    int i = -1;
    do {
        i = i + 1;
    } while (i < 25);

    i = 0;
    while (i < 42) {
        i = i + 1;
    }

    for (int j = 0; j < 42; j = j + 1) { }
    int j;
    for (j = 0; j < 42; j++) { }
}

void testSemantic(float foo : attribute(42))
{

}

float foo(int a, int b, M m, thread int* foo, thread float[] floatArray) {
    int[42] x;
    x[m.a = 25] = 44;
    float result = 42;
    m.a = 25;
    if (a) {
        m.b = 25;
    } else {
        m.c = 25.5;
    }

    int aaaa = 42, bbbb = 45;

    int cccc = aaaa || bbbb;
    cccc = aaaa && bbbb;
    cccc = !cccc;

    thread int* myPointer = &aaaa;
    thread int[] myArrayPointer = @x;

    int qmark = aaaa ? aaaa : cccc;

    qmark += 20;

    qmark = (10, 20, 30);

    qmark = *foo;

    qmark = qmark++;
    qmark = ++qmark;

    //qmark++;
    ++qmark;

    switch (aaaa) {
    case 20: {
        qmark = qmark * 2;
        break;
    }
    case 25: {
        qmark = qmark * 42;
        break;
    }
    default: {
        qmark = qmark / 2;
        break;
    }
    }

    switch (aaaa) {
    case 20:
        qmark = qmark * 2;
        break;
    case 25:
        qmark = qmark * 42;
        break;
    default:
        switch (aaaa) {
        case 20:
            qmark = qmark * 2;
            break;
        case 25:
            qmark = qmark * 42;
            fallthrough;
        default:
            qmark = qmark / 2;
            break;
        }
        qmark = qmark / 2;
        break;
    }

    Foo2 foo2;

    return result;
    trap;
}
```

Prints as:

```
native typedef void;
native typedef bool;
native typedef uchar;
native typedef ushort;
native typedef uint;
native typedef char;
native typedef short;
native typedef int;
native typedef half;
native typedef float;
native typedef atomic_int;
native typedef atomic_uint;
native typedef vector<bool, 2>;
native typedef vector<bool, 3>;
native typedef vector<bool, 4>;
native typedef vector<uchar, 2>;
native typedef vector<uchar, 3>;
native typedef vector<uchar, 4>;
native typedef vector<ushort, 2>;
native typedef vector<ushort, 3>;
native typedef vector<ushort, 4>;
native typedef vector<uint, 2>;
native typedef vector<uint, 3>;
native typedef vector<uint, 4>;
native typedef vector<char, 2>;
native typedef vector<char, 3>;
native typedef vector<char, 4>;
native typedef vector<short, 2>;
native typedef vector<short, 3>;
native typedef vector<short, 4>;
native typedef vector<int, 2>;
native typedef vector<int, 3>;
native typedef vector<int, 4>;
native typedef vector<half, 2>;
native typedef vector<half, 3>;
native typedef vector<half, 4>;
native typedef vector<float, 2>;
native typedef vector<float, 3>;
native typedef vector<float, 4>;
native typedef matrix<half, 2, 2>;
native typedef matrix<half, 2, 3>;
native typedef matrix<half, 2, 4>;
native typedef matrix<half, 3, 2>;
native typedef matrix<half, 3, 3>;
native typedef matrix<half, 3, 4>;
native typedef matrix<half, 4, 2>;
native typedef matrix<half, 4, 3>;
native typedef matrix<half, 4, 4>;
native typedef matrix<float, 2, 2>;
native typedef matrix<float, 2, 3>;
native typedef matrix<float, 2, 4>;
native typedef matrix<float, 3, 2>;
native typedef matrix<float, 3, 3>;
native typedef matrix<float, 3, 4>;
native typedef matrix<float, 4, 2>;
native typedef matrix<float, 4, 3>;
native typedef matrix<float, 4, 4>;
native typedef sampler;
native typedef Texture1D<ushort>;
native typedef Texture1D<ushort2>;
native typedef Texture1D<ushort3>;
native typedef Texture1D<ushort4>;
native typedef Texture1D<uint>;
native typedef Texture1D<uint2>;
native typedef Texture1D<uint3>;
native typedef Texture1D<uint4>;
native typedef Texture1D<short>;
native typedef Texture1D<short2>;
native typedef Texture1D<short3>;
native typedef Texture1D<short4>;
native typedef Texture1D<int>;
native typedef Texture1D<int2>;
native typedef Texture1D<int3>;
native typedef Texture1D<int4>;
native typedef Texture1D<half>;
native typedef Texture1D<half2>;
native typedef Texture1D<half3>;
native typedef Texture1D<half4>;
native typedef Texture1D<float>;
native typedef Texture1D<float2>;
native typedef Texture1D<float3>;
native typedef Texture1D<float4>;
native typedef RWTexture1D<ushort>;
native typedef RWTexture1D<ushort2>;
native typedef RWTexture1D<ushort3>;
native typedef RWTexture1D<ushort4>;
native typedef RWTexture1D<uint>;
native typedef RWTexture1D<uint2>;
native typedef RWTexture1D<uint3>;
native typedef RWTexture1D<uint4>;
native typedef RWTexture1D<short>;
native typedef RWTexture1D<short2>;
native typedef RWTexture1D<short3>;
native typedef RWTexture1D<short4>;
native typedef RWTexture1D<int>;
native typedef RWTexture1D<int2>;
native typedef RWTexture1D<int3>;
native typedef RWTexture1D<int4>;
native typedef RWTexture1D<half>;
native typedef RWTexture1D<half2>;
native typedef RWTexture1D<half3>;
native typedef RWTexture1D<half4>;
native typedef RWTexture1D<float>;
native typedef RWTexture1D<float2>;
native typedef RWTexture1D<float3>;
native typedef RWTexture1D<float4>;
native typedef Texture1DArray<ushort>;
native typedef Texture1DArray<ushort2>;
native typedef Texture1DArray<ushort3>;
native typedef Texture1DArray<ushort4>;
native typedef Texture1DArray<uint>;
native typedef Texture1DArray<uint2>;
native typedef Texture1DArray<uint3>;
native typedef Texture1DArray<uint4>;
native typedef Texture1DArray<short>;
native typedef Texture1DArray<short2>;
native typedef Texture1DArray<short3>;
native typedef Texture1DArray<short4>;
native typedef Texture1DArray<int>;
native typedef Texture1DArray<int2>;
native typedef Texture1DArray<int3>;
native typedef Texture1DArray<int4>;
native typedef Texture1DArray<half>;
native typedef Texture1DArray<half2>;
native typedef Texture1DArray<half3>;
native typedef Texture1DArray<half4>;
native typedef Texture1DArray<float>;
native typedef Texture1DArray<float2>;
native typedef Texture1DArray<float3>;
native typedef Texture1DArray<float4>;
native typedef RWTexture1DArray<ushort>;
native typedef RWTexture1DArray<ushort2>;
native typedef RWTexture1DArray<ushort3>;
native typedef RWTexture1DArray<ushort4>;
native typedef RWTexture1DArray<uint>;
native typedef RWTexture1DArray<uint2>;
native typedef RWTexture1DArray<uint3>;
native typedef RWTexture1DArray<uint4>;
native typedef RWTexture1DArray<short>;
native typedef RWTexture1DArray<short2>;
native typedef RWTexture1DArray<short3>;
native typedef RWTexture1DArray<short4>;
native typedef RWTexture1DArray<int>;
native typedef RWTexture1DArray<int2>;
native typedef RWTexture1DArray<int3>;
native typedef RWTexture1DArray<int4>;
native typedef RWTexture1DArray<half>;
native typedef RWTexture1DArray<half2>;
native typedef RWTexture1DArray<half3>;
native typedef RWTexture1DArray<half4>;
native typedef RWTexture1DArray<float>;
native typedef RWTexture1DArray<float2>;
native typedef RWTexture1DArray<float3>;
native typedef RWTexture1DArray<float4>;
native typedef Texture2D<ushort>;
native typedef Texture2D<ushort2>;
native typedef Texture2D<ushort3>;
native typedef Texture2D<ushort4>;
native typedef Texture2D<uint>;
native typedef Texture2D<uint2>;
native typedef Texture2D<uint3>;
native typedef Texture2D<uint4>;
native typedef Texture2D<short>;
native typedef Texture2D<short2>;
native typedef Texture2D<short3>;
native typedef Texture2D<short4>;
native typedef Texture2D<int>;
native typedef Texture2D<int2>;
native typedef Texture2D<int3>;
native typedef Texture2D<int4>;
native typedef Texture2D<half>;
native typedef Texture2D<half2>;
native typedef Texture2D<half3>;
native typedef Texture2D<half4>;
native typedef Texture2D<float>;
native typedef Texture2D<float2>;
native typedef Texture2D<float3>;
native typedef Texture2D<float4>;
native typedef RWTexture2D<ushort>;
native typedef RWTexture2D<ushort2>;
native typedef RWTexture2D<ushort3>;
native typedef RWTexture2D<ushort4>;
native typedef RWTexture2D<uint>;
native typedef RWTexture2D<uint2>;
native typedef RWTexture2D<uint3>;
native typedef RWTexture2D<uint4>;
native typedef RWTexture2D<short>;
native typedef RWTexture2D<short2>;
native typedef RWTexture2D<short3>;
native typedef RWTexture2D<short4>;
native typedef RWTexture2D<int>;
native typedef RWTexture2D<int2>;
native typedef RWTexture2D<int3>;
native typedef RWTexture2D<int4>;
native typedef RWTexture2D<half>;
native typedef RWTexture2D<half2>;
native typedef RWTexture2D<half3>;
native typedef RWTexture2D<half4>;
native typedef RWTexture2D<float>;
native typedef RWTexture2D<float2>;
native typedef RWTexture2D<float3>;
native typedef RWTexture2D<float4>;
native typedef Texture2DArray<ushort>;
native typedef Texture2DArray<ushort2>;
native typedef Texture2DArray<ushort3>;
native typedef Texture2DArray<ushort4>;
native typedef Texture2DArray<uint>;
native typedef Texture2DArray<uint2>;
native typedef Texture2DArray<uint3>;
native typedef Texture2DArray<uint4>;
native typedef Texture2DArray<short>;
native typedef Texture2DArray<short2>;
native typedef Texture2DArray<short3>;
native typedef Texture2DArray<short4>;
native typedef Texture2DArray<int>;
native typedef Texture2DArray<int2>;
native typedef Texture2DArray<int3>;
native typedef Texture2DArray<int4>;
native typedef Texture2DArray<half>;
native typedef Texture2DArray<half2>;
native typedef Texture2DArray<half3>;
native typedef Texture2DArray<half4>;
native typedef Texture2DArray<float>;
native typedef Texture2DArray<float2>;
native typedef Texture2DArray<float3>;
native typedef Texture2DArray<float4>;
native typedef RWTexture2DArray<ushort>;
native typedef RWTexture2DArray<ushort2>;
native typedef RWTexture2DArray<ushort3>;
native typedef RWTexture2DArray<ushort4>;
native typedef RWTexture2DArray<uint>;
native typedef RWTexture2DArray<uint2>;
native typedef RWTexture2DArray<uint3>;
native typedef RWTexture2DArray<uint4>;
native typedef RWTexture2DArray<short>;
native typedef RWTexture2DArray<short2>;
native typedef RWTexture2DArray<short3>;
native typedef RWTexture2DArray<short4>;
native typedef RWTexture2DArray<int>;
native typedef RWTexture2DArray<int2>;
native typedef RWTexture2DArray<int3>;
native typedef RWTexture2DArray<int4>;
native typedef RWTexture2DArray<half>;
native typedef RWTexture2DArray<half2>;
native typedef RWTexture2DArray<half3>;
native typedef RWTexture2DArray<half4>;
native typedef RWTexture2DArray<float>;
native typedef RWTexture2DArray<float2>;
native typedef RWTexture2DArray<float3>;
native typedef RWTexture2DArray<float4>;
native typedef Texture3D<ushort>;
native typedef Texture3D<ushort2>;
native typedef Texture3D<ushort3>;
native typedef Texture3D<ushort4>;
native typedef Texture3D<uint>;
native typedef Texture3D<uint2>;
native typedef Texture3D<uint3>;
native typedef Texture3D<uint4>;
native typedef Texture3D<short>;
native typedef Texture3D<short2>;
native typedef Texture3D<short3>;
native typedef Texture3D<short4>;
native typedef Texture3D<int>;
native typedef Texture3D<int2>;
native typedef Texture3D<int3>;
native typedef Texture3D<int4>;
native typedef Texture3D<half>;
native typedef Texture3D<half2>;
native typedef Texture3D<half3>;
native typedef Texture3D<half4>;
native typedef Texture3D<float>;
native typedef Texture3D<float2>;
native typedef Texture3D<float3>;
native typedef Texture3D<float4>;
native typedef RWTexture3D<ushort>;
native typedef RWTexture3D<ushort2>;
native typedef RWTexture3D<ushort3>;
native typedef RWTexture3D<ushort4>;
native typedef RWTexture3D<uint>;
native typedef RWTexture3D<uint2>;
native typedef RWTexture3D<uint3>;
native typedef RWTexture3D<uint4>;
native typedef RWTexture3D<short>;
native typedef RWTexture3D<short2>;
native typedef RWTexture3D<short3>;
native typedef RWTexture3D<short4>;
native typedef RWTexture3D<int>;
native typedef RWTexture3D<int2>;
native typedef RWTexture3D<int3>;
native typedef RWTexture3D<int4>;
native typedef RWTexture3D<half>;
native typedef RWTexture3D<half2>;
native typedef RWTexture3D<half3>;
native typedef RWTexture3D<half4>;
native typedef RWTexture3D<float>;
native typedef RWTexture3D<float2>;
native typedef RWTexture3D<float3>;
native typedef RWTexture3D<float4>;
native typedef TextureCube<ushort>;
native typedef TextureCube<ushort2>;
native typedef TextureCube<ushort3>;
native typedef TextureCube<ushort4>;
native typedef TextureCube<uint>;
native typedef TextureCube<uint2>;
native typedef TextureCube<uint3>;
native typedef TextureCube<uint4>;
native typedef TextureCube<short>;
native typedef TextureCube<short2>;
native typedef TextureCube<short3>;
native typedef TextureCube<short4>;
native typedef TextureCube<int>;
native typedef TextureCube<int2>;
native typedef TextureCube<int3>;
native typedef TextureCube<int4>;
native typedef TextureCube<half>;
native typedef TextureCube<half2>;
native typedef TextureCube<half3>;
native typedef TextureCube<half4>;
native typedef TextureCube<float>;
native typedef TextureCube<float2>;
native typedef TextureCube<float3>;
native typedef TextureCube<float4>;
native typedef TextureDepth2D<float>;
native typedef RWTextureDepth2D<float>;
native typedef TextureDepth2DArray<float>;
native typedef RWTextureDepth2DArray<float>;
native typedef TextureDepthCube<float>;


native float operator.x(float4 ) 
native float operator.y(float4 ) 
native float operator.z(float4 ) 
native float operator.w(float4 ) 
native float4 operator.x=(float4 , float ) 
native float4 operator.y=(float4 , float ) 
native float4 operator.z=(float4 , float ) 
native float4 operator.w=(float4 , float ) 
native float ddx(float ) 
native float ddy(float ) 
native void AllMemoryBarrierWithGroupSync() 
native void DeviceMemoryBarrierWithGroupSync() 
native void GroupMemoryBarrierWithGroupSync() 


typedef bool2 = vector<bool, 2>;
typedef bool3 = vector<bool, 3>;
typedef bool4 = vector<bool, 4>;
typedef uchar2 = vector<uchar, 2>;
typedef uchar3 = vector<uchar, 3>;
typedef uchar4 = vector<uchar, 4>;
typedef ushort2 = vector<ushort, 2>;
typedef ushort3 = vector<ushort, 3>;
typedef ushort4 = vector<ushort, 4>;
typedef uint2 = vector<uint, 2>;
typedef uint3 = vector<uint, 3>;
typedef uint4 = vector<uint, 4>;
typedef char2 = vector<char, 2>;
typedef char3 = vector<char, 3>;
typedef char4 = vector<char, 4>;
typedef short2 = vector<short, 2>;
typedef short3 = vector<short, 3>;
typedef short4 = vector<short, 4>;
typedef int2 = vector<int, 2>;
typedef int3 = vector<int, 3>;
typedef int4 = vector<int, 4>;
typedef half2 = vector<half, 2>;
typedef half3 = vector<half, 3>;
typedef half4 = vector<half, 4>;
typedef float2 = vector<float, 2>;
typedef float3 = vector<float, 3>;
typedef float4 = vector<float, 4>;
typedef half2x2 = matrix<half, 2, 2>;
typedef half2x3 = matrix<half, 2, 3>;
typedef half2x4 = matrix<half, 2, 4>;
typedef half3x2 = matrix<half, 3, 2>;
typedef half3x3 = matrix<half, 3, 3>;
typedef half3x4 = matrix<half, 3, 4>;
typedef half4x2 = matrix<half, 4, 2>;
typedef half4x3 = matrix<half, 4, 3>;
typedef half4x4 = matrix<half, 4, 4>;
typedef float2x2 = matrix<float, 2, 2>;
typedef float2x3 = matrix<float, 2, 3>;
typedef float2x4 = matrix<float, 2, 4>;
typedef float3x2 = matrix<float, 3, 2>;
typedef float3x3 = matrix<float, 3, 3>;
typedef float3x4 = matrix<float, 3, 4>;
typedef float4x2 = matrix<float, 4, 2>;
typedef float4x3 = matrix<float, 4, 3>;
typedef float4x4 = matrix<float, 4, 4>;
typedef Foo = int;
typedef Foo2 = float[42];


struct M {
   int a;
   int b;
   float c;
}



[numthreads(10, 20, 30)] compute float test1() {
   return 25;
}

vertex float test2() {
   return 25;
}

vertex float test3() : SVPosition {
   bool b = bool(25) ? true : false;
   return 25;
}

void testLoops() {
   float negativeFloat = -1.500000;
   int i = 1;
   do {
      i = operator+(i, 1);
   } while(operator<(i, 25));
   i = 0;
   while (operator<(i, 42)){
      i = operator+(i, 1);
   };
   for (int j = 0; operator<(j, 42); j = operator+(j, 1)) {
   };
   int j;
   for (j = 0; operator<(j, 42); ($(0x7fe48864f480) = j, $(0x7fe48864f518) = operator++($(0x7fe48864f480)), j = $(0x7fe48864f518), $(0x7fe48864f480))) {
   };
}

void testSemantic(float foo : attribute(42)) {
}

float foo(int a, int b, M m, thread int* foo, thread float[] floatArray) {
   int[42] x;
   x[m.a = 25] = 44;
   float result = 42;
   m.a = 25;
   if (bool(a)) {
      m.b = 25;
   } else {
      m.c = 25.500000;
   };
   int aaaa = (42, bbbb = 45);
   int cccc = aaaa || bbbb;
   cccc = aaaa && bbbb;
   cccc = !bool(cccc);
   thread int* myPointer = &aaaa;
   thread int[] myArrayPointer = @x;
   int qmark = bool(aaaa) ? aaaa : cccc;
   ($(0x7fe488651710) = qmark, $(0x7fe4886517a8) = operator+($(0x7fe488651710), 20), qmark = $(0x7fe4886517a8), $(0x7fe4886517a8));
   qmark = (10, 20, 30);
   qmark = *foo;
   qmark = ($(0x7fe488651fa0) = qmark, $(0x7fe488652038) = operator++($(0x7fe488651fa0)), qmark = $(0x7fe488652038), $(0x7fe488651fa0));
   qmark = ($(0x7fe488652370) = qmark, $(0x7fe488652408) = operator++($(0x7fe488652370)), qmark = $(0x7fe488652408), $(0x7fe488652408));
   ($(0x7fe488652670) = qmark, $(0x7fe488652708) = operator++($(0x7fe488652670)), qmark = $(0x7fe488652708), $(0x7fe488652708));
   switch (aaaa) {
   case 20: {
      {
         qmark = operator*(qmark, 2);
         break;
      };
   }
   case 25: {
      {
         qmark = operator*(qmark, 42);
         break;
      };
   }
   default: {
      {
         qmark = operator/(qmark, 2);
         break;
      };
   }
   };
   switch (aaaa) {
   case 20: {
      qmark = operator*(qmark, 2);
      break;
   }
   case 25: {
      qmark = operator*(qmark, 42);
      break;
   }
   default: {
      switch (aaaa) {
      case 20: {
         qmark = operator*(qmark, 2);
         break;
      }
      case 25: {
         qmark = operator*(qmark, 42);
         fallthrough;
      }
      default: {
         qmark = operator/(qmark, 2);
         break;
      }
      };
      qmark = operator/(qmark, 2);
      break;
   }
   };
   Foo2 foo2;
   return result;
   trap;
}

```
Comment 4 Saam Barati 2019-05-21 18:03:37 PDT
Created attachment 370362 [details]
patch
Comment 5 EWS Watchlist 2019-05-21 18:05:49 PDT
Attachment 370362 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp:91:  Missing space before ( in while(  [whitespace/parens] [5]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:168:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLAST.h:45:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Alex Christensen 2019-05-21 18:13:21 PDT
Comment on attachment 370362 [details]
patch

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

> Source/WebCore/ChangeLog:32
> +        No new tests because this is used for logging.

This could be used for some great tests.
Comment 7 Myles C. Maxfield 2019-05-21 18:17:23 PDT
Comment on attachment 370362 [details]
patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:264
> +    Base::visit(constantExpression);

cool

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:577
> +        m_out.print(" && ");

Aren't parentheses required to make this non ambiguous?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:589
> +    visit(logicalNotExpression.operand());

ditto

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:609
> +    m_out.print("(");

makeString() will make these easier to read.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:44
> +    void visit(AST::UnnamedType&) override;

These can all be private because they'll all only be called from the base class.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:139
> +    dataLogLn(toString(program));

Shouldn't we make a for realsies new logging channel and use WTFLog()?
Comment 8 Saam Barati 2019-05-21 19:22:50 PDT
Comment on attachment 370362 [details]
patch

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

>> Source/WebCore/ChangeLog:32
>> +        No new tests because this is used for logging.
> 
> This could be used for some great tests.

I don't want to make tests tied to dump format.
Comment 9 Saam Barati 2019-05-21 19:26:39 PDT
Comment on attachment 370362 [details]
patch

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

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:577
>> +        m_out.print(" && ");
> 
> Aren't parentheses required to make this non ambiguous?

Yup. Will fix.

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:589
>> +    visit(logicalNotExpression.operand());
> 
> ditto

Yup. Will fix.

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:609
>> +    m_out.print("(");
> 
> makeString() will make these easier to read.

I don't follow. How do you propose to do that here? I'm calling visit() between each print call.

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:44
>> +    void visit(AST::UnnamedType&) override;
> 
> These can all be private because they'll all only be called from the base class.

will do

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:139
>> +    dataLogLn(toString(program));
> 
> Shouldn't we make a for realsies new logging channel and use WTFLog()?

Not sure what you mean here. I've never done such a thing before. Can you elaborate?
Comment 10 Saam Barati 2019-05-21 19:35:27 PDT
Created attachment 370373 [details]
patch for landing
Comment 11 Saam Barati 2019-05-21 19:37:34 PDT
Created attachment 370374 [details]
patch for landing
Comment 12 WebKit Commit Bot 2019-05-21 20:18:10 PDT
Comment on attachment 370374 [details]
patch for landing

Clearing flags on attachment: 370374

Committed r245613: <https://trac.webkit.org/changeset/245613>
Comment 13 WebKit Commit Bot 2019-05-21 20:18:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2019-05-21 20:19:19 PDT
<rdar://problem/51013223>