2017-05-11 20:23 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 05/11/2017 08:48 PM, Matteo Bruni wrote:
2017-05-11 18:48 GMT+02:00 Paul Gofman gofmanp@gmail.com:
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.
Implementing an effect compiler is an interesting thing, though a big one. I didn't look at what is there in the current effect compiler framework yet though, getting into it alone requires some time
You don't need the whole thing, just the "bytecode writer" part, which is pretty separate from the rest.
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.
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.
It is possible to generate corrupted effect blob, exactly the same way the code was corrupting it before that fix. Anyway, I understand your reasoning, but maybe there is some simpler option to add a test for 'cmp' instruction and maybe improve the existing test then stepping towards effect compiler?
As I said in my first reply, I'm okay with accepting the original patch. I'm not particularly happy with it and I hoped you would give at least a half-hearted try to the alternative but I'm not vetoing it.