[PATCH] D99683: [HIP] Support ThinLTO

Wed May 12 13:43:46 PDT 2021

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

================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496
             dbgs() << "ignored! No qualifying callee with summary found.\n");
         continue;
       }
----------------
tejohnson wrote:
> yaxunl wrote:
> > tejohnson wrote:
> > > Probably better to issue an error here with the import failure reason?
> > My understanding is that the import failure reason is only available if PrintImportFailures is enabled. Also it can only print the GUID and can not print the failed callee name since it is not available, therefore the information is cryptic. It seems to me the current error msg at line 1332 is more suitable for common users. For compiler developers, they can enable PrintImportFailures and see the reason of failed imports.
> selectCallee always sets the Reason. And we have the name in addition to the GUID in normal circumstances (linking from modules). It would only not be available in certain debugging situations (e.g. linking from an existing combined module with llvm-lto). Also, by failing here, you don't need to wait until the LTO backends to issue the error, so it fails a little earlier.
I see. Will do.

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

https://reviews.llvm.org/D99683