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

[Clang] Clang vs. GCC Frame Pointer Differences #108019

Open
guoxin049 opened this issue Sep 10, 2024 · 9 comments
Open

[Clang] Clang vs. GCC Frame Pointer Differences #108019

guoxin049 opened this issue Sep 10, 2024 · 9 comments
Labels
backend:ARM question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!

Comments

@guoxin049
Copy link
Contributor

We used Clang 15 to build our kernel and encountered a problem during the process. I'm not sure if this is a bug or an issue with our configuration.

enum regs {
#ifdef CONFIG_THUMB2_KERNEL
	FP = 7,
#else
	FP = 11,
#endif
	SP = 13,
	LR = 14,
	PC = 15
};

When using CONFIG_THUMB2_KERNEL, the frame pointer (FP) is set to r7. However, after compiling with Clang 15 and setting the option '-mframe-chain=aapcs+leaf', the frame pointer changes to r11.

  • I noticed that this part of the llvm code caused
    MCPhysReg getFramePointerReg() const {
    if (isTargetDarwin() ||
    (!isTargetWindows() && isThumb() && !createAAPCSFrameChain()))
    return ARM::R7;
    return ARM::R11;
    }

When the -mframe-chain=aapcs+leaf option is set, the createAAPCSFrameChain() function returns true, and the getFramePointerReg() function uses ARM::R11 as the frame pointer. However, when I compile with GCC using the options -mthumb -mabi=aapcs, it still uses r7 as the frame pointer.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Sep 10, 2024
@EugeneZelenko EugeneZelenko added backend:ARM and removed clang Clang issues not falling into any other category labels Sep 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2024

@llvm/issue-subscribers-backend-arm

Author: gxlayer (guoxin049)

We used Clang 15 to build our kernel and encountered a problem during the process. I'm not sure if this is a bug or an issue with our configuration.
enum regs {
#ifdef CONFIG_THUMB2_KERNEL
	FP = 7,
#else
	FP = 11,
#endif
	SP = 13,
	LR = 14,
	PC = 15
};

When using CONFIG_THUMB2_KERNEL, the frame pointer (FP) is set to r7. However, after compiling with Clang 15 and setting the option '-mframe-chain=aapcs+leaf', the frame pointer changes to r11.

  • I noticed that this part of the llvm code caused
    MCPhysReg getFramePointerReg() const {
    if (isTargetDarwin() ||
    (!isTargetWindows() && isThumb() && !createAAPCSFrameChain()))
    return ARM::R7;
    return ARM::R11;
    }

When the -mframe-chain=aapcs+leaf option is set, the createAAPCSFrameChain() function returns true, and the getFramePointerReg() function uses ARM::R11 as the frame pointer. However, when I compile with GCC using the options -mthumb -mabi=aapcs, it still uses r7 as the frame pointer.

@efriedma-quic
Copy link
Collaborator

-mframe-chain=aapcs+leaf means that all functions should using an AAPCS frame pointer... and the spec in question says the frame pointer register is r11. If you really want frame pointers in r7 for some reason, -mframe-chain=none -fno-omit-frame-pointer does that, I think.

I have no idea whether gcc has equivalent options.

@efriedma-quic efriedma-quic closed this as not planned Won't fix, can't repro, duplicate, stale Sep 10, 2024
@guoxin049
Copy link
Contributor Author

-mframe-chain=aapcs+leaf means that all functions should using an AAPCS frame pointer... and the spec in question says the frame pointer register is r11. If you really want frame pointers in r7 for some reason, -mframe-chain=none -fno-omit-frame-pointer does that, I think.

I have no idea whether gcc has equivalent options.

Thank you for your reply. I would like to know whether the frame pointer (FP) uses r11 or r7 when both -mframe-chain=aapcs+leaf and -mthumb options are used together. Could you clarify which register is selected for the frame pointer in this case? Thank you.

