Skip to content

Commit 1500f12

Browse files
authored
[SPIR-V] Fix OpBitFieldExtract emulation for unsigned integer (microsoft#7167)
For an unsigned integer, we should OpShiftRightLogical instead of OpShiftRightArithmetic. Otherwise the value may be extended incorrectly. For example: ``` struct S { uint16_t m1 : 8; uint16_t m2 : 8; }; void main() { S s; s.m1 = 0xff; s.m2 = 0xff; uint16_t result = s.m1; } ``` If we do OpShiftRightArithmetic here, the result will be 0xffff.
1 parent 2eeb47e commit 1500f12

File tree

2 files changed

+12
-8
lines changed

2 files changed

+12
-8
lines changed

tools/clang/lib/SPIRV/SpirvBuilder.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -978,11 +978,15 @@ SpirvInstruction *SpirvBuilder::createEmulatedBitFieldExtract(
978978
base, leftShiftOffset, loc, range);
979979

980980
// input: BBBBCCCCCCCCDDDD0000
981-
// output: SSSSSSSSSSSSSSSSBBBB
981+
// output: SSSSSSSSSSSSSSSSBBBB for signed
982+
// 0000000000000000BBBB for unsigned
983+
auto rightShiftOp = resultType->isSignedIntegerOrEnumerationType()
984+
? spv::Op::OpShiftRightArithmetic
985+
: spv::Op::OpShiftRightLogical;
982986
auto *rightShiftOffset = getConstantInt(
983987
astContext.UnsignedIntTy, llvm::APInt(32, baseTypeBitwidth - bitCount));
984-
auto *rightShift = createBinaryOp(spv::Op::OpShiftRightArithmetic, resultType,
985-
leftShift, rightShiftOffset, loc, range);
988+
auto *rightShift = createBinaryOp(rightShiftOp, resultType, leftShift,
989+
rightShiftOffset, loc, range);
986990

987991
if (resultType == QualType({})) {
988992
auto baseType = dyn_cast<IntegerType>(base->getResultType());

tools/clang/test/CodeGenSPIRV/op.struct.access.bitfield.sized.read.hlsl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ void main() {
3232
// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s1 %int_0
3333
// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
3434
// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_32
35-
// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_32
35+
// CHECK: [[out:%[0-9]+]] = OpShiftRightLogical %ulong [[tmp]] %uint_32
3636
// CHECK: OpStore %vulong [[out]]
3737
vulong = s1.f2;
3838
// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s1 %int_0
3939
// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
4040
// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_31
41-
// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_63
41+
// CHECK: [[out:%[0-9]+]] = OpShiftRightLogical %ulong [[tmp]] %uint_63
4242
// CHECK: OpStore %vulong [[out]]
4343

4444
S2 s2;
@@ -60,20 +60,20 @@ void main() {
6060
// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s3 %int_0
6161
// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
6262
// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_19
63-
// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_19
63+
// CHECK: [[out:%[0-9]+]] = OpShiftRightLogical %ulong [[tmp]] %uint_19
6464
// CHECK: OpStore %vulong [[out]]
6565
vulong = s3.f2;
6666
// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ulong %s3 %int_0
6767
// CHECK: [[raw:%[0-9]+]] = OpLoad %ulong [[ptr]]
6868
// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ulong [[raw]] %uint_9
69-
// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ulong [[tmp]] %uint_54
69+
// CHECK: [[out:%[0-9]+]] = OpShiftRightLogical %ulong [[tmp]] %uint_54
7070
// CHECK: OpStore %vulong [[out]]
7171

7272
vushort = s3.f3;
7373
// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_ushort %s3 %int_1
7474
// CHECK: [[raw:%[0-9]+]] = OpLoad %ushort [[ptr]]
7575
// CHECK: [[tmp:%[0-9]+]] = OpShiftLeftLogical %ushort [[raw]] %uint_9
76-
// CHECK: [[out:%[0-9]+]] = OpShiftRightArithmetic %ushort [[tmp]] %uint_9
76+
// CHECK: [[out:%[0-9]+]] = OpShiftRightLogical %ushort [[tmp]] %uint_9
7777
// CHECK: OpStore %vushort [[out]]
7878

7979
vuint = s3.f4;

0 commit comments

Comments
 (0)