This was broken by commit e390bc35e2c9b0a2110370f916033eea2366317e; that
commit fixed the source count for these instructions, but didn't adjust
shader_sm1_skip_opcode(). Note that this only affects shader model 1;
later versions have a token count embedded in the initial opcode token.
Native does not always do this. For example, functions whose parameters are
float and float1 always result in an "ambiguous function call" error.
This does not fix any tests, because the relevant tests previously (incorrectly)
succeeded, and now fail with:
E5017: Aborting due to not yet implemented feature: Prioritize between multiple compatible function overloads.
when they should simply fail.
The choice to store them in an rbtree was made early on. It does not seem likely
that HLSL programs would define many overloads for any of their functions, but I
suspect the idea was rather that intrinsics would be defined as plain
hlsl_ir_function_decl structures [cf. 447463e590]
and that some intrinsics that could operate on any type would therefore need
many overrides.
This is not how we deal with intrinsics, however. When the first intrinsics were
implemented I made the choice disregard this intended design, and instead match
and convert their types manually, in C. Nothing that has happened in the time
since has led me to question that choice, and in fact, the flexibility with
which we must accommodate functions has led me to believe that matching in this
way was definitely the right choice. The main other designs I see would have
been:
* define each intrinsic variant separately using existing HLSL types. Besides
efficiency concerns (i.e. this would take more space in memory, and would take
longer to generate each variant), the normal type-matching rules don't really
apply to intrinsics.
[For example: elementwise intrinsics like abs() return the same type as the
input, including preserving the distinction between float and float1. It is
legal to define separate HLSL overloads taking float and float1, but trying to
invoke these functions yields an "ambiguous function call" error.]
* introduce new (semi-)generic types. This is far more code and ends up acting
like our current scheme (with helpers) in a slightly more complex form.
So I think we can go ahead and rip out this vestige of the original design for
intrinsics.
As for why to change it: rbtrees are simply more complex to deal with, and it
seems unlikely to me that the difference is going to matter. I do not expect any
program to define large quantities of intrinsics; linked list search should be
good enough.
Specifically, map COLOROUT to OUTPUT, and map INCONTROLPOINT to INPUT for domain
shaders as well as hull shaders.
Obscure the non-existent differences from the view of the backend.
Hull shaders have a different temps count for each phase, and the
parser only reports the count for the patch constant phase.
In order to properly check for temps count on hull shaders, we first
need to decode its phases.
For relative addressing, the vkd3d_shader_registers must point to
another vkd3d_shader_src_param. For now, use the sm4_instruction to save
them, since the only purpose of this struct is to be used as paramter
for write_sm4_instruction.
RWBuffer objects would trigger a vkd3d_unreachable() in sm4_base_type().
It would be easy enough to add the required case there, but (manual,
unfortunately) tests show that we aren't supposed to write constant
buffer entries for objects in the first place, as you'd expect.
This particular path ends up being exercised by vkd3d's internal UAV
clear shaders, but unfortunately it looks like our RDEF data may have
more issues; the ability to write tests for it would seem helpful.
None of these currently have any meaning, and none of these can currently be
parsed as distinct tokens either (i.e. they will generate a syntax error
anyway).
Co-authored-by: Evan Tang <etang@codeweavers.com>
Evan Tang reported that new fixmes appeared on the shader_runner when
running some of his tests after
f50d0ae2cb.
vkd3d:652593:fixme:shader_sm4_read_src_param Unhandled mask 0x4.
The change to blame seems to be this added line in
sm4_src_from_constant_value().
+ src->swizzle = VKD3D_SHADER_NO_SWIZZLE;
On tpf binaries the last 12 bits of each src register in an instruction
specify the swizzle, and there are 5 possible combinations:
Dimension NONE
-------- 00
Dimension SCALAR
-------- 01
Dimension VEC4, with a 4 bit writemask:
---- xxxx 00 01
Dimension VEC4, with an 8 bit swizzle:
xx xx xx xx 01 01
Dimension VEC4, with a 2bit scalar dimension number:
------ xx 10 01
So far, we have only seen src registers use 4 bit writemasks in a
single case: for vec4 constants, and it is always zero.
So we expect this:
---- 0000 00 01
Now, I probably wanted to initialize src->swizzle to zero when writing
constants, but VKD3D_SHADER_NO_SWIZZLE is not zero, it is actually the
default swizzle:
11 10 01 00
And the last 4 bits (0x4) get written in the mask part, which causes
the reader to complain.
On macOS vkd3d_shader_global_flags has underlying type unsigned long,
while uint64_t is defined as unsigned long long. This difference
causes a few warnings to be raised.
The hull shader barrier used for this was broken by I/O normalization, since
vocp is no longer exposed to the spirv backend.
Restore this barrier by checking for vocp during normalization instead.
So that they are dumped even if parsing fails, which is a circumstance
in which one likely wants to see the problematic shader.
The downside of that is that for shader types other than HLSL
the profile is not written any more in the filename. This should
not be a big problem, because in those cases the shader describes
its own type.
When dumping an HLSL shader, the id is brought in front of the profile
in the file name, in order to make it more tab-friendly: when dealing
with a directory full of shaders it's likely that the id determines
the profile, but the other way around.
This field is now analogous to vkd3d_shader_register_index.rel_addr.
Also, it makes sense to rename it now because all the constant part of
the offset is now handled to hlsl_deref.const_offset. Consequently, it
may also be NULL now.
This uint will be used for the following:
- Since SM4's relative addressing (the capability of passing a register
as an index to another register) only has whole-register granularity,
we will need to make the offset node express the offset in
whole-registers and specify the register component in this uint,
otherwise we would have to add additional / and % operations in the
output binary.
- If, after we apply constant folding and copy propagation, we determine
that the offset is a single constant node, we can store all the offset
in this uint constant, and remove the offset src.
This allows DCE to remove a good bunch of the nodes previously required
only for the offset constants, which makes the output more liteweight
and readable, and simplifies the implementation of relative addressing
when writing tpf in the following patches.
In dump_deref(), we use "c" to indicate components instead of whole
registers. Since now both the offset node and the offset uint are in
components a lowered deref would look like:
var[@42c + 2c]
But, once we express the offset node in whole registers we will remove
the "c" from the node part:
var[@22 + 3c]
Some functions work with dereferences and need to know if they are
lowered yet.
This can be known checking if deref->offset.node is NULL or
deref->data_type is NULL. I am using the latter since it keeps working
even after the following patches that split deref->offset into
constant and variable parts.
