https://github.com/shader-slang/slang
Revision 256a20a163ef6ee93a817472adcb24c076b0c0dc authored by Tim Foley on 16 March 2020, 16:03:19 UTC, committed by GitHub on 16 March 2020, 16:03:19 UTC
* Define compound intrinsic ops in the standard library

The current stdlib code has a notion of "compound" intrinsic ops, which use the `__intrinsic_op` modifier but don't actually map to a single IR instruction.
Instead, most* of these map to multiple IR instructions using hard-coded logic in `slang-ir-lower.cpp`.

(* One special case is `kCompoundOp_Pos` that is used for unary `operator+` and that maps to *zero* IR instructions)

All of the opcodes that used to use the `kCompoundOp_` enumeration values now have definitions directly in the stdlib and use the new `[__unsafeForceInlineEarly]` attribute to ensure that they get inlined into their use sites so that the output code is as close as possible to the original.

For the most part, generating the stdlib definitions for the compound ops is straightforward, but here's some notes:

* The unary `operator+` I just defined directly in Slang code, since it doesn't share much structure with other cases

* The unary increment/decrement ops are generated as a cross product of increment/decrement and prefix/postfix. The logic is a bit messy but given that we have scalar, vector, and matrix versions to deal with it still saves code overall

* Because all the compound/assignment cases got moved out, the existing code for generating unary/binary ops can be simplified a bit

* All the no-op bit-cast operations like `asfloat(float)` are now inline identity functions

* A few other small cleanups are made by not having to worry about the compound ops (which used to be called "pseudo ops") sometimes being encoded in to the same type of value as a real IR opcode.

The one big detail here is a fix for how IR lowering works for `let` declarations: they were previously being `materialize()`d which only guarantees that they've been placed in a contiguous and addressable location, but doesn't actually convert them to an r-value. As a result a `let` declaration could accidentally capture a mutable location by reference, which is definitely *not* what we wanted it to do. Fixing this was needed to make the new postfix `++` definition work (several existing tests end up covering this).

One important forward-looking note:

One subtle (but significant) choice in this change is that we actually reduce the number of declarations in the stdlib, because instead of having the compound operators include both per-type and generic overloads (just listing scalar cases for now):

    float operator+=(in out float left, float right) { ... }
    int operator+=(in out int left, int right) { ... }
    ...
    T operator+= <T:__BuiltinBlahBlah>(in out T left, T right) { ... }

We now have *only* the single generic version:

    T operator+= <T:__BuiltinBlahBlah>(in out T left, T right) { ... }

In running our current tests, this change didn't lead to any regressions (perhaps surprisingly).

Given that we were able to reduce the number of overloads for `operator+=` by a factor of N (where N is the number of built-in types), it seems worth considering whether we could also reduce the number of overloads of `operator+` by the same factor by only having generic rather than per-type versions.

One concern that this forward-looking question raises is whether the quality of diagnostic messages around bad calls to the operators might suffer when there are only generic overloads instead of per-type overloads. In order to feel out this problem I added a test case that includes some bad operator calls both to `+=` (which is now only generic with this change) and `+` (which still has per-type overloads). Overall, I found the quality of the error messages (in terms of the candidates that get listed) isn't perfect for either, but personally I prefer the output in the generic case.

As part of adding that test, I also added some fixups to how overload resolution messages get printed, to make sure the function name is printed in more cases, and also that the candidates print more consistently. These changes affected the expected output for one other diagnostic test.

* fixup: disable bad operator test on non-Windows targets
1 parent c1743a5
History
Tip revision: 256a20a163ef6ee93a817472adcb24c076b0c0dc authored by Tim Foley on 16 March 2020, 16:03:19 UTC
Define compound intrinsic ops in the standard library (#1273)
Tip revision: 256a20a
File Mode Size
docs
examples
external
prelude
source
tests
tools
.editorconfig -rw-r--r-- 937 bytes
.gitattributes -rw-r--r-- 95 bytes
.gitignore -rw-r--r-- 480 bytes
.gitmodules -rw-r--r-- 774 bytes
.travis.yml -rw-r--r-- 1.7 KB
CODE_OF_CONDUCT.md -rw-r--r-- 3.1 KB
LICENSE -rw-r--r-- 1.1 KB
README.md -rw-r--r-- 7.1 KB
appveyor.yml -rw-r--r-- 4.0 KB
premake5.lua -rw-r--r-- 30.5 KB
slang-com-helper.h -rw-r--r-- 4.8 KB
slang-com-ptr.h -rw-r--r-- 4.8 KB
slang-tag-version.h -rw-r--r-- 36 bytes
slang.h -rw-r--r-- 126.9 KB
slang.sln -rw-r--r-- 10.8 KB
test.bat -rw-r--r-- 1.4 KB
travis_build.sh -rw-r--r-- 460 bytes
travis_test.sh -rw-r--r-- 435 bytes

README.md

back to top