D3D shader assembler (round 2)

[Stefan Dösinger]

Am Wednesday 12 August 2009 21:57:36 schrieb Matteo Bruni:
> > I think it doesn't need two include files, especially since both of them
> > are d3dx9-private anyway. The question is, is it better to just merge
> > them into d3dx9_36_private.h, or have assembler things in a separate
> > file? I tend towards putting everything into d3dx9_36_private.h, but I
> > don't have a strong opinion about that.
>
> I merged everything into d3dx9_36_private.h
Ok.

> >> +%option prefix="asmshader_"
> >> +    buffer = asmshader__scan_string(text, asm_ctx.yyscanner);
> >
> > It seems that flex adds an underscore to the prefix anyway, so I guess
> > its fine to remove the one from the prefix option.
>
> Removing the underscore from the flex prefix produces a (better)
> asmshader_scan_string function but also an ugly asmshaderlex_init. I
> decided to keep the underscore.
Fair enough

> >> +    WINED3DSIO_NOP = 0,
> >> +    WINED3DSIO_MOV = 1,
> >> +    WINED3DSIO_ADD = 2,
> >> and others
> >
> > Can you give them a different name prefix, to make it clear that they
> > don't have any relationship with wined3d any more?
Hmm. Maybe D3DXSIO_*, or WINED3DXSIO_*, but I'll think about a better one. 
Also this applies to many other enums.

> I named them BWRITERxxxx (the same for wine_shader, now it is named
> bwriter_shader, enforcing the idea that that represents the bytecode
> writer interface). Is it ok?
Aren't they used by the parser as well? Ie, to generate the wineshader 
structure?

> > A swich-case statement or a lookup table would probably be nicer. (A
> > lookup table has the disadvantage that you still have to check min and
> > max separately though)
>
> Looking better at this, I noticed that I wasn't checking the register
> number, while native has this check. So I reworked this part, and now
> the code is as the attached file.
Looks ok to me

> Regarding Henri's review, I followed everything, except keeping
> my_alloc/realloc/free (now asm_alloc and c.) functions, as it seems
> Stefan likes them very much :)
I added them after I got fed up with writing HeapAlloc(GetProcessHeap(), 
HEAP_ZERO_MEMORY, size); all the time, and I considered them ugly
(A while ago there was some talk about a wine-wide HeapAlloc wrapper macro 
too)

> Supposing my changes are ok, what should I do now? Wait for reviews of
> the rest? Or maybe send the updated code? In this case, should I send
> it again in wine-devel or have a try with wine-patches?
I'd say give wine-patches a try :-) Make sure Alexandre can see how these 
patches will work together with the wpp ones and the actual implementation of 
D3DXAssembleShader