Reuse Store logic for Modf and Frexp output parameters
The Modf and Frexp instructions from the GLSL.std.450 extended SPIR-V
instruction set take a pointer argument to write one of their results
to. This makes them the only arithmetic instructions which need to know
how to explicitly access memory.
This change replaces the partial duplication of store logic with a call
to the underlying implementation of OpStore. To support storing
intermediate values not associated with SPIR-V objects, the Operand
class can now also wrap an independent Intermediate instance.
Bug: b/153641251
Change-Id: Iebab43640b45ed6c27a77576168481d1a27158b6
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/43728
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index b098cb1..9a9ccd3 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -2431,6 +2431,13 @@
ASSERT(intermediate || (object.kind == SpirvShader::Object::Kind::Constant));
}
+SpirvShader::Operand::Operand(const Intermediate &value)
+ : constant(nullptr)
+ , intermediate(&value)
+ , componentCount(value.componentCount)
+{
+}
+
SpirvRoutine::SpirvRoutine(vk::PipelineLayout const *pipelineLayout)
: pipelineLayout(pipelineLayout)
{
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 6051176..7df782e 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -992,6 +992,7 @@
{
public:
Operand(const SpirvShader *shader, const EmitState *state, SpirvShader::Object::ID objectId);
+ Operand(const Intermediate &value);
RValue<SIMD::Float> Float(uint32_t i) const
{
diff --git a/src/Pipeline/SpirvShaderGLSLstd450.cpp b/src/Pipeline/SpirvShaderGLSLstd450.cpp
index 5b8d953..3f85800 100644
--- a/src/Pipeline/SpirvShaderGLSLstd450.cpp
+++ b/src/Pipeline/SpirvShaderGLSLstd450.cpp
@@ -354,24 +354,17 @@
{
auto val = Operand(this, state, insn.word(5));
auto ptrId = Object::ID(insn.word(6));
- auto ptrTy = getType(getObject(ptrId));
- auto ptr = GetPointerToData(ptrId, 0, state);
- bool interleavedByLane = IsStorageInterleavedByLane(ptrTy.storageClass);
- // TODO: GLSL modf() takes an output parameter and thus the pointer is assumed
- // to be in bounds even for inactive lanes.
- // - Clarify the SPIR-V spec.
- // - Eliminate lane masking and assume interleaving.
- auto robustness = OutOfBoundsBehavior::UndefinedBehavior;
+
+ Intermediate whole(type.componentCount);
for(auto i = 0u; i < type.componentCount; i++)
{
- SIMD::Float whole, frac;
- std::tie(whole, frac) = Modf(val.Float(i));
- dst.move(i, frac);
- auto p = ptr + (i * sizeof(float));
- if(interleavedByLane) { p = InterleaveByLane(p); }
- p.Store(whole, robustness, state->activeLaneMask());
+ auto wholeAndFrac = Modf(val.Float(i));
+ dst.move(i, wholeAndFrac.second);
+ whole.move(i, wholeAndFrac.first);
}
+
+ Store(ptrId, whole, false, std::memory_order_relaxed, state);
break;
}
case GLSLstd450ModfStruct:
@@ -380,10 +373,9 @@
for(auto i = 0u; i < val.componentCount; i++)
{
- SIMD::Float whole, frac;
- std::tie(whole, frac) = Modf(val.Float(i));
- dst.move(i, frac);
- dst.move(i + val.componentCount, whole);
+ auto wholeAndFrac = Modf(val.Float(i));
+ dst.move(i, wholeAndFrac.second);
+ dst.move(val.componentCount + i, wholeAndFrac.first);
}
break;
}
@@ -499,27 +491,17 @@
{
auto val = Operand(this, state, insn.word(5));
auto ptrId = Object::ID(insn.word(6));
- auto ptrTy = getType(getObject(ptrId));
- auto ptr = GetPointerToData(ptrId, 0, state);
- bool interleavedByLane = IsStorageInterleavedByLane(ptrTy.storageClass);
- // TODO: GLSL frexp() takes an output parameter and thus the pointer is assumed
- // to be in bounds even for inactive lanes.
- // - Clarify the SPIR-V spec.
- // - Eliminate lane masking and assume interleaving.
- auto robustness = OutOfBoundsBehavior::UndefinedBehavior;
+
+ Intermediate exp(type.componentCount);
for(auto i = 0u; i < type.componentCount; i++)
{
- SIMD::Float significand;
- SIMD::Int exponent;
- std::tie(significand, exponent) = Frexp(val.Float(i));
-
- dst.move(i, significand);
-
- auto p = ptr + (i * sizeof(float));
- if(interleavedByLane) { p = InterleaveByLane(p); }
- p.Store(exponent, robustness, state->activeLaneMask());
+ auto significandAndExponent = Frexp(val.Float(i));
+ dst.move(i, significandAndExponent.first);
+ exp.move(i, significandAndExponent.second);
}
+
+ Store(ptrId, exp, false, std::memory_order_relaxed, state);
break;
}
case GLSLstd450FrexpStruct:
