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

[RISCV] Disable zcmp push/pop for variadic functions. #65302

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

yetingk
Copy link
Contributor

@yetingk yetingk commented Sep 5, 2023

Variadic function needs a save region for variable arguement and the region is possible to be overlaped with the region of zcmp push/pop used.

@yetingk yetingk requested a review from a team as a code owner September 5, 2023 07:00
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 5, 2023

Surely you can use Zcmp, you just have to do a separate stack adjustment and saving of a0-a7?

@yetingk
Copy link
Contributor Author

yetingk commented Sep 6, 2023

Surely you can use Zcmp, you just have to do a separate stack adjustment and saving of a0-a7?

If we don't want to break the concept that the area of register varargs is higher than the area of callee-saves, we should emit cm.push before saving varargs first. Could I just add a TODO for this PR?

Variadic function needs a save region for variable arguement and the region is
possible to be overlaped with the region of zcmp push/pop used.
@kito-cheng
Copy link
Member

It's bug fix so personally I would like to have this fix and backport to LLVM 17 if possible at this moment.

cc @asb

@yetingk yetingk merged commit 1f15155 into llvm:main Sep 11, 2023
1 of 2 checks passed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Variadic function needs a save region for variable arguement and the
region is possible to be overlaped with the region of zcmp push/pop
used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants