JonChesterfield added a comment.

Looks much simpler, thanks!



================
Comment at: clang/lib/Driver/Driver.cpp:755
+                      *this, TT, *HostTC, C.getInputArgs(), 
Action::OFK_OpenMP);
+                else if (TT.isAMDGCN())
+                  DeviceTC =
----------------
Minor suggestion,
```
if (TT.isNVPTX() {
 ...
} else {
  assert(TT.isAMDGCN());
  ...
}```

? Semantically equivalent, but `if () else if ()` looks like there is a missing 
else clause.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:43
+
+static void addOptLevelArgs(const llvm::opt::ArgList &Args,
+                            llvm::opt::ArgStringList &CmdArgs) {
----------------
Maybe rename this since opt is no longer involved. addLLCOptArg or similar? 
There's probably the same logic in another toolchain


================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:64
+                 .Case("g", "1")
+                 .Default("2");
+    }
----------------
Default("0") and update the comment 'we map anything else'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94961

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

Reply via email to