Issue 8558 in angleproject: Metal: Rewritten fragment inputs may have conflicting names

[kkinn… via monorail]

Status: Started
Owner: kkinn...@apple.com
CC: k...@chromium.org, lexa....@gmail.com
Components: Translator
OS: Mac iOS
Priority: Medium
Renderer: Metal
Type: Defect

New issue 8558 by kkinn...@apple.com: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558

varying mat2 a_;
varying mat3 a;
void main(){
gl_FragColor = vec4(a_) + vec4(a);
}

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[kkinn… via monorail]

Comment #1 on issue 8558 by kkinn...@apple.com: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c1

* frame #0: 0x000000010128a830 angle_unittests`gl::LogMessage::~LogMessage(this=0x000000016fdfa9a0) at debug.cpp:208:13
frame #1: 0x000000010128a1f8 angle_unittests`gl::LogMessage::~LogMessage(this=0x000000016fdfa9a0) at debug.cpp:182:1
frame #2: 0x0000000101521c18 angle_unittests`sh::TIntermBinary::promote(this=0x000000013b0352e0) at IntermNode.cpp:1907:13
frame #3: 0x0000000101520f7c angle_unittests`sh::TIntermBinary::TIntermBinary(this=0x000000013b0352e0, op=EOpAssign, left=0x000000013b034b40, right=0x000000013b034f40) at IntermNode.cpp:1552:5
frame #4: 0x0000000101519e6c angle_unittests`sh::TIntermBinary::TIntermBinary(this=0x000000013b0352e0, op=EOpAssign, left=0x000000013b034b40, right=0x000000013b034f40) at IntermNode.cpp:1549:1
frame #5: 0x000000010176ad78 angle_unittests`(anonymous namespace)::ConvertStructState::publish(this=0x000000016fdfbb20, original=0x000000013b034b40, modified=0x000000013b034f40)::'lambda'(sh::TIntermTyped&, sh::TIntermTyped&)::operator()(sh::TIntermTyped&, sh::TIntermTyped&) const at ModifyStruct.cpp:230:46
frame #6: 0x0000000101760ea0 angle_unittests`(anonymous namespace)::ConvertStructState::publish(this=0x000000016fdfc0f8, originalStruct=0x000000013b031ae0, modifiedStructName=0x000000016fdfce28) at ModifyStruct.cpp:280:17
frame #7: 0x000000010176007c angle_unittests`sh::TryCreateModifiedStruct(compiler=0x000000013b01a800, symbolEnv=0x000000016fdfdf68, idGen=0x000000016fdfdd00, config=0x000000016fdfce40, originalStruct=0x000000013b031ae0, modifiedStructName=0x000000016fdfce28, outMachineries=0x000000016fdfce98, isUBO=false, allowPadding=true, useAttributeAliasing=false) at ModifyStruct.cpp:1089:11
frame #8: 0x000000010177768c angle_unittests`(anonymous namespace)::GeneratePipelineStruct::exec(this=0x000000016fdfcf70, root=0x000000013b02b6f0) at RewritePipelines.cpp:147:31
frame #9: 0x0000000101776398 angle_unittests`(anonymous namespace)::GeneratePipelineStruct::Exec(out=0x000000016fdfd288, compiler=0x000000013b01a800, root=0x000000013b02b6f0, idGen=0x000000016fdfdd00, pipeline=0x000000016fdfd4c0, symbolEnv=0x000000016fdfdf68, variableInfos=0x000000013b01a8b0 size=2) at RewritePipelines.cpp:95:19
frame #10: 0x0000000101776018 angle_unittests`(anonymous namespace)::RewritePipeline(compiler=0x000000013b01a800, root=0x000000013b02b6f0, idGen=0x000000016fdfdd00, pipeline=0x000000016fdfd4c0, symbolEnv=0x000000016fdfdf68, variableInfo=0x000000013b01a8b0 size=2, outStruct=0x000000016fdfdae8) at RewritePipelines.cpp:985:10
frame #11: 0x0000000101775df8 angle_unittests`sh::RewritePipelines(compiler=0x000000013b01a800, root=0x000000013b02b6f0, inputVaryings=size=2, outputVaryings=size=0, idGen=0x000000016fdfdd00, angleUniformsGlobalInstanceVar=0x000000016fdfe190, symbolEnv=0x000000016fdfdf68, outStructs=0x000000016fdfdae8) at RewritePipelines.cpp:1079:14
frame #12: 0x00000001017b2b84 angle_unittests`sh::TranslatorMSL::translateImpl(this=0x000000013b01a800, sink=0x000000013b01b3f8, root=0x000000013b02b6f0, compileOptions=0x000000016fdfe290, (null)=0x000000016fdfe280, specConst=0x000000016fdfe1b8, driverUniforms=0x000000016fdfe190) at TranslatorMSL.cpp:1497:10
frame #13: 0x00000001017b66e4 angle_unittests`sh::TranslatorMSL::translate(this=0x000000013b01a800, root=0x000000013b02b6f0, compileOptions=0x000000016fdfe290, perfDiagnostics=0x000000016fdfe280) at TranslatorMSL.cpp:1561:10
frame #14: 0x00000001014fcef4 angle_unittests`sh::TCompiler::compile(this=0x000000013b01a800, shaderStrings=0x000000016fdfe360, numStrings=1, compileOptionsIn=0x000000016fdfe340) at Compiler.cpp:1343:18
frame #15: 0x00000001003e92e4 angle_unittests`sh::compileTestShader(type=35632, spec=SH_GLES3_1_SPEC, output=SH_MSL_METAL_OUTPUT, shaderString="precision mediump float;\nvarying mat2 a_;\nvarying mat3 a;\nvoid main(){\n gl_FragColor = vec4(a_) + vec4(a);\n}", resources=0x000000013babd088, compileOptions=0x000000013babd068, translatedCode="", infoLog="") at compiler_test.cpp:83:44
frame #16: 0x00000001003e9cb0 angle_unittests`sh::MatchOutputCodeTest::compileWithSettings(this=0x000000013babd050, output=SH_MSL_METAL_OUTPUT, shaderString="precision mediump float;\nvarying mat2 a_;\nvarying mat3 a;\nvoid main(){\n gl_FragColor = vec4(a_) + vec4(a);\n}", compileOptions=0x000000013babd068, translatedCode="", infoLog="") at compiler_test.cpp:161:12
frame #17: 0x00000001003e9a60 angle_unittests`sh::MatchOutputCodeTest::compile(this=0x000000013babd050, shaderString="precision mediump float;\nvarying mat2 a_;\nvarying mat3 a;\nvoid main(){\n gl_FragColor = vec4(a_) + vec4(a);\n}", compileOptions=0x000000013babd068) at compiler_test.cpp:147:13
frame #18: 0x00000001003e9964 angle_unittests`sh::MatchOutputCodeTest::compile(this=0x000000013babd050, shaderString="precision mediump float;\nvarying mat2 a_;\nvarying mat3 a;\nvoid main(){\n gl_FragColor = vec4(a_) + vec4(a);\n}") at compiler_test.cpp:137:5
frame #19: 0x00000001006f30b0 angle_unittests`MSLOutputTest_VaryingRewriteUnderscoreNoCrash_Test::TestBody(this=0x000000013babd050) at MSLOutput_test.cpp:875:5
frame #20: 0x00000001007a4e58 angle_unittests`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x000000013babd050, method=0x00000000000000010000000000000020, location="the test body") at gtest.cc:2631:10
frame #21: 0x000000010077b6cc angle_unittests`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x000000013babd050, method=0x00000000000000010000000000000020, location="the test body") at gtest.cc:2686:12
frame #22: 0x000000010077b61c angle_unittests`testing::Test::Run(this=0x000000013babd050) at gtest.cc:2706:5
frame #23: 0x000000010077c0f8 angle_unittests`testing::TestInfo::Run(this=0x000000013a7baa80) at gtest.cc:2885:11
frame #24: 0x000000010077cfc8 angle_unittests`testing::TestSuite::Run(this=0x000000013a7b6ff0) at gtest.cc:3039:28
frame #25: 0x0000000100787d6c angle_unittests`testing::internal::UnitTestImpl::RunAllTests(this=0x000000013a7080a0) at gtest.cc:5897:44
frame #26: 0x00000001007a56e0 angle_unittests`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x000000013a7080a0, method=(angle_unittests`testing::internal::UnitTestImpl::RunAllTests() at gtest.cc:5772), location="auxiliary test code (environments or event listeners)") at gtest.cc:2631:10
frame #27: 0x000000010078786c angle_unittests`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x000000013a7080a0, method=(angle_unittests`testing::internal::UnitTestImpl::RunAllTests() at gtest.cc:5772), location="auxiliary test code (environments or event listeners)") at gtest.cc:2686:12
frame #28: 0x000000010078776c angle_unittests`testing::UnitTest::Run(this=0x0000000101be30f0) at gtest.cc:5464:10
frame #29: 0x000000010070eb48 angle_unittests`RUN_ALL_TESTS() at gtest.h:2492:46
frame #30: 0x000000010070dee0 angle_unittests`angle::TestSuite::run(this=0x000000016fdfec20) at TestSuite.cpp:1660:22
frame #31: 0x00000001004b5f8c angle_unittests`main(argc=2, argv=0x000000016fdff178) at angle_unittest_main.cpp:46:22
frame #32: 0x00000001806a60e0 dyld`start + 2360

