Skip to content

ANGLE: Failure to compile TensorFlow.js WebGL shaders #47229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

kkinnunen-apple
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple commented Jun 26, 2025

adfd582

ANGLE: Failure to compile TensorFlow.js WebGL shaders
https://bugs.webkit.org/show_bug.cgi?id=294738
rdar://153836970

Reviewed by Mike Wyrzykowski.

Increment, decrement operators with swizzles would fail to compile for
integer vectors:
    ivec4 vec;
    vec.xyz++;

This would produce:
    metal::int4 _uii;
    ANGLE_postIncrementInt(ANGLE_swizzle_ref(_uii, 0u, 1u, 2u));

ANGLE_postIncrementInt(T &a) would not match because the argument was
rvalue ANGLE_SwizzleRef, and rvalues cannot bind to non-const lvalue
references. Even if this was worked around somehow, the template would
not work because T == ANGLE_SwizzleRef, when the original intention
was that the T would be metal int, int2, int3, int4.

Write the ANGLE_postIncrementInt and other functions without templates.
This way the overload resolution applies the
ANGLE_SwizzleRef::operator intX&() conversion operator. Since the
conversion operator is applied, the returned value is a lvalue reference
and can be bound to the arg.

Due to the use of temporaries changes ANGLE_preIncrementInt,
ANGLE_preDecrementInt functions to return the result by value instead
by reference. In GLSL, these operators are not specified to produce a
lvalue. As per the current code, the translator treats the result as
rvalue.

* Source/ThirdParty/ANGLE/src/compiler/translator/msl/ProgramPrelude.cpp:
* Source/ThirdParty/ANGLE/src/compiler/translator/tree_ops/msl/RewriteUnaddressableReferences.cpp:
(sh::ReturnsReference):
(sh::IsUnaryLValueOp):
* Source/ThirdParty/ANGLE/src/tests/compiler_tests/MSLOutput_test.cpp:
((MSLOutputTest, UintAssignmentOperators)):
((MSLOutputTest, UintSwizzleAssignmentOperators)):
* Source/ThirdParty/ANGLE/src/tests/gl_tests/GLSLTest.cpp:

Canonical link: https://commits.webkit.org/296744@main

fbca0a3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this Jun 26, 2025
@kkinnunen-apple kkinnunen-apple added the ANGLE Bugs related to the ANGLE project label Jun 26, 2025
@kkinnunen-apple kkinnunen-apple force-pushed the angle-preincrement-swizzle-1 branch from 7da6bdd to 5863ae4 Compare June 26, 2025 11:28
@kkinnunen-apple kkinnunen-apple changed the title WebGL: Failure to compile TensorFlow.js WebGL shaders ANGLE: Failure to compile TensorFlow.js WebGL shaders Jun 26, 2025
@kkinnunen-apple kkinnunen-apple force-pushed the angle-preincrement-swizzle-1 branch from 5863ae4 to 259e737 Compare June 26, 2025 11:54
@kkinnunen-apple kkinnunen-apple force-pushed the angle-preincrement-swizzle-1 branch from 259e737 to 8f1f6bb Compare June 26, 2025 17:35
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 26, 2025
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Jun 27, 2025
@kkinnunen-apple kkinnunen-apple force-pushed the angle-preincrement-swizzle-1 branch from 8f1f6bb to fbca0a3 Compare June 27, 2025 12:07
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 27, 2025
@kkinnunen-apple kkinnunen-apple added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jun 27, 2025
https://bugs.webkit.org/show_bug.cgi?id=294738
rdar://153836970

Reviewed by Mike Wyrzykowski.

Increment, decrement operators with swizzles would fail to compile for
integer vectors:
    ivec4 vec;
    vec.xyz++;

This would produce:
    metal::int4 _uii;
    ANGLE_postIncrementInt(ANGLE_swizzle_ref(_uii, 0u, 1u, 2u));

ANGLE_postIncrementInt(T &a) would not match because the argument was
rvalue ANGLE_SwizzleRef, and rvalues cannot bind to non-const lvalue
references. Even if this was worked around somehow, the template would
not work because T == ANGLE_SwizzleRef, when the original intention
was that the T would be metal int, int2, int3, int4.

Write the ANGLE_postIncrementInt and other functions without templates.
This way the overload resolution applies the
ANGLE_SwizzleRef::operator intX&() conversion operator. Since the
conversion operator is applied, the returned value is a lvalue reference
and can be bound to the arg.

Due to the use of temporaries changes ANGLE_preIncrementInt,
ANGLE_preDecrementInt functions to return the result by value instead
by reference. In GLSL, these operators are not specified to produce a
lvalue. As per the current code, the translator treats the result as
rvalue.

* Source/ThirdParty/ANGLE/src/compiler/translator/msl/ProgramPrelude.cpp:
* Source/ThirdParty/ANGLE/src/compiler/translator/tree_ops/msl/RewriteUnaddressableReferences.cpp:
(sh::ReturnsReference):
(sh::IsUnaryLValueOp):
* Source/ThirdParty/ANGLE/src/tests/compiler_tests/MSLOutput_test.cpp:
((MSLOutputTest, UintAssignmentOperators)):
((MSLOutputTest, UintSwizzleAssignmentOperators)):
* Source/ThirdParty/ANGLE/src/tests/gl_tests/GLSLTest.cpp:

Canonical link: https://commits.webkit.org/296744@main
@webkit-commit-queue webkit-commit-queue force-pushed the angle-preincrement-swizzle-1 branch from fbca0a3 to adfd582 Compare June 27, 2025 19:35
@webkit-commit-queue
Copy link
Collaborator

Committed 296744@main (adfd582): https://commits.webkit.org/296744@main

Reviewed commits have been landed. Closing PR #47229 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit adfd582 into WebKit:main Jun 27, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ANGLE Bugs related to the ANGLE project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants