Implement SPV_NV_bindless_texture related changes #4847
Conversation
| @@ -363,6 +363,7 @@ spv_result_t ModuleLayoutPass(ValidationState_t& _, const Instruction* inst) { | |||
| case kLayoutExtensions: | |||
| case kLayoutExtInstImport: | |||
| case kLayoutMemoryModel: | |||
| case kLayoutSamplerImageAddressMode: | |||
There was a problem hiding this comment.
Tests belong in test/val/val_layout_test.cpp
pmistryNV
force-pushed
the
bindless_texture
branch
2 times, most recently
from
8d2b8f3 to
b94e026
Compare
|
@alan-baker I have addressed all of your review comments. Kindly have a look. |
pmistryNV
force-pushed
the
bindless_texture
branch
from
b94e026 to
0b0d975
Compare
pmistryNV
force-pushed
the
bindless_texture
branch
from
0b0d975 to
fca1e82
Compare
pmistryNV
force-pushed
the
bindless_texture
branch
from
14bfe17 to
fca1e82
Compare
pmistryNV
force-pushed
the
bindless_texture
branch
from
fca1e82 to
0cc20bb
Compare
|
@alan-baker CI builds are failing, but I am not able to access the failure logs. For example for CI-ubuntu-clang-release "http://fusion/runanalysis/info/prod%3Agraphics_shader_compiler%2Fspirv-tools%2Flinux-clang-release%2Fpresubmit/prod%3Agraphics_shader_compiler%2Fspirv-tools%2Flinux-clang-release%2Fpresubmit/KOKORO/8653d6e5-37d9-4c08-9dd8-d1c8d9590c5b/0/prod%3Agraphics_shader_compiler%2Fspirv-tools%2Flinux-clang-release%2Fpresubmit/Targets" this link doesn't work for me. How do I access it? |
| @@ -483,6 +483,22 @@ spv_result_t InstructionPass(ValidationState_t& _, const Instruction* inst) { | |||
| if (auto error = LimitCheckNumVars(_, inst->id(), storage_class)) { | |||
| return error; | |||
| } | |||
| } else if (opcode == SpvOpSamplerImageAddressingModeNV) { | |||
There was a problem hiding this comment.
Should this also check whether another SpvOpSamplerImageAddressingModeNV instruction has been seen? The extension isn't explicit, but I assume only a single instance of this instruction can be specified.
There was a problem hiding this comment.
yes only one is allowed. I can add a check
| "SPV_NV_bindless_texture"; | ||
| } | ||
| uint32_t bitwidth = inst->GetOperandAs<uint32_t>(0); | ||
| if (_.samplerimage_variable_address_mode() != 0) { |
There was a problem hiding this comment.
@alan-baker this is where it checks to ensure OpSamplerImageAddressingModeNV is called only once. Is this enough?
There was a problem hiding this comment.
I think so, can you just cover it with a test?
There was a problem hiding this comment.
added a test "NVBindlessAddressModeFromLayoutSpecifiedTwice" in val_layout_test.cpp
pmistryNV
force-pushed
the
bindless_texture
branch
from
0cc20bb to
2cb6b38
Compare
@alan-baker kindly let me know where should I add test cases for this extension?
Note: Previous PR #4597 was closed. I have addressed all the comments except adding test cases.