HLSL: phase 2b: add l-value operator[] for RWTexture/RWBuffer

This commit adds l-value support for RW texture and buffer objects.
Supported are:

- pre and post inc/decrement
- function out parameters
- op-assignments, such as *=, +-, etc.
- result values from op-assignments.  e.g, val=(MyRwTex[loc] *= 2);

Not supported are:
- Function inout parameters
- multiple post-inc/decrement operators.  E.g, MyRWTex[loc]++++;

Test/hlsl.rw.bracket.frag

@@ -35,6 +35,10 @@ uniform int2  o2;
 uniform int3  o3;
 uniform int4  o4;
 
+uniform float4 uf4;
+uniform int4   ui4;
+uniform uint4  uu4;
+
 int4   Fn1(in int4 x)   { return x; }
 uint4  Fn1(in uint4 x)  { return x; }
 float4 Fn1(in float4 x) { return x; }
@@ -43,12 +47,12 @@ void Fn2(out int4 x)   { x = int4(0); }
 void Fn2(out uint4 x)  { x = uint4(0); }
 void Fn2(out float4 x) { x = float4(0); }
 
+float4 SomeValue() { return c4; }
+
 PS_OUTPUT main()
 {
    PS_OUTPUT psout;
 
-   // Test as R-values
-
    // 1D
    g_tTex1df4[c1];
    
@@ -66,47 +70,66 @@ PS_OUTPUT main()
    int4   r21 = g_tTex3di4[c3];
    uint4  r22 = g_tTex3du4[c3];
 
-   // // Test as L-values
+   float4 lf4 = uf4;
-   // // 1D
-   // g_tTex1df4[c1] = float4(1,2,3,4);
-   // g_tTex1di4[c1] = int4(1,2,3,4);
-   // g_tTex1du4[c1] = uint4(1,2,3,4);
 
-   // // 2D
+   // Test as L-values
-   // g_tTex2df4[c2] = float4(1,2,3,4);
+   // 1D
-   // g_tTex2di4[c2] = int4(1,2,3,4);
+   g_tTex1df4[c1] = SomeValue(); // complex L-value
-   // g_tTex2du4[c2] = uint4(1,2,3,4);
+   g_tTex1df4[c1] = lf4;
+   g_tTex1di4[c1] = int4(2,2,3,4);
+   g_tTex1du4[c1] = uint4(3,2,3,4);
 
-   // // 3D
+   // Test some operator= things, which need to do both a load and a store.
-   // g_tTex3df4[c3] = float4(1,2,3,4);
+   float4 val1 = (g_tTex1df4[c1] *= 2.0);
-   // g_tTex3di4[c3] = int4(1,2,3,4);
+   g_tTex1df4[c1] -= 3.0;
-   // g_tTex3du4[c3] = uint4(1,2,3,4);
+   g_tTex1df4[c1] += 4.0;
+   
+   g_tTex1di4[c1] /= 2;
+   g_tTex1di4[c1] %= 2;
+   g_tTex1di4[c1] &= 0xffff;
+   g_tTex1di4[c1] |= 0xf0f0;
+   g_tTex1di4[c1] <<= 2;
+   g_tTex1di4[c1] >>= 2;
+
+   // 2D
+   g_tTex2df4[c2] = SomeValue(); // complex L-value
+   g_tTex2df4[c2] = lf4;
+   g_tTex2di4[c2] = int4(5,2,3,4);
+   g_tTex2du4[c2] = uint4(6,2,3,4);
+   
+   // 3D
+   g_tTex3df4[c3] = SomeValue(); // complex L-value
+   g_tTex3df4[c3] = lf4;
+   g_tTex3di4[c3] = int4(8,6,7,8);
+   g_tTex3du4[c3] = uint4(9,2,3,4);
 
    // // Test function calling
    Fn1(g_tTex1df4[c1]);  // in
    Fn1(g_tTex1di4[c1]);  // in
    Fn1(g_tTex1du4[c1]);  // in
 
-   // Fn2(g_tTex1df4[c1]);  // out
+   Fn2(g_tTex1df4[c1]);  // out
-   // Fn2(g_tTex1di4[c1]);  // out
+   Fn2(g_tTex1di4[c1]);  // out
-   // Fn2(g_tTex1du4[c1]);  // out
+   Fn2(g_tTex1du4[c1]);  // out
 
-   // // Test increment operators
+   // Test increment operators
-   // g_tTex1df4[c1]++;
+   // pre-ops
-   // g_tTex1di4[c1]++;
+   ++g_tTex1df4[c1];
-   // g_tTex1du4[c1]++;
+   ++g_tTex1di4[c1];
+   ++g_tTex1du4[c1];
 
-   // g_tTex1df4[c1]--;
+   --g_tTex1df4[c1];
-   // g_tTex1di4[c1]--;
+   --g_tTex1di4[c1];
-   // g_tTex1du4[c1]--;
+   --g_tTex1du4[c1];
 
-   // ++g_tTex1df4[c1];
+   // post-ops
-   // ++g_tTex1di4[c1];
+   g_tTex1df4[c1]++;
-   // ++g_tTex1du4[c1];
+   g_tTex1du4[c1]--;
+   g_tTex1di4[c1]++;
 
-   // --g_tTex1df4[c1];
+   g_tTex1df4[c1]--;
-   // --g_tTex1di4[c1];
+   g_tTex1di4[c1]++;
-   // --g_tTex1du4[c1];
+   g_tTex1du4[c1]--;
 
    psout.Color = 1.0;
 

glslang/MachineIndependent/Intermediate.cpp

@@ -2045,6 +2045,10 @@ bool TIntermBinary::promote()
         }
         break;
 
