tra added inline comments.
================
Comment at: clang/include/clang/Basic/Cuda.h:105-107
+constexpr CudaArch DefaultCudaArch = CudaArch::SM_35;
+constexpr CudaArch DefaultHIPArch = CudaArch::GFX803;
+
----------------
Nit: those could be just enum values.
```
...
LAST,
DefaultCudaArch = SM_35,
DefaultHIPArch = GFX803,
};
```
================
Comment at: clang/lib/Driver/Driver.cpp:4219
+ for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)
DeviceActions.push_back(C.MakeAction<InputAction>(*InputArg, InputType));
----------------
This loop can be folded into the loop above.
Alternatively, for simple loops like this one could use `llvm::for_each`.
================
Comment at: clang/lib/Driver/Driver.cpp:4244
A = C.MakeAction<OffloadAction>(HDep, DDep);
+ ++TCAndArch;
+ } else if (isa<AssembleJobAction>(A) && Kind == Action::OFK_Cuda) {
----------------
Could you elaborate why TCAndArch is incremented only here? Don't other cases
need to advance it, too? At the very least we need some comments here and for
the loop in general, describing what's going on.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6998
+ // The CUDA toolchain should have a bound arch appended to the filename.
+ StringRef Arch = File.rsplit(".").first.rsplit('-').second;
+ CmdArgs.push_back(Args.MakeArgString(
----------------
Extracting arch name from the file name looks... questionable. Where does that
file name come from? Can't we get this information directly?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120272/new/
https://reviews.llvm.org/D120272
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
