Page 1 of 6
x86_64 patch
Posted: 08 Nov 2006, 03:16
by pseudonym404
Hi, I've just successfully persuaded warzone to build and run on an amd64 machine, patch against current svn (revision 469, 20061108) is attached

Re: x86_64 patch
Posted: 09 Nov 2006, 19:01
by DevUrandom
WOW! Cool!

Many have tried and as many have failed, you made it.
I had just a quick look at the patch and saw that you replaced a lot of UDWORD with UPWORD (Uint64 on x86_64, Uint32 on others).
This is ok for a quick hack, but did you see a chance to replace at least most of that pointers hiding as ints by real pointers?
If not, ok, then it's ok for now. Perhaps we can really fix it later, then.
I'll take a deeper look at the patch when I got more time, maybe on the weekend.
Re: x86_64 patch
Posted: 10 Nov 2006, 09:29
by pseudonym404
The main problem is the script compiler and interpreter.. [script struct]->pCode is being used to store a variety of types in one chunk of malloc'ed memory, all sizes of integer and pointers.
Short of a rewrite, the only fix was to make pCode large enough to contain the largest type, and fix the script compiler and interpreter to work at the larger size..
Could probably eliminate some of the pointer/integer casts elsewhere, but as most of them were in the script compiler and interpreter, which wasn't sensibly fixable any other way, I didn't bother

Re: x86_64 patch
Posted: 10 Nov 2006, 14:15
by Troman
There are strange coincidents sometimes. Some days ago I started rewriting parts of the compiler and interpreter, i'm planning to make it use a structure that will hold both information about data type and data itself (as a union). This will make the scripting engine more extendable and will hopefully eliminate some porting issues. Although i'm not yet 100% sure my idea will work, i'll know when i'm finished.
I took a brief look at your patch, looks like we will have some fun resolving conflicts in bison grammar files and in the interpreter.
Re: x86_64 patch
Posted: 11 Nov 2006, 14:05
by DevUrandom
pseudonym404: We'll wait with commiting your patch till Troman commited his changes as otherwise we'd end up with a lot of conflicts... And maybe that allready resolves the need for an UPWORD.
Re: x86_64 patch
Posted: 11 Nov 2006, 20:18
by pseudonym404
No problem
I'll take another look when that's done, maybe find a cleaner way to fix the rest for amd64.
Re: x86_64 patch
Posted: 17 Nov 2006, 13:57
by Troman
Sorry for the delay, my house has been a big construction site.
First changes are commited, i'm not completely done yet, but things like .slo compiler (script_parser.y) should be ready and will probably not be touched anymore in the near future. I'm not sure when I will change the rest, maybe it makes sense to commit your patch in parts?
Anyway, hopefully that will allow to make a cleaner port to amd64.
Re: x86_64 patch
Posted: 22 Nov 2006, 02:07
by pseudonym404
OK, I'm back on it now
Currently making treap keys void pointers instead of UDWORD, as they're mostly (edit: looks like always) used to store pointers.
An integer type the same size as pointers is still needed for calculating offsets [ie. (pointer_type*)((integer_type)variable + offset) ], so I'm still using a UPWORD typedef, but based on the uintptr_t typedef in stdint.h
Re: x86_64 patch
Posted: 22 Nov 2006, 10:34
by DevUrandom
I _think_ (means I don't know for sure) that stdint.h is not present in C89 environments like MSVC.

So either you find some special defintion for them in MSVC, or (what I think is better than YetAnotherType) you plainly use pointers to calculate your stuff... In my experience pointer+int (like in your example) is not a problem and you will get a pointer out of it (just don't do it the other way round: int+pointer, which would give an int)...
Re: x86_64 patch
Posted: 22 Nov 2006, 16:06
by pseudonym404
It was getting late last night, and I was running into multiple instances of casting to an integer to add an offset to cast back to a pointer, and completely failed to realised the casts were superfluous, d'oh
I realised that msvc (and some of the bsd's?) might not have stdint.h, I wasn't actually using it directly, I'd just copied it's uintptr_t typedef into types.h as a upword typedef

If I can avoid using it, I'll take it back out again
---
Not entirely superfluous casts.. Seem to be running into struct alignment issues.. eg. psStats += statSize gets the wrong address and segfaults, but psStats = (BASE_STATS*)((void*)psStats + statSize) works.. doesn't need an integer type though
---
The main problem I have left, is PUSHGLOBAL is being used for ST_ type object pointers as well as integers.. currently trying to find a way to use v.oval for these instead of v.ival (troman's changes to the slo compiler seems to have fixed the rest of the scripting issues, nice work

)
---
MERGE: Multiposts (DevUrandom)
Re: x86_64 patch
Posted: 22 Nov 2006, 18:41
by Troman
pseudonym404 wrote:
The main problem I have left, is PUSHGLOBAL is being used for ST_ type object pointers as well as integers.. currently trying to find a way to use v.oval for these instead of v.ival
It shoudln't matter what type is being pushed with OP_PUSHGLOBAL, since all it does it calling stackPush and passing it a pointer to INTERP_VAL, which is then copied as a union to the stack (.ival is only used to store variable index, not the value itself).
Problematic could be the way how this INTERP_VAL was created, if ival or oval was used to assign a value to it.
scriptCodeConstant() is the place we should make the differentiation between ival and oval (there's a TODO).
Changing that will probably make it necessary to change the way values are read out of INTER_VAL (and changing .vlo parser - "scriptvals_parser.y", which assignes values to public variables, but I have a patch for that, let me know if you need it), and that in turn will probably make it necessary to rewrite load/save routines. Didn't have enough time to rewrite all this.
To find out if a certain type is used as a pointer or integer you can use something like this in script.c for example:
Code: Select all
/* returns true if passed INTERP_TYPE is used as a pointer in INTERP_VAL, false otherwise */
BOOL scriptTypeIsPointer(INTERP_TYPE type)
{
switch (type)
{
case ST_INTMESSAGE:
case ST_BASEOBJECT:
case ST_DROID:
case ST_STRUCTURE:
case ST_FEATURE:
case ST_TEMPLATE:
case ST_TEXTSTRING:
case ST_LEVEL:
case ST_RESEARCH:
return TRUE;
break;
}
return FALSE;
}
Just a suggestion.
Re: x86_64 patch
Posted: 22 Nov 2006, 20:31
by pseudonym404
The problem seems to be in stackPopParams (stack.c), specifically *((SDWORD*)pData) = psVal->v.ival; for everything except floats and strings, I'm currently trying to work out how this is not segfaulting for scrSetAssemblyPoint (scriptfuncs.c) (ival really shouldn't be able to hold valid pointers on amd64)
edit: even more bizarrely, i get a valid STRUCTURE* pointer in scrSetAssemblyPoint when built --enable-debug, but not otherwise..
Re: x86_64 patch
Posted: 22 Nov 2006, 20:59
by Troman
I see, that's what I meant with "read out".
stackPopParams is one of those problematic places left where we have to fix the pointer->integer conversion.
Right now pointers and integers are read as integers
Code: Select all
*((SDWORD*)pData) = psVal->v.ival;
All that is needed here is probably a check with scriptTypeIsPointer(), so that pointers will be read as .oval. I think that will suffice.
EDIT: all vars with VAL_REF set in type should also be handled as pointers, forgot to include this check in scriptTypeIsPointer().
Re: x86_64 patch
Posted: 22 Nov 2006, 21:30
by pseudonym404
Yeah, that's what I'm currently working on, the bit that's confusing me is .oval doesn't contain a valid STRUCTURE* pointer when stackPopParams is called from scrSetAssemblyPoint (and even more bizarrely, ival does.. but only when built --enable-debug), reading oval for pointer types works as expected otherwise.
edit: you missed ST_GROUP too (I hadn't spotted VAL_REF though)
edit: the scriptvals_parser.y patch you mentioned, does that modify eventSetContextVar to use .oval for pointers too?
Re: x86_64 patch
Posted: 22 Nov 2006, 22:03
by Troman
pseudonym404 wrote:
edit: the scriptvals_parser.y patch you mentioned, does that modify eventSetContextVar to use .oval for pointers too?
Yep