@EugeneZelenko EugeneZelenko added the question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! label Sep 11, 2024
@efriedma-quic
Copy link
Collaborator

-mframe-chain=aapcs+leaf forces the frame pointer to r11 even in Thumb mode.

@guoxin049
Copy link
Contributor Author

@efriedma-quic

void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
BitVector &SavedRegs,
RegScavenger *RS) const {
TargetFrameLowering::determineCalleeSaves(MF, SavedRegs, RS);
// This tells PEI to spill the FP as if it is any other callee-save register
// to take advantage the eliminateFrameIndex machinery. This also ensures it
// is spilled in the order specified by getCalleeSavedRegs() to make it easier
// to combine multiple loads / stores.
bool CanEliminateFrame = !(requiresAAPCSFrameRecord(MF) && hasFP(MF));

Thank you for your reply. But I'm running into a problem.
I’m trying to understand the rationale behind the condition !(requiresAAPCSFrameRecord(MF) && hasFP(MF)) for setting the boolean CanEliminateFrame. Specifically, why do both conditions need to be true simultaneously?

Given that hasFP(MF) being true implies that requiresAAPCSFrameRecord(MF) would be redundant, can this logic be simplified to just bool CanEliminateFrame = !hasFP(MF)?

@efriedma-quic
Copy link
Collaborator

I think what happened there is that the patch that added the line (https://reviews.llvm.org/D125094; CC @pratlucas) was mostly concerned about the behavior with -mframe-chain=aapcs. So the behavior changed for that case, but not for the non-AAPCS frame chain case.

As a practical matter, it's very unlikely to make a significant difference: functions without a stack frame are generally tiny anyways.

@guoxin049
Copy link
Contributor Author

I think what happened there is that the patch that added the line (https://reviews.llvm.org/D125094; CC @pratlucas) was mostly concerned about the behavior with -mframe-chain=aapcs. So the behavior changed for that case, but not for the non-AAPCS frame chain case.

As a practical matter, it's very unlikely to make a significant difference: functions without a stack frame are generally tiny anyways.

Thank you for your reply. Does LLVM currently generate stack pointers for smaller leaf functions, or are they optimized out in certain cases? Has the community considered the possibility of always generating stack pointers for such functions? thank you

This is my demo to illustrate clang and gcc the difference:
https://godbolt.org/z/Kf1e3crMc

@dongjianqiang2
Copy link
Contributor

I think what happened there is that the patch that added the line (https://reviews.llvm.org/D125094; CC @pratlucas) was mostly concerned about the behavior with -mframe-chain=aapcs. So the behavior changed for that case, but not for the non-AAPCS frame chain case.

As a practical matter, it's very unlikely to make a significant difference: functions without a stack frame are generally tiny anyways.

I think the problem @guoxin049 found is that using the -fno-omit-frame-pointer option does not generate fp for leaf functions (Dopra uses fp to backtrace), so the option -mframe-chain=aapcs+leaf is used. However, the -mframe-chain=aapcs+leaf option is changed from r7 to r11 in thumb mode, then the kernel has compatibility issues. May I ask if you have any suggestions?

@efriedma-quic
Copy link
Collaborator

-mno-omit-leaf-frame-pointer should be the option to do that, but it looks like that doesn't currently work on 32-bit Arm. Which I guess is a bug.

@efriedma-quic efriedma-quic reopened this Sep 16, 2024
hstk30-hw pushed a commit that referenced this issue Oct 17, 2024
…RM (#109628)

The -mno-omit-leaf-frame-pointer flag works on 32-bit ARM architectures
and addresses the bug reported in #108019
EricWF pushed a commit to efcs/llvm-project that referenced this issue Oct 22, 2024
…RM (llvm#109628)

The -mno-omit-leaf-frame-pointer flag works on 32-bit ARM architectures
and addresses the bug reported in llvm#108019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!
Projects
None yet
Development

No branches or pull requests

5 participants