2017-05-11 18:48 GMT+02:00 Paul Gofman gofmanp@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.