Skip to content

Emit OpInBoundsAccessChain for GLSL shaders reading arrays/etc #4206

Description

@TheBlueMatt

createAccessChain builds a Op::OpAccessChain even though it appears to be used only in cases where, at least for GLSL/ESSL shaders, it could build an Op::OpInBoundsAccessChain (array, vector, and matrix indexing with out-of-bounds indices are undefined behavior as far as I can tell). This, in turn, results in a few additional (though not that many) optimizations downstream (eg in mesa). Claude seems to think its as easy as the following (plus regenerating some test output), which appears to generate valid GLSL -> SPIR-V at least for the shaders I'm looking at:

diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp
index b65ec306..400a32e9 100644
--- a/SPIRV/SpvBuilder.cpp
+++ b/SPIRV/SpvBuilder.cpp
@@ -3215,8 +3215,11 @@ Id Builder::createAccessChain(StorageClass storageClass, Id base, const std::vec
     Id typeId = getResultingAccessChainType();
     typeId = makePointer(storageClass, typeId);

-    // Make the instruction
-    Instruction* chain = new Instruction(getUniqueId(), typeId, Op::OpAccessChain);
+    // GLSL and ESSL define out-of-bounds indexing as undefined behavior,
+    // so the compiler may assume all access chain indices are in bounds.
+    Op op = (sourceLang == SourceLanguage::GLSL || sourceLang == SourceLanguage::ESSL)
+               ? Op::OpInBoundsAccessChain : Op::OpAccessChain;
+    Instruction* chain = new Instruction(getUniqueId(), typeId, op);
     chain->reserveOperands(offsets.size() + 1);
     chain->addIdOperand(base);
     for (int i = 0; i < (int)offsets.size(); ++i)
diff --git a/SPIRV/SpvPostProcess.cpp b/SPIRV/SpvPostProcess.cpp
index 2d5c3209..2faf6c9a 100644
--- a/SPIRV/SpvPostProcess.cpp
+++ b/SPIRV/SpvPostProcess.cpp
@@ -177,6 +177,7 @@ void Builder::postProcessType(const Instruction& inst, Id typeId)
         }
         break;
     case Op::OpAccessChain:
+    case Op::OpInBoundsAccessChain:
     case Op::OpPtrAccessChain:
         if (isPointerType(typeId))
             break;
@@ -283,7 +284,8 @@ void Builder::postProcess(Instruction& inst)
             // the reference type and any scalar component selection in the accesschain,
             // and this function computes the rest from the SPIR-V Offset decorations.
             Instruction *accessChain = module.getInstruction(inst.getIdOperand(0));
-            if (accessChain->getOpCode() == Op::OpAccessChain) {
+            if (accessChain->getOpCode() == Op::OpAccessChain ||
+                accessChain->getOpCode() == Op::OpInBoundsAccessChain) {
                 const Instruction* base = module.getInstruction(accessChain->getIdOperand(0));
                 // Get the type of the base of the access chain. It must be a pointer type.
                 Id typeId = base->getTypeId();

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions