2017-08-29 17:08 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 08/29/2017 05:35 PM, Matteo Bruni wrote:
Right, I think checking that during parsing is a good idea regardless of everything else.
I'd argue that either the expression above is too generic (allowing / handling register_count > 1) or the other register_count expression (the merge one I brought up earlier) is too naive, not expecting the same. As it is it just doesn't look right.
I updated the expression in 'merge' phase by now to do the check, more exactly, added a check for the case under discussion inside the 'if'.
To block the situation when register components exceed register size, Iactually should check not just component count but offset also. Perhaps this should also be done not just for output but for input registers only. This can't be done cleanly for input registers though for all the cases, as there might be a relative offset which can still effectively make a multicomponent operation cross register boundary. Not sure if we should add runtime checks for that. Native implementation seems to allow all this and does not refuse to parse preshaders which crossses a register boundary in instruction even without relative addressing. It does allow some things we already deny now, so I think I should improve the checks for components during parsing and block them early when possible, and after that consider preshader instruction output to affect just one register always in this part.
Native may allow that but there is some good chance that it computes incorrect outputs / sets incorrect device states in those cases (i.e. I imagine instructions straddling multiple registers to be unexpected for native too). It's probably not relevant in practice. Anyway, sure, I'm okay with adding those checks in our implementation if you want (mostly to print some WARNs if the unexpected happens) but simply assuming / enforcing a single output register per instruction, along the lines of my last email, seems enough to me.