quarnster

In as_callfunc_arm.cpp, replace the existing paramBuffer code with this https://gist.github.com/3372000

This makes all the current test_feature tests pass. However, a new test is needed to catch a case that this patch doesn't fix which isn't caught by the current set of tests. Replace testcdecl_class.cpp with the following https://gist.github.com/3372005

The new failure happens with the call to:

c4.class2_2ByVal(c)

As the class2_2 argument should be 64 bit aligned. How to detect this and differ class2_2 from class2 I do not know though as they both have the same size in memory and yet one must be aligned but not the other. Any suggestions? Some clever template trick perhaps?

[  0%] Building CXX object CMakeFiles/Angelscript.dir/Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_builder.cpp.o
In file included from /Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_builder.h:43,
				 from /Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_builder.cpp:40:
/Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_symboltable.h: In member function 'void asCSymbolTable<ENTRY_TYPE>::BuildKey(const asSNameSpace*, const asCString&, asCString&) const':
/Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_symboltable.h:527: error: invalid use of incomplete type 'const struct asSNameSpace'
/Users/quarnster/code/3rdparty/angelscript/sdk/angelscript/source/as_symboltable.h:60: error: forward declaration of 'const struct asSNameSpace'

Same error for multiple files. Just moving the definition of asSNameSpace into as_symboltable seemed to make it work.

I'm confused how the reference counting works for native functions and for the generic call convention. In my engine I have:

1) Functions returning objects increase the reference count of the returned object.

2) If the argument isn't returned (see 1), the callee increases the reference count of arguments if and only if it stores a copy. This means no unneeded work done, but the caller can't decrease its own reference until after the callee returns, which is exactly how you'd have to do it anyway from any c-like language (Suppose it wasn't reference counted, but a normal pointer, you can't create an object, delete it and then call into a function taking it as an argument can you? You have to create it, call into the function and then delete it)


So the expectation is for example:

// Some function in c++
	 Shape* sphere = world->CreateSphereShape(1.0); // sphere comes in as count +1
	 RigidBody* sphereBody = world->CreateRigidBody(1.0, sphere); // spherebody comes in as +1, what sphere is we don't know and don't care, but it'll be at least 1 still
	 world->AddRigidBody(sphereBody);
	 // No longer references sphere and sphereBody here so we get rid of our references
	 sphere->DelRef();
	 sphereBody->DelRef();

// ... snippity snip ...

void PhysicsWorld::AddRigidBody(RigidBody* node)
{
	 if (something)
		return; // Meh, we didn't want it

	 // yeah, we wanted it
	 node->AddRef();
	 mNodes.push_back(node);
}

// ... snippity snip ...

Something* Something::Blah(Something* arg)
{
	// We don't care about this object, but the call convention is to increase the ref count for stuff returned, so that's what we'll do
	arg->AddRef();
	return arg;
}

Now from http://angelcode.com...obj_handle.html we can read that functions should proactively call AddRef when returning (same as I have), but also call Release on arguments (in other words the caller increases the ref count before calling the function). Is there a specific reason for that? Imagine a long callstack where an object is passed through but only the endpoint actually needs to store the reference. With the AS call convention a reference is added and released in each step in the callstack whereas in my version it's only done where needed. In a multithreaded environment changes to the reference counter needs to be an atomic operation so isn't necessarily free and it's unneeded work anyway. Am I high? Have I missed something? Scripts wouldn't necessarily call into performance critical sections, but the rest of the engine might and if that section happens to be reference counted we've now introduced unneeded overhead.

For the generic call convention we have SetReturnObject and SetReturnAddress which will (if needed) increase the reference count and do nothing to the count respectively. For arguments though release is called always on the arguments.

This means that if I have a native function taking reference counted arguments, I need to decrease the reference count in the native function when calling the native function directly. If I have a wrapper though, I need to undo this release by calling AddRef in the wrapper. In a way that is consistent with the call convention used, but it's still all unneeded work isn't it?

Then there's auto handles which gives some aid here. To automatically fit the AS call convention into mine, I'd define functions as for example:

RigidBody@ CreateRigidBody(float, Shape@+)

But there's the caveat mentioned:

However, it is not recommended to use this feature unless you can't change the functions you want to register to properly handle the reference counters. When using the auto handles, AngelScript needs to process all of the handles which causes an extra overhead when calling application registered functions.

But this is an extra overhead that's not really needed in the first place, is it? Have I missed something?

Since there's an assembly file for MSVC x86_64 this should be added before or after the ANDROID block in sdk/AngelScript/projects/cmake/CMakeLists.txt:

