Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimization for t194 should use gcc or clang? #1242

Open
leonpano2006 opened this issue Feb 1, 2024 · 20 comments
Open

optimization for t194 should use gcc or clang? #1242

leonpano2006 opened this issue Feb 1, 2024 · 20 comments

Comments

@leonpano2006
Copy link
Contributor

leonpano2006 commented Feb 1, 2024

i find out is maybe better to use clang cause by clang-11 and higher have mcpu/mtune=carmel for t194(xavier)
however gcc does not, even gcc-13 does not
but this means users maybe have to install llvm stuff
so i open this issue to confirm should i change to mtune/mcpu=carmel or keep like now(cortex-a76) or make both options(like TEGRA_T194_CLANG and TEGRA_T194_GCC, this is just example and t234 is not affected since is cortex-a based(note: t2XX are cortex-a based, but t1XX is not)

cortex-a76 is suggested by nvidia staff on dev fourm
https://forums.developer.nvidia.com/t/gcc-mcpu-mtune-for-carmel-cpu/146345/6?u=leonpano

@leonpano2006
Copy link
Contributor Author

and this is same for box86
i don't think i should open issue on both side since is basically same thing

@ptitSeb
Copy link
Owner

ptitSeb commented Feb 1, 2024

Also, keep in mind that clang or gcc optimisation will affect only box64 (/box86) code, and not the generated Dynarec by box64. Don't expect too much difference between optimized build unless you use the Interpreter intensly. In normal use case, most of the time is spent in generated dynarec code...

@leonpano2006
Copy link
Contributor Author

i got some errors in clang

/home/leonpano/box64/src/custommem.c:1289:40: warning: passing 'int *' to parameter of type 'uint32_t *' (aka 'unsigned int *') converts between pointers to integer types with different sign [-Wpointer-sign]
 1289 |         if(!rb_get_end(mapallmem, cur, &prot, &bend)) {
      |                                        ^~~~~
/home/leonpano/box64/src/include/rbtree.h:12:56: note: passing argument to parameter 'val' here
   12 | int rb_get_end(rbtree* tree, uintptr_t add[ 13%] Building C object CMakeFiles/box64.dir/src/elfs/elfloader.c.o
r, uint32_t* val, uintptr_t* end);
      |                                                        ^
/home/leonpano/box64/src/custommem.c:1322:40: warning: passing 'int *' to parameter of type 'uint32_t *' (aka 'unsigned int *') converts between pointers to integer types with different sign [-Wpointer-sign]
 1322 |         if(!rb_get_end(mapallmem, cur, &prot, &bend)) {
      |                                        ^~~~~
/home/leonpano/box64/src/include/rbtree.h:12:56: note: passing argument to parameter 'val' here
   12 | int rb_get_end(rbtree* tree, uintptr_t addr, uint32_t* val, uintptr_t* end);
      |                                                        ^
/home/leonpano/box64/src/custommem.c:1353:36: warning: passing 'int *' to parameter of type 'uint32_t *' (aka 'unsigned int *') converts between pointers to integer types with different sign [-Wpointer-sign]
 1353 |     if(!rb_get_end(mapallmem, cur, &prot, &bend)) {
      |                                    ^~~~~
/home/leonpano/box64/src/include/rbtree.h:12:56: note: passing argument to parameter 'val' here
   12 | int rb_get_end(rbtree* tree, uintptr_t addr, uint32_t* val, uintptr_t* end);
      |                                                        ^
/home/leonpano/box64/src/custommem.c:1430:40: warning: passing 'int *' to parameter of type 'uint32_t *' (aka 'unsigned int *') converts between pointers to integer types with different sign [-Wpointer-sign]
 1430 |         if(!rb_get_end(mapallmem, cur, &prot, &bend)) {
      |                                        ^~~~~
/home/leonpano/box64/src/include/rbtree.h:12:56: note: passing argument to parameter 'val' here
   12 | int rb_get_end(rbtree* tree, uintptr_t addr, uint32_t* val, uintptr_t* end);

and

/home/leonpano/box64/src/emu/x64emu.c:195:13: error: call to undeclared function 'internal_munmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  195 |         if(!internal_munmap(emu->stack2free, emu->size_stack))
      |             ^
/home/leonpano/box64/src/emu/x64emu.c:196:13: error: call to undeclared function 'freeProtection'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  196 |             freeProtection((uintptr_t)emu->stack2free, emu->size_stack);
      |             ^
[ 15%] Building C object CMakeFiles/box64.dir/src/emu/x64primop.c.o
/home/leonpano/box64/src/emu/x64emu.c:619:21: error: use of unknown builtin '__builtin_aarch64_get_fpcr' [-Wimplicit-function-declaration]
  619 |     uint64_t fpcr = __builtin_aarch64_get_fpcr();
      |                     ^
/home/leonpano/box64/src/emu/x64emu.c:627:5: error: use of unknown builtin '__builtin_aarch64_set_fpcr' [-Wimplicit-function-declaration]
  627 |     __builtin_aarch64_set_fpcr(fpcr);
      |     ^

@ptitSeb
Copy link
Owner

ptitSeb commented Feb 1, 2024

the 1st block is probably easy to fix, but the second part, it will need to be disable to build...

@leonpano2006
Copy link
Contributor Author

1st part is not fatal
but 2nd part is problem to cause compile fail

@mcagabe19
Copy link
Contributor

mcagabe19 commented Feb 2, 2024

/home/leonpano/box64/src/emu/x64emu.c:195:13: error: call to undeclared function 'internal_munmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
195 | if(!internal_munmap(emu->stack2free, emu->size_stack))
| ^
/home/leonpano/box64/src/emu/x64emu.c:196:13: error: call to undeclared function 'freeProtection'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
196 | freeProtection((uintptr_t)emu->stack2free, emu->size_stack);
| ^

idk these

/home/leonpano/box64/src/emu/x64emu.c:619:21: error: use of unknown builtin '__builtin_aarch64_get_fpcr' [-Wimplicit-function-declaration]
619 | uint64_t fpcr = __builtin_aarch64_get_fpcr();
| ^
/home/leonpano/box64/src/emu/x64emu.c:627:5: error: use of unknown builtin '__builtin_aarch64_set_fpcr' [-Wimplicit-function-declaration]
627 | __builtin_aarch64_set_fpcr(fpcr);
| ^

these macros are doesn't exists in clang

@mcagabe19
Copy link
Contributor

so yeah if ur linux ur stuck with gcc

@ptitSeb
Copy link
Owner

ptitSeb commented Feb 2, 2024

@leonpano2006 about the second blocks of error: it seems you are building box64 without dynarec! Is that on purpose?

@leonpano2006
Copy link
Contributor Author

@leonpano2006 about the second blocks of error: it seems you are building box64 without dynarec! Is that on purpose?

Build with dynarec

@ptitSeb
Copy link
Owner

ptitSeb commented Feb 2, 2024

@leonpano2006 about the second blocks of error: it seems you are building box64 without dynarec! Is that on purpose?

Build with dynarec

Those warning/error:

/home/leonpano/box64/src/emu/x64emu.c:195:13: error: call to undeclared function 'internal_munmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  195 |         if(!internal_munmap(emu->stack2free, emu->size_stack))
      |             ^
/home/leonpano/box64/src/emu/x64emu.c:196:13: error: call to undeclared function 'freeProtection'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  196 |             freeProtection((uintptr_t)emu->stack2free, emu->size_stack);
      |             ^

Only appeared on non-dynarec build. I have just pushed a fix to it, the #include "custommem.h" was behind a #ifdef DYNAREC

@leonpano2006
Copy link
Contributor Author

[ 44%] Building C object CMakeFiles/box64.dir/src/emu/x87emu_private.c.o
/home/leonpano/box64/src/emu/x64emu.c:619:21: error: use of unknown builtin '__builtin_aarch64_get_fpcr' [-Wimplicit-function-declaration]
  619 |     uint64_t fpcr = __builtin_aarch64_get_fpcr();
      |                     ^
/home/leonpano/box64/src/emu/x64emu.c:627:5: error: use of unknown builtin '__builtin_aarch64_set_fpcr' [-Wimplicit-function-declaration]
  627 |     __builtin_aarch64_set_fpcr(fpcr);
      |     ^
2 errors generated.
make[2]: *** [CMakeFiles/box64.dir/build.make:527: CMakeFiles/box64.dir/src/emu/x64emu.c.o] Error 1

build with cmake .. -D TEGRA_T194=1 -D CMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=/usr/bin/clang-19 -D ARM_DYNAREC=ON
but i find sometimes i have to rm build then mkdir build again or it may have such issue like this

leonpano@leonpano-desktop:~/box64/build$ cmake .. -D TEGRA_T194=1 -D CMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=/usr/bin/clang-19 -D ARM_DYNAREC=ON
-- The C compiler identification is Clang 19.0.0
-- The ASM compiler identification is Clang with GNU-like command-line
-- Found assembler: /usr/bin/clang-18
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/clang-19 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done (1.8s)
-- Generating done (0.2s)
-- Build files have been written to: /home/leonpano/box64/build

@leonpano2006
Copy link
Contributor Author

and after i do this to make it use right version

leonpano@leonpano-desktop:~/box64/build$ cd ..
leonpano@leonpano-desktop:~/box64$ rm -rf build;mkdir build; cd build/
leonpano@leonpano-desktop:~/box64/build$ cmake .. -D TEGRA_T194=1 -D CMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=/usr/bin/clang-19 -D ARM_DYNAREC=ON
-- Found Python3: /usr/bin/python3.9 (found version "3.9.5") found components: Interpreter 
-- The C compiler identification is Clang 19.0.0
-- The ASM compiler identification is Clang with GNU-like command-line
-- Found assembler: /usr/bin/clang-19
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/clang-19 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done (3.4s)
-- Generating done (0.2s)
-- Build files have been written to: /home/leonpano/box64/build

@leonpano2006
Copy link
Contributor Author

and part you said i build without ARM dynarec just because of error build
but appears issue is not cause by ARM dynarec

@leonpano2006
Copy link
Contributor Author

leonpano2006 commented Feb 2, 2024

i think for now clang is not possible
unfortunately or someone have to fix code for clang to work
i tried clang-16 and 17 and 18 and 19
all clang are the same

build log form clang-19
https://gist.github.com/leonpano2006/72f2a340a8fd3edb42f8a4e5bda80c16

build log form gcc-13
https://gist.github.com/leonpano2006/a85fd3856b5188628f728a1e45e13b8b

@ptitSeb
Copy link
Owner

ptitSeb commented Feb 2, 2024

it's easy to fix, but I don't want to have to maintain clang build. If someone do a PR about it, I'll merge it, but I don't want to it myself as I have too many other things to maintain already.

@mcagabe19
Copy link
Contributor

@leonpano2006 after this gets pulled u can build in clang

@leonpano2006
Copy link
Contributor Author

@leonpano2006 after this gets pulled u can build in clang

sorry for very late reply
but now it build failed at linking part

[100%] Building C object CMakeFiles/box64.dir/src/wrapped/wrappedanl.c.o
240 warnings generated.
[100%] Linking C executable box64
/usr/bin/ld: unrecognized option '--image-base=0x34800000'
/usr/bin/ld: use the --help option for usage information
clang-19: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [CMakeFiles/box64.dir/build.make:5016: box64] Error 1
make[1]: *** [CMakeFiles/Makefile2:140: CMakeFiles/box64.dir/all] Error 2
make: *** [Makefile:166: all] Error 2

@leonpano2006
Copy link
Contributor Author

when i test t194 flag for clang is bellow

elseif(TEGRA_T194)
    add_definitions(-DTEGRA_T194)
    add_definitions(-pipe -march=armv8.2-a+fp16+simd+crypto+predres -mcpu=carmel+crypto -mtune=carmel)
    set(CMAKE_ASM_FLAGS  "-pipe -march=armv8.2-a+fp16+simd+crypto+predres -mcpu=carmel+crypto -mtune=carmel")

@leonpano2006
Copy link
Contributor Author

leonpano2006 commented Apr 26, 2024

but it works now with clang-19+mold
but still have lots of warning on compiling
like
warning: pointer type mismatch ('void *' and 'int (*)(void *, void *, void *)') [-Wpointer-type-mismatch]
warning: pointer type mismatch ('void *' and 'int (*)(void *, void *)') [-Wpointer-type-mismatch]
warning: pointer type mismatch ('void *' and 'int (*)(void *, int *)') [-Wpointer-type-mismatch]
warning: pointer type mismatch ('void *' and 'int (*)(void *)') [-Wpointer-type-mismatch]

@mcagabe19
Copy link
Contributor

box64 made with gnu cpp, except these warnings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants