Fix sem::Swizzle::HasSideEffects() not returning true when structure has side-effects

E.g. sem info for "f().x" returned false for HasSideEffects().

Bug: tint:1300
Change-Id: I789f75eef834c58a93e07d93c8334635d39981c3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/83100
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This commit is contained in:
Antonio Maiorano 2022-03-15 13:51:17 +00:00 committed by Tint LUCI CQ
parent 383e9b7b86
commit 93e600dd54
4 changed files with 24 additions and 9 deletions

View File

@ -1715,6 +1715,10 @@ sem::Expression* Resolver::MemberAccessor(
const sem::Type* ret = nullptr; const sem::Type* ret = nullptr;
std::vector<uint32_t> swizzle; std::vector<uint32_t> swizzle;
// Structure may be a side-effecting expression (e.g. function call).
auto* sem_structure = Sem(expr->structure);
bool has_side_effects = sem_structure && sem_structure->HasSideEffects();
if (auto* str = storage_ty->As<sem::Struct>()) { if (auto* str = storage_ty->As<sem::Struct>()) {
Mark(expr->member); Mark(expr->member);
auto symbol = expr->member->symbol; auto symbol = expr->member->symbol;
@ -1741,10 +1745,6 @@ sem::Expression* Resolver::MemberAccessor(
ref->Access()); ref->Access());
} }
// Structure may be a side-effecting expression (e.g. function call).
auto* sem_structure = Sem(expr->structure);
bool has_side_effects = sem_structure && sem_structure->HasSideEffects();
return builder_->create<sem::StructMemberAccess>( return builder_->create<sem::StructMemberAccess>(
expr, ret, current_statement_, member, has_side_effects); expr, ret, current_statement_, member, has_side_effects);
} }
@ -1819,7 +1819,7 @@ sem::Expression* Resolver::MemberAccessor(
static_cast<uint32_t>(size)); static_cast<uint32_t>(size));
} }
return builder_->create<sem::Swizzle>(expr, ret, current_statement_, return builder_->create<sem::Swizzle>(expr, ret, current_statement_,
std::move(swizzle)); std::move(swizzle), has_side_effects);
} }
AddError( AddError(

View File

@ -217,7 +217,7 @@ TEST_F(SideEffectsTest, MemberAccessor_Vector) {
EXPECT_FALSE(sem->HasSideEffects()); EXPECT_FALSE(sem->HasSideEffects());
} }
TEST_F(SideEffectsTest, MemberAccessor_VectorSwizzle) { TEST_F(SideEffectsTest, MemberAccessor_VectorSwizzleNoSE) {
auto* var = Decl(Var("a", ty.vec4<f32>())); auto* var = Decl(Var("a", ty.vec4<f32>()));
auto* expr = MemberAccessor("a", "xzyw"); auto* expr = MemberAccessor("a", "xzyw");
WrapInFunction(var, expr); WrapInFunction(var, expr);
@ -229,6 +229,18 @@ TEST_F(SideEffectsTest, MemberAccessor_VectorSwizzle) {
EXPECT_FALSE(sem->HasSideEffects()); EXPECT_FALSE(sem->HasSideEffects());
} }
TEST_F(SideEffectsTest, MemberAccessor_VectorSwizzleSE) {
MakeSideEffectFunc("se", [&] { return ty.vec4<f32>(); });
auto* expr = MemberAccessor(Call("se"), "xzyw");
WrapInFunction(expr);
EXPECT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(expr);
EXPECT_TRUE(sem->Is<sem::Swizzle>());
ASSERT_NE(sem, nullptr);
EXPECT_TRUE(sem->HasSideEffects());
}
TEST_F(SideEffectsTest, Binary_NoSE) { TEST_F(SideEffectsTest, Binary_NoSE) {
auto* a = Decl(Var("a", ty.i32())); auto* a = Decl(Var("a", ty.i32()));
auto* b = Decl(Var("b", ty.i32())); auto* b = Decl(Var("b", ty.i32()));

View File

@ -46,8 +46,9 @@ StructMemberAccess::~StructMemberAccess() = default;
Swizzle::Swizzle(const ast::MemberAccessorExpression* declaration, Swizzle::Swizzle(const ast::MemberAccessorExpression* declaration,
const sem::Type* type, const sem::Type* type,
const Statement* statement, const Statement* statement,
std::vector<uint32_t> indices) std::vector<uint32_t> indices,
: Base(declaration, type, statement, /* has_side_effects */ false), bool has_side_effects)
: Base(declaration, type, statement, has_side_effects),
indices_(std::move(indices)) {} indices_(std::move(indices)) {}
Swizzle::~Swizzle() = default; Swizzle::~Swizzle() = default;

View File

@ -88,10 +88,12 @@ class Swizzle final : public Castable<Swizzle, MemberAccessorExpression> {
/// @param type the resolved type of the expression /// @param type the resolved type of the expression
/// @param statement the statement that owns this expression /// @param statement the statement that owns this expression
/// @param indices the swizzle indices /// @param indices the swizzle indices
/// @param has_side_effects whether this expression may have side effects
Swizzle(const ast::MemberAccessorExpression* declaration, Swizzle(const ast::MemberAccessorExpression* declaration,
const sem::Type* type, const sem::Type* type,
const Statement* statement, const Statement* statement,
std::vector<uint32_t> indices); std::vector<uint32_t> indices,
bool has_side_effects);
/// Destructor /// Destructor
~Swizzle() override; ~Swizzle() override;