[Git Watcher via monorail]

Comment #2 on issue 8558 by Git Watcher: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c2

The following revision refers to this bug:
https://chromium.googlesource.com/angle/angle/+/27423bffffaea5bc48e62e3fd2192387d357f224

commit 27423bffffaea5bc48e62e3fd2192387d357f224
Author: Kimmo Kinnunen <kkin...@apple.com>
Date: Tue Mar 05 15:57:24 2024

Metal: Generate names for rewritten inputs

When expanding multi-component fields to multiple single-component
fields, use AngleInternal namespace for the new names. The names are
generated with form "someField_0" where _0 is the component index.
If these are not created in AngleInternal, caller is able to create
a name clash by introducing single-component field "someField_0".

Fixes an assert where the vec4(a_) + vec4(a) would assert on size
mismatch because the variable lookup for "a_" would find a rewritten
variable for the expanded matrix row of "a".

Bug: angleproject:8558
Change-Id: I64b7a755d7d534543fdb0f4c43008dd5c63f4aad
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5323060
Auto-Submit: Kimmo Kinnunen <kkin...@apple.com>
Reviewed-by: Kenneth Russell <k...@chromium.org>
Commit-Queue: Kenneth Russell <k...@chromium.org>
Reviewed-by: Kyle Piddington <kpidd...@apple.com>

