[PATCH] D60620: [HIP] Support target id by --offload-arch

Sat May 23 07:55:57 PDT 2020

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

================
Comment at: clang/lib/Basic/HIP.cpp:16
+const llvm::SmallVector<llvm::StringRef, 4>
+getAllPossibleTargetIdFeatures(llvm::StringRef Device) {
+  llvm::SmallVector<llvm::StringRef, 4> Ret;
----------------
tra wrote:
> Nit: there's an unfortunate clash with already [[ https://github.com/llvm/llvm-project/blob/6a3469f58d0c230e86043f6975f048968334dfa4/clang/include/clang/Driver/CC1Options.td#L23 | target-feature ]] in clang & LLVM.
> 
> Would something like `GPUProperties` be a reasonable name?
We call it target id feature to differentiate it from target feature. A target id feature usually corresponds to a target feature although it may not necessarily true.

Since target id feature sounds too close to target feature, it is reasonable to give it a different name.

How about OffloadArchFeatures ? Since they are used as features of the extended -offload-arch option.

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

https://reviews.llvm.org/D60620