<div dir="ltr">Ping.<div><br></div><div>While I CC'ed Hal on this directly because I know he's interested in AA stuff, if anyone else has time to review, it would definitely be appreciated.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jul 23, 2015 at 2:04 AM Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc created this revision.<br>
chandlerc added a reviewer: hfinkel.<br>
chandlerc added a subscriber: llvm-commits.<br>
<br>
I don't actually know how the global deletion side of this bug hasn't<br>
been hit before, but for the other it just-so-happens that functions<br>
aren't likely to be deleted in the particular part of the LTO pipeline<br>
where we currently enable GMR, so we got lucky.<br>
<br>
With this patch, I can self-host with GMR enabled in the normal pass<br>
pipeline!<br>
<br>
I was a bit concerned about the compile-time impact of this chang, which<br>
is part of what motivated my prior string of patches to make the<br>
per-function datastructure very dense and fast to walk. With those<br>
changes in place, I can't measure a significant compile time difference<br>
(the difference is around 0.1% which is *way* below the noise) before<br>
and after this patch when building a linked bitcode for all of Clang.<br>
<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11453&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=TSDeXFqYVOd9nWJuCqHdI1Scj0MPl_LARrgMk9UYhKE&s=Udg7YmNqTDgBc2qDs9WidunKgBOb7LhKm8mnwLBP0-I&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11453</a><br>
<br>
Files:<br>
lib/Analysis/IPA/GlobalsModRef.cpp<br>
<br>
Index: lib/Analysis/IPA/GlobalsModRef.cpp<br>
===================================================================<br>
--- lib/Analysis/IPA/GlobalsModRef.cpp<br>
+++ lib/Analysis/IPA/GlobalsModRef.cpp<br>
@@ -180,6 +180,13 @@<br>
GlobalMRI = ModRefInfo(GlobalMRI | NewMRI);<br>
}<br>
<br>
+ /// Clear a global's ModRef info. Should be used when a global is being<br>
+ /// deleted.<br>
+ void eraseModRefInfoForGlobal(const GlobalValue &GV) {<br>
+ if (AlignedMap *P = Info.getPointer())<br>
+ P->Map.erase(&GV);<br>
+ }<br>
+<br>
private:<br>
/// All of the information is encoded into a single pointer, with a three bit<br>
/// integer in the low three bits. The high bit provides a flag for when this<br>
@@ -215,19 +222,26 @@<br>
<br>
void deleted() override {<br>
Value *V = getValPtr();<br>
+ if (auto *F = dyn_cast<Function>(V))<br>
+ GMR.FunctionInfos.erase(F);<br>
+<br>
if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {<br>
if (GMR.NonAddressTakenGlobals.erase(GV)) {<br>
// This global might be an indirect global. If so, remove it and<br>
- // remove<br>
- // any AllocRelatedValues for it.<br>
+ // remove any AllocRelatedValues for it.<br>
if (GMR.IndirectGlobals.erase(GV)) {<br>
// Remove any entries in AllocsForIndirectGlobals for this global.<br>
for (auto I = GMR.AllocsForIndirectGlobals.begin(),<br>
E = GMR.AllocsForIndirectGlobals.end();<br>
I != E; ++I)<br>
if (I->second == GV)<br>
GMR.AllocsForIndirectGlobals.erase(I);<br>
}<br>
+<br>
+ // Scan the function info we have collected and remove tihs global<br>
+ // from all of them.<br>
+ for (auto &FIPair : GMR.FunctionInfos)<br>
+ FIPair.second.eraseModRefInfoForGlobal(*GV);<br>
}<br>
}<br>
<br>
@@ -361,11 +375,13 @@<br>
/// (really, their address passed to something nontrivial), record this fact,<br>
/// and record the functions that they are used directly in.<br>
void GlobalsModRef::AnalyzeGlobals(Module &M) {<br>
+ SmallPtrSet<Function *, 64> TrackedFunctions;<br>
for (Function &F : M)<br>
if (F.hasLocalLinkage())<br>
if (!AnalyzeUsesOfPointer(&F)) {<br>
// Remember that we are tracking this global.<br>
NonAddressTakenGlobals.insert(&F);<br>
+ TrackedFunctions.insert(&F);<br>
Handles.emplace_front(*this, &F);<br>
Handles.front().I = Handles.begin();<br>
++NumNonAddrTakenFunctions;<br>
@@ -381,12 +397,22 @@<br>
Handles.emplace_front(*this, &GV);<br>
Handles.front().I = Handles.begin();<br>
<br>
- for (Function *Reader : Readers)<br>
+ for (Function *Reader : Readers) {<br>
+ if (TrackedFunctions.insert(Reader).second) {<br>
+ Handles.emplace_front(*this, Reader);<br>
+ Handles.front().I = Handles.begin();<br>
+ }<br>
FunctionInfos[Reader].addModRefInfoForGlobal(GV, MRI_Ref);<br>
+ }<br>
<br>
if (!GV.isConstant()) // No need to keep track of writers to constants<br>
- for (Function *Writer : Writers)<br>
+ for (Function *Writer : Writers) {<br>
+ if (TrackedFunctions.insert(Writer).second) {<br>
+ Handles.emplace_front(*this, Writer);<br>
+ Handles.front().I = Handles.begin();<br>
+ }<br>
FunctionInfos[Writer].addModRefInfoForGlobal(GV, MRI_Mod);<br>
+ }<br>
++NumNonAddrTakenGlobalVars;<br>
<br>
// If this global holds a pointer type, see if it is an indirect global.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>