[PATCH 8/9] d3dx9: Add test for 'cmp' preshader opcode.

[Matteo Bruni]

2017-05-11 18:48 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 05/11/2017 07:15 PM, Matteo Bruni wrote:
>> It might make sense to create a small tool to help with the effect
>> blob construction. For example, something that fills the various
>> offsets for you, like relocations. If so, it should come up quite
>> naturally while preparing the blob.
>
> Any change of any field in size will require updating offsets / sizes in the
> blob.

It depends on the change, e.g. adding a preshader instruction
shouldn't require too many fixups elsewhere.

> I can't even think of maintaining that fully manually. At the same
> time, creating a stand alone small "compiler" is not terribly difficult (if
> it relies on the precompiled effect and not actually creating everything
> from scratch and updating every size / offset)? If to make a tool which will
> actually "fixup" the blob for offsets, it is a bit bigger thing though,
> doesn't it look a bit too much for a private tool used for nothing but
> supporting the tests of a few opcodes?

I don't know what tools will be useful in practice. Generally that
becomes pretty clear once one starts working on it.
The relocation thing was just an idea and I was thinking of something
pretty ad-hoc, like driven by a table filled by hand and / or with the
"base" blob containing some special symbols to be replaced by offsets,
or something like that.

FWIW it doesn't necessarily have to be a 100% private ad-hoc tool used
only for this test. We want to support generating effects from
d3dcompiler at some point and for that we'll need to extend
bytecodewriter.c to generate effects bytecode. One way to do that is
to introduce a struct bwriter_effect storing a representation of all
the parameters and techniques (and thus passes, states, ...) of an
effect, similar to the current struct bwriter_shader, and a
bwriter_write_effect() function which serializes that to bytecode, in
the same vein as SlWriteBytecode(). The tool itself could then simply
build a struct bwriter_effect with all the stuff you need for the test
and call bwriter_write_effect() on it.
I don't know if it makes sense to get there right now. It's certainly
overkill for the test alone but on the other hand it would be an
excuse to implement a d3dcompiler feature which will be eventually
useful for the compiler itself.

>> Maybe I'm wrong and this is utterly insane, but it doesn't feel like
>> it to me at the moment (but then again that's what fools think all the
>> time, I guess...) Try to give it a shot though. If it's terrible and
>> ugly and kills kittens, just drop everything and continue using the
>> current test scheme.
>>
> The initially added opcode were tested in the test blob (which can be
> (re)compiled by fxc and had the source in the test file). Then I suggested
> this new test function some time ago for easier adding the opcodes without
> changing the blob, which is there now. If we forget about the 'cmp'
> instruction for a moment it is not hard to just compile (separate) effect
> blob(s) for every instruction and place blobs in the test. It though looks
> like a step backward to me as I was sure the table based "blob-free" test
> for a bunch of instructions, extendable for new tests with the new data as
> easy as adding just one line with parameter values is better. After that, I
> could craft just once a blob with 'cmp' instruction, as it is the only
> instruction I encountered so far which I cannot generate by now (apart from
> a bunch of instructions which return 0 for any parameters, which I suppose
> are never used by compiler and probably should not be added or tested). But
> to be honest I would like to avoid that if possible, at least until some new
> unexpected opcodes / cases will be discovered which would make the things
> worse.
>
> Maybe alternative somewhat "intermediate" approach would be to build just a
> preshader blob for "cmp" instruction instead of generating it and test it in
> a separate test function? This would be a ready blob for preshader to be
> just copied to the specified location in the existing effect blob, while the
> existing code in test_preshader_opcode would be left unchanged.

That doesn't help much with the issues I mentioned with the current
tests though.