[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/compiler/translator/msl/Name.cpp
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/compiler/translator/msl/Name.h
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/tests/compiler_tests/MSLOutput_test.cpp
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/compiler/translator/msl/ModifyStruct.cpp
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/libANGLE/renderer/metal/mtl_common.h
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/compiler/translator/msl/AstHelpers.h
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/libANGLE/renderer/metal/mtl_msl_utils.mm
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/tests/gl_tests/VertexAttributeTest.cpp
[modify] https://crrev.com/27423bffffaea5bc48e62e3fd2192387d357f224/src/compiler/translator/msl/AstHelpers.cpp

[Git Watcher via monorail]

Comment #3 on issue 8558 by Git Watcher: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c3

The following revision refers to this bug:

https://chromium.googlesource.com/angle/angle/+/4e9fbb36f2d87b7c0e227e95066ae21ab1901293

commit 4e9fbb36f2d87b7c0e227e95066ae21ab1901293
Author: Kimmo Kinnunen <kkin...@apple.com>
Date: Wed Mar 06 13:06:07 2024

Metal: Remove AccessField(.., ImmutableString)

The variant is incorrect, as ImmutableString does not describe the field
name completely. Names belong to different namespaces denoted by
SymbolType.

Bug: angleproject:8558
Change-Id: I8332f574dfa8439d02af119fd858a6f5acdec73d
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5349698
Commit-Queue: Kenneth Russell <k...@chromium.org>

Auto-Submit: Kimmo Kinnunen <kkin...@apple.com>
Reviewed-by: Kenneth Russell <k...@chromium.org>

[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/msl/RewritePipelines.cpp
[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/msl/ModifyStruct.cpp
[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/msl/ToposortStructs.cpp
[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/msl/AstHelpers.h
[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/tree_ops/msl/ReduceInterfaceBlocks.cpp
[modify] https://crrev.com/4e9fbb36f2d87b7c0e227e95066ae21ab1901293/src/compiler/translator/msl/AstHelpers.cpp

[Git Watcher via monorail]

Comment #4 on issue 8558 by Git Watcher: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c4

The following revision refers to this bug:

https://chromium.googlesource.com/angle/angle/+/533fa5cedc6164dbb98279f1dfa339f29b66247e

commit 533fa5cedc6164dbb98279f1dfa339f29b66247e
Author: Kimmo Kinnunen <kkin...@apple.com>
Date: Sat Mar 09 08:15:33 2024

Metal: Update few asserts when resolving FS output

Update few asserts when resolving FS output locations and indexes.

Bug: angleproject:8558
Change-Id: Ic3c3c1df19b2de9db78a73b1c14f26444442d74f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5358814
Auto-Submit: Kimmo Kinnunen <kkin...@apple.com>
Commit-Queue: Kimmo Kinnunen <kkin...@apple.com>
Reviewed-by: Alexey Knyazev <lexa.k...@gmail.com>
Reviewed-by: Kenneth Russell <k...@chromium.org>

[modify] https://crrev.com/533fa5cedc6164dbb98279f1dfa339f29b66247e/src/libANGLE/renderer/metal/mtl_msl_utils.mm

[k… via monorail]

Comment #5 on issue 8558 by k...@chromium.org: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c5

Note, this is effectively blocked on crbug.com/335744344 - can't make that association right now while ANGLE and Chromium are in different issue trackers - but that other bug is important to backport since it may have unknown consequences on existing WebGL content.

[kkinn… via monorail]

Updates:
Status: Fixed

Comment #6 on issue 8558 by kkinn...@apple.com: Metal: Rewritten fragment inputs may have conflicting names
https://bugs.chromium.org/p/angleproject/issues/detail?id=8558#c6

(No comment was entered for this change.)