On 03/29/2017 09:17 PM, Matteo Bruni wrote:
I haven't looked at the v3 patches yet, but in the meantime:
2017-03-29 2:09 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Oh, its a bug actually now, we should not mind absolute offset here ifrelative addressing is on, we know if it is out of bound or not when we add index register value only, which can be negative. Yes, it looks the only tables where we need to do this update table size are temporary register table (for which we don't have any size known in advance) and output tables. I would check that and if I am not missing something now would leave just update table size where required.
Yeah and temporaries are supposed to be written before being read so in theory you can entirely skip the update_table_size() calls for source arguments.
But update_table_sizes is required to calculate memory allocation size for table, what is the difference if temporary registers are written first in this regard? In v3 I u sed correct offset for updating table sizes when relative addressing is used in operand. I didn't change table size update though to update temporary register table size only, as doing so currently breaks one test. It is test_effect_preshader_op(), which writes instructions to the effect blob and unintentionally corrupts the data past end of the preshader code being updated. This effectively results in destroying 'CTAB' for the next preshader in blob, so that preshader is parsed ok but it does not have input constant definition anymore, thus no table sizes set from input registers definition. Still such an effect and preshader is created ok by native implementation, and also by our current implementation. So So I thought it is better to keep the current logic of table size updates based on actual register access in preshader for all tables, as according to the test native implementation does not fail preshader creation if it access the constants outside defined input range.