Skip to content

Commit

Permalink
HLSL-IR: Implement polyfill_reflect_vec2_f32
Browse files Browse the repository at this point in the history 
This polyfill is a workaround for calling builtin reflect on vec2<f32>
arguments on certain Intel GPUs. This was implemented on the AST path,
and is now implement on the IR path.

Bug: 377357960
Bug: 42250836
Change-Id: Ic17dc97897a0518a336419b5ac43c8950fe84717
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214294
Reviewed-by: James Price <[email protected]>
Commit-Queue: Antonio Maiorano <[email protected]>
Reviewed-by: dan sinclair <[email protected]>
  • Loading branch information
@amaiorano
amaiorano authored and Dawn LUCI CQ committed Nov 7, 2024
1 parent 022ccae commit ed79e01
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 2 deletions.
41 changes: 41 additions & 0 deletions src/tint/lang/core/ir/transform/builtin_polyfill.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ struct State {
worklist.Push(builtin);
}
break;
case core::BuiltinFn::kReflect:
if (config.reflect_vec2_f32) {
// Polyfill for vec2<f32>. See crbug.com/tint/1798
auto* vec_ty = builtin->Result(0)->Type()->As<core::type::Vector>();
if (vec_ty->Width() == 2 && vec_ty->Type()->Is<core::type::F32>()) {
worklist.Push(builtin);
}
}
break;
case core::BuiltinFn::kSaturate:
if (config.saturate) {
worklist.Push(builtin);
Expand Down Expand Up @@ -201,6 +210,9 @@ struct State {
case core::BuiltinFn::kRadians:
Radians(builtin);
break;
case core::BuiltinFn::kReflect:
Reflect(builtin);
break;
case core::BuiltinFn::kSaturate:
Saturate(builtin);
break;
Expand Down Expand Up @@ -670,6 +682,35 @@ struct State {
call->Destroy();
}

/// Polyfill a `reflect()` builtin call.
/// @param call the builtin call instruction
void Reflect(ir::CoreBuiltinCall* call) {
auto* e1 = call->Args()[0];
auto* e2 = call->Args()[1];
auto* vec_ty = e1->Type()->As<core::type::Vector>();
// Only polyfills vec2<f32> (crbug.com/tint/1798)
TINT_ASSERT(vec_ty && vec_ty->Width() == 2 && vec_ty->Type()->Is<core::type::F32>());

b.InsertBefore(call, [&] {
// The generated HLSL must effectively be emitted as:
// e1 + (-2 * dot(e1,e2) * e2)
// Rather than the mathemetically equivalent:
// e1 - 2 * dot(e2,e2) * e2
//
// When FXC compiles HLSL reflect, or the second case above,
// it emits a `dp4` instruction for `2 * dot(e1,e2)`, which is
// miscompiled by certain Intel drivers. The workaround (first
// case above) results in FXC emitting a `dp2` for the dot,
// followed by a `mul 2`, which works around the bug.
auto* dot = b.Call(ty.f32(), core::BuiltinFn::kDot, e1, e2);
auto* factor = b.Multiply(ty.f32(), -2.0_f, dot);
auto* vfactor = b.Construct(vec_ty, factor);
auto* mul = b.Multiply(vec_ty, vfactor, e2);
b.AddWithResult(call->DetachResult(), e1, mul);
});
call->Destroy();
}

/// Polyfill a `saturate()` builtin call.
/// @param call the builtin call instruction
void Saturate(ir::CoreBuiltinCall* call) {
Expand Down
2 changes: 2 additions & 0 deletions src/tint/lang/core/ir/transform/builtin_polyfill.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ struct BuiltinPolyfillConfig {
BuiltinPolyfillLevel insert_bits = BuiltinPolyfillLevel::kNone;
/// Should `radians()` be polyfilled?
bool radians = false;
/// Should `reflect()` be polyfilled for vec2<f32>?
bool reflect_vec2_f32 = false;
/// Should `saturate()` be polyfilled?
bool saturate = false;
/// Should `textureSampleBaseClampToEdge()` be polyfilled for texture_2d<f32> textures?
Expand Down
152 changes: 152 additions & 0 deletions src/tint/lang/core/ir/transform/builtin_polyfill_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2454,5 +2454,157 @@ TEST_F(IR_BuiltinPolyfillTest, Dot4U8Packed) {
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_NoPolyfill) {
Build(core::BuiltinFn::kReflect, ty.vec2<f32>(), Vector{ty.vec2<f32>(), ty.vec2<f32>()});
auto* src = R"(
%foo = func(%arg:vec2<f32>, %arg_1:vec2<f32>):vec2<f32> { # %arg_1: 'arg'
$B1: {
%result:vec2<f32> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = false;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

// Reflect polyfill (reflect_vec2_f32) should only affect vec2<f32>
TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec2F32) {
Build(core::BuiltinFn::kReflect, ty.vec2<f32>(), Vector{ty.vec2<f32>(), ty.vec2<f32>()});
auto* src = R"(
%foo = func(%arg:vec2<f32>, %arg_1:vec2<f32>):vec2<f32> { # %arg_1: 'arg'
$B1: {
%result:vec2<f32> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = R"(
%foo = func(%arg:vec2<f32>, %arg_1:vec2<f32>):vec2<f32> { # %arg_1: 'arg'
$B1: {
%4:f32 = dot %arg, %arg_1
%5:f32 = mul -2.0f, %4
%6:vec2<f32> = construct %5
%7:vec2<f32> = mul %6, %arg_1
%result:vec2<f32> = add %arg, %7
ret %result
}
}
)";

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec3F32) {
Build(core::BuiltinFn::kReflect, ty.vec3<f32>(), Vector{ty.vec3<f32>(), ty.vec3<f32>()});
auto* src = R"(
%foo = func(%arg:vec3<f32>, %arg_1:vec3<f32>):vec3<f32> { # %arg_1: 'arg'
$B1: {
%result:vec3<f32> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec4F32) {
Build(core::BuiltinFn::kReflect, ty.vec4<f32>(), Vector{ty.vec4<f32>(), ty.vec4<f32>()});
auto* src = R"(
%foo = func(%arg:vec4<f32>, %arg_1:vec4<f32>):vec4<f32> { # %arg_1: 'arg'
$B1: {
%result:vec4<f32> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec2F16) {
Build(core::BuiltinFn::kReflect, ty.vec2<f16>(), Vector{ty.vec2<f16>(), ty.vec2<f16>()});
auto* src = R"(
%foo = func(%arg:vec2<f16>, %arg_1:vec2<f16>):vec2<f16> { # %arg_1: 'arg'
$B1: {
%result:vec2<f16> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec3F16) {
Build(core::BuiltinFn::kReflect, ty.vec3<f16>(), Vector{ty.vec3<f16>(), ty.vec3<f16>()});
auto* src = R"(
%foo = func(%arg:vec3<f16>, %arg_1:vec3<f16>):vec3<f16> { # %arg_1: 'arg'
$B1: {
%result:vec3<f16> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

TEST_F(IR_BuiltinPolyfillTest, Reflect_Vec4F16) {
Build(core::BuiltinFn::kReflect, ty.vec4<f16>(), Vector{ty.vec4<f16>(), ty.vec4<f16>()});
auto* src = R"(
%foo = func(%arg:vec4<f16>, %arg_1:vec4<f16>):vec4<f16> { # %arg_1: 'arg'
$B1: {
%result:vec4<f16> = reflect %arg, %arg_1
ret %result
}
}
)";
auto* expect = src;

EXPECT_EQ(src, str());

BuiltinPolyfillConfig config;
config.reflect_vec2_f32 = true;
Run(BuiltinPolyfill, config);
EXPECT_EQ(expect, str());
}

} // namespace
} // namespace tint::core::ir::transform
55 changes: 55 additions & 0 deletions src/tint/lang/hlsl/writer/builtin_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4443,5 +4443,60 @@ void foo() {
)");
}

TEST_F(HlslWriterTest, BuiltinReflect_Vec2f32_NoPolyfill) {
auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
b.Append(func->Block(), [&] {
auto* vec_ty = ty.vec2<f32>();
auto* x = b.Let("x", b.MatchWidth(1_f, vec_ty));
auto* y = b.Let("y", b.MatchWidth(2_f, vec_ty));

auto* c = b.Call(vec_ty, core::BuiltinFn::kReflect, x, y);
b.Let("w", c);
b.Return(func);
});

tint::hlsl::writer::Options options;
options.polyfill_reflect_vec2_f32 = false;
ASSERT_TRUE(Generate(options)) << err_ << output_.hlsl;
EXPECT_EQ(output_.hlsl, R"(
void foo() {
float2 x = (1.0f).xx;
float2 y = (2.0f).xx;
float2 w = reflect(x, y);
}
)");
}

// The generated HLSL must effectively be emitted as:
// x + (-2.0 * dot(x,y) * y)
// Rather than:
// x - 2.0 * dot(x,y) * y
// See crbug.com/tint/1798
TEST_F(HlslWriterTest, BuiltinReflect_Vec2f32_Polyfill) {
auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
b.Append(func->Block(), [&] {
auto* vec_ty = ty.vec2<f32>();
auto* x = b.Let("x", b.MatchWidth(1_f, vec_ty));
auto* y = b.Let("y", b.MatchWidth(2_f, vec_ty));

auto* c = b.Call(vec_ty, core::BuiltinFn::kReflect, x, y);
b.Let("w", c);
b.Return(func);
});

tint::hlsl::writer::Options options;
options.polyfill_reflect_vec2_f32 = true;
ASSERT_TRUE(Generate(options)) << err_ << output_.hlsl;
EXPECT_EQ(output_.hlsl, R"(
void foo() {
float2 x = (1.0f).xx;
float2 y = (2.0f).xx;
float2 w = (x + (float2(((-2.0f * dot(x, y))).xx) * y));
}
)");
}

} // namespace
} // namespace tint::hlsl::writer
3 changes: 1 addition & 2 deletions src/tint/lang/hlsl/writer/raise/raise.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ Result<SuccessType> Raise(core::ir::Module& module, const Options& options) {
core_polyfills.pack_4xu8_clamp = true;
core_polyfills.pack_unpack_4x8 = options.polyfill_pack_unpack_4x8;
core_polyfills.radians = true;
// TODO(crbug.com/377357960): Implement reflect_vec2_f32
// core_polyfills.reflect_vec2_f32 = options.polyfill_reflect_vec2_f32;
core_polyfills.reflect_vec2_f32 = options.polyfill_reflect_vec2_f32;
core_polyfills.texture_sample_base_clamp_to_edge_2d_f32 = true;
RUN_TRANSFORM(core::ir::transform::BuiltinPolyfill, module, core_polyfills);
}
Expand Down

0 comments on commit ed79e01

Please sign in to comment.