[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Really nice to see some shared code being elevated out of Clang into LLVM, 
thanks!

I've only reviewed on the Flang driver changes. I will let the OpenMP experts 
to review the remaining bits. All in all looks good, I've only made some small 
suggestions.

Thanks for working on this!




Comment at: flang/lib/Frontend/FrontendActions.cpp:93
 
+std::string CodeGenAction::getAllTargetFeatures() {
+  std::string allFeaturesStr;

This method could be simplified by following 
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code.
 For example:

```
std::string CodeGenAction::getAllTargetFeatures()  {
  if (!triple.isAMDGPU()) {
allFeaturesStr = llvm::join(targetOpts.featuresAsWritten.begin(),
targetOpts.featuresAsWritten.end(), ",");
return allFeaturesStr;
  }

  // The logic for AMDGPU
  // Perhaps add a dedicated hook: getExplicitAndImplicitAMDGPUTargetFeatures()
} 
```

Btw, this method does different things depending on the triple. Perhaps rename 
it as something more generic, e.g. `getTargetFeatures`? I think that the 
current name, `getAllTargetFeatures`, is a bit misleading (i.e. what does "all" 
mean?).

Also:
* make it `static`
* document


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

https://reviews.llvm.org/D145579

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

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

[Dominik Adamski via Phabricator via cfe-commits]

domada added a comment.

D146612  presents the lowering from MLIR 
attributes to LLVM IR.


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

https://reviews.llvm.org/D145579

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

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 507269.
domada retitled this revision from "[WIP][Flang][AMDGPU][OpenMP] Save target 
features in OpenMP MLIR  dialect" to "[Flang][AMDGPU][OpenMP] Save target 
features in OpenMP MLIR  dialect".
domada added a comment.

Patch rebased


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

https://reviews.llvm.org/D145579

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendActions.h
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/target-cpu-features.f90
  flang/test/Lower/OpenMP/target_cpu_features.f90
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1437,6 +1437,40 @@
   return false;
 }
 
+// Set the omp.target_cpu attribute on the module with the specified string
+void OpenMPDialect::setTargetCpu(Operation *module, llvm::StringRef cpu) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu"}),
+  mlir::StringAttr::get(module->getContext(), cpu));
+}
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpu(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+void OpenMPDialect::setTargetCpuFeatures(Operation *module,
+ llvm::StringRef cpuFeatures) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu_features"}),
+  mlir::StringAttr::get(module->getContext(), cpuFeatures));
+}
+
+// Return the value of the omp.target_cpu_features attribute stored in the
+// module if it exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpuFeatures(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu_features"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -36,6 +36,21 @@
 // Return the value of the omp.is_device attribute stored in the module if it
 // exists, otherwise return false by default
 static bool getIsDevice(Operation* module);
+
+// Set the omp.target_cpu attribute on the module with the specified string
+static void setTargetCpu(Operation* module, StringRef cpu);
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+static std::string getTargetCpu(Operation* module);
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+static void setTargetCpuFeatures(Operation* module, StringRef cpuFeatures);
+
+// Return the value of the omp.target_cpu_features attribute stored in
+// the module if it exists, otherwise return empty by default
+static std::string getTargetCpuFeatures(Operation* module);
   }];
 }
 
Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -251,3 +251,212 @@
 
   return T.isAMDGCN() ? getArchNameAMDGCN(ProcKind) : getArchNameR600(ProcKind);
 }
+
+void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
+  StringMap &Features) {
+  // XXX - What does the member GPU mean if device name string passed here?
+  if (T.isAMDGCN()) {
+switch (parseArchAMDGCN(GPU)) {
+case GK_GFX1103:
+case GK_GFX1102:
+case GK_GFX1101:
+case GK_GFX1100:
+  Features["ci-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot8-insts"] = true;
+  Features["dot9-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] =

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 503672.
domada added a comment.

Fixed formatting


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

https://reviews.llvm.org/D145579

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendActions.h
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/target-cpu-features.f90
  flang/test/Lower/OpenMP/target_cpu_features.f90
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1437,6 +1437,39 @@
   return false;
 }
 
+// Set the omp.target_cpu attribute on the module with the specified string
+void OpenMPDialect::setTargetCpu(Operation *module, llvm::StringRef cpu) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu"}),
+  mlir::StringAttr::get(module->getContext(), cpu));
+}
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpu(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+void OpenMPDialect::setTargetCpuFeatures(Operation *module,
+ llvm::StringRef cpuFeatures) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu_features"}),
+  mlir::StringAttr::get(module->getContext(), cpuFeatures));
+}
+
+// Return the value of the omp.target_cpu_features attribute stored in the
+// module if it exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpuFeatures(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu_features"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -36,6 +36,21 @@
 // Return the value of the omp.is_device attribute stored in the module if it
 // exists, otherwise return false by default
 static bool getIsDevice(Operation* module);
