AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Use references instead of pointers for Metal out params.

Browse Source 
Pointers require decorating the variable with a * to read back the
value, which the code generator did not properly handle. There was a
special case to add the * but it only supported assignment into the
variable, not reading back. References require no special decoration.

This change fixes compile errors in Functions.sksl with the "bar"
function. (This test marks `x` as an inout but never actually mutates
it.) It also allows us to remove a special-case workaround for `frexp`,
an intrinsic function which uses a reference for its out-parameter.

Additionally, this CL adds a non-inlining copy of "OutParams.sksl" to
the Metal test directory, as most of our tests which use out-parameters
end up inlining all the code, which hides these sorts of bugs.

Change-Id: I31c4db04f6b512b4cd4fe65b3347b82bdbf039cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341000
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-12-04 11:58:26 -05:00 committed by Skia Commit-Bot
parent bc3c41b874
commit f2bd501ce3
6 changed files with 215 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gn/sksl_tests.gni
View File

@ -184,6 +184,7 @@ sksl_metal_tests = [
"$_tests/sksl/metal/NumericGlobals.sksl",
"$_tests/sksl/metal/OpaqueTypeInInterfaceBlock.sksl",
"$_tests/sksl/metal/OpaqueTypeInStruct.sksl",
"$_tests/sksl/metal/OutParams.sksl",
"$_tests/sksl/metal/OutVarsRequireLocation.sksl",
"$_tests/sksl/metal/SamplerGlobals.sksl",
]

24
src/sksl/SkSLMetalCodeGenerator.cpp
View File

@ -279,17 +279,6 @@ void MetalCodeGenerator::writeFunctionCall(const FunctionCall& c) {
} else if (builtin && name == "inverse") {
SkASSERT(arguments.size() == 1);
this->writeInverseHack(*arguments[0]);
} else if (builtin && name == "frexp") {
// Our Metal codegen assumes that out params are pointers, but Metal's built-in frexp
// actually takes a reference for the exponent, not a pointer. We add in a helper function
// here to translate.
SkASSERT(arguments.size() == 2);
auto [iter, newlyCreated] = fHelpers.insert("frexp");
if (newlyCreated) {
fExtraFunctions.printf(
"float frexp(float arg, thread int* exp) { return frexp(arg, *exp); }\n");
}
this->write("frexp");
} else if (builtin && name == "dFdx") {
this->write("dfdx");
} else if (builtin && name == "dFdy") {
@ -324,14 +313,10 @@ void MetalCodeGenerator::writeFunctionCall(const FunctionCall& c) {
this->write("_fragCoord");
separator = ", ";
}
const std::vector<const Variable*>& parameters = function.parameters();
for (size_t i = 0; i < arguments.size(); ++i) {
const Expression& arg = *arguments[i];
this->write(separator);
separator = ", ";
if (parameters[i]->modifiers().fFlags & Modifiers::kOut_Flag) {
this->write("&");
}
this->writeExpression(arg, kSequence_Precedence);
}
this->write(")");
@ -943,13 +928,6 @@ void MetalCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
if (needParens) {
this->write("(");
}
if (Compiler::IsAssignment(op) && left.is<VariableReference>() &&
left.as<VariableReference>().variable()->storage() == Variable::Storage::kParameter &&
left.as<VariableReference>().variable()->modifiers().fFlags & Modifiers::kOut_Flag) {
// writing to an out parameter. Since we have to turn those into pointers, we have to
// dereference it here.
this->write("*");
}
if (op == Token::Kind::TK_STAREQ && leftType.isMatrix() && rightType.isMatrix()) {
this->writeMatrixTimesEqualHelper(leftType, rightType, b.type());
}
@ -1153,7 +1131,7 @@ bool MetalCodeGenerator::writeFunctionDeclaration(const FunctionDeclaration& f)
const Type* type = &param->type();
this->writeBaseType(*type);
if (param->modifiers().fFlags & Modifiers::kOut_Flag) {
this->write("*");
this->write("&");
}
this->write(" ");
this->writeName(param->name());

73
tests/sksl/metal/OutParams.sksl Normal file
View File

@ -0,0 +1,73 @@
/*#pragma settings NoInline*/
void out_half (out half v) { v = 1; }
void out_half2(out half2 v) { v = half2(2); }
void out_half3(out half3 v) { v = half3(3); }
void out_half4(out half4 v) { v = half4(4); }
void out_half2x2(out half2x2 v) { v = half2x2(2); }
void out_half3x3(out half3x3 v) { v = half3x3(3); }
void out_half4x4(out half4x4 v) { v = half4x4(4); }
void out_int (out int v) { v = 1; }
void out_int2(out int2 v) { v = int2(2); }
void out_int3(out int3 v) { v = int3(3); }
void out_int4(out int4 v) { v = int4(4); }
void out_float (out float v) { v = 1; }
void out_float2(out float2 v) { v = float2(2); }
void out_float3(out float3 v) { v = float3(3); }
void out_float4(out float4 v) { v = float4(4); }
void out_float2x2(out float2x2 v) { v = float2x2(2); }
void out_float3x3(out float3x3 v) { v = float3x3(3); }
void out_float4x4(out float4x4 v) { v = float4x4(4); }
void out_bool (out bool v) { v = true; }
void out_bool2(out bool2 v) { v = bool2(false); }
void out_bool3(out bool3 v) { v = bool3(true); }
void out_bool4(out bool4 v) { v = bool4(false); }
void main() {
half h; out_half (h);
half2 h2; out_half2(h2);
half3 h3; out_half3(h3);
half4 h4; out_half4(h4);
out_half2(h3.xz);
out_half4(h4.zwxy);
sk_FragColor = half4(h, h2.x, h3.x, h4.x);
half2x2 h2x2; out_half2x2(h2x2);
half3x3 h3x3; out_half3x3(h3x3);
half4x4 h4x4; out_half4x4(h4x4);
out_half3(h3x3[1]);
out_half(h4x4[3].w);
sk_FragColor = half4(h2x2[0][0], h3x3[0][0], h4x4[0][0], 1);
int i; out_int (i);
int2 i2; out_int2(i2);
int3 i3; out_int3(i3);
int4 i4; out_int4(i4);
out_int3(i4.xyz);
sk_FragColor = half4(i, i2.x, i3.x, i4.x);
float f; out_float (f);
float2 f2; out_float2(f2);
float3 f3; out_float3(f3);
float4 f4; out_float4(f4);
out_float2(f3.xy);
sk_FragColor = half4(half(f), half(f2.x), half(f3.x), half(f4.x));
float2x2 f2x2; out_float2x2(f2x2);
float3x3 f3x3; out_float3x3(f3x3);
float4x4 f4x4; out_float4x4(f4x4);
out_float(f2x2[0][0]);
sk_FragColor = half4(half(f2x2[0][0]), half(f3x3[0][0]), half(f4x4[0][0]), 1);
bool b; out_bool (b);
bool2 b2; out_bool2(b2);
bool3 b3; out_bool3(b3);
bool4 b4; out_bool4(b4);
out_bool2(b4.xw);
sk_FragColor = half4(half(b), half(b2.x), half(b3.x), half(b4.x));
}

137
tests/sksl/metal/golden/OutParams.metal Normal file
View File

@ -0,0 +1,137 @@
#include <metal_stdlib>
#include <simd/simd.h>
using namespace metal;
struct Inputs {
};
struct Outputs {
float4 sk_FragColor [[color(0)]];
};
void out_half(thread float& v) {
v = 1.0;
}
void out_half2(thread float2& v) {
v = float2(2.0);
}
void out_half3(thread float3& v) {
v = float3(3.0);
}
void out_half4(thread float4& v) {
v = float4(4.0);
}
void out_half2x2(thread float2x2& v) {
v = float2x2(2.0);
}
void out_half3x3(thread float3x3& v) {
v = float3x3(3.0);
}
void out_half4x4(thread float4x4& v) {
v = float4x4(4.0);
}
void out_int(thread int& v) {
v = 1;
}
void out_int2(thread int2& v) {
v = int2(2);
}
void out_int3(thread int3& v) {
v = int3(3);
}
void out_int4(thread int4& v) {
v = int4(4);
}
void out_float(thread float& v) {
v = 1.0;
}
void out_float2(thread float2& v) {
v = float2(2.0);
}
void out_float3(thread float3& v) {
v = float3(3.0);
}
void out_float4(thread float4& v) {
v = float4(4.0);
}
void out_float2x2(thread float2x2& v) {
v = float2x2(2.0);
}
void out_float3x3(thread float3x3& v) {
v = float3x3(3.0);
}
void out_float4x4(thread float4x4& v) {
v = float4x4(4.0);
}
void out_bool(thread bool& v) {
v = true;
}
void out_bool2(thread bool2& v) {
v = bool2(false);
}
void out_bool3(thread bool3& v) {
v = bool3(true);
}
void out_bool4(thread bool4& v) {
v = bool4(false);
}
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
float h;
out_half(h);
float2 h2;
out_half2(h2);
float3 h3;
out_half3(h3);
float4 h4;
out_half4(h4);
out_half2(h3.xz);
out_half4(h4.zwxy);
_out->sk_FragColor = float4(h, h2.x, h3.x, h4.x);
float2x2 h2x2;
out_half2x2(h2x2);
float3x3 h3x3;
out_half3x3(h3x3);
float4x4 h4x4;
out_half4x4(h4x4);
out_half3(h3x3[1]);
out_half(h4x4[3].w);
_out->sk_FragColor = float4(h2x2[0][0], h3x3[0][0], h4x4[0][0], 1.0);
int i;
out_int(i);
int2 i2;
out_int2(i2);
int3 i3;
out_int3(i3);
int4 i4;
out_int4(i4);
out_int3(i4.xyz);
_out->sk_FragColor = float4(float(i), float(i2.x), float(i3.x), float(i4.x));
float f;
out_float(f);
float2 f2;
out_float2(f2);
float3 f3;
out_float3(f3);
float4 f4;
out_float4(f4);
out_float2(f3.xy);
_out->sk_FragColor = float4(f, f2.x, f3.x, f4.x);
float2x2 f2x2;
out_float2x2(f2x2);
float3x3 f3x3;
out_float3x3(f3x3);
float4x4 f4x4;
out_float4x4(f4x4);
out_float(f2x2[0][0]);
_out->sk_FragColor = float4(f2x2[0][0], f3x3[0][0], f4x4[0][0], 1.0);
bool b;
out_bool(b);
bool2 b2;
out_bool2(b2);
bool3 b3;
out_bool3(b3);
bool4 b4;
out_bool4(b4);
out_bool2(b4.xw);
_out->sk_FragColor = float4(b ? 1.0 : 0.0, b2.x ? 1.0 : 0.0, b3.x ? 1.0 : 0.0, b4.x ? 1.0 : 0.0);
return *_out;
}

3
tests/sksl/shared/golden/FrExp.metal
View File

@ -6,12 +6,11 @@ struct Inputs {
struct Outputs {
float4 sk_FragColor [[color(0)]];
};
float frexp(float arg, thread int* exp) { return frexp(arg, *exp); }
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
int exp;
float foo = frexp(0.5, &exp);
float foo = frexp(0.5, exp);
_out->sk_FragColor = float4(float(exp));
return *_out;
}

4
tests/sksl/shared/golden/Functions.metal
View File

@ -9,7 +9,7 @@ struct Outputs {
float foo(float v[2]) {
return v[0] * v[1];
}
void bar(thread float* x) {
void bar(thread float& x) {
float y[2];
y[0] = x;
@ -20,7 +20,7 @@ fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
float x = 10.0;
bar(&x);
bar(x);
_out->sk_FragColor = float4(x);
return *_out;
}