[PATCH] D84822: Add target ID to AMDGPU documentation

[Yaxun Liu via Phabricator via llvm-commits]

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


================
Comment at: llvm/docs/AMDGPUUsage.rst:386
+
+  *On*
+    Specified by ``+``, indicating the target feature is enabled. A code
----------------
JonChesterfield wrote:
> The on/off division seems likely to cause us problems later. How about 0/1 instead, so that we can later add 2, 3 etc when a feature is added that has more than two states?
> 
> Or strings.
Using +/- is more concise and easier to parse. If we need multi-value for future attributes, it is not difficult to differentiate them by checking the last character.


================
Comment at: llvm/docs/AMDGPUUsage.rst:1318
+     Offload Kind  Description
+     ============= ==============================================================
+     host          This offload kind is used for the first dummy empty entry
----------------
JonChesterfield wrote:
> I'm not sure we want to encode artefacts of clang-offload-bundler in this spec
This is due to a restriction of `clang-offload-bundler`. Here we document the current situation. We plan to fix that by removing the artifact. 


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

https://reviews.llvm.org/D84822