Sign in to follow this  

Some fixes.

This topic is 1971 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

How would you prefer receiving code patches? For now you'll see the commit history and changes at [url="https://github.com/quarnster/angelscript/compare/patchesforandreas."]https://github.com/q...chesforandreas.[/url]

(SVN is crap for collaboration. If you were using a distributed version control system you could have just merged these changes directly or easily cherry-pick the ones you wanted with retained commit history/comments.)


Another issue I noticed but didn't track down was that for my simple test for the AOT compiler ([url="https://github.com/quarnster/asaot/blob/master/test.cpp#L32"]https://github.com/q...er/test.cpp#L32[/url]) the compiler gets invoked twice for the class member function, the second time it has a bytecode length of 0. Is that intentional? The asIScriptFunction pointer has a different value but the function name appears to be the same.

Share this post


Link to post
Share on other sites
Either the full modified source files, or standard .patch files are best for me. However, I look into the patches you posted at github the way they are. You don't need to do anything else.

SVN may not be the best tool for collaboration, but I try not to check in any code directly to the SVN without testing it myself first anyway, so it works very well for me.

I believe the second function without bytecode is the virtual method. You can check that with the function type, the one with bytecode should be asFUNC_SCRIPT, and the other should be asFUNC_VIRTUAL. The virtual method simply holds the information the VM needs to find the real function that should be executed.

Share this post


Link to post
Share on other sites
The SpawnObject changes in https://github.com/quarnster/angelscript/commit/71e6510501873c9eae7bbd0e3e535557eaaa5de3 weren't applied. Was that intentional? Without it everything ends up in (0,0).

Share this post


Link to post
Share on other sites
I couldn't understand why it was necessary.

What is the value of RAND_MAX on your system? Is it overflowing the integer value? Is that why it ends up as 0? Or is the compiler making a mistake and computing 10/RAND_MAX at compile time?

Share this post


Link to post
Share on other sites
I can't say I understand x86* assembly, but here's the output:

[code]// RAND_MAX:
#define RAND_MAX 0x7fffffff

// test.cpp:
#include <stdlib.h>

int hello()
{
return 10*rand()/RAND_MAX;
}


// Produced assembly on x86_64:
__Z5hellov:
pushq %rbp
movq %rsp, %rbp
subq $16, %rsp
callq _rand
movl %eax, %ecx
imull $10, %ecx, %ecx
movl $1073741825, %esi
movl %ecx, %eax
imull %esi
movl %edx, %eax
movl %eax, %ecx
shrl $31, %ecx
sarl $29, %eax
leal (%rax,%rcx), %eax
movl %eax, -8(%rbp)
movl %eax, -4(%rbp)
movl -4(%rbp), %eax
addq $16, %rsp
popq %rbp
ret

// Produced assembly on i386:
__Z5hellov:
pushl %ebp
movl %esp, %ebp
subl $8, %esp
call _rand
imull $10, %eax, %eax
movl $1073741825, %ecx
imull %ecx
movl %edx, %eax
shrl $31, %eax
sarl $29, %edx
leal (%edx,%eax), %eax
movl %eax, -8(%ebp)
movl %eax, -4(%ebp)
movl -4(%ebp), %eax
addl $8, %esp
popl %ebp
ret[/code]


Also there are a couple of more fixes for Android in specific at https://github.com/quarnster/angelscript/compare/caa82a0...patchesforandreas

Its all compiling, linking and mostly running too. There's an issue with unaligned 64 bit arguments so I created two new tests to catch that better, but probably won't spend time looking closer at fixing the native call functions (any one really use/need double/i64 on android devices?).

Share this post


Link to post
Share on other sites
I tested the game sample on Linux 64bit and it also put everything on (0,0). In my opinion GNUC is producing incorrect code here. The disassembly you showed, do seem to be correct in that it first multiplies the result from rand() with 10 and then divides with RAND_MAX, but it is possible that in the game sample, the optimizer produces a different result. However, I'm not going to spend time investigating it further.

I made a simpler change to fix this, which is to simply do a modulus operation, i.e. rand()%10. I know the modulus doesn't give a proper uniform randomness, but I really don't care about that in this sample. :)


Thanks for the patch for Android. I'll apply those changes as well.

I already knew about the problem with 64bit primitives on Android (as reported on the [url="http://www.angelcode.com/angelscript/wip.html"]WIP page[/url]), and I have a fairly good idea on how to fix it. I just haven't had the time to work on it.


Regards,
Andreas

Share this post


Link to post
Share on other sites
The comment on the problem in aswrappedcall.h is from the following thread: [url="http://www.gamedev.net/topic/623916-casting-with-generic-call-conv/"]http://www.gamedev.net/topic/623916-casting-with-generic-call-conv/[/url]

I guess the situation was probably caused by a different version of gnuc (possibly older) rather than the compilation for Android being made by a different compiler.

Which version of gnuc are you using to compile on Android?

Share this post


Link to post
Share on other sites
Gcc version is arm-linux-androideabi-gcc (GCC) 4.4.3, which is the same as has been shipped since December 2010. I thought maybe it was because I'm building with android-cmake rather than the ndk-build script, however I run into the same error with the ndk-build script.

[code]cd angelscript/sdk/angelscript/projects
mkdir build-android
cd build-android/
android create project -p . -t android-8 -a NativeActivity -k com.arne
mkdir jni
echo "LOCAL_PATH:= \$(call my-dir)
include \$(CLEAR_VARS)
LOCAL_CFLAGS := -I../../../add_on/ -I../../include
LOCAL_SRC_FILES:= test.cpp
LOCAL_MODULE:= libtest
include \$(BUILD_SHARED_LIBRARY)" > jni/Android.mk
echo "#include \"autowrapper/aswrappedcall.h\"
class Alma
{
public:
void Foo()
{
}
};
void test()
{
WRAP_MFN( Alma, Foo );
}
" > jni/test.cpp
ndk-build
[/code]

With aswrappedcall.h unmodified I get:
[code]Compile++ thumb : test <= test.cpp
jni/test.cpp: In function 'void test()':
jni/test.cpp:11: error: 'template' (as a disambiguator) is only allowed within templates
make: *** [obj/local/armeabi/objs/test/test.o] Error 1[/code]

But with my change it works:
[code]Compile++ thumb : test <= test.cpp
StaticLibrary : libstdc++.a
SharedLibrary : libtest.so
Install : libtest.so => libs/armeabi/libtest.so[/code]

So I'm not really sure why brumi needed this to compile... For reference I'm on Android NDK r8.

Share this post


Link to post
Share on other sites
I made it so that the template keyword is only included if the GNUC version is 4.3 or lower. I'm not sure which version of GNUC that brumi was using, but at least it shouldn't cause any problems with the more current versions.

Regards,
Andreas

Share this post


Link to post
Share on other sites

This topic is 1971 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this