Ver código fonte

Updated spirv-tools.

Бранимир Караџић 6 anos atrás
pai
commit
aaa1cc01b2

+ 1 - 1
3rdparty/spirv-tools/include/generated/build-version.inc

@@ -1 +1 @@
-"v2019.5-dev", "SPIRV-Tools v2019.5-dev v2019.4-186-gb334829a"
+"v2019.5-dev", "SPIRV-Tools v2019.5-dev v2019.4-192-g983b5b4f"

+ 10 - 0
3rdparty/spirv-tools/include/spirv-tools/optimizer.hpp

@@ -827,6 +827,16 @@ Optimizer::PassToken CreateSplitInvalidUnreachablePass();
 //   of range. (The module is already invalid if that is the case.)
 // - TODO(dneto): The OpImageTexelPointer coordinate component is not 32-bits
 // wide.
+//
+// NOTE: Access chain indices are always treated as signed integers.  So
+//   if an array has a fixed size of more than 2^31 elements, then elements
+//   from 2^31 and above are never accessible with a 32-bit index,
+//   signed or unsigned.  For this case, this pass will clamp the index
+//   between 0 and at 2^31-1, inclusive.
+//   Similarly, if an array has more then 2^15 element and is accessed with
+//   a 16-bit index, then elements from 2^15 and above are not accessible.
+//   In this case, the pass will clamp the index between 0 and 2^15-1
+//   inclusive.
 Optimizer::PassToken CreateGraphicsRobustAccessPass();
 
 // Create descriptor scalar replacement pass.

+ 15 - 102
3rdparty/spirv-tools/source/fuzz/fuzzer_util.cpp

@@ -14,6 +14,8 @@
 
 #include "source/fuzz/fuzzer_util.h"
 
+#include "source/opt/build_module.h"
+
 namespace spvtools {
 namespace fuzz {
 
@@ -180,108 +182,6 @@ opt::BasicBlock::iterator GetIteratorForInstruction(
   return block->end();
 }
 
-bool NewEdgeRespectsUseDefDominance(opt::IRContext* context,
-                                    opt::BasicBlock* bb_from,
-                                    opt::BasicBlock* bb_to) {
-  assert(bb_from->terminator()->opcode() == SpvOpBranch);
-
-  // If there is *already* an edge from |bb_from| to |bb_to|, then adding
-  // another edge is fine from a dominance point of view.
-  if (bb_from->terminator()->GetSingleWordInOperand(0) == bb_to->id()) {
-    return true;
-  }
-
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/2919): the
-  //  solution below to determining whether a new edge respects dominance
-  //  rules is incomplete.  Test
-  //  TransformationAddDeadContinueTest::DISABLED_Miscellaneous6 exposes the
-  //  problem.  In practice, this limitation does not bite too often, and the
-  //  worst it does is leads to SPIR-V that spirv-val rejects.
-
-  // Let us assume that the module being manipulated is valid according to the
-  // rules of the SPIR-V language.
-  //
-  // Suppose that some block Y is dominated by |bb_to| (which includes the case
-  // where Y = |bb_to|).
-  //
-  // Suppose that Y uses an id i that is defined in some other block X.
-  //
-  // Because the module is valid, X must dominate Y.  We are concerned about
-  // whether an edge from |bb_from| to |bb_to| could *stop* X from dominating
-  // Y.
-  //
-  // Because |bb_to| dominates Y, a new edge from |bb_from| to |bb_to| can
-  // only affect whether X dominates Y if X dominates |bb_to|.
-  //
-  // So let us assume that X does dominate |bb_to|, so that we have:
-  //
-  //   (X defines i) dominates |bb_to| dominates (Y uses i)
-  //
-  // The new edge from |bb_from| to |bb_to| will stop the definition of i in X
-  // from dominating the use of i in Y exactly when the new edge will stop X
-  // from dominating |bb_to|.
-  //
-  // Now, the block X that we are worried about cannot dominate |bb_from|,
-  // because in that case X would still dominate |bb_to| after we add an edge
-  // from |bb_from| to |bb_to|.
-  //
-  // Also, it cannot be that X = |bb_to|, because nothing can stop a block
-  // from dominating itself.
-  //
-  // So we are looking for a block X such that:
-  //
-  // - X strictly dominates |bb_to|
-  // - X does not dominate |bb_from|
-  // - X defines an id i
-  // - i is used in some block Y
-  // - |bb_to| dominates Y
-
-  // Walk the dominator tree backwards, starting from the immediate dominator
-  // of |bb_to|.  We can stop when we find a block that also dominates
-  // |bb_from|.
-  auto dominator_analysis = context->GetDominatorAnalysis(bb_from->GetParent());
-  for (auto dominator = dominator_analysis->ImmediateDominator(bb_to);
-       dominator != nullptr &&
-       !dominator_analysis->Dominates(dominator, bb_from);
-       dominator = dominator_analysis->ImmediateDominator(dominator)) {
-    // |dominator| is a candidate for block X in the above description.
-    // We now look through the instructions for a candidate instruction i.
-    for (auto& inst : *dominator) {
-      // Consider all the uses of this instruction.
-      if (!context->get_def_use_mgr()->WhileEachUse(
-              &inst,
-              [bb_to, context, dominator_analysis](
-                  opt::Instruction* user, uint32_t operand_index) -> bool {
-                // If this use is in an OpPhi, we need to check that dominance
-                // of the relevant *parent* block is not spoiled.  Otherwise we
-                // need to check that dominance of the block containing the use
-                // is not spoiled.
-                opt::BasicBlock* use_block_or_phi_parent =
-                    user->opcode() == SpvOpPhi
-                        ? context->cfg()->block(
-                              user->GetSingleWordOperand(operand_index + 1))
-                        : context->get_instr_block(user);
-
-                // There might not be any relevant block, e.g. if the use is in
-                // a decoration; in this case the new edge is unproblematic.
-                if (use_block_or_phi_parent == nullptr) {
-                  return true;
-                }
-
-                // With reference to the above discussion,
-                // |use_block_or_phi_parent| is a candidate for the block Y.
-                // If |bb_to| dominates this block, the new edge would be
-                // problematic.
-                return !dominator_analysis->Dominates(bb_to,
-                                                      use_block_or_phi_parent);
-              })) {
-        return false;
-      }
-    }
-  }
-  return true;
-}
-
 bool BlockIsReachableInItsFunction(opt::IRContext* context,
                                    opt::BasicBlock* bb) {
   auto enclosing_function = bb->GetParent();
@@ -399,6 +299,19 @@ uint32_t GetArraySize(const opt::Instruction& array_type_instruction,
   return array_length_constant->GetU32();
 }
 
+bool IsValid(opt::IRContext* context) {
+  std::vector<uint32_t> binary;
+  context->module()->ToBinary(&binary, false);
+  return SpirvTools(context->grammar().target_env()).Validate(binary);
+}
+
+std::unique_ptr<opt::IRContext> CloneIRContext(opt::IRContext* context) {
+  std::vector<uint32_t> binary;
+  context->module()->ToBinary(&binary, false);
+  return BuildModule(context->grammar().target_env(), nullptr, binary.data(),
+                     binary.size());
+}
+
 }  // namespace fuzzerutil
 
 }  // namespace fuzz

+ 7 - 8
3rdparty/spirv-tools/source/fuzz/fuzzer_util.h

@@ -69,14 +69,6 @@ bool BlockIsInLoopContinueConstruct(opt::IRContext* context, uint32_t block_id,
 opt::BasicBlock::iterator GetIteratorForInstruction(
     opt::BasicBlock* block, const opt::Instruction* inst);
 
-// The function determines whether adding an edge from |bb_from| to |bb_to| -
-// is legitimate with respect to the SPIR-V rule that a definition must
-// dominate all of its uses.  This is because adding such an edge can change
-// dominance in the control flow graph, potentially making the module invalid.
-bool NewEdgeRespectsUseDefDominance(opt::IRContext* context,
-                                    opt::BasicBlock* bb_from,
-                                    opt::BasicBlock* bb_to);
-
 // Returns true if and only if there is a path to |bb| from the entry block of
 // the function that contains |bb|.
 bool BlockIsReachableInItsFunction(opt::IRContext* context,
@@ -117,6 +109,13 @@ uint32_t GetNumberOfStructMembers(
 uint32_t GetArraySize(const opt::Instruction& array_type_instruction,
                       opt::IRContext* context);
 
+// Returns true if and only if |context| is valid, according to the validator.
+bool IsValid(opt::IRContext* context);
+
+// Returns a clone of |context|, by writing |context| to a binary and then
+// parsing it again.
+std::unique_ptr<opt::IRContext> CloneIRContext(opt::IRContext* context);
+
 }  // namespace fuzzerutil
 
 }  // namespace fuzz

+ 21 - 8
3rdparty/spirv-tools/source/fuzz/transformation_add_dead_break.cpp

@@ -175,18 +175,23 @@ bool TransformationAddDeadBreak::IsApplicable(
     return false;
   }
 
-  // Check that adding the break would not violate the property that a
-  // definition must dominate all of its uses.
-  return fuzzerutil::NewEdgeRespectsUseDefDominance(context, bb_from, bb_to);
+  // Adding the dead break is only valid if SPIR-V rules related to dominance
+  // hold.  Rather than checking these rules explicitly, we defer to the
+  // validator.  We make a clone of the module, apply the transformation to the
+  // clone, and check whether the transformed clone is valid.
+  //
+  // In principle some of the above checks could be removed, with more reliance
+  // being places on the validator.  This should be revisited if we are sure
+  // the validator is complete with respect to checking structured control flow
+  // rules.
+  auto cloned_context = fuzzerutil::CloneIRContext(context);
+  ApplyImpl(cloned_context.get());
+  return fuzzerutil::IsValid(cloned_context.get());
 }
 
 void TransformationAddDeadBreak::Apply(opt::IRContext* context,
                                        FactManager* /*unused*/) const {
-  fuzzerutil::AddUnreachableEdgeAndUpdateOpPhis(
-      context, context->cfg()->block(message_.from_block()),
-      context->cfg()->block(message_.to_block()),
-      message_.break_condition_value(), message_.phi_id());
-
+  ApplyImpl(context);
   // Invalidate all analyses
   context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisNone);
 }
@@ -197,5 +202,13 @@ protobufs::Transformation TransformationAddDeadBreak::ToMessage() const {
   return result;
 }
 
+void TransformationAddDeadBreak::ApplyImpl(
+    spvtools::opt::IRContext* context) const {
+  fuzzerutil::AddUnreachableEdgeAndUpdateOpPhis(
+      context, context->cfg()->block(message_.from_block()),
+      context->cfg()->block(message_.to_block()),
+      message_.break_condition_value(), message_.phi_id());
+}
+
 }  // namespace fuzz
 }  // namespace spvtools

+ 8 - 0
3rdparty/spirv-tools/source/fuzz/transformation_add_dead_break.h

@@ -67,6 +67,14 @@ class TransformationAddDeadBreak : public Transformation {
   bool AddingBreakRespectsStructuredControlFlow(opt::IRContext* context,
                                                 opt::BasicBlock* bb_from) const;
 
+  // Used by 'Apply' to actually apply the transformation to the module of
+  // interest, and by 'IsApplicable' to do a dry-run of the transformation on a
+  // cloned module, in order to check that the transformation leads to a valid
+  // module.  This is only invoked by 'IsApplicable' after certain basic
+  // applicability checks have been made, ensuring that the invocation of this
+  // method is legal.
+  void ApplyImpl(opt::IRContext* context) const;
+
   protobufs::TransformationAddDeadBreak message_;
 };
 

+ 30 - 20
3rdparty/spirv-tools/source/fuzz/transformation_add_dead_continue.cpp

@@ -106,42 +106,52 @@ bool TransformationAddDeadContinue::IsApplicable(
     return false;
   }
 
-  // Check that adding the continue would not violate the property that a
-  // definition must dominate all of its uses.
-  if (!fuzzerutil::NewEdgeRespectsUseDefDominance(
-          context, bb_from, context->cfg()->block(continue_block))) {
+  // Check whether the data passed to extend OpPhi instructions is appropriate.
+  if (!fuzzerutil::PhiIdsOkForNewEdge(context, bb_from,
+                                      context->cfg()->block(continue_block),
+                                      message_.phi_id())) {
     return false;
   }
 
-  // The transformation is good if and only if the given phi ids are sufficient
-  // to extend relevant OpPhi instructions in the continue block.
-  return fuzzerutil::PhiIdsOkForNewEdge(context, bb_from,
-                                        context->cfg()->block(continue_block),
-                                        message_.phi_id());
+  // Adding the dead break is only valid if SPIR-V rules related to dominance
+  // hold.  Rather than checking these rules explicitly, we defer to the
+  // validator.  We make a clone of the module, apply the transformation to the
+  // clone, and check whether the transformed clone is valid.
+  //
+  // In principle some of the above checks could be removed, with more reliance
+  // being places on the validator.  This should be revisited if we are sure
+  // the validator is complete with respect to checking structured control flow
+  // rules.
+  auto cloned_context = fuzzerutil::CloneIRContext(context);
+  ApplyImpl(cloned_context.get());
+  return fuzzerutil::IsValid(cloned_context.get());
 }
 
 void TransformationAddDeadContinue::Apply(opt::IRContext* context,
                                           FactManager* /*unused*/) const {
+  ApplyImpl(context);
+  // Invalidate all analyses
+  context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisNone);
+}
+
+protobufs::Transformation TransformationAddDeadContinue::ToMessage() const {
+  protobufs::Transformation result;
+  *result.mutable_add_dead_continue() = message_;
+  return result;
+}
+
+void TransformationAddDeadContinue::ApplyImpl(
+    spvtools::opt::IRContext* context) const {
   auto bb_from = context->cfg()->block(message_.from_block());
   auto continue_block =
       bb_from->IsLoopHeader()
           ? bb_from->ContinueBlockId()
           : context->GetStructuredCFGAnalysis()->LoopContinueBlock(
                 message_.from_block());
-  assert(continue_block &&
-         "Precondition for this transformation requires that "
-         "message_.from_block must be in a loop.");
+  assert(continue_block && "message_.from_block must be in a loop.");
   fuzzerutil::AddUnreachableEdgeAndUpdateOpPhis(
       context, bb_from, context->cfg()->block(continue_block),
       message_.continue_condition_value(), message_.phi_id());
-  // Invalidate all analyses
-  context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisNone);
-}
-
-protobufs::Transformation TransformationAddDeadContinue::ToMessage() const {
-  protobufs::Transformation result;
-  *result.mutable_add_dead_continue() = message_;
-  return result;
 }
 
 }  // namespace fuzz

+ 8 - 0
3rdparty/spirv-tools/source/fuzz/transformation_add_dead_continue.h

@@ -64,6 +64,14 @@ class TransformationAddDeadContinue : public Transformation {
   protobufs::Transformation ToMessage() const override;
 
  private:
+  // Used by 'Apply' to actually apply the transformation to the module of
+  // interest, and by 'IsApplicable' to do a dry-run of the transformation on a
+  // cloned module, in order to check that the transformation leads to a valid
+  // module.  This is only invoked by 'IsApplicable' after certain basic
+  // applicability checks have been made, ensuring that the invocation of this
+  // method is legal.
+  void ApplyImpl(opt::IRContext* context) const;
+
   protobufs::TransformationAddDeadContinue message_;
 };
 

+ 0 - 1
3rdparty/spirv-tools/source/opt/folding_rules.cpp

@@ -553,7 +553,6 @@ uint32_t PerformOperation(analysis::ConstantManager* const_mgr, SpvOp opcode,
                           const analysis::Constant* input1,
                           const analysis::Constant* input2) {
   assert(input1 && input2);
-  assert(input1->type() == input2->type());
   const analysis::Type* type = input1->type();
   std::vector<uint32_t> words;
   if (const analysis::Vector* vector_type = type->AsVector()) {

+ 143 - 57
3rdparty/spirv-tools/source/opt/graphics_robust_access_pass.cpp

@@ -121,6 +121,10 @@
 //      the result of an OpArrayLength instruction acting on the pointer of
 //      the containing structure as noted above.
 //
+//      - Access chain indices are always treated as signed, so:
+//        - Clamp the upper bound at the signed integer maximum.
+//        - Use SClamp for all clamping.
+//
 //    - TODO(dneto): OpImageTexelPointer:
 //      - Clamp coordinate to the image size returned by OpImageQuerySize
 //      - If multi-sampled, clamp the sample index to the count returned by
@@ -141,6 +145,7 @@
 #include <cstring>
 #include <functional>
 #include <initializer_list>
+#include <limits>
 #include <utility>
 
 #include "constants.h"
@@ -246,6 +251,7 @@ bool GraphicsRobustAccessPass::ProcessAFunction(opt::Function* function) {
   }
   for (auto* inst : access_chains) {
     ClampIndicesForAccessChain(inst);
+    if (module_status_.failed) return module_status_.modified;
   }
 
   for (auto* inst : image_texel_pointers) {
@@ -261,6 +267,8 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
   auto* constant_mgr = context()->get_constant_mgr();
   auto* def_use_mgr = context()->get_def_use_mgr();
   auto* type_mgr = context()->get_type_mgr();
+  const bool have_int64_cap =
+      context()->get_feature_mgr()->HasCapability(SpvCapabilityInt64);
 
   // Replaces one of the OpAccessChain index operands with a new value.
   // Updates def-use analysis.
@@ -268,37 +276,66 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
                                             Instruction* new_value) {
     inst.SetOperand(operand_index, {new_value->result_id()});
     def_use_mgr->AnalyzeInstUse(&inst);
+    return SPV_SUCCESS;
   };
 
   // Replaces one of the OpAccesssChain index operands with a clamped value.
   // Replace the operand at |operand_index| with the value computed from
-  // unsigned_clamp(%old_value, %min_value, %max_value).  It also analyzes
+  // signed_clamp(%old_value, %min_value, %max_value).  It also analyzes
   // the new instruction and records that them module is modified.
-  auto clamp_index = [&inst, this, &replace_index](
+  // Assumes %min_value is signed-less-or-equal than %max_value. (All callees
+  // use 0 for %min_value).
+  auto clamp_index = [&inst, type_mgr, this, &replace_index](
                          uint32_t operand_index, Instruction* old_value,
                          Instruction* min_value, Instruction* max_value) {
-    auto* clamp_inst = MakeClampInst(old_value, min_value, max_value, &inst);
-    replace_index(operand_index, clamp_inst);
+    auto* clamp_inst =
+        MakeSClampInst(*type_mgr, old_value, min_value, max_value, &inst);
+    return replace_index(operand_index, clamp_inst);
   };
 
   // Ensures the specified index of access chain |inst| has a value that is
   // at most |count| - 1.  If the index is already a constant value less than
   // |count| then no change is made.
-  auto clamp_to_literal_count = [&inst, this, &constant_mgr, &type_mgr,
-                                 &replace_index, &clamp_index](
-                                    uint32_t operand_index, uint64_t count) {
+  auto clamp_to_literal_count =
+      [&inst, this, &constant_mgr, &type_mgr, have_int64_cap, &replace_index,
+       &clamp_index](uint32_t operand_index, uint64_t count) -> spv_result_t {
     Instruction* index_inst =
         this->GetDef(inst.GetSingleWordOperand(operand_index));
     const auto* index_type =
         type_mgr->GetType(index_inst->type_id())->AsInteger();
     assert(index_type);
+    const auto index_width = index_type->width();
+
     if (count <= 1) {
       // Replace the index with 0.
-      replace_index(operand_index, GetValueForType(0, index_type));
-      return;
+      return replace_index(operand_index, GetValueForType(0, index_type));
     }
 
-    const auto index_width = index_type->width();
+    uint64_t maxval = count - 1;
+
+    // Compute the bit width of a viable type to hold |maxval|.
+    // Look for a bit width, up to 64 bits wide, to fit maxval.
+    uint32_t maxval_width = index_width;
+    while ((maxval_width < 64) && (0 != (maxval >> maxval_width))) {
+      maxval_width *= 2;
+    }
+    // Determine the type for |maxval|.
+    analysis::Integer signed_type_for_query(maxval_width, true);
+    auto* maxval_type =
+        type_mgr->GetRegisteredType(&signed_type_for_query)->AsInteger();
+    // Access chain indices are treated as signed, so limit the maximum value
+    // of the index so it will always be positive for a signed clamp operation.
+    maxval = std::min(maxval, ((uint64_t(1) << (maxval_width - 1)) - 1));
+
+    if (index_width > 64) {
+      return this->Fail() << "Can't handle indices wider than 64 bits, found "
+                             "constant index with "
+                          << index_width << " bits as index number "
+                          << operand_index << " of access chain "
+                          << inst.PrettyPrint();
+    }
+
+    // Split into two cases: the current index is a constant, or not.
 
     // If the index is a constant then |index_constant| will not be a null
     // pointer.  (If index is an |OpConstantNull| then it |index_constant| will
@@ -313,55 +350,62 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
         value = int64_t(int_index_constant->GetS32BitValue());
       } else if (index_width <= 64) {
         value = int_index_constant->GetS64BitValue();
-      } else {
-        this->Fail() << "Can't handle indices wider than 64 bits, found "
-                        "constant index with "
-                     << index_type->width() << "bits";
-        return;
       }
       if (value < 0) {
-        replace_index(operand_index, GetValueForType(0, index_type));
-      } else if (uint64_t(value) < count) {
+        return replace_index(operand_index, GetValueForType(0, index_type));
+      } else if (uint64_t(value) <= maxval) {
         // Nothing to do.
-        return;
+        return SPV_SUCCESS;
       } else {
-        // Replace with count - 1.
+        // Replace with maxval.
         assert(count > 0);  // Already took care of this case above.
-        replace_index(operand_index, GetValueForType(count - 1, index_type));
+        return replace_index(operand_index,
+                             GetValueForType(maxval, maxval_type));
       }
     } else {
       // Generate a clamp instruction.
-
-      // Compute the bit width of a viable type to hold (count-1).
-      const auto maxval = count - 1;
-      const auto* maxval_type = index_type;
-      // Look for a bit width, up to 64 bits wide, to fit maxval.
-      uint32_t maxval_width = index_width;
-      while ((maxval_width < 64) && (0 != (maxval >> maxval_width))) {
-        maxval_width *= 2;
+      assert(maxval >= 1);
+      assert(index_width <= 64);  // Otherwise, already returned above.
+      if (index_width >= 64 && !have_int64_cap) {
+        // An inconsistent module.
+        return Fail() << "Access chain index is wider than 64 bits, but Int64 "
+                         "is not declared: "
+                      << index_inst->PrettyPrint();
       }
       // Widen the index value if necessary
       if (maxval_width > index_width) {
-        // Find the wider type.  We only need this case if a constant (array)
-        // bound is too big.  This never requires us to *add* a capability
-        // declaration for Int64 because the existence of the array bound would
-        // already have required that declaration.
+        // Find the wider type.  We only need this case if a constant array
+        // bound is too big.
+
+        // From how we calculated maxval_width, widening won't require adding
+        // the Int64 capability.
+        assert(have_int64_cap || maxval_width <= 32);
+        if (!have_int64_cap && maxval_width >= 64) {
+          // Be defensive, but this shouldn't happen.
+          return this->Fail()
+                 << "Clamping index would require adding Int64 capability. "
+                 << "Can't clamp 32-bit index " << operand_index
+                 << " of access chain " << inst.PrettyPrint();
+        }
         index_inst = WidenInteger(index_type->IsSigned(), maxval_width,
                                   index_inst, &inst);
-        maxval_type = type_mgr->GetType(index_inst->type_id())->AsInteger();
       }
+
       // Finally, clamp the index.
-      clamp_index(operand_index, index_inst, GetValueForType(0, maxval_type),
-                  GetValueForType(maxval, maxval_type));
+      return clamp_index(operand_index, index_inst,
+                         GetValueForType(0, maxval_type),
+                         GetValueForType(maxval, maxval_type));
     }
+    return SPV_SUCCESS;
   };
 
   // Ensures the specified index of access chain |inst| has a value that is at
   // most the value of |count_inst| minus 1, where |count_inst| is treated as an
-  // unsigned integer.
+  // unsigned integer. This can log a failure.
   auto clamp_to_count = [&inst, this, &constant_mgr, &clamp_to_literal_count,
-                         &clamp_index, &type_mgr](uint32_t operand_index,
-                                                  Instruction* count_inst) {
+                         &clamp_index,
+                         &type_mgr](uint32_t operand_index,
+                                    Instruction* count_inst) -> spv_result_t {
     Instruction* index_inst =
         this->GetDef(inst.GetSingleWordOperand(operand_index));
     const auto* index_type =
@@ -378,12 +422,11 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
       } else if (width <= 64) {
         value = count_constant->AsIntConstant()->GetU64BitValue();
       } else {
-        this->Fail() << "Can't handle indices wider than 64 bits, found "
-                        "constant index with "
-                     << index_type->width() << "bits";
-        return;
+        return this->Fail() << "Can't handle indices wider than 64 bits, found "
+                               "constant index with "
+                            << index_type->width() << "bits";
       }
-      clamp_to_literal_count(operand_index, value);
+      return clamp_to_literal_count(operand_index, value);
     } else {
       // Widen them to the same width.
       const auto index_width = index_type->width();
@@ -406,9 +449,21 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
           &inst, SpvOpISub, type_mgr->GetId(wider_type), TakeNextId(),
           {{SPV_OPERAND_TYPE_ID, {count_inst->result_id()}},
            {SPV_OPERAND_TYPE_ID, {one->result_id()}}});
-      clamp_index(operand_index, index_inst, GetValueForType(0, wider_type),
-                  count_minus_1);
+      auto* zero = GetValueForType(0, wider_type);
+      // Make sure we clamp to an upper bound that is at most the signed max
+      // for the target type.
+      const uint64_t max_signed_value =
+          ((uint64_t(1) << (target_width - 1)) - 1);
+      // Use unsigned-min to ensure that the result is always non-negative.
+      // That ensures we satisfy the invariant for SClamp, where the "min"
+      // argument we give it (zero), is no larger than the third argument.
+      auto* upper_bound =
+          MakeUMinInst(*type_mgr, count_minus_1,
+                       GetValueForType(max_signed_value, wider_type), &inst);
+      // Now clamp the index to this upper bound.
+      return clamp_index(operand_index, index_inst, zero, upper_bound);
     }
+    return SPV_SUCCESS;
   };
 
   const Instruction* base_inst = GetDef(inst.GetSingleWordInOperand(0));
@@ -483,6 +538,7 @@ void GraphicsRobustAccessPass::ClampIndicesForAccessChain(
           return;
         }
         clamp_to_count(idx, array_len);
+        if (module_status_.failed) return;
         pointee_type = GetDef(pointee_type->GetSingleWordOperand(1));
       } break;
 
@@ -557,22 +613,51 @@ opt::Instruction* opt::GraphicsRobustAccessPass::WidenInteger(
   return conversion;
 }
 
