[PATCH] D78075: [Clang][OpenMP] Added support for nowait target in CodeGen

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
Herald added subscribers: cfe-commits, guansong, yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Previously for nowait target, CG emitted a function call to
`__tgt_target_nowait`, etc. However, in OpenMP RTL, these functions just
directly call the no-nowait version, which means nowait is not working as
expected.

OpenMP specification says a target is acutally a target task, which is an untied
and detachable task. It is natural to go to the direction that generates a task
for a nowait target. However, OpenMP task has a problem that it must be within
to a parallel region; otherwise the task will be executed immediately. As a
result, if we directly wrap to a regular task, the nowait target outside of a
parallel region is still a synchronous version.

In D77609 , I added the support for unshackled 
task in OpenMP RTL. Basically,
unshackled task is a task that is not bound to any parallel region. So all
nowait target will be tranformed into an unshackled task. In order to
distinguish from regular task, a new flag bit is set for unshackled task. This
flag will be used by RTL for later process.

Signed-off-by: Shilei Tian 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78075

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp

Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3615,6 +3615,8 @@
   OMPTaskDataTy Data;
   // The task is not final.
   Data.Final.setInt(/*IntVal=*/false);
+  // The task is unshackled
+  Data.Unshackled = true;
   // Get list of firstprivate variables.
   for (const auto *C : S.getClausesOfKind()) {
 auto IRef = C->varlist_begin();
Index: clang/lib/CodeGen/CGOpenMPRuntime.h
===
--- clang/lib/CodeGen/CGOpenMPRuntime.h
+++ clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -118,6 +118,7 @@
   unsigned NumberOfParts = 0;
   bool Tied = true;
   bool Nogroup = false;
+  bool Unshackled = false;
 };
 
 /// Class intended to support codegen of all kind of the reduction clauses.
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5045,6 +5045,7 @@
 DestructorsFlag = 0x8,
 PriorityFlag = 0x20,
 DetachableFlag = 0x40,
+UnshackledFlag = 0x80,
   };
   unsigned Flags = Data.Tied ? TiedFlag : 0;
   bool NeedsCleanup = false;
@@ -5057,6 +5058,8 @@
 Flags = Flags | PriorityFlag;
   if (D.hasClausesOfKind())
 Flags = Flags | DetachableFlag;
+  if (Data.Unshackled)
+Flags |= UnshackledFlag;
   llvm::Value *TaskFlags =
   Data.Final.getPointer()
   ? CGF.Builder.CreateSelect(Data.Final.getPointer(),
@@ -9789,6 +9792,7 @@
   assert(OutlinedFn && "Invalid outlined function!");
 
   const bool RequiresOuterTask = D.hasClausesOfKind();
+  const bool HasNowaitClause = D.hasClausesOfKind();
   llvm::SmallVector CapturedVars;
   const CapturedStmt &CS = *D.getCapturedStmt(OMPD_target);
   auto &&ArgsCodegen = [&CS, &CapturedVars](CodeGenFunction &CGF,
@@ -9801,7 +9805,8 @@
   llvm::Value *MapTypesArray = nullptr;
   // Fill up the pointer arrays and transfer execution to the device.
   auto &&ThenGen = [this, Device, OutlinedFn, OutlinedFnID, &D, &InputInfo,
-&MapTypesArray, &CS, RequiresOuterTask, &CapturedVars,
+&MapTypesArray, &CS, RequiresOuterTask, HasNowaitClause,
+&CapturedVars,
 SizeEmitter](CodeGenFunction &CGF, PrePostActionTy &) {
 if (Device.getInt() == OMPC_DEVICE_ancestor) {
   // Reverse offloading is not supported, so just execute on the host.
@@ -9851,7 +9856,6 @@
 // Emit tripcount for the target loop-based directive.
 emitTargetNumIterationsCall(CGF, D, DeviceID, SizeEmitter);
 
-bool HasNowait = D.hasClausesOfKind();
 // The target region is an outlined function launched by the runtime
 // via calls __tgt_target() or __tgt_target_teams().
 //
@@ -9897,9 +9901,7 @@
NumTeams,
NumThreads};
   Return = CGF.EmitRuntimeCall(
-  createRuntimeFunction(HasNowait ? OMPRTL__tgt_target_teams_nowait
-  : OMPRTL__tgt_target_teams),
-  OffloadingArgs);
+  createRuntimeFunction(OMPRTL__tgt_target_teams), OffloadingArgs);
 } else {
   llvm::Value *OffloadingArgs[] = {DeviceID,
OutlinedFnID,
@@ -9908,10 +9910,8 @@
InputInfo.PointersArray.getPointer(),
InputInfo

[PATCH] D78075: [Clang][OpenMP] Added support for nowait target in CodeGen

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D78075#1980746 , @jdoerfert wrote:

> You need to update the tests too.


Yeah, I will do that. Basically I would like to do that if this direction is 
not wrong... :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78075/new/

https://reviews.llvm.org/D78075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80439: Replace separator in OpenMP variant name mangling.

[Shilei Tian via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bd7e4188a09: Replace separator in OpenMP variant name 
mangling. (authored by LukasSommerTu, committed by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80439/new/

https://reviews.llvm.org/D80439

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/OpenMPClause.cpp
  clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp

Index: clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -fopenmp-version=50 | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t -fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - -fopenmp-version=50 | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// expected-no-diagnostics
+
+// CHECK-DAG: @_Z3barv
+// CHECK-DAG: @_Z3bazv
+// CHECK-DAG: @"_Z54bar$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"
+// CHECK-DAG: @"_Z54baz$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"
+// CHECK-DAG: call i32 @"_Z54bar$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"()
+// CHECK-DAG: call i32 @"_Z54baz$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"()
+
+#ifndef HEADER
+#define HEADER
+
+#pragma omp declare target
+
+int bar() { return 1; }
+
+int baz() { return 5; }
+
+#pragma omp begin declare variant match(device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any)})
+
+int bar() { return 2; }
+
+int baz() { return 6; }
+
+#pragma omp end declare variant
+
+#pragma omp end declare target
+
+int main() {
+  int res;
+#pragma omp target map(from \
+   : res)
+  res = bar() + baz();
+  return res;
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/AST/OpenMPClause.cpp
===
--- clang/lib/AST/OpenMPClause.cpp
+++ clang/lib/AST/OpenMPClause.cpp
@@ -2167,22 +2167,21 @@
   std::string MangledName;
   llvm::raw_string_ostream OS(MangledName);
   for (const OMPTraitSet &Set : Sets) {
-OS << '.' << 'S' << unsigned(Set.Kind);
+OS << '$' << 'S' << unsigned(Set.Kind);
 for (const OMPTraitSelector &Selector : Set.Selectors) {
 
   bool AllowsTraitScore = false;
   bool RequiresProperty = false;
   isValidTraitSelectorForTraitSet(
   Selector.Kind, Set.Kind, AllowsTraitScore, RequiresProperty);
-  OS << '.' << 's' << unsigned(Selector.Kind);
+  OS << '$' << 's' << unsigned(Selector.Kind);
 
   if (!RequiresProperty ||
   Selector.Kind == TraitSelector::user_condition)
 continue;
 
   for (const OMPTraitProperty &Property : Selector.Properties)
-OS << '.' << 'P'
-   << getOpenMPContextTraitPropertyName(Property.Kind);
+OS << '$' << 'P' << getOpenMPContextTraitPropertyName(Property.Kind);
 }
   }
   return OS.str();
@@ -2191,7 +2190,7 @@
 OMPTraitInfo::OMPTraitInfo(StringRef MangledName) {
   unsigned long U;
   do {
-if (!MangledName.consume_front(".S"))
+if (!MangledName.consume_front("$S"))
   break;
 if (MangledName.consumeInteger(10, U))
   break;
@@ -2199,7 +2198,7 @@
 OMPTraitSet &Set = Sets.back();
 Set.Kind = TraitSet(U);
 do {
-  if (!MangledName.consume_front(".s"))
+  if (!MangledName.consume_front("$s"))
 break;
   if (MangledName.consumeInteger(10, U))
 break;
@@ -2207,11 +2206,11 @@
   OMPTraitSelector &Selector = Set.Selectors.back();
   Selector.Kind = TraitSelector(U);
   do {
-if (!MangledName.consume_front(".P"))
+if (!MangledName.consume_front("$P"))
   break;
 Selector.Properties.push_back(OMPTraitProperty());
 OMPTraitProperty &Property = Selector.Properties.back();
-std::pair PropRestPair = MangledName.split('.');
+std::pair PropRestPair = MangledName.split('$');
 Property.Kind =
 getOpenMPContextTraitPropertyKind(Set.Kind, PropRestPair.first);
 MangledName = PropRestPair.second;
Index: clang/include/clang/AST/Decl.h

[PATCH] D78075: [Clang][OpenMP] Added support for nowait target in CodeGen

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 274223.
tianshilei1992 added a comment.
Herald added a subscriber: sstefan1.

Will update failed tests later


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78075/new/

https://reviews.llvm.org/D78075

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9297,7 +9297,8 @@
 
   assert(OutlinedFn && "Invalid outlined function!");
 
-  const bool RequiresOuterTask = D.hasClausesOfKind();
+  const bool RequiresOuterTask = D.hasClausesOfKind() ||
+ D.hasClausesOfKind();
   llvm::SmallVector CapturedVars;
   const CapturedStmt &CS = *D.getCapturedStmt(OMPD_target);
   auto &&ArgsCodegen = [&CS, &CapturedVars](CodeGenFunction &CGF,


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9297,7 +9297,8 @@
 
   assert(OutlinedFn && "Invalid outlined function!");
 
-  const bool RequiresOuterTask = D.hasClausesOfKind();
+  const bool RequiresOuterTask = D.hasClausesOfKind() ||
+ D.hasClausesOfKind();
   llvm::SmallVector CapturedVars;
   const CapturedStmt &CS = *D.getCapturedStmt(OMPD_target);
   auto &&ArgsCodegen = [&CS, &CapturedVars](CodeGenFunction &CGF,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D129008#3640233 , @jdoerfert wrote:

> In D129008#3640194 , 
> @tianshilei1992 wrote:
>
>> `callCStructCopyConstructor` is actually for Objective-C…Cannot use it here.
>
> Don't we generate copies of things elsewhere already?

No we don't. I think the best place to emit copy is where the globalized 
variable is generated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129008/new/

https://reviews.llvm.org/D129008

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Is it a good idea to define variables starting with `CMAKE`? That might be 
confusing for later maintenance by other developers because chances are that 
they will think those variables are CMake provided variables, try to look up 
into CMake document to see what they are, and then find out they are not at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch fixes the issue that the globalized variable is not properly
initialized when it is a byval struct function argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129008

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/OpenMP/globalization_byval_struct.c

Index: clang/test/OpenMP/globalization_byval_struct.c
===
--- /dev/null
+++ clang/test/OpenMP/globalization_byval_struct.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+extern int printf(const char *, ...);
+
+struct S {
+  int a;
+  float b;
+};
+
+// CHECK: define{{.*}}void @test(%struct.S* noundef byval(%struct.S) align {{[0-9]+}} [[arg:%[0-9a-zA-Z]+]])
+// CHECK: [[g:%[0-9a-zA-Z]+]] = call align {{[0-9]+}} i8* @__kmpc_alloc_shared
+// CHECK: bitcast i8* [[g]] to %struct.S*
+// CHECK: bitcast %struct.S* [[arg]] to i8**
+// CHECK: call void [[cc:@__copy_constructor[_0-9a-zA-Z]+]]
+// CHECK: void [[cc]]
+void test(struct S s) {
+#pragma omp parallel for
+  for (int i = 0; i < s.a; ++i) {
+printf("%i : %i : %f\n", i, s.a, s.b);
+  }
+}
+
+void foo() {
+  #pragma omp target teams num_teams(1)
+  {
+struct S s;
+s.a = 7;
+s.b = 11;
+test(s);
+  }
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2471,65 +2471,66 @@
   Address DeclPtr = Address::invalid();
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
+  bool DoCopy = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
-  // If we already have a pointer to the argument, reuse the input pointer.
-  if (Arg.isIndirect()) {
-// If we have a prettier pointer type at this point, bitcast to that.
-DeclPtr = Arg.getIndirectAddress();
-DeclPtr = Builder.CreateElementBitCast(DeclPtr, ConvertTypeForMem(Ty),
-   D.getName());
-// Indirect argument is in alloca address space, which may be different
-// from the default address space.
-auto AllocaAS = CGM.getASTAllocaAddressSpace();
-auto *V = DeclPtr.getPointer();
-AllocaPtr = DeclPtr;
-auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
-auto DestLangAS =
-getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
-if (SrcLangAS != DestLangAS) {
-  assert(getContext().getTargetAddressSpace(SrcLangAS) ==
- CGM.getDataLayout().getAllocaAddrSpace());
-  auto DestAS = getContext().getTargetAddressSpace(DestLangAS);
-  auto *T = DeclPtr.getElementType()->getPointerTo(DestAS);
-  DeclPtr = DeclPtr.withPointer(getTargetHooks().performAddrSpaceCast(
-  *this, V, SrcLangAS, DestLangAS, T, true));
-}
 
-// Push a destructor cleanup for this parameter if the ABI requires it.
-// Don't push a cleanup in a thunk for a method that will also emit a
-// cleanup.
-if (Ty->isRecordType() && !CurFuncIsThunk &&
-Ty->castAs()->getDecl()->isParamDestroyedInCallee()) {
-  if (QualType::DestructionKind DtorKind =
-  D.needsDestruction(getContext())) {
-assert((DtorKind == QualType::DK_cxx_destructor ||
-DtorKind == QualType::DK_nontrivial_c_struct) &&
-   "unexpected destructor type");
-pushDestroy(DtorKind, DeclPtr, Ty);
-CalleeDestructedParamCleanups[cast(&D)] =
-EHStack.stable_begin();
-  }
-}
-  } else {
-// Check if the parameter address is controlled by OpenMP runtime.
-Address OpenMPLocalAddr =
-getLangOpts().OpenMP
-? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
-: Address::invalid();
-if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
-  DeclPtr = OpenMPLocalAddr;
+  // We first check if the parameter address is controlled by OpenMP.
+  if (getLangOpts().OpenMP) {
+DeclPtr = CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D);
+if (DeclPtr.isValid()) {
   AllocaPtr = DeclPtr;
-} else {
-  // Otherwise, create a temporary to hold the value.
-  DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(&D),
-  D.getName() + ".addr", &AllocaPtr);
+

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 441742.
tianshilei1992 added a comment.

fix unused variable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129008/new/

https://reviews.llvm.org/D129008

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/OpenMP/globalization_byval_struct.c

Index: clang/test/OpenMP/globalization_byval_struct.c
===
--- /dev/null
+++ clang/test/OpenMP/globalization_byval_struct.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+extern int printf(const char *, ...);
+
+struct S {
+  int a;
+  float b;
+};
+
+// CHECK: define{{.*}}void @test(%struct.S* noundef byval(%struct.S) align {{[0-9]+}} [[arg:%[0-9a-zA-Z]+]])
+// CHECK: [[g:%[0-9a-zA-Z]+]] = call align {{[0-9]+}} i8* @__kmpc_alloc_shared
+// CHECK: bitcast i8* [[g]] to %struct.S*
+// CHECK: bitcast %struct.S* [[arg]] to i8**
+// CHECK: call void [[cc:@__copy_constructor[_0-9a-zA-Z]+]]
+// CHECK: void [[cc]]
+void test(struct S s) {
+#pragma omp parallel for
+  for (int i = 0; i < s.a; ++i) {
+printf("%i : %i : %f\n", i, s.a, s.b);
+  }
+}
+
+void foo() {
+  #pragma omp target teams num_teams(1)
+  {
+struct S s;
+s.a = 7;
+s.b = 11;
+test(s);
+  }
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2470,66 +2470,66 @@
 
   Address DeclPtr = Address::invalid();
   Address AllocaPtr = Address::invalid();
-  bool DoStore = false;
+  bool DoCopy = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
-  // If we already have a pointer to the argument, reuse the input pointer.
-  if (Arg.isIndirect()) {
-// If we have a prettier pointer type at this point, bitcast to that.
-DeclPtr = Arg.getIndirectAddress();
-DeclPtr = Builder.CreateElementBitCast(DeclPtr, ConvertTypeForMem(Ty),
-   D.getName());
-// Indirect argument is in alloca address space, which may be different
-// from the default address space.
-auto AllocaAS = CGM.getASTAllocaAddressSpace();
-auto *V = DeclPtr.getPointer();
-AllocaPtr = DeclPtr;
-auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
-auto DestLangAS =
-getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
-if (SrcLangAS != DestLangAS) {
-  assert(getContext().getTargetAddressSpace(SrcLangAS) ==
- CGM.getDataLayout().getAllocaAddrSpace());
-  auto DestAS = getContext().getTargetAddressSpace(DestLangAS);
-  auto *T = DeclPtr.getElementType()->getPointerTo(DestAS);
-  DeclPtr = DeclPtr.withPointer(getTargetHooks().performAddrSpaceCast(
-  *this, V, SrcLangAS, DestLangAS, T, true));
-}
 
-// Push a destructor cleanup for this parameter if the ABI requires it.
-// Don't push a cleanup in a thunk for a method that will also emit a
-// cleanup.
-if (Ty->isRecordType() && !CurFuncIsThunk &&
-Ty->castAs()->getDecl()->isParamDestroyedInCallee()) {
-  if (QualType::DestructionKind DtorKind =
-  D.needsDestruction(getContext())) {
-assert((DtorKind == QualType::DK_cxx_destructor ||
-DtorKind == QualType::DK_nontrivial_c_struct) &&
-   "unexpected destructor type");
-pushDestroy(DtorKind, DeclPtr, Ty);
-CalleeDestructedParamCleanups[cast(&D)] =
-EHStack.stable_begin();
-  }
-}
-  } else {
-// Check if the parameter address is controlled by OpenMP runtime.
-Address OpenMPLocalAddr =
-getLangOpts().OpenMP
-? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
-: Address::invalid();
-if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
-  DeclPtr = OpenMPLocalAddr;
+  // We first check if the parameter address is controlled by OpenMP.
+  if (getLangOpts().OpenMP) {
+DeclPtr = CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D);
+if (DeclPtr.isValid()) {
   AllocaPtr = DeclPtr;
-} else {
-  // Otherwise, create a temporary to hold the value.
-  DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(&D),
-  D.getName() + ".addr", &AllocaPtr);
+  DoCopy = true;
 }
-DoStore = true;
   }
 
-  llvm::Value *ArgVal = (DoStore ? Arg.getDirectValue() : nullptr);
+  // The parameter is not controlled by OpenMP.
+  if (!DeclPtr.isValid()) {
+// If we already have a pointer to the argum

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 441908.
tianshilei1992 added a comment.

rebase and fix test error


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129008/new/

https://reviews.llvm.org/D129008

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/OpenMP/globalization_byval_struct.c

Index: clang/test/OpenMP/globalization_byval_struct.c
===
--- /dev/null
+++ clang/test/OpenMP/globalization_byval_struct.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+extern int printf(const char *, ...);
+
+struct S {
+  int a;
+  float b;
+};
+
+// CHECK: define{{.*}}void @test(%struct.S* noundef byval(%struct.S) align {{[0-9]+}} [[arg:%[0-9a-zA-Z]+]])
+// CHECK: [[g:%[0-9a-zA-Z]+]] = call align {{[0-9]+}} i8* @__kmpc_alloc_shared
+// CHECK: bitcast i8* [[g]] to %struct.S*
+// CHECK: bitcast %struct.S* [[arg]] to i8**
+// CHECK: call void [[cc:@__copy_constructor[_0-9a-zA-Z]+]]
+// CHECK: void [[cc]]
+void test(struct S s) {
+#pragma omp parallel for
+  for (int i = 0; i < s.a; ++i) {
+printf("%i : %i : %f\n", i, s.a, s.b);
+  }
+}
+
+void foo() {
+  #pragma omp target teams num_teams(1)
+  {
+struct S s;
+s.a = 7;
+s.b = 11;
+test(s);
+  }
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2468,33 +2468,7 @@
 (IPD->getParameterKind() == ImplicitParamDecl::ThreadPrivateVar);
   }
 
-  Address DeclPtr = Address::invalid();
-  Address AllocaPtr = Address::invalid();
-  bool DoStore = false;
-  bool IsScalar = hasScalarEvaluationKind(Ty);
-  // If we already have a pointer to the argument, reuse the input pointer.
-  if (Arg.isIndirect()) {
-// If we have a prettier pointer type at this point, bitcast to that.
-DeclPtr = Arg.getIndirectAddress();
-DeclPtr = Builder.CreateElementBitCast(DeclPtr, ConvertTypeForMem(Ty),
-   D.getName());
-// Indirect argument is in alloca address space, which may be different
-// from the default address space.
-auto AllocaAS = CGM.getASTAllocaAddressSpace();
-auto *V = DeclPtr.getPointer();
-AllocaPtr = DeclPtr;
-auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
-auto DestLangAS =
-getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
-if (SrcLangAS != DestLangAS) {
-  assert(getContext().getTargetAddressSpace(SrcLangAS) ==
- CGM.getDataLayout().getAllocaAddrSpace());
-  auto DestAS = getContext().getTargetAddressSpace(DestLangAS);
-  auto *T = DeclPtr.getElementType()->getPointerTo(DestAS);
-  DeclPtr = DeclPtr.withPointer(getTargetHooks().performAddrSpaceCast(
-  *this, V, SrcLangAS, DestLangAS, T, true));
-}
-
+  auto PushCleanupIfNeeded = [this, Ty, &D](Address DeclPtr) {
 // Push a destructor cleanup for this parameter if the ABI requires it.
 // Don't push a cleanup in a thunk for a method that will also emit a
 // cleanup.
@@ -2510,87 +2484,123 @@
 EHStack.stable_begin();
   }
 }
-  } else {
-// Check if the parameter address is controlled by OpenMP runtime.
-Address OpenMPLocalAddr =
-getLangOpts().OpenMP
-? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
-: Address::invalid();
-if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
-  DeclPtr = OpenMPLocalAddr;
-  AllocaPtr = DeclPtr;
+  };
+
+  Address DeclPtr = Address::invalid();
+  Address AllocaPtr = Address::invalid();
+  Address OpenMPLocalAddr =
+  getLangOpts().OpenMP
+  ? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
+  : Address::invalid();
+  if (OpenMPLocalAddr.isValid()) {
+DeclPtr = OpenMPLocalAddr;
+AllocaPtr = DeclPtr;
+LValue Dst = MakeAddrLValue(DeclPtr, Ty);
+if (Arg.isIndirect()) {
+  LValue Src = MakeAddrLValue(Arg.getIndirectAddress(), Ty);
+  callCStructCopyConstructor(Dst, Src);
+  PushCleanupIfNeeded(Arg.getIndirectAddress());
 } else {
-  // Otherwise, create a temporary to hold the value.
+  EmitStoreOfScalar(Arg.getDirectValue(), Dst, /* isInitialization */ true);
+}
+  } else {
+bool DoStore = false;
+bool IsScalar = hasScalarEvaluationKind(Ty);
+// If we already have a pointer to the argument, reuse the input pointer.
+if (Arg.isIndirect()) {
+  // If we have a prettier pointer type at th

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 442585.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127042/new/

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 442588.
tianshilei1992 added a comment.

remove unrelated changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127042/new/

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129211: [Clang][Doc] Update the release note for clang

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Add the support for `atomic compare` and `atomic compare capture` in the
release note of clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129211

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -511,6 +511,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 
@@ -576,7 +578,7 @@
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-  
+
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -511,6 +511,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 
@@ -576,7 +578,7 @@
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-  
+
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG83837a61981c: [Clang][OpenMP] Enable floating-point 
operation for `atomic compare` series (authored by tianshilei1992).

Changed prior to commit:
  https://reviews.llvm.org/D127042?vs=442588&id=442618#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127042/new/

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 planned changes to this revision.
tianshilei1992 added a comment.

`callCStructCopyConstructor` is actually for Objective-C…Cannot use it here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129008/new/

https://reviews.llvm.org/D129008

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129211: [Clang][Doc] Update the release note for clang

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG114df244ec50: [Clang][Doc] Update the release note for clang 
(authored by tianshilei1992).

Changed prior to commit:
  https://reviews.llvm.org/D129211?vs=442593&id=447800#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129211/new/

https://reviews.llvm.org/D129211

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -592,6 +592,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -592,6 +592,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: pengfei, guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Currently if an OpenMP program uses `linear` clause, and is compiled with
optimization, `llvm.lifetime.end` for variables listed in `linear` clause are
emitted too early such that there could still be uses after that. Let's take the
following code as example:

  c
  // loop.c
  int j;
  int *u;
  
  void loop(int n) {
int i;
for (i = 0; i < n; ++i) {
  ++j;
  u = &j;
}
  }

We compile using the command:

  shell
  clang -cc1 -fopenmp-simd -O3 -x c -triple x86_64-apple-darwin10 -emit-llvm 
loop.c -o loop.ll

The following IR (simplified) will be generated:

  llvm
  @j = local_unnamed_addr global i32 0, align 4
  @u = local_unnamed_addr global ptr null, align 8
  
  define void @loop(i32 noundef %n) local_unnamed_addr {
  entry:
%j = alloca i32, align 4
%cmp = icmp sgt i32 %n, 0
br i1 %cmp, label %simd.if.then, label %simd.if.end
  
  simd.if.then: ; preds = %entry
call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %j)
store ptr %j, ptr @u, align 8
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
%0 = load i32, ptr %j, align 4
store i32 %0, ptr @j, align 4
br label %simd.if.end
  
  simd.if.end:  ; preds = %simd.if.then, 
%entry
ret void
  }

The most important part is:

  llvm
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
%0 = load i32, ptr %j, align 4
store i32 %0, ptr @j, align 4

`%j` is still loaded after `@llvm.lifetime.end.p0(i64 4, ptr nonnull %j)`. This
could cause the backend incorrectly optimizes the code and further generates
incorrect code. The root cause is, when we emit a construct that could have
`linear` clause, it usually has the following pattern:

  cpp
  EmitOMPLinearClauseInit(S)
  {
OMPPrivateScope LoopScope(*this);
...
EmitOMPLinearClause(S, LoopScope);
...
(void)LoopScope.Privatize();
...
  }
  EmitOMPLinearClauseFinal(S, [](CodeGenFunction &) { return nullptr; });

Variables that need to be privatized are added into `LoopScope`, which also
serves as a RAII object. When `LoopScope` is destructed and if optimization is
enabled, a `@llvm.lifetime.end` is also emitted for each privatized variable.
However, the writing back to original variables in `linear` clause happens after
the scope in `EmitOMPLinearClauseFinal`, causing the issue we see above.

A quick "fix" seems to be, moving `EmitOMPLinearClauseFinal` inside the scope.
However, it doesn't work. That's because the local variable map has been updated
by `LoopScope` such that a variable declaration is mapped to the privatized
variable, instead of the actual one. In that way, the following code will be
generated:

  llvm
%0 = load i32, ptr %j, align 4
store i32 %0, ptr %j, align 4
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)

Well, now the life time is correct, but apparently the writing back is broken.

In this patch, a new function `OMPPrivateScope::restoreMap` is added and called
before calling `EmitOMPLinearClauseFinal`. This can make sure that
`EmitOMPLinearClauseFinal` can find the orignal varaibls to write back.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131272

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/bug56913.c
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp

Index: clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
===
--- clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
+++ clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
@@ -1847,8 +1847,12 @@
 // CHECK6-NEXT:[[TMP38:%.*]] = icmp ne i32 [[TMP37]], 0
 // CHECK6-NEXT:br i1 [[TMP38]], label [[DOTOMP_LINEAR_PU_I:%.*]], label [[DOTOMP_OUTLINED__1_EXIT:%.*]]
 // CHECK6:   .omp.linear.pu.i:
-// CHECK6-NEXT:[[TMP39:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
-// CHECK6-NEXT:store i32 [[TMP39]], i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:[[TMP39:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 0
+// CHECK6-NEXT:[[TMP40:%.*]] = load i32*, i32** [[TMP39]], align 8
+// CHECK6-NEXT:[[TMP41:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 1
+// CHECK6-NEXT:[[TMP42:%.*]] = load i32*, i32** [[TMP41]], align 8
+// CHECK6-NEXT:[[TMP43:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:store i32 [[TMP43]], i32* [[TMP42]], align 4
 // CHECK6-NEXT:br label [

[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:1070
 void operator=(const OMPPrivateScope &) = delete;
+bool MapRestored = false;
 

ABataev wrote:
> Better to make it a property of MappedVars. Also, are there any issues if 
> just restore it several times without checking?
> Better to make it a property of MappedVars.
Will do.
> Also, are there any issues if just restore it several times without checking?
TBH, I don't know clearly, but I feel it should not be a problem. We will not 
have two scopes at the same time, will we? But I will add an assertion that if 
the mapped variables have been restored, `addPrivate` should not be called 
anymore.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131272/new/

https://reviews.llvm.org/D131272

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 450469.
tianshilei1992 added a comment.

rebase and fix comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131272/new/

https://reviews.llvm.org/D131272

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/bug56913.c
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp

Index: clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
===
--- clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
+++ clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
@@ -1847,8 +1847,12 @@
 // CHECK6-NEXT:[[TMP38:%.*]] = icmp ne i32 [[TMP37]], 0
 // CHECK6-NEXT:br i1 [[TMP38]], label [[DOTOMP_LINEAR_PU_I:%.*]], label [[DOTOMP_OUTLINED__1_EXIT:%.*]]
 // CHECK6:   .omp.linear.pu.i:
-// CHECK6-NEXT:[[TMP39:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
-// CHECK6-NEXT:store i32 [[TMP39]], i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:[[TMP39:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 0
+// CHECK6-NEXT:[[TMP40:%.*]] = load i32*, i32** [[TMP39]], align 8
+// CHECK6-NEXT:[[TMP41:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 1
+// CHECK6-NEXT:[[TMP42:%.*]] = load i32*, i32** [[TMP41]], align 8
+// CHECK6-NEXT:[[TMP43:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:store i32 [[TMP43]], i32* [[TMP42]], align 4
 // CHECK6-NEXT:br label [[DOTOMP_OUTLINED__1_EXIT]]
 // CHECK6:   .omp_outlined..1.exit:
 // CHECK6-NEXT:ret i32 0
Index: clang/test/OpenMP/for_linear_codegen.cpp
===
--- clang/test/OpenMP/for_linear_codegen.cpp
+++ clang/test/OpenMP/for_linear_codegen.cpp
@@ -322,18 +322,18 @@
 // CHECK1-NEXT:br label [[OMP_LOOP_EXIT:%.*]]
 // CHECK1:   omp.loop.exit:
 // CHECK1-NEXT:call void @__kmpc_for_static_fini(%struct.ident_t* @[[GLOB2]], i32 [[TMP5]])
-// CHECK1-NEXT:[[TMP19:%.*]] = bitcast i64* [[DOTLVAR__ADDR]] to i8*
-// CHECK1-NEXT:call void @__kmpc_free(i32 [[TMP5]], i8* [[TMP19]], i8* inttoptr (i64 5 to i8*))
-// CHECK1-NEXT:[[TMP20:%.*]] = load i32, i32* [[DOTOMP_IS_LAST]], align 4
-// CHECK1-NEXT:[[TMP21:%.*]] = icmp ne i32 [[TMP20]], 0
-// CHECK1-NEXT:br i1 [[TMP21]], label [[DOTOMP_LINEAR_PU:%.*]], label [[DOTOMP_LINEAR_PU_DONE:%.*]]
+// CHECK1-NEXT:[[TMP19:%.*]] = load i32, i32* [[DOTOMP_IS_LAST]], align 4
+// CHECK1-NEXT:[[TMP20:%.*]] = icmp ne i32 [[TMP19]], 0
+// CHECK1-NEXT:br i1 [[TMP20]], label [[DOTOMP_LINEAR_PU:%.*]], label [[DOTOMP_LINEAR_PU_DONE:%.*]]
 // CHECK1:   .omp.linear.pu:
-// CHECK1-NEXT:[[TMP22:%.*]] = load float*, float** [[PVAR2]], align 8
-// CHECK1-NEXT:store float* [[TMP22]], float** [[TMP0]], align 8
-// CHECK1-NEXT:[[TMP23:%.*]] = load i64, i64* [[DOTLVAR__ADDR]], align 8
-// CHECK1-NEXT:store i64 [[TMP23]], i64* [[TMP1]], align 8
+// CHECK1-NEXT:[[TMP21:%.*]] = load float*, float** [[PVAR2]], align 8
+// CHECK1-NEXT:store float* [[TMP21]], float** [[TMP0]], align 8
+// CHECK1-NEXT:[[TMP22:%.*]] = load i64, i64* [[DOTLVAR__ADDR]], align 8
+// CHECK1-NEXT:store i64 [[TMP22]], i64* [[TMP1]], align 8
 // CHECK1-NEXT:br label [[DOTOMP_LINEAR_PU_DONE]]
 // CHECK1:   .omp.linear.pu.done:
+// CHECK1-NEXT:[[TMP23:%.*]] = bitcast i64* [[DOTLVAR__ADDR]] to i8*
+// CHECK1-NEXT:call void @__kmpc_free(i32 [[TMP5]], i8* [[TMP23]], i8* inttoptr (i64 5 to i8*))
 // CHECK1-NEXT:call void @__kmpc_barrier(%struct.ident_t* @[[GLOB1]], i32 [[TMP5]])
 // CHECK1-NEXT:ret void
 //
Index: clang/test/OpenMP/bug56913.c
===
--- /dev/null
+++ clang/test/OpenMP/bug56913.c
@@ -0,0 +1,32 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -fopenmp-simd -O1 -x c -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK
+
+int j;
+int *u;
+
+void loop(int n) {
+  int i;
+#pragma omp parallel master taskloop simd linear(j)
+  for (i = 0; i < n; ++i) {
+++j;
+u = &j;
+  }
+}
+// CHECK-LABEL: define {{[^@]+}}@loop
+// CHECK-SAME: (i32 noundef [[N:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[CMP:%.*]] = icmp sgt i32 [[N]], 0
+// CHECK-NEXT:br i1 [[CMP]], label [[SIMD_IF_THEN:%.*]], label [[SIMD_IF_END:%.*]]
+// CHECK:   simd.if.then:
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr @j, align 4, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT:call void @

[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:1070
 void operator=(const OMPPrivateScope &) = delete;
+bool MapRestored = false;
 

tianshilei1992 wrote:
> ABataev wrote:
> > Better to make it a property of MappedVars. Also, are there any issues if 
> > just restore it several times without checking?
> > Better to make it a property of MappedVars.
> Will do.
> > Also, are there any issues if just restore it several times without 
> > checking?
> TBH, I don't know clearly, but I feel it should not be a problem. We will not 
> have two scopes at the same time, will we? But I will add an assertion that 
> if the mapped variables have been restored, `addPrivate` should not be called 
> anymore.
We don't have to worry about restore for multiple times as every time it is 
called, the container is cleared.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131272/new/

https://reviews.llvm.org/D131272

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe21202dac18e: [Clang][OpenMP] Fix the issue that 
`llvm.lifetime.end` is emitted too early for… (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131272/new/

https://reviews.llvm.org/D131272

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/bug56913.c
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp

Index: clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
===
--- clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
+++ clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
@@ -1847,8 +1847,12 @@
 // CHECK6-NEXT:[[TMP38:%.*]] = icmp ne i32 [[TMP37]], 0
 // CHECK6-NEXT:br i1 [[TMP38]], label [[DOTOMP_LINEAR_PU_I:%.*]], label [[DOTOMP_OUTLINED__1_EXIT:%.*]]
 // CHECK6:   .omp.linear.pu.i:
-// CHECK6-NEXT:[[TMP39:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
-// CHECK6-NEXT:store i32 [[TMP39]], i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:[[TMP39:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 0
+// CHECK6-NEXT:[[TMP40:%.*]] = load i32*, i32** [[TMP39]], align 8
+// CHECK6-NEXT:[[TMP41:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 1
+// CHECK6-NEXT:[[TMP42:%.*]] = load i32*, i32** [[TMP41]], align 8
+// CHECK6-NEXT:[[TMP43:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:store i32 [[TMP43]], i32* [[TMP42]], align 4
 // CHECK6-NEXT:br label [[DOTOMP_OUTLINED__1_EXIT]]
 // CHECK6:   .omp_outlined..1.exit:
 // CHECK6-NEXT:ret i32 0
Index: clang/test/OpenMP/for_linear_codegen.cpp
===
--- clang/test/OpenMP/for_linear_codegen.cpp
+++ clang/test/OpenMP/for_linear_codegen.cpp
@@ -322,18 +322,18 @@
 // CHECK1-NEXT:br label [[OMP_LOOP_EXIT:%.*]]
 // CHECK1:   omp.loop.exit:
 // CHECK1-NEXT:call void @__kmpc_for_static_fini(%struct.ident_t* @[[GLOB2]], i32 [[TMP5]])
-// CHECK1-NEXT:[[TMP19:%.*]] = bitcast i64* [[DOTLVAR__ADDR]] to i8*
-// CHECK1-NEXT:call void @__kmpc_free(i32 [[TMP5]], i8* [[TMP19]], i8* inttoptr (i64 5 to i8*))
-// CHECK1-NEXT:[[TMP20:%.*]] = load i32, i32* [[DOTOMP_IS_LAST]], align 4
-// CHECK1-NEXT:[[TMP21:%.*]] = icmp ne i32 [[TMP20]], 0
-// CHECK1-NEXT:br i1 [[TMP21]], label [[DOTOMP_LINEAR_PU:%.*]], label [[DOTOMP_LINEAR_PU_DONE:%.*]]
+// CHECK1-NEXT:[[TMP19:%.*]] = load i32, i32* [[DOTOMP_IS_LAST]], align 4
+// CHECK1-NEXT:[[TMP20:%.*]] = icmp ne i32 [[TMP19]], 0
+// CHECK1-NEXT:br i1 [[TMP20]], label [[DOTOMP_LINEAR_PU:%.*]], label [[DOTOMP_LINEAR_PU_DONE:%.*]]
 // CHECK1:   .omp.linear.pu:
-// CHECK1-NEXT:[[TMP22:%.*]] = load float*, float** [[PVAR2]], align 8
-// CHECK1-NEXT:store float* [[TMP22]], float** [[TMP0]], align 8
-// CHECK1-NEXT:[[TMP23:%.*]] = load i64, i64* [[DOTLVAR__ADDR]], align 8
-// CHECK1-NEXT:store i64 [[TMP23]], i64* [[TMP1]], align 8
+// CHECK1-NEXT:[[TMP21:%.*]] = load float*, float** [[PVAR2]], align 8
+// CHECK1-NEXT:store float* [[TMP21]], float** [[TMP0]], align 8
+// CHECK1-NEXT:[[TMP22:%.*]] = load i64, i64* [[DOTLVAR__ADDR]], align 8
+// CHECK1-NEXT:store i64 [[TMP22]], i64* [[TMP1]], align 8
 // CHECK1-NEXT:br label [[DOTOMP_LINEAR_PU_DONE]]
 // CHECK1:   .omp.linear.pu.done:
+// CHECK1-NEXT:[[TMP23:%.*]] = bitcast i64* [[DOTLVAR__ADDR]] to i8*
+// CHECK1-NEXT:call void @__kmpc_free(i32 [[TMP5]], i8* [[TMP23]], i8* inttoptr (i64 5 to i8*))
 // CHECK1-NEXT:call void @__kmpc_barrier(%struct.ident_t* @[[GLOB1]], i32 [[TMP5]])
 // CHECK1-NEXT:ret void
 //
Index: clang/test/OpenMP/bug56913.c
===
--- /dev/null
+++ clang/test/OpenMP/bug56913.c
@@ -0,0 +1,32 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -fopenmp-simd -O1 -x c -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK
+
+int j;
+int *u;
+
+void loop(int n) {
+  int i;
+#pragma omp parallel master taskloop simd linear(j)
+  for (i = 0; i < n; ++i) {
+++j;
+u = &j;
+  }
+}
+// CHECK-LABEL: define {{[^@]+}}@loop
+// CHECK-SAME: (i32 noundef [[N:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[CMP:%.*]] = icmp sgt i32 [[N]], 0
+// CHECK-NEXT:br i1 [[CMP]], label [[SIMD_IF_THEN:%.*]], label [[SIMD_IF_END:%.*]]
+// CHECK:   simd.if.then:
+// CHE

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

`bool` is used by a function declaration `__ockl_fdot2` in 
`clang/lib/Headers/__clang_hip_libdevice_declares.h`, which is included by 
`clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131639/new/

https://reviews.llvm.org/D131639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

This patch causes wrong arguments in nested parallel regions. 
`openmp/libomptarget/test/offloading/bug49779.cpp` fails and after reverting 
this patch, it passed. You can easily find out the wrong pointer with a tiny 
change of the test case:

  void work(int *C) {
printf("work: &C = %p\n", C);
  #pragma omp atomic
++(*C);
  }
  
  void use(int *C) {
printf("use: &C = %p\n", C);
  #pragma omp parallel num_threads(2)
work(C);
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102107/new/

https://reviews.llvm.org/D102107

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100768: [Clang][OpenMP] Remove the mandatory flush for capture for OpenMP 5.1

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 361387.
tianshilei1992 added a comment.

rebase and ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100768/new/

https://reviews.llvm.org/D100768

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/atomic_capture_codegen.cpp

Index: clang/test/OpenMP/atomic_capture_codegen.cpp
===
--- clang/test/OpenMP/atomic_capture_codegen.cpp
+++ clang/test/OpenMP/atomic_capture_codegen.cpp
@@ -1,11 +1,19 @@
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -x c -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-50 %s
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp-simd -x c -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp-simd -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
@@ -343,7 +351,7 @@
 // CHECK: [[IM_CAST:%.+]] = fptrunc double [[NEW_IM]] to float
 // CHECK: store float [[RE_CAST]], float* getelementptr inbounds ({ float, float }, { float, float }* @{{.+}}, i32 0, i32 0),
 // CHECK: store float [[IM_CAST]], float* getelementptr inbounds ({ float, float }, { float, float }* @{{.+}}, i32 0, i32 1),
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture seq_cst
   {cdx = cdx - cdv; cfv = cdx;}
 // CHECK: [[BV:%.+]] = load i8, i8* @{{.+}}
@@ -393,7 +401,7 @@
 // CHECK: br i1 [[SUCCESS_FAIL]], label %[[EXIT:.+]], label %[[CONT]]
 // CHECK: [[EXIT]]
 // CHECK: store i8 [[NEW]], i8* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture, seq_cst
   {cx = cx >> ucv; cv = cx;}
 // CHECK: [[SV:%.+]]  = load i16, i16* @{{.+}},
@@ -436,7 +444,7 @@
 // CHECK: [[OLD:%.+]] = atomicrmw or i32* @{{.+}}, i32 [[EXPR]] seq_cst, align 4
 // CHECK: [[DESIRED:%.+]] = or i32 [[EXPR]], [[OLD]]
 // CHECK: store i32 [[DESIRED]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic seq_cst, capture
   {uix = iv | uix; uiv = uix;}
 // CHECK: [[EXPR:%.+]] = load i32, i32* @{{.+}}
@@ -904,7 +912,7 @@
 // CHECK: [[EXIT]]
 // CHECK: [[NEW_VAL:%.+]] = trunc i64 [[CONV]] to i32
 // CHECK: store i32 [[NEW_VAL]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture release
   {bfx4.b /= ldv; iv = bfx4.b;}
 // CHECK: [[EXPR:%.+]] = load x86_fp80, x86_fp80* @{{.+}}
@@ -937,7 +945,7 @@
 // CHECK: [[EXIT]]
 // CHECK: [[NEW_VAL_I32:%.+]] = trunc i64 [[NEW_VAL]] to i32
 // CHECK: store i32 [[NEW_VAL_I32]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture acquire
   iv = bfx4_packed.b += ldv;
 // CHECK: load i64, i64*
@@ -963,7 +971,7 @@
 // CHECK: br i1 [[FAIL_SUCCESS]], label %[[EXIT:.+]], label %[[CONT]]
 // CHECK: [[EXIT]]
 // CHECK: store float [[X]], float* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture acq_rel
   {fv = float2x.x; float2x.x = ulv - float2x.x;}
 // CHECK: [[EXPR:%.+]] = load double, double* @{{.+}},
@@ -973,7 +981,7 @@
 // CHECK: [[NEW_VAL:%.+]] = fptosi double [[DIV]] to i32
 // CHECK: call void @llvm.write_registe

[PATCH] D100768: [Clang][OpenMP] Remove the mandatory flush for capture for OpenMP 5.1

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

@ABataev I updated the description. If you agree with my understanding, I'll 
land it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100768/new/

https://reviews.llvm.org/D100768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100768: [Clang][OpenMP] Remove the mandatory flush for capture for OpenMP 5.1

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3274cdc83ecd: [Clang][OpenMP] Remove the mandatory flush for 
capture for OpenMP 5.1 (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100768/new/

https://reviews.llvm.org/D100768

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/atomic_capture_codegen.cpp

Index: clang/test/OpenMP/atomic_capture_codegen.cpp
===
--- clang/test/OpenMP/atomic_capture_codegen.cpp
+++ clang/test/OpenMP/atomic_capture_codegen.cpp
@@ -1,11 +1,19 @@
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -x c -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-50 %s
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp-simd -x c -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp-simd -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
@@ -343,7 +351,7 @@
 // CHECK: [[IM_CAST:%.+]] = fptrunc double [[NEW_IM]] to float
 // CHECK: store float [[RE_CAST]], float* getelementptr inbounds ({ float, float }, { float, float }* @{{.+}}, i32 0, i32 0),
 // CHECK: store float [[IM_CAST]], float* getelementptr inbounds ({ float, float }, { float, float }* @{{.+}}, i32 0, i32 1),
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture seq_cst
   {cdx = cdx - cdv; cfv = cdx;}
 // CHECK: [[BV:%.+]] = load i8, i8* @{{.+}}
@@ -393,7 +401,7 @@
 // CHECK: br i1 [[SUCCESS_FAIL]], label %[[EXIT:.+]], label %[[CONT]]
 // CHECK: [[EXIT]]
 // CHECK: store i8 [[NEW]], i8* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture, seq_cst
   {cx = cx >> ucv; cv = cx;}
 // CHECK: [[SV:%.+]]  = load i16, i16* @{{.+}},
@@ -436,7 +444,7 @@
 // CHECK: [[OLD:%.+]] = atomicrmw or i32* @{{.+}}, i32 [[EXPR]] seq_cst, align 4
 // CHECK: [[DESIRED:%.+]] = or i32 [[EXPR]], [[OLD]]
 // CHECK: store i32 [[DESIRED]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic seq_cst, capture
   {uix = iv | uix; uiv = uix;}
 // CHECK: [[EXPR:%.+]] = load i32, i32* @{{.+}}
@@ -904,7 +912,7 @@
 // CHECK: [[EXIT]]
 // CHECK: [[NEW_VAL:%.+]] = trunc i64 [[CONV]] to i32
 // CHECK: store i32 [[NEW_VAL]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture release
   {bfx4.b /= ldv; iv = bfx4.b;}
 // CHECK: [[EXPR:%.+]] = load x86_fp80, x86_fp80* @{{.+}}
@@ -937,7 +945,7 @@
 // CHECK: [[EXIT]]
 // CHECK: [[NEW_VAL_I32:%.+]] = trunc i64 [[NEW_VAL]] to i32
 // CHECK: store i32 [[NEW_VAL_I32]], i32* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture acquire
   iv = bfx4_packed.b += ldv;
 // CHECK: load i64, i64*
@@ -963,7 +971,7 @@
 // CHECK: br i1 [[FAIL_SUCCESS]], label %[[EXIT:.+]], label %[[CONT]]
 // CHECK: [[EXIT]]
 // CHECK: store float [[X]], float* @{{.+}},
-// CHECK: call{{.*}} @__kmpc_flush(
+// CHECK-50: call{{.*}} @__kmpc_flush(
 #pragma omp atomic capture acq_rel
   {fv = float2x.x; float2x.x = ulv - float2x.x;}
 // CHECK: [[

[PATCH] D100582: [WIP][OpenMP][Offloading] New device runtime library - Not for review

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 abandoned this revision.
tianshilei1992 added a comment.
Herald added a subscriber: ormris.

We're moving to D106803 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100582/new/

https://reviews.llvm.org/D100582

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106298: [OpenMP] Creating the `omp_target_num_teams` and `omp_target_thread_limit` attributes to outlined functions

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0276db14167b: [OpenMP] Creating the `omp_target_num_teams` 
and `omp_target_thread_limit`… (authored by Jose M Monsalve Diaz 
, committed by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106298/new/

https://reviews.llvm.org/D106298

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_map_codegen_03.cpp
  clang/test/OpenMP/target_num_teams_num_threads_attributes.cpp
  clang/test/OpenMP/target_parallel_codegen.cpp
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_parallel_if_codegen.cpp
  clang/test/OpenMP/target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_collapse_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_dist_schedule_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_private_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/target_teams_num_teams_codegen.cpp
  clang/test/OpenMP/target_teams_thread_limit_codegen.cpp
  clang/test/OpenMP/teams_codegen.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106033: [OpenMP] Folding threadLimit and numThreads when single value in kernels

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ab6aedda9d9: [OpenMP] Folding threadLimit and numThreads 
when single value in kernels (authored by Jose M Monsalve Diaz 
, committed by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106033/new/

https://reviews.llvm.org/D106033

Files:
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
  openmp/libomptarget/deviceRTLs/target_interface.h

Index: openmp/libomptarget/deviceRTLs/target_interface.h
===
--- openmp/libomptarget/deviceRTLs/target_interface.h
+++ openmp/libomptarget/deviceRTLs/target_interface.h
@@ -18,8 +18,8 @@
 // Calls to the NVPTX layer (assuming 1D layout)
 EXTERN int __kmpc_get_hardware_thread_id_in_block();
 EXTERN int GetBlockIdInKernel();
-EXTERN int __kmpc_get_hardware_num_blocks();
-EXTERN int __kmpc_get_hardware_num_threads_in_block();
+EXTERN NOINLINE int __kmpc_get_hardware_num_blocks();
+EXTERN NOINLINE int __kmpc_get_hardware_num_threads_in_block();
 EXTERN unsigned GetWarpId();
 EXTERN unsigned GetWarpSize();
 EXTERN unsigned GetLaneId();
Index: llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
===
--- /dev/null
+++ llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
@@ -0,0 +1,128 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-globals
+; RUN: opt -S -passes=openmp-opt < %s | FileCheck %s
+target triple = "nvptx64"
+
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+@kernel0_exec_mode = weak constant i8 1
+
+@G = external global i32
+;.
+; CHECK: @[[G:[a-zA-Z0-9_$"\\.-]+]] = external global i32
+;.
+define weak void @kernel0() #0 {
+; CHECK-LABEL: define {{[^@]+}}@kernel0()
+; CHECK: #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:[[I:%.*]] = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 true, i1 false, i1 false)
+; CHECK-NEXT:call void @helper0()
+; CHECK-NEXT:call void @helper1()
+; CHECK-NEXT:call void @helper2()
+; CHECK-NEXT:call void @__kmpc_target_deinit(%struct.ident_t* null, i1 true, i1 false)
+; CHECK-NEXT:ret void
+;
+  %i = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 true, i1 false, i1 false)
+  call void @helper0()
+  call void @helper1()
+  call void @helper2()
+  call void @__kmpc_target_deinit(%struct.ident_t* null, i1 true, i1 false)
+  ret void
+}
+
+@kernel1_exec_mode = weak constant i8 1
+
+define weak void @kernel1() #0 {
+; CHECK-LABEL: define {{[^@]+}}@kernel1()
+; CHECK: #[[ATTR0]] {
+; CHECK-NEXT:[[I:%.*]] = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 true, i1 false, i1 false)
+; CHECK-NEXT:call void @helper1()
+; CHECK-NEXT:call void @__kmpc_target_deinit(%struct.ident_t* null, i1 false, i1 false)
+; CHECK-NEXT:ret void
+;
+  %i = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 true, i1 false, i1 false)
+  call void @helper1()
+  call void @__kmpc_target_deinit(%struct.ident_t* null, i1 false, i1 false)
+  ret void
+}
+
+@kernel2_exec_mode = weak constant i8 1
+
+define weak void @kernel2() #0 {
+; CHECK-LABEL: define {{[^@]+}}@kernel2()
+; CHECK: #[[ATTR0]] {
+; CHECK-NEXT:[[I:%.*]] = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 false, i1 false, i1 false)
+; CHECK-NEXT:call void @helper0()
+; CHECK-NEXT:call void @helper1()
+; CHECK-NEXT:call void @helper2()
+; CHECK-NEXT:call void @__kmpc_target_deinit(%struct.ident_t* null, i1 false, i1 false)
+; CHECK-NEXT:ret void
+;
+  %i = call i32 @__kmpc_target_init(%struct.ident_t* null, i1 false, i1 false, i1 false)
+  call void @helper0()
+  call void @helper1()
+  call void @helper2()
+  call void @__kmpc_target_deinit(%struct.ident_t* null, i1 false, i1 false)
+  ret void
+}
+
+define internal void @helper0() {
+; CHECK-LABEL: define {{[^@]+}}@helper0() {{#[0-9]+}} {
+; CHECK-NEXT:store i32 666, i32* @G, align 4
+; CHECK-NEXT:ret void
+;
+  %threadLimit = call i32 @__kmpc_get_hardware_num_threads_in_block()
+  store i32 %threadLimit, i32* @G
+  ret void
+}
+
+define internal void @helper1() {
+; CHECK-LABEL: define {{[^@]+}}@helper1() {{#[0-9]+}} {
+; CHECK-NEXT:br label [[F:%.*]]
+; CHECK:   t:
+; CHECK-NEXT:unreachable
+; CHECK:   f:
+; CHECK-NEXT:ret void
+;
+  %threadLimit = call i32 @__kmpc_get_hardware_num_threads_in_block()
+  %c = icmp eq i32 %threadLimit, 666
+  br i1 %c, label %f, label %t
+t:
+  call void @helper0()
+  ret void
+f:
+  ret void
+}
+
+define internal void @helper2() {
+; CHECK-LABEL: define {{[^@]+}}@helper2() {{#[0-9]+}} {
+; CHECK-NEXT:store i32 666, i32* @G
+; CHECK-NEXT:  

[PATCH] D106674: Runtime for Interop directive

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/src/interop.cpp:13
+static omp_interop_rc_t
+__kmpc_interop_get_property_err_type(omp_interop_property_t property) {
+  switch (property) {

tianshilei1992 wrote:
> There are some conventions in current `libomptarget`.
> 1. If a function is internal, use LLVM code style.
> 2. If a function is exported and exposed to compiler, it should be `extern 
> "C"` and use code style similar to existing functions whose name prefix with 
> `__tgt`.
> 
> So basically, if these functions are only visible to this file, please format 
> it with LLVM code style, and use anonymous name space.
I mean, this function doesn't have to start with `__tgt` because it is 
internal. Functions starting with `__tgt` are usually interface functions. From 
my perspective, it is better to name it as `getPropertyErrorType`, a.k.a. to 
use LLVM code style.



Comment at: openmp/libomptarget/src/interop.cpp:63
+template 
+PropertyTy __tgt_interop_get_property(omp_interop_val_t &interop_val,
+  omp_interop_property_t property,

Same here



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1501
+return (*fptr)(interop);
+  } else { // liboffload & libomptarget don't exist
+return 0;

ditto



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1515
+return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+return 0;

ditto



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1527
+return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+return (void *)0;

ditto



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1539
+return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+return (const char *)0;

ditto



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1550
+return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+return (const char *)0;

same as below



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1561
+return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+return (const char *)0;

same as below



Comment at: openmp/runtime/src/kmp_ftn_entry.h:1570-1574
+  if ((*(void **)(&fptr) = KMP_DLSYM_NEXT("omp_get_interop_rec_desc"))) {
+return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+return (const char *)0;
+  }




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106674/new/

https://reviews.llvm.org/D106674

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106674: Runtime for Interop directive

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:1150-1156
+  CUresult Err = cuDeviceGet(&Dev, DeviceId);
+  if (Err == CUDA_SUCCESS) {
+DeviceInfo->Device = reinterpret_cast(Dev);
+  } else {
+cuGetErrorString(Err, errStr);
+return OFFLOAD_FAIL;
+  }





Comment at: openmp/libomptarget/src/interop.cpp:43
+const char *getVendorIdToStr(intptr_t vendor_id) {
+  switch (vendor_id) {
+  case 1:

Does it make sense to use enum here?



Comment at: openmp/libomptarget/src/interop.cpp:13
+static omp_interop_rc_t
+__kmpc_interop_get_property_err_type(omp_interop_property_t property) {
+  switch (property) {

tianshilei1992 wrote:
> tianshilei1992 wrote:
> > There are some conventions in current `libomptarget`.
> > 1. If a function is internal, use LLVM code style.
> > 2. If a function is exported and exposed to compiler, it should be `extern 
> > "C"` and use code style similar to existing functions whose name prefix 
> > with `__tgt`.
> > 
> > So basically, if these functions are only visible to this file, please 
> > format it with LLVM code style, and use anonymous name space.
> I mean, this function doesn't have to start with `__tgt` because it is 
> internal. Functions starting with `__tgt` are usually interface functions. 
> From my perspective, it is better to name it as `getPropertyErrorType`, 
> a.k.a. to use LLVM code style.
Looks better. Can you check https://llvm.org/docs/CodingStandards.html for the 
code style?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106674/new/

https://reviews.llvm.org/D106674

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121466: [OpenMP] Replace math headers with OpenMP wrapper calls

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

LGTM. Support the header file is actually generated automatically. :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121466/new/

https://reviews.llvm.org/D121466

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122133: [LTO] Add configuartion option to use default optimization pipeline

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

I tested it locally, and it fixes the performance regression. Since it is 
internal flag, it will not break any existing code. I'm okay with the change. 
Not sure if we want to ask the gate keeper of `LTOBackend`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122133/new/

https://reviews.llvm.org/D122133

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123460: [OpenMP] Make generating offloading entries more generic

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

If we'd like it to be used by CUDA or other programming models, 
`llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp` is not a good place. Other than 
that, the patch looks good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123460/new/

https://reviews.llvm.org/D123460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123235: atomic compare fail : Parser & AST support

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D123235#3444644 , @koops wrote:

> Can I please have the detailed log for the build failures? I do not have the 
> necessary setup to test those builds.

You can find it 
https://buildkite.com/llvm-project/premerge-checks/builds/87307#7ab569e5-cc98-48c0-b88d-1cab7d95dd47.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123235/new/

https://reviews.llvm.org/D123235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123862: [Clang][OpenMP] Use bitfields for flags in `OMPAtomicDirective`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: ABataev, jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123862

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp


Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2449,8 +2449,8 @@
 void ASTStmtReader::VisitOMPAtomicDirective(OMPAtomicDirective *D) {
   VisitStmt(D);
   VisitOMPExecutableDirective(D);
-  D->IsXLHSInRHSPart = Record.readBool();
-  D->IsPostfixUpdate = Record.readBool();
+  D->Flags.IsXLHSInRHSPart = Record.readBool() ? 1 : 0;
+  D->Flags.IsPostfixUpdate = Record.readBool() ? 1 : 0;
 }
 
 void ASTStmtReader::VisitOMPTargetDirective(OMPTargetDirective *D) {
Index: clang/lib/AST/StmtOpenMP.cpp
===
--- clang/lib/AST/StmtOpenMP.cpp
+++ clang/lib/AST/StmtOpenMP.cpp
@@ -875,8 +875,8 @@
   Dir->setUpdateExpr(Exprs.UE);
   Dir->setD(Exprs.D);
   Dir->setCond(Exprs.Cond);
-  Dir->IsXLHSInRHSPart = Exprs.IsXLHSInRHSPart;
-  Dir->IsPostfixUpdate = Exprs.IsPostfixUpdate;
+  Dir->Flags.IsXLHSInRHSPart = Exprs.IsXLHSInRHSPart ? 1 : 0;
+  Dir->Flags.IsPostfixUpdate = Exprs.IsPostfixUpdate ? 1 : 0;
   return Dir;
 }
 
Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -2827,25 +2827,28 @@
 class OMPAtomicDirective : public OMPExecutableDirective {
   friend class ASTStmtReader;
   friend class OMPExecutableDirective;
-  /// Used for 'atomic update' or 'atomic capture' constructs. They may
-  /// have atomic expressions of forms
-  /// \code
-  /// x = x binop expr;
-  /// x = expr binop x;
-  /// \endcode
-  /// This field is true for the first form of the expression and false for the
-  /// second. Required for correct codegen of non-associative operations (like
-  /// << or >>).
-  bool IsXLHSInRHSPart = false;
-  /// Used for 'atomic update' or 'atomic capture' constructs. They may
-  /// have atomic expressions of forms
-  /// \code
-  /// v = x; ;
-  /// ; v = x;
-  /// \endcode
-  /// This field is true for the first(postfix) form of the expression and 
false
-  /// otherwise.
-  bool IsPostfixUpdate = false;
+
+  struct FlagTy {
+/// Used for 'atomic update' or 'atomic capture' constructs. They may
+/// have atomic expressions of forms:
+/// \code
+/// x = x binop expr;
+/// x = expr binop x;
+/// \endcode
+/// This field is 1 for the first form of the expression and 0 for the
+/// second. Required for correct codegen of non-associative operations 
(like
+/// << or >>).
+uint8_t IsXLHSInRHSPart : 1;
+/// Used for 'atomic update' or 'atomic capture' constructs. They may
+/// have atomic expressions of forms:
+/// \code
+/// v = x; ;
+/// ; v = x;
+/// \endcode
+/// This field is 1 for the first(postfix) form of the expression and 0
+/// otherwise.
+uint8_t IsPostfixUpdate : 1;
+  } Flags;
 
   /// Build directive with the given start and end location.
   ///
@@ -2956,10 +2959,10 @@
   /// Return true if helper update expression has form
   /// 'OpaqueValueExpr(x) binop OpaqueValueExpr(expr)' and false if it has form
   /// 'OpaqueValueExpr(expr) binop OpaqueValueExpr(x)'.
-  bool isXLHSInRHSPart() const { return IsXLHSInRHSPart; }
+  bool isXLHSInRHSPart() const { return Flags.IsXLHSInRHSPart; }
   /// Return true if 'v' expression must be updated to original value of
   /// 'x', false if 'v' must be updated to the new value of 'x'.
-  bool isPostfixUpdate() const { return IsPostfixUpdate; }
+  bool isPostfixUpdate() const { return Flags.IsPostfixUpdate; }
   /// Get 'v' part of the associated expression/statement.
   Expr *getV() {
 return cast_or_null(Data->getChildren()[DataPositionTy::POS_V]);


Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2449,8 +2449,8 @@
 void ASTStmtReader::VisitOMPAtomicDirective(OMPAtomicDirective *D) {
   VisitStmt(D);
   VisitOMPExecutableDirective(D);
-  D->IsXLHSInRHSPart = Record.readBool();
-  D->IsPostfixUpdate = Record.readBool();
+  D->Flags.IsXLHSInRHSPart = Record.readBool() ? 1 : 0;
+  D->Flags.IsPostfixUpdate = Record.readBool() ? 1 : 0;
 }
 
 void ASTStmtReader::VisitOMPTargetDirective(OMPTargetDirective *D) {
Index: clang/lib/AST/StmtOpenMP.cpp
=

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 423132.
tianshilei1992 added a comment.
Herald added a project: All.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120290/new/

https://reviews.llvm.org/D120290

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/include/clang/AST/StmtOpenMP.h:2835-2847
   bool IsXLHSInRHSPart = false;
   /// Used for 'atomic update' or 'atomic capture' constructs. They may
   /// have atomic expressions of forms
   /// \code
   /// v = x; ;
   /// ; v = x;
   /// \endcode

tianshilei1992 wrote:
> ABataev wrote:
> > Transform these booleans to bitfields?
> Will do in another patch.
In D123862.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6216
-  KindsEncountered.contains(OMPC_capture)) {
-IsCompareCapture = true;
 Kind = OMPC_compare;

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > Can this be fixed in a separate patch?
> > Well, I think it's part of this patch because we can't tell if it's compare 
> > or compare capture before, but now we can. If we really want to do that, we 
> > can have another patch including all changes in this patch related to 
> > `OMPAtomicDirective`.
> Kind of NFC? Would be good
I think we don't need an extra NFC patch for this part because it's not gonna 
be very neat w/o some other code in this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120290/new/

https://reviews.llvm.org/D120290

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123862: [Clang][OpenMP] Use bitfields for flags in `OMPAtomicDirective`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D123862#3454380 , @ABataev wrote:

> What about ASTStmtWriter?

It's using accessor function `isXLHSInRHSPart()` and so on.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123862/new/

https://reviews.llvm.org/D123862

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123862: [Clang][OpenMP] Use bitfields for flags in `OMPAtomicDirective`

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8760b51ee0f: [Clang][OpenMP] Use bitfields for flags in 
`OMPAtomicDirective` (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123862/new/

https://reviews.llvm.org/D123862

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp


Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2449,8 +2449,8 @@
 void ASTStmtReader::VisitOMPAtomicDirective(OMPAtomicDirective *D) {
   VisitStmt(D);
   VisitOMPExecutableDirective(D);
-  D->IsXLHSInRHSPart = Record.readBool();
-  D->IsPostfixUpdate = Record.readBool();
+  D->Flags.IsXLHSInRHSPart = Record.readBool() ? 1 : 0;
+  D->Flags.IsPostfixUpdate = Record.readBool() ? 1 : 0;
 }
 
 void ASTStmtReader::VisitOMPTargetDirective(OMPTargetDirective *D) {
Index: clang/lib/AST/StmtOpenMP.cpp
===
--- clang/lib/AST/StmtOpenMP.cpp
+++ clang/lib/AST/StmtOpenMP.cpp
@@ -875,8 +875,8 @@
   Dir->setUpdateExpr(Exprs.UE);
   Dir->setD(Exprs.D);
   Dir->setCond(Exprs.Cond);
-  Dir->IsXLHSInRHSPart = Exprs.IsXLHSInRHSPart;
-  Dir->IsPostfixUpdate = Exprs.IsPostfixUpdate;
+  Dir->Flags.IsXLHSInRHSPart = Exprs.IsXLHSInRHSPart ? 1 : 0;
+  Dir->Flags.IsPostfixUpdate = Exprs.IsPostfixUpdate ? 1 : 0;
   return Dir;
 }
 
Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -2827,25 +2827,28 @@
 class OMPAtomicDirective : public OMPExecutableDirective {
   friend class ASTStmtReader;
   friend class OMPExecutableDirective;
-  /// Used for 'atomic update' or 'atomic capture' constructs. They may
-  /// have atomic expressions of forms
-  /// \code
-  /// x = x binop expr;
-  /// x = expr binop x;
-  /// \endcode
-  /// This field is true for the first form of the expression and false for the
-  /// second. Required for correct codegen of non-associative operations (like
-  /// << or >>).
-  bool IsXLHSInRHSPart = false;
-  /// Used for 'atomic update' or 'atomic capture' constructs. They may
-  /// have atomic expressions of forms
-  /// \code
-  /// v = x; ;
-  /// ; v = x;
-  /// \endcode
-  /// This field is true for the first(postfix) form of the expression and 
false
-  /// otherwise.
-  bool IsPostfixUpdate = false;
+
+  struct FlagTy {
+/// Used for 'atomic update' or 'atomic capture' constructs. They may
+/// have atomic expressions of forms:
+/// \code
+/// x = x binop expr;
+/// x = expr binop x;
+/// \endcode
+/// This field is 1 for the first form of the expression and 0 for the
+/// second. Required for correct codegen of non-associative operations 
(like
+/// << or >>).
+uint8_t IsXLHSInRHSPart : 1;
+/// Used for 'atomic update' or 'atomic capture' constructs. They may
+/// have atomic expressions of forms:
+/// \code
+/// v = x; ;
+/// ; v = x;
+/// \endcode
+/// This field is 1 for the first(postfix) form of the expression and 0
+/// otherwise.
+uint8_t IsPostfixUpdate : 1;
+  } Flags;
 
   /// Build directive with the given start and end location.
   ///
@@ -2956,10 +2959,10 @@
   /// Return true if helper update expression has form
   /// 'OpaqueValueExpr(x) binop OpaqueValueExpr(expr)' and false if it has form
   /// 'OpaqueValueExpr(expr) binop OpaqueValueExpr(x)'.
-  bool isXLHSInRHSPart() const { return IsXLHSInRHSPart; }
+  bool isXLHSInRHSPart() const { return Flags.IsXLHSInRHSPart; }
   /// Return true if 'v' expression must be updated to original value of
   /// 'x', false if 'v' must be updated to the new value of 'x'.
-  bool isPostfixUpdate() const { return IsPostfixUpdate; }
+  bool isPostfixUpdate() const { return Flags.IsPostfixUpdate; }
   /// Get 'v' part of the associated expression/statement.
   Expr *getV() {
 return cast_or_null(Data->getChildren()[DataPositionTy::POS_V]);


Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2449,8 +2449,8 @@
 void ASTStmtReader::VisitOMPAtomicDirective(OMPAtomicDirective *D) {
   VisitStmt(D);
   VisitOMPExecutableDirective(D);
-  D->IsXLHSInRHSPart = Record.readBool();
-  D->IsPostfixUpdate = Record.readBool();
+  D->Flags.IsXLHSInRHSPart = Record.readBool() ? 1 : 0;
+  D->Flags.IsPostfixUpdate = Record.readBool() ? 1 : 0;
 }
 
 void ASTStmtReader::VisitOMPTargetDirective(OMPTargetDirective *D) {
Index: clang/lib/AST/StmtOpenMP.cpp
=

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 423198.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120290/new/

https://reviews.llvm.org/D120290

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11663
   X = BO->getLHS();
-  D = BO->getRHS();
+  D = BO->getRHS()->IgnoreImpCasts();
 

ABataev wrote:
> Why do we need to use `IgnoreImpCasts()` here and in other places?
Clang usually inserts implicit casts. For example, if we have:
```
char a, b, c;
#pragma omp atomic compare capture
  { r = a; if (a > c) { a = b; } }
```
Clang inserts an implicit cast from `char` to `int` for all statements except 
`r = a`. In this case, what should be the right solution? I'm not quite sure 
actually.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120290/new/

https://reviews.llvm.org/D120290

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405645.
tianshilei1992 added a comment.

Fix template instantiation and add one test.
More tests are on the way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -5,6 +5,8 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify=omp51 -fopenmp -fopenmp-version=51 -ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -12,6 +14,7 @@
 
 template 
 T foo(T argc) {
+  T c = T();
   T b = T();
   T a = T();
 #pragma omp atomic
@@ -29,6 +32,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic seq_cst
   a++;
 #pragma omp atomic read seq_cst
@@ -44,6 +55,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare seq_cst // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic seq_cst compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare seq_cst // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read
@@ -59,6 +78,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare acq_rel // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic acq_rel compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare acq_rel // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read acquire
@@ -74,6 +101,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare acquire // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic acquire compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare acquire // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic release
   a++;
 #pragma omp atomic read
@@ -89,6 +124,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare release // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic release compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare release // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic relaxed
   a++;
 #pragma omp atomic read
@@ -104,6 +147,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare relaxed // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic relaxed compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic com

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Not sure if it's good to merge such a large patch. We could potentially split 
the patch to three independent patches: tool chain, device runtime, and the 
OpenMPOpt pass to support expansion of shared variable (which for some reason 
is not included in this patch. That is actually very important component 
otherwise the backend will complain about it).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113359/new/

https://reviews.llvm.org/D113359

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405703.
tianshilei1992 added a comment.

refine the test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify=omp50 -fopenmp -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify=omp50 %s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify=omp50 -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify=omp50 %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify=omp51 -fopenmp -fopenmp-version=51 -ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -12,6 +14,7 @@
 
 template 
 T foo(T argc) {
+  T c = T();
   T b = T();
   T a = T();
 #pragma omp atomic
@@ -29,6 +32,24 @@
 a = b;
 b++;
   }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an lvalue expression with scalar type}}
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a > b ? b : a; }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an lvalue expression with scalar type}}
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a < b ? b : a; }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an lvalue expression with scalar type}}
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a == b ? c : a; }
 #pragma omp atomic seq_cst
   a++;
 #pragma omp atomic read seq_cst
@@ -44,6 +65,24 @@
 a = b;
 b++;
   }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an lvalue expression with scalar type}}
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare seq_cst
+  { a = a > b ? b : a; }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an lvalue expression with scalar type}}
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic seq_cst compare
+  { a = a < b ? b : a; }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}
+// omp50-error@+3 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop ex

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

@ABataev @jdoerfert I got a question about writing the test. `atomic compare` 
is supported from 5.1. Basically we have three ways to guard those `atomic 
compare` code:

1. Use macro `_OPENMP`. This pretty much works with one exception: we don't set 
the macro for `-fopenmp-simd`. Then we cannot test `-fopenmp-simd`.
2. Use those `omp50-error` and `omp50-note` which are shown in this patch. 
`-fopenmp-simd` can be tested now. However, it doesn't work with `-emit-pch`. 
The compiler emits errors that `unexpected OpenMP clause 'compare' in directive 
'#pragma omp atomic'` for the lower version tests. If we add `-verify` to the 
`-emit-pch` line, then the pch will not be generated.
3. Put `atomic compare` tests in another file.

For now only the 3rd method could work, but I'm not sure if there are ways to 
work around the limitation in the first two approaches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:35
   }
+// omp50-error@+4 {{unexpected OpenMP clause 'compare' in directive '#pragma 
omp atomic'}}
+// omp50-note@+4 {{expected an expression statement}}

ABataev wrote:
> ast print tests are usually successful, better to add/modify special tests 
> for incorrect cases (the ones ending with _messages.cpp)
Yeah, I'll also do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3294390 , @ABataev wrote:

> In D116637#3294361 , 
> @tianshilei1992 wrote:
>
>> @ABataev @jdoerfert I got a question about writing the test. `atomic 
>> compare` is supported from 5.1. Basically we have three ways to guard those 
>> `atomic compare` code:
>>
>> 1. Use macro `_OPENMP`. This pretty much works with one exception: we don't 
>> set the macro for `-fopenmp-simd`. Then we cannot test `-fopenmp-simd`.
>> 2. Use those `omp50-error` and `omp50-note` which are shown in this patch. 
>> `-fopenmp-simd` can be tested now. However, it doesn't work with 
>> `-emit-pch`. The compiler emits errors that `unexpected OpenMP clause 
>> 'compare' in directive '#pragma omp atomic'` for the lower version tests. If 
>> we add `-verify` to the `-emit-pch` line, then the pch will not be generated.
>> 3. Put `atomic compare` tests in another file.
>>
>> For now only the 3rd method could work, but I'm not sure if there are ways 
>> to work around the limitation in the first two approaches.
>
> The second or the 3rd approach.
> The tests should work with emit pch and include pch options, otherwise 
> precompiled modules are broken. Need to test it too.

For the 2nd method, `-verify` doesn't work with `-emit-pch`. Any way to work 
around it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3294474 , @ABataev wrote:

> In D116637#3294466 , 
> @tianshilei1992 wrote:
>
>> In D116637#3294390 , @ABataev 
>> wrote:
>>
>>> In D116637#3294361 , 
>>> @tianshilei1992 wrote:
>>>
 @ABataev @jdoerfert I got a question about writing the test. `atomic 
 compare` is supported from 5.1. Basically we have three ways to guard 
 those `atomic compare` code:

 1. Use macro `_OPENMP`. This pretty much works with one exception: we 
 don't set the macro for `-fopenmp-simd`. Then we cannot test 
 `-fopenmp-simd`.
 2. Use those `omp50-error` and `omp50-note` which are shown in this patch. 
 `-fopenmp-simd` can be tested now. However, it doesn't work with 
 `-emit-pch`. The compiler emits errors that `unexpected OpenMP clause 
 'compare' in directive '#pragma omp atomic'` for the lower version tests. 
 If we add `-verify` to the `-emit-pch` line, then the pch will not be 
 generated.
 3. Put `atomic compare` tests in another file.

 For now only the 3rd method could work, but I'm not sure if there are ways 
 to work around the limitation in the first two approaches.
>>>
>>> The second or the 3rd approach.
>>> The tests should work with emit pch and include pch options, otherwise 
>>> precompiled modules are broken. Need to test it too.
>>
>> For the 2nd method, `-verify` doesn't work with `-emit-pch`. Any way to work 
>> around it?
>
> Do not use it with emit-pch, just like in the test you modified

Then those tests w/o `-fopenmp-version=51`, `clang` emits error "unexpected 
OpenMP clause 'compare' in directive '#pragma omp atomic'" and exits w/ error 
code, and the test fails.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3294498 , @ABataev wrote:

> In D116637#3294490 , 
> @tianshilei1992 wrote:
>
>> In D116637#3294474 , @ABataev 
>> wrote:
>>
>>> In D116637#3294466 , 
>>> @tianshilei1992 wrote:
>>>
 In D116637#3294390 , @ABataev 
 wrote:

> In D116637#3294361 , 
> @tianshilei1992 wrote:
>
>> @ABataev @jdoerfert I got a question about writing the test. `atomic 
>> compare` is supported from 5.1. Basically we have three ways to guard 
>> those `atomic compare` code:
>>
>> 1. Use macro `_OPENMP`. This pretty much works with one exception: we 
>> don't set the macro for `-fopenmp-simd`. Then we cannot test 
>> `-fopenmp-simd`.
>> 2. Use those `omp50-error` and `omp50-note` which are shown in this 
>> patch. `-fopenmp-simd` can be tested now. However, it doesn't work with 
>> `-emit-pch`. The compiler emits errors that `unexpected OpenMP clause 
>> 'compare' in directive '#pragma omp atomic'` for the lower version 
>> tests. If we add `-verify` to the `-emit-pch` line, then the pch will 
>> not be generated.
>> 3. Put `atomic compare` tests in another file.
>>
>> For now only the 3rd method could work, but I'm not sure if there are 
>> ways to work around the limitation in the first two approaches.
>
> The second or the 3rd approach.
> The tests should work with emit pch and include pch options, otherwise 
> precompiled modules are broken. Need to test it too.

 For the 2nd method, `-verify` doesn't work with `-emit-pch`. Any way to 
 work around it?
>>>
>>> Do not use it with emit-pch, just like in the test you modified
>>
>> Then those tests w/o `-fopenmp-version=51`, `clang` emits error "unexpected 
>> OpenMP clause 'compare' in directive '#pragma omp atomic'" and exits w/ 
>> error code, and the test fails.
>
> You can define your own macro for OpenMP51 tests and guard the new code with 
> `#ifdef value`

Oh yeah, that is a good way. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405725.
tianshilei1992 added a comment.

update test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -5,6 +5,10 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp -fopenmp-version=51 -ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
+
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp-simd -fopenmp-version=51 -ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -12,6 +16,7 @@
 
 template 
 T foo(T argc) {
+  T c = T();
   T b = T();
   T a = T();
 #pragma omp atomic
@@ -29,6 +34,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a < b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic seq_cst
   a++;
 #pragma omp atomic read seq_cst
@@ -44,6 +60,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare seq_cst
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic seq_cst compare
+  { a = a < b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare seq_cst
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read
@@ -59,6 +86,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare acq_rel
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic acq_rel compare
+  { a = a < b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare acq_rel
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read acquire
@@ -74,6 +112,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare acquire
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic acquire compare
+  { a = a < b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare acquire
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic release
   a++;
 #pragma omp atomic read
@@ -89,6 +138,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare release
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic release compare
+  { a = a < b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare release
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic relaxed
   a++;
 #pragma omp atomic read
@@ -104,6 +164,17 @@
 a = b;
 b++;
   }
+#ifdef OMP51
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare relaxed
+  { a = a > b ? b : a; }
+// omp51-error@+1 {{atomic compare is not supported for now}} omp51-error@+1 {{atomic compar

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:10
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp -fopenmp-version=51 
-ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
+
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp-simd -fopenmp-version=51 
-ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s

@ABataev But still in this path, we cannot test `-emit-pch` because we will 
emit `atomic compare is not supported for now` anyway, and it cannot be worked 
around. I'll add it in the codegen patch or in a following patch of codegen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:964
+  // CHECK-51-NEXT: }
+  // omp51-note@+1 {{in instantiation of function template specialization 
'foo' requested here}}
   return foo(a);

I cannot figure out why I have to add this line and it's not needed before.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118934: [OpenMP] Completely remove old device runtime

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2470
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,

Since LLVM 14 has this flags, we probably want to keep this argument, mark it 
as deprecated, and emit a warning saying there is no old runtime anymore.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118934/new/

https://reviews.llvm.org/D118934

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118934: [OpenMP] Completely remove old device runtime

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

If this patch can be back ported to 14, then it's totally fine to remove the 
argument. Otherwise keeping it sounds more reasonable. If we keep it, we'd 
better to emit a warning and then we could remove it in 15 or other version w/o 
any concern that we don't tell users this flag is deprecated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118934/new/

https://reviews.llvm.org/D118934

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:964
+  // CHECK-51-NEXT: }
+  // omp51-note@+1 {{in instantiation of function template specialization 
'foo' requested here}}
   return foo(a);

jdoerfert wrote:
> tianshilei1992 wrote:
> > I cannot figure out why I have to add this line and it's not needed before.
> it's caused by the error.
Aha, I see. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118944: [OpenMP] Add Cuda path to linker wrapper tool

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8163
+CmdArgs.push_back(Args.MakeArgString(
+"-cuda-path=" + CudaInstallation.getInstallPath()));
+  break;

Would be better to keep aligned with `clang` to use `--cuda-path`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118944/new/

https://reviews.llvm.org/D118944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118944: [OpenMP] Add Cuda path to linker wrapper tool

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118944/new/

https://reviews.llvm.org/D118944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405827.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

add more tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.c
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 int foo() {
 L1:
@@ -914,6 +914,16 @@
 // expected-note@+1 2 {{'capture' clause used here}}
 #pragma omp atomic capture read
   a = ++b;
+#ifdef OMP51
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   return T();
 }
 
@@ -935,6 +945,16 @@
 // expected-note@+1 {{'write' clause used here}}
 #pragma omp atomic write capture
   a = b;
+#ifdef OMP51
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -394,3 +396,82 @@
 #pragma omp atomic hint(1) hint(1) // omp45-error 2 {{unexpected OpenMP clause 'hint' in directive '#pragma omp atomic'}} expected-error {{directive '#pragma omp atomic' cannot contain more than one 'hint' clause}}
   a += 1;
 }
+
+#ifdef OMP51
+extern void bbar();
+extern int ffoo();
+
+void compare() {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare
+  {}
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11206
+if (!E->isInstantiationDependent()) {
+  auto Type = E->getType();
+  if (!Type->isScalarType()) {

ABataev wrote:
> `QualType`
Do you mean by not using `auto`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405837.
tianshilei1992 added a comment.

add final test and fix comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.c
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 int foo() {
 L1:
@@ -914,6 +914,16 @@
 // expected-note@+1 2 {{'capture' clause used here}}
 #pragma omp atomic capture read
   a = ++b;
+#ifdef OMP51
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   return T();
 }
 
@@ -935,6 +945,16 @@
 // expected-note@+1 {{'write' clause used here}}
 #pragma omp atomic write capture
   a = b;
+#ifdef OMP51
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -394,3 +396,92 @@
 #pragma omp atomic hint(1) hint(1) // omp45-error 2 {{unexpected OpenMP clause 'hint' in directive '#pragma omp atomic'}} expected-error {{directive '#pragma omp atomic' cannot contain more than one 'hint' clause}}
   a += 1;
 }
+
+#ifdef OMP51
+extern void bbar();
+extern int ffoo();
+
+void compare() {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare
+  {}
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', 

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:10
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp -fopenmp-version=51 
-ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
+
+// RUN: %clang_cc1 -DOMP51 -verify=omp51 -fopenmp-simd -fopenmp-version=51 
-ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s

ABataev wrote:
> tianshilei1992 wrote:
> > @ABataev But still in this path, we cannot test `-emit-pch` because we will 
> > emit `atomic compare is not supported for now` anyway, and it cannot be 
> > worked around. I'll add it in the codegen patch or in a following patch of 
> > codegen.
> Why? It should be tested.
> ```
> // RUN: %clang_cc1 -fopenmp -DOMP51 -fopenmp-version=51 -x c++ -std=c++11 
> -emit-pch -o %t %s
> // RUN: %clang_cc1 -fopenmp -DOMP51 -fopenmp-version=51 -std=c++11 
> -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
> ```
> Could you try this?
I did, but since the first command can emit errors, the test fails. It should 
be tested. I'll add it back when we don't emit error in Sema.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405967.
tianshilei1992 added a comment.

add TODO for PCH test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.c
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 int foo() {
 L1:
@@ -914,6 +914,16 @@
 // expected-note@+1 2 {{'capture' clause used here}}
 #pragma omp atomic capture read
   a = ++b;
+#ifdef OMP51
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   return T();
 }
 
@@ -935,6 +945,16 @@
 // expected-note@+1 {{'write' clause used here}}
 #pragma omp atomic write capture
   a = b;
+#ifdef OMP51
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -394,3 +396,92 @@
 #pragma omp atomic hint(1) hint(1) // omp45-error 2 {{unexpected OpenMP clause 'hint' in directive '#pragma omp atomic'}} expected-error {{directive '#pragma omp atomic' cannot contain more than one 'hint' clause}}
   a += 1;
 }
+
+#ifdef OMP51
+extern void bbar();
+extern int ffoo();
+
+void compare() {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare
+  {}
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x or

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:40
+#ifdef OMP51
+// omp51-error@+1 2 {{atomic compare is not supported for now}}
+#pragma omp atomic compare

ABataev wrote:
> Try to add `-fsyntax-only` in RUN to avoid error message in the test
Adding `-fsyntax-only` will not print the AST, just the error message.
```
➜  clang -cc1 -x c++ -fopenmp -fopenmp-version=51 -ast-print atomic.cpp
atomic.cpp:12:20: error: atomic compare is not supported for now
#pragma omp atomic compare
   ^
class C {
int x;
public:
int &val() {
return this->x;
}
};
void compare() {
C x;
C e;
C d;
#pragma omp atomic compare
if (x.val() == e.val())
x.val() = d.val();
}
1 error generated.
➜  clang -cc1 -x c++ -fopenmp -fopenmp-version=51 -ast-print -fsyntax-only 
atomic.cpp
atomic.cpp:12:20: error: atomic compare is not supported for now
#pragma omp atomic compare
   ^
1 error generated.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_ast_print.cpp:40
+#ifdef OMP51
+// omp51-error@+1 2 {{atomic compare is not supported for now}}
+#pragma omp atomic compare

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > Try to add `-fsyntax-only` in RUN to avoid error message in the test
> > Adding `-fsyntax-only` will not print the AST, just the error message.
> > ```
> > ➜  clang -cc1 -x c++ -fopenmp -fopenmp-version=51 -ast-print atomic.cpp
> > atomic.cpp:12:20: error: atomic compare is not supported for now
> > #pragma omp atomic compare
> >^
> > class C {
> > int x;
> > public:
> > int &val() {
> > return this->x;
> > }
> > };
> > void compare() {
> > C x;
> > C e;
> > C d;
> > #pragma omp atomic compare
> > if (x.val() == e.val())
> > x.val() = d.val();
> > }
> > 1 error generated.
> > ➜  clang -cc1 -x c++ -fopenmp -fopenmp-version=51 -ast-print -fsyntax-only 
> > atomic.cpp
> > atomic.cpp:12:20: error: atomic compare is not supported for now
> > #pragma omp atomic compare
> >^
> > 1 error generated.
> > ```
> Ah, yes. Can you move the error message to codegen from Sema to avoid all 
> these problems?
Do we support error message in code gen? I didn't see similar code in 
`clang/lib/CodeGen/CGStmtOpenMP.cpp`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405985.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

move error to code gen and add missing tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.c
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 int foo() {
 L1:
@@ -914,6 +914,16 @@
 // expected-note@+1 2 {{'capture' clause used here}}
 #pragma omp atomic capture read
   a = ++b;
+#ifdef OMP51
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   return T();
 }
 
@@ -935,18 +945,16 @@
 // expected-note@+1 {{'write' clause used here}}
 #pragma omp atomic write capture
   a = b;
+#ifdef OMP51
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
-
-#if _OPENMP >= 202011
-int compare() {
-  int a, b, c;
-// omp51-error@+1 {{atomic compare is not supported for now}}
-#pragma omp atomic compare
-  {
-if (a == b)
-  a = c;
-  }
-}
-#endif
Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -394,3 +396,92 @@
 #pragma omp atomic hint(1) hint(1) // omp45-error 2 {{unexpected OpenMP clause 'hint' in directive '#pragma omp atomic'}} expected-error {{directive '#pragma omp atomic' cannot contain more than one 'hint' clause}}
   a += 1;
 }
+
+#ifdef OMP51
+extern void bbar();
+extern int ffoo();
+
+void compare() {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, '

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb35be6fe98e3: [Clang][Sema][OpenMP] Sema support for `atomic 
compare` (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.c
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
 
 int foo() {
 L1:
@@ -914,6 +914,16 @@
 // expected-note@+1 2 {{'capture' clause used here}}
 #pragma omp atomic capture read
   a = ++b;
+#ifdef OMP51
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 2 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   return T();
 }
 
@@ -935,18 +945,16 @@
 // expected-note@+1 {{'write' clause used here}}
 #pragma omp atomic write capture
   a = b;
+#ifdef OMP51
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'write' clause used here}}
+#pragma omp atomic write compare
+  a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'read', 'write', 'update', 'capture', or 'compare' clause}}
+// expected-note@+1 {{'read' clause used here}}
+#pragma omp atomic read compare
+  a = b;
+#endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
-
-#if _OPENMP >= 202011
-int compare() {
-  int a, b, c;
-// omp51-error@+1 {{atomic compare is not supported for now}}
-#pragma omp atomic compare
-  {
-if (a == b)
-  a = c;
-  }
-}
-#endif
Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -394,3 +396,92 @@
 #pragma omp atomic hint(1) hint(1) // omp45-error 2 {{unexpected OpenMP clause 'hint' in directive '#pragma omp atomic'}} expected-error {{directive '#pragma omp atomic' cannot contain more than one 'hint' clause}}
   a += 1;
 }
+
+#ifdef OMP51
+extern void bbar();
+extern int ffoo();
+
+void compare() {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e 

[PATCH] D118632: [WIP][Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 406008.
tianshilei1992 added a comment.

rebase and make preparation for codegen in clang


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -572,6 +572,7 @@
   int c = 0;
   int b = 0;
   int a = 0;
+  int c = 0;
 // CHECK: int a = 0;
 #pragma omp atomic
   a++;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11767,14 +11767,18 @@
   << ErrorInfo.Error << ErrorInfo.NoteRange;
   return StmtError();
 }
-// TODO: We don't set X, D, E, etc. here because in code gen we will emit
-// error directly.
+X = Checker.getX();
+E = Checker.getE();
+D = Checker.getD();
+CE = Checker.getCond();
+// We reuse this bool variable to tell if it is in the form 'x ordop expr'.
+IsXLHSInRHSPart = Checker.isXBinopExpr();
   }
 
   setFunctionHasBranchProtectedScope();
 
   return OMPAtomicDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt,
-X, V, E, UE, IsXLHSInRHSPart,
+X, V, E, UE, D, CE, IsXLHSInRHSPart,
 IsPostfixUpdate);
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6011,11 +6011,52 @@
   }
 }
 
+static void emitOMPAtomicCompareExpr(CodeGenFunction &CGF,
+ llvm::AtomicOrdering AO, const Expr *X,
+ const Expr *E, const Expr *D,
+ const Expr *CE, bool IsXBinopExpr,
+ SourceLocation Loc) {
+
+  llvm::OpenMPIRBuilder &OMPBuilder =
+  CGF.CGM.getOpenMPRuntime().getOMPBuilder();
+  // llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
+  // CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
+
+  OMPAtomicCompareOp Op;
+  assert(isa(CE) && "CE is not a BinaryOperator");
+  switch (cast(CE)->getOpcode()) {
+  case BO_EQ:
+Op = OMPAtomicCompareOp::EQ;
+break;
+  case BO_LT:
+Op = OMPAtomicCompareOp::MIN;
+break;
+  case BO_GT:
+Op = OMPAtomicCompareOp::MAX;
+break;
+  default:
+llvm_unreachable("unsupported atomic compare binary operator");
+  }
+
+  LValue XLVal = CGF.EmitLValue(X);
+  llvm::Value *XPtr = XLVal.getPointer(CGF);
+  llvm::Value *EVal = CGF.EmitScalarExpr(E);
+  llvm::Value *DVal = D ? CGF.EmitScalarExpr(D) : nullptr;
+
+  llvm::OpenMPIRBuilder::AtomicOpValue XOpVal{
+  XPtr, XPtr->getType()->getPointerElementType(),
+  X->getType().isVolatileQualified(),
+  X->getType()->hasSignedIntegerRepresentation()};
+
+  CGF.Builder.restoreIP(OMPBuilder.createAtomicCompare(
+  CGF.Builder, XOpVal, EVal, DVal, AO, Op, IsXBinopExpr));
+}
+
 static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
   llvm::AtomicOrdering AO, bool IsPostfixUpdate,
   const Expr *X, const Expr *V, const Expr *E,
-  const Expr *UE, bool IsXLHSInRHSPart,
-  SourceLocation Loc) {
+  const Expr *UE, const Expr *D, const Expr *CE,
+  bool IsXLHSInRHSPart, SourceLocation Loc) {
   switch (Kind) {
   case OMPC_read:
 emitOMPAtomicReadExpr(CGF, AO, X, V, Loc);
@@ -6031,11 +6072,8 @@
 emitOMPAtomicCaptureExpr(CGF, AO, IsPostfixUpdate, V, X, E, UE,
  IsXLHSInRHSPart, Loc);
 break;
-  case OMPC_compare: {
-// Emit an error here.
-unsigned DiagID = CGF.CGM.getDiags().getCustomDiagID(
-DiagnosticsEngine::Error, "'atomic compare' is not supported for now");
-CGF.CGM.getDiags().Report(DiagID);
+  case OMPC_compare:
+emitOMPAtomicCompareExpr(CGF, AO, X, E, D, CE, IsXLHSInRHSPart, Loc);
 break;
   }
   case OMPC_if:
@@ -6182,8 +6220,8 @@
   LexicalScope Scope(*this, S.getSourceRange());
   EmitStopPoint(S.getAssociatedStmt());
   emitOMPAtomicExpr(*this, Kind, AO, S.isPostfixUpdate(), S.getX(), S.getV(),
-S.getExpr(), S.getUpdateExpr(), S.isXLHSInRHSPart(),
-S.getBeginLoc());
+S.getExpr(), S.getUpdateExpr(), S.getD(), S.getCondExpr(),
+S.isXLHSInRHSPart(), S.getBeginLoc());
 }
 

[PATCH] D118632: [WIP][Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 406012.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

remove leftovers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp

Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11767,14 +11767,18 @@
   << ErrorInfo.Error << ErrorInfo.NoteRange;
   return StmtError();
 }
-// TODO: We don't set X, D, E, etc. here because in code gen we will emit
-// error directly.
+X = Checker.getX();
+E = Checker.getE();
+D = Checker.getD();
+CE = Checker.getCond();
+// We reuse this bool variable to tell if it is in the form 'x ordop expr'.
+IsXLHSInRHSPart = Checker.isXBinopExpr();
   }
 
   setFunctionHasBranchProtectedScope();
 
   return OMPAtomicDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt,
-X, V, E, UE, IsXLHSInRHSPart,
+X, V, E, UE, D, CE, IsXLHSInRHSPart,
 IsPostfixUpdate);
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6011,11 +6011,50 @@
   }
 }
 
+static void emitOMPAtomicCompareExpr(CodeGenFunction &CGF,
+ llvm::AtomicOrdering AO, const Expr *X,
+ const Expr *E, const Expr *D,
+ const Expr *CE, bool IsXBinopExpr,
+ SourceLocation Loc) {
+
+  llvm::OpenMPIRBuilder &OMPBuilder =
+  CGF.CGM.getOpenMPRuntime().getOMPBuilder();
+
+  OMPAtomicCompareOp Op;
+  assert(isa(CE) && "CE is not a BinaryOperator");
+  switch (cast(CE)->getOpcode()) {
+  case BO_EQ:
+Op = OMPAtomicCompareOp::EQ;
+break;
+  case BO_LT:
+Op = OMPAtomicCompareOp::MIN;
+break;
+  case BO_GT:
+Op = OMPAtomicCompareOp::MAX;
+break;
+  default:
+llvm_unreachable("unsupported atomic compare binary operator");
+  }
+
+  LValue XLVal = CGF.EmitLValue(X);
+  llvm::Value *XPtr = XLVal.getPointer(CGF);
+  llvm::Value *EVal = CGF.EmitScalarExpr(E);
+  llvm::Value *DVal = D ? CGF.EmitScalarExpr(D) : nullptr;
+
+  llvm::OpenMPIRBuilder::AtomicOpValue XOpVal{
+  XPtr, XPtr->getType()->getPointerElementType(),
+  X->getType().isVolatileQualified(),
+  X->getType()->hasSignedIntegerRepresentation()};
+
+  CGF.Builder.restoreIP(OMPBuilder.createAtomicCompare(
+  CGF.Builder, XOpVal, EVal, DVal, AO, Op, IsXBinopExpr));
+}
+
 static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
   llvm::AtomicOrdering AO, bool IsPostfixUpdate,
   const Expr *X, const Expr *V, const Expr *E,
-  const Expr *UE, bool IsXLHSInRHSPart,
-  SourceLocation Loc) {
+  const Expr *UE, const Expr *D, const Expr *CE,
+  bool IsXLHSInRHSPart, SourceLocation Loc) {
   switch (Kind) {
   case OMPC_read:
 emitOMPAtomicReadExpr(CGF, AO, X, V, Loc);
@@ -6031,11 +6070,8 @@
 emitOMPAtomicCaptureExpr(CGF, AO, IsPostfixUpdate, V, X, E, UE,
  IsXLHSInRHSPart, Loc);
 break;
-  case OMPC_compare: {
-// Emit an error here.
-unsigned DiagID = CGF.CGM.getDiags().getCustomDiagID(
-DiagnosticsEngine::Error, "'atomic compare' is not supported for now");
-CGF.CGM.getDiags().Report(DiagID);
+  case OMPC_compare:
+emitOMPAtomicCompareExpr(CGF, AO, X, E, D, CE, IsXLHSInRHSPart, Loc);
 break;
   }
   case OMPC_if:
@@ -6182,8 +6218,8 @@
   LexicalScope Scope(*this, S.getSourceRange());
   EmitStopPoint(S.getAssociatedStmt());
   emitOMPAtomicExpr(*this, Kind, AO, S.isPostfixUpdate(), S.getX(), S.getV(),
-S.getExpr(), S.getUpdateExpr(), S.isXLHSInRHSPart(),
-S.getBeginLoc());
+S.getExpr(), S.getUpdateExpr(), S.getD(), S.getCondExpr(),
+S.isXLHSInRHSPart(), S.getBeginLoc());
 }
 
 static void emitCommonOMPTargetDirective(CodeGenFunction &CGF,
Index: clang/lib/AST/StmtOpenMP.cpp
===
--- clang/lib/AST/StmtOpenMP.cpp
+++ clang/lib/AST/StmtOpenMP.cpp
@@ -863,16 +863,20 @@
!IsStandalone);
 }
 
-OMPAtomicDirective *OMPAtomicDirective::Create(
-const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
-ArrayRef Clauses, Stmt *

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3297387 , @cmtice wrote:

> This commit is causing some of our builds to fail with these errors:
>
> clang/lib/Sema/SemaOpenMP.cpp:11372:9: error: unused variable 'D' 
> [-Werror,-Wunused-variable]
>
>   Expr *D = nullptr;
> ^
>
> clang/lib/Sema/SemaOpenMP.cpp:11373:9: error: unused variable 'CE' 
> [-Werror,-Wunused-variable]
>
>   Expr *CE = nullptr;
> ^
>
> 2 errors generated.
>
> Would you please fix this?

Yeah, I noticed that. It's been fixed now. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3297785 , @kda wrote:

> This still seems to be breaking sanitizers fast: 
> https://lab.llvm.org/buildbot/#/builders/5/builds/18563
>
> To reproduce these results:
> https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild

I pushed a fix about half an hour ago. It should fix the issue. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3297833 , @jdoerfert wrote:

> In D116637#3297790 , 
> @tianshilei1992 wrote:
>
>> In D116637#3297785 , @kda wrote:
>>
>>> This still seems to be breaking sanitizers fast: 
>>> https://lab.llvm.org/buildbot/#/builders/5/builds/18563
>>>
>>> To reproduce these results:
>>> https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild
>>
>> I pushed a fix about half an hour ago. It should fix the issue. Thanks.
>
> I doubt it is fixed. Also, please reference the phab review in a fix to make 
> it easier to find it.

It will. The BuildBot link here is still the one for this patch. It is fixed in 
`b8ec430de71766d9a35a6b737c8a789c0c7cf812`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D116637#3298635 , @cmtice wrote:

> When building/running 
> third_party/llvm/llvm-project/clang/test/OpenMP/atomic_messages.c and 
> third_party/llvm/llvm-project/clang/test/OpenMP/atomic_ast_print.cpp, we get 
> use-of-uninitialized-variable error messages:
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value 
> third_party/llvm/llvm-project/clang/lib/Sema/SemaOpenMP.cpp:11214:40 in 
> (anonymous namespace)::OpenMPAtomicCompareChecker::checkType((anonymous 
> namespace)::OpenMPAtomicCompareChecker::ErrorInfoTy&) 
> const::$_57::operator()(clang::Expr const*, llvm::omp::OMPAtomicCompareOp, 
> bool) const
> Exiting
>
> You should probably look into this.

Thanks! It has been fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116637/new/

https://reviews.llvm.org/D116637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [WIP][Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 406310.
tianshilei1992 added a comment.

add tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [WIP][Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex 
"__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" 
"pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 
-fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s

The test case itself was generated from a Python script.



Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:3
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 
-fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s
+// %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 
-target-cpu core2 -emit-pch -o %t %s
+// %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 
-target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s

This test cannot be done because clang crashed. I filed a bug 
https://github.com/llvm/llvm-project/issues/53601.



Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:6
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 
-fopenmp-simd -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
+// %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple 
x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s

CodeGen for `-fopen-simd` is wrong. It doesn't emit corresponding atomic 
operations. I'll fix it.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3496
 : Builder.CreateBitCast(X.Var, IntCastTy->getPointerTo(Addrspace));
+Value *NewE = E->getType()->isFloatingPointTy()
+  ? Builder.CreateFPToSI(E, IntCastTy)

This change will be separated.

Actually I got a question about whether we want support equality comparison for 
floating point values. In the patch, we don't support '<' or '>' for floating 
point values because we don't have corresponding atomic instruction for them. 
For '==', we cast them to integers and then compare the two integers. However, 
We all know it doesn't make too much sense to compare two floating point 
variables, such as `x == d`. The comparison of floating point variables should 
always do like `x - d < epsilon`. However, like we said before, that comparison 
cannot be done because of lack of instruction. So I'm not sure if we want to 
support floating point variables at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [WIP][Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 406546.
tianshilei1992 added a comment.

fix clang crash and rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:1990
+// CHECK-NEXT:[[DD:%.*]] = alloca double, align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i8, i8* [[CE]], align 1
+// CHECK-NEXT:[[TMP1:%.*]] = atomicrmw umin i8* [[CX]], i8 [[TMP0]] 
monotonic, align 1

I think the `store` here is redundant. Is it because I'm using 
`CGF.EmitScalarExpr`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:1990
+// CHECK-NEXT:[[DD:%.*]] = alloca double, align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i8, i8* [[CE]], align 1
+// CHECK-NEXT:[[TMP1:%.*]] = atomicrmw umin i8* [[CX]], i8 [[TMP0]] 
monotonic, align 1

tianshilei1992 wrote:
> I think the `store` here is redundant. Is it because I'm using 
> `CGF.EmitScalarExpr`?
Oh, shoot. `load` here, instead of `store`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6041
+  llvm::Value *EVal = CGF.EmitScalarExpr(E->IgnoreImpCasts());
+  llvm::Value *DVal = D ? CGF.EmitScalarExpr(D->IgnoreImpCasts()) : nullptr;
+

Using `D->IgnoreImpCasts()` can make sure to avoid the case that `char` is 
casted to `int` in binary operation. However, say, if user writes the following 
code:
```
int x;
#pragma omp atomic compare
  x = x > 1.01 ? 1.01 : x;
```
`1.01` here will be casted to `1` by clang, and a warning will be emitted. 
Because we ignore the implicit cast, in Sema, it is taken as floating point 
value. However, we already told user that it is casted to `1`, which is a 
little weird to emit an error then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 406867.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -181,6 +181,10 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+// A dummy clause if compare and capture clauses are present.
+def OMPC_CompareCapture : Clause<"compare_capture"> {
+  let clangClass = "OMPCompareCaptureClause";
+}
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2277,6 +2277,9 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPCompareCaptureClause(
+const OMPCompareCaptureClause *) {}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,26 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#if _OPENMP >= 202011
+int compare() {
+  int a, b, c;
+// omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  {
+if (a == b)
+  a = c;
+  }
+}
+
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6295,6 +6295,8 @@
 
 void OMPClauseWriter::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseWriter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {}
+
 void OMPClauseWriter::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseWriter::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -11786,6 +11786,9 @@
   case llvm::omp::OMPC_compare:
 C = new (Context) OMPCompareClause();
 break;
+  case llvm::omp::OMPC_compare_capture:
+C = new (Context) OMPCompareCaptureClause();
+break;
   case llvm::omp::OMPC_seq_cst:
 C = new (Context) OMPSeqCstClause();
 break;
@@ -12146,6 +12149,8 @@
 
 void OMPClauseReader::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseReader::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {}
+
 void OMPClauseReader::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseReader::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9476,6 +9476,13 @@
   return C;
 }
 
+template 
+OMPClause *TreeTransform::TransformOMPCompareCaptureClause(
+OMPCompareCaptureClause *C) {
+  // No need to rebuild this clause, no template-dependent parameters.
+  return C;
+}
+
 template 
 OMPClause *
 TreeTransform::TransformOMPSeqCstClause(OMPSeqCstClause *C) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/IndexedMap.h"
 #include "ll

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/AST/OpenMPClause.cpp:1804
+void OMPClausePrinter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) 
{
+  // Do nothing as it is dummy.
+}

ABataev wrote:
> Output?
I did it on purpose because `OMPCompareCaptureClause` is a dummy node. When it 
is visited (I really doubt if it can be visited because we don't create it 
explicitly), it should not output any thing. `compare` and `capture` are 
printed when visiting the two clauses.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11363-11364
   break;
+case OMPC_compare_capture:
+  llvm_unreachable("OMPC_compare_capture should never be created");
 default:

ABataev wrote:
> Why?
For the same reason for the output in AST.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407307.
tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added a comment.

rebase and mark related functions as unreachable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -181,6 +181,10 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+// A dummy clause if compare and capture clauses are present.
+def OMPC_CompareCapture : Clause<"compare_capture"> {
+  let clangClass = "OMPCompareCaptureClause";
+}
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2277,6 +2277,11 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPCompareCaptureClause(
+const OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,26 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#if _OPENMP >= 202011
+int compare() {
+  int a, b, c;
+// omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  {
+if (a == b)
+  a = c;
+  }
+}
+
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6295,6 +6295,10 @@
 
 void OMPClauseWriter::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseWriter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseWriter::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseWriter::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -11786,6 +11786,8 @@
   case llvm::omp::OMPC_compare:
 C = new (Context) OMPCompareClause();
 break;
+  case llvm::omp::OMPC_compare_capture:
+llvm_unreachable("OMPCompareCaptureClause should never be reached");
   case llvm::omp::OMPC_seq_cst:
 C = new (Context) OMPSeqCstClause();
 break;
@@ -12146,6 +12148,10 @@
 
 void OMPClauseReader::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseReader::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseReader::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseReader::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9476,6 +9476,12 @@
   return C;
 }
 
+template 
+OMPClause *TreeTransform::TransformOMPCompareCaptureClause(
+OMPCompareCaptureClause *C) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 template 
 OMPClause *
 TreeTransform::Tran

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/AST/OpenMPClause.cpp:1804
+void OMPClausePrinter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) 
{
+  // Do nothing as it is dummy.
+}

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > Output?
> > I did it on purpose because `OMPCompareCaptureClause` is a dummy node. When 
> > it is visited (I really doubt if it can be visited because we don't create 
> > it explicitly), it should not output any thing. `compare` and `capture` are 
> > printed when visiting the two clauses.
> Why? I see ActOnOpenMPCompareCaptureClause, which creates such nodes.
I updated those functions. They should be unreachable.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11807-11812
+// TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+// code gen.
+unsigned DiagID = Diags.getCustomDiagID(
+DiagnosticsEngine::Error,
+"atomic compare capture is not supported for now");
+Diag(AtomicKindLoc, DiagID);

ABataev wrote:
> Emit in codegen
Since we are lack of Sema, putting it in Sema for now sounds more reasonable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407309.
tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added a comment.

use `LLVM_FALLTHROUGH`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -181,6 +181,10 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+// A dummy clause if compare and capture clauses are present.
+def OMPC_CompareCapture : Clause<"compare_capture"> {
+  let clangClass = "OMPCompareCaptureClause";
+}
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2277,6 +2277,11 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPCompareCaptureClause(
+const OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,26 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#if _OPENMP >= 202011
+int compare() {
+  int a, b, c;
+// omp51-error@+1 {{atomic compare is not supported for now}}
+#pragma omp atomic compare
+  {
+if (a == b)
+  a = c;
+  }
+}
+
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6295,6 +6295,10 @@
 
 void OMPClauseWriter::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseWriter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseWriter::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseWriter::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -11786,6 +11786,8 @@
   case llvm::omp::OMPC_compare:
 C = new (Context) OMPCompareClause();
 break;
+  case llvm::omp::OMPC_compare_capture:
+llvm_unreachable("OMPCompareCaptureClause should never be reached");
   case llvm::omp::OMPC_seq_cst:
 C = new (Context) OMPSeqCstClause();
 break;
@@ -12146,6 +12148,10 @@
 
 void OMPClauseReader::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseReader::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseReader::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseReader::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9476,6 +9476,12 @@
   return C;
 }
 
+template 
+OMPClause *TreeTransform::TransformOMPCompareCaptureClause(
+OMPCompareCaptureClause *C) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 template 
 OMPClause *
 TreeTransform::TransformOMPSeqCstClause(OMPSe

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407312.
tianshilei1992 added a comment.

cleanup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -181,6 +181,10 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+// A dummy clause if compare and capture clauses are present.
+def OMPC_CompareCapture : Clause<"compare_capture"> {
+  let clangClass = "OMPCompareCaptureClause";
+}
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2277,6 +2277,11 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPCompareCaptureClause(
+const OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,16 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#ifdef OMP51
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6295,6 +6295,10 @@
 
 void OMPClauseWriter::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseWriter::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseWriter::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseWriter::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -11786,6 +11786,8 @@
   case llvm::omp::OMPC_compare:
 C = new (Context) OMPCompareClause();
 break;
+  case llvm::omp::OMPC_compare_capture:
+llvm_unreachable("OMPCompareCaptureClause should never be reached");
   case llvm::omp::OMPC_seq_cst:
 C = new (Context) OMPSeqCstClause();
 break;
@@ -12146,6 +12148,10 @@
 
 void OMPClauseReader::VisitOMPCompareClause(OMPCompareClause *) {}
 
+void OMPClauseReader::VisitOMPCompareCaptureClause(OMPCompareCaptureClause *) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 void OMPClauseReader::VisitOMPSeqCstClause(OMPSeqCstClause *) {}
 
 void OMPClauseReader::VisitOMPAcqRelClause(OMPAcqRelClause *) {}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9476,6 +9476,12 @@
   return C;
 }
 
+template 
+OMPClause *TreeTransform::TransformOMPCompareCaptureClause(
+OMPCompareCaptureClause *C) {
+  llvm_unreachable("OMPCompareCaptureClause should never be reached");
+}
+
 template 
 OMPClause *
 TreeTransform::TransformOMPSeqCstClause(OMPSeqCstClause *C) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/IndexedMap.h"
 

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: guansong, yaxunl.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This is a follow-up patch of D119378 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119392

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c

Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -473,15 +473,5 @@
   x = e;
 d = e;
   }
-  float fx = 0.0f;
-  float fd = 0.0f;
-  float fe = 0.0f;
-// omp51-error@+5 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
-// omp51-note@+4 {{expect integer value}}
-#pragma omp atomic compare
-  {
-if (fx > fe)
-  fx = fe;
-  }
 }
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11007,8 +11007,6 @@
   Expr *C = nullptr;
   /// True if the cond expr is in the form of 'x ordop expr'.
   bool IsXBinopExpr = true;
-  /// The atomic compare operator.
-  OMPAtomicCompareOp Op;
 
   /// Check if it is a valid conditional update statement (cond-update-stmt).
   bool checkCondUpdateStmt(IfStmt *S, ErrorInfoTy &ErrorInfo);
@@ -11065,23 +11063,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11094,7 +11076,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11109,6 +11094,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11159,23 +11151,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11188,7 +11164,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11203,6 +11182,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11212,8 +11198,7 @@
   // 'x' and 'e' cannot be nullptr
   assert(X && E && "X and E cannot be nullptr");
 
-  auto CheckValue = [&ErrorInfo](const Expr *E, OMPAtomicCompareOp Op,
- bool ShouldBeLValue) {
+  auto CheckValue = [&ErrorInfo](const Expr *E, bool ShouldBeLValue) {
 if (ShouldBeLValue && !E->isLVal

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: README.md:71
+  mlir, openmp, polly, or pstl. ``LLVM_ENABLE_RUNTIMES`` can include 
any of
+  libcxx, libcxxabi, libunwind, or compiler-rt.
 

FWIW, OpenMP can be put into either of them. In OpenMP doc 
(https://openmp.llvm.org/SupportAndFAQ.html#q-how-to-build-an-openmp-gpu-offload-capable-compiler),
 we recommend to use `LLVM_ENABLE_RUNTIMES` to build OpenMP if users would like 
to use offloading features.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119351/new/

https://reviews.llvm.org/D119351

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407387.
tianshilei1992 added a comment.

recover test cases removed by mistake


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119392/new/

https://reviews.llvm.org/D119392

Files:
  clang/lib/Sema/SemaOpenMP.cpp

Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11007,8 +11007,6 @@
   Expr *C = nullptr;
   /// True if the cond expr is in the form of 'x ordop expr'.
   bool IsXBinopExpr = true;
-  /// The atomic compare operator.
-  OMPAtomicCompareOp Op;
 
   /// Check if it is a valid conditional update statement (cond-update-stmt).
   bool checkCondUpdateStmt(IfStmt *S, ErrorInfoTy &ErrorInfo);
@@ -11065,23 +11063,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11094,7 +11076,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11109,6 +11094,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11159,23 +11151,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11188,7 +11164,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11203,6 +11182,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11212,8 +11198,7 @@
   // 'x' and 'e' cannot be nullptr
   assert(X && E && "X and E cannot be nullptr");
 
-  auto CheckValue = [&ErrorInfo](const Expr *E, OMPAtomicCompareOp Op,
- bool ShouldBeLValue) {
+  auto CheckValue = [&ErrorInfo](const Expr *E, bool ShouldBeLValue) {
 if (ShouldBeLValue && !E->isLValue()) {
   ErrorInfo.Error = ErrorTy::XNotLValue;
   ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
@@ -11230,7 +11215,7 @@
 return false;
   }
 
-  if (Op != OMPAtomicCompareOp::EQ && !QTy->isIntegerType()) {
+  if (!QTy->isIntegerType()) {
 ErrorInfo.Error = ErrorTy::NotInteger;
 ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
 ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
@@ -11241,13 +11226,13 @@
 return true;
   };
 
-  if (!CheckValue(X, Op, true))
+  if (!CheckValue(X, true))
 return false;
 
-  if (!CheckValue(E, Op, false))
+  if (!CheckValue(E, false))
 return false;
 
-  if (D && !CheckValue(D, Op, false))
+  if (D && !CheckValue(D, false))
 return false;
 
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11801-11807
+  } else if (AtomicKind == OMPC_compare_capture) {
+// TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+// code gen.
+unsigned DiagID = Diags.getCustomDiagID(
+DiagnosticsEngine::Error,
+"atomic compare capture is not supported for now");
+Diag(AtomicKindLoc, DiagID);

ABataev wrote:
> Better to build a node and emit error in codegen. Without it you're unable to 
> create ast-print/dump tests, test for serialization/deserialization etc.
IIUC, `OMPC_compare_capture` will only be used in Sema and CodeGen to tell we 
actually want `compare_capture` instead of `compare` or `capture`. The 
corresponding class(es) have no actual use. If I could directly have 
`OMPC_compare_capture` w/o adding a class, I would like to do it. On the other 
hand, we already have a node for `compare`, and a node for `capture`, we don't 
want a node for `compare` and `capture`, especially the spec doesn't say they 
should be tightly close. That being said, it should not affect other 
functionality.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11801-11807
+  } else if (AtomicKind == OMPC_compare_capture) {
+// TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+// code gen.
+unsigned DiagID = Diags.getCustomDiagID(
+DiagnosticsEngine::Error,
+"atomic compare capture is not supported for now");
+Diag(AtomicKindLoc, DiagID);

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > Better to build a node and emit error in codegen. Without it you're 
> > > unable to create ast-print/dump tests, test for 
> > > serialization/deserialization etc.
> > IIUC, `OMPC_compare_capture` will only be used in Sema and CodeGen to tell 
> > we actually want `compare_capture` instead of `compare` or `capture`. The 
> > corresponding class(es) have no actual use. If I could directly have 
> > `OMPC_compare_capture` w/o adding a class, I would like to do it. On the 
> > other hand, we already have a node for `compare`, and a node for `capture`, 
> > we don't want a node for `compare` and `capture`, especially the spec 
> > doesn't say they should be tightly close. That being said, it should not 
> > affect other functionality.
> Then why do you need OMPC_compare_capture? Just emit compare and capture and 
> check for both of them at the same time.
`OMPC_compare_capture` is the natural way, especially when we call 
`emitOMPAtomicExpr`. Of course we can set `Kind` to `OMPC_compare` and add a 
boolean argument `bool IsCompareCapture = false`. It's just not as clear as 
this way, but TBH not a bad way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407701.
tianshilei1992 added a comment.

remove the dummy node


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,16 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#ifdef OMP51
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -11315,14 +11316,18 @@
   SourceLocation AtomicKindLoc;
   OpenMPClauseKind MemOrderKind = OMPC_unknown;
   SourceLocation MemOrderLoc;
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {
 switch (C->getClauseKind()) {
 case OMPC_read:
 case OMPC_write:
 case OMPC_update:
+  MutexClauseEncountered = true;
+  LLVM_FALLTHROUGH;
 case OMPC_capture:
 case OMPC_compare: {
-  if (AtomicKind != OMPC_unknown) {
+  if (AtomicKind != OMPC_unknown && MutexClauseEncountered) {
 Diag(C->getBeginLoc(), diag::err_omp_atomic_several_clauses)
 << SourceRange(C->getBeginLoc(), C->getEndLoc());
 Diag(AtomicKindLoc, diag::note_omp_previous_mem_order_clause)
@@ -11330,6 +11335,7 @@
   } else {
 AtomicKind = C->getClauseKind();
 AtomicKindLoc = C->getBeginLoc();
+EncounteredAtomicKinds.insert(C->getClauseKind());
   }
   break;
 }
@@ -11357,6 +11363,8 @@
   llvm_unreachable("unknown clause is encountered");
 }
   }
+  bool IsCompareCapture = EncounteredAtomicKinds.contains(OMPC_compare) &&
+  EncounteredAtomicKinds.contains(OMPC_capture);
   // OpenMP 5.0, 2.17.7 atomic Construct, Restrictions
   // If atomic-clause is read then memory-order-clause must not be acq_rel or
   // release.
@@ -11775,17 +11783,26 @@
 if (CurContext->isDependentContext())
   UE = V = E = X = nullptr;
   } else if (AtomicKind == OMPC_compare) {
-OpenMPAtomicCompareChecker::ErrorInfoTy ErrorInfo;
-OpenMPAtomicCompareChecker Checker(*this);
-if (!Checker.checkStmt(Body, ErrorInfo)) {
-  Diag(ErrorInfo.ErrorLoc, diag::err_omp_atomic_compare)
-  << ErrorInfo.ErrorRange;
-  Diag(ErrorInfo.NoteLoc, diag::note_omp_atomic_compare)
-  << ErrorInfo.Error << ErrorInfo.NoteRange;
-  return StmtError();
+if (IsCompareCapture) {
+  // TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+  // code gen.
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "atomic compare capture is not supported for now");
+  Diag(AtomicKindLoc, DiagID);
+} else {
+  OpenMPAtomicCompareChecker::ErrorInfoTy ErrorInfo;
+  OpenMPAtomicCompareChecker Checker(*this);
+  if (!Checker.checkStmt(Body, ErrorInfo)) {
+Diag(ErrorInfo.ErrorLoc, diag::err_omp_atomic_compare)
+<< ErrorInfo.ErrorRange;
+Diag(ErrorInfo.NoteLoc, diag::note_omp_atomic_compare)
+<< ErrorInfo.Error << ErrorInfo.NoteRange;
+return StmtError();
+  }
+  // TODO: We don't set X, D, E, etc. here because in code gen we will emit
+  // error directly.
 }
-// TODO: We don't set X, D, E, etc. here because in code gen we will emit
-// error directly.
   }
 
   setFunctionHasBranchProtectedScope();
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
@@ -6015,7 +6016,7 @@
   llvm::AtomicOrdering AO, bool IsPostfixUpdate,
   const Expr *X, const Expr *V, c

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11801-11807
+  } else if (AtomicKind == OMPC_compare_capture) {
+// TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+// code gen.
+unsigned DiagID = Diags.getCustomDiagID(
+DiagnosticsEngine::Error,
+"atomic compare capture is not supported for now");
+Diag(AtomicKindLoc, DiagID);

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > tianshilei1992 wrote:
> > > > ABataev wrote:
> > > > > Better to build a node and emit error in codegen. Without it you're 
> > > > > unable to create ast-print/dump tests, test for 
> > > > > serialization/deserialization etc.
> > > > IIUC, `OMPC_compare_capture` will only be used in Sema and CodeGen to 
> > > > tell we actually want `compare_capture` instead of `compare` or 
> > > > `capture`. The corresponding class(es) have no actual use. If I could 
> > > > directly have `OMPC_compare_capture` w/o adding a class, I would like 
> > > > to do it. On the other hand, we already have a node for `compare`, and 
> > > > a node for `capture`, we don't want a node for `compare` and `capture`, 
> > > > especially the spec doesn't say they should be tightly close. That 
> > > > being said, it should not affect other functionality.
> > > Then why do you need OMPC_compare_capture? Just emit compare and capture 
> > > and check for both of them at the same time.
> > `OMPC_compare_capture` is the natural way, especially when we call 
> > `emitOMPAtomicExpr`. Of course we can set `Kind` to `OMPC_compare` and add 
> > a boolean argument `bool IsCompareCapture = false`. It's just not as clear 
> > as this way, but TBH not a bad way.
> I think it is better to have a flag (or an internal enum) rather than adding 
> an extra external enum and a class just for internal processing.
Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 407868.
tianshilei1992 added a comment.

fix errors


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,3 +958,16 @@
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
 }
+
+#ifdef OMP51
+int compare_capture() {
+  int a, b, c, x;
+// omp51-error@+1 {{atomic compare capture is not supported for now}}
+#pragma omp atomic compare capture
+  {
+x = a;
+if (a == b)
+  a = c;
+  }
+}
+#endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -11315,14 +11316,18 @@
   SourceLocation AtomicKindLoc;
   OpenMPClauseKind MemOrderKind = OMPC_unknown;
   SourceLocation MemOrderLoc;
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {
 switch (C->getClauseKind()) {
 case OMPC_read:
 case OMPC_write:
 case OMPC_update:
+  MutexClauseEncountered = true;
+  LLVM_FALLTHROUGH;
 case OMPC_capture:
 case OMPC_compare: {
-  if (AtomicKind != OMPC_unknown) {
+  if (AtomicKind != OMPC_unknown && MutexClauseEncountered) {
 Diag(C->getBeginLoc(), diag::err_omp_atomic_several_clauses)
 << SourceRange(C->getBeginLoc(), C->getEndLoc());
 Diag(AtomicKindLoc, diag::note_omp_previous_mem_order_clause)
@@ -11330,6 +11335,7 @@
   } else {
 AtomicKind = C->getClauseKind();
 AtomicKindLoc = C->getBeginLoc();
+EncounteredAtomicKinds.insert(C->getClauseKind());
   }
   break;
 }
@@ -11357,6 +11363,12 @@
   llvm_unreachable("unknown clause is encountered");
 }
   }
+  bool IsCompareCapture = false;
+  if (EncounteredAtomicKinds.contains(OMPC_compare) &&
+  EncounteredAtomicKinds.contains(OMPC_capture)) {
+IsCompareCapture = true;
+AtomicKind = OMPC_compare;
+  }
   // OpenMP 5.0, 2.17.7 atomic Construct, Restrictions
   // If atomic-clause is read then memory-order-clause must not be acq_rel or
   // release.
@@ -11775,17 +11787,26 @@
 if (CurContext->isDependentContext())
   UE = V = E = X = nullptr;
   } else if (AtomicKind == OMPC_compare) {
-OpenMPAtomicCompareChecker::ErrorInfoTy ErrorInfo;
-OpenMPAtomicCompareChecker Checker(*this);
-if (!Checker.checkStmt(Body, ErrorInfo)) {
-  Diag(ErrorInfo.ErrorLoc, diag::err_omp_atomic_compare)
-  << ErrorInfo.ErrorRange;
-  Diag(ErrorInfo.NoteLoc, diag::note_omp_atomic_compare)
-  << ErrorInfo.Error << ErrorInfo.NoteRange;
-  return StmtError();
+if (IsCompareCapture) {
+  // TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+  // code gen.
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "atomic compare capture is not supported for now");
+  Diag(AtomicKindLoc, DiagID);
+} else {
+  OpenMPAtomicCompareChecker::ErrorInfoTy ErrorInfo;
+  OpenMPAtomicCompareChecker Checker(*this);
+  if (!Checker.checkStmt(Body, ErrorInfo)) {
+Diag(ErrorInfo.ErrorLoc, diag::err_omp_atomic_compare)
+<< ErrorInfo.ErrorRange;
+Diag(ErrorInfo.NoteLoc, diag::note_omp_atomic_compare)
+<< ErrorInfo.Error << ErrorInfo.NoteRange;
+return StmtError();
+  }
+  // TODO: We don't set X, D, E, etc. here because in code gen we will emit
+  // error directly.
 }
-// TODO: We don't set X, D, E, etc. here because in code gen we will emit
-// error directly.
   }
 
   setFunctionHasBranchProtectedScope();
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
@@ -6015,7 +6016,7 @@
   llvm::AtomicOrdering AO, bool IsPostfixUpdate,
 

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11790-11797
+if (IsCompareCapture) {
+  // TODO: For now we emit an error here and in emitOMPAtomicExpr we ignore
+  // code gen.
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "atomic compare capture is not supported for now");
+  Diag(AtomicKindLoc, DiagID);

ABataev wrote:
> Better not to emit error here, if possible. What's prevent this?
I put it here because I think we don't have Sema support yet, so it is not good 
to move it to later phase like codegen because that will implicitly mean Sema 
has already passed but apparently it doesn't. Of course like atomic compare, 
I'll move it to codegen in the Sema patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119392/new/

https://reviews.llvm.org/D119392

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118632: [Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118632/new/

https://reviews.llvm.org/D118632

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> SmallVector with 2 elements should be enough, no need to use SmallSet here.
`SmallVector` doesn't provide `contains`, which we use to check if 
`OMPC_compare` and `OMPC_capture` exist at the same time. Using `SmallVector` 
requires us to have a complicated comparison, for example:
```
(EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] == 
OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
EncounteredAtomicKinds[0] == OMPC_capture)
```
which is not as neat as `SmallSet`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > SmallVector with 2 elements should be enough, no need to use SmallSet 
> > > here.
> > `SmallVector` doesn't provide `contains`, which we use to check if 
> > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > `SmallVector` requires us to have a complicated comparison, for example:
> > ```
> > (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] == 
> > OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> > EncounteredAtomicKinds[0] == OMPC_capture)
> > ```
> > which is not as neat as `SmallSet`.
> Use llvm::find
That is linear time. Why it is better than `SmallSet`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > tianshilei1992 wrote:
> > > > ABataev wrote:
> > > > > SmallVector with 2 elements should be enough, no need to use SmallSet 
> > > > > here.
> > > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > > `SmallVector` requires us to have a complicated comparison, for example:
> > > > ```
> > > > (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] 
> > > > == OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > ```
> > > > which is not as neat as `SmallSet`.
> > > Use llvm::find
> > That is linear time. Why it is better than `SmallSet`?
> Because there are only 2 elements, no?
It could have more. If user writes it like `atomic compare compare capture 
capture capture`, then it will have 5 elements. `SmallSet::insert` returns a 
`pair` and we know if it is already in the set, which I have not added it yet 
but I'll. `SmallVector` then is not that easy to use. Of course you could say 
do `llvm::find` beforehand but it's not neat.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > tianshilei1992 wrote:
> > > > > ABataev wrote:
> > > > > > tianshilei1992 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > SmallVector with 2 elements should be enough, no need to use 
> > > > > > > > SmallSet here.
> > > > > > > `SmallVector` doesn't provide `contains`, which we use to check 
> > > > > > > if `OMPC_compare` and `OMPC_capture` exist at the same time. 
> > > > > > > Using `SmallVector` requires us to have a complicated comparison, 
> > > > > > > for example:
> > > > > > > ```
> > > > > > > (EncounteredAtomicKinds[0] == OMPC_compare && 
> > > > > > > EncounteredAtomicKinds[1] == OMPC_capture) || 
> > > > > > > (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > > > > ```
> > > > > > > which is not as neat as `SmallSet`.
> > > > > > Use llvm::find
> > > > > That is linear time. Why it is better than `SmallSet`?
> > > > Because there are only 2 elements, no?
> > > It could have more. If user writes it like `atomic compare compare 
> > > capture capture capture`, then it will have 5 elements. 
> > > `SmallSet::insert` returns a `pair` and we know if it is already in 
> > > the set, which I have not added it yet but I'll. `SmallVector` then is 
> > > not that easy to use. Of course you could say do `llvm::find` beforehand 
> > > but it's not neat.
> > Is this correct at all?
> You still can scan over the small vector and check if it has specified clause 
> already. 
It's not correct. We can easily check via the following code:
```
if (!EncounteredAtomicKinds.insert(C->getClauseKind()).second) {
  // emit an error
}
```
Using `SmallVector` we need to:
```
if (llvm::find(EncounteredAtomicKinds.begin(), EncounteredAtomicKinds.end(), 
C->getClauseKind() != EncounteredAtomicKinds.end()) {
  // emit an error
} else {
  EncounteredAtomicKinds.push_back(C->getClauseKind());
}
```
It's obvious which one looks better and have better readability. I can't see 
any point of using `SmallVector` here, unless there is huge performance 
difference, which I really doubt that matters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 408226.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

add tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116261/new/

https://reviews.llvm.org/D116261

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -20,6 +20,7 @@
 
 template 
 T foo(T argc) {
+  T v = T();
   T c = T();
   T b = T();
   T a = T();
@@ -45,6 +46,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic seq_cst
   a++;
@@ -68,6 +75,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare seq_cst
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare seq_cst capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -91,6 +104,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acq_rel
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acq_rel capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -114,6 +133,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acquire
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acquire
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acquire capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acquire
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic release
   a++;
@@ -137,6 +162,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare release
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture release
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare release capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture release
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic relaxed
   a++;
@@ -160,6 +191,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare relaxed
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare relaxed capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic hint(6)
   a++;
@@ -183,6 +220,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare hint(6)
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare hint(6) capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a == b; if (v) a = c; }
 #endif
   return T();
 }
@@ -215,6 +258,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a > b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a < b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a == b;
+// CHECK-51-NEXT: if (v)
+// CHECK-51-NEXT: a = c;
+// CHECK-51-NEXT: }
 // CHECK-NEXT: #pragma omp atomic seq_cst
 // CHECK-NEXT: a++;
 // CHECK-NEXT: #pragma omp atomic read seq_cst
@@ -242,6 +301,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture seq_cst
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a > b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare seq_cst capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a < b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture seq_cst
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a == b;
+// CHECK-51-NEXT: if (v)
+// CHECK-51-NEXT: a = c;
+// CHECK-51-NEXT: }
 // CHECK-NEXT: #pragma omp atomic
 // CHECK-NEXT: a++;
 // CHECK-NEXT: #pragma omp atomic read
@@ -269,6 +344,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture acq_rel
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a