GLSL: implement host-visible memory padding.

Since GLSL ES does not support the offset= attribute, struct members
with explicit @align or @size attributes require adding explicit
padding members.  This in turn requires rewriting any constructor
calls to initialize the new padding to zero, handled in the same
transform.

Note that this is currently overly-verbose, and will add padding where
GLSL doesn't technically need it (e.g., padding a vec3 out to 16 bytes).

Bug: tint:1415
Change-Id: Ia9ba513066a0e84f4c43247fcbbe02f5fadd6630
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101720
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/BUILD.gn

@@ -514,6 +514,8 @@ libtint_source_set("libtint_core_all_src") {
     "transform/multiplanar_external_texture.h",
     "transform/num_workgroups_from_uniform.cc",
     "transform/num_workgroups_from_uniform.h",
+    "transform/pad_structs.cc",
+    "transform/pad_structs.h",
     "transform/promote_initializers_to_let.cc",
     "transform/promote_initializers_to_let.h",
     "transform/promote_side_effects_to_decl.cc",
@@ -1207,6 +1209,7 @@ if (tint_build_unittests) {
       "transform/module_scope_var_to_entry_point_param_test.cc",
       "transform/multiplanar_external_texture_test.cc",
       "transform/num_workgroups_from_uniform_test.cc",
+      "transform/pad_structs_test.cc",
       "transform/promote_initializers_to_let_test.cc",
       "transform/promote_side_effects_to_decl_test.cc",
       "transform/remove_continue_in_switch_test.cc",

src/tint/CMakeLists.txt

@@ -426,6 +426,8 @@ set(TINT_LIB_SRCS
   transform/multiplanar_external_texture.h
   transform/num_workgroups_from_uniform.cc
   transform/num_workgroups_from_uniform.h
+  transform/pad_structs.cc
+  transform/pad_structs.h
   transform/promote_initializers_to_let.cc
   transform/promote_initializers_to_let.h
   transform/promote_side_effects_to_decl.cc
@@ -1123,6 +1125,7 @@ if(TINT_BUILD_TESTS)
       transform/module_scope_var_to_entry_point_param_test.cc
       transform/multiplanar_external_texture_test.cc
       transform/num_workgroups_from_uniform_test.cc
+      transform/pad_structs_test.cc
       transform/promote_initializers_to_let_test.cc
       transform/promote_side_effects_to_decl_test.cc
       transform/remove_continue_in_switch_test.cc

src/tint/transform/pad_structs.cc

@@ -0,0 +1,146 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/tint/transform/pad_structs.h"
+
+#include <string>
+#include <unordered_map>
+#include <utility>
+
+#include "src/tint/ast/parameter.h"
+#include "src/tint/program_builder.h"
+#include "src/tint/sem/call.h"
+#include "src/tint/sem/module.h"
+#include "src/tint/sem/type_constructor.h"
+
+using namespace tint::number_suffixes;  // NOLINT
+
+TINT_INSTANTIATE_TYPEINFO(tint::transform::PadStructs);
+
+namespace tint::transform {
+
+namespace {
+
+void CreatePadding(utils::Vector<const ast::StructMember*, 8>* new_members,
+                   utils::Hashset<const ast::StructMember*, 8>* padding_members,
+                   ProgramBuilder* b,
+                   uint32_t bytes) {
+    for (uint32_t i = 0; i < bytes / 4u; ++i) {
+        auto name = b->Symbols().New("pad");
+        auto* member = b->Member(name, b->ty.u32());
+        padding_members->Add(member);
+        new_members->Push(member);
+    }
+}
+
+}  // namespace
+
+PadStructs::PadStructs() = default;
+
+PadStructs::~PadStructs() = default;
+
+void PadStructs::Run(CloneContext& ctx, const DataMap&, DataMap&) const {
+    auto& sem = ctx.src->Sem();
+
+    std::unordered_map<const ast::Struct*, const ast::Struct*> replaced_structs;
+    utils::Hashset<const ast::StructMember*, 8> padding_members;
+
+    ctx.ReplaceAll([&](const ast::Struct* ast_str) -> const ast::Struct* {
+        auto* str = sem.Get<sem::Struct>(ast_str);
+        if (!str || !str->IsHostShareable()) {
+            return nullptr;
+        }
+        uint32_t offset = 0;
+        bool has_runtime_sized_array = false;
+        utils::Vector<const ast::StructMember*, 8> new_members;
+        for (auto* mem : str->Members()) {
+            auto name = ctx.src->Symbols().NameFor(mem->Name());
+
+            if (offset < mem->Offset()) {
+                CreatePadding(&new_members, &padding_members, ctx.dst, mem->Offset() - offset);
+                offset = mem->Offset();
+            }
+
+            auto* ty = mem->Type();
+            const ast::Type* type = CreateASTTypeFor(ctx, ty);
+
+            new_members.Push(ctx.dst->Member(name, type));
+
+            uint32_t size = ty->Size();
+            if (ty->Is<sem::Struct>() && str->UsedAs(ast::StorageClass::kUniform)) {
+                // std140 structs should be padded out to 16 bytes.
+                size = utils::RoundUp(16u, size);
+            } else if (auto* array_ty = ty->As<sem::Array>()) {
+                if (array_ty->Count() == 0) {
+                    has_runtime_sized_array = true;
+                }
+            }
+            offset += size;
+        }
+
+        // Add any required padding after the last member, if it's not a runtime-sized array.
+        uint32_t struct_size = str->Size();
+        if (str->UsedAs(ast::StorageClass::kUniform)) {
+            struct_size = utils::RoundUp(16u, struct_size);
+        }
+        if (offset < struct_size && !has_runtime_sized_array) {
+            CreatePadding(&new_members, &padding_members, ctx.dst, struct_size - offset);
+        }
+        auto* new_struct = ctx.dst->create<ast::Struct>(ctx.Clone(ast_str->name),
+                                                        std::move(new_members), utils::Empty);
+        replaced_structs[ast_str] = new_struct;
+        return new_struct;
+    });
+
+    ctx.ReplaceAll([&](const ast::CallExpression* ast_call) -> const ast::CallExpression* {
+        if (ast_call->args.Length() == 0) {
+            return nullptr;
+        }
+
+        auto* call = sem.Get<sem::Call>(ast_call);
+        if (!call) {
+            return nullptr;
+        }
+        auto* cons = call->Target()->As<sem::TypeConstructor>();
+        if (!cons) {
+            return nullptr;
+        }
+        auto* str = cons->ReturnType()->As<sem::Struct>();
+        if (!str) {
+            return nullptr;
+        }
+
+        auto* new_struct = replaced_structs[str->Declaration()];
+        if (!new_struct) {
+            return nullptr;
+        }
+
+        utils::Vector<const ast::Expression*, 8> new_args;
+
+        auto* arg = ast_call->args.begin();
+        for (auto* member : new_struct->members) {
+            if (padding_members.Contains(member)) {
+                new_args.Push(ctx.dst->Expr(0_u));
+            } else {
+                new_args.Push(ctx.Clone(*arg));
+                arg++;
+            }
+        }
+        return ctx.dst->Construct(CreateASTTypeFor(ctx, str), new_args);
+    });
+
+    ctx.Clone();
+}
+
+}  // namespace tint::transform

src/tint/transform/pad_structs.h

@@ -0,0 +1,45 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_TINT_TRANSFORM_PAD_STRUCTS_H_
+#define SRC_TINT_TRANSFORM_PAD_STRUCTS_H_
+
+#include "src/tint/transform/transform.h"
+
+namespace tint::transform {
+
+/// This transform turns all explicit alignment and sizing into padding
+/// members of structs. This is required for GLSL ES, since it not support
+/// the offset= decoration.
+class PadStructs final : public Castable<PadStructs, Transform> {
+  public:
+    /// Constructor
+    PadStructs();
+
+    /// Destructor
+    ~PadStructs() override;
+
+  protected:
+    /// Runs the transform using the CloneContext built for transforming a
+    /// program. Run() is responsible for calling Clone() on the CloneContext.
+    /// @param ctx the CloneContext primed with the input program and
+    /// ProgramBuilder
+    /// @param inputs optional extra transform-specific input data
+    /// @param outputs optional extra transform-specific output data
+    void Run(CloneContext& ctx, const DataMap& inputs, DataMap& outputs) const override;
+};
+
+}  // namespace tint::transform
+
+#endif  // SRC_TINT_TRANSFORM_PAD_STRUCTS_H_

src/tint/transform/pad_structs_test.cc

@@ -0,0 +1,597 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/tint/transform/pad_structs.h"
+
+#include <memory>
+#include <utility>
+
+#include "src/tint/transform/test_helper.h"
+
+namespace tint::transform {
+namespace {
+
+using PadStructsTest = TransformTest;
+
+TEST_F(PadStructsTest, EmptyModule) {
+    auto* src = "";
+    auto* expect = src;
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, Uniform) {
+    auto* src = R"(
+struct S {
+  x : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+fn main() {
+  let x = u.x;
+}
+)";
+    auto* expect = R"(
+struct S {
+  x : i32,
+  pad : u32,
+  pad_1 : u32,
+  pad_2 : u32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+fn main() {
+  let x = u.x;
+}
+)";
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, Size) {
+    auto* src = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+fn main() {
+  let x = u.x;
+}
+)";
+    auto* expect = R"(
+struct S {
+  x : i32,
+  pad : u32,
+  pad_1 : u32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+fn main() {
+  let x = u.x;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, SizeUniformAndPrivate) {
+    auto* src = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+var<private> p : S;
+
+fn main() {
+  p.x = u.x;
+}
+)";
+    auto* expect = R"(
+struct S {
+  x : i32,
+  pad : u32,
+  pad_1 : u32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+var<private> p : S;
+
+fn main() {
+  p.x = u.x;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, SizeStorageAndPrivate) {
+    auto* src = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+var<private> p : S;
+
+fn main() {
+  p.x = 123;
+  s.x = p.x;
+}
+)";
+    auto* expect = R"(
+struct S {
+  x : i32,
+  pad : u32,
+  pad_1 : u32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+var<private> p : S;
+
+fn main() {
+  p.x = 123;
+  s.x = p.x;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, SizeUniformAndStorage) {
+    auto* src = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+@group(0) @binding(1) var<storage, read_write> s : S;
+
+fn main() {
+  s.x = u.x;
+}
+)";
+    auto* expect = R"(
+struct S {
+  x : i32,
+  pad : u32,
+  pad_1 : u32,
+  y : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+@group(0) @binding(1) var<storage, read_write> s : S;
+
+fn main() {
+  s.x = u.x;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, SizePrivateOnly) {
+    // Structs that are not host-visible should have no explicit padding.
+    auto* src = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+var<private> p : S;
+
+fn main() {
+  p.x = 123;
+}
+)";
+    auto* expect = R"(
+struct S {
+  @size(12)
+  x : i32,
+  y : i32,
+}
+
+var<private> p : S;
+
+fn main() {
+  p.x = 123;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, AlignUniformAndPrivate) {
+    auto* src = R"(
+struct S {
+  a : i32,
+  @align(16)
+  b : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+var<private> p : S;
+
+fn main() {
+  p.a = u.b;
+  p.b = u.a;
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : i32,
+  pad : u32,
+  pad_1 : u32,
+  pad_2 : u32,
+  b : i32,
+  pad_3 : u32,
+  pad_4 : u32,
+  pad_5 : u32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+var<private> p : S;
+
+fn main() {
+  p.a = u.b;
+  p.b = u.a;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, AlignStorageAndPrivate) {
+    auto* src = R"(
+struct S {
+  a : i32,
+  @align(16)
+  b : i32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+var<private> p : S;
+
+fn main() {
+  p.a = 123;
+  p.b = 321;
+  s.a = p.b;
+  s.b = p.a;
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : i32,
+  pad : u32,
+  pad_1 : u32,
+  pad_2 : u32,
+  b : i32,
+  pad_3 : u32,
+  pad_4 : u32,
+  pad_5 : u32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+var<private> p : S;
+
+fn main() {
+  p.a = 123;
+  p.b = 321;
+  s.a = p.b;
+  s.b = p.a;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, AlignUniformAndStorage) {
+    auto* src = R"(
+struct S {
+  a : i32,
+  @align(16)
+  b : i32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+@group(0) @binding(1) var<storage, read_write> s : S;
+
+fn main() {
+  s.a = u.b;
+  s.b = u.a;
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : i32,
+  pad : u32,
+  pad_1 : u32,
+  pad_2 : u32,
+  b : i32,
+  pad_3 : u32,
+  pad_4 : u32,
+  pad_5 : u32,
+}
+
+@group(0) @binding(0) var<uniform> u : S;
+
+@group(0) @binding(1) var<storage, read_write> s : S;
+
+fn main() {
+  s.a = u.b;
+  s.b = u.a;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, AlignPrivateOnly) {
+    // Structs that are not host-visible should have no explicit padding.
+    auto* src = R"(
+struct S {
+  a : i32,
+  @align(16)
+  b : i32,
+}
+
+var<private> p : S;
+
+fn main() {
+  p.a = 123;
+  p.b = 321;
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : i32,
+  @align(16)
+  b : i32,
+}
+
+var<private> p : S;
+
+fn main() {
+  p.a = 123;
+  p.b = 321;
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, LastMemberRuntimeSizeArray) {
+    // Structs with runtime-sized arrays should not be padded after the
+    // last member.
+    auto* src = R"(
+struct T {
+  a : f32,
+  b : i32,
+}
+
+struct S {
+  a : vec4<f32>,
+  b : array<T>,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s.b[0] = T(1.0f, 23);
+}
+)";
+    auto* expect = R"(
+struct T {
+  a : f32,
+  b : i32,
+}
+
+struct S {
+  a : vec4<f32>,
+  b : array<T>,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s.b[0] = T(1.0f, 23);
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, LastMemberFixedSizeArray) {
+    // Structs without runtime-sized arrays should be padded after the last
+    // member.
+    auto* src = R"(
+struct T {
+  a : f32,
+  b : i32,
+}
+
+struct S {
+  a : vec4<f32>,
+  b : array<T, 1u>,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s.b[0] = T(1.0f, 23);
+}
+)";
+    auto* expect = R"(
+struct T {
+  a : f32,
+  b : i32,
+}
+
+struct S {
+  a : vec4<f32>,
+  b : array<T, 1u>,
+  pad : u32,
+  pad_1 : u32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s.b[0] = T(1.0f, 23);
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, Constructor) {
+    // Calls to a constructor of a padded struct must be modified to initialize the padding.
+    auto* src = R"(
+struct S {
+  a : f32,
+  @align(8)
+  b : i32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s = S(1.0f, 2);
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : f32,
+  pad : u32,
+  b : i32,
+  pad_1 : u32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s = S(1.0f, 0u, 2, 0u);
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, ConstructorZeroArgs) {
+    // Calls to a zero-argument constructor of a padded struct should not be modified.
+    auto* src = R"(
+struct S {
+  a : f32,
+  @align(8)
+  b : i32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s = S();
+}
+)";
+    auto* expect = R"(
+struct S {
+  a : f32,
+  pad : u32,
+  b : i32,
+  pad_1 : u32,
+}
+
+@group(0) @binding(0) var<storage, read_write> s : S;
+
+fn main() {
+  s = S();
+}
+)";
+
+    DataMap data;
+    auto got = Run<PadStructs>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
+}  // namespace
+}  // namespace tint::transform

src/tint/writer/glsl/generator_impl.cc

@@ -58,6 +58,7 @@
 #include "src/tint/transform/fold_trivial_single_use_lets.h"
 #include "src/tint/transform/loop_to_for_loop.h"
 #include "src/tint/transform/manager.h"
+#include "src/tint/transform/pad_structs.h"
 #include "src/tint/transform/promote_initializers_to_let.h"
 #include "src/tint/transform/promote_side_effects_to_decl.h"
 #include "src/tint/transform/remove_phonies.h"
@@ -220,6 +221,7 @@ SanitizedResult Sanitize(const Program* in,
     manager.Add<transform::ExpandCompoundAssignment>();
     manager.Add<transform::PromoteSideEffectsToDecl>();
     manager.Add<transform::Std140>();  // Must come after PromoteSideEffectsToDecl
+    manager.Add<transform::PadStructs>();
     manager.Add<transform::UnwindDiscardFunctions>();
     manager.Add<transform::SimplifyPointers>();
 
@@ -1910,13 +1912,10 @@ bool GeneratorImpl::EmitUniformVariable(const ast::Var* var, const sem::Variable
     auto bp = sem->As<sem::GlobalVariable>()->BindingPoint();
     {
         auto out = line();
-        out << "layout(binding = " << bp.binding;
-        if (version_.IsDesktop()) {
-            out << ", std140";
-        }
+        out << "layout(binding = " << bp.binding << ", std140";
         out << ") uniform " << UniqueIdentifier(StructName(str) + "_ubo") << " {";
     }
-    EmitStructMembers(current_buffer_, str, /* emit_offsets */ true);
+    EmitStructMembers(current_buffer_, str);
     auto name = builder_.Symbols().NameFor(var->symbol);
     line() << "} " << name << ";";
     line();
@@ -1934,7 +1933,7 @@ bool GeneratorImpl::EmitStorageVariable(const ast::Var* var, const sem::Variable
     auto bp = sem->As<sem::GlobalVariable>()->BindingPoint();
     line() << "layout(binding = " << bp.binding << ", std430) buffer "
            << UniqueIdentifier(StructName(str) + "_ssbo") << " {";
-    EmitStructMembers(current_buffer_, str, /* emit_offsets */ true);
+    EmitStructMembers(current_buffer_, str);
     auto name = builder_.Symbols().NameFor(var->symbol);
     line() << "} " << name << ";";
     line();
@@ -2859,7 +2858,7 @@ bool GeneratorImpl::EmitTypeAndName(std::ostream& out,
 bool GeneratorImpl::EmitStructType(TextBuffer* b, const sem::Struct* str) {
     auto storage_class_uses = str->StorageClassUsage();
     line(b) << "struct " << StructName(str) << " {";
-    EmitStructMembers(b, str, false);
+    EmitStructMembers(b, str);
     line(b) << "};";
     line(b);
 
@@ -2874,7 +2873,7 @@ bool GeneratorImpl::EmitStructTypeOnce(TextBuffer* buffer, const sem::Struct* st
     return EmitStructType(buffer, str);
 }
 
-bool GeneratorImpl::EmitStructMembers(TextBuffer* b, const sem::Struct* str, bool emit_offsets) {
+bool GeneratorImpl::EmitStructMembers(TextBuffer* b, const sem::Struct* str) {
     ScopedIndent si(b);
     for (auto* mem : str->Members()) {
         auto name = builder_.Symbols().NameFor(mem->Name());
@@ -2883,10 +2882,6 @@ bool GeneratorImpl::EmitStructMembers(TextBuffer* b, const sem::Struct* str, boo
 
         auto out = line(b);
 
-        // Note: offsets are unsupported on GLSL ES.
-        if (emit_offsets && version_.IsDesktop() && mem->Offset() != 0) {
-            out << "layout(offset=" << mem->Offset() << ") ";
-        }
         if (!EmitTypeAndName(out, ty, ast::StorageClass::kNone, ast::Access::kReadWrite, name)) {
             return false;
         }

src/tint/writer/glsl/generator_impl.h

@@ -432,9 +432,8 @@ class GeneratorImpl : public TextGenerator {
     /// Handles generating the members of a structure
     /// @param buffer the text buffer that the struct members will be written to
     /// @param ty the struct to generate
-    /// @param emit_offsets whether offsets should be emitted as offset=
     /// @returns true if the struct members are emitted
-    bool EmitStructMembers(TextBuffer* buffer, const sem::Struct* ty, bool emit_offsets);
+    bool EmitStructMembers(TextBuffer* buffer, const sem::Struct* ty);
     /// Handles a unary op expression
     /// @param out the output of the expression stream
     /// @param expr the expression to emit

src/tint/writer/glsl/generator_impl_function_test.cc

@@ -384,7 +384,7 @@ struct UBO {
   vec4 coord;
 };
 
-layout(binding = 0) uniform UBO_ubo {
+layout(binding = 0, std140) uniform UBO_ubo {
   vec4 coord;
 } ubo;
 
@@ -425,7 +425,7 @@ struct Uniforms {
   vec4 coord;
 };
 
-layout(binding = 0) uniform Uniforms_ubo {
+layout(binding = 0, std140) uniform Uniforms_ubo {
   vec4 coord;
 } uniforms;
 
@@ -635,7 +635,7 @@ struct S {
   float x;
 };
 
-layout(binding = 0) uniform S_ubo {
+layout(binding = 0, std140) uniform S_ubo {
   float x;
 } coord;
 

src/tint/writer/glsl/generator_impl_member_accessor_test.cc

@@ -279,6 +279,9 @@ precision mediump float;
 
 layout(binding = 0, std430) buffer Data_ssbo {
   int a;
+  uint pad;
+  uint pad_1;
+  uint pad_2;
   mat2x3 b;
 } data;
 
@@ -320,6 +323,9 @@ precision mediump float;
 
 layout(binding = 0, std430) buffer Data_ssbo {
   float z;
+  uint pad;
+  uint pad_1;
+  uint pad_2;
   mat4x3 a;
 } data;
 
@@ -497,7 +503,9 @@ precision mediump float;
 
 struct Inner {
   vec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {
@@ -552,7 +560,9 @@ precision mediump float;
 
 struct Inner {
   vec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {
@@ -608,7 +618,9 @@ precision mediump float;
 
 struct Inner {
   vec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {
@@ -663,7 +675,9 @@ precision mediump float;
 
 struct Inner {
   vec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {
@@ -717,7 +731,9 @@ precision mediump float;
 
 struct Inner {
   vec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {
@@ -772,7 +788,9 @@ precision mediump float;
 
 struct Inner {
   ivec3 a;
+  uint pad;
   vec3 b;
+  uint pad_1;
 };
 
 layout(binding = 0, std430) buffer Data_ssbo {

src/tint/writer/glsl/generator_impl_storage_buffer_test.cc

@@ -82,8 +82,8 @@ struct Nephews {
 
 layout(binding = 0, std430) buffer Nephews_ssbo {
   float huey;
-  layout(offset=256) float dewey;
-  layout(offset=512) float louie;
+  float dewey;
+  float louie;
 } nephews;
 
 )");

src/tint/writer/glsl/generator_impl_uniform_buffer_test.cc

@@ -37,7 +37,7 @@ struct Simple {
   float member;
 };
 
-layout(binding = 0) uniform Simple_ubo {
+layout(binding = 0, std140) uniform Simple_ubo {
   float member;
 } simple;
 

src/tint/writer/spirv/builder_block_test.cc

@@ -26,8 +26,7 @@ TEST_F(BuilderTest, Block) {
     // Note, this test uses shadow variables which aren't allowed in WGSL but
     // serves to prove the block code is pushing new scopes as needed.
     auto* inner = Block(Decl(Var("var", ty.f32())), Assign("var", 2_f));
-    auto* outer = Block(Decl(Var("var", ty.f32())), Assign("var", 1_f),
-                        inner, Assign("var", 3_f));
+    auto* outer = Block(Decl(Var("var", ty.f32())), Assign("var", 1_f), inner, Assign("var", 3_f));
 
     WrapInFunction(outer);