-
Notifications
You must be signed in to change notification settings - Fork 136
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
[CIR][CIRGen] Enable comdat for static variables #1015
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good, but I think the test can be simpler :)
// RUN: %clang_cc1 -triple aarch64-none-linux-android24 -fclangir -emit-cir %s -o %t1.cir | ||
// RUN: FileCheck --check-prefix=CIR-AARCH64 --input-file=%t1.cir %s | ||
// RUN: %clang_cc1 -triple aarch64-none-linux-android24 -fclangir -emit-llvm %s -o %t1.ll | ||
// RUN: FileCheck --check-prefix=LLVM-AARCH64 --input-file=%t1.ll %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this to be AArch64-specific, or can we just reuse the existing RUN lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, no need for comdat, I was just thinking to prepare for future target-dependent features testing in this file. But probably we should wait until that comes.
class b { | ||
public: | ||
// CIR-AARCH64-DAG: cir.global linkonce_odr comdat @_ZZN1b4testEvE1c = #cir.int<0> : !s32i | ||
|
||
// LLVM-AARCH64-DAG: $_ZZN1b4testEvE1c = comdat any | ||
// LLVM-AARCH64-DAG: @_ZZN1b4testEvE1c = linkonce_odr global i32 0, comdat, align 4 | ||
void test() { static int c; } | ||
// CIR-AARCH64-LABEL: @_ZN1b4testEv | ||
// CIR-AARCH64: {{%.*}} = cir.get_global @_ZZN1b4testEvE1c : !cir.ptr<!s32i> | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an static inside an inline function should do the trick: https://godbolt.org/z/8anK6G9x5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.