-
Couldn't load subscription status.
- Fork 175
[CIR] Add syncscope support for atomic load operations #1958
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
base: main
Are you sure you want to change the base?
Conversation
5b25f33 to
af464b6
Compare
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.
Thanks for working on this. Almost good, some issues need addressing!
| @@ -1,7 +1,5 @@ | |||
| // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -I%S/../Inputs -mconstructor-aliases -fclangir -fclangir-lifetime-check="history=all" -fclangir-skip-system-headers -clangir-verify-diagnostics -emit-cir %s -o %t.cir | |||
|
|
|||
| // XFAIL: * | |||
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.
Let's address this separately, because it's not failing in all platforms, I'll merge the PR regardless if the only thing failing is related to this test (no worries).
| // RUN: %clang_cc1 -x c -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s | ||
|
|
||
| // CHECK-LABEL: @scoped_load_thread |
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.
- We also need to check for LLVM output. Please rename
CHECKtoCIRand add both aLLVMand aOGCGone (seeclang/test/CIR/CodeGen/opaque.cppfor an exmple). - Change the ARM64 test for your change to check for the scope value.
- There are changes to
CIRGenAtomic.cppso maybe it's worth taking a look and changing where appropriate existing tests in:
./CodeGen/atomic-xchg-field.c
./CodeGen/atomic-thread-fence.c
./CodeGen/c11atomic.c
./CodeGen/atomic.cpp
./CodeGen/atomic-runtime.cpp
| cir::AtomicFetchKindAttr fetchAttr; | ||
| bool fetchFirst = true; | ||
| auto scopeAttr = cir::MemScopeKindAttr::get(builder.getContext(), Scope); | ||
| std::optional<cir::MemScopeKind> scopeOpt; |
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.
Looks like these 3 lines are not needed?
Fix #1868