tra added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> VersionFile =
+        FS.getBufferForFile(BinPath + "/.hipVersion");
+    if (!VersionFile)
----------------
arsenm wrote:
> yaxunl wrote:
> > arsenm wrote:
> > > arsenm wrote:
> > > > yaxunl wrote:
> > > > > arsenm wrote:
> > > > > > I don't think the detection should fail if this is missing
> > > > > why? this file is unique to HIP installation. If it is missing, the 
> > > > > installation is broken.
> > > > Because I should be able to use this without a complete hip 
> > > > installation. Without a specified version, it should assume the most 
> > > > modern layout. This will for example break pointing --rocm-path at the 
> > > > device library build directory for library tests
> > > I also don't see what value checking the version really provides; it may 
> > > be informative to print it, but I don't think it's useful to derive 
> > > information from it
> > what is the directory structure of your most modern layout?
> /opt/rocm/amdgcn/bitcode/foo.bc
> 
> The plan is to remove this and rely on symlinks in the resource directory 
> though
> I also don't see what value checking the version really provides; it may be 
> informative to print it, but I don't think it's useful to derive information 
> from it

In CUDA it's used to detect which back-end features to enable (they depend on 
what's supported by ptxas supplied by that CUDA version). I don't think that 
would be relevant to AMDGPU as it does not need external dependencies to 
generate GPU code.

It may be useful for filesystem layout detection. E.g. where to find bitcode 
files and what names to expect. This part seems to be relevant for ROCm, but 
I'm not sure if the layout is closely tied to the version.



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

https://reviews.llvm.org/D82930



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to