x86_64 patch

[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

[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.

[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

[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.

[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.

[pseudonym404]

No problem

I'll take another look when that's done, maybe find a cleaner way to fix the rest for amd64.

[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.

[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

[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)...

[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)

[Troman]

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.

[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..

[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().

[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?

[Troman]

edit: the scriptvals_parser.y patch you mentioned, does that modify eventSetContextVar to use .oval for pointers too?

Yep