[PATCH] D150998: [OpenMP] Fix using the target ID when using the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8465-8470
+    if (TC->getTriple().isAMDGPU()) {
+      for (StringRef Feature : llvm::split(Arch.split(':').second, ':')) {
+        FeatureArgs.emplace_back(
+            Args.MakeArgString(Feature.take_back() + Feature.drop_back()));
+      }
+    }
----------------
saiislam wrote:
> May be use `parseTargetIDWithFormatCheckingOnly()`?
I tried that but it didn't return the strings in the format required by `llc` for the `-mattrs` list.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8475
         "triple=" + TC->getTripleString(),
-        "arch=" + Arch.str(),
+        "arch=" + getProcessorFromTargetID(TC->getTriple(), Arch).str(),
         "kind=" + Kind.str(),
----------------
saiislam wrote:
> Shouldn't Arch (targetID here) should be passed along instead of just the processor?
> 
> For example, `gfx90a:xnack+` and `gfx90a:xnack-` should be treated differently.
So the problem there is that this will cause us to no longer link in something like the OpenMP runtime library since `gfx90a` != `gfx90a:xnack+`. Right now the behavior is that we will link them both together since the architecture matches but then the attributes will get resolved the same way we handle `-mattr=+x,-x`. I'm not sure what the expected behaviour is here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150998