[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval updated this revision to Diff 270481.
clementval added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Keep same ordering than OMPKinds.def + add dependency


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736

Files:
  clang/lib/Tooling/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/Frontend/Directive/DirectiveBase.td
  llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/lib/Frontend/OpenMP/CMakeLists.txt
  llvm/test/TableGen/directive.td
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/DirectiveEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -90,6 +90,7 @@
 void EmitRegisterBank(RecordKeeper &RK, raw_ostream &OS);
 void EmitExegesis(RecordKeeper &RK, raw_ostream &OS);
 void EmitAutomata(RecordKeeper &RK, raw_ostream &OS);
+void EmitDirectives(RecordKeeper &RK, raw_ostream &OS);
 
 } // End llvm namespace
 
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -54,6 +54,7 @@
   GenRegisterBank,
   GenExegesis,
   GenAutomata,
+  GenDirectives,
 };
 
 namespace llvm {
@@ -128,7 +129,9 @@
"Generate registers bank descriptions"),
 clEnumValN(GenExegesis, "gen-exegesis",
"Generate llvm-exegesis tables"),
-clEnumValN(GenAutomata, "gen-automata", "Generate generic automata")));
+clEnumValN(GenAutomata, "gen-automata", "Generate generic automata"),
+clEnumValN(GenDirectives, "gen-directive-decls",
+   "Generate directive related declaration code")));
 
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
@@ -253,6 +256,8 @@
   case GenAutomata:
 EmitAutomata(Records, OS);
 break;
+  case GenDirectives:
+EmitDirectives(Records, OS);
   }
 
   return false;
Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -0,0 +1,103 @@
+//===- DirectiveEmitter.cpp - Directive Language Emitter --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// DirectiveEmitter uses the descriptions of directives and clauses to construct
+// common code declarations to be used in Frontends.
+//
+//===--===//
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/TableGenBackend.h"
+
+namespace llvm {
+void EmitDirectives(RecordKeeper &Records, raw_ostream &OS) {
+
+  const auto &directiveLanguages =
+  Records.getAllDerivedDefinitions("DirectiveLanguage");
+
+  if (directiveLanguages.size() != 1) {
+PrintError("A single definition of DirectiveLanguage is needed.");
+return;
+  }
+
+  const auto &directiveLanguage = directiveLanguages[0];
+  StringRef directivePrefix =
+  directiveLanguage->getValueAsString("directivePrefix");
+  StringRef clausePrefix = directiveLanguage->getValueAsString("clausePrefix");
+  StringRef cppNamespace = directiveLanguage->getValueAsString("cppNamespace");
+  bool makeEnumAvailableInNamespace =
+  directiveLanguage->getValueAsBit("makeEnumAvailableInNamespace");
+  bool enableBitmaskEnumInNamespace =
+  directiveLanguage->getValueAsBit("enableBitmaskEnumInNamespace");
+
+  if (enableBitmaskEnumInNamespace)
+OS << "#include \"llvm/ADT/BitmaskEnum.h\"\n";
+
+  OS << "namespace llvm {\n";
+
+  // Open namespaces defined in the directive language
+  llvm::SmallVector namespaces;
+  llvm::SplitString(cppNamespace, namespaces, "::");
+  for (auto ns : namespaces)
+OS << "namespace " << ns << " {\n";
+
+  if (enableBitmaskEnumInNamespace)
+OS << "LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();\n";
+
+  // Emit Directive enumeration
+  OS << "enum class Directive {\n";
+  const auto &directives = Records.getAllDerivedDefinitions("Directive");
+  for (const auto &d : directives) {
+const auto name = d->getValueAsString("name");
+std::string n = name.str();
+std::replace(n.begin(), n

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496
+def OMP_EndDeclareVariant : Directive<"end declare variant"> {}
+def OMP_Unknown : Directive<"unknown"> {}

jdoerfert wrote:
> I guess we go over this in a follow up and use the `requiredClauses` and 
> similar features, that seems reasonable to me. 
Yes, that was the plan. I tried to stick to the way it is done today. Then we 
can use the extra abstraction when more things are in place. 



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:50
-#include "llvm/Frontend/OpenMP/OMPKinds.def"
-
 /// IDs for all omp runtime library (RTL) functions.

jdoerfert wrote:
> While the MACRO MAGIC worked surprisingly well, I'm glad it is going away.
I hope we can replace as much as possible the MACRO MAGIC. We might have to 
keep some of it were there is no clear abstraction. Let's see.  



Comment at: llvm/test/TableGen/directive.td:34
+// CHECK-NEXT: }
+// CHECK-NEXT: }

jdoerfert wrote:
> How does the `allowedClauses` affect the result?
> 
It does not affect the result in this case. It will be used in a next patch 
were with can replace the `isAllowedClauseForDirective` code generation with 
TableGen. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D81736#2093926 , @jdoerfert wrote:

> Assuming this passes all the tests, it looks good to me. Let's wait a day or 
> two for people to take a look though :)


@jdoerfert Sure let's wait a bit. I see a failure because of some clang-tidy 
warnings. How strongly should I fix them? I see some conflict with other code I 
see in TableGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D81736#2096336 , @jdoerfert wrote:

> In D81736#2095947 , @clementval 
> wrote:
>
> > In D81736#2093926 , @jdoerfert 
> > wrote:
> >
> > > Assuming this passes all the tests, it looks good to me. Let's wait a day 
> > > or two for people to take a look though :)
> >
> >
> > @jdoerfert Sure let's wait a bit. I see a failure because of some 
> > clang-tidy warnings. How strongly should I fix them? I see some conflict 
> > with other code I see in TableGen.
>
>
> Clang-tidy run by Phab is not binding but if it's reasonable modify it. Keep 
> the TableGen style consistent.


@jdoerfert I made some fix to fit in the TableGen style. So this is ready to 
land on my side.
I'm working on the next patches. One that replace the specific enums in Flang 
to use the one defined here. Another one that continue to replace the MACROS in 
OMPConstants.h/.cpp with TableGen code generation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval marked 10 inline comments as done.
clementval added a comment.

@jdenny I will have a look at your suggestion to dump the name instead of the 
id. If it looks straight forward I will do it. Other comments were addressed. 
Thanks.




Comment at: clang/test/AST/ast-dump-openmp-target-parallel-for.c:105
 // CHECK-NEXT: |   | | `-FieldDecl {{.*}}  col:3 implicit 'int'
-// CHECK-NEXT: |   | |   `-OMPCaptureKindAttr {{.*}} <> 
Implicit 9
+// CHECK-NEXT: |   | |   `-OMPCaptureKindAttr {{.*}} <> 
Implicit 23
 // CHECK-NEXT: |   | `-CapturedDecl {{.*}} <>  
nothrow

jdenny wrote:
> Why 23 not `{{.*}}` like the others?
Good catch!



Comment at: llvm/include/llvm/Frontend/Directive/DirectiveBase.td:22
+  //
+  // By default, uses the name of the dialect as the only namespace. To avoid
+  // placing in any namespace, use "". To specify nested namespaces, use "::"

jdenny wrote:
> "dialect" -> "directive language" as above?
Should be "directive language". copy-paste error.



Comment at: llvm/test/TableGen/directive.td:11
+  let clausePrefix = "TDLC_";
+  let makeEnumAvailableInNamespace = 1;
+}

jdenny wrote:
> Does it make sense to go ahead and test the case where this is 0?  What about 
> testing enableBitmaskEnumInNamespace?
I added a test for that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval accepted this revision.
clementval added a comment.

LGTM. So later the DEPENDS omp_gen that are in clang subdirectories could be 
removed right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@simon_tatham Can you just update the description so it is consistent with your 
fix when it lands?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D82659#2130022 , @simon_tatham 
wrote:

> @jdoerfert , @clementval : over in D83032  
> is a polished-up version of the script I used to check where the missing deps 
> needed to go. Might be useful for the next problem of this kind. But I'm not 
> sure who to get to review it; perhaps one of you might look at it?


No idea who can review this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D82659#2136909 , @michele.scandale 
wrote:

> Why `omp_gen` is now a dependency of `clang-tablegen-targets` rather than 
> being in the `LLVM_COMMON_DEPENDS` list like `clang-tablegen-targets`?
>
> Moreover I've noticed that with the recent changes where  `omp_gen` has been 
> added as a dependency in several libraries, this was done unconditionally 
> breaking the Clang standalone build.
>  For the same issue `intrinsics_gen` is added only if `CLANG_BUILT_STANDALONE 
> ` is false.
>
> At this point I think that something like:
>
>   # All targets below may depend on all tablegen'd files.
>   get_property(CLANG_TABLEGEN_TARGETS GLOBAL PROPERTY CLANG_TABLEGEN_TARGETS)
>   add_custom_target(clang-tablegen-targets DEPENDS ${CLANG_TABLEGEN_TARGETS})
>   set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "Misc")
>   list(APPEND LLVM_COMMON_DEPENDS clang-tablegen-targets)
>   if(NOT CLANG_BUILT_STANDALONE)
> list(APPEND LLVM_COMMON_DEPENDS omg_gen)
>   endif()
>
>
> would fix all the issues, and it would allow removing the explicit 
> dependencies added to each clang library.
>
> Is there any issue with my reasoning?


Looks good but just one question ... When clang is built as standalone it does 
not build the OpenMP part inside Clang? I haven't seen any code to avoid 
compiling the OpenMP parsing and semantic checking inside clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D82659#2138228 , @michele.scandale 
wrote:

> In D82659#2136999 , @clementval 
> wrote:
>
> > Looks good but just one question ... When clang is built as standalone it 
> > does not build the OpenMP part inside Clang? I haven't seen any code to 
> > avoid compiling the OpenMP parsing and semantic checking inside clang.
>
>
> I don't think there is a way to avoid compiling the OpenMP support in Clang. 
> The standalone build is just building the content of the `clang` directory as 
> a separate CMake project reusing the an already built LLVM -- therefore the 
> `libLLVMFrontendOpenMP` as well as the `OMP.h.inc` would have been generated 
> already.


Ok then your fix should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@jdoerfert @jdenny Should we wait until Monday to go ahead with this patch? I 
have several other patches that will follow this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

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

Missing dependencies


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval requested review of this revision.
clementval added a comment.

@jdoerfert @jdenny I had to add a bunch of dependencies so that the file is 
generated correctly when needed. Do you have any feedback on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D81736#2107202 , @jdenny wrote:

