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.

Sign up for GitHub

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

Jump to bottom

[CIR][LowerToLLVM][CXXABI] Lower cir.va.arg #573

Merged

bcardosolopes merged 3 commits into llvm:main from ghehg:main

May 28, 2024

Collaborator

ghehg commented Apr 29, 2024

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make LoweringPrepareArm64CXXABI class inherit more generic LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this official doc.

The following pic is a easier-to-understand and close to lowered code explanation

Screenshot 2024-04-29 at 4 02 17 PM

bcardosolopes requested changes

View reviewed changes

Member

bcardosolopes left a comment

Hey, thanks for the PR, this is much needed.

You should also add LLVM checks on the relevant part of the end result, we need to make sure it's correct overall (i.e. if parts are connected right).

clang/lib/CIR/Dialect/Transforms/LoweringPrepareArm64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareArm64CXXABI.cpp Outdated Show resolved Hide resolved

ghehg force-pushed the main branch 3 times, most recently from 168ca49 to 0a6e761 Compare

May 6, 2024 16:31

bcardosolopes changed the title ~~[CIR][Lowering][CXXABI]lower var_arg op~~ [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg

bcardosolopes requested changes

View reviewed changes

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated

@@ @@ -1,4 +1,5 @@ @@
-              //====- LoweringPrepareItaniumCXXABI.h - Itanium ABI specific code --------===//
+              //====- LoweringPrepareItaniumCXXABI.cpp - Itanium ABI specific code
+              //--------===//

Member

bcardosolopes May 7, 2024

ops, this should fit in 80 instead

Member

bcardosolopes May 10, 2024

Not resolved, please address this.

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated Show resolved Hide resolved

clang/test/CIR/CodeGen/var-arg-float.c Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

ghehg force-pushed the main branch from 0a6e761 to 2e48c55 Compare

May 9, 2024 03:20

bcardosolopes reviewed

View reviewed changes

clang/test/CIR/CodeGen/var-arg-float.c Outdated Show resolved Hide resolved

ghehg force-pushed the main branch 2 times, most recently from 3adfff7 to acede3e Compare

May 10, 2024 01:29

ghehg requested a review from bcardosolopes

May 10, 2024 19:11

bcardosolopes requested changes

View reviewed changes

clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated

@@ @@ -1,4 +1,5 @@ @@
-              //====- LoweringPrepareItaniumCXXABI.h - Itanium ABI specific code --------===//
+              //====- LoweringPrepareItaniumCXXABI.cpp - Itanium ABI specific code
+              //--------===//

Member

bcardosolopes May 10, 2024

Not resolved, please address this.

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.h Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareItaniumCXXABI.cpp Outdated Show resolved Hide resolved

clang/test/CIR/CodeGen/variadics.c Outdated Show resolved Hide resolved

ghehg force-pushed the main branch from acede3e to a5e55c2 Compare

May 20, 2024 18:37

bcardosolopes reviewed

View reviewed changes

Member

bcardosolopes left a comment

Thanks for applying the reviews, few more nits!

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/LoweringPrepareAArch64CXXABI.cpp Show resolved Hide resolved

ghehg force-pushed the main branch from a5e55c2 to 5214ce7 Compare

May 21, 2024 15:36

ghehg added 3 commits

May 28, 2024 05:53


          AArch64 ABI lowering cir.var_arg, squashed related commits

8d65d17


          add more skeleton code and missing features

09b1536


          use CIRDataLayout to get some ABI info

2f2214c

ghehg force-pushed the main branch from 5214ce7 to 2f2214c Compare

May 28, 2024 12:54

bcardosolopes approved these changes

View reviewed changes

Member

bcardosolopes left a comment

Thanks for your patience, LGTM!

bcardosolopes merged commit b5f3862 into llvm:main

6 checks passed

ChuanqiXu9 mentioned this pull request

How should we handle va_arg? #862

Open

bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this pull request


          [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg (llvm#573)

eba61e7

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make
LoweringPrepareArm64CXXABI class inherit more generic
LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only
meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this
official doc.

[](https://github.com/ARM-software/abi-aa/blob/617079d8a0d45bec83d351974849483cf0cc66d5/aapcs64/aapcs64.rst#appendix-variable-argument-lists)

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request


          [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg (llvm#573)

34d9461

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make
LoweringPrepareArm64CXXABI class inherit more generic
LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only
meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this
official doc.

[](https://github.com/ARM-software/abi-aa/blob/617079d8a0d45bec83d351974849483cf0cc66d5/aapcs64/aapcs64.rst#appendix-variable-argument-lists)

smeenai pushed a commit to smeenai/clangir that referenced this pull request


          [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg (llvm#573)

89a89cf

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make
LoweringPrepareArm64CXXABI class inherit more generic
LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only
meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this
official doc.

[](https://github.com/ARM-software/abi-aa/blob/617079d8a0d45bec83d351974849483cf0cc66d5/aapcs64/aapcs64.rst#appendix-variable-argument-lists)

smeenai mentioned this pull request

clang crashed when handling va_list #977

Open

keryell pushed a commit to keryell/clangir that referenced this pull request


          [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg (llvm#573)

db40707

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make
LoweringPrepareArm64CXXABI class inherit more generic
LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only
meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this
official doc.

[](https://github.com/ARM-software/abi-aa/blob/617079d8a0d45bec83d351974849483cf0cc66d5/aapcs64/aapcs64.rst#appendix-variable-argument-lists)

lanza pushed a commit that referenced this pull request


          [CIR][LowerToLLVM][CXXABI] Lower cir.va.arg (#573)

b67d1ba

lowering var_arg op for ARM64 architecture. This is CIR lowering.

This PR modified LoweringPrepare CXXABI code to make
LoweringPrepareArm64CXXABI class inherit more generic
LoweringPrepareItaniumCXXABI, this way lowering var_arg would be only
meaningful for arm64 targets and for other arch its no op for now.

The ABI doc and detailed algorithm description can be found in this
official doc.

[](https://github.com/ARM-software/abi-aa/blob/617079d8a0d45bec83d351974849483cf0cc66d5/aapcs64/aapcs64.rst#appendix-variable-argument-lists)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet