[PATCH] D84822: Add target ID to AMDGPU documentation

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/docs/AMDGPUUsage.rst:386
+
+  *On*
+    Specified by ``+``, indicating the target feature is enabled. A code
----------------
yaxunl wrote:
> 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.
I'm with @yaxunl. This is literally duplicating something else we have in the IR already, maybe not also diverge for no real reason.


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

https://reviews.llvm.org/D84822