if(MSVC AND CMAKE_CL_64)
	    enable_language(ASM_MASM)
	    if(CMAKE_ASM_MASM_COMPILER_WORKS)
			    set(ANGELSCRIPT_SOURCE ${ANGELSCRIPT_SOURCE} ../../source/as_callfunc_x64_msvc_asm.asm)
	    else()
			    message(FATAL ERROR "MSVC x86_64 target requires a working assembler")
	    endif()
endif()

I also had to edit the code block around line 1131 in angelscript.h from

if(sizeof(void*) == 4)

to

#ifndef AS_64BIT_PTR

, otherwise the compiler would complain about not being able to static cast the data types involved when trying to register CallMe1 in testmultipleinheritance.cpp.

BTW TestSaveLoad reports (but still says it passed)

The saved byte code is not of the expected size. It is 1760 bytes
The saved byte code contains a different amount of zeroes than the expected. Co
nted 530
The saved byte code has different checksum than the expected. Got 0x87C6163E

Not sure if that's a real problem or not as the test passed anyway.

I'm trying to make my AOT compiler "inline" native function calls so it generates a C signature based on the information available and thus bypassing the register copying mechanisms involved when making a non-generic native call. I've run into a problem I've yet to be able to figure out though.

It seems there is a difference in how AngelScript stores the parameter for a call to:

static void CopyConstructString(const string &other, string *thisPointer);

and a call to

static void func26(const MyClass3& a0);

The AOT generated code for these calls are:

// (CopyConstructString)
// _beh_0_, 10, 0, 2, 0, 0, 0
// ret: 0, 0, 0, 1, 0, 0, 0
// arg0: 67108876, 1, 7938, 0, 0, 0, 1, 0, 1, 1, 2, 2
typedef void (*funcptr)(asQWORD&, void*);
funcptr a = (funcptr)sysFunc->func;
a(**(asQWORD**)(&args[0]), obj);

// func26, 2, 0, 2, 0, 0, 0
// ret: 0, 0, 0, 1, 0, 0, 0
// arg0: 67108879, 1, 7938, 0, 0, 0, 1, 0, 1, 1, 2, 2
typedef void (*funcptr)(asQWORD&);
funcptr a = (funcptr)sysFunc->func;
a(**(asQWORD**)(&args[0]));

with the comments being

snprintf(buf, 128, "// %s, %d, %d, %d, %d, %d, %d\n", descr->GetName(), sysFunc->callConv, sysFunc->takesObjByVal, sysFunc->paramSize, sysFunc->hostReturnInMemory, sysFunc->hasAutoHandles, sysFunc->scriptReturnSize);
snprintf(buf, 128, "// ret: %d, %d, %d, %d, %d, %d, %d\n", sysFunc->hostReturnFloat, retType.IsObject(), retType.IsReference(), retType.IsPrimitive(), retType.GetSizeInMemoryDWords(), retType.GetSizeOnStackDWords(), sysFunc->hostReturnSize);
snprintf(buf, 128, "// arg%d: %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d\n", n, descr->GetParamTypeId(n), dt.IsObject(), dt.GetObjectType()->GetFlags(),dt.IsObjectHandle(), dt.IsScriptObject(), dt.IsHandleToConst(), dt.IsReference(), dt.IsPrimitive(), dt.CanBeCopied(), dt.CanBeInstanciated(),  dt.GetSizeInMemoryDWords(), dt.GetSizeOnStackDWords());

So to me it looks like the string and the MyClass3 are as close to each other as can be, but the problem is that the call to CopyConstructString only works with

a(*(asQWORD*)(&args[0]), obj); // single dereference

and the call to func26 only works with

a(**(asQWORD**)(&args[0])); // double dereference

Now I hear you saying "maybe it's because you use a asQWORD rather than the real type". Unfortunately even if I put the real type in there the situation is the same:

typedef void (*funcptr)(const std::string&, std::string*);
funcptr a = (funcptr)sysFunc->func;
a(*(std::string*)(&args[0]), (std::string*) obj); // Only single dereference works

typedef void (*funcptr)(const MyClass3&);
funcptr a = (funcptr)sysFunc->func;
a(**(MyClass3**)(&args[0])); // Only double dereference works

So now the signature is a perfect match between the function pointer and the real function, agree? If you do, then to me it looks like AngelScript is indeed storing these two parameters differently on the stack, agree? How do I detect whether I should dereference once or twice?

Worth mentioning is that if I call into CallSystemFunctionNative instead of this call inlining, everything works as expected so the stack is properly set up. I tried looking in CallSystemFunctionNative for a few platforms to see if there's any special handling, but that only seems to be the case when sysFunc->takesObjByVal, which isn't the case for these two functions, or maybe the information needed isn't available when the jit compiler is invoked to compile the function?

Any ideas or suggestions?

Cheers