On 05/11/2017 07:15 PM, Matteo Bruni wrote:
2017-05-11 17:52 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 05/11/2017 06:42 PM, Matteo Bruni wrote:
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).
Well yes, I am changing the blob, but just a very small part of it. The preshader encoding is not that terrible as for the whole effect, its format is pretty straightforward and does not have any cross references. Please note that if you want to add some test now for a new preshader opcode of already supported number of arguments, or extend the test for existing opcode with more cases, you don't have to change that part, just add the readable line to the test table with values. And you don't have to manually decompile the blob or trust the comments to see what values are tested there.
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.
Any change of any field in size will require updating offsets / sizes in the blob. 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?
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.