WGSL: implement comma operator

There is no commma operator in WGSL, nor a way to nest multiple
expressions inside one.

Similarly to ternaries, this is implemented by pulling the
expressions into a functions, and all temporaries inside
the expression are converted into globals.

Bug: angleproject:42267100
Change-Id: I7b663571fc2beb92fa1e8b1bcb3e79cc1b989590
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7057452
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

Author: mdenton8

src/compiler/translator/tree_ops/wgsl/PullExpressionsIntoFunctions.cpp

@@ -116,6 +116,18 @@ class PullExpressionsIntoFunctionsTraverser : public TIntermTraverser
         return true;
     }
 
+    bool visitBinary(Visit visit, TIntermBinary *binary) override
+    {
+        if (binary->getOp() == EOpComma)
+        {
+            handleUntranslatableConstruct(
+                visit, UntranslatableConstructAndMetadata{UntranslatableCommaOperator{binary},
+                                                          getParentNode(), mCurrentFunction});
+        }
+
+        return true;
+    }
+
     bool foundUntranslatableConstruct() const { return !mUntranslatableConstructs.empty(); }
 
     bool update(TIntermBlock *root)
@@ -126,8 +138,13 @@ class PullExpressionsIntoFunctionsTraverser : public TIntermTraverser
 
   private:
     using UntranslatableTernary = TIntermTernary *;
+    struct UntranslatableCommaOperator
+    {
+        TIntermBinary *commaOperator;
+    };
 
-    using UntranslatableConstruct = std::variant<UntranslatableTernary>;
+    using UntranslatableConstruct =
+        std::variant<UntranslatableTernary, UntranslatableCommaOperator>;
 
     struct UntranslatableConstructAndMetadata
     {
@@ -228,6 +245,64 @@ class PullExpressionsIntoFunctionsTraverser : public TIntermTraverser
         return substituteFunction;
     }
 
+    TFunction *replaceSequenceOperator(TIntermBinary *sequenceOperator,
+                                       const TIntermFunctionDefinition *parentFunction,
+                                       TIntermSequence &newFunctionPrototypes,
+                                       TIntermSequence &newFunctionDefinitions)
+    {
+        ASSERT(sequenceOperator->getOp() == EOpComma);
+
+        // Pull into function that just puts one statement after the other.
+
+        TIntermSequence extractedStmts;
+
+        // Flatten the nested comma operators into a sequence of statements.
+        std::stack<TIntermTyped *, TVector<TIntermTyped *>> stmts;
+        stmts.push(sequenceOperator->getRight());
+        stmts.push(sequenceOperator->getLeft());
+        while (!stmts.empty())
+        {
+            TIntermTyped *stmt = stmts.top();
+            stmts.pop();
+
+            if (TIntermBinary *nestedSequenceOperator = stmt->getAsBinaryNode();
+                nestedSequenceOperator && nestedSequenceOperator->getOp() == EOpComma)
+            {
+                stmts.push(nestedSequenceOperator->getRight());
+                stmts.push(nestedSequenceOperator->getLeft());
+                continue;
+            }
+
+            extractedStmts.push_back(stmt);
+        }
+
+        // The last statement needs a return, if it is not of type void (i.e. the type of a function
+        // call to a void-returning function).
+        TIntermNode *lastStmt = extractedStmts.back();
+        if (lastStmt->getAsTyped()->getBasicType() != EbtVoid)
+        {
+            extractedStmts.back() = new TIntermBranch(TOperator::EOpReturn, lastStmt->getAsTyped());
+        }
+
+        TIntermBlock *substituteFunctionBody = new TIntermBlock(std::move(extractedStmts));
+
+        TFunction *substituteFunction =
+            new TFunction(mSymbolTable, kEmptyImmutableString, SymbolType::AngleInternal,
+                          GetHelperType(sequenceOperator->getType(), EvqTemporary),
+                          !sequenceOperator->hasSideEffects());
+
+        // Make sure to insert new function definitions and prototypes.
+        newFunctionPrototypes.push_back(new TIntermFunctionPrototype(substituteFunction));
+        TIntermFunctionDefinition *substituteFunctionDef = new TIntermFunctionDefinition(
+            new TIntermFunctionPrototype(substituteFunction), substituteFunctionBody);
+        newFunctionDefinitions.push_back(substituteFunctionDef);
+
+        addParamsFromOtherFunctionAndReplace(substituteFunction, substituteFunctionDef,
+                                             parentFunction);
+
+        return substituteFunction;
+    }
+
     bool replaceTempVarsWithGlobals(TIntermBlock *root)
     {
         TIntermSequence globalDeclarations;
@@ -326,6 +401,14 @@ class PullExpressionsIntoFunctionsTraverser : public TIntermTraverser
                 substituteFunction = replaceTernary(*ternary, constructAndMetadata.parentFunction,
                                                     newFunctionPrototypes, newFunctionDefinitions);
             }
