Skip to content

Commit 3bd5f9c

Browse files
author
Greg Roth
authored
Errors on non-immediate load/gather offsets (#3283)
 Gather operations that take four separate offsets are meant to allow for programmable, non-immediate values. This removes them from the immediate and range validations by giving the immediate versions their own opcodes For these intrinsics and loads, errors are generated when offsets are non-literal or out of range. These largely use the slightly altered validation paths that the sample intrinsics use. Reword error when texture access offsets are not immediates to be more all encompassing and grammatical. Incidentally remove duplicate shaders being used for the validation test from the old directory while identical copies in the validation directory went unused. Redirected validation test to the appropriate copies. This is consistent with the test shader re-org's stated intent Sample operations have a maximum of 3 args, but gather has a maximum of two since it always operates on 2D images. So gather operations only pass in two offset args to the offset validation, which resulted in an invalid access. Rather than adding a specialized condition to evade this, just iterate over the number of elements in the array. For sample it will be 3 and for gather 2 and it will still check for expected undefined args appropriately. For the offset legalization pass, the opcode is used to determine the start and end of the offset args Only produce the loop unroll suggestion when within a loop Base error line on call instruction instead of source of the offset Sort by location in source when possible and remove duplicates Adapt tests to verify and match these changes Fixes #2590 Fixes #2713
1 parent 19360a8 commit 3bd5f9c

23 files changed

Lines changed: 516 additions & 146 deletions

File tree

‎docs/DXIL.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2318,6 +2318,8 @@ ID Name Description
23182318
219 Unpack4x8 unpacks 4 8-bit signed or unsigned values into int32 or int16 vector
23192319
220 Pack4x8 packs vector of 4 signed or unsigned values into a packed datatype, drops or clamps unused bits
23202320
221 IsHelperLane returns true on helper lanes in pixel shaders
2321+
222 TextureGatherImm same as TextureGather, except offsets are limited to immediate values between -8 and 7
2322+
223 TextureGatherCmpImm same as TextureGatherCmp, except offsets are limited to immediate values between -8 and 7
23212323
=== ===================================================== =======================================================================================================================================================================================================================
23222324

23232325

‎include/dxc/DXIL/DxilConstants.h‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,8 @@ namespace DXIL {
593593
// Resources - gather
594594
TextureGather = 73, // gathers the four texels that would be used in a bi-linear filtering operation
595595
TextureGatherCmp = 74, // same as TextureGather, except this instrution performs comparison on texels, similar to SampleCmp
596+
TextureGatherCmpImm = 223, // same as TextureGatherCmp, except offsets are limited to immediate values between -8 and 7
597+
TextureGatherImm = 222, // same as TextureGather, except offsets are limited to immediate values between -8 and 7
596598

597599
// Resources - sample
598600
RenderTargetGetSampleCount = 77, // gets the number of samples for a render target
@@ -718,7 +720,7 @@ namespace DXIL {
718720
NumOpCodes_Dxil_1_5 = 216,
719721
NumOpCodes_Dxil_1_6 = 222,
720722

721-
NumOpCodes = 222 // exclusive last value of enumeration
723+
NumOpCodes = 224 // exclusive last value of enumeration
722724
};
723725
// OPCODE-ENUM:END
724726

@@ -900,6 +902,8 @@ namespace DXIL {
900902
// Resources - gather
901903
TextureGather,
902904
TextureGatherCmp,
905+
TextureGatherCmpImm,
906+
TextureGatherImm,
903907

904908
// Resources - sample
905909
RenderTargetGetSampleCount,
@@ -983,7 +987,7 @@ namespace DXIL {
983987
NumOpClasses_Dxil_1_5 = 143,
984988
NumOpClasses_Dxil_1_6 = 149,
985989

986-
NumOpClasses = 149 // exclusive last value of enumeration
990+
NumOpClasses = 151 // exclusive last value of enumeration
987991
};
988992
// OPCODECLASS-ENUM:END
989993

@@ -1073,8 +1077,7 @@ namespace DXIL {
10731077
const unsigned kTextureGatherCoord3OpIdx = 6;
10741078
const unsigned kTextureGatherOffset0OpIdx = 7;
10751079
const unsigned kTextureGatherOffset1OpIdx = 8;
1076-
const unsigned kTextureGatherOffset2OpIdx = 9;
1077-
const unsigned kTextureGatherChannelOpIdx = 10;
1080+
const unsigned kTextureGatherChannelOpIdx = 9;
10781081
// TextureGatherCmp.
10791082
const unsigned kTextureGatherCmpCmpValOpIdx = 11;
10801083

@@ -1090,6 +1093,11 @@ namespace DXIL {
10901093
const unsigned kTextureSampleOffset2OpIdx = 9;
10911094
const unsigned kTextureSampleClampOpIdx = 10;
10921095

1096+
// TextureLoad.
1097+
const unsigned kTextureLoadOffset0OpIdx = 6;
1098+
const unsigned kTextureLoadOffset1OpIdx = 8;
1099+
const unsigned kTextureLoadOffset2OpIdx = 9;
1100+
10931101
// AtomicBinOp.
10941102
const unsigned kAtomicBinOpHandleOpIdx = 1;
10951103
const unsigned kAtomicBinOpCoord0OpIdx = 3;

‎include/dxc/DXIL/DxilInstructions.h‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7164,5 +7164,106 @@ struct DxilInst_IsHelperLane {
71647164
// Metadata
71657165
bool requiresUniformInputs() const { return false; }
71667166
};
7167+
7168+
/// This instruction same as TextureGather, except offsets are limited to immediate values between -8 and 7
7169+
struct DxilInst_TextureGatherImm {
7170+
llvm::Instruction *Instr;
7171+
// Construction and identification
7172+
DxilInst_TextureGatherImm(llvm::Instruction *pInstr) : Instr(pInstr) {}
7173+
operator bool() const {
7174+
return hlsl::OP::IsDxilOpFuncCallInst(Instr, hlsl::OP::OpCode::TextureGatherImm);
7175+
}
7176+
// Validation support
7177+
bool isAllowed() const { return true; }
7178+
bool isArgumentListValid() const {
7179+
if (10 != llvm::dyn_cast<llvm::CallInst>(Instr)->getNumArgOperands()) return false;
7180+
return true;
7181+
}
7182+
// Metadata
7183+
bool requiresUniformInputs() const { return false; }
7184+
// Operand indexes
7185+
enum OperandIdx {
7186+
arg_srv = 1,
7187+
arg_sampler = 2,
7188+
arg_coord0 = 3,
7189+
arg_coord1 = 4,
7190+
arg_coord2 = 5,
7191+
arg_coord3 = 6,
7192+
arg_offset0 = 7,
7193+
arg_offset1 = 8,
7194+
arg_channel = 9,
7195+
};
7196+
// Accessors
7197+
llvm::Value *get_srv() const { return Instr->getOperand(1); }
7198+
void set_srv(llvm::Value *val) { Instr->setOperand(1, val); }
7199+
llvm::Value *get_sampler() const { return Instr->getOperand(2); }
7200+
void set_sampler(llvm::Value *val) { Instr->setOperand(2, val); }
7201+
llvm::Value *get_coord0() const { return Instr->getOperand(3); }
7202+
void set_coord0(llvm::Value *val) { Instr->setOperand(3, val); }
7203+
llvm::Value *get_coord1() const { return Instr->getOperand(4); }
7204+
void set_coord1(llvm::Value *val) { Instr->setOperand(4, val); }
7205+
llvm::Value *get_coord2() const { return Instr->getOperand(5); }
7206+
void set_coord2(llvm::Value *val) { Instr->setOperand(5, val); }
7207+
llvm::Value *get_coord3() const { return Instr->getOperand(6); }
7208+
void set_coord3(llvm::Value *val) { Instr->setOperand(6, val); }
7209+
llvm::Value *get_offset0() const { return Instr->getOperand(7); }
7210+
void set_offset0(llvm::Value *val) { Instr->setOperand(7, val); }
7211+
llvm::Value *get_offset1() const { return Instr->getOperand(8); }
7212+
void set_offset1(llvm::Value *val) { Instr->setOperand(8, val); }
7213+
llvm::Value *get_channel() const { return Instr->getOperand(9); }
7214+
void set_channel(llvm::Value *val) { Instr->setOperand(9, val); }
7215+
};
7216+
7217+
/// This instruction same as TextureGatherCmp, except offsets are limited to immediate values between -8 and 7
7218+
struct DxilInst_TextureGatherCmpImm {
7219+
llvm::Instruction *Instr;
7220+
// Construction and identification
7221+
DxilInst_TextureGatherCmpImm(llvm::Instruction *pInstr) : Instr(pInstr) {}
7222+
operator bool() const {
7223+
return hlsl::OP::IsDxilOpFuncCallInst(Instr, hlsl::OP::OpCode::TextureGatherCmpImm);
7224+
}
7225+
// Validation support
7226+
bool isAllowed() const { return true; }
7227+
bool isArgumentListValid() const {
7228+
if (11 != llvm::dyn_cast<llvm::CallInst>(Instr)->getNumArgOperands()) return false;
7229+
return true;
7230+
}
7231+
// Metadata
7232+
bool requiresUniformInputs() const { return false; }
7233+
// Operand indexes
7234+
enum OperandIdx {
7235+
arg_srv = 1,
7236+
arg_sampler = 2,
7237+
arg_coord0 = 3,
7238+
arg_coord1 = 4,
7239+
arg_coord2 = 5,
7240+
arg_coord3 = 6,
7241+
arg_offset0 = 7,
7242+
arg_offset1 = 8,
7243+
arg_channel = 9,
7244+
arg_compareVale = 10,
7245+
};
7246+
// Accessors
7247+
llvm::Value *get_srv() const { return Instr->getOperand(1); }
7248+
void set_srv(llvm::Value *val) { Instr->setOperand(1, val); }
7249+
llvm::Value *get_sampler() const { return Instr->getOperand(2); }
7250+
void set_sampler(llvm::Value *val) { Instr->setOperand(2, val); }
7251+
llvm::Value *get_coord0() const { return Instr->getOperand(3); }
7252+
void set_coord0(llvm::Value *val) { Instr->setOperand(3, val); }
7253+
llvm::Value *get_coord1() const { return Instr->getOperand(4); }
7254+
void set_coord1(llvm::Value *val) { Instr->setOperand(4, val); }
7255+
llvm::Value *get_coord2() const { return Instr->getOperand(5); }
7256+
void set_coord2(llvm::Value *val) { Instr->setOperand(5, val); }
7257+
llvm::Value *get_coord3() const { return Instr->getOperand(6); }
7258+
void set_coord3(llvm::Value *val) { Instr->setOperand(6, val); }
7259+
llvm::Value *get_offset0() const { return Instr->getOperand(7); }
7260+
void set_offset0(llvm::Value *val) { Instr->setOperand(7, val); }
7261+
llvm::Value *get_offset1() const { return Instr->getOperand(8); }
7262+
void set_offset1(llvm::Value *val) { Instr->setOperand(8, val); }
7263+
llvm::Value *get_channel() const { return Instr->getOperand(9); }
7264+
void set_channel(llvm::Value *val) { Instr->setOperand(9, val); }
7265+
llvm::Value *get_compareVale() const { return Instr->getOperand(10); }
7266+
void set_compareVale(llvm::Value *val) { Instr->setOperand(10, val); }
7267+
};
71677268
// INSTR-HELPER:END
71687269
} // namespace hlsl

‎lib/DXIL/DxilCounters.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,9 @@ bool CountDxilOp_tex_bias(unsigned op) {
173173
return op == 61;
174174
}
175175
bool CountDxilOp_tex_cmp(unsigned op) {
176-
// Instructions: SampleCmp=64, SampleCmpLevelZero=65, TextureGatherCmp=74
177-
return (64 <= op && op <= 65) || op == 74;
176+
// Instructions: SampleCmp=64, SampleCmpLevelZero=65, TextureGatherCmp=74,
177+
// TextureGatherCmpImm=223
178+
return (64 <= op && op <= 65) || op == 74 || op == 223;
178179
}
179180
bool CountDxilOp_tex_grad(unsigned op) {
180181
// Instructions: SampleGrad=63
@@ -185,8 +186,9 @@ bool CountDxilOp_tex_load(unsigned op) {
185186
return op == 66 || op == 68 || op == 139;
186187
}
187188
bool CountDxilOp_tex_norm(unsigned op) {
188-
// Instructions: Sample=60, SampleLevel=62, TextureGather=73
189-
return op == 60 || op == 62 || op == 73;
189+
// Instructions: Sample=60, SampleLevel=62, TextureGather=73,
190+
// TextureGatherImm=222
191+
return op == 60 || op == 62 || op == 73 || op == 222;
190192
}
191193
bool CountDxilOp_tex_store(unsigned op) {
192194
// Instructions: TextureStore=67, BufferStore=69, RawBufferStore=140,

‎lib/DXIL/DxilOperations.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,10 @@ const OP::OpCodeProperty OP::m_OpCodeProps[(unsigned)OP::OpCode::NumOpCodes] = {
404404

405405
// Helper Lanes void, h, f, d, i1, i8, i16, i32, i64, udt, obj , function attribute
406406
{ OC::IsHelperLane, "IsHelperLane", OCC::IsHelperLane, "isHelperLane", { false, false, false, false, true, false, false, false, false, false, false}, Attribute::ReadOnly, },
407+
408+
// Resources - gather void, h, f, d, i1, i8, i16, i32, i64, udt, obj , function attribute
409+
{ OC::TextureGatherImm, "TextureGatherImm", OCC::TextureGatherImm, "textureGatherImm", { false, true, true, false, false, false, true, true, false, false, false}, Attribute::ReadOnly, },
410+
{ OC::TextureGatherCmpImm, "TextureGatherCmpImm", OCC::TextureGatherCmpImm, "textureGatherCmpImm", { false, true, true, false, false, false, true, true, false, false, false}, Attribute::ReadOnly, },
407411
};
408412
// OPCODE-OLOADS:END
409413

@@ -847,6 +851,11 @@ void OP::GetMinShaderModelAndMask(OpCode C, bool bWithTranslation,
847851
major = 6; minor = 6;
848852
return;
849853
}
854+
// Instructions: TextureGatherImm=222, TextureGatherCmpImm=223
855+
if ((222 <= op && op <= 223)) {
856+
major = 6; minor = 15;
857+
return;
858+
}
850859
// OPCODE-SMMASK:END
851860
}
852861

@@ -1433,6 +1442,10 @@ Function *OP::GetOpFunc(OpCode opCode, Type *pOverloadType) {
14331442

14341443
// Helper Lanes
14351444
case OpCode::IsHelperLane: A(pI1); A(pI32); break;
1445+
1446+
// Resources - gather
1447+
case OpCode::TextureGatherImm: RRT(pETy); A(pI32); A(pRes); A(pRes); A(pF32); A(pF32); A(pF32); A(pF32); A(pI32); A(pI32); A(pI32); break;
1448+
case OpCode::TextureGatherCmpImm: RRT(pETy); A(pI32); A(pRes); A(pRes); A(pF32); A(pF32); A(pF32); A(pF32); A(pI32); A(pI32); A(pI32); A(pF32); break;
14361449
// OPCODE-OLOAD-FUNCS:END
14371450
default: DXASSERT(false, "otherwise unhandled case"); break;
14381451
}
@@ -1705,6 +1718,8 @@ llvm::Type *OP::GetOverloadType(OpCode opCode, llvm::Function *F) {
17051718
case OpCode::TextureGatherCmp:
17061719
case OpCode::RawBufferLoad:
17071720
case OpCode::Unpack4x8:
1721+
case OpCode::TextureGatherImm:
1722+
case OpCode::TextureGatherCmpImm:
17081723
{
17091724
StructType *ST = cast<StructType>(Ty);
17101725
return ST->getElementType(0);

0 commit comments

Comments
 (0)