> My cmake skills are lacking.  Why are there are so many new DEPENDS 
> relationships where there were none before?  Is it because omp_gen is 
> generating a header file that's included (indirectly) in all those places, 
> where apparently that sort of dependency was unusual?
>
> Have you tried building from a new, empty build directory on your local 
> system?  Can you reproduce the CI fails without this fixup?


Yeah they are needed because of the indirect inclusion of the generated file. I 
could reproduce the failure from an empty build directory. This leads to the 
addition of the different depends in cmake files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2
+set(LLVM_TARGET_DEFINITIONS OMP.td)
+tablegen(LLVM OMP.h.inc --gen-directive-decls)
+add_public_tablegen_target(omp_gen)

thakis wrote:
> All other tblgen outputs are called .inc, not .h.inc. Any reason this one's 
> different?
There is a `.cpp.inc` coming in a following patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2
+set(LLVM_TARGET_DEFINITIONS OMP.td)
+tablegen(LLVM OMP.h.inc --gen-directive-decls)
+add_public_tablegen_target(omp_gen)

thakis wrote:
> jdoerfert wrote:
> > clementval wrote:
> > > thakis wrote:
> > > > All other tblgen outputs are called .inc, not .h.inc. Any reason this 
> > > > one's different?
> > > There is a `.cpp.inc` coming in a following patch. 
> > @clementval ^ 
> ...why would you want to include a cpp file?
> 
> If it's for definitions of generated functions, I think the usual pattern is 
> to put that in the .inc too behind a define and define that in one cpp file 
> that includes the .inc. (Examples: GET_DAGISEL_BODY, GET_INSTRINFO_MC_DESC, 
> PRINT_ALIAS_INSTR etc -- `rg -B5 '#include.*\.inc' clang llvm` shows many 
> examples).
Yeah this was the idea. I was following the same pattern as MLIR use of 
TableGen. I'm happy with a single `.inc` file with define. If you don't mind 
I'll update the filename in the next patch since this one  has landed already. 

FYI MLIR TableGen .cpp.inc -> 
https://github.com/llvm/llvm-project/blob/master/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp#L42


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D82518: [openmp] Use Directive_enumSize instead of OMPD_unknown position

[Valentin Clement via Phabricator via cfe-commits]

clementval created this revision.
clementval added reviewers: vdmitrie, jdoerfert, jdenny.
Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl.
Herald added a project: clang.

Previously OMPD_unknown was last item in the Directive enumeration and its 
position was
used in various comparison and assertion. With the new Directive enumeration, 
this should be
change with  llvm::omp::Directive_enumSize. This patch fix two place where it 
was not done in
D81736 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82518

Files:
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return DKind < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(DKind <= llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return DKind < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(DKind <= llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseOpenMP.cpp:197
   }
   return DKind < OMPD_unknown ? static_cast(DKind)
   : OMPD_unknown;

vdmitrie wrote:
> Should this be a comparison against llvm::omp::Directive_enumSize rather than 
> OMPD_unknown?
> 
> 
> And there is an assertion in file clang/lib/Basic/OpenMPKinds.cpp
> that I guess needs to be updated the same way:
> void clang::getOpenMPCaptureRegions(
> SmallVectorImpl &CaptureRegions,
> OpenMPDirectiveKind DKind) {
>   assert(DKind <= OMPD_unknown);
> 
Good catch thx. I just created a patch to fix this D82518



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D82518: [openmp] Use Directive_enumSize instead of OMPD_unknown position

[Valentin Clement via Phabricator via cfe-commits]

clementval updated this revision to Diff 273216.
clementval added a comment.

add unsigned cast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82518

Files:
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return unsigned(DKind) < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) <= llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return unsigned(DKind) < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) <= llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82518: [openmp] Use Directive_enumSize instead of OMPD_unknown position

[Valentin Clement via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/OpenMPKinds.cpp:583
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) <= llvm::omp::Directive_enumSize);
   switch (DKind) {

jdoerfert wrote:
> `<`, right?
You are right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82518



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

[PATCH] D82518: [openmp] Use Directive_enumSize instead of OMPD_unknown position

[Valentin Clement via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b9ce07a761f: [openmp] Use Directive_enumSize instead of 
OMPD_unknown position (authored by clementval).

Changed prior to commit:
  https://reviews.llvm.org/D82518?vs=273216&id=273327#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82518

Files:
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return unsigned(DKind) < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) < llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -194,8 +194,9 @@
   DKind = F[I][2];
 }
   }
-  return DKind < OMPD_unknown ? static_cast(DKind)
-  : OMPD_unknown;
+  return unsigned(DKind) < llvm::omp::Directive_enumSize
+ ? static_cast(DKind)
+ : OMPD_unknown;
 }
 
 static DeclarationName parseOpenMPReductionId(Parser &P) {
Index: clang/lib/Basic/OpenMPKinds.cpp
===
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -580,7 +580,7 @@
 void clang::getOpenMPCaptureRegions(
 SmallVectorImpl &CaptureRegions,
 OpenMPDirectiveKind DKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) < llvm::omp::Directive_enumSize);
   switch (DKind) {
   case OMPD_parallel:
   case OMPD_parallel_for:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via cfe-commits]

clementval marked an inline comment as done.
clementval added a comment.

In D81736#2117858 , @thakis wrote:

> This seems to cause build errors sometimes when buildling libclang. Maybe 
> there are more missing dependencies? Example failure: 
> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8876400305251533648/+/steps/gclient_runhooks/0/stdout


Could this patch solve your problem? https://reviews.llvm.org/D82659


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736



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

[PATCH] D81098: [OpenMP] Upgrade default version of OpenMP to 5.0

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D81098#2112159 , @ABataev wrote:

> LG


Since this revision landed two tests are failing.

` libomp.env::kmp_set_dispatch_buf.c` and 
`libomp.worksharing/for::kmp_set_dispatch_buf.c`. It was also reported on the 
mailing list 
(http://lists.llvm.org/pipermail/openmp-dev/2020-June/003507.html). Any idea 
how we can fix this quickly? @jdoerfert
 OR maybe this is known and will be taken care later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81098



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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:185
   {"flang", "--driver-mode=flang"},
+  {"flang-new", "--driver-mode=flang"},
   {"clang-dxc", "--driver-mode=dxc"},

Why do we need two lines here? Shouldn't we have a single line with the name 
chosen by the cmake option?



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:132
   const auto& D = C.getDriver();
-  // TODO: Replace flang-new with flang once the new driver replaces the
-  // throwaway driver
-  const char *Exec = Args.MakeArgString(D.GetProgramPath("flang-new", TC));
+  // Get the name of this executable. The `getClangProgramPath` hook  predates
+  // Flang, hence the name assumes that it's a Clang program. In practice, it





Comment at: flang/CMakeLists.txt:411
 
+option(FLANG_USE_LEGACY_NAME "Use the legacy name for the Flang driver." ON)
+

Maybe add a comment that says what is the legacy name.



Comment at: flang/test/lit.cfg.py:88
+ToolSubst('%flang', command=FindTool(driver_name), unresolved='fatal'),
+ToolSubst('%flang_fc1', command=FindTool(driver_name), extra_args=['-fc1'],
 unresolved='fatal')]

Is this the correct indentation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105584#3617456 , 
@abidmalikwaterloo wrote:

>   // CHECK-LABEL: omp_DistributeOp
>   func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, 
> %data_var : memref, %chunk_var : i32) -> () {
>  // CHECK: omp.DistributeOp collapse(2)
> "omp.DistributeOp" (%lb, %ub, %step) ({
>   ^bb0(%iv: index):
>omp.yield
> }) {operand_segment_sizes = dense<[1,1,1,0,0]> : vector<5xi32>, 
> collapse_val = 2} :
>   (index, index, index) -> ()
>
>return
>}
>
> Is this a valid test case for the operation?

Should it be `"omp.distribute" (%lb, %ub, %step) ({`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:215
+LogicalResult DistributeOp::verify(){
+  if (this->lowerBound().empty()) {
+return emitOpError() << "empty lowerbound for distribute loop operation";

No brace - 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:219
+  
+  if (this->upperBound().empty()) {
+return emitOpError() << "empty upperbound for distribute loop operation";

no brace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Overall the patch looks ok from a technical point. Shouldn't we just wait until 
we can make the permanent renaming so we do not add unnecessary cmake option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Valentin Clement via Phabricator via cfe-commits]

clementval requested changes to this revision.
clementval added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/ToolChain.cpp:185
   {"flang", "--driver-mode=flang"},
+  {"flang-new", "--driver-mode=flang"},
   {"clang-dxc", "--driver-mode=dxc"},

This is counter intuitive. We rename flang-new but we add flang-new here. It 
should be configurable. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:185
   {"flang", "--driver-mode=flang"},
+  {"flang-new", "--driver-mode=flang"},
   {"clang-dxc", "--driver-mode=dxc"},

awarzynski wrote:
> richard.barton.arm wrote:
> > clementval wrote:
> > > This is counter intuitive. We rename flang-new but we add flang-new here. 
> > > It should be configurable. 
> > This is a list of all the valid prefixes of binaries that the driver 
> > supports. With this change, an additional one will be supported so I don't 
> > think it's an issue to have both flang and flang-new here.
> > 
> > The thing that confuses me is how flang-new works today without this 
> > change, given that "flang" is not a suffix of "flang-new"? @awarzynski , do 
> > you know why that is? Perhaps the line here is not needed at all? Or is 
> > this a bug fix for flang-new that is actually unrelated to this change?
> > This is counter intuitive. 
> 
> I can add a comment to clarify this.
> 
> > It should be configurable.
> 
> It is, in Flang's [[ 
> https://github.com/llvm/llvm-project/blob/main/flang/tools/flang-driver/driver.cpp
>  | driver.cpp ]]. Originally, the suffix was hard-coded as:
> ```
> clang::driver::ParsedClangName targetandMode("flang", "--driver-mode=flang");
> ```
> (i.e. the `clangDriver` library used `flang` internally despite the actual 
> name being `flang-new`). This is now being replaced with (see in 
> "driver.cpp"):
> ```
>  clang::driver::ParsedClangName targetandMode =
>   clang::driver::ToolChain::getTargetAndModeFromProgramName(argv[0]);
> ```
> 
> But the change in "driver.cpp" means that we can no longer make assumptions 
> about the suffix and hence the update here. 
> 
> Like I mentioned earlier, we should not make this file build-time 
> configurable. One possible option would be to try to update Clang's [[ 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Config/config.h.cmake
>  | config.h.cmake ]], but that's would lead to more Flang-specific changes in 
> Clang's core set-up. Also, I'm not convinced it would work here. 
> 
> > @awarzynski , do you know why that is? 
> 
> Yeah, check Flang's "driver.cpp". We should've captured this earlier. The 
> original set-up from ToolChain.cpp predates `flang-new`. And then in 
> "driver.cpp" we just matched what was here. There was a lot of confusion 
> around naming  back then and this has slipped in. 
> 
> > This is counter intuitive. 
> 
> I can add a comment to clarify this.
> 
There should be at least a comment here. 


> Like I mentioned earlier, we should not make this file build-time 
> configurable. One possible option would be to try to update Clang's [[ 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Config/config.h.cmake
>  | config.h.cmake ]], but that's would lead to more Flang-specific changes in 
> Clang's core set-up. Also, I'm not convinced it would work here. 
> 

If it cannot be configurable I think this is a good reason to wait a bit and 
make a direct renaming without all the options and duplicate. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D125788#3621744 , @awarzynski 
wrote:

> We discussed this in our call on Monday and agreed to go ahead provided that 
> this change is technically sound. IIUC, this has now been confirmed:
>
>> Overall the patch looks ok from a technical point.
>
> As always, please correct me if I missed or misinterpreted something!

I still have reserved on this patch so please do not take my "Overall the patch 
looks ok from a technical point." as an approval. There are open discussion so 
wait for other to confirm or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: clang/lib/Testing/CMakeLists.txt:30
+  clangBasic
+  clangFrontend
   )

You still have these changes that are unrelated. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@pengfei We are also hitting the following assertion with this patch. Do you 
have any idea why?

  /llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4333: void 
{anonymous}::SelectionDAGLegalize::ConvertNodeToLibcall(llvm::SDNode*): 
Assertion `cast(Node->getOperand(IsStrict ? 2 : 1))->isZero() 
&& "Unable to expand as libcall if it is not normal rounding"' failed.

LLVM IR triggering the assertion.

  ; ModuleID = 'FIRModule'
  source_filename = "FIRModule"
  target triple = "x86_64-unknown-linux-gnu"
  
  @_QMhp237Ea11 = global half 0xH3D00
  @_QMhp237Eb1a = global half 0xH5640
  @_QMf90_kindECascii = external constant i32
  @_QMf90_kindECbyte = external constant i32
  @_QMf90_kindECdouble = external constant i32
  @_QMiso_fortran_envECint16 = external constant i32
  @_QMiso_fortran_envECint32 = external constant i32
  @_QMiso_fortran_envECint64 = external constant i32
  @_QMiso_fortran_envECint8 = external constant i32
  @_QMf90_kindECjis = external constant i32
  @_QMiso_fortran_envEClogical16 = external constant i32
  @_QMiso_fortran_envEClogical32 = external constant i32
  @_QMiso_fortran_envEClogical64 = external constant i32
  @_QMiso_fortran_envEClogical8 = external constant i32
  @_QMf90_kindECnot_available = external constant i32
  @_QMf90_kindECquad = external constant i32
  @_QMiso_fortran_envECreal128 = external constant i32
  @_QMf90_kindECreal16 = external constant i32
  @_QMiso_fortran_envECreal32 = external constant i32
  @_QMiso_fortran_envECreal64 = external constant i32
  @_QMf90_kindECreal64x2 = external constant i32
  @_QMf90_kindECsingle = external constant i32
  @_QMf90_kindECtwobyte = external constant i32
  @_QMf90_kindECucs2 = external constant i32
  @_QMf90_kindECucs4 = external constant i32
  @_QMf90_kindECword = external constant i32
  @_QQcl.2E2F627567312E66393000 = linkonce constant [11 x i8] c"./bug1.f90\00"
  @_QQcl.2831362C313629 = linkonce constant [7 x i8] c"(16,16)"
  @_QQcl.28346631302E3329 = linkonce constant [8 x i8] c"(4f10.3)"
  
  declare ptr @malloc(i64)
  
  declare void @free(ptr)
  
  define void @_QQmain() !dbg !3 {
%1 = alloca { ptr, i64, i32, i8, i8, i8, i8 }, align 8, !dbg !7
%2 = alloca half, i64 1, align 2, !dbg !9
%3 = call ptr @_FortranAioBeginExternalListOutput(i32 -1, ptr 
@_QQcl.2E2F627567312E66393000, i32 9), !dbg !10
%4 = call i1 @_FortranAioOutputAscii(ptr %3, ptr @_QQcl.2831362C313629, i64 
7), !dbg !11
%5 = call i32 @_FortranAioEndIoStatement(ptr %3), !dbg !12
%6 = call ptr @_FortranAioBeginExternalFormattedOutput(ptr 
@_QQcl.28346631302E3329, i64 8, i32 -1, ptr @_QQcl.2E2F627567312E66393000, i32 
10), !dbg !13
%7 = load half, ptr @_QMhp237Ea11, align 2, !dbg !14
%8 = load half, ptr @_QMhp237Eb1a, align 2, !dbg !15
%9 = fpext half %7 to float, !dbg !16
%10 = fpext half %8 to float, !dbg !17
%11 = call float @llvm.copysign.f32(float %9, float %10), !dbg !18
%12 = fptrunc float %11 to half, !dbg !19
store half %12, ptr %2, align 2, !dbg !20
%13 = insertvalue { ptr, i64, i32, i8, i8, i8, i8 } { ptr undef, i64 2, i32 
20180515, i8 0, i8 25, i8 0, i8 0 }, ptr %2, 0, !dbg !7
store { ptr, i64, i32, i8, i8, i8, i8 } %13, ptr %1, align 8, !dbg !7
%14 = call i1 @_FortranAioOutputDescriptor(ptr %6, ptr %1), !dbg !21
%15 = call i32 @_FortranAioEndIoStatement(ptr %6), !dbg !22
ret void, !dbg !23
  }
  
  declare ptr @_FortranAioBeginExternalListOutput(i32, ptr, i32)
  
  declare i1 @_FortranAioOutputAscii(ptr, ptr, i64)
  
  declare i32 @_FortranAioEndIoStatement(ptr)
  
  declare ptr @_FortranAioBeginExternalFormattedOutput(ptr, i64, i32, ptr, i32)
  
  declare i1 @_FortranAioOutputDescriptor(ptr, ptr)
  
  ; Function Attrs: nocallback nofree nosync nounwind readnone speculatable 
willreturn
  declare float @llvm.copysign.f32(float, float) #0
  
  attributes #0 = { nocallback nofree nosync nounwind readnone speculatable 
willreturn }
  
  !llvm.dbg.cu = !{!0}
  !llvm.module.flags = !{!2}
  
  !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "mlir", 
isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
  !1 = !DIFile(filename: "FIRModule", directory: "/")
  !2 = !{i32 2, !"Debug Info Version", i32 3}
  !3 = distinct !DISubprogram(name: "_QQmain", linkageName: "_QQmain", scope: 
null, file: !4, line: 9, type: !5, scopeLine: 9, spFlags: DISPFlagDefinition | 
DISPFlagOptimized, unit: !0, retainedNodes: !6)
  !4 = !DIFile(filename: "", directory: 
"/local/home/vclement/llvm-project/build")
  !5 = !DISubroutineType(types: !6)
  !6 = !{}
  !7 = !DILocation(line: 39, column: 9, scope: !8)
  !8 = !DILexicalBlockFile(scope: !3, file: !4, discriminator: 0)
  !9 = !DILocation(line: 10, column: 8, scope: !8)
  !10 = !DILocation(line: 17, column: 8, scope: !8)
  !11 = !DILocation(line: 22, column: 8, scope: !8)
  !12 = !DILocation(line: 23, column: 9, scope: !8)
  !13 = !DILocation(line: 31, column: 9, scope: !8)
  !14 = !DILoc

[PATCH] D122008: [flang][driver] Add support for generating executables

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D122008#3426847 , @awarzynski 
wrote:

> We discussed this patch in our community call today and some people expressed 
> their reservations about merging this just now. As I mentioned, I would like 
> to have this merged to unblock the CMake PR 
> . As promised, 
> here are the options that we've considered:
>
> 1. merge this patch "today",
> 2. merge this patch "today", but add a flag so that by default `flang-new 
> file.f90` wouldn't generate an executable (we would remove this flag at a 
> later time),
> 3. wait until the upstreaming of fir-dev is complete (based on today's 
> update, I think that that would be in June/July the earliest).
>
> Could you tell me what your preference is?
>
> Please keep in mind that even once CMake support is available, it's going to 
> land in the "latest" version of CMake. Folks will have to download the latest 
> binaries from https://github.com/Kitware/CMake/releases in order to benefit 
> from this.  With time, CMake shipped with various OSes will catch-up and 
> become "modern" enough. But there's going to be a delay and hence it makes 
> sense to get the CMake support sooner rather than later.
>
> Please comment if I missed or misinterpreted anything. Also, I will add more 
> reviewers - basically everyone who discussed this in the call today. Please 
> add anyone that I've missed!
>
> Thanks for taking a look!

I'll personally in favor of number 3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D130822: Fixed loads of typos

[Valentin Clement via Phabricator via cfe-commits]

clementval requested changes to this revision.
clementval added inline comments.
This revision now requires changes to proceed.



Comment at: flang/docs/Extensions.md:157
   with each other.
-* Values for whole anonymous parent components in structure constructors
   (e.g., `EXTENDEDTYPE(PARENTTYPE(1,2,3))` rather than `EXTENDEDTYPE(1,2,3)`

This is just wrong


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130822

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105255#3707495 , @raghavendhra 
wrote:

> In D105255#3707088 , 
> @abidmalikwaterloo wrote:
>
>> In D105255#3706602 , @raghavendhra 
>> wrote:
>>
>>> @abidmalikwaterloo Can you please check your last patch pushed for review? 
>>> https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can 
>>> not apply your patch something to do with the usage of diff --update 
>>> ? Can you please check?
>>
>> @raghavendhra  `I tried to correct it but could not figure out the solution. 
>> Do you have any? I am not experienced with the phabricator framework.  One 
>> solution is to submit another patch with all changes till today and abandon 
>> this one.`
>
> Sorry, I am new to phabricator as well. I use "arc diff" to upload my 
> revisions for review after addressing review comments. If no one responds 
> probably I am fine with new review and abandoning this one.

Did you try `arc diff --update D105255 ` from your branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105255#3708258 , 
@abidmalikwaterloo wrote:

> In D105255#3707533 , @clementval 
> wrote:
>
>> In D105255#3707495 , @raghavendhra 
>> wrote:
>>
>>> In D105255#3707088 , 
>>> @abidmalikwaterloo wrote:
>>>
 In D105255#3706602 , 
 @raghavendhra wrote:

> @abidmalikwaterloo Can you please check your last patch pushed for 
> review? https://buildkite.com/llvm-project/diff-checks/builds/117796 
> states it can not apply your patch something to do with the usage of diff 
> --update ? Can you please check?

 @raghavendhra  `I tried to correct it but could not figure out the 
 solution. Do you have any? I am not experienced with the phabricator 
 framework.  One solution is to submit another patch with all changes till 
 today and abandon this one.`
>>>
>>> Sorry, I am new to phabricator as well. I use "arc diff" to upload my 
>>> revisions for review after addressing review comments. If no one responds 
>>> probably I am fine with new review and abandoning this one.
>>
>> Did you try `arc diff --update D105255 ` from your branch?
>
> `yes, I tried this. Actually,  I updated the patch. through this`

The problem is likely that the commit before the commit of this patch is not in 
tree,

The easiest way is to create a new branch from main and cherry-pick the commit 
you need for this patch on the top of it. Then you just use arc diff --update 
and it should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Do you plan to discuss this again at during the next call? Note that today is a 
holiday in various country in Europe (maybe elsewhere too) so the one on 4/27 
is probably better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449
  ParentOneOf<["WsLoopOp", "ReductionDeclareOp", 
- "AtomicUpdateOp", "SimdLoopOp"]>]> {
+ "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> {
   let summary = "loop yield and termination operation";




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Why not adding the assemblyFormat directly for these operations like your patch 
D105584 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval created this revision.
clementval added reviewers: jdoerfert, jdenny, kiranchandramohan.
Herald added subscribers: jfb, arphaman, guansong, yaxunl.
clementval requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

Remove the OpenMP clause information from the OMPKinds.def file and use the
information from the new OMP.td file. There is now a single source of truth for 
the 
directives and clauses.

To avoid generate lots of specific small code from tablegen, the macros 
previously
used in OMPKinds.def are generated almost as identical. This can be polished and
possibly removed in a further patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92955

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/include/llvm/TableGen/DirectiveEmitter.h
  llvm/test/TableGen/directive2.td
  llvm/utils/TableGen/DirectiveEmitter.cpp

Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- llvm/utils/TableGen/DirectiveEmitter.cpp
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -645,6 +645,72 @@
   GenerateFlangClauseUnparse(DirLang, OS);
 }
 
+void GenerateClauseClassMacro(const DirectiveLanguage &DirLang,
+  raw_ostream &OS) {
+  // Generate macros style information for legacy code in clang
+  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+
+  OS << "\n";
+
+  OS << "#ifndef CLAUSE\n";
+  OS << "#define CLAUSE(Enum, Str, Implicit)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_CLASS\n";
+  OS << "#define CLAUSE_CLASS(Enum, Str, Class)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_NO_CLASS\n";
+  OS << "#define CLAUSE_NO_CLASS(Enum, Str)\n";
+  OS << "#endif\n";
+  OS << "\n";
+  OS << "#define __CLAUSE(Name, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, #Name, Class)\n";
+  OS << "#define __CLAUSE_NO_CLASS(Name)\\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, #Name)\n";
+  OS << "#define __IMPLICIT_CLAUSE_CLASS(Name, Str, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, Str, Class)\n";
+  OS << "#define __IMPLICIT_CLAUSE_NO_CLASS(Name, Str)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, Str)\n";
+  OS << "\n";
+
+  for (const auto &R : DirLang.getClauses()) {
+Clause C{R};
+if (C.getClangClass().empty()) { // NO_CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_NO_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\")\n";
+  } else {
+OS << "__CLAUSE_NO_CLASS(" << C.getFormattedName() << ")\n";
+  }
+} else { // CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\", " << C.getClangClass() << ")\n";
+  } else {
+OS << "__CLAUSE(" << C.getFormattedName() << ", " << C.getClangClass()
+   << ")\n";
+  }
+}
+  }
+
+  OS << "\n";
+  OS << "#undef __IMPLICIT_CLAUSE_NO_CLASS\n";
+  OS << "#undef __IMPLICIT_CLAUSE_CLASS\n";
+  OS << "#undef __CLAUSE\n";
+  OS << "#undef CLAUSE_NO_CLASS\n";
+  OS << "#undef CLAUSE_CLASS\n";
+  OS << "#undef CLAUSE\n";
+}
+
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
@@ -653,6 +719,8 @@
 return;
 
   EmitDirectivesFlangImpl(DirLang, OS);
+
+  GenerateClauseClassMacro(DirLang, OS);
 }
 
 // Generate the implemenation for the enumeration in the directive
Index: llvm/test/TableGen/directive2.td
===
--- llvm/test/TableGen/directive2.td
+++ llvm/test/TableGen/directive2.td
@@ -23,10 +23,15 @@
   let isValueList = 1;
 }
 def TDLC_ClauseC : Clause<"clausec"> {
+  let clan

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval updated this revision to Diff 310589.
clementval added a comment.

Remove useless define macro


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/include/llvm/TableGen/DirectiveEmitter.h
  llvm/test/TableGen/directive2.td
  llvm/utils/TableGen/DirectiveEmitter.cpp

Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- llvm/utils/TableGen/DirectiveEmitter.cpp
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -645,6 +645,72 @@
   GenerateFlangClauseUnparse(DirLang, OS);
 }
 
+void GenerateClauseClassMacro(const DirectiveLanguage &DirLang,
+  raw_ostream &OS) {
+  // Generate macros style information for legacy code in clang
+  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+
+  OS << "\n";
+
+  OS << "#ifndef CLAUSE\n";
+  OS << "#define CLAUSE(Enum, Str, Implicit)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_CLASS\n";
+  OS << "#define CLAUSE_CLASS(Enum, Str, Class)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_NO_CLASS\n";
+  OS << "#define CLAUSE_NO_CLASS(Enum, Str)\n";
+  OS << "#endif\n";
+  OS << "\n";
+  OS << "#define __CLAUSE(Name, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, #Name, Class)\n";
+  OS << "#define __CLAUSE_NO_CLASS(Name)\\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, #Name)\n";
+  OS << "#define __IMPLICIT_CLAUSE_CLASS(Name, Str, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, Str, Class)\n";
+  OS << "#define __IMPLICIT_CLAUSE_NO_CLASS(Name, Str)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, Str)\n";
+  OS << "\n";
+
+  for (const auto &R : DirLang.getClauses()) {
+Clause C{R};
+if (C.getClangClass().empty()) { // NO_CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_NO_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\")\n";
+  } else {
+OS << "__CLAUSE_NO_CLASS(" << C.getFormattedName() << ")\n";
+  }
+} else { // CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\", " << C.getClangClass() << ")\n";
+  } else {
+OS << "__CLAUSE(" << C.getFormattedName() << ", " << C.getClangClass()
+   << ")\n";
+  }
+}
+  }
+
+  OS << "\n";
+  OS << "#undef __IMPLICIT_CLAUSE_NO_CLASS\n";
+  OS << "#undef __IMPLICIT_CLAUSE_CLASS\n";
+  OS << "#undef __CLAUSE\n";
+  OS << "#undef CLAUSE_NO_CLASS\n";
+  OS << "#undef CLAUSE_CLASS\n";
+  OS << "#undef CLAUSE\n";
+}
+
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
@@ -653,6 +719,8 @@
 return;
 
   EmitDirectivesFlangImpl(DirLang, OS);
+
+  GenerateClauseClassMacro(DirLang, OS);
 }
 
 // Generate the implemenation for the enumeration in the directive
Index: llvm/test/TableGen/directive2.td
===
--- llvm/test/TableGen/directive2.td
+++ llvm/test/TableGen/directive2.td
@@ -23,10 +23,15 @@
   let isValueList = 1;
 }
 def TDLC_ClauseC : Clause<"clausec"> {
+  let clangClass = "ClauseC";
   let flangClassValue = "Name";
   let defaultValue = "*";
   let isValueOptional = 1;
 }
+def TDLC_ClauseD : Clause<"claused"> {
+  let clangClass = "ClauseD";
+  let isImplicit = 1;
+}
 
 def TDL_DirA : Directive<"dira"> {
   let allowedClauses = [
@@ -53,9 +58,10 @@
 // CHECK-NEXT:TDLC_clausea,
 // CHECK-NEXT:TDLC_clauseb,
 // CHECK-NEXT:TDLC_clausec,
+// CHECK-NEXT:TDLC_claused,
 // CHECK-NEXT:  };
 // CHECK-EMPTY:
-// CHECK-NEXT:  static constexpr std::size_t Clause_enumSize = 3;
+// CHECK-NEXT:  static constexpr std::size_t 

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D92955#2443313 , @jdoerfert wrote:

> I like this a lot. We might even look into generating the clang OpenMPClause 
> classes via TableGen later, thanks a lot.
>
> There is a file missing, right? The list of actual clauses.

The list of actual clauses is already in and used by Flang 
(`llvm/include/llvm/Frontend/OpenMP/OMP.td`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement 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 rGa7b2847216b4: [openmp] Remove clause from OMPKinds.def and 
use OMP.td info (authored by clementval).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/include/llvm/TableGen/DirectiveEmitter.h
  llvm/test/TableGen/directive2.td
  llvm/utils/TableGen/DirectiveEmitter.cpp

Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- llvm/utils/TableGen/DirectiveEmitter.cpp
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -645,6 +645,72 @@
   GenerateFlangClauseUnparse(DirLang, OS);
 }
 
+void GenerateClauseClassMacro(const DirectiveLanguage &DirLang,
+  raw_ostream &OS) {
+  // Generate macros style information for legacy code in clang
+  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+
+  OS << "\n";
+
+  OS << "#ifndef CLAUSE\n";
+  OS << "#define CLAUSE(Enum, Str, Implicit)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_CLASS\n";
+  OS << "#define CLAUSE_CLASS(Enum, Str, Class)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_NO_CLASS\n";
+  OS << "#define CLAUSE_NO_CLASS(Enum, Str)\n";
+  OS << "#endif\n";
+  OS << "\n";
+  OS << "#define __CLAUSE(Name, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, #Name, Class)\n";
+  OS << "#define __CLAUSE_NO_CLASS(Name)\\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, #Name)\n";
+  OS << "#define __IMPLICIT_CLAUSE_CLASS(Name, Str, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, Str, Class)\n";
+  OS << "#define __IMPLICIT_CLAUSE_NO_CLASS(Name, Str)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, Str)\n";
+  OS << "\n";
+
+  for (const auto &R : DirLang.getClauses()) {
+Clause C{R};
+if (C.getClangClass().empty()) { // NO_CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_NO_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\")\n";
+  } else {
+OS << "__CLAUSE_NO_CLASS(" << C.getFormattedName() << ")\n";
+  }
+} else { // CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\", " << C.getClangClass() << ")\n";
+  } else {
+OS << "__CLAUSE(" << C.getFormattedName() << ", " << C.getClangClass()
+   << ")\n";
+  }
+}
+  }
+
+  OS << "\n";
+  OS << "#undef __IMPLICIT_CLAUSE_NO_CLASS\n";
+  OS << "#undef __IMPLICIT_CLAUSE_CLASS\n";
+  OS << "#undef __CLAUSE\n";
+  OS << "#undef CLAUSE_NO_CLASS\n";
+  OS << "#undef CLAUSE_CLASS\n";
+  OS << "#undef CLAUSE\n";
+}
+
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
@@ -653,6 +719,8 @@
 return;
 
   EmitDirectivesFlangImpl(DirLang, OS);
+
+  GenerateClauseClassMacro(DirLang, OS);
 }
 
 // Generate the implemenation for the enumeration in the directive
Index: llvm/test/TableGen/directive2.td
===
--- llvm/test/TableGen/directive2.td
+++ llvm/test/TableGen/directive2.td
@@ -23,10 +23,15 @@
   let isValueList = 1;
 }
 def TDLC_ClauseC : Clause<"clausec"> {
+  let clangClass = "ClauseC";
   let flangClassValue = "Name";
   let defaultValue = "*";
   let isValueOptional = 1;
 }
+def TDLC_ClauseD : Clause<"claused"> {
+  let clangClass = "ClauseD";
+  let isImplicit = 1;
+}
 
 def TDL_DirA : Directive<"dira"> {
   let allowedClauses = [
@@ -53,9 +58,10 @@
 // CHECK-NEXT:TDLC_clausea,
 // CHECK-NEXT:TDLC_clauseb,
 // CHECK-NEXT:TDLC_clausec,
+// CHECK-NEXT:TDLC_claused,
 // CH

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D92955#2445815 , @thakis wrote:

> And a less philosophical comment: This causes a bunch of 
> Wcovered-switch-default warnings:
>
> ../../clang/include/clang/AST/OpenMPClause.h::5: warning: default label 
> in switch which covers all enumeration values [-Wcovered-switch-default]
> ../../clang/include/clang/AST/RecursiveASTVisitor.h:2962:3: warning: default 
> label in switch which covers all enumeration values [-Wcovered-switch-default]
>
> (and possibly more).

Yeah just saw that and reverted the patch. I'll rework a bit the patch to 
include your suggestion from D83636 . MLIR is 
doing it this way so I'm fine with doing it the other way as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval updated this revision to Diff 310980.
clementval added a comment.
Herald added a subscriber: mgorny.

Fix -Werror=covered-switch-default problems + rename OMP.cpp.inc to OMP.inc 
since the .cpp does
not make sense anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndex.cpp
  flang/include/flang/Parser/dump-parse-tree.h
  flang/include/flang/Parser/parse-tree.h
  flang/lib/Parser/unparse.cpp
  flang/lib/Semantics/check-omp-structure.h
  llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/include/llvm/TableGen/DirectiveEmitter.h
  llvm/test/TableGen/directive2.td
  llvm/utils/TableGen/DirectiveEmitter.cpp

Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- llvm/utils/TableGen/DirectiveEmitter.cpp
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -645,6 +645,72 @@
   GenerateFlangClauseUnparse(DirLang, OS);
 }
 
+void GenerateClauseClassMacro(const DirectiveLanguage &DirLang,
+  raw_ostream &OS) {
+  // Generate macros style information for legacy code in clang
+  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+
+  OS << "\n";
+
+  OS << "#ifndef CLAUSE\n";
+  OS << "#define CLAUSE(Enum, Str, Implicit)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_CLASS\n";
+  OS << "#define CLAUSE_CLASS(Enum, Str, Class)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_NO_CLASS\n";
+  OS << "#define CLAUSE_NO_CLASS(Enum, Str)\n";
+  OS << "#endif\n";
+  OS << "\n";
+  OS << "#define __CLAUSE(Name, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, #Name, Class)\n";
+  OS << "#define __CLAUSE_NO_CLASS(Name)\\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, #Name)\n";
+  OS << "#define __IMPLICIT_CLAUSE_CLASS(Name, Str, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, Str, Class)\n";
+  OS << "#define __IMPLICIT_CLAUSE_NO_CLASS(Name, Str)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, Str)\n";
+  OS << "\n";
+
+  for (const auto &R : DirLang.getClauses()) {
+Clause C{R};
+if (C.getClangClass().empty()) { // NO_CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_NO_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\")\n";
+  } else {
+OS << "__CLAUSE_NO_CLASS(" << C.getFormattedName() << ")\n";
+  }
+} else { // CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\", " << C.getClangClass() << ")\n";
+  } else {
+OS << "__CLAUSE(" << C.getFormattedName() << ", " << C.getClangClass()
+   << ")\n";
+  }
+}
+  }
+
+  OS << "\n";
+  OS << "#undef __IMPLICIT_CLAUSE_NO_CLASS\n";
+  OS << "#undef __IMPLICIT_CLAUSE_CLASS\n";
+  OS << "#undef __CLAUSE\n";
+  OS << "#undef CLAUSE_NO_CLASS\n";
+  OS << "#undef CLAUSE_CLASS\n";
+  OS << "#undef CLAUSE\n";
+}
+
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
@@ -653,6 +719,8 @@
 return;
 
   EmitDirectivesFlangImpl(DirLang, OS);
+
+  GenerateClauseClassMacro(DirLang, OS);
 }
 
 // Generate the implemenation for the enumeration in the directive
Index: llvm/test/TableGen/directive2.td
===
--- llvm/test/TableGen/directive2.td
+++ llvm/test/TableGen/directive2.td
@@ -23,10 +23,15 @@
   let isValueList = 1;
 }
 def TDLC_ClauseC : Clause<"clausec"> {
+  let clangClass = "ClauseC";
   let flangClassValue = "Name";
   let defaultValue = "*";
   let isValueOptional = 1;
 }
+def TDLC_ClauseD : Clause<"claused"> {
+  let clangClass = "ClauseD";
+  let isImplicit = 1;
+}
 
 def TDL_DirA : Dire

[PATCH] D93301: [flang][driver] Add support for `-c` and `-emit-obj`

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4629
 
-} // let Flags = [CC1Option]
+} // let Flags = [CC1Option, NoDriverOption]
+

Is it intended to modify the SYCL options? 



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:104
   // TODO:
   // case clang::driver::options::OPT_emit_obj:
   // case calng::driver::options::OPT_emit_llvm:

Should you remove this line since you have the case for it?



Comment at: flang/test/Flang-Driver/code-gen.f90:11
+! placeholder and running them leads to a driver error. This test makes sure
+! that these actions are indeed run (rather then `-c` or `-emit-obj` being
+! rejected earlier).

than instead of then? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93301

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Ping since there was a phab email delivery problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

[Valentin Clement 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 rGf4c8b8031800: [openmp] Remove clause from OMPKinds.def and 
use OMP.td info (authored by clementval).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92955

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndex.cpp
  flang/include/flang/Parser/dump-parse-tree.h
  flang/include/flang/Parser/parse-tree.h
  flang/lib/Parser/unparse.cpp
  flang/lib/Semantics/check-omp-structure.h
  llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/include/llvm/TableGen/DirectiveEmitter.h
  llvm/test/TableGen/directive2.td
  llvm/utils/TableGen/DirectiveEmitter.cpp

Index: llvm/utils/TableGen/DirectiveEmitter.cpp
===
--- llvm/utils/TableGen/DirectiveEmitter.cpp
+++ llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -645,6 +645,72 @@
   GenerateFlangClauseUnparse(DirLang, OS);
 }
 
+void GenerateClauseClassMacro(const DirectiveLanguage &DirLang,
+  raw_ostream &OS) {
+  // Generate macros style information for legacy code in clang
+  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+
+  OS << "\n";
+
+  OS << "#ifndef CLAUSE\n";
+  OS << "#define CLAUSE(Enum, Str, Implicit)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_CLASS\n";
+  OS << "#define CLAUSE_CLASS(Enum, Str, Class)\n";
+  OS << "#endif\n";
+  OS << "#ifndef CLAUSE_NO_CLASS\n";
+  OS << "#define CLAUSE_NO_CLASS(Enum, Str)\n";
+  OS << "#endif\n";
+  OS << "\n";
+  OS << "#define __CLAUSE(Name, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, #Name, Class)\n";
+  OS << "#define __CLAUSE_NO_CLASS(Name)\\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, #Name, /* Implicit */ false) \\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, #Name)\n";
+  OS << "#define __IMPLICIT_CLAUSE_CLASS(Name, Str, Class)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_CLASS(" << DirLang.getClausePrefix()
+ << "##Name, Str, Class)\n";
+  OS << "#define __IMPLICIT_CLAUSE_NO_CLASS(Name, Str)  \\\n";
+  OS << "  CLAUSE(" << DirLang.getClausePrefix()
+ << "##Name, Str, /* Implicit */ true)\\\n";
+  OS << "  CLAUSE_NO_CLASS(" << DirLang.getClausePrefix() << "##Name, Str)\n";
+  OS << "\n";
+
+  for (const auto &R : DirLang.getClauses()) {
+Clause C{R};
+if (C.getClangClass().empty()) { // NO_CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_NO_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\")\n";
+  } else {
+OS << "__CLAUSE_NO_CLASS(" << C.getFormattedName() << ")\n";
+  }
+} else { // CLASS
+  if (C.isImplicit()) {
+OS << "__IMPLICIT_CLAUSE_CLASS(" << C.getFormattedName() << ", \""
+   << C.getFormattedName() << "\", " << C.getClangClass() << ")\n";
+  } else {
+OS << "__CLAUSE(" << C.getFormattedName() << ", " << C.getClangClass()
+   << ")\n";
+  }
+}
+  }
+
+  OS << "\n";
+  OS << "#undef __IMPLICIT_CLAUSE_NO_CLASS\n";
+  OS << "#undef __IMPLICIT_CLAUSE_CLASS\n";
+  OS << "#undef __CLAUSE\n";
+  OS << "#undef CLAUSE_NO_CLASS\n";
+  OS << "#undef CLAUSE_CLASS\n";
+  OS << "#undef CLAUSE\n";
+}
+
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
@@ -653,6 +719,8 @@
 return;
 
   EmitDirectivesFlangImpl(DirLang, OS);
+
+  GenerateClauseClassMacro(DirLang, OS);
 }
 
 // Generate the implemenation for the enumeration in the directive
Index: llvm/test/TableGen/directive2.td
===
--- llvm/test/TableGen/directive2.td
+++ llvm/test/TableGen/directive2.td
@@ -23,10 +23,15 @@
   let isValueList = 1;
 }
 def TDLC_ClauseC : Clause<"clausec"> {
+  let clangClass = "ClauseC";
   let flangClassValue = "Name";
   let defaultValue = "*";
   let isValueOptional = 1;
 }
+def TDLC_ClauseD : Clause<"claused"> {
+  let clangClass = "ClauseD";
+  let isImplicit = 1;
+}
 
 def

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@Meinersbur This patch is making Flang buildbots failing.

Since you have added a clause in `OMP.td` you have to add lines here as well -> 
https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/flang/lib/Semantics/check-omp-structure.cpp#L681

  CHECK_SIMPLE_CLAUSE(Sizes, OMPC_sizes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

A fix is ready hereL https://reviews.llvm.org/D96808


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:828
+(`use_device_addr` `(` $use_device_addr^ `:` type($use_device_addr) `)` )?
+(`map` `(` $map_type_modifier_val `,` $map_type_val `:` $map_operands^ `:` 
type($map_operands) `)` )?
+$region attr-dict

It's not clear how to you deal with multiple map clauses here? Or do you plan 
to have an operation generation per map clause coming from the frontend?



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:879
+(`nowait` $nowait^ )?
+(`map` `(` $map_type_modifier_val `,` $map_type_val `:` $map_operands^ `:` 
type($map_operands) `)` )?
+attr-dict

Same comment here about multiple map clauses. You probably need a custom 
parser/printer for the map part together with the change requested by @TIFitis. 





Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:930
+(`nowait` $nowait^ )?
+(`map` `(` $map_type_modifier_val `,` $map_type_val `:` $map_operands^ `:` 
type($map_operands) `)` )?
+attr-dict

Same comment here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131915

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105255#3723865 , 
@abidmalikwaterloo wrote:

> [MLIR][OpenMP] Added target data, exit data, and enter data operation 
> definition for MLIR

Looks like it's not the complete revision. There are almost no diffs anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

I get a failure  since this commit. `Could not load library By.so` ... `cannot 
open shared object file: No such file or directory`. Maybe a missing dependency?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129156

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D129156#3836768 , @tarunprabhu 
wrote:

> I think there is a typo somehow. It should be "Bye.so". Not sure how that 
> happened. I'm looking into this.

I probably didn't copy the error correctly .. this is the correct libs that is 
not found `llvm-project/build/lib/Bye.so`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129156

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

As @awarzynski mentioned, it is likely a missing dependency on the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129156

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D129156#3848692 , @awarzynski 
wrote:

> In D129156#3847396 , @tarunprabhu 
> wrote:
>
>> Added `examples` to `REQUIRES` in `test/Driver/pass-plugin.f90.
>
> Thanks for the update!
>
>> I still cannot reproduce the build failure on my end. @MatsPetersson tested 
>> this patch and the tests passed.
>
> @MatsPetersson & @clementval , could you share you build command so that the 
> failure can be reproduced before this re-lands?

I shared it with @tarunprabhu and I think the only major difference is the use 
of Ninja vs. Unix Makefiles.

  cmake \
-G "Unix Makefiles" \
-DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_ASSERTIONS=ON \
-DLLVM_TARGETS_TO_BUILD=host \
-DLLVM_ENABLE_PROJECTS="clang;mlir;flang" \
-DLLVM_ENABLE_RUNTIMES="compiler-rt"


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

https://reviews.llvm.org/D129156

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

[Valentin Clement via Phabricator via cfe-commits]

clementval requested changes to this revision.
clementval added a comment.
This revision now requires changes to proceed.

You need to update the driver-help tests.




Comment at: flang/include/flang/Tools/CLOptions.inc:194
 #if !defined(FLANG_EXCLUDE_CODEGEN)
-inline void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm) {
+inline void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm, bool 
Underscoring = 1) {
   fir::addBoxedProcedurePass(pm);

Please follow the same formatting in this file. 



Comment at: flang/include/flang/Tools/CLOptions.inc:213
+inline void createMLIRToLLVMPassPipeline(mlir::PassManager &pm,
+llvm::OptimizationLevel optLevel = defaultOptLevel, bool Underscoring = 1) 
{
   // Add default optimizer pass pipeline.

Doesn't follow the formatting here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

You should have a look at 
https://mlir.llvm.org/docs/PassManagement/#instance-specific-pass-options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140795

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.
Herald added a reviewer: nicolasvasilache.
Herald added subscribers: Moerafaat, zero9178.

Are you still working on this? Otherwise please close it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

I still see some review comments not done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D137329: [flang] Add -f[no-]associative-math and -mreassociate

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.
Herald added a subscriber: jdoerfert.

Wouldn't it be good to have a RFC for all these options and what they will do 
in Flang instead of just adding them all? Or did I miss the RFC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137329

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

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449
  ParentOneOf<["WsLoopOp", "ReductionDeclareOp", 
- "AtomicUpdateOp", "SimdLoopOp"]>]> {
+ "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> {
   let summary = "loop yield and termination operation";

abidmalikwaterloo wrote:
> clementval wrote:
> > 
> What's the problem here? It seems fine to me.
syntax doesn't match the existing code. Change is highlighted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Small suggestion




Comment at: flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp:41-45
+  std::string newName{result.second.name};
+  if (appendUnderscore)
+newName = newName + "_";
+
+  return newName;

To avoid new copy


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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Can you also add test in `flang/test/Fir/external-mangling.fir`?


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

https://reviews.llvm.org/D140795

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

Instead of copy pasting this from 
`mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` can you 
extract it and put it in a common shared file so bith translation can use the 
same code without duplication?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

kiranchandramohan wrote:
> clementval wrote:
> > Instead of copy pasting this from 
> > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` can 
> > you extract it and put it in a common shared file so bith translation can 
> > use the same code without duplication?
> @raghavendhra put up a patch some time back and he faced some issues. It 
> might be good to check with him or may be he can comment here.
> https://reviews.llvm.org/D127037
> https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
Just moving the three functions should be trivial. I'm not talking about the 
processMapOperand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

TIFitis wrote:
> clementval wrote:
> > kiranchandramohan wrote:
> > > clementval wrote:
> > > > Instead of copy pasting this from 
> > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` 
> > > > can you extract it and put it in a common shared file so bith 
> > > > translation can use the same code without duplication?
> > > @raghavendhra put up a patch some time back and he faced some issues. It 
> > > might be good to check with him or may be he can comment here.
> > > https://reviews.llvm.org/D127037
> > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > Just moving the three functions should be trivial. I'm not talking about 
> > the processMapOperand.
> I've moved `getSizeInBytes`.
> 
> The other two functions make use of `mlir::Location` and thus can't be moved 
> trivially.
> 
> I can still try to move them by individually passing the elements of 
> `mlir::Location` but that might not be ideal. Is that what you'd like?
What about a new header file in `mlir/include/mlir/Target/LLVMIR/Dialect/**` 
shared by 
`mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` and 
`mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`. That 
should be doable. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

TIFitis wrote:
> clementval wrote:
> > TIFitis wrote:
> > > clementval wrote:
> > > > kiranchandramohan wrote:
> > > > > clementval wrote:
> > > > > > Instead of copy pasting this from 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > >  can you extract it and put it in a common shared file so bith 
> > > > > > translation can use the same code without duplication?
> > > > > @raghavendhra put up a patch some time back and he faced some issues. 
> > > > > It might be good to check with him or may be he can comment here.
> > > > > https://reviews.llvm.org/D127037
> > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > Just moving the three functions should be trivial. I'm not talking 
> > > > about the processMapOperand.
> > > I've moved `getSizeInBytes`.
> > > 
> > > The other two functions make use of `mlir::Location` and thus can't be 
> > > moved trivially.
> > > 
> > > I can still try to move them by individually passing the elements of 
> > > `mlir::Location` but that might not be ideal. Is that what you'd like?
> > What about a new header file in 
> > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` and 
> > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`. That 
> > should be doable. 
> `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` and 
> `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp` already 
> have access to the common `mlir::Location` type.
> 
> Problem is that `OMPIRBuilder.cpp` is the only common file between them  
> where I can move the two functions to. Currently there are no `mlir/**` 
> include files in `OMPIRBuilder.cpp` and it seems to me like a strict design 
> choice to have it that way.
The functions can be header only. Why do you need to put them in the 
OMPIRBuilder.cpp? I think it is better than duplicate the exact same code over. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

TIFitis wrote:
> clementval wrote:
> > TIFitis wrote:
> > > clementval wrote:
> > > > TIFitis wrote:
> > > > > clementval wrote:
> > > > > > kiranchandramohan wrote:
> > > > > > > clementval wrote:
> > > > > > > > Instead of copy pasting this from 
> > > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > > >  can you extract it and put it in a common shared file so bith 
> > > > > > > > translation can use the same code without duplication?
> > > > > > > @raghavendhra put up a patch some time back and he faced some 
> > > > > > > issues. It might be good to check with him or may be he can 
> > > > > > > comment here.
> > > > > > > https://reviews.llvm.org/D127037
> > > > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > > > Just moving the three functions should be trivial. I'm not talking 
> > > > > > about the processMapOperand.
> > > > > I've moved `getSizeInBytes`.
> > > > > 
> > > > > The other two functions make use of `mlir::Location` and thus can't 
> > > > > be moved trivially.
> > > > > 
> > > > > I can still try to move them by individually passing the elements of 
> > > > > `mlir::Location` but that might not be ideal. Is that what you'd like?
> > > > What about a new header file in 
> > > > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` 
> > > > and 
> > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`. 
> > > > That should be doable. 
> > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` 
> > > and `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp` 
> > > already have access to the common `mlir::Location` type.
> > > 
> > > Problem is that `OMPIRBuilder.cpp` is the only common file between them  
> > > where I can move the two functions to. Currently there are no `mlir/**` 
> > > include files in `OMPIRBuilder.cpp` and it seems to me like a strict 
> > > design choice to have it that way.
> > The functions can be header only. Why do you need to put them in the 
> > OMPIRBuilder.cpp? I think it is better than duplicate the exact same code 
> > over. 
> Sorry, I misunderstood you earlier.
> I've added a new header file 
> `mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h`, this is my first attempt 
> at adding a new header file, please let me know if you find any issues.
Thanks! That's what I had in mind. We might want to check with MLIR folks if 
`mlir::utils` is suited for that. I don't mind if it is `mlir::omp::builder` or 
smth similar since it is related to the OMPIRBuilder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

The new test embed.f90 and embed-error.f90 are failing on my side. Do they 
assume a specific configuration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D142244#4096512 , @jsjodin wrote:

> No, it should work for any configuration as far as I know. How are you 
> running the test?

Just with `check-flang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D142244#4096546 , @jsjodin wrote:

> In D142244#4096538 , @clementval 
> wrote:
>
>> In D142244#4096512 , @jsjodin 
>> wrote:
>>
>>> No, it should work for any configuration as far as I know. How are you 
>>> running the test?
>>
>> Just with `check-flang`
>
> How does it fail? Can you show the output? Also what CPU are you running on?



  
  FAIL: Flang :: Driver/embed.f90 (2 of 1993)
   TEST 'Flang :: Driver/embed.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   llvm-project/build/bin/flang-new -fc1 -emit-llvm -o - 
-fembed-offload-object=llvm-project/flang/test/Driver/Inputs/hello.f90 
llvm-project/flang/test/Driver/embed.f90 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed.f90
  : 'RUN: at line 8';   llvm-project/build/bin/flang-new -fc1 -emit-llvm-bc -o 
llvm-project/build/tools/flang/test/Driver/Output/embed.f90.tmp.bc 
llvm-project/flang/test/Driver/embed.f90 2>&1
  : 'RUN: at line 9';   llvm-project/build/bin/flang-new -fc1 -emit-llvm -o - 
-fembed-offload-object=llvm-project/flang/test/Driver/Inputs/hello.f90 
llvm-project/build/tools/flang/test/Driver/Output/embed.f90.tmp.bc 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed.f90
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  llvm-project/flang/test/Driver/embed.f90:11:10: error: CHECK: expected string 
not found in input
  ! CHECK: @[[OBJECT_1:.+]] = private constant [61 x i8] c"program hello\0A 
write(*,*), \22Hello world!\22\0Aend program hello\0A", section 
".llvm.offloading", align 8, !exclude !0
   ^
  :1:1: note: scanning from here
  ; ModuleID = 'FIRModule'
  ^
  
  Input file: 
  Check file: llvm-project/flang/test/Driver/embed.f90
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
1: ; ModuleID = 'FIRModule' 
  check:11 X error: no match found
2: source_filename = "FIRModule" 
  check:11 ~~
3: target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
  check:11 
~
4: target triple = "x86_64-unknown-linux-gnu" 
  check:11 ~~~
5:  
  check:11 ~
6: @_QFECi = internal constant i32 1 
  check:11 ~~
.
.
.
  >>



  `
  FAIL: Flang :: Driver/embed-error.f90 (1 of 1993)
   TEST 'Flang :: Driver/embed-error.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   not llvm-project/build/bin/flang-new -fc1 -emit-llvm -o 
- -fembed-offload-object=llvm-project/flang/test/Driver/Inputs/missing.f90 
llvm-project/flang/test/Driver/embed-error.f90 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed-error.f90 
--check-prefix=ERROR
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  llvm-project/flang/test/Driver/embed-error.f90:8:10: error: ERROR: expected 
string not found in input
  ! ERROR: error: could not open
   ^
  :1:1: note: scanning from here
  ; ModuleID = 'FIRModule'
  ^
  :6:14: note: possible intended match here
  @_QFECi = internal constant i32 1
   ^
  
  Input file: 
  Check file: llvm-project/flang/test/Driver/embed-error.f90
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: ; ModuleID = 'FIRModule' 
  check:8'0 X error: no match found
 2: source_filename = "FIRModule" 
  check:8'0 ~~
 3: target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
  check:8'0 
~
 4: target triple = "x86_64-unknown-linux-gnu" 
  check:8'0 ~~~
 5:  
  check:8'0 ~
 6: @_QFECi = internal constant i32 1 
  check:8'0 ~~
  check:8'1  ? possible intended match
 7: @_QQEnvironmentDefaults = constant ptr null 
  check:8'0 
 8:  
  check:8'0 ~
 9: declare ptr @malloc(i64) 
  check:8'0 ~
10:  
  check:8'0 ~
11: declare void @free(ptr) 
  check:8'0 
 .
 .
 .

I run on `x86_64`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llv

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D142244#4096648 , @jsjodin wrote:

> In D142244#4096579 , @clementval 
> wrote:
>
>> In D142244#4096546 , @jsjodin 
>> wrote:
>>
>>> In D142244#4096538 , @clementval 
>>> wrote:
>>>
 In D142244#4096512 , @jsjodin 
 wrote:

> No, it should work for any configuration as far as I know. How are you 
> running the test?

 Just with `check-flang`
>>>
>>> How does it fail? Can you show the output? Also what CPU are you running on?
>>
>>
>>
>>   I run on `x86_64`
>
> How did you configure your build? I'd like to reproduce your build and see if 
> I can make it fail on my machine. Right now the test passes.

My bad .. my `CompilerInvocation.cpp` was overwritten and missed part of this 
patch. It works fine. Sorry to have bothered you with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

TIFitis wrote:
> clementval wrote:
> > TIFitis wrote:
> > > clementval wrote:
> > > > TIFitis wrote:
> > > > > clementval wrote:
> > > > > > TIFitis wrote:
> > > > > > > clementval wrote:
> > > > > > > > kiranchandramohan wrote:
> > > > > > > > > clementval wrote:
> > > > > > > > > > Instead of copy pasting this from 
> > > > > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > > > > >  can you extract it and put it in a common shared file so 
> > > > > > > > > > bith translation can use the same code without duplication?
> > > > > > > > > @raghavendhra put up a patch some time back and he faced some 
> > > > > > > > > issues. It might be good to check with him or may be he can 
> > > > > > > > > comment here.
> > > > > > > > > https://reviews.llvm.org/D127037
> > > > > > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > > > > > Just moving the three functions should be trivial. I'm not 
> > > > > > > > talking about the processMapOperand.
> > > > > > > I've moved `getSizeInBytes`.
> > > > > > > 
> > > > > > > The other two functions make use of `mlir::Location` and thus 
> > > > > > > can't be moved trivially.
> > > > > > > 
> > > > > > > I can still try to move them by individually passing the elements 
> > > > > > > of `mlir::Location` but that might not be ideal. Is that what 
> > > > > > > you'd like?
> > > > > > What about a new header file in 
> > > > > > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > >  and 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`.
> > > > > >  That should be doable. 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > >  and 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp` 
> > > > > already have access to the common `mlir::Location` type.
> > > > > 
> > > > > Problem is that `OMPIRBuilder.cpp` is the only common file between 
> > > > > them  where I can move the two functions to. Currently there are no 
> > > > > `mlir/**` include files in `OMPIRBuilder.cpp` and it seems to me like 
> > > > > a strict design choice to have it that way.
> > > > The functions can be header only. Why do you need to put them in the 
> > > > OMPIRBuilder.cpp? I think it is better than duplicate the exact same 
> > > > code over. 
> > > Sorry, I misunderstood you earlier.
> > > I've added a new header file 
> > > `mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h`, this is my first 
> > > attempt at adding a new header file, please let me know if you find any 
> > > issues.
> > Thanks! That's what I had in mind. We might want to check with MLIR folks 
> > if `mlir::utils` is suited for that. I don't mind if it is 
> > `mlir::omp::builder` or smth similar since it is related to the 
> > OMPIRBuilder.
> Since the utils file is common to all the dialects I kept it as `mlir::utils`.
> 
> How do I get the opinion from people working in MLIR on this, can you suggest 
> some reviewers whom I can add?
It's only valid for translation to the `llvmir` dialect so that why 
`mlir::utils` seems to generic to me. 

Maybe @ftynse has some thoughts on this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Just small nit




Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:628-629
+int64_t mapTypeBits = 0x00;
+auto mapOp = map_operands[i];
+auto mapTypeOp = map_types[i];
+

auto should be spelled out here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131915

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

[PATCH] D156901: [OpenMP] Change OpenMP default version in documentation and help text for -fopenmp-version

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: flang/test/Driver/driver-help.f90:55
 ! HELP-NEXT: -fopenmp-version=
-! HELP-NEXT:Set OpenMP version (e.g. 45 for OpenMP 
4.5, 50 for OpenMP 5.0). Default value is 50 for Clang and 11 for Flang
+! HELP-NEXT:Set OpenMP version (e.g. 45 for OpenMP 
4.5, 51 for OpenMP 5.1). Default value is 51 for Clang and 11 for Flang
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel 
code.

AntonRydahl wrote:
> tianshilei1992 wrote:
> > Does Flang also switch to 5.1 by default?
> As far as I know, `flang` is still OpenMP 1.1 by default but still depends on 
> the same command line options as `clang`. 
`flang` does not differentiate between versions. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156901

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

[PATCH] D156901: [OpenMP] Change OpenMP default version in documentation and help text for -fopenmp-version

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: flang/test/Driver/driver-help.f90:55
 ! HELP-NEXT: -fopenmp-version=
-! HELP-NEXT:Set OpenMP version (e.g. 45 for OpenMP 
4.5, 50 for OpenMP 5.0). Default value is 50 for Clang and 11 for Flang
+! HELP-NEXT:Set OpenMP version (e.g. 45 for OpenMP 
4.5, 51 for OpenMP 5.1). Default value is 51 for Clang and 11 for Flang
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel 
code.

jdoerfert wrote:
> tianshilei1992 wrote:
> > clementval wrote:
> > > AntonRydahl wrote:
> > > > tianshilei1992 wrote:
> > > > > Does Flang also switch to 5.1 by default?
> > > > As far as I know, `flang` is still OpenMP 1.1 by default but still 
> > > > depends on the same command line options as `clang`. 
> > > `flang` does not differentiate between versions. 
> > I see.
> I would remove the flang part of the wording for now.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156901

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

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.
Herald added a subscriber: sunshaoce.



Comment at: flang/test/Driver/pic-flags.f90:3
 
-! RUN: %flang -### %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu 2>&1 | 
FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fpie 2>&1 | 
FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL1,CHECK-PIE-LEVEL1-IR

This test has been failing on my side for very long time. Just curious why we 
check for PIE-LEVEL-2 when no level is given. Where is the default given?

On my machine I have `-mrelocation-model static` for this run line. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131533

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

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: flang/test/Driver/pic-flags.f90:3
 
-! RUN: %flang -### %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu 2>&1 | 
FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fpie 2>&1 | 
FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL1,CHECK-PIE-LEVEL1-IR

awarzynski wrote:
> clementval wrote:
> > This test has been failing on my side for very long time. Just curious why 
> > we check for PIE-LEVEL-2 when no level is given. Where is the default given?
> > 
> > On my machine I have `-mrelocation-model static` for this run line. 
> > This test has been failing on my side for very long time. 
> 
> If this is failing for you then it should also fail on one of the Flang 
> buildbots, right? Do you know what's different/special in your configuration?
> 
> > Where is the default given?
> 
> You want to check:
> * [[ 
> https://github.com/llvm/llvm-project/blob/2712b2805b47f10b3864ab19a4016ea306126ad7/clang/lib/Driver/ToolChains/Flang.cpp#L149-L168
>  | Flang::addPicOption ]] (Flang.cpp)
> * [[ 
> https://github.com/llvm/llvm-project/blob/2712b2805b47f10b3864ab19a4016ea306126ad7/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1399
>  | ParsePICArgs ]] (CommonArgs.cpp)
>If this is failing for you then it should also fail on one of the Flang 
>buildbots, right? Do you know what's different/special in your configuration?

Well I probably have a config that is not in any buildbot. 

In my configuration the PICLevel is 0 for `flang-new -v -S -emit-llvm -o - 
/local/home/vclement/llvm-project/flang/test/Driver/pic-flags.f90 
--target=aarch64-linux-gnu 2>&1`

Which makes sense since no pic option is given. I'm wondering how it default to 
2 on buildbot. I'll try to investigate. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131533

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

[PATCH] D143704: [flang] Feature list plugin

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

You have some failed buildbots. Can you revert it if you don't have a quick fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143704

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

[PATCH] D143704: [flang] Feature list plugin

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D143704#4202146 , @elmcdonough 
wrote:

> Haven't been able to check this yet, but I think it might be due to the tests 
> expecting a deterministic output.  I've only ever tested on Ubuntu WSL, so 
> its possible the order is different on different platforms.  I think 
> replacing CHECK-NEXT with CHECK-DAG might fix the issue.

Please revert until you have a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143704

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

[PATCH] D143704: [flang] Feature list plugin

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

pre-commit ci is showing a different failure:

   TEST 'Flang :: Examples/feature-list-class.f90' FAILED 

  Script:
  --
  : 'RUN: at line 4';   
/var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new -fc1 -load 
/var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so  
   -plugin feature-list 
/var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90
 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck 
/var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90
  --
  Exit Code: 1
   
  Command Output (stderr):
  --
  
/var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90:31:10:
 error: CHECK: expected string not found in input
  ! CHECK: Name: 32
   ^
  :1:1: note: scanning from here
  error: unable to load plugin 
'/var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so': 
'/var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so: 
cannot open shared object file: No such file or directory'
  ^
  :1:198: note: possible intended match here
  error: unable to load plugin 
'/var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so': 
'/var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so: 
cannot open shared object file: No such file or directory'


   ^
   
  Input file: 
  Check file: 
/var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90
   
  -dump-input=help explains the following input dump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143704

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

[PATCH] D143704: [flang] Feature list plugin

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

I reverted your patch since the buildbot was failing for couple of hours. Feel 
free to re-land it when you have a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143704

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

[PATCH] D143704: [flang] Feature list plugin

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

The pre-commit was failing. Please do not commit without at least checking that 
the pre-commit is fine.

https://buildkite.com/llvm-project/premerge-checks/builds/142487#018707c0-3ec0-4daa-a3a4-ffc815807994


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143704

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

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Thanks. Can you add tests in `flang/test/Driver`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145845

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

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D145845#4186607 , @awarzynski 
wrote:

> Thanks!
>
> In D145845#4186590 , @clementval 
> wrote:
>
>> Thanks. Can you add tests in `flang/test/Driver`?
>
> Hm, this is a `clangDriver` change rather than anything Flang specific 🤔 . 
> Btw, how does https://github.com/llvm/llvm-project/issues/61260 manifest to 
> you? I'm trying to figure out how to best test it (I couldn't find any other 
> tests for this specifically).

Cannot we just have a a .f03 file and compile it and see if we have an 
executable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145845

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

[PATCH] D153281: [flang] add -flang-experimental-polymorphism flag to flang-new

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

Looks ok to me. Note that some of the polymorphic TODOs will only be removed 
when HLFIR is in place.

Looks like you have a clang-format issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153281

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

[PATCH] D145183: [libc++] Implement `stop_token`

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.
Herald added a subscriber: JDevlieghere.

Something seems wrong with this patch. It contains multiple unrelated changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145183

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

[PATCH] D151445: [Flang] Add main-file-name flag to flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

You should be able to get this information from the source location in MLIR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151445

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

[PATCH] D143478: [RFC][Flang][driver] Try to support `flang -fc1as`

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

There should be some tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143478

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

[PATCH] D143493: [flang][driver] Add support for -include flag in flang -fc1

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D143493#4110230 , @awarzynski 
wrote:

> In what cases would this flag be used in practice? I've scanned Clang and 
> couldn't find any answers.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143493

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: flang/test/Fir/external-mangling.fir:36
+
+// CHECK-NOUNDER: func @foo
+// CHECK-NOUNDER: %{{.*}} = fir.address_of(@a) : !fir.ref>

You should check at least the character after it because here the check line 
would also succeed with an underscore. 



Comment at: flang/test/Fir/external-mangling.fir:39-40
+// CHECK-NOUNDER: %{{.*}} = fir.address_of(@__BLNK__) : 
!fir.ref>
+// CHECK-NOUNDER: fir.call @bar
+// CHECK-NOUNDER: fir.call @bar2
+// CHECK-NOUNDER: fir.global common @a(dense<0> : vector<4xi8>) : 
!fir.array<4xi8>

Same here. 



Comment at: flang/test/Fir/external-mangling.fir:85-88
+// CHECK-NOUNDER: func @callee
+// CHECK-NOUNDER: fir.call @callee
+// CHECK-NOUNDER: func @caller
+// CHECK-NOUNDER: fir.call @callee

Same here. 


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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

You still have clang-format issues in your patch. Can you update that.


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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

The pre-commit check is still failing with these tests:

Flang :: Fir/alloc.fir

  Flang :: Fir/embox.fir
  Flang :: Fir/optional.fir
  Flang :: Fir/rebox.fir
  Flang :: Lower/common-block.f90
  Flang :: Lower/forall/character-1.f90


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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

You still have style mismatch in `flang/include/flang/Tools/CLOptions.inc`. 
This file is likely not run through clang-format so that's why.




Comment at: flang/include/flang/Tools/CLOptions.inc:214
+llvm::OptimizationLevel optLevel = defaultOptLevel,
+bool Underscoring = true) {
   fir::addBoxedProcedurePass(pm);





Comment at: flang/include/flang/Tools/CLOptions.inc:233
 llvm::OptimizationLevel optLevel = defaultOptLevel,
-bool stackArrays = false) {
+bool stackArrays = false, bool Underscoring = true) {
   // Add default optimizer pass pipeline.





Comment at: flang/include/flang/Tools/CLOptions.inc:238
   // Add codegen pass pipeline.
-  fir::createDefaultFIRCodeGenPassPipeline(pm, optLevel);
+  fir::createDefaultFIRCodeGenPassPipeline(pm, optLevel, Underscoring);
 }




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

https://reviews.llvm.org/D140795

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[Valentin Clement via Phabricator via cfe-commits]

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

LGTM. Thanks for working on this.


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

https://reviews.llvm.org/D140795

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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

This commit is failing couple of buildbots. Can you revert or fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.
Herald added a subscriber: jdoerfert.

I have the `Driver/fast_math.f90` failing on my side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D138675#3988110 , @tblah wrote:

> In D138675#3987836 , @clementval 
> wrote:
>
>> I have the `Driver/fast_math.f90` failing on my side.
>>
>>   llvm-project/flang/test/Driver/fast_math.f90:63:14: error: CHECK-CRT: 
>> expected string not found in input
>>   ! CHECK-CRT: crtbeginS.o
>>
>> I have a `crtbegin.o` in the link line but not a `crtbeginS.o `
>
> Thanks for the heads up. This should be fixed by 
> https://github.com/llvm/llvm-project/commit/bea45027b43ec61a3efc4b487ceca2da25504432

Thanks for the fix. `crtendS.o` has the same issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:584
+std::stringstream typeMod, type;
+if (mapTypeBits & 0x04)
+  typeMod << "always ";

jdoerfert wrote:
> TIFitis wrote:
> > kiranchandramohan wrote:
> > > There is a `bitn` function in `printSynchronizationHint` and 
> > > `verifySynchronizationHint`, can that be used here?
> > I've added something similar. I got the bit values from Clang codegen and 
> > they use hex so I've kept it that way for uniformity. Let me know if you'd 
> > rather it be in n'th bit format.
> Clang codegen magic constants (and everything else) is moved to 
> llvm/.../Frontend/OpenMP for a reason. Hardcoding the numbers again somewhere 
> else seems like exactly the wrong thing to do.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131915

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