spirv-val: Add OpPtrAccessChain Base checks #4965
Conversation
sjfricke
force-pushed
the
sjfricke-ptrac
branch
from
e98081f
to
ec81fa0
Compare
sjfricke
force-pushed
the
sjfricke-ptrac
branch
from
f7474f1
to
b9f29c0
Compare
source/val/validate_memory.cpp
Outdated
| } else if (base_type_storage_class == | ||
| SpvStorageClassPhysicalStorageBuffer) { | ||
| if (_.addressing_model() != SpvAddressingModelPhysicalStorageBuffer64) { |
There was a problem hiding this comment.
This feels like the wrong spot for this check. This probably belongs in the validation of OpTypePointer.
There was a problem hiding this comment.
So the VUID was for a OpPtrAccessChain who's Base's is using PhysicalStorageBuffer so I guess not sure how I would be able to check that in OpTypePointer
There was a problem hiding this comment.
Well in the SPIR-V spec for the physical storage buffer rules it says:
If the addressing model is not PhysicalStorageBuffer64, then the PhysicalStorageBuffer storage class must not be used.
That's why I suggested OpTypePointer. It will catch more cases.
There was a problem hiding this comment.
Ok I see, I agree, I think the VUID-StandaloneSpirv-Base-04707 probably could just be reworded, going to remove this check from this PR (and make a new PR with it in OpTypePointer and have more tests aimmed towards that) and going to remove the VUID label as it will probably get a new one after my purposed spec clarification
sjfricke
force-pushed
the
sjfricke-ptrac
branch
from
b9f29c0
to
1b14ebb
Compare
adds
VUID-StandaloneSpirv-Base-04707and the SPIR-V spec forShadersaying