This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Don't import GV which contains blockaddress
ClosedPublic

Authored by evgeny777 on Oct 11 2018, 7:51 AM.

Details

Summary

PR39241 uncovered an interesting problem with importing global variables containing blockaddress. Such variables can be imported only with function(s) referenced by blockaddress statements. As this is hard to check, I suggest simpler fix which does the following:

  • Marks all GVs which contain blockaddrss statement as "not eligible to import".
  • Instead of checking for dummy module in computeImportForReferencedGlobals check "not eligible to import" flag. This shouldn't break things as everything in non-thinlto module is marked as not eligible to import.

Diff Detail

Event Timeline

evgeny777 created this revision.Oct 11 2018, 7:51 AM
tejohnson added inline comments.Oct 11 2018, 8:32 AM
lib/Analysis/ModuleSummaryAnalysis.cpp
76

Document return value. Also, there are callers of findRefEdges in computeFunctionSummary. Are those ever expected to reference a blockaddress? From what I can tell in https://llvm.org/docs/LangRef.html#addresses-of-basic-blocks, it seems they could be referenced by an indirectbr instruction. Either those callsites need to do something similar with the return value, or they should assert findRefEdges returns false.

Are those ever expected to reference a blockaddress?

Yes, they are. But instruction in any given function can only reference basic block in the same function.
So we're not interested in return value of findRefEdges. I've reflected this in a comment to findRefEdges

tejohnson accepted this revision.Oct 11 2018, 10:59 AM

LGTM with a minor comment fix noted below.

lib/Analysis/ModuleSummaryAnalysis.cpp
78

s/kbow/know/

This revision is now accepted and ready to land.Oct 11 2018, 10:59 AM
This revision was automatically updated to reflect the committed changes.