[SPIR-V] Emulate OpBitFieldExtract for type != i32 (#6500)

Keenuts · web-flow · commit dda80a98c492 · 2024-04-18T13:36:44.000+02:00
This is a follow up to #6491 which adds emulation for OpBitField*Extract This is required because Vulkan requires OpBitField*Extract operands to be 32-bit integers. Fixes #6327 --------- Signed-off-by: Nathan Gauër <brioche@google.com>
diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h
@@ -451,11 +451,10 @@ class SpirvBuilder {
 
   /// \brief Creates an OpBitFieldUExtract or OpBitFieldSExtract SPIR-V
   /// instruction for the given arguments.
-  SpirvBitFieldExtract *createBitFieldExtract(QualType resultType,
-                                              SpirvInstruction *base,
-                                              SpirvInstruction *offset,
-                                              SpirvInstruction *count,
-                                              bool isSigned, SourceLocation);
+  SpirvInstruction *createBitFieldExtract(QualType resultType,
+                                          SpirvInstruction *base,
+                                          unsigned bitOffset, unsigned bitCount,
+                                          SourceLocation, SourceRange);
 
   /// \brief Creates an OpEmitVertex instruction.
   void createEmitVertex(SourceLocation, SourceRange range = {});
@@ -840,6 +839,12 @@ class SpirvBuilder {
                                unsigned bitOffset, unsigned bitCount,
                                SourceLocation, SourceRange);
 
+  SpirvInstruction *
+  createEmulatedBitFieldExtract(QualType resultType, uint32_t baseTypeBitwidth,
+                                SpirvInstruction *base, unsigned bitOffset,
+                                unsigned bitCount, SourceLocation loc,
+                                SourceRange range);
+
 private:
   ASTContext &astContext;
   SpirvContext &context; ///< From which we allocate various SPIR-V object
diff --git a/tools/clang/include/clang/SPIRV/SpirvInstruction.h b/tools/clang/include/clang/SPIRV/SpirvInstruction.h
@@ -1141,7 +1141,7 @@ class SpirvBitFieldExtract : public SpirvBitField {
 public:
   SpirvBitFieldExtract(QualType resultType, SourceLocation loc,
                        SpirvInstruction *base, SpirvInstruction *offset,
-                       SpirvInstruction *count, bool isSigned);
+                       SpirvInstruction *count);
 
   DEFINE_RELEASE_MEMORY_FOR_CLASS(SpirvBitFieldExtract)
 
@@ -1151,10 +1151,6 @@ class SpirvBitFieldExtract : public SpirvBitField {
   }
 
   bool invokeVisitor(Visitor *v) override;
-
-  uint32_t isSigned() const {
-    return getopcode() == spv::Op::OpBitFieldSExtract;
-  }
 };
 
 class SpirvBitFieldInsert : public SpirvBitField {
diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp
@@ -226,17 +226,9 @@ SpirvInstruction *SpirvBuilder::createLoad(QualType resultType,
   if (!bitfieldInfo.hasValue())
     return instruction;
 
-  auto *offset = getConstantInt(
-      astContext.UnsignedIntTy,
-      llvm::APInt(32, static_cast<uint64_t>(bitfieldInfo->offsetInBits),
-                  /* isSigned= */ false));
-  auto *count = getConstantInt(
-      astContext.UnsignedIntTy,
-      llvm::APInt(32, static_cast<uint64_t>(bitfieldInfo->sizeInBits),
-                  /* isSigned= */ false));
-  return createBitFieldExtract(
-      resultType, instruction, offset, count,
-      pointer->getAstResultType()->isSignedIntegerOrEnumerationType(), loc);
+  return createBitFieldExtract(resultType, instruction,
+                               bitfieldInfo->offsetInBits,
+                               bitfieldInfo->sizeInBits, loc, range);
 }
 
 SpirvCopyObject *SpirvBuilder::createCopyObject(QualType resultType,
@@ -980,12 +972,71 @@ SpirvBuilder::createBitFieldInsert(QualType resultType, SpirvInstruction *base,
   return inst;
 }
 
-SpirvBitFieldExtract *SpirvBuilder::createBitFieldExtract(
-    QualType resultType, SpirvInstruction *base, SpirvInstruction *offset,
-    SpirvInstruction *count, bool isSigned, SourceLocation loc) {
+SpirvInstruction *SpirvBuilder::createEmulatedBitFieldExtract(
+    QualType resultType, uint32_t baseTypeBitwidth, SpirvInstruction *base,
+    unsigned bitOffset, unsigned bitCount, SourceLocation loc,
+    SourceRange range) {
+  assert(bitCount <= 64 &&
+         "Bitfield extraction emulation can only extract at most 64 bits.");
+
+  // The base is a raw struct field, which can contain several bitfields:
+  //   raw field: AAAABBBBCCCCCCCCDDDD
+  // Extracting B means shifting it right until B's LSB is the basetype LSB.
+  // But first, we need to left shift until B's MSB becomes the basetype MSB:
+  //   - is B is signed, its sign bits won't necessarily extend up to the
+  //   basetype MSB.
+  //   - meaning a right-shift could fail to sign-extend.
+  //   - shifting left first, then right makes sure the sign extension happens.
+
+  //  input: AAAABBBBCCCCCCCCDDDD
+  // output: BBBBCCCCCCCCDDDD0000
+  auto *leftShiftOffset =
+      getConstantInt(astContext.UnsignedIntTy,
+                     llvm::APInt(32, baseTypeBitwidth - bitOffset - bitCount));
+  auto *leftShift = createBinaryOp(spv::Op::OpShiftLeftLogical, resultType,
+                                   base, leftShiftOffset, loc, range);
+
+  //  input: BBBBCCCCCCCCDDDD0000
+  // output: SSSSSSSSSSSSSSSSBBBB
+  auto *rightShiftOffset = getConstantInt(
+      astContext.UnsignedIntTy, llvm::APInt(32, baseTypeBitwidth - bitCount));
+  auto *rightShift = createBinaryOp(spv::Op::OpShiftRightArithmetic, resultType,
+                                    leftShift, rightShiftOffset, loc, range);
+
+  if (resultType == QualType({})) {
+    auto baseType = dyn_cast<IntegerType>(base->getResultType());
+    leftShift->setResultType(baseType);
+    rightShift->setResultType(baseType);
+  }
+
+  return rightShift;
+}
+
+SpirvInstruction *
+SpirvBuilder::createBitFieldExtract(QualType resultType, SpirvInstruction *base,
+                                    unsigned bitOffset, unsigned bitCount,
+                                    SourceLocation loc, SourceRange range) {
   assert(insertPoint && "null insert point");
-  auto *inst = new (context)
-      SpirvBitFieldExtract(resultType, loc, base, offset, count, isSigned);
+
+  uint32_t bitWidth = 0;
+  if (resultType == QualType({})) {
+    assert(base->hasResultType() && "No type information for bitfield.");
+    bitWidth = dyn_cast<IntegerType>(base->getResultType())->getBitwidth();
+  } else {
+    bitWidth = getElementSpirvBitwidth(astContext, resultType,
+                                       spirvOptions.enable16BitTypes);
+  }
+
+  if (bitWidth != 32)
+    return createEmulatedBitFieldExtract(resultType, bitWidth, base, bitOffset,
+                                         bitCount, loc, range);
+
+  auto *offset =
+      getConstantInt(astContext.UnsignedIntTy, llvm::APInt(32, bitOffset));
+  auto *count =
+      getConstantInt(astContext.UnsignedIntTy, llvm::APInt(32, bitCount));
+  auto *inst =
+      new (context) SpirvBitFieldExtract(resultType, loc, base, offset, count);
   insertPoint->addInstruction(inst);
   inst->setRValue(true);
   return inst;
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -8298,14 +8298,9 @@ SpirvInstruction *SpirvEmitter::castToInt(SpirvInstruction *fromVal,
       firstField = spvBuilder.createCompositeExtract(fieldType, fromVal, {0},
                                                      srcLoc, srcRange);
       if (fieldDecl->isBitField()) {
-        auto offset = spvBuilder.getConstantInt(astContext.UnsignedIntTy,
-                                                llvm::APInt(32, 0));
-        auto width = spvBuilder.getConstantInt(
-            astContext.UnsignedIntTy,
-            llvm::APInt(32, fieldDecl->getBitWidthValue(astContext)));
         firstField = spvBuilder.createBitFieldExtract(
-            fieldType, firstField, offset, width,
-            toIntType->hasSignedIntegerRepresentation(), srcLoc);
+            fieldType, firstField, 0, fieldDecl->getBitWidthValue(astContext),
+            srcLoc, srcRange);
       }
     }
 
@@ -9367,10 +9362,7 @@ SpirvEmitter::processIntrinsicMsad4(const CallExpr *callExpr) {
   llvm::SmallVector<SpirvInstruction *, 4> isRefByteZero;
   for (uint32_t i = 0; i < 4; ++i) {
     refBytes.push_back(spvBuilder.createBitFieldExtract(
-        uintType, reference, /*offset*/
-        spvBuilder.getConstantInt(astContext.UnsignedIntTy,
-                                  llvm::APInt(32, i * 8)),
-        /*count*/ uint8, /*isSigned*/ false, loc));
+        uintType, reference, /*offset*/ i * 8, /*count*/ 8, loc, range));
     signedRefBytes.push_back(spvBuilder.createUnaryOp(
         spv::Op::OpBitcast, intType, refBytes.back(), loc));
     isRefByteZero.push_back(spvBuilder.createBinaryOp(
@@ -9381,11 +9373,8 @@ SpirvEmitter::processIntrinsicMsad4(const CallExpr *callExpr) {
     for (uint32_t byteCount = 0; byteCount < 4; ++byteCount) {
       // 'count' is always 8 because we are extracting 8 bits out of 32.
       auto *srcByte = spvBuilder.createBitFieldExtract(
-          uintType, sources[msadNum],
-          /*offset*/
-          spvBuilder.getConstantInt(astContext.UnsignedIntTy,
-                                    llvm::APInt(32, 8 * byteCount)),
-          /*count*/ uint8, /*isSigned*/ false, loc);
+          uintType, sources[msadNum], /*offset*/ 8 * byteCount, /*count*/ 8,
+          loc, range);
       auto *signedSrcByte =
           spvBuilder.createUnaryOp(spv::Op::OpBitcast, intType, srcByte, loc);
       auto *sub = spvBuilder.createBinaryOp(spv::Op::OpISub, intType,
diff --git a/tools/clang/lib/SPIRV/SpirvInstruction.cpp b/tools/clang/lib/SPIRV/SpirvInstruction.cpp
@@ -474,12 +474,15 @@ SpirvBitField::SpirvBitField(Kind kind, spv::Op op, QualType resultType,
     : SpirvInstruction(kind, op, resultType, loc), base(baseInst),
       offset(offsetInst), count(countInst) {}
 
-SpirvBitFieldExtract::SpirvBitFieldExtract(
-    QualType resultType, SourceLocation loc, SpirvInstruction *baseInst,
-    SpirvInstruction *offsetInst, SpirvInstruction *countInst, bool isSigned)
+SpirvBitFieldExtract::SpirvBitFieldExtract(QualType resultType,
+                                           SourceLocation loc,
+                                           SpirvInstruction *baseInst,
+                                           SpirvInstruction *offsetInst,
+                                           SpirvInstruction *countInst)
     : SpirvBitField(IK_BitFieldExtract,
-                    isSigned ? spv::Op::OpBitFieldSExtract
-                             : spv::Op::OpBitFieldUExtract,
+                    resultType->isSignedIntegerOrEnumerationType()
+                        ? spv::Op::OpBitFieldSExtract
+                        : spv::Op::OpBitFieldUExtract,
                     resultType, loc, baseInst, offsetInst, countInst) {}
 
 SpirvBitFieldInsert::SpirvBitFieldInsert(QualType resultType,
diff --git a/tools/clang/test/CodeGenSPIRV/op.struct.access.bitfield.sized.read.hlsl b/tools/clang/test/CodeGenSPIRV/op.struct.access.bitfield.sized.read.hlsl
@@ -0,0 +1,84 @@
+// RUN: %dxc -T cs_6_2 -E main -spirv -fcgl -enable-16bit-types %s | FileCheck %s
+
+struct S1 {
+  uint64_t f1 : 32;
+  uint64_t f2 : 1;
+};
+// CHECK-DAG: %S1 = OpTypeStruct %ulong
+
+struct S2 {
+  int64_t f1 : 32;
+  int64_t f2 : 1;
+};
+// CHECK-DAG: %S2 = OpTypeStruct %long
+
+struct S3 {
+  uint64_t f1 : 45;
+  uint64_t f2 : 10;
+  uint16_t f3 : 7;
+  uint32_t f4 : 5;
+};
+// CHECK-DAG: %S3 = OpTypeStruct %ulong %ushort %uint
+
+[numthreads(1, 1, 1)]
+void main() {
+  uint64_t vulong;
+  uint32_t vuint;
+  uint16_t vushort;
+  int64_t vlong;
+
+  S1 s1;
+  vulong = s1.f1;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s1 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_32
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_32
+// CHECK:                   OpStore %vulong [[out]]
+  vulong = s1.f2;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s1 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_31
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_63
+// CHECK:                   OpStore %vulong [[out]]
+
+  S2 s2;
+  vlong = s2.f1;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_long %s2 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %long [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %long [[raw]] %uint_32
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %long [[tmp]] %uint_32
+// CHECK:                   OpStore %vlong [[out]]
+  vlong = s2.f2;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_long %s2 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %long [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %long [[raw]] %uint_31
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %long [[tmp]] %uint_63
+// CHECK:                   OpStore %vlong [[out]]
+
+  S3 s3;
+  vulong = s3.f1;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s3 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_19
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_19
+// CHECK:                   OpStore %vulong [[out]]
+  vulong = s3.f2;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s3 %int_0
+// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_9
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_54
+// CHECK:                   OpStore %vulong [[out]]
+
+  vushort = s3.f3;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ushort %s3 %int_1
+// CHECK: [[raw:%[0-9]+]] = OpLoad %ushort [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ushort [[raw]] %uint_9
+// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ushort [[tmp]] %uint_9
+// CHECK:                   OpStore %vushort [[out]]
+
+  vuint = s3.f4;
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_uint %s3 %int_2
+// CHECK: [[raw:%[0-9]+]] = OpLoad %uint [[ptr]]
+// CHECK: [[tmp:%[0-9]+]] = OpBitFieldUExtract %uint [[raw]] %uint_0 %uint_5
+// CHECK:                   OpStore %vuint [[tmp]]
+}
