Address comments

microsoft · Oct 15, 2024 · 4668b5d · 4668b5d
1 parent 6f15e7c
commit 4668b5d
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 10 deletions.
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -11593,12 +11593,7 @@ SpirvEmitter::processIntrinsicAsType(const CallExpr *callExpr) {
   case 3: {
     // Handling Method 6.
     const Expr *arg1 = callExpr->getArg(1);
-    SourceLocation arg1loc = arg1->getExprLoc();
-    SourceRange arg1range = arg1->getSourceRange();
-
     const Expr *arg2 = callExpr->getArg(2);
-    SourceLocation arg2loc = arg2->getExprLoc();
-    SourceRange arg2range = arg2->getSourceRange();
 
     SpirvInstruction *value = doExpr(arg0);
     SpirvInstruction *lowbits = doExpr(arg1);
@@ -11621,10 +11616,8 @@ SpirvEmitter::processIntrinsicAsType(const CallExpr *callExpr) {
                         lowbitsResult, highbitsResult, loc, range);
     }
 
-    spvBuilder.createStore(lowbits, lowbitsResult, arg1loc, arg1range);
-    spvBuilder.createStore(highbits, highbitsResult, arg2loc, arg2range);
-
-    // TODO: handle matrices
+    spvBuilder.createStore(lowbits, lowbitsResult, loc, range);
+    spvBuilder.createStore(highbits, highbitsResult, loc, range);
 
     return nullptr;
   }

diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.h b/tools/clang/lib/SPIRV/SpirvEmitter.h
@@ -1312,16 +1312,39 @@ class SpirvEmitter : public ASTConsumer {
   /// the Vulkan memory model capability has been added to the module.
   bool UpgradeToVulkanMemoryModelIfNeeded(std::vector<uint32_t> *module);
 
-  // TODO: docs
+  // Splits the `value`, which must be a 64-bit scalar, into two 32-bit wide
+  // uints, stored in `lowbits` and `highbits`.
   void splitDouble(SpirvInstruction *value, SpirvInstruction *&lowbits,
                    SpirvInstruction *&highbits, SourceLocation loc,
                    SourceRange range);
 
+  // Splits the value, which must be a vector with element type `elemType` and
+  // size `count`, into two composite values of size `count` and type
+  // `outputType`. The elements are split component-wise: the vector
+  // {0x0123456789abcdef, 0x0123456789abcdef} is split into `lowbits`
+  // {0x89abcdef, 0x89abcdef} and and `highbits` {0x01234567, 0x01234567}.
   void splitDoubleVector(QualType elemType, uint32_t count, QualType outputType,
                          SpirvInstruction *value, SpirvInstruction *&lowbits,
                          SpirvInstruction *&highbits, SourceLocation loc,
                          SourceRange range);
 
+  // Splits the value, which must be a matrix with element type `elemType` and
+  // dimensions `rowCount` and `colCount`, into two composite values of
+  // dimensions `rowCount` and `colCount`. The elements are split
+  // component-wise: the matrix
+  //
+  // { 0x0123456789abcdef, 0x0123456789abcdef,
+  //   0x0123456789abcdef, 0x0123456789abcdef }
+  //
+  // is split into `lowbits`
+  //
+  // { 0x89abcdef, 0x89abcdef,
+  //   0x89abcdef, 0x89abcdef }
+  //
+  //  and `highbits`
+  //
+  // { 0x012345678, 0x012345678,
+  //   0x012345678, 0x012345678 }.
   void splitDoubleMatrix(QualType elemType, uint32_t rowCount,
                          uint32_t colCount, QualType outputType,
                          SpirvInstruction *value, SpirvInstruction *&lowbits,

diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.asuint.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.asuint.hlsl
@@ -180,4 +180,40 @@ void main() {
 // CHECK-NEXT:                          OpStore %lowbits3x2 [[low]]
 // CHECK-NEXT:                          OpStore %highbits3x2 [[high]]
     asuint(value3x2, lowbits3x2, highbits3x2);
+
+    double2x1 value2x1;
+    uint2x1 lowbits2x1;
+    uint2x1 highbits2x1;
+// CHECK-NEXT:      [[value:%[0-9]+]] = OpLoad %v2double %value2x1
+// CHECK-NEXT:     [[value0:%[0-9]+]] = OpCompositeExtract %double [[value]] 0
+// CHECK-NEXT: [[resultVec0:%[0-9]+]] = OpBitcast %v2uint [[value0]]
+// CHECK-NEXT:       [[low0:%[0-9]+]] = OpCompositeExtract %uint [[resultVec0]] 0
+// CHECK-NEXT:      [[high0:%[0-9]+]] = OpCompositeExtract %uint [[resultVec0]] 1
+// CHECK-NEXT:     [[value1:%[0-9]+]] = OpCompositeExtract %double [[value]] 1
+// CHECK-NEXT: [[resultVec1:%[0-9]+]] = OpBitcast %v2uint [[value1]]
+// CHECK-NEXT:       [[low1:%[0-9]+]] = OpCompositeExtract %uint [[resultVec1]] 0
+// CHECK-NEXT:      [[high1:%[0-9]+]] = OpCompositeExtract %uint [[resultVec1]] 1
+// CHECK-NEXT:        [[low:%[0-9]+]] = OpCompositeConstruct %v2uint [[low0]] [[low1]]
+// CHECK-NEXT:       [[high:%[0-9]+]] = OpCompositeConstruct %v2uint [[high0]] [[high1]]
+// CHECK-NEXT:                          OpStore %lowbits2x1 [[low]]
+// CHECK-NEXT:                          OpStore %highbits2x1 [[high]]
+    asuint(value2x1, lowbits2x1, highbits2x1);
+
+    double1x2 value1x2;
+    uint1x2 lowbits1x2;
+    uint1x2 highbits1x2;
+// CHECK-NEXT:      [[value:%[0-9]+]] = OpLoad %v2double %value1x2
+// CHECK-NEXT:     [[value0:%[0-9]+]] = OpCompositeExtract %double [[value]] 0
+// CHECK-NEXT: [[resultVec0:%[0-9]+]] = OpBitcast %v2uint [[value0]]
+// CHECK-NEXT:       [[low0:%[0-9]+]] = OpCompositeExtract %uint [[resultVec0]] 0
+// CHECK-NEXT:      [[high0:%[0-9]+]] = OpCompositeExtract %uint [[resultVec0]] 1
+// CHECK-NEXT:     [[value1:%[0-9]+]] = OpCompositeExtract %double [[value]] 1
+// CHECK-NEXT: [[resultVec1:%[0-9]+]] = OpBitcast %v2uint [[value1]]
+// CHECK-NEXT:       [[low1:%[0-9]+]] = OpCompositeExtract %uint [[resultVec1]] 0
+// CHECK-NEXT:      [[high1:%[0-9]+]] = OpCompositeExtract %uint [[resultVec1]] 1
+// CHECK-NEXT:        [[low:%[0-9]+]] = OpCompositeConstruct %v2uint [[low0]] [[low1]]
+// CHECK-NEXT:       [[high:%[0-9]+]] = OpCompositeConstruct %v2uint [[high0]] [[high1]]
+// CHECK-NEXT:                          OpStore %lowbits1x2 [[low]]
+// CHECK-NEXT:                          OpStore %highbits1x2 [[high]]
+    asuint(value1x2, lowbits1x2, highbits1x2);
 }