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

EEGCC output's bad asm / asm alignment. #364

Closed
weirdbeardgame opened this issue Apr 5, 2024 · 5 comments
Closed

EEGCC output's bad asm / asm alignment. #364

weirdbeardgame opened this issue Apr 5, 2024 · 5 comments

Comments

@weirdbeardgame
Copy link

weirdbeardgame commented Apr 5, 2024

When running through Fatal Frame 2 with splat's EEGCC compiler option enabled, I noticed I started to get function already defined errors.

In the ASM of these functions, the align is set to 3

.align 3
glabel InitGPhaseSys__Fv
/* B09A8 001AF9A8 F0FFBD27 */  addiu      $29, $29, -0x10
/* B09AC 001AF9AC 4B00023C */  lui        $2, %hi(D_4AF6E0)
/* B09B0 001AF9B0 E0F64224 */  addiu      $2, $2, %lo(D_4AF6E0)
/* B09B4 001AF9B4 0000BFFF */  sd         $31, 0x0($29)
/* B09B8 001AF9B8 FFFF0424 */  addiu      $4, $0, -0x1
/* B09BC 001AF9BC 05000324 */  addiu      $3, $0, 0x5
/* B09C0 001AF9C0 14004224 */  addiu      $2, $2, 0x14
/* B09C4 001AF9C4 00000000 */  nop

Which causes the issue. Manually removing the .align statements in these files will fix the compiler issue.

@weirdbeardgame
Copy link
Author

This issue does not occur on the normal GCC compiler.

@ethteck
Copy link
Owner

ethteck commented Apr 5, 2024

thanks for the report @weirdbeardgame - can you link a repro branch?

@weirdbeardgame
Copy link
Author

https://github.com/weirdbeardgame/Fatal-Frame-2 - Uncomment Gphase.c and the file right below it then try and run it

@AngheloAlf
Copy link
Collaborator

AngheloAlf commented Apr 5, 2024

This behavior is expected due to how the INCLUDE_ASM macro is defined.
What is happening is both your INCLUDE_ASM macro and the function asm file are defining the same symbol (InitGPhaseSys__Fv in this case) by the "\t.globl\t" #NAME "\n" and the glabel InitGPhaseSys__Fv respectively.

The processed asm ends up looking something like this:

.section .text
    .align    3
    .globl    InitGPhaseSys__Fv
    .ent    InitGPhaseSys__Fv
InitGPhaseSys__Fv:
    .set noreorder
    .set noat
.align 3
glabel InitGPhaseSys__Fv
/* B09A8 001AF9A8 F0FFBD27 */  addiu      $29, $29, -0x10
# ...

Notice how the symbol is defined twice. This usually doesn't matter, because in normal circumstances there's nothing that may change the alignment of the symbol between the first and second declaration, but here there's an alignment directive that (in the eyes of the assembler) may change the address of the symbol, then the assembler decides to bark at you.

You can rewrite the INCLUDE_ASM macro to something like this to fix it:

#define INCLUDE_ASM(FOLDER, NAME) \
    __asm__( \
        ".section .text\n" \
        "    .set noat\n" \
        "    .set noreorder\n" \
        "    .include \""FOLDER"/"#NAME".s\"\n" \
        "    .set reorder\n" \
        "    .set at\n" \
        "    .globl    " #NAME ".NON_MATCHING\n" \
        "    " #NAME ".NON_MATCHING" " = " #NAME "\n" \
    )

The important part is the macro is not defining the symbol again, it is letting the asm file to do that instead.
I know it works because that macro is what I'm using at hit_and_run

btw, why is your INCLUDE_ASM macro so cursed? why do you even need something like INCLUDE_ASM_INTERNAL?

@weirdbeardgame
Copy link
Author

That seemed to fix this issue. Thanks!

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