saiislam updated this revision to Diff 255288.
saiislam added a comment.
Changes:
1. Moved builtin definition with rest of atomic builtins
2. Updated validity checking of memory order using exact mathches instead of
range matching
3. Added a sucessful test case which passes arbitrary string scope
4. Corrected formatting
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75917/new/
https://reviews.llvm.org/D75917
Files:
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGenHIP/builtin_memory_fence.cpp
clang/test/Sema/builtins.c
Index: clang/test/Sema/builtins.c
===================================================================
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -322,13 +322,13 @@
}
void test_memory_fence_errors() {
- __builtin_memory_fence(__ATOMIC_SEQ_CST + 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
+ __builtin_memory_fence(__ATOMIC_SEQ_CST + 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
- __builtin_memory_fence(__ATOMIC_ACQUIRE - 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
+ __builtin_memory_fence(__ATOMIC_ACQUIRE - 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
- __builtin_memory_fence(4); // expected-error {{too few arguments to function call, expected 2}}
+ __builtin_memory_fence(4); // expected-error {{too few arguments to function call, expected 2}}
- __builtin_memory_fence(4, 4, 4); // expected-error {{too many arguments to function call, expected 2}}
+ __builtin_memory_fence(4, 4, 4); // expected-error {{too many arguments to function call, expected 2}}
- __builtin_memory_fence(3.14, ""); // expected-warning {{implicit conversion from 'double' to 'unsigned int' changes value from 3.14 to 3}}
+ __builtin_memory_fence(3.14, ""); // expected-warning {{implicit conversion from 'double' to 'unsigned int' changes value from 3.14 to 3}}
}
Index: clang/test/CodeGenHIP/builtin_memory_fence.cpp
===================================================================
--- clang/test/CodeGenHIP/builtin_memory_fence.cpp
+++ clang/test/CodeGenHIP/builtin_memory_fence.cpp
@@ -19,4 +19,7 @@
// CHECK: fence syncscope("workgroup") release
__builtin_memory_fence(3, "workgroup");
+
+ // CHECK: fence syncscope("foobar") release
+ __builtin_memory_fence(3, "foobar");
}
\ No newline at end of file
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1869,8 +1869,7 @@
? "__builtin_return_address"
: "__builtin_frame_address")
<< TheCall->getSourceRange();
- }
- break;
+ } break;
case Builtin::BI__builtin_memory_fence: {
ExprResult Arg = TheCall->getArg(0);
@@ -1878,22 +1877,27 @@
Expr::EvalResult ArgResult;
if(!ArgExpr->EvaluateAsInt(ArgResult, Context)) {
- Diag(ArgExpr->getExprLoc(), diag::err_typecheck_expect_int) <<
- ArgExpr->getType();
+ Diag(ArgExpr->getExprLoc(), diag::err_typecheck_expect_int)
+ << ArgExpr->getType();
return ExprError();
}
int ord = ArgResult.Val.getInt().getZExtValue();
// Check valididty of memory ordering as per C11 / C++11's memody model.
- if (ord < static_cast<int>(llvm::AtomicOrderingCABI::acquire) ||
- ord > static_cast<int>(llvm::AtomicOrderingCABI::seq_cst)) {
- Diag(ArgExpr->getBeginLoc(),
- diag::warn_atomic_op_has_invalid_memory_order)
- << ArgExpr->getSourceRange();
- return ExprError();
- }
+ switch (static_cast<llvm::AtomicOrderingCABI>(ord)) {
+ case llvm::AtomicOrderingCABI::acquire:
+ case llvm::AtomicOrderingCABI::release:
+ case llvm::AtomicOrderingCABI::acq_rel:
+ case llvm::AtomicOrderingCABI::seq_cst:
+ break;
+ default: {
+ Diag(ArgExpr->getBeginLoc(),
+ diag::warn_atomic_op_has_invalid_memory_order)
+ << ArgExpr->getSourceRange();
+ return ExprError();
+ }
}
- break;
+ } break;
}
// Since the target specific builtins for each arch overlap, only check those
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13624,7 +13624,7 @@
Value *Order = EmitScalarExpr(E->getArg(0));
Value *Scope = EmitScalarExpr(E->getArg(1));
- if ( isa<llvm::ConstantInt>(Order)) {
+ if (isa<llvm::ConstantInt>(Order)) {
int ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
// Map C11/C++11 memory ordering to LLVM memory ordering
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -785,6 +785,11 @@
BUILTIN(__sync_fetch_and_umin, "UiUiD*Ui", "n")
BUILTIN(__sync_fetch_and_umax, "UiUiD*Ui", "n")
+// clang builtin to expose llvm fence instruction
+// First argument : uint in range [2, 5] i.e. [acquire, seq_cst]
+// Second argument : target specific sync scope string
+BUILTIN(__builtin_memory_fence, "vUicC*", "n")
+
// Random libc builtins.
BUILTIN(__builtin_abort, "v", "Fnr")
BUILTIN(__builtin_index, "c*cC*i", "Fn")
@@ -1577,11 +1582,6 @@
BUILTIN(__builtin_ms_va_end, "vc*&", "n")
BUILTIN(__builtin_ms_va_copy, "vc*&c*&", "n")
-// clang builtin to expose llvm fence instruction
-// First argument : uint in range [2, 5] i.e. [acquire, seq_cst]
-// Second argument : target specific sync scope string
-BUILTIN(__builtin_memory_fence, "vUicC*", "n")
-
#undef BUILTIN
#undef LIBBUILTIN
#undef LANGBUILTIN
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
