chore: cherry-pick 3 changes from 2-M131 (#45039)

This commit is contained in:
Pedro Pontes 2024-12-16 19:54:56 +00:00 committed by GitHub
parent ead37523ae
commit 2745bcfbca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 2058 additions and 0 deletions

View file

@ -1,2 +1,4 @@
tint_validate_that_align_is_large_enough.patch
ir_fix_robustness_transform_on_textureload_of_sampled_and_depth.patch
tint_validate_layout_constraints_for_all_address_spaces.patch
msl_use_packed_vec3_for_workgroup_storage.patch

File diff suppressed because it is too large Load diff

View file

@ -0,0 +1,126 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Wed, 20 Nov 2024 19:06:01 +0000
Subject: [tint] Validate layout constraints for all address spaces
The WGSL spec has a non-normative note that the layout constraints
should be validated for all non-host-shareable address spaces, using
the same constraints as for storage.
See linked bug for details of why this is important.
Bug: 378725734
Change-Id: I3fb02506d8ded000dc3510bdc1b4a24a95089281
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214754
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
(cherry picked from commit dfa46d12ce63f131c437041d6d97a6d97c54c1b7)
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/215936
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc b/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
index f1e14a36a3cdea07f5fc7735c3d9483515d754f1..6ef150c4476e9176e934a94c32d903c2e23083b8 100644
--- a/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
@@ -730,7 +730,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, RelaxedUniformLayout_ArrayStrid
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
-TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall) {
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Storage) {
// struct S {
// @align(4) vector : vec4u;
// scalar : u32;
@@ -754,5 +754,73 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall) {
56:78 note: 'S' used in address space 'storage' here)");
}
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Workgroup) {
+ // struct S {
+ // @align(4) vector : vec4u;
+ // scalar : u32;
+ // };
+ //
+ // var<workgroup> a : array<S, 4>;
+ Structure(
+ "S", Vector{
+ Member("vector", ty.vec4<u32>(), Vector{MemberAlign(Expr(Source{{12, 34}}, 4_a))}),
+ Member("scalar", ty.u32()),
+ });
+
+ GlobalVar(Source{{56, 78}}, "a", ty("S"), core::AddressSpace::kWorkgroup, Group(0_a));
+
+ ASSERT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: alignment must be a multiple of '16' bytes for the 'workgroup' address space
+56:78 note: 'S' used in address space 'workgroup' here)");
+}
+
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Private) {
+ // struct S {
+ // @align(4) vector : vec4u;
+ // scalar : u32;
+ // };
+ //
+ // var<private> a : array<S, 4>;
+ Structure(
+ "S", Vector{
+ Member("vector", ty.vec4<u32>(), Vector{MemberAlign(Expr(Source{{12, 34}}, 4_a))}),
+ Member("scalar", ty.u32()),
+ });
+
+ GlobalVar(Source{{56, 78}}, "a", ty("S"), core::AddressSpace::kPrivate, Group(0_a));
+
+ ASSERT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: alignment must be a multiple of '16' bytes for the 'private' address space
+56:78 note: 'S' used in address space 'private' here)");
+}
+
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Function) {
+ // struct S {
+ // @align(4) vector : vec4u;
+ // scalar : u32;
+ // };
+ //
+ // fn foo() {
+ // var a : array<S, 4>;
+ // }
+ Structure(
+ "S", Vector{
+ Member("vector", ty.vec4<u32>(), Vector{MemberAlign(Expr(Source{{12, 34}}, 4_a))}),
+ Member("scalar", ty.u32()),
+ });
+
+ GlobalVar(Source{{56, 78}}, "a", ty("S"), core::AddressSpace::kFunction, Group(0_a));
+
+ ASSERT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: alignment must be a multiple of '16' bytes for the 'function' address space
+56:78 note: 'S' used in address space 'function' here)");
+}
+
} // namespace
} // namespace tint::resolver
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index 2dc5253df1658f62e9b092ae40d2360f2c2eb69f..34da774dc1fb4505e53a0985589eaec9820a7a38 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -577,10 +577,6 @@ bool Validator::AddressSpaceLayout(const core::type::Type* store_ty,
return true;
}
- if (!core::IsHostShareable(address_space)) {
- return true;
- }
-
auto note_usage = [&] {
AddNote(source) << style::Type(store_ty->FriendlyName()) << " used in address space "
<< style::Enum(address_space) << " here";

View file

@ -6,3 +6,4 @@ merged_don_t_assume_all_turbofan_frames_are_javascript.patch
merged_wasm_do_not_inline_wrappers_with_ref_extern_parameter.patch
cherry-pick-153d4e84e5d1.patch
cherry-pick-d9893f4856af.patch
merged_liftoff_fix_clobbered_scratch_register.patch

View file

@ -0,0 +1,75 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Clemens Backes <clemensb@chromium.org>
Date: Fri, 15 Nov 2024 16:00:15 +0100
Subject: Merged: [liftoff] Fix clobbered scratch register
`GetMemOp` returns an `Operand` which can contain `kScratchRegister`. We
should hence not clobber that register until after the last use of the
`Operand`.
This CL changes the scratch register to `kScratchRegister2` which has
much fewer uses, and in particular none which collides with `GetMemOp`.
R=mliedtke@chromium.org
Bug: 378779897
(cherry picked from commit 57a017e611a5abfb0e4b59f6de028bc4070a3615)
Change-Id: I43a52d675064dbec8828cb00cb6dcf3287b9dbbf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6050018
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.0@{#39}
Cr-Branched-From: 4be854bd71ea878a25b236a27afcecffa2e29360-refs/heads/13.0.245@{#1}
Cr-Branched-From: 1f5183f7ad6cca21029fd60653d075730c644432-refs/heads/main@{#96103}
diff --git a/src/wasm/baseline/x64/liftoff-assembler-x64-inl.h b/src/wasm/baseline/x64/liftoff-assembler-x64-inl.h
index 4837a9f6f31c792413948cd863b0dec4ec7aebfa..a4657c70ffd4a82e1356cd5105fca4c7d9236a63 100644
--- a/src/wasm/baseline/x64/liftoff-assembler-x64-inl.h
+++ b/src/wasm/baseline/x64/liftoff-assembler-x64-inl.h
@@ -51,6 +51,8 @@ constexpr Operand kInstanceDataOperand =
constexpr Operand kOSRTargetSlot = GetStackSlot(kOSRTargetOffset);
+// Note: The returned Operand might contain {kScratchRegister2}; make sure not
+// to clobber that until after the last use of the Operand.
inline Operand GetMemOp(LiftoffAssembler* assm, Register addr,
Register offset_reg, uintptr_t offset_imm,
ScaleFactor scale_factor = times_1) {
@@ -61,7 +63,7 @@ inline Operand GetMemOp(LiftoffAssembler* assm, Register addr,
: Operand(addr, offset_reg, scale_factor, offset_imm32);
}
// Offset immediate does not fit in 31 bits.
- Register scratch = kScratchRegister;
+ Register scratch = kScratchRegister2;
assm->MacroAssembler::Move(scratch, offset_imm);
if (offset_reg != no_reg) assm->addq(scratch, offset_reg);
return Operand(addr, scratch, scale_factor, 0);
diff --git a/test/mjsunit/regress/wasm/regress-378779897.js b/test/mjsunit/regress/wasm/regress-378779897.js
new file mode 100644
index 0000000000000000000000000000000000000000..fed1bc807165e1b9e83195a2df30aac33a544470
--- /dev/null
+++ b/test/mjsunit/regress/wasm/regress-378779897.js
@@ -0,0 +1,22 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
+
+const builder = new WasmModuleBuilder();
+builder.addMemory(49149);
+
+builder.addFunction('main', kSig_i_v).addBody([
+ ...wasmI32Const(-1118406780),
+ ...wasmI32Const(-1),
+ kAtomicPrefix, kExprI32AtomicOr8U, 0, 0
+]).exportFunc();
+
+let instance;
+try {
+ instance = builder.instantiate();
+} catch (e) {
+ assertException(e, RangeError, /Out of memory/);
+}
+if (instance) instance.exports.main();