On 03/19/2016 12:55 AM, Matteo Bruni wrote:
2016-03-17 12:59 GMT+01:00 Paul Gofman gofmanp@gmail.com:
+enum PRES_REG_TABLES +{
- PRES_REGTAB_NONE,
- PRES_REGTAB_IMMED, /* immediate double constants from CLIT */
- PRES_REGTAB_CONST,
- PRES_REGTAB_VERTEX, /* not used */
- PRES_REGTAB_OCONST,
- PRES_REGTAB_OBCONST,
- PRES_REGTAB_OICONST,
- PRES_REGTAB_REG,
- PRES_REGTAB_MAX,
- PRES_REGTAB_FIRST = PRES_REGTAB_IMMED,
- PRES_REGTAB_LAST = PRES_REGTAB_REG,
+};
You can drop PRES_REGTAB_NONE unless it's going to be used.
These constants currently match values in preshader bytecode. PRES_REGTAB_NONE is used as a placeholder for value 0. I can remove it, but then have PRES_REGTAB_IMMED = 1 explicitly, or introduce a separate mapping table between original constants and these enum.
+#define PRES_VS_BITS_PER_WORD (sizeof(unsigned int) * 8)
What does the _VS_ part of the name mean? I'm also a bit thrown off by the _WORD part, mostly because in Win32 WORD is a 16 bit while DWORD is 32 bit and unsigned int is also 32 bit. Maybe it's just me, I don't know.
VS means "value set" :) Yes, I realize WORD is confusing (what MS calls DWORD is actually a halfword nowadays). I suggest to change unsigned int to uintptr_t (which is functionally more appropriate), and call it PRES_BITMASK_BLOCK_SIZE.
The traces of the disassembled preshader can also potentially go to a separate patch (leaving only dump_shader_bytecode() in the initial implementation - btw, I think it should be called dump_preshader_bytecode() given the renaming of the other stuff).
It actually used to dump the whole binary blob, which can contains shader also (or just shader sometimes). I suggest I better use just dump_bytecode then.