Skip to content
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

Implement constant folding for many transcendentals #3166

Merged
merged 2 commits into from
Feb 3, 2020
Merged

Implement constant folding for many transcendentals #3166

merged 2 commits into from
Feb 3, 2020

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Jan 29, 2020

This change adds support for folding of sin/cos/tan/asin/acos/atan,
exp/log/exp2/log2, sqrt, atan2 and pow.

The mechanism allows to use any C function to implement folding in the
future; for now I limited the actual additions to the most commonly used
intrinsics in the shaders.

Unary folder had to be tweaked to work with extended instructions - for
extended instructions, constants.size() == 2 and constants[0] ==
nullptr. This adjustment is similar to the one binary folder already
performs.

Fixes #1390.

This change adds support for folding of sin/cos/tan/asin/acos/atan,
exp/log/exp2/log2, sqrt, atan2 and pow.

The mechanism allows to use any C function to implement folding in the
future; for now I limited the actual additions to the most commonly used
intrinsics in the shaders.

Unary folder had to be tweaked to work with extended instructions - for
extended instructions, constants.size() == 2 and constants[0] ==
nullptr. This adjustment is similar to the one binary folder already
performs.

Fixes #1390.
Copy link
Collaborator

@s-perron s-perron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This looks great.

@zeux
Copy link
Contributor Author

zeux commented Jan 30, 2020

Apparently Android's standard library implementation used by the build doesn't have std::exp2 :-/ I think this is because it's using a (deprecated) libstdc++ library instead of libc++...

I'm going to change exp2/log2 to use global (C99) versions.

@zeux
Copy link
Contributor Author

zeux commented Jan 30, 2020

This didn't solve this completely :(

The build looks like it's using r15c NDK and targets Android 15 ABI (4.1).
I've downloaded r15c NDK and somehow it has exp2 but not log2 in that ABI. It only has log2 starting from Android 18 (4.3).

I'm surprised this still targets the toolchain/ABI this old, but I could implement a shim that uses log(value) / log(2.0) for Android which should hopefully fix this.

On old versions of Android NDK, we don't get std::exp2/std::log2
because of partial C++11 support.

We do get ::exp2, but not ::log2 so we need to emulate that.
@zeux
Copy link
Contributor Author

zeux commented Jan 30, 2020

clang-debug build timed out during building so probably unrelated to this change; at least Android is green now 😅

@s-perron
Copy link
Collaborator

macos builds passed earlier, so that is not an issue. I just want to take a little time to see if it is possible to avoid the if-def that was added in the middle of AddFoldingRules. It is a bit of an eyesore.

@zeux
Copy link
Contributor Author

zeux commented Jan 31, 2020

macos builds passed earlier, so that is not an issue. I just want to take a little time to see if it is possible to avoid the if-def that was added in the middle of AddFoldingRules. It is a bit of an eyesore.

Sure, but it's a necessity. I don't know why the Android ABI that is used is so low - if the ABI can be bumped, we can use global exp2/log2 unconditionally. If the Android build stops using deprecated STL implementation and becomes C++11 compliant, we can use C++11 std::exp2/std::log2.

Alternatively we can always use ::exp2 and log2 emulation but it seems wrong to do this because surely, eventually Android builds will migrate to modern versions of the toolchain.

@s-perron
Copy link
Collaborator

s-perron commented Feb 3, 2020

I'll fix this up later. I know we cannot remove the if-def completely. However, it could be moved somewhere out of the way.

@s-perron s-perron merged commit 0265a9d into KhronosGroup:master Feb 3, 2020
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ 19ec0d2..5e86b28 (3 commits)

KhronosGroup/glslang@19ec0d2...5e86b28

$ git log 19ec0d2..5e86b28 --date=short --no-merges --format='%ad %ae %s'
2020-01-29 jbolz Use NOT ... VERSION_LESS instead of VERSION_GREATER_EQUAL
2020-01-28 jordan.l.justen standalone: Fix --help
2020-01-27 rharrison Use correct enum type in case statement

Roll third_party/re2/ 85c014206..05faa8db3 (2 commits)

google/re2@85c0142...05faa8d

$ git log 85c014206..05faa8db3 --date=short --no-merges --format='%ad %ae %s'
2020-01-31 junyer Avoid using the forward DFA in "ANCHOR_END" cases.
2020-01-29 junyer Make the fuzzer use FuzzedDataProvider.

Roll third_party/spirv-cross/ 68bf0f824..6b2add8e2 (4 commits)

KhronosGroup/SPIRV-Cross@68bf0f8...6b2add8

$ git log 68bf0f824..6b2add8e2 --date=short --no-merges --format='%ad %ae %s'
2020-02-03 post Use GNUInstallDirs for include path as well.
2020-02-01 orbea cmake: Don't hardcode the pkg-config file.
2020-02-01 orbea cmake: Use GNUInstallDirs.
2020-02-01 post CMake: Avoid warning when parent project uses VERSION in project().

Roll third_party/spirv-tools/ 1b34410..ddcc117 (7 commits)

KhronosGroup/SPIRV-Tools@1b34410...ddcc117

$ git log 1b34410..ddcc117 --date=short --no-merges --format='%ad %ae %s'
2020-02-03 stevenperron Update CHANGES
2020-02-03 arseny.kapoulkine Implement constant folding for many transcendentals (KhronosGroup#3166)
2020-01-30 afdx Fix typo in comment. (KhronosGroup#3163)
2020-01-30 afdx spirv-fuzz: Arbitrary variable facts (KhronosGroup#3165)
2020-01-29 afdx spirv-fuzz: Add outlining test (KhronosGroup#3164)
2020-01-29 afdx spirv-fuzz: Make functions "livesafe" during donation (KhronosGroup#3146)
2020-01-28 stevenperron Dead branch elim fix (KhronosGroup#3160)

Created with:
  roll-dep third_party/effcee third_party/glslang third_party/googletest third_party/re2 third_party/spirv-cross third_party/spirv-headers third_party/spirv-tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing fp const folding for std.450 extended instructions
3 participants