Update SkSL type priorities to differentiate signed/unsigned types.

Previously, coercion between a signed type and an unsigned type was
treated as "no cost" because these types shared the exact same priority.
This meant that we couldn't choose the proper overload with function
calls that only differed in signed-ness, like:

  void fn(int4 x);
  void fn(uint4 x);

So we would always choose the int4 version since we encountered it
first. Now, we can choose the correct overload; signed types now have
a slightly elevated priority over unsigned types, allowing coercion
costs to work normally.

Also added some comments to `determineFinalTypes` while trying to see
if that needed some improvements as well, but this turned out to be
a red herring--it didn't need any functional changes.

Change-Id: I334debae9ad0e9b290109658d2fde8f6526770a2
Bug: skia:10999
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344017
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2020-12-14 11:39:20 -05:00 committed by Skia Commit-Bot
parent ea16670e71
commit 1f0dc9cd1b
12 changed files with 163 additions and 103 deletions

View File

@ -24,69 +24,69 @@ public:
: fInvalid_Type(Type::MakeOtherType("<INVALID>"))
, fVoid_Type(Type::MakeOtherType("void"))
, fNull_Type(Type::MakeOtherType("null"))
, fFloatLiteral_Type(Type::MakeScalarType(
"$floatLiteral", Type::NumberKind::kFloat, /*priority=*/3))
, fIntLiteral_Type(Type::MakeScalarType(
"$intLiteral", Type::NumberKind::kSigned, /*priority=*/1))
, fFloat_Type(Type::MakeScalarType(
"float", Type::NumberKind::kFloat, /*priority=*/5, /*highPrecision=*/true))
, fFloatLiteral_Type(Type::MakeScalarType("$floatLiteral", Type::NumberKind::kFloat,
/*priority=*/8))
, fIntLiteral_Type(Type::MakeScalarType("$intLiteral", Type::NumberKind::kSigned,
/*priority=*/5))
, fFloat_Type(Type::MakeScalarType("float", Type::NumberKind::kFloat, /*priority=*/10,
/*highPrecision=*/true))
, fFloat2_Type(Type::MakeVectorType("float2", *fFloat_Type, /*columns=*/2))
, fFloat3_Type(Type::MakeVectorType("float3", *fFloat_Type, /*columns=*/3))
, fFloat4_Type(Type::MakeVectorType("float4", *fFloat_Type, /*columns=*/4))
, fHalf_Type(Type::MakeScalarType("half", Type::NumberKind::kFloat, /*priority=*/4))
, fHalf_Type(Type::MakeScalarType("half", Type::NumberKind::kFloat, /*priority=*/9))
, fHalf2_Type(Type::MakeVectorType("half2", *fHalf_Type, /*columns=*/2))
, fHalf3_Type(Type::MakeVectorType("half3", *fHalf_Type, /*columns=*/3))
, fHalf4_Type(Type::MakeVectorType("half4", *fHalf_Type, /*columns=*/4))
, fUInt_Type(Type::MakeScalarType(
"uint", Type::NumberKind::kUnsigned, /*priority=*/2, /*highPrecision=*/true))
, fUInt2_Type(Type::MakeVectorType("uint2", *fUInt_Type, /*columns=*/2))
, fUInt3_Type(Type::MakeVectorType("uint3", *fUInt_Type, /*columns=*/3))
, fUInt4_Type(Type::MakeVectorType("uint4", *fUInt_Type, /*columns=*/4))
, fInt_Type(Type::MakeScalarType(
"int", Type::NumberKind::kSigned, /*priority=*/2, /*highPrecision=*/true))
, fInt_Type(Type::MakeScalarType("int", Type::NumberKind::kSigned, /*priority=*/7,
/*highPrecision=*/true))
, fInt2_Type(Type::MakeVectorType("int2", *fInt_Type, /*columns=*/2))
, fInt3_Type(Type::MakeVectorType("int3", *fInt_Type, /*columns=*/3))
, fInt4_Type(Type::MakeVectorType("int4", *fInt_Type, /*columns=*/4))
, fUShort_Type(
Type::MakeScalarType("ushort", Type::NumberKind::kUnsigned, /*priority=*/0))
, fUShort2_Type(Type::MakeVectorType("ushort2", *fUShort_Type, /*columns=*/2))
, fUShort3_Type(Type::MakeVectorType("ushort3", *fUShort_Type, /*columns=*/3))
, fUShort4_Type(Type::MakeVectorType("ushort4", *fUShort_Type, /*columns=*/4))
, fShort_Type(Type::MakeScalarType("short", Type::NumberKind::kSigned, /*priority=*/0))
, fUInt_Type(Type::MakeScalarType("uint", Type::NumberKind::kUnsigned, /*priority=*/6,
/*highPrecision=*/true))
, fUInt2_Type(Type::MakeVectorType("uint2", *fUInt_Type, /*columns=*/2))
, fUInt3_Type(Type::MakeVectorType("uint3", *fUInt_Type, /*columns=*/3))
, fUInt4_Type(Type::MakeVectorType("uint4", *fUInt_Type, /*columns=*/4))
, fShort_Type(Type::MakeScalarType("short", Type::NumberKind::kSigned, /*priority=*/4))
, fShort2_Type(Type::MakeVectorType("short2", *fShort_Type, /*columns=*/2))
, fShort3_Type(Type::MakeVectorType("short3", *fShort_Type, /*columns=*/3))
, fShort4_Type(Type::MakeVectorType("short4", *fShort_Type, /*columns=*/4))
, fUByte_Type(
Type::MakeScalarType("ubyte", Type::NumberKind::kUnsigned, /*priority=*/0))
, fUByte2_Type(Type::MakeVectorType("ubyte2", *fUByte_Type, /*columns=*/2))
, fUByte3_Type(Type::MakeVectorType("ubyte3", *fUByte_Type, /*columns=*/3))
, fUByte4_Type(Type::MakeVectorType("ubyte4", *fUByte_Type, /*columns=*/4))
, fByte_Type(Type::MakeScalarType("byte", Type::NumberKind::kSigned, /*priority=*/0))
, fUShort_Type(Type::MakeScalarType("ushort", Type::NumberKind::kUnsigned,
/*priority=*/3))
, fUShort2_Type(Type::MakeVectorType("ushort2", *fUShort_Type, /*columns=*/2))
, fUShort3_Type(Type::MakeVectorType("ushort3", *fUShort_Type, /*columns=*/3))
, fUShort4_Type(Type::MakeVectorType("ushort4", *fUShort_Type, /*columns=*/4))
, fByte_Type(Type::MakeScalarType("byte", Type::NumberKind::kSigned, /*priority=*/2))
, fByte2_Type(Type::MakeVectorType("byte2", *fByte_Type, /*columns=*/2))
, fByte3_Type(Type::MakeVectorType("byte3", *fByte_Type, /*columns=*/3))
, fByte4_Type(Type::MakeVectorType("byte4", *fByte_Type, /*columns=*/4))
, fBool_Type(Type::MakeScalarType("bool", Type::NumberKind::kBoolean, /*priority=*/-1))
, fUByte_Type(Type::MakeScalarType("ubyte", Type::NumberKind::kUnsigned,
/*priority=*/1))
, fUByte2_Type(Type::MakeVectorType("ubyte2", *fUByte_Type, /*columns=*/2))
, fUByte3_Type(Type::MakeVectorType("ubyte3", *fUByte_Type, /*columns=*/3))
, fUByte4_Type(Type::MakeVectorType("ubyte4", *fUByte_Type, /*columns=*/4))
, fBool_Type(Type::MakeScalarType("bool", Type::NumberKind::kBoolean, /*priority=*/0))
, fBool2_Type(Type::MakeVectorType("bool2", *fBool_Type, /*columns=*/2))
, fBool3_Type(Type::MakeVectorType("bool3", *fBool_Type, /*columns=*/3))
, fBool4_Type(Type::MakeVectorType("bool4", *fBool_Type, /*columns=*/4))
, fFloat2x2_Type(
Type::MakeMatrixType("float2x2", *fFloat_Type, /*columns=*/2, /*rows=*/2))
, fFloat2x3_Type(
Type::MakeMatrixType("float2x3", *fFloat_Type, /*columns=*/2, /*rows=*/3))
, fFloat2x4_Type(
Type::MakeMatrixType("float2x4", *fFloat_Type, /*columns=*/2, /*rows=*/4))
, fFloat3x2_Type(
Type::MakeMatrixType("float3x2", *fFloat_Type, /*columns=*/3, /*rows=*/2))
, fFloat3x3_Type(
Type::MakeMatrixType("float3x3", *fFloat_Type, /*columns=*/3, /*rows=*/3))
, fFloat3x4_Type(
Type::MakeMatrixType("float3x4", *fFloat_Type, /*columns=*/3, /*rows=*/4))
, fFloat4x2_Type(
Type::MakeMatrixType("float4x2", *fFloat_Type, /*columns=*/4, /*rows=*/2))
, fFloat4x3_Type(
Type::MakeMatrixType("float4x3", *fFloat_Type, /*columns=*/4, /*rows=*/3))
, fFloat4x4_Type(
Type::MakeMatrixType("float4x4", *fFloat_Type, /*columns=*/4, /*rows=*/4))
, fFloat2x2_Type(Type::MakeMatrixType("float2x2", *fFloat_Type,
/*columns=*/2, /*rows=*/2))
, fFloat2x3_Type(Type::MakeMatrixType("float2x3", *fFloat_Type,
/*columns=*/2, /*rows=*/3))
, fFloat2x4_Type(Type::MakeMatrixType("float2x4", *fFloat_Type,
/*columns=*/2, /*rows=*/4))
, fFloat3x2_Type(Type::MakeMatrixType("float3x2", *fFloat_Type,
/*columns=*/3, /*rows=*/2))
, fFloat3x3_Type(Type::MakeMatrixType("float3x3", *fFloat_Type,
/*columns=*/3, /*rows=*/3))
, fFloat3x4_Type(Type::MakeMatrixType("float3x4", *fFloat_Type,
/*columns=*/3, /*rows=*/4))
, fFloat4x2_Type(Type::MakeMatrixType("float4x2", *fFloat_Type,
/*columns=*/4, /*rows=*/2))
, fFloat4x3_Type(Type::MakeMatrixType("float4x3", *fFloat_Type,
/*columns=*/4, /*rows=*/3))
, fFloat4x4_Type(Type::MakeMatrixType("float4x4", *fFloat_Type,
/*columns=*/4, /*rows=*/4))
, fHalf2x2_Type(Type::MakeMatrixType("half2x2", *fHalf_Type, /*columns=*/2, /*rows=*/2))
, fHalf2x3_Type(Type::MakeMatrixType("half2x3", *fHalf_Type, /*columns=*/2, /*rows=*/3))
, fHalf2x4_Type(Type::MakeMatrixType("half2x4", *fHalf_Type, /*columns=*/2, /*rows=*/4))
@ -302,36 +302,36 @@ public:
const std::unique_ptr<Type> fHalf3_Type;
const std::unique_ptr<Type> fHalf4_Type;
const std::unique_ptr<Type> fUInt_Type;
const std::unique_ptr<Type> fUInt2_Type;
const std::unique_ptr<Type> fUInt3_Type;
const std::unique_ptr<Type> fUInt4_Type;
const std::unique_ptr<Type> fInt_Type;
const std::unique_ptr<Type> fInt2_Type;
const std::unique_ptr<Type> fInt3_Type;
const std::unique_ptr<Type> fInt4_Type;
const std::unique_ptr<Type> fUShort_Type;
const std::unique_ptr<Type> fUShort2_Type;
const std::unique_ptr<Type> fUShort3_Type;
const std::unique_ptr<Type> fUShort4_Type;
const std::unique_ptr<Type> fUInt_Type;
const std::unique_ptr<Type> fUInt2_Type;
const std::unique_ptr<Type> fUInt3_Type;
const std::unique_ptr<Type> fUInt4_Type;
const std::unique_ptr<Type> fShort_Type;
const std::unique_ptr<Type> fShort2_Type;
const std::unique_ptr<Type> fShort3_Type;
const std::unique_ptr<Type> fShort4_Type;
const std::unique_ptr<Type> fUByte_Type;
const std::unique_ptr<Type> fUByte2_Type;
const std::unique_ptr<Type> fUByte3_Type;
const std::unique_ptr<Type> fUByte4_Type;
const std::unique_ptr<Type> fUShort_Type;
const std::unique_ptr<Type> fUShort2_Type;
const std::unique_ptr<Type> fUShort3_Type;
const std::unique_ptr<Type> fUShort4_Type;
const std::unique_ptr<Type> fByte_Type;
const std::unique_ptr<Type> fByte2_Type;
const std::unique_ptr<Type> fByte3_Type;
const std::unique_ptr<Type> fByte4_Type;
const std::unique_ptr<Type> fUByte_Type;
const std::unique_ptr<Type> fUByte2_Type;
const std::unique_ptr<Type> fUByte3_Type;
const std::unique_ptr<Type> fUByte4_Type;
const std::unique_ptr<Type> fBool_Type;
const std::unique_ptr<Type> fBool2_Type;
const std::unique_ptr<Type> fBool3_Type;

View File

@ -116,28 +116,35 @@ public:
outParameterTypes->reserve_back(arguments.size());
int genericIndex = -1;
for (size_t i = 0; i < arguments.size(); i++) {
// Non-generic parameters are final as-is.
const Type& parameterType = parameters[i]->type();
if (parameterType.typeKind() == Type::TypeKind::kGeneric) {
const std::vector<const Type*>& types = parameterType.coercibleTypes();
if (genericIndex == -1) {
for (size_t j = 0; j < types.size(); j++) {
if (arguments[i]->type().canCoerceTo(*types[j], /*allowNarrowing=*/true)) {
genericIndex = j;
break;
}
}
if (genericIndex == -1) {
return false;
if (parameterType.typeKind() != Type::TypeKind::kGeneric) {
outParameterTypes->push_back(&parameterType);
continue;
}
// We use the first generic parameter we find to lock in the generic index;
// e.g. if we find `float3` here, all `$genType`s will be assumed to be `float3`.
const std::vector<const Type*>& types = parameterType.coercibleTypes();
if (genericIndex == -1) {
for (size_t j = 0; j < types.size(); j++) {
if (arguments[i]->type().canCoerceTo(*types[j], /*allowNarrowing=*/true)) {
genericIndex = j;
break;
}
}
outParameterTypes->push_back(types[genericIndex]);
} else {
outParameterTypes->push_back(&parameterType);
if (genericIndex == -1) {
// The passed-in type wasn't a match for ANY of the generic possibilities.
// This function isn't a match at all.
return false;
}
}
outParameterTypes->push_back(types[genericIndex]);
}
// Apply the generic index to our return type.
const Type& returnType = this->returnType();
if (returnType.typeKind() == Type::TypeKind::kGeneric) {
if (genericIndex == -1) {
// We don't support functions with a generic return type and no other generics.
return false;
}
*outReturnType = returnType.coercibleTypes()[genericIndex];

View File

@ -1 +1,8 @@
in half4 a, b; void main() { sk_FragColor.x = lessThan(a, b).x ? 1 : 0; }
in half4 a, b;
in uint2 c, d;
in int3 e, f;
void main() {
sk_FragColor.x = lessThan(a, b).x ? 1 : 0;
sk_FragColor.y = lessThan(c, d).y ? 1 : 0;
sk_FragColor.z = lessThan(e, f).z ? 1 : 0;
}

View File

@ -38,11 +38,10 @@ OpDecorate %sk_Clockwise BuiltIn FrontFacing
%19 = OpConvertSToF %float %20
%22 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_0
OpStore %22 %19
%28 = OpLoad %uint %b
%27 = OpBitcast %int %28
%27 = OpLoad %uint %b
%26 = OpExtInst %int %1 FindILsb %27
%25 = OpConvertSToF %float %26
%29 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_1
OpStore %29 %25
%28 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_1
OpStore %28 %25
OpReturn
OpFunctionEnd

View File

@ -4,5 +4,5 @@ in int a;
in uint b;
void main() {
sk_FragColor.x = float(findLSB(a));
sk_FragColor.y = float(findLSB(int(b)));
sk_FragColor.y = float(findLSB(b));
}

View File

@ -14,8 +14,8 @@ fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
int _skTemp0;
int _skTemp1;
uint _skTemp1;
_out->sk_FragColor.x = float((_skTemp0 = (_in.a), select(ctz(_skTemp0), int(-1), _skTemp0 == int(0))));
_out->sk_FragColor.y = float((_skTemp1 = (int(_in.b)), select(ctz(_skTemp1), int(-1), _skTemp1 == int(0))));
_out->sk_FragColor.y = float((_skTemp1 = (_in.b), select(ctz(_skTemp1), uint(-1), _skTemp1 == uint(0))));
return *_out;
}

View File

@ -38,11 +38,10 @@ OpDecorate %sk_Clockwise BuiltIn FrontFacing
%19 = OpConvertSToF %float %20
%22 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_0
OpStore %22 %19
%28 = OpLoad %uint %b
%27 = OpBitcast %int %28
%26 = OpExtInst %int %1 FindSMsb %27
%27 = OpLoad %uint %b
%26 = OpExtInst %int %1 FindUMsb %27
%25 = OpConvertSToF %float %26
%29 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_1
OpStore %29 %25
%28 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_1
OpStore %28 %25
OpReturn
OpFunctionEnd

View File

@ -4,5 +4,5 @@ in int a;
in uint b;
void main() {
sk_FragColor.x = float(findMSB(a));
sk_FragColor.y = float(findMSB(int(b)));
sk_FragColor.y = float(findMSB(b));
}

View File

@ -14,6 +14,6 @@ fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
_out->sk_FragColor.x = float(findMSB(_in.a));
_out->sk_FragColor.y = float(findMSB(int(_in.b)));
_out->sk_FragColor.y = float(findMSB(_in.b));
return *_out;
}

View File

@ -1,12 +1,16 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %sk_FragColor %sk_Clockwise %a %b
OpEntryPoint Fragment %main "main" %sk_FragColor %sk_Clockwise %a %b %c %d %e %f
OpExecutionMode %main OriginUpperLeft
OpName %sk_FragColor "sk_FragColor"
OpName %sk_Clockwise "sk_Clockwise"
OpName %a "a"
OpName %b "b"
OpName %c "c"
OpName %d "d"
OpName %e "e"
OpName %f "f"
OpName %main "main"
OpDecorate %sk_FragColor RelaxedPrecision
OpDecorate %sk_FragColor Location 0
@ -15,8 +19,8 @@ OpDecorate %sk_Clockwise RelaxedPrecision
OpDecorate %sk_Clockwise BuiltIn FrontFacing
OpDecorate %a RelaxedPrecision
OpDecorate %b RelaxedPrecision
OpDecorate %18 RelaxedPrecision
OpDecorate %19 RelaxedPrecision
OpDecorate %28 RelaxedPrecision
OpDecorate %29 RelaxedPrecision
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
@ -27,22 +31,50 @@ OpDecorate %19 RelaxedPrecision
%_ptr_Input_v4float = OpTypePointer Input %v4float
%a = OpVariable %_ptr_Input_v4float Input
%b = OpVariable %_ptr_Input_v4float Input
%void = OpTypeVoid
%14 = OpTypeFunction %void
%v4bool = OpTypeVector %bool 4
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%_ptr_Input_v2uint = OpTypePointer Input %v2uint
%c = OpVariable %_ptr_Input_v2uint Input
%d = OpVariable %_ptr_Input_v2uint Input
%int = OpTypeInt 32 1
%v3int = OpTypeVector %int 3
%_ptr_Input_v3int = OpTypePointer Input %v3int
%e = OpVariable %_ptr_Input_v3int Input
%f = OpVariable %_ptr_Input_v3int Input
%void = OpTypeVoid
%24 = OpTypeFunction %void
%v4bool = OpTypeVector %bool 4
%int_1 = OpConstant %int 1
%int_0 = OpConstant %int 0
%_ptr_Output_float = OpTypePointer Output %float
%main = OpFunction %void None %14
%15 = OpLabel
%18 = OpLoad %v4float %a
%19 = OpLoad %v4float %b
%17 = OpFOrdLessThan %v4bool %18 %19
%21 = OpCompositeExtract %bool %17 0
%22 = OpSelect %int %21 %int_1 %int_0
%16 = OpConvertSToF %float %22
%26 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_0
OpStore %26 %16
%v2bool = OpTypeVector %bool 2
%v3bool = OpTypeVector %bool 3
%int_2 = OpConstant %int 2
%main = OpFunction %void None %24
%25 = OpLabel
%28 = OpLoad %v4float %a
%29 = OpLoad %v4float %b
%27 = OpFOrdLessThan %v4bool %28 %29
%31 = OpCompositeExtract %bool %27 0
%32 = OpSelect %int %31 %int_1 %int_0
%26 = OpConvertSToF %float %32
%35 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_0
OpStore %35 %26
%39 = OpLoad %v2uint %c
%40 = OpLoad %v2uint %d
%38 = OpULessThan %v2bool %39 %40
%42 = OpCompositeExtract %bool %38 1
%43 = OpSelect %int %42 %int_1 %int_0
%37 = OpConvertSToF %float %43
%44 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_1
OpStore %44 %37
%47 = OpLoad %v3int %e
%48 = OpLoad %v3int %f
%46 = OpSLessThan %v3bool %47 %48
%50 = OpCompositeExtract %bool %46 2
%51 = OpSelect %int %50 %int_1 %int_0
%45 = OpConvertSToF %float %51
%52 = OpAccessChain %_ptr_Output_float %sk_FragColor %int_2
OpStore %52 %45
OpReturn
OpFunctionEnd

View File

@ -2,6 +2,12 @@
out vec4 sk_FragColor;
in vec4 a;
in vec4 b;
in uvec2 c;
in uvec2 d;
in ivec3 e;
in ivec3 f;
void main() {
sk_FragColor.x = float(lessThan(a, b).x ? 1 : 0);
sk_FragColor.y = float(lessThan(c, d).y ? 1 : 0);
sk_FragColor.z = float(lessThan(e, f).z ? 1 : 0);
}

View File

@ -4,15 +4,25 @@ using namespace metal;
struct Inputs {
float4 a;
float4 b;
uint2 c;
uint2 d;
int3 e;
int3 f;
};
struct Outputs {
float4 sk_FragColor [[color(0)]];
};
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
Outputs _outputStruct;
thread Outputs* _out = &_outputStruct;
_out->sk_FragColor.x = float((_in.a < _in.b).x ? 1 : 0);
_out->sk_FragColor.y = float((_in.c < _in.d).y ? 1 : 0);
_out->sk_FragColor.z = float((_in.e < _in.f).z ? 1 : 0);
return *_out;
}