From: Hans de G. <j.w...@hh...> - 2005-05-10 08:09:13
|
Hi Daniel, Daniel Borca wrote: > Just a side-note: GLIDE_DEBUG is hardly critical. > OTOH, the makefiles do cope with all the necessary #defines. > Die, autotools, die! ;> > I don't want to put it that strongly but I do believe removing the autoXXX stuff is a good idea since it currently is broken. Any objections? >> For h3 xdraw2.inc.S contains: >> mov _gc-4(%esp) , gc >> mov _vb-4(%esp) , fb /* get base address of vertex B */ >> In the prologue, culling case, however xdraw2.inc contains: >> mov gc, edx ; our hoopti calling conventions pass this >> <snip> >> mov fb, [esp + _vb$ - 4] ; get base address of vertex B >> So they don't agree on how the gc paramter is passed afaik the GNU >> code is correct for PIC and the nasm for non-PIC. Which means that we >> should >> add a PIC macro to the asm defines and assemble seperate PIC and >> non-PIC versions. >> The same goes for the non-culling case. > > > None of the ASM specializations (either .S or .asm) are PIC-aware. > However, I think the sourcecode uses "hoopti calling convention" for > all OSes (including Linux). This reminds me that .S files are pretty much > unsafe. > Erm, actually someone changed the .S code from "mov gc, edx" to "mov _gc-4(%esp) , gc" but then in gnu-as ofcourse in the official tree, not the devel branch. I picked up that patch from the official Glide tree in my linux package, that is why I took a look at the .asm files in the first place, asm is not my thing. Are you really, _really_ sure that the current .asm code is correct? I asked Alan Cox (linux kernel guru) to check the .S changes from the stable branch for me and to him they looked correct when used from PIC compiled C-code. Also the asm code may not be aware of PIC/non-PIC but it really should work as part of PIC code since it is in a .so file under Linux. >> Also there is a lot of "%if GLIDE_CLIP_COORDS" in xdraw2.inc, but the >> file starts with >> %if GLIDE_CLIP_COORD >> <snip> >> %else >> So al the "%if GLIDE_CLIP_COORDS" code in the block below the else we >> never be assembled. The code might be easier to read if we just remove >> those blocks. > > > xdraw2.inc is a template. It is included by xdraw2.asm several times, > with different flags. Take great care before removing existing code. I'm not planning on removing it, I just noticed that it contained: %if GLIDE_CLIP_COORD ... %else ... %endif And then more %if GLIDE_CLIP_COORD ... %endif Blocks in the else block, which no mather what xdraw2.asm defines will never get assembled because they are in the else block. But I'm not planning on touching the asm, with C I'm pretty experienced and confident, with asm I'm not. >> Line 653-655 of swlibs/newpci/pcilib/fxpci.c read: >> // Quick optimization, if there is no device or vendor ID... >> if ((!dID) && (!dID)) >> continue; >> So either the comment or the code is wrong, this needs fixing or removal. > > > Here, I could only take a guess (without re-reading much the sources): > if ((!vID) || (!dID)) // most likely, imo > or > if ((!vID) && (!dID)) // ummm, really? > Regards, > Daniel Borca > > For my linux package I changed the && to an || I will do the same for the devel branch. Regards, Hans |