+    case EOpVectorTimesScalarAssign:
+        if (left->isVector() && right->isScalar())
+            return true;
+
     default:
         return false;
     }

hlsl/hlslGrammar.cpp

@@ -1783,6 +1783,8 @@ bool HlslGrammar::acceptAssignmentExpression(TIntermTyped*& node)
     }
 
     node = parseContext.handleAssign(loc, assignOp, node, rightNode);
+    node = parseContext.handleLvalue(loc, "assign", node);
+
     if (node == nullptr) {
         parseContext.error(loc, "could not create assignment", "", "");
         return false;
@@ -1946,6 +1948,7 @@ bool HlslGrammar::acceptUnaryExpression(TIntermTyped*& node)
         return true;
 
     node = intermediate.addUnaryMath(unaryOp, node, loc);
+    node = parseContext.handleLvalue(loc, "", node);
 
     return node != nullptr;
 }
@@ -2061,6 +2064,7 @@ bool HlslGrammar::acceptPostfixExpression(TIntermTyped*& node)
         case EOpPostDecrement:
             // DEC_OP
             node = intermediate.addUnaryMath(postOp, node, loc);
+            node = parseContext.handleLvalue(loc, "", node);
             break;
         default:
             assert(0);

hlsl/hlslParseHelper.cpp

@@ -130,6 +130,249 @@ bool HlslParseContext::parseShaderStrings(TPpContext& ppContext, TInputScanner&
     return numErrors == 0;
 }
 