+
+// Set the omp.target_cpu attribute on the module with the specified string
+static void setTargetCpu(Operation* module, StringRef cpu);
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+static std::string getTargetCpu(Operation* module);
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+static void setTargetCpuFeatures(Operation* module, StringRef cpuFeatures);
+
+// Return the value of the omp.target_cpu_features attribute stored in
+// the module if it exists, otherwise return empty by default
+static std::string getTargetCpuFeatures(Operation* module);
   }];
 }
 
Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -251,3 +251,212 @@
 
   return T.isAMDGCN() ? getArchNameAMDGCN(ProcKind) : getArchNameR600(ProcKind);
 }
+
+void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
+  StringMap &Features) {
+  // XXX - What does the member GPU mean if device name string passed here?
+  if (T.isAMDGCN()) {
+switch (parseArchAMDGCN(GPU)) {
+case GK_GFX1103:
+case GK_GFX1102:
+case GK_GFX1101:
+case GK_GFX1100:
+  Features["ci-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot8-insts"] = true;
+  Features["dot9-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] = true;
+  Features["gfx9-insts"] = true;
+  Features["gfx10-insts"] = true;
+  Features["gfx10-3-insts"] = true;
+  Features["gfx11-insts"] = true;
+  break;
+case

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

[Dominik Adamski via Phabricator via cfe-commits]

domada created this revision.
domada added reviewers: jsjodin, agozillon, skatrak, TIFitis, kiranktp, 
dpalermo, kiranchandramohan, NimishMishra, awarzynski.
domada added projects: Flang, OpenMP, MLIR, AMDGPU.
Herald added subscribers: sunshaoce, Moerafaat, zero9178, bzcheeseman, kosarev, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, kerbowa, liufengdb, aartbik, mgester, 
arpith-jacob, csigg, antiagainst, shauheen, rriddle, mehdi_amini, jdoerfert, 
thopre, guansong, hiraditya, tpr, dstuttard, yaxunl, jvesely, kzhuravl.
Herald added a reviewer: sscalpone.
Herald added a project: All.
domada requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1, 
stephenneuendorffer, nicolasvasilache, MaskRay, wdng.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: nicolasvasilache.
Herald added projects: clang, LLVM.

Scope of changes:

1. Add AMDGPU target as one of valid targets for Flang.
2. Extract common code between Clang and Flang for parsing AMDGPU features
3. Store information about target in OpenMP MLIR dialect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145579

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendActions.h
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/target-cpu-features.f90
  flang/test/Lower/OpenMP/target_cpu_features.f90
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1437,6 +1437,39 @@
   return false;
 }
 
+// Set the omp.target_cpu attribute on the module with the specified string
+void OpenMPDialect::setTargetCpu(Operation *module, llvm::StringRef cpu) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu"}),
+  mlir::StringAttr::get(module->getContext(), cpu));
+}
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpu(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+void OpenMPDialect::setTargetCpuFeatures(Operation *module,
+ llvm::StringRef cpuFeatures) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu_features"}),
+  mlir::StringAttr::get(module->getContext(), cpuFeatures));
+}
+
+// Return the value of the omp.target_cpu_features attribute stored in the
+// module if it exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpuFeatures(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu_features"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -36,6 +36,21 @@
 // Return the value of the omp.is_device attribute stored in the module if it
 // exists, otherwise return false by default
 static bool getIsDevice(Operation* module);
+
+// Set the omp.target_cpu attribute on the module with the specified string
+static void setTargetCpu(Operation* module, StringRef cpu);
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+static std::string getTargetCpu(Operation* module);
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+static void setTargetCpuFeatures(Operation* module, StringRef cpuFeatures);
+
+// Return the value of the omp.target_cpu_features attribute stored in
+// the module if it exists, otherwise return empty by default
+static std::string getTargetCpuFeatures(Operation* module);
   }];
 }
 
Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser