Skip to content

Conversation

ChuanqiXu9
Copy link
Member

@ChuanqiXu9 ChuanqiXu9 commented Nov 8, 2024

This patch implements transformations for VAArg in X86_64 ABI in shape. In shape means it can't work properly due to the dependent X86_64 ABI is not robust. e.g., when we want to use VAArg with long double, we need #1087.

This patch literally implement https://github.com/llvm/llvm-project/blob/d233fedfb0de882353c348cd1ac57dab619efa6d/clang/lib/CodeGen/Targets/X86.cpp#L3015-L3240 in CIR.

There some differences due to the traditional pipeline are converting AST to LLVM and we're transforming CIR to CIR. And also to get the ABI Info, I moved X86_64ABIInfo to the header.

Copy link

github-actions bot commented Nov 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@ChuanqiXu9 ChuanqiXu9 marked this pull request as ready for review November 8, 2024 06:56
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Mostly good, pending one question

@@ -1,7 +1,5 @@
// RUN: %clang_cc1 -triple aarch64-none-linux-android21 -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
// RUN: %clang_cc1 -triple aarch64-none-linux-android21 -fclangir -emit-llvm -fno-clangir-call-conv-lowering %s -o %t.ll
// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to loose the LLVM testing here, even though it might be wrong in the ABI sense for AArch64, it's still testing some important parts. Can you elaborate on why are you remove this? Should this be moved elsewhere instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, this is a surprise to me too. My bad. I should check the PR. BTW, I like to close this one and create a new one to make the stacked PR for #1087

@bcardosolopes bcardosolopes changed the title [CIR] [Lowering] [X86_64] Support VAArg in shape [CIR][Lowering][x86_64] initial VAArg support Nov 8, 2024
@bcardosolopes bcardosolopes changed the title [CIR][Lowering][x86_64] initial VAArg support [CIR][Lowering][x86_64] Initial VAArg support Nov 8, 2024
@ChuanqiXu9
Copy link
Member Author

Close to create stacked PR

@ChuanqiXu9 ChuanqiXu9 closed this Nov 11, 2024
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

Successfully merging this pull request may close these issues.

2 participants