commit 3c0e0582acfa6acb675700e22d00dc1d84d2c310
author Nicolas Capens <capn@google.com> Wed Aug 26 11:04:37 2020 -0400
committer Nicolas Capens <nicolascapens@google.com> Wed Sep 02 17:22:22 2020 +0000
tree 96d1f4f9a475c6a91e0cff3b7b6f57968afec182
parent 6287c18b1d249152563f0cb2d5cb0c6d0eb9e3d6

Skip texel address sample offset if zero

The SPIR-V spec states that the Sample operand of OpImageTexelPointer
"must be a valid <id> for the value 0 if the OpTypeImage has MS of 0."

This change optimizes for this common case by no performing texel
address sample offset and bounds check calculations.

Bug: b/163142358
Change-Id: I75932c264d45df1012fa8451fc4c57671191b59b
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/47808
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

src/Pipeline/SpirvShader.cpp

diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index e308e42..37c7bd5 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -2481,11 +2481,11 @@
 {}
 
 SpirvShader::Operand::Operand(const EmitState *state, const Object &object)
-    : constant(object.constantValue.data())
+    : constant(object.kind == SpirvShader::Object::Kind::Constant ? object.constantValue.data() : nullptr)
     , intermediate(object.kind == SpirvShader::Object::Kind::Intermediate ? &state->getIntermediate(object.id()) : nullptr)
     , componentCount(intermediate ? intermediate->componentCount : object.constantValue.size())
 {
-	ASSERT(intermediate || (object.kind == SpirvShader::Object::Kind::Constant));
+	ASSERT(intermediate || constant);
 }
 
 SpirvShader::Operand::Operand(const Intermediate &value)
@@ -2495,6 +2495,24 @@
 {
 }
 
+bool SpirvShader::Operand::isConstantZero() const
+{
+	if(!constant)
+	{
+		return false;
+	}
+
+	for(uint32_t i = 0; i < componentCount; i++)
+	{
+		if(constant[i] != 0)
+		{
+			return false;
+		}
+	}
+
+	return true;
+}
+
 SpirvRoutine::SpirvRoutine(vk::PipelineLayout const *pipelineLayout)
     : pipelineLayout(pipelineLayout)
 {

src/Pipeline/SpirvShader.hpp

diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index fb8ad86..2f6cb07 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -1069,6 +1069,8 @@
 			return SIMD::UInt(constant[i]);
 		}
 
+		bool isConstantZero() const;
+
 	private:
 		RR_PRINT_ONLY(friend struct rr::PrintValue::Ty<Operand>;)
 

src/Pipeline/SpirvShaderImage.cpp

diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index efd62fa..7e10905 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -558,8 +558,11 @@
 	if(sampleId.value())
 	{
 		Operand sample(this, state, sampleId);
-		n = sample.Int(0);
-		ptrOffset += n * samplePitch;
+		if(!sample.isConstantZero())
+		{
+			n = sample.Int(0);
+			ptrOffset += n * samplePitch;
+		}
 	}
 
 	// If the out-of-bounds behavior is set to nullify, then each coordinate must be tested individually.
@@ -584,8 +587,12 @@
 
 		if(sampleId.value())
 		{
-			SIMD::UInt sampleCount = *Pointer<UInt>(descriptor + OFFSET(vk::StorageImageDescriptor, sampleCount));
-			oobMask |= As<SIMD::Int>(CmpNLT(As<SIMD::UInt>(n), sampleCount));
+			Operand sample(this, state, sampleId);
+			if(!sample.isConstantZero())
+			{
+				SIMD::UInt sampleCount = *Pointer<UInt>(descriptor + OFFSET(vk::StorageImageDescriptor, sampleCount));
+				oobMask |= As<SIMD::Int>(CmpNLT(As<SIMD::UInt>(n), sampleCount));
+			}
 		}
 
 		constexpr int32_t OOB_OFFSET = 0x7FFFFFFF - 16;  // SIMD pointer offsets are signed 32-bit, so this is the largest offset (for 16-byte texels).