+            else if (UntranslatableCommaOperator *commaOperator =
+                         std::get_if<UntranslatableCommaOperator>(&construct))
+            {
+                untranslatableNode = commaOperator->commaOperator;
+                substituteFunction = replaceSequenceOperator(
+                    commaOperator->commaOperator, constructAndMetadata.parentFunction,
+                    newFunctionPrototypes, newFunctionDefinitions);
+            }
             else
             {
                 UNREACHABLE();

src/compiler/translator/wgsl/TranslatorWGSL.cpp

@@ -506,7 +506,8 @@ OperatorInfo OutputWGSLTraverser::useOperatorAndGetInfo(TIntermNode *current,
         case TOperator::EOpComma:
             // WGSL does not have a comma operator or any other way to implement "statement list as
             // an expression", so nested expressions will have to be pulled out into statements.
-            UNIMPLEMENTED();
+            // This should have been done by a preprocessing.
+            UNREACHABLE();
             return {"TODO_operator"};
         case TOperator::EOpAssign:
             return {"="};

src/tests/compiler_tests/WGSLOutput_test.cpp

@@ -1370,6 +1370,138 @@ fn wgslMain(ANGLE_input_annotated : ANGLE_Input_Annotated) -> ANGLE_Output_Annot
     EXPECT_TRUE(foundInCode(outputString.c_str()));
 }
 
+TEST_F(WGSLOutputTest, CommaOperator)
+{
+    const std::string &shaderString =
+        R"(#version 300 es
+precision mediump float;
+
+float globVar;
+
+in float inVar;
+out vec4 fragColor;
+
+void setGlobVar() {
+  globVar = 1.0;
+}
+
+void main() {
+  fragColor = vec4(0.0);
+  float tempVar;
+  fragColor.x = (globVar = inVar, tempVar = globVar, tempVar);
+
+  (tempVar = 5.0, globVar = 6.0, setGlobVar());
+
+  float a,b,c,d,e;
+  fragColor.x += ((a = 1.0, b = a), (c = b, (d = c)), (setGlobVar(), e = d, e));
+})";
+    const std::string &outputString =
+        R"(diagnostic(warning,derivative_uniformity);
+struct ANGLE_Input_Global {
+  inVar : f32,
+};
+
+var<private> ANGLE_input_global : ANGLE_Input_Global;
+
+struct ANGLE_Input_Annotated {
+  @location(@@@@@@) inVar : f32,
+};
+
+struct ANGLE_Output_Global {
+  fragColor : vec4<f32>,
+};
+
+var<private> ANGLE_output_global : ANGLE_Output_Global;
+
+struct ANGLE_Output_Annotated {
+  @location(@@@@@@) fragColor : vec4<f32>,
+};
+
+@group(2) @binding(0) var<uniform> ANGLEUniforms : ANGLEUniformBlock;
+
+struct ANGLEDepthRangeParams
+{
+  near : f32,
+  far : f32,
+  diff : f32,
+};
+
+var<private> _uglobVar : f32;
+;
+;
+var<private> _utempVar : f32;
+var<private> _ua : f32;
+var<private> _ub : f32;
+var<private> _uc : f32;
+var<private> _ud : f32;
+var<private> _ue : f32;
+
+struct ANGLEUniformBlock
+{
+  @align(16) acbBufferOffsets : vec2<u32>,
+  depthRange : vec2<f32>,
+  renderArea : u32,
+  flipXY : u32,
+  dither : u32,
+  misc : u32,
+};
+
+;
+;
+;
+;
+
+fn _usetGlobVar()
+{
+  (_uglobVar) = (1.0f);
+}
+
+fn _umain()
+{
+  (ANGLE_output_global.fragColor) = (vec4<f32>(0.0f, 0.0f, 0.0f, 0.0f));
+  ((ANGLE_output_global.fragColor).x) = (sbca());
+  sbcb();
+  ((ANGLE_output_global.fragColor).x) += (sbcc());
+}
+
+fn sbca() -> f32
+{
+  (_uglobVar) = (ANGLE_input_global.inVar);
+  (_utempVar) = (_uglobVar);
+  return _utempVar;
+}
+
+fn sbcb()
+{
+  (_utempVar) = (5.0f);
+  (_uglobVar) = (6.0f);
+  _usetGlobVar();
+}
+
+fn sbcc() -> f32
+{
+  (_ua) = (1.0f);
+  (_ub) = (_ua);
+  (_uc) = (_ub);
+  (_ud) = (_uc);
+  _usetGlobVar();
+  (_ue) = (_ud);
+  return _ue;
+}
+@fragment
+fn wgslMain(ANGLE_input_annotated : ANGLE_Input_Annotated) -> ANGLE_Output_Annotated
+{
+  ANGLE_input_global.inVar = ANGLE_input_annotated.inVar;
+  _umain();
+  var ANGLE_output_annotated : ANGLE_Output_Annotated;
+  ANGLE_output_annotated.fragColor = ANGLE_output_global.fragColor;
+  return ANGLE_output_annotated;
+}
+)";
+    compile(shaderString);
+    EXPECT_TRUE(foundInCode(outputString.c_str()));
+}
+
 TEST_F(WGSLOutputTest, UniformsWithMatCx2)
 {
     const std::string &shaderString =