[PATCH] D97598: [NFC][AMDGPU] Document the AMDGPU target feature defaults

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: llvm/docs/AMDGPUUsage.rst:509-528
+                                                  If not specified for code object V2 to V3, generate
+                                                  code that can be loaded and executed in a process
+                                                  with XNACK replay enabled.
+
+                                                  If not specified for code object V4, generate
+                                                  code that can be loaded and executed in a process
+                                                  with either setting of XNACK replay.
----------------
foad wrote:
> t-tye wrote:
> > foad wrote:
> > > t-tye wrote:
> > > > foad wrote:
> > > > > Isn't your third paragraph here just explaining that the first two are identical? I.e. "generate code that works with xnack replay enabled" and "generate code that works whether xnack replay is enabled or not" are the same thing.
> > > > Code object v2/v3 does not support the concept of ANY and so the default has to be ether ON or OFF. The second paragraph specifies that ON is used.
> > > > 
> > > > Code object V4 has the concept of ANY and the third paragraph states that the default is ANY.
> > > > 
> > > > The fourth paragraph describes the consequences of these defaults.
> > > But ANY and ON are synonyms, aren't they? They both seem to tell the compiler to do exactly the same thing, so the default behaviour hasn't changed in any observable way.
> > No they are not. Code object v2-v3 only supports ON and OFF. Code object supports ON, OFF, and ANY. ANY tells the loader that it is safe to load the code on a device configured with either ON or OFF. ON tells the loader it can only load onto a device with the feature ON.
> > 
> > It may be that the compiler generates the same code for ON and ANY but that is distinct from how the loader acts on them.
> > 
> > This is explained more in the TargetID section of the clang offload bundler.
> > ANY tells the loader that it is safe to load the code on a device configured with either ON or OFF. ON tells the loader it can only load onto a device with the feature ON.
> 
> Thanks. But doesn't that contradict what you wrote: "Executing code that was generated with XNACK replay enabled [...] on a device that does not have XNACK replay enabled will execute correctly". What does it mean to say it "will execute correctly" if the loader would refuse to load it?
> 
> In any case, if the loader treats ANY and ON differently, then I think you should mention that here. Otherwise there is nothing to explain that ANY and ON are different, which makes changing the default from ON (v2/v3) to ANY (v4) sound rather pointless :-)
> > ANY tells the loader that it is safe to load the code on a device configured with either ON or OFF. ON tells the loader it can only load onto a device with the feature ON.
> 
> Thanks. But doesn't that contradict what you wrote: "Executing code that was generated with XNACK replay enabled [...] on a device that does not have XNACK replay enabled will execute correctly". What does it mean to say it "will execute correctly" if the loader would refuse to load it?

It is not a contradiction. It explains that the code is the same, even though the loader does the enforcement described. Some users like to know this, and the original text said this so I left it here:-)

> 
> In any case, if the loader treats ANY and ON differently, then I think you should mention that here. Otherwise there is nothing to explain that ANY and ON are different, which makes changing the default from ON (v2/v3) to ANY (v4) sound rather pointless :-)

The documentation on ANY and ON appears in the following section on Target ID which links to the Clang Offload Bundler documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97598