-Instruction* GraphicsRobustAccessPass::MakeClampInst(Instruction* x,
-                                                     Instruction* min,
-                                                     Instruction* max,
-                                                     Instruction* where) {
+Instruction* GraphicsRobustAccessPass::MakeUMinInst(
+    const analysis::TypeManager& tm, Instruction* x, Instruction* y,
+    Instruction* where) {
+  // Get IDs of instructions we'll be referencing. Evaluate them before calling
+  // the function so we force a deterministic ordering in case both of them need
+  // to take a new ID.
+  const uint32_t glsl_insts_id = GetGlslInsts();
+  uint32_t smin_id = TakeNextId();
+  const auto xwidth = tm.GetType(x->type_id())->AsInteger()->width();
+  const auto ywidth = tm.GetType(y->type_id())->AsInteger()->width();
+  assert(xwidth == ywidth);
+  (void)xwidth;
+  (void)ywidth;
+  auto* smin_inst = InsertInst(
+      where, SpvOpExtInst, x->type_id(), smin_id,
+      {
+          {SPV_OPERAND_TYPE_ID, {glsl_insts_id}},
+          {SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER, {GLSLstd450UMin}},
+          {SPV_OPERAND_TYPE_ID, {x->result_id()}},
+          {SPV_OPERAND_TYPE_ID, {y->result_id()}},
+      });
+  return smin_inst;
+}
+
+Instruction* GraphicsRobustAccessPass::MakeSClampInst(
+    const analysis::TypeManager& tm, Instruction* x, Instruction* min,
+    Instruction* max, Instruction* where) {
   // Get IDs of instructions we'll be referencing. Evaluate them before calling
   // the function so we force a deterministic ordering in case both of them need
   // to take a new ID.
   const uint32_t glsl_insts_id = GetGlslInsts();
   uint32_t clamp_id = TakeNextId();
-  assert(x->type_id() == min->type_id());
-  assert(x->type_id() == max->type_id());
+  const auto xwidth = tm.GetType(x->type_id())->AsInteger()->width();
+  const auto minwidth = tm.GetType(min->type_id())->AsInteger()->width();
+  const auto maxwidth = tm.GetType(max->type_id())->AsInteger()->width();
+  assert(xwidth == minwidth);
+  assert(xwidth == maxwidth);
+  (void)xwidth;
+  (void)minwidth;
+  (void)maxwidth;
   auto* clamp_inst = InsertInst(
       where, SpvOpExtInst, x->type_id(), clamp_id,
       {
           {SPV_OPERAND_TYPE_ID, {glsl_insts_id}},
-          {SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER, {GLSLstd450UClamp}},
+          {SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER, {GLSLstd450SClamp}},
           {SPV_OPERAND_TYPE_ID, {x->result_id()}},
           {SPV_OPERAND_TYPE_ID, {min->result_id()}},
           {SPV_OPERAND_TYPE_ID, {max->result_id()}},
@@ -716,6 +801,7 @@ Instruction* GraphicsRobustAccessPass::MakeRuntimeArrayLengthInst(
 spv_result_t GraphicsRobustAccessPass::ClampCoordinateForImageTexelPointer(
     opt::Instruction* image_texel_pointer) {
   // TODO(dneto): Write tests for this code.
+  // TODO(dneto): Use signed-clamp
   return SPV_SUCCESS;
 
   // Example:
@@ -916,9 +1002,9 @@ spv_result_t GraphicsRobustAccessPass::ClampCoordinateForImageTexelPointer(
         {constant_mgr->GetDefiningInstruction(coordinate_1)->result_id()}}});
 
   // Clamp the coordinate
-  auto* clamp_coord =
-      MakeClampInst(coord, constant_mgr->GetDefiningInstruction(coordinate_0),
-                    query_max_including_faces, image_texel_pointer);
+  auto* clamp_coord = MakeSClampInst(
+      *type_mgr, coord, constant_mgr->GetDefiningInstruction(coordinate_0),
+      query_max_including_faces, image_texel_pointer);
   image_texel_pointer->SetInOperand(1, {clamp_coord->result_id()});
 
   // Clamp the sample index
@@ -936,8 +1022,8 @@ spv_result_t GraphicsRobustAccessPass::ClampCoordinateForImageTexelPointer(
                                    {{SPV_OPERAND_TYPE_ID, {query_samples_id}},
                                     {SPV_OPERAND_TYPE_ID, {component_1_id}}});
 
-    auto* clamp_samples = MakeClampInst(
-        samples, constant_mgr->GetDefiningInstruction(coordinate_0),
+    auto* clamp_samples = MakeSClampInst(
+        *type_mgr, samples, constant_mgr->GetDefiningInstruction(coordinate_0),
         max_samples, image_texel_pointer);
     image_texel_pointer->SetInOperand(2, {clamp_samples->result_id()});
 

+ 24 - 11
3rdparty/spirv-tools/source/opt/graphics_robust_access_pass.h

@@ -18,13 +18,13 @@
 #include <map>
 #include <unordered_map>
 
-#include "source/diagnostic.h"
-
 #include "constants.h"
 #include "def_use_manager.h"
 #include "instruction.h"
 #include "module.h"
 #include "pass.h"
+#include "source/diagnostic.h"
+#include "type_manager.h"
 
 namespace spvtools {
 namespace opt {
@@ -49,7 +49,8 @@ class GraphicsRobustAccessPass : public Pass {
   // consumer.
   spvtools::DiagnosticStream Fail();
 
-  // Returns SPV_SUCCESS if this pass can correctly process the module.
+  // Returns SPV_SUCCESS if this pass can correctly process the module,
+  // as far as we can tell from capabilities and the memory model.
   // Otherwise logs a message and returns a failure code.
   spv_result_t IsCompatibleModule();
 
@@ -59,12 +60,12 @@ class GraphicsRobustAccessPass : public Pass {
   spv_result_t ProcessCurrentModule();
 
   // Process the given function.  Updates the state value |_|.  Returns true
-  // if the module was modified.
+  // if the module was modified.  This can log a failure.
   bool ProcessAFunction(opt::Function*);
 
   // Clamps indices in the OpAccessChain or OpInBoundsAccessChain instruction
   // |access_chain|. Inserts instructions before the given instruction.  Updates
-  // analyses and records that the module is modified.
+  // analyses and records that the module is modified.  This can log a failure.
   void ClampIndicesForAccessChain(Instruction* access_chain);
 
   // Returns the id of the instruction importing the "GLSL.std.450" extended
@@ -85,17 +86,29 @@ class GraphicsRobustAccessPass : public Pass {
   Instruction* WidenInteger(bool sign_extend, uint32_t bit_width,
                             Instruction* value, Instruction* before_inst);
 
-  // Returns a new instruction that invokes the UClamp GLSL.std.450 extended
+  // Returns a new instruction that invokes the UMin GLSL.std.450 extended
+  // instruction with the two given operands.  That is, the result of the
+  // instruction is:
+  //  - |x| if |x| is unsigned-less than |y|
+  //  - |y| otherwise
+  // We assume that |x| and |y| are scalar integer types with the same
+  // width.  The instruction is inserted before |where|.
+  opt::Instruction* MakeUMinInst(const analysis::TypeManager& tm,
+                                 Instruction* x, Instruction* y,
+                                 Instruction* where);
+
+  // Returns a new instruction that invokes the SClamp GLSL.std.450 extended
   // instruction with the three given operands.  That is, the result of the
   // instruction is:
-  //  - |min| if |x| is unsigned-less than |min|
-  //  - |max| if |x| is unsigned-more than |max|
+  //  - |min| if |x| is signed-less than |min|
+  //  - |max| if |x| is signed-more than |max|
   //  - |x| otherwise.
-  // We assume that |min| is unsigned-less-or-equal to |max|, and that the
+  // We assume that |min| is signed-less-or-equal to |max|, and that the
   // operands all have the same scalar integer type.  The instruction is
   // inserted before |where|.
-  opt::Instruction* MakeClampInst(Instruction* x, Instruction* min,
-                                  Instruction* max, Instruction* where);
+  opt::Instruction* MakeSClampInst(const analysis::TypeManager& tm,
+                                   Instruction* x, Instruction* min,
+                                   Instruction* max, Instruction* where);
 
   // Returns a new instruction which evaluates to the length the runtime array
   // referenced by the access chain at the specfied index.  The instruction is

+ 1 - 2
3rdparty/spirv-tools/source/opt/optimizer.cpp

@@ -230,8 +230,7 @@ Optimizer& Optimizer::RegisterSizePasses() {
 }
 
 Optimizer& Optimizer::RegisterVulkanToWebGPUPasses() {
-  return RegisterPass(CreateStripDebugInfoPass())
-      .RegisterPass(CreateStripAtomicCounterMemoryPass())
+  return RegisterPass(CreateStripAtomicCounterMemoryPass())
       .RegisterPass(CreateGenerateWebGPUInitializersPass())
       .RegisterPass(CreateLegalizeVectorShufflePass())
       .RegisterPass(CreateSplitInvalidUnreachablePass())

+ 52 - 22
3rdparty/spirv-tools/source/val/validate_type.cpp

@@ -19,29 +19,40 @@
 #include "source/val/instruction.h"
 #include "source/val/validate.h"
 #include "source/val/validation_state.h"
+#include "spirv/unified1/spirv.h"
 
 namespace spvtools {
 namespace val {
 namespace {
 
-// True if the integer constant is > 0. |const_words| are words of the
-// constant-defining instruction (either OpConstant or
-// OpSpecConstant). typeWords are the words of the constant's-type-defining
-// OpTypeInt.
-bool AboveZero(const std::vector<uint32_t>& const_words,
-               const std::vector<uint32_t>& type_words) {
-  const uint32_t width = type_words[2];
-  const bool is_signed = type_words[3] > 0;
+// Returns, as an int64_t, the literal value from an OpConstant or the
+// default value of an OpSpecConstant, assuming it is an integral type.
+// For signed integers, relies the rule that literal value is sign extended
+// to fill out to word granularity.  Assumes that the constant value
+// has
+int64_t ConstantLiteralAsInt64(uint32_t width,
+                               const std::vector<uint32_t>& const_words) {
   const uint32_t lo_word = const_words[3];
-  if (width > 32) {
-    // The spec currently doesn't allow integers wider than 64 bits.
-    const uint32_t hi_word = const_words[4];  // Must exist, per spec.
-    if (is_signed && (hi_word >> 31)) return false;
-    return (lo_word | hi_word) > 0;
-  } else {
-    if (is_signed && (lo_word >> 31)) return false;
-    return lo_word > 0;
-  }
+  if (width <= 32) return int32_t(lo_word);
+  assert(width <= 64);
+  assert(const_words.size() > 4);
+  const uint32_t hi_word = const_words[4];  // Must exist, per spec.
+  return static_cast<int64_t>(uint64_t(lo_word) | uint64_t(hi_word) << 32);
+}
+
+// Returns, as an uint64_t, the literal value from an OpConstant or the
+// default value of an OpSpecConstant, assuming it is an integral type.
+// For signed integers, relies the rule that literal value is sign extended
+// to fill out to word granularity.  Assumes that the constant value
+// has
+int64_t ConstantLiteralAsUint64(uint32_t width,
+                                const std::vector<uint32_t>& const_words) {
+  const uint32_t lo_word = const_words[3];
+  if (width <= 32) return lo_word;
+  assert(width <= 64);
+  assert(const_words.size() > 4);
+  const uint32_t hi_word = const_words[4];  // Must exist, per spec.
+  return (uint64_t(lo_word) | uint64_t(hi_word) << 32);
 }
 
 // Validates that type declarations are unique, unless multiple declarations
@@ -258,14 +269,33 @@ spv_result_t ValidateTypeArray(ValidationState_t& _, const Instruction* inst) {
 
   switch (length->opcode()) {
     case SpvOpSpecConstant:
-    case SpvOpConstant:
-      if (AboveZero(length->words(), const_result_type->words())) break;
-    // Else fall through!
-    case SpvOpConstantNull: {
+    case SpvOpConstant: {
+      auto& type_words = const_result_type->words();
+      const bool is_signed = type_words[3] > 0;
+      const uint32_t width = type_words[2];
+      const int64_t ivalue = ConstantLiteralAsInt64(width, length->words());
+      if (ivalue == 0 || (ivalue < 0 && is_signed)) {
+        return _.diag(SPV_ERROR_INVALID_ID, inst)
+               << "OpTypeArray Length <id> '" << _.getIdName(length_id)
+               << "' default value must be at least 1: found " << ivalue;
+      }
+      if (spvIsWebGPUEnv(_.context()->target_env)) {
+        // WebGPU has maximum integer width of 32 bits, and max array size
+        // is one more than the max signed integer representation.
+        const uint64_t max_permitted = (uint64_t(1) << 31);
+        const uint64_t uvalue = ConstantLiteralAsUint64(width, length->words());
+        if (uvalue > max_permitted) {
+          return _.diag(SPV_ERROR_INVALID_ID, inst)
+                 << "OpTypeArray Length <id> '" << _.getIdName(length_id)
+                 << "' size exceeds max value " << max_permitted
+                 << " permitted by WebGPU: got " << uvalue;
+        }
+      }
+    } break;
+    case SpvOpConstantNull:
       return _.diag(SPV_ERROR_INVALID_ID, inst)
              << "OpTypeArray Length <id> '" << _.getIdName(length_id)
              << "' default value must be at least 1.";
-    }
     case SpvOpSpecConstantOp:
       // Assume it's OK, rather than try to evaluate the operation.
       break;

+ 4 - 4
3rdparty/spirv-tools/test/fuzz/fuzzer_replayer_test.cpp

@@ -419,8 +419,8 @@ TEST(FuzzerReplayerTest, Miscellaneous2) {
          %86 = OpLabel
         %184 = OpPhi %26 %27 %78 %126 %89
          %92 = OpSLessThan %8 %184 %56
-               OpLoopMerge %88 %89 None
-               OpBranchConditional %92 %87 %88
+               OpLoopMerge %1000 %89 None
+               OpBranchConditional %92 %87 %1000
          %87 = OpLabel
          %95 = OpIAdd %26 %183 %74
          %96 = OpSLessThan %8 %184 %95
@@ -462,6 +462,8 @@ TEST(FuzzerReplayerTest, Miscellaneous2) {
          %89 = OpLabel
         %126 = OpIAdd %26 %184 %74
                OpBranch %86
+       %1000 = OpLabel
+               OpBranch %88
          %88 = OpLabel
         %128 = OpIAdd %26 %183 %74
                OpBranch %77
@@ -722,7 +724,6 @@ TEST(FuzzerReplayerTest, Miscellaneous3) {
          %37 = OpSDiv %6 %302 %35
          %38 = OpIMul %6 %35 %37
          %40 = OpIEqual %17 %38 %302
-               OpSelectionMerge %42 None
                OpBranchConditional %40 %41 %42
          %41 = OpLabel
          %50 = OpConvertSToF %20 %302
@@ -750,7 +751,6 @@ TEST(FuzzerReplayerTest, Miscellaneous3) {
                OpBranch %59
          %75 = OpLabel
          %78 = OpSGreaterThan %17 %304 %9
-               OpSelectionMerge %80 None
                OpBranchConditional %78 %79 %80
          %79 = OpLabel
          %83 = OpISub %6 %304 %54

+ 4 - 4
3rdparty/spirv-tools/test/fuzz/fuzzer_shrinker_test.cpp

@@ -528,8 +528,8 @@ TEST(FuzzerShrinkerTest, Miscellaneous2) {
          %86 = OpLabel
         %184 = OpPhi %26 %27 %78 %126 %89
          %92 = OpSLessThan %8 %184 %56
-               OpLoopMerge %88 %89 None
-               OpBranchConditional %92 %87 %88
+               OpLoopMerge %1000 %89 None
+               OpBranchConditional %92 %87 %1000
          %87 = OpLabel
          %95 = OpIAdd %26 %183 %74
          %96 = OpSLessThan %8 %184 %95
@@ -571,6 +571,8 @@ TEST(FuzzerShrinkerTest, Miscellaneous2) {
          %89 = OpLabel
         %126 = OpIAdd %26 %184 %74
                OpBranch %86
+       %1000 = OpLabel
+               OpBranch %88
          %88 = OpLabel
         %128 = OpIAdd %26 %183 %74
                OpBranch %77
@@ -829,7 +831,6 @@ TEST(FuzzerShrinkerTest, Miscellaneous3) {
          %37 = OpSDiv %6 %302 %35
          %38 = OpIMul %6 %35 %37
          %40 = OpIEqual %17 %38 %302
-               OpSelectionMerge %42 None
                OpBranchConditional %40 %41 %42
          %41 = OpLabel
          %50 = OpConvertSToF %20 %302
@@ -857,7 +858,6 @@ TEST(FuzzerShrinkerTest, Miscellaneous3) {
                OpBranch %59
          %75 = OpLabel
          %78 = OpSGreaterThan %17 %304 %9
-               OpSelectionMerge %80 None
                OpBranchConditional %78 %79 %80
          %79 = OpLabel
          %83 = OpISub %6 %304 %54

+ 3 - 6
3rdparty/spirv-tools/test/fuzz/transformation_add_dead_continue_test.cpp

@@ -1514,11 +1514,8 @@ TEST(TransformationAddDeadContinueTest, Miscellaneous5) {
   ASSERT_FALSE(bad_transformation.IsApplicable(context.get(), fact_manager));
 }
 
-TEST(TransformationAddDeadContinueTest, DISABLED_Miscellaneous6) {
-  // A miscellaneous test that exposing a known bug in spirv-fuzz.
-
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/2919): re-enable
-  //  this test as an when the known issue is fixed.
+TEST(TransformationAddDeadContinueTest, Miscellaneous6) {
+  // A miscellaneous test that exposed a bug in spirv-fuzz.
 
   std::string shader = R"(
                OpCapability Shader
@@ -1536,7 +1533,7 @@ TEST(TransformationAddDeadContinueTest, DISABLED_Miscellaneous6) {
                OpBranch %10
          %10 = OpLabel
                OpLoopMerge %13 %12 None
-               OpBranchConditional %9 %13 %11
+               OpBranch %11
          %11 = OpLabel
          %20 = OpCopyObject %6 %9
                OpBranch %12

+ 17 - 0
3rdparty/spirv-tools/test/opt/fold_test.cpp

@@ -6111,6 +6111,23 @@ INSTANTIATE_TEST_SUITE_P(MergeSubTest, MatchingInstructionFoldingTest,
       "%4 = OpFSub %float %float_2 %3\n" +
       "OpReturn\n" +
       "OpFunctionEnd\n",
+    4, true),
+  // Test case 13: merge subtract of subtract with mixed types.
+  // 2 - (1 - x) = x + 1
+  InstructionFoldingCase<bool>(
+    Header() +
+      "; CHECK: [[int:%\\w+]] = OpTypeInt 32 1\n" +
+      "; CHECK: [[int_1:%\\w+]] = OpConstant [[int]] 1\n" +
+      "; CHECK: [[ld:%\\w+]] = OpLoad [[int]]\n" +
+      "; CHECK: %4 = OpIAdd [[int]] [[ld]] [[int_1]]\n" +
+      "%main = OpFunction %void None %void_func\n" +
+      "%main_lab = OpLabel\n" +
+      "%var = OpVariable %_ptr_int Function\n" +
+      "%2 = OpLoad %int %var\n" +
+      "%3 = OpISub %int %uint_1 %2\n" +
+      "%4 = OpISub %int %int_2 %3\n" +
+      "OpReturn\n" +
+      "OpFunctionEnd\n",
     4, true)
 ));
 

+ 145 - 47
3rdparty/spirv-tools/test/opt/graphics_robust_access_test.cpp

@@ -271,7 +271,7 @@ TEST_F(GraphicsRobustAccessTest, ACVectorExcessConstantClamped) {
        %uint_4 = OpConstant %uint 4
        )"
             << MainPrefix() << R"(
-       %var = OpVariable %var_ty Function)" << ACCheck(ac, "%uint_4", "%uint_3")
+       %var = OpVariable %var_ty Function)" << ACCheck(ac, "%uint_4", "%int_3")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
   }
@@ -329,7 +329,7 @@ TEST_F(GraphicsRobustAccessTest, ACVectorGeneralClamped) {
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_3 = OpConstant %int 3
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_3
        %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -354,7 +354,7 @@ TEST_F(GraphicsRobustAccessTest, ACVectorGeneralShortClamped) {
        ; CHECK-DAG: %short_3 = OpConstant %short 3
        ; CHECK-NOT: = OpTypeInt 32
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %short %[[GLSLSTD450]] UClamp %i %short_0 %short_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %short %[[GLSLSTD450]] SClamp %i %short_0 %short_3
        %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -375,11 +375,11 @@ TEST_F(GraphicsRobustAccessTest, ACVectorGeneralUShortClamped) {
             << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-NOT: = OpTypeInt 32
-       ; CHECK-DAG: %ushort_0 = OpConstant %ushort 0
-       ; CHECK-DAG: %ushort_3 = OpConstant %ushort 3
+       ; CHECK-DAG: %short_0 = OpConstant %short 0
+       ; CHECK-DAG: %short_3 = OpConstant %short 3
        ; CHECK-NOT: = OpTypeInt 32
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %ushort %[[GLSLSTD450]] UClamp %i %ushort_0 %ushort_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ushort %[[GLSLSTD450]] SClamp %i %short_0 %short_3
        %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -404,7 +404,7 @@ TEST_F(GraphicsRobustAccessTest, ACVectorGeneralLongClamped) {
        ; CHECK-DAG: %long_3 = OpConstant %long 3
        ; CHECK-NOT: = OpTypeInt 32
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] UClamp %i %long_0 %long_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] SClamp %i %long_0 %long_3
        %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -425,11 +425,11 @@ TEST_F(GraphicsRobustAccessTest, ACVectorGeneralULongClamped) {
             << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-NOT: = OpTypeInt 32
-       ; CHECK-DAG: %ulong_0 = OpConstant %ulong 0
-       ; CHECK-DAG: %ulong_3 = OpConstant %ulong 3
+       ; CHECK-DAG: %long_0 = OpConstant %long 0
+       ; CHECK-DAG: %long_3 = OpConstant %long 3
        ; CHECK-NOT: = OpTypeInt 32
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] UClamp %i %ulong_0 %ulong_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] SClamp %i %long_0 %long_3
        %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
             << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -486,10 +486,9 @@ TEST_F(GraphicsRobustAccessTest, ACMatrixExcessConstantClamped) {
        %uint_1 = OpConstant %uint 1
        %uint_4 = OpConstant %uint 4
        )" << MainPrefix() << R"(
-       ; CHECK: %uint_3 = OpConstant %uint 3
+       ; CHECK: %int_3 = OpConstant %int 3
        %var = OpVariable %var_ty Function)"
-            << ACCheck(ac, "%uint_4 %uint_1", "%uint_3 %uint_1")
-            << MainSuffix();
+            << ACCheck(ac, "%uint_4 %uint_1", "%int_3 %uint_1") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
   }
 }
@@ -529,7 +528,7 @@ TEST_F(GraphicsRobustAccessTest, ACMatrixGeneralClamped) {
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_3 = OpConstant %int 3
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_3
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_3
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i %uint_1", "%[[clamp]] %uint_1") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -585,7 +584,7 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralClamped) {
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_199 = OpConstant %int 199
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_199
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_199
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -606,11 +605,11 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralShortIndexUIntBoundsClamped) {
        %i = OpUndef %short
        )" << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
-       ; CHECK-DAG: %uint_0 = OpConstant %uint 0
-       ; CHECK-DAG: %uint_69999 = OpConstant %uint 69999
+       ; CHECK-DAG: %int_0 = OpConstant %int 0
+       ; CHECK-DAG: %int_69999 = OpConstant %int 69999
        ; CHECK: OpLabel
        ; CHECK: %[[i_ext:\w+]] = OpSConvert %uint %i
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %[[i_ext]] %uint_0 %uint_69999
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %[[i_ext]] %int_0 %int_69999
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -631,11 +630,11 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralUShortIndexIntBoundsClamped) {
        %i = OpUndef %ushort
        )" << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
-       ; CHECK-DAG: %uint_0 = OpConstant %uint 0
-       ; CHECK-DAG: %uint_69999 = OpConstant %uint 69999
+       ; CHECK-DAG: %int_0 = OpConstant %int 0
+       ; CHECK-DAG: %int_69999 = OpConstant %int 69999
        ; CHECK: OpLabel
        ; CHECK: %[[i_ext:\w+]] = OpUConvert %uint %i
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %[[i_ext]] %uint_0 %uint_69999
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %[[i_ext]] %int_0 %int_69999
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -656,10 +655,10 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralUIntIndexShortBoundsClamped) {
        %i = OpUndef %uint
        )" << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
-       ; CHECK-DAG: %uint_0 = OpConstant %uint 0
-       ; CHECK-DAG: %uint_199 = OpConstant %uint 199
+       ; CHECK-DAG: %int_0 = OpConstant %int 0
+       ; CHECK-DAG: %int_199 = OpConstant %int 199
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %i %uint_0 %uint_199
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %i %int_0 %int_199
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -683,7 +682,7 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralIntIndexUShortBoundsClamped) {
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_199 = OpConstant %int 199
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_199
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_199
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -707,7 +706,7 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralLongIndexUIntBoundsClamped) {
        ; CHECK-DAG: %long_0 = OpConstant %long 0
        ; CHECK-DAG: %long_199 = OpConstant %long 199
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] UClamp %i %long_0 %long_199
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] SClamp %i %long_0 %long_199
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -728,16 +727,91 @@ TEST_F(GraphicsRobustAccessTest, ACArrayGeneralULongIndexIntBoundsClamped) {
        %i = OpUndef %ulong
        )" << MainPrefix() << R"(
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
-       ; CHECK-DAG: %ulong_0 = OpConstant %ulong 0
-       ; CHECK-DAG: %ulong_199 = OpConstant %ulong 199
+       ; CHECK-DAG: %long_0 = OpConstant %long 0
+       ; CHECK-DAG: %long_199 = OpConstant %long 199
+       ; CHECK: OpLabel
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] SClamp %i %long_0 %long_199
+       %var = OpVariable %var_ty Function)"
+            << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
+    SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
+  }
+}
+
+TEST_F(GraphicsRobustAccessTest,
+       ACArrayGeneralShortIndeArrayBiggerThanShortMaxClipsToShortIntMax) {
+  for (auto* ac : AccessChains()) {
+    std::ostringstream shaders;
+    shaders << "OpCapability Int16\n"
+            << ShaderPreambleAC({"i"}) << TypesVoid() << TypesShort()
+            << TypesInt() << TypesFloat() << R"(
+       %uint_50000 = OpConstant %uint 50000
+       %arr = OpTypeArray %float %uint_50000
+       %var_ty = OpTypePointer Function %arr
+       %ptr_ty = OpTypePointer Function %float
+       %i = OpUndef %ushort
+       )" << MainPrefix() << R"(
+       ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
+       ; CHECK-DAG: %short_0 = OpConstant %short 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %short 32767
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] UClamp %i %ulong_0 %ulong_199
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ushort %[[GLSLSTD450]] SClamp %i %short_0 %[[intmax]]
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
   }
 }
 
+TEST_F(GraphicsRobustAccessTest,
+       ACArrayGeneralIntIndexArrayBiggerThanIntMaxClipsToSignedIntMax) {
+  for (auto* ac : AccessChains()) {
+    std::ostringstream shaders;
+    shaders << ShaderPreambleAC({"i"}) << TypesVoid() << TypesInt()
+            << TypesFloat() << R"(
+       %uint_3000000000 = OpConstant %uint 3000000000
+       %arr = OpTypeArray %float %uint_3000000000
+       %var_ty = OpTypePointer Function %arr
+       %ptr_ty = OpTypePointer Function %float
+       %i = OpUndef %uint
+       )" << MainPrefix() << R"(
+       ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
+       ; CHECK-DAG: %int_0 = OpConstant %int 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
+       ; CHECK: OpLabel
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %i %int_0 %[[intmax]]
+       %var = OpVariable %var_ty Function)"
+            << ACCheck(ac, "%i", "%[[clamp]]") << MainSuffix();
+    SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
+  }
+}
+
+TEST_F(GraphicsRobustAccessTest,
+       ACArrayGeneralLongIndexArrayBiggerThanLongMaxClipsToSignedLongMax) {
+  for (auto* ac : AccessChains()) {
+    std::ostringstream shaders;
+    shaders << "OpCapability Int64\n"
+            << ShaderPreambleAC({"i"}) << TypesVoid() << TypesInt()
+            << TypesLong()
+            << TypesFloat()
+            // 2^63 == 9,223,372,036,854,775,807
+            << R"(
+       %ulong_9223372036854775999 = OpConstant %ulong 9223372036854775999
+       %arr = OpTypeArray %float %ulong_9223372036854775999
+       %var_ty = OpTypePointer Function %arr
+       %ptr_ty = OpTypePointer Function %float
+       %i = OpUndef %ulong
+       )"
+            << MainPrefix() << R"(
+       ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
+       ; CHECK-DAG: %long_0 = OpConstant %long 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %long 9223372036854775807
+       ; CHECK: OpLabel
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] SClamp %i %long_0 %[[intmax]]
+       %var = OpVariable %var_ty Function)" << ACCheck(ac, "%i", "%[[clamp]]")
+            << MainSuffix();
+    SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
+  }
+}
+
 TEST_F(GraphicsRobustAccessTest, ACArraySpecIdSizedAlwaysClamped) {
   for (auto* ac : AccessChains()) {
     std::ostringstream shaders;
@@ -753,9 +827,11 @@ TEST_F(GraphicsRobustAccessTest, ACArraySpecIdSizedAlwaysClamped) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %uint_0 = OpConstant %uint 0
        ; CHECK-DAG: %uint_1 = OpConstant %uint 1
+       ; CHECK-DAG: %[[uint_intmax:\w+]] = OpConstant %uint 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[max:\w+]] = OpISub %uint %spec200 %uint_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %uint_5 %uint_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UMin %[[max]] %[[uint_intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %uint_5 %uint_0 %[[smin]]
        %var = OpVariable %var_ty Function)"
             << ACCheck(ac, "%uint_5", "%[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -908,11 +984,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayLeastInboundClamped) {
        %int_0 = OpConstant %int 0
        %int_2 = OpConstant %int 2
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
-       ; CHECK: %int_1 = OpConstant %int 1
+       ; CHECK-DAG: %int_1 = OpConstant %int 1
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %int_0 %int_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %int_0 %int_0 %[[smin]]
        )"
             << MainPrefix() << ACCheck(ac, "%int_2 %int_0", "%int_2 %[[clamp]]")
             << MainSuffix();
@@ -937,11 +1015,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralShortIndexClamped) {
        ; CHECK: %uint = OpTypeInt 32 0
        ; CHECK-DAG: %uint_1 = OpConstant %uint 1
        ; CHECK-DAG: %uint_0 = OpConstant %uint 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %uint 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK-DAG: %[[max:\w+]] = OpISub %uint %[[arrlen]] %uint_1
        ; CHECK-DAG: %[[i_ext:\w+]] = OpSConvert %uint %i
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %[[i_ext]] %uint_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %[[i_ext]] %uint_0 %[[smin]]
        )"
             << MainPrefix() << ACCheck(ac, "%short_2 %i", "%short_2 %[[clamp]]")
             << MainSuffix();
@@ -966,11 +1046,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralUShortIndexClamped) {
        ; CHECK: %uint = OpTypeInt 32 0
        ; CHECK-DAG: %uint_1 = OpConstant %uint 1
        ; CHECK-DAG: %uint_0 = OpConstant %uint 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %uint 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK-DAG: %[[max:\w+]] = OpISub %uint %[[arrlen]] %uint_1
        ; CHECK-DAG: %[[i_ext:\w+]] = OpSConvert %uint %i
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %[[i_ext]] %uint_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %[[i_ext]] %uint_0 %[[smin]]
        )"
             << MainPrefix() << ACCheck(ac, "%short_2 %i", "%short_2 %[[clamp]]")
             << MainSuffix();
@@ -993,10 +1075,12 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralIntIndexClamped) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %int_1 = OpConstant %int 1
        ; CHECK-DAG: %int_0 = OpConstant %int 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %[[smin]]
        )"
             << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
             << MainSuffix();
@@ -1019,10 +1103,12 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralUIntIndexClamped) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %uint_1 = OpConstant %uint 1
        ; CHECK-DAG: %uint_0 = OpConstant %uint 0
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %uint 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[max:\w+]] = OpISub %uint %[[arrlen]] %uint_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %i %uint_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] SClamp %i %uint_0 %[[smin]]
        )"
             << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
             << MainSuffix();
@@ -1046,11 +1132,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralLongIndexClamped) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %long_0 = OpConstant %long 0
        ; CHECK-DAG: %long_1 = OpConstant %long 1
+       ; CHECK-DAG: %[[longmax:\w+]] = OpConstant %long 9223372036854775807
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[arrlen_ext:\w+]] = OpUConvert %ulong %[[arrlen]]
        ; CHECK: %[[max:\w+]] = OpISub %long %[[arrlen_ext]] %long_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] UClamp %i %long_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %long %[[GLSLSTD450]] UMin %[[max]] %[[longmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] SClamp %i %long_0 %[[smin]]
        )" << MainPrefix()
             << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -1073,11 +1161,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralULongIndexClamped) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %ulong_0 = OpConstant %ulong 0
        ; CHECK-DAG: %ulong_1 = OpConstant %ulong 1
+       ; CHECK-DAG: %[[longmax:\w+]] = OpConstant %ulong 9223372036854775807
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[arrlen_ext:\w+]] = OpUConvert %ulong %[[arrlen]]
        ; CHECK: %[[max:\w+]] = OpISub %ulong %[[arrlen_ext]] %ulong_1
-       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] UClamp %i %ulong_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] UMin %[[max]] %[[longmax]]
+       ; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] SClamp %i %ulong_0 %[[smin]]
        )" << MainPrefix()
             << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
     SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
@@ -1109,11 +1199,13 @@ TEST_F(GraphicsRobustAccessTest, ACRTArrayStructVectorElem) {
        ; CHECK: %[[GLSLSTD450:\w+]] = OpExtInstImport "GLSL.std.450"
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_3 = OpConstant %int 3
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %[[max]]
-       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %j %int_0 %int_3
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %[[smin]]
+       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %j %int_0 %int_3
        )" << MainPrefix()
             << ACCheck(ac, "%int_2 %i %int_1 %j",
                        "%int_2 %[[clamp_i]] %int_1 %[[clamp_j]]")
@@ -1148,6 +1240,7 @@ TEST_F(GraphicsRobustAccessTest, ACArrayRTArrayStructVectorElem) {
        ; CHECK-DAG: %[[ssbo_p:\w+]] = OpTypePointer Uniform %ssbo_s
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_9 = OpConstant %int 9
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
        ; This access chain is manufatured only so we can compute the array length.
        ; Note that the %int_9 is already clamped
@@ -1155,7 +1248,8 @@ TEST_F(GraphicsRobustAccessTest, ACArrayRTArrayStructVectorElem) {
             << R"( %[[ssbo_p]] %var %int_9
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %[[ssbo_base]] 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %[[max]]
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %[[smin]]
        )" << MainPrefix()
             << ACCheck(ac, "%int_17 %int_2 %i %int_1 %int_2",
                        "%int_9 %int_2 %[[clamp_i]] %int_1 %int_2")
@@ -1195,8 +1289,9 @@ TEST_F(GraphicsRobustAccessTest, ACSplitACArrayRTArrayStructVectorElem) {
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_9 = OpConstant %int 9
        ; CHECK-DAG: %int_3 = OpConstant %int 3
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_9
+       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_9
        ; CHECK: %ac_ssbo = )" << ac
             << R"( %ssbo_pty %var %[[clamp_i]]
        ; CHECK: %ac_rtarr = )"