+bool HlslParseContext::shouldConvertLValue(const TIntermNode* node) const
+{
+    if (node == nullptr)
+        return false;
+
+    const TIntermAggregate* lhsAsAggregate = node->getAsAggregate();
+    if (lhsAsAggregate != nullptr && lhsAsAggregate->getOp() == EOpImageLoad)
+        return true;
+
+    return false;
+}
+
+
+//
+// This function handles l-value conversions and verifications.  It uses, but is not synonymous
+// with lValueErrorCheck.  That function accepts an l-value directly, while this one must be
+// given the surrounding tree - e.g, with an assignment, so we can convert the assign into a
+// series of other image operations.
+//
+// Most things are passed through unmodified, except for error checking.
+// 
+TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char* op, TIntermTyped* node)
+{
+    TIntermBinary* nodeAsBinary = node->getAsBinaryNode();
+    TIntermUnary* nodeAsUnary = node->getAsUnaryNode();
+    TIntermAggregate* sequence = nullptr;
+
+    TIntermTyped* lhs = nodeAsUnary  ? nodeAsUnary->getOperand() :
+                        nodeAsBinary ? nodeAsBinary->getLeft() :
+                        nullptr;
+
+
+    // Early bail out if there is no conversion to apply
+    if (!shouldConvertLValue(lhs)) {
+        // TODO: >..
+        // if (lhs != nullptr)
+        //     if (lValueErrorCheck(loc, op, lhs))
+        //         return nullptr;
+        return node;
+    }
+
+    // *** If we get here, we're going to apply some conversion to an l-value.
+
+    // Helper to create a load.
+    const auto makeLoad = [&](TIntermSymbol* rhsTmp, TIntermTyped* object, TIntermTyped* coord, const TType& derefType) {
+        TIntermAggregate* loadOp = new TIntermAggregate(EOpImageLoad);
+        loadOp->setLoc(loc);
+        loadOp->getSequence().push_back(object);
+        loadOp->getSequence().push_back(intermediate.addSymbol(*coord->getAsSymbolNode()));
+        loadOp->setType(derefType);
+
+        sequence = intermediate.growAggregate(sequence,
+                                              intermediate.addAssign(EOpAssign, rhsTmp, loadOp, loc),
+                                              loc);
+    };
+
+    // Helper to create a store.
+    const auto makeStore = [&](TIntermTyped* object, TIntermTyped* coord, TIntermSymbol* rhsTmp) {
+        TIntermAggregate* storeOp = new TIntermAggregate(EOpImageStore);
+        storeOp->getSequence().push_back(object);
+        storeOp->getSequence().push_back(coord);
+        storeOp->getSequence().push_back(intermediate.addSymbol(*rhsTmp));
+        storeOp->setLoc(loc);
+        storeOp->setType(TType(EbtVoid));
+
+        sequence = intermediate.growAggregate(sequence, storeOp);
+    };
+
+    // Helper to create an assign.
+    const auto makeAssign = [&](TOperator assignOp, TIntermTyped* lhs, TIntermTyped* rhs) {
+        sequence = intermediate.growAggregate(sequence,
+                                              intermediate.addAssign(assignOp, lhs, rhs, loc),
+                                              loc);
+    };
+
+    // Helper to complete sequence by adding trailing variable, so we evaluate to the right value.
+    const auto finishSequence = [&](TIntermSymbol* rhsTmp, const TType& derefType) {
+        // Add a trailing use of the temp, so the sequence returns the proper value.
+        sequence = intermediate.growAggregate(sequence, intermediate.addSymbol(*rhsTmp));
+        sequence->setOperator(EOpSequence);
+        sequence->setLoc(loc);
+        sequence->setType(derefType);
+
+        return sequence;
+    };
+
+    // Helper to add unary op
+    const auto addUnary = [&](TOperator op, TIntermSymbol* rhsTmp) {
+        sequence = intermediate.growAggregate(sequence,
+                                              intermediate.addUnaryMath(op, intermediate.addSymbol(*rhsTmp), loc),
+                                              loc);
+    };
+
+    // helper to create a temporary variable
+    const auto addTmpVar = [&](const char* name, const TType& derefType) {
+        TVariable* tmpVar = makeInternalVariable(name, derefType);
+        tmpVar->getWritableType().getQualifier().makeTemporary();
+        return intermediate.addSymbol(*tmpVar, loc);
+    };
+    
+    TIntermAggregate* lhsAsAggregate = lhs->getAsAggregate();
+    TIntermTyped* object = lhsAsAggregate->getSequence()[0]->getAsTyped();
+    TIntermTyped* coord  = lhsAsAggregate->getSequence()[1]->getAsTyped();
+
+    const TLayoutFormat fmt = object->getType().getQualifier().layoutFormat;
+    // We only handle 4 component formats at the moment.
+
+    assert(fmt == ElfRgba32f || fmt == ElfRgba32i || fmt == ElfRgba32ui);
+    const TType objDerefType(object->getType().getSampler().type, EvqTemporary, 4);
+
+    if (nodeAsBinary) {
+        TIntermTyped* rhs = nodeAsBinary->getRight();
+        const TOperator assignOp = nodeAsBinary->getOp();
+
+        bool isModifyOp = false;
+
+        switch (assignOp) {
+        case EOpAddAssign:
+        case EOpSubAssign:
+        case EOpMulAssign:
+        case EOpVectorTimesMatrixAssign:
+        case EOpVectorTimesScalarAssign:
+        case EOpMatrixTimesScalarAssign:
+        case EOpMatrixTimesMatrixAssign:
+        case EOpDivAssign:
+        case EOpModAssign:
+        case EOpAndAssign:
+        case EOpInclusiveOrAssign:
+        case EOpExclusiveOrAssign:
+        case EOpLeftShiftAssign:
+        case EOpRightShiftAssign:
+            isModifyOp = true;
+            // fall through...
+        case EOpAssign:
+            {
+                // Since this is an lvalue, we'll convert an image load to a sequence like this (to still provide the value):
+                //   OpSequence
+                //      OpImageStore(object, lhs, rhs)
+                //      rhs
+                // But if it's not a simple symbol RHS (say, a fn call), we don't want to duplicate the RHS, so we'll convert
+                // instead to this:
+                //   OpSequence
+                //      rhsTmp = rhs
+                //      OpImageStore(object, coord, rhsTmp)
+                //      rhsTmp
+                // If this is a read-modify-write op, like +=, we issue:
+                //   OpSequence
+                //      coordtmp = load's param1
+                //      rhsTmp = OpImageLoad(object, coordTmp)
+                //      rhsTmp op= rhs
+                //      OpImageStore(object, coordTmp, rhsTmp)
+                //      rhsTmp
+
+                TIntermSymbol* rhsTmp = rhs->getAsSymbolNode();
+                TIntermTyped* coordTmp = coord;
+ 
+                if (rhsTmp == nullptr || isModifyOp) {
+                    rhsTmp = addTmpVar("storeTemp", objDerefType);
+
+                    // Assign storeTemp = rhs
+                    if (isModifyOp) {
+                        // We have to make a temp var for the coordinate, to avoid evaluating it twice.
+                        coordTmp = addTmpVar("coordTemp", coord->getType());
+                        makeAssign(EOpAssign, coordTmp, coord); // coordtmp = load[param1]
+                        makeLoad(rhsTmp, object, coordTmp, objDerefType); // rhsTmp = OpImageLoad(object, coordTmp)
+                    }
+
+                    // rhsTmp op= rhs.
+                    makeAssign(assignOp, intermediate.addSymbol(*rhsTmp), rhs);
+                }
+                
+                makeStore(object, coordTmp, rhsTmp);         // add a store
+                return finishSequence(rhsTmp, objDerefType); // return rhsTmp from sequence
+            }
+
+        default:
+            break;
+        }
+    }
+
+    if (nodeAsUnary) {
+        const TOperator assignOp = nodeAsUnary->getOp();
+
+        switch (assignOp) {
+        case EOpPreIncrement:
+        case EOpPreDecrement:
+            {
+                // We turn this into:
+                //   OpSequence
+                //      coordtmp = load's param1
+                //      rhsTmp = OpImageLoad(object, coordTmp)
+                //      rhsTmp op
+                //      OpImageStore(object, coordTmp, rhsTmp)
+                //      rhsTmp
+                
+                TIntermSymbol* rhsTmp = addTmpVar("storeTemp", objDerefType);
+                TIntermTyped* coordTmp = addTmpVar("coordTemp", coord->getType());
+ 
+                makeAssign(EOpAssign, coordTmp, coord);           // coordtmp = load[param1]
+                makeLoad(rhsTmp, object, coordTmp, objDerefType); // rhsTmp = OpImageLoad(object, coordTmp)
+                addUnary(assignOp, rhsTmp);                       // op rhsTmp
+                makeStore(object, coordTmp, rhsTmp);              // OpImageStore(object, coordTmp, rhsTmp)
+                return finishSequence(rhsTmp, objDerefType);      // return rhsTmp from sequence
+            }
+
+        case EOpPostIncrement:
+        case EOpPostDecrement:
+            {
+                // We turn this into:
+                //   OpSequence
+                //      coordtmp = load's param1
+                //      rhsTmp1 = OpImageLoad(object, coordTmp)
+                //      rhsTmp2 = rhsTmp1
+                //      rhsTmp2 op
+                //      OpImageStore(object, coordTmp, rhsTmp2)
+                //      rhsTmp1 (pre-op value)
+                TIntermSymbol* rhsTmp1 = addTmpVar("storeTempPre",  objDerefType);
+                TIntermSymbol* rhsTmp2 = addTmpVar("storeTempPost", objDerefType);
+                TIntermTyped* coordTmp = addTmpVar("coordTemp", coord->getType());
+
+                makeAssign(EOpAssign, coordTmp, coord);            // coordtmp = load[param1]
+                makeLoad(rhsTmp1, object, coordTmp, objDerefType); // rhsTmp1 = OpImageLoad(object, coordTmp)
+                makeAssign(EOpAssign, rhsTmp2, rhsTmp1);           // rhsTmp2 = rhsTmp1
+                addUnary(assignOp, rhsTmp2);                       // rhsTmp op
+                makeStore(object, coordTmp, rhsTmp2);              // OpImageStore(object, coordTmp, rhsTmp2)
+                return finishSequence(rhsTmp1, objDerefType);      // return rhsTmp from sequence
+
+                break;
+            }
+                
+        default:
+            break;
+        }
+    }
+
+    // TODO:
+    // if (lhs)
+    //     if (lValueErrorCheck(loc, op, lhs))
+    //         return nullptr;
+ 
+    return node;
+}
+
 void HlslParseContext::handlePragma(const TSourceLoc& loc, const TVector<TString>& tokens)
 {
     if (pragmaCallback)
@@ -965,6 +1208,9 @@ void HlslParseContext::handleFunctionArgument(TFunction* function, TIntermTyped*
 // to intermediate.addAssign().
 TIntermTyped* HlslParseContext::handleAssign(const TSourceLoc& loc, TOperator op, TIntermTyped* left, TIntermTyped* right) const
 {
+    if (left == nullptr || right == nullptr)
+        return nullptr;
+
     const auto mustFlatten = [&](const TIntermTyped& node) {
         return shouldFlatten(node.getType()) && node.getAsSymbolNode() &&
                flattenMap.find(node.getAsSymbolNode()->getId()) != flattenMap.end();
@@ -2288,12 +2534,13 @@ void HlslParseContext::addInputArgumentConversions(const TFunction& function, TI
 //
 // Returns a node of a subtree that evaluates to the return value of the function.
 //
-TIntermTyped* HlslParseContext::addOutputArgumentConversions(const TFunction& function, TIntermAggregate& intermNode) const
+TIntermTyped* HlslParseContext::addOutputArgumentConversions(const TFunction& function, TIntermAggregate& intermNode)
 {
     TIntermSequence& arguments = intermNode.getSequence();
     const auto needsConversion = [&](int argNum) {
         return function[argNum].type->getQualifier().isParamOutput() &&
                (*function[argNum].type != arguments[argNum]->getAsTyped()->getType() ||
+                shouldConvertLValue(arguments[argNum]) ||
                 shouldFlatten(arguments[argNum]->getAsTyped()->getType()));
     };
 
@@ -2341,7 +2588,8 @@ TIntermTyped* HlslParseContext::addOutputArgumentConversions(const TFunction& fu
             TIntermSymbol* tempArgNode = intermediate.addSymbol(*tempArg, intermNode.getLoc());
 
             // This makes the deepest level, the member-wise copy
-            TIntermTyped* tempAssign = handleAssign(arguments[i]->getLoc(), EOpAssign, arguments[i]->getAsTyped(), tempArgNode)->getAsAggregate();
+            TIntermTyped* tempAssign = handleAssign(arguments[i]->getLoc(), EOpAssign, arguments[i]->getAsTyped(), tempArgNode);
+            tempAssign = handleLvalue(arguments[i]->getLoc(), "assign", tempAssign);
             conversionTree = intermediate.growAggregate(conversionTree, tempAssign, arguments[i]->getLoc());
 
             // replace the argument with another node for the same tempArg variable

hlsl/hlslParseHelper.h

@@ -81,7 +81,7 @@ public:
     void decomposeSampleMethods(const TSourceLoc&, TIntermTyped*& node, TIntermNode* arguments);
     TIntermTyped* handleLengthMethod(const TSourceLoc&, TFunction*, TIntermNode*);
     void addInputArgumentConversions(const TFunction&, TIntermNode*&) const;
-    TIntermTyped* addOutputArgumentConversions(const TFunction&, TIntermAggregate&) const;
+    TIntermTyped* addOutputArgumentConversions(const TFunction&, TIntermAggregate&);
     void builtInOpCheck(const TSourceLoc&, const TFunction&, TIntermOperator&);
     TFunction* handleConstructorCall(const TSourceLoc&, const TType&);
     void handleSemantic(TSourceLoc, TQualifier&, const TString& semantic);
@@ -152,6 +152,9 @@ public:
     void pushSwitchSequence(TIntermSequence* sequence) { switchSequenceStack.push_back(sequence); }
     void popSwitchSequence() { switchSequenceStack.pop_back(); }
 
+    // Apply L-value conversions.  E.g, turning a write to a RWTexture into an ImageStore.
+    TIntermTyped* handleLvalue(const TSourceLoc&, const char* op, TIntermTyped* node);
+
 protected:
     void inheritGlobalDefaults(TQualifier& dst) const;
     TVariable* makeInternalVariable(const char* name, const TType&) const;
@@ -161,6 +164,9 @@ protected:
     TIntermTyped* convertInitializerList(const TSourceLoc&, const TType&, TIntermTyped* initializer);
     TOperator mapAtomicOp(const TSourceLoc& loc, TOperator op, bool isImage);
 
+    // Return true if this node requires L-value conversion (e.g, to an imageStore).
+    bool shouldConvertLValue(const TIntermNode*) const;
+
     // Array and struct flattening
     bool shouldFlatten(const TType& type) const { return shouldFlattenIO(type) || shouldFlattenUniform(type); }
     TIntermTyped* flattenAccess(TIntermTyped* base, int member);