[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D99683#2744764 <https://reviews.llvm.org/D99683#2744764>, @yaxunl wrote:

> In D99683#2683308 <https://reviews.llvm.org/D99683#2683308>, @tejohnson wrote:
>
>> To do what I suggested in the prior comment, you'd probably want to add a new index-wide flag (since we don't read IR in the thin link). See for example how EnableSplitLTOUnit is set and used. You could add a flag like ForceImportAll or something like that. Then you don't necessarily even need to bump up the importing threshold or add the new import-noinline flag. Just key off of that in the importer to try to force import everything. If something cannot be imported, fail with a clear error.
>
> will do

I noticed you implemented with an internal error rather than a flag in the index. I think this is ok for now, especially if the support will eventually be removed because the linker will support external functions as noted in your TODO (note in order to do this in the index you would need to set up the flag during the compile step, not the linker invocation as you are doing here, which has some advantages if this will persist longer term). I have a suggestion about the error detection below, so that you can report errors earlier along with the failure reason.



================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496
             dbgs() << "ignored! No qualifying callee with summary found.\n");
         continue;
       }
----------------
Probably better to issue an error here with the import failure reason?


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

https://reviews.llvm.org/D99683