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

[Matteo Bruni]

2017-05-11 17:52 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 05/11/2017 06:42 PM, Matteo Bruni wrote:
>>
>> 2017-05-11 17:32 GMT+02:00 Matteo Bruni <matteo.mystral at gmail.com>:
>>>
>>> 2017-05-11 17:08 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>>>
>>>> The problem is I met this opcode in the wild and thus added it in the
>>>> first
>>>> version of preshader implementation (and can "model" and test it by
>>>> manually
>>>> editing preshader code), but could not make native d3dx compiler to
>>>> generate
>>>> it so far. Whatever I tried in HLSL which would be a straightforward use
>>>> of
>>>> 'cmp' resulted in different opcode sequences. It was either generated by
>>>> some earlier versions of compiler or I just could not find the
>>>> conditions
>>>> under which it will be used. As for adding some other 3-parameter opcode
>>>> to
>>>> effect blob, while scanning all the opcodes in the 3 hex digit opcode
>>>> range
>>>> I have found just one other 3 parameter opcode 'movc', but it seem to
>>>> always
>>>> return 0 and I cannot generate it in effect blob from source either. So
>>>> by
>>>> now I didn't find any other way to test this opcode rather than manually
>>>> code it in the effect blob.
>>>>
>>>>      Maybe I would rather generate the code for all the other cases too?
>>>> It
>>>> won't make this part less evident but it will be a uniform approach.
>>>
>>> Well, yes, I mean creating by hand an effect blob with the specific
>>> instruction encodings you're currently testing in
>>> test_effect_preshader_ops(). You might make a separate preshader /
>>> state for each instruction or not (or anything in between), whatever
>>> is easier.
>>
>> Not sure we're on the same page so let me try to say it differently:
>> what I'd like to see is a hand-crafted effect blob which contains
>> preshader(s) which use those "interesting" instructions. That would be
>> used in a test replacing test_effect_preshader_ops().
>>
>> Hand-crafting an effect file is probably going to be a bit tedious
>> (e.g. the typedefs can get quite convoluted) so I suggest you to start
>> by picking a few pieces of one of the existing effect blobs (e.g.
>> test_effect_preshader_effect_blob[]) and fixing up the offsets and
>> counts.
>>
> The only feasible way for me to do and support it is to compile a small
> "sample" effect blob, take my current code as a "compiler" which will "fix"
> instructions and generate a blob and put a ready blob into the test. Do you
> think it really worth it? Instead of the table with parameters where one can
> see which values are tested, and some code which makes the localized changes
> to it, we will get a binary blob in test which has no public tool to compile
> it from any source or to decompile it. What if anyone else will want to add
> another test case for some instruction with different parameters? Adding
> such a test will require a full understanding of effect binary structure and
> will be a sort of nightmare.

It is already the case though, you're changing the blob at runtime in
the test and that implies knowing the encoding of the effect bytecode.
The blob hacking as it is now is also pretty obscure and fragile (e.g.
see the accidental overwriting of the CTAB you fixed recently).

If anything, an effect blob annotated with comments should be MORE
readable and allows you to test exactly what you want. About
maintaining it / adding more tests, it shouldn't be too terrible. The
most annoying change I can think of is probably adding new effect
parameters. You could put a few extra (unused) parameters right away
to reduce that risk. Same for extra states / preshaders, maybe.

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.

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.