@@ -1207,8 +1302,9 @@ TEST_F(GraphicsRobustAccessTest, ACSplitACArrayRTArrayStructVectorElem) {
        ; definition to find the base pointer %ac_ssbo.
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %ac_ssbo 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %j %int_0 %[[max]]
-       ; CHECK: %[[clamp_k:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %k %int_0 %int_3
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %j %int_0 %[[smin]]
+       ; CHECK: %[[clamp_k:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %k %int_0 %int_3
        ; CHECK: %ac = )" << ac
             << R"( %ptr_ty %ac_rtarr %[[clamp_j]] %int_1 %[[clamp_k]]
        ; CHECK-NOT: AccessChain
@@ -1258,8 +1354,9 @@ TEST_F(GraphicsRobustAccessTest,
        ; CHECK-DAG: %int_0 = OpConstant %int 0
        ; CHECK-DAG: %int_9 = OpConstant %int 9
        ; CHECK-DAG: %int_3 = OpConstant %int 3
+       ; CHECK-DAG: %[[intmax:\w+]] = OpConstant %int 2147483647
        ; CHECK: OpLabel
-       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %int_9
+       ; CHECK: %[[clamp_i:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %i %int_0 %int_9
        ; CHECK: %ac_ssbo = )" << ac
             << R"( %ssbo_pty %var %[[clamp_i]]
        ; CHECK: %bb1 = OpLabel
@@ -1272,8 +1369,9 @@ TEST_F(GraphicsRobustAccessTest,
        ; definition to find the base pointer %ac_ssbo.
        ; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %ac_ssbo 2
        ; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
-       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %j %int_0 %[[max]]
-       ; CHECK: %[[clamp_k:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %k %int_0 %int_3
+       ; CHECK: %[[smin:\w+]] = OpExtInst %int %[[GLSLSTD450]] UMin %[[max]] %[[intmax]]
+       ; CHECK: %[[clamp_j:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %j %int_0 %[[smin]]
+       ; CHECK: %[[clamp_k:\w+]] = OpExtInst %int %[[GLSLSTD450]] SClamp %k %int_0 %int_3
        ; CHECK: %ac = )" << ac
             << R"( %ptr_ty %ac_rtarr %[[clamp_j]] %int_1 %[[clamp_k]]
        ; CHECK-NOT: AccessChain

+ 0 - 30
3rdparty/spirv-tools/test/opt/optimizer_test.cpp

@@ -234,7 +234,6 @@ TEST(Optimizer, VulkanToWebGPUSetsCorrectPasses) {
                                               "eliminate-dead-code-aggressive",
                                               "eliminate-dead-const",
                                               "flatten-decorations",
-                                              "strip-debug",
                                               "strip-atomic-counter-memory",
                                               "generate-webgpu-initializers",
                                               "legalize-vector-shuffle",
@@ -330,35 +329,6 @@ INSTANTIATE_TEST_SUITE_P(
          "OpFunctionEnd\n",
          // pass
          "flatten-decorations"},
-        // Strip Debug
-        {// input
-         "OpCapability Shader\n"
-         "OpCapability VulkanMemoryModel\n"
-         "OpExtension \"SPV_KHR_vulkan_memory_model\"\n"
-         "OpMemoryModel Logical Vulkan\n"
-         "OpEntryPoint Vertex %func \"shader\"\n"
-         "OpName %main \"main\"\n"
-         "OpName %void_fn \"void_fn\"\n"
-         "%void = OpTypeVoid\n"
-         "%void_f = OpTypeFunction %void\n"
-         "%func = OpFunction %void None %void_f\n"
-         "%label = OpLabel\n"
-         "OpReturn\n"
-         "OpFunctionEnd\n",
-         // expected
-         "OpCapability Shader\n"
-         "OpCapability VulkanMemoryModel\n"
-         "OpExtension \"SPV_KHR_vulkan_memory_model\"\n"
-         "OpMemoryModel Logical Vulkan\n"
-         "OpEntryPoint Vertex %1 \"shader\"\n"
-         "%void = OpTypeVoid\n"
-         "%3 = OpTypeFunction %void\n"
-         "%1 = OpFunction %void None %3\n"
-         "%4 = OpLabel\n"
-         "OpReturn\n"
-         "OpFunctionEnd\n",
-         // pass
-         "strip-debug"},
         // Eliminate Dead Constants
         {// input
          "OpCapability Shader\n"

+ 107 - 19
3rdparty/spirv-tools/test/val/val_id_test.cpp

@@ -749,20 +749,40 @@ TEST_F(ValidateIdWithMessage, OpTypeArrayElementTypeBad) {
 // Signed or unsigned.
 enum Signed { kSigned, kUnsigned };
 
-// Creates an assembly snippet declaring OpTypeArray with the given length.
-std::string MakeArrayLength(const std::string& len, Signed isSigned,
-                            int width) {
+// Creates an assembly module declaring OpTypeArray with the given length.
+std::string MakeArrayLength(const std::string& len, Signed isSigned, int width,
+                            int max_int_width = 64,
+                            bool use_vulkan_memory_model = false) {
   std::ostringstream ss;
   ss << R"(
     OpCapability Shader
-    OpCapability Linkage
-    OpCapability Int16
-    OpCapability Int64
   )";
-  ss << "OpMemoryModel Logical GLSL450\n";
+  if (use_vulkan_memory_model) {
+    ss << " OpCapability VulkanMemoryModel\n";
+  }
+  if (width == 16) {
+    ss << " OpCapability Int16\n";
+  }
+  if (max_int_width > 32) {
+    ss << "\n  OpCapability Int64\n";
+  }
+  if (use_vulkan_memory_model) {
+    ss << " OpExtension \"SPV_KHR_vulkan_memory_model\"\n";
+    ss << "OpMemoryModel Logical Vulkan\n";
+  } else {
+    ss << "OpMemoryModel Logical GLSL450\n";
+  }
+  ss << "OpEntryPoint GLCompute %main \"main\"\n";
+  ss << "OpExecutionMode %main LocalSize 1 1 1\n";
   ss << " %t = OpTypeInt " << width << (isSigned == kSigned ? " 1" : " 0");
   ss << " %l = OpConstant %t " << len;
   ss << " %a = OpTypeArray %t %l";
+  ss << " %void = OpTypeVoid \n"
+        " %voidfn = OpTypeFunction %void \n"
+        " %main = OpFunction %void None %voidfn \n"
+        " %entry = OpLabel\n"
+        " OpReturn\n"
+        " OpFunctionEnd\n";
   return ss.str();
 }
 
@@ -772,7 +792,8 @@ class OpTypeArrayLengthTest
     : public spvtest::TextToBinaryTestBase<::testing::TestWithParam<int>> {
  protected:
   OpTypeArrayLengthTest()
-      : position_(spv_position_t{0, 0, 0}),
+      : env_(SPV_ENV_UNIVERSAL_1_0),
+        position_(spv_position_t{0, 0, 0}),
         diagnostic_(spvDiagnosticCreate(&position_, "")) {}
 
   ~OpTypeArrayLengthTest() { spvDiagnosticDestroy(diagnostic_); }
@@ -783,7 +804,7 @@ class OpTypeArrayLengthTest
     spvDiagnosticDestroy(diagnostic_);
     diagnostic_ = nullptr;
     const auto status =
-        spvValidate(ScopedContext().context, &cbinary, &diagnostic_);
+        spvValidate(ScopedContext(env_).context, &cbinary, &diagnostic_);
     if (status != SPV_SUCCESS) {
       spvDiagnosticPrint(diagnostic_);
       EXPECT_THAT(std::string(diagnostic_->error),
@@ -792,12 +813,15 @@ class OpTypeArrayLengthTest
     return status;
   }
 
+ protected:
+  spv_target_env env_;
+
  private:
   spv_position_t position_;  // For creating diagnostic_.
   spv_diagnostic diagnostic_;
 };
 
-TEST_P(OpTypeArrayLengthTest, LengthPositive) {
+TEST_P(OpTypeArrayLengthTest, LengthPositiveSmall) {
   const int width = GetParam();
   EXPECT_EQ(SPV_SUCCESS,
             Val(CompileSuccessfully(MakeArrayLength("1", kSigned, width))));
@@ -814,20 +838,19 @@ TEST_P(OpTypeArrayLengthTest, LengthPositive) {
   const std::string fpad(width / 4 - 1, 'F');
   EXPECT_EQ(
       SPV_SUCCESS,
-      Val(CompileSuccessfully(MakeArrayLength("0x7" + fpad, kSigned, width))));
-  EXPECT_EQ(SPV_SUCCESS, Val(CompileSuccessfully(
-                             MakeArrayLength("0xF" + fpad, kUnsigned, width))));
+      Val(CompileSuccessfully(MakeArrayLength("0x7" + fpad, kSigned, width))))
+      << MakeArrayLength("0x7" + fpad, kSigned, width);
 }
 
 TEST_P(OpTypeArrayLengthTest, LengthZero) {
   const int width = GetParam();
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength("0", kSigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength("0", kUnsigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
 }
 
@@ -835,23 +858,88 @@ TEST_P(OpTypeArrayLengthTest, LengthNegative) {
   const int width = GetParam();
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength("-1", kSigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength("-2", kSigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength("-123", kSigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
   const std::string neg_max = "0x8" + std::string(width / 4 - 1, '0');
   EXPECT_EQ(SPV_ERROR_INVALID_ID,
             Val(CompileSuccessfully(MakeArrayLength(neg_max, kSigned, width)),
-                "OpTypeArray Length <id> '2\\[%.*\\]' default value must be at "
+                "OpTypeArray Length <id> '3\\[%.*\\]' default value must be at "
                 "least 1."));
 }
 
+// Returns the string form of an integer of the form 0x80....0 of the
+// given bit width.
+std::string big_num_ending_0(int bit_width) {
+  return "0x8" + std::string(bit_width / 4 - 1, '0');
+}
+
+// Returns the string form of an integer of the form 0x80..001 of the
+// given bit width.
+std::string big_num_ending_1(int bit_width) {
+  return "0x8" + std::string(bit_width / 4 - 2, '0') + "1";
+}
+
+TEST_P(OpTypeArrayLengthTest, LengthPositiveHugeEnding0InVulkan) {
+  env_ = SPV_ENV_VULKAN_1_0;
+  const int width = GetParam();
+  for (int max_int_width : {32, 64}) {
+    if (width > max_int_width) {
+      // Not valid to even make the OpConstant in this case.
+      continue;
+    }
+    const auto module = CompileSuccessfully(MakeArrayLength(
+        big_num_ending_0(width), kUnsigned, width, max_int_width));
+    EXPECT_EQ(SPV_SUCCESS, Val(module));
+  }
+}
+
+TEST_P(OpTypeArrayLengthTest, LengthPositiveHugeEnding1InVulkan) {
+  env_ = SPV_ENV_VULKAN_1_0;
+  const int width = GetParam();
+  for (int max_int_width : {32, 64}) {
+    if (width > max_int_width) {
+      // Not valid to even make the OpConstant in this case.
+      continue;
+    }
+    const auto module = CompileSuccessfully(MakeArrayLength(
+        big_num_ending_1(width), kUnsigned, width, max_int_width));
+    EXPECT_EQ(SPV_SUCCESS, Val(module));
+  }
+}
+
+TEST_P(OpTypeArrayLengthTest, LengthPositiveHugeEnding0InWebGPU) {
+  env_ = SPV_ENV_WEBGPU_0;
+  const int width = GetParam();
+  // WebGPU only has 32 bit integers.
+  if (width != 32) return;
+  const int max_int_width = 32;
+  const auto module = CompileSuccessfully(MakeArrayLength(
+      big_num_ending_0(width), kUnsigned, width, max_int_width, true));
+  EXPECT_EQ(SPV_SUCCESS, Val(module));
+}
+
+TEST_P(OpTypeArrayLengthTest, LengthPositiveHugeEnding1InWebGPU) {
+  env_ = SPV_ENV_WEBGPU_0;
+  const int width = GetParam();
+  // WebGPU only has 32 bit integers.
+  if (width != 32) return;
+  const int max_int_width = 32;
+  const auto module = CompileSuccessfully(MakeArrayLength(
+      big_num_ending_1(width), kUnsigned, width, max_int_width, true));
+  EXPECT_EQ(SPV_ERROR_INVALID_ID,
+            Val(module,
+                "OpTypeArray Length <id> '3\\[%.*\\]' size exceeds max value "
+                "2147483648 permitted by WebGPU: got 2147483649"));
+}
+
 // The only valid widths for integers are 8, 16, 32, and 64.
 // Since the Int8 capability requires the Kernel capability, and the Kernel
 // capability prohibits usage of signed integers, we can skip 8-bit integers