[naga spv-out] Bounds check generation could be cleaner and smarter

[jimblandy]

naga::back::spv::index::BoundsCheckResult doesn't really cover all the outcomes that it could, and as a result the code is less clear than it ought to be.

When the backend encounters an indexing operation that needs a bounds check applied, logically there are four possible outcomes:

• The index is statically known to have value i...
  • ... which is in bounds.
  • ... which is out of bounds.
• The index's value is not statically known...
  • ... but here is a SPIR-V instruction whose value has been restricted to ensure it is safe to use.
  • ... but here is a SPIR-V instruction whose value is a boolean that is true if the index is safe to use.

At the moment, the "statically known to be out of bounds" case is absent from BoundsCheckResult and the code that produces it, which makes it look weird. If the code simply took each of the above cases in turn, I think it would be easier to follow, and produce (marginally) better code.

[jimblandy]

There's a comment in src/back/spv/index.rs that says:

                    // In theory, when `known_index` is bad, we could return a new
                    // `KnownOutOfBounds` variant here. But it's simpler just to fall
                    // through and let the bounds check take place. The shader is
                    // broken anyway, so it doesn't make sense to invest in emitting
                    // the ideal code for it.

I wrote that comment in May 2021. Coming back to look at this code later, I don't really agree: I think code is harder to follow when there are cases you know must arise, but that don't seem to be handled anywhere.