<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 5, 2014, at 12:48 PM, Jakob Stoklund Olesen <<a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 5, 2014, at 12:34 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div><br class="Apple-interchange-newline">On Feb 5, 2014, at 9:46 AM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Wed, Feb 5, 2014 at 9:39 AM, Jakob Stoklund Olesen <<a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a>> wrote:<br><blockquote type="cite"><br>Looking at the top of InterferenceCache::get(), the PhysRegEntries array is used in the same way as the sparse array in llvm::SparseSet, see also Preston's paper:<span class="Apple-converted-space"> </span><a href="http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.30.7319">http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.30.7319</a><br><br>The double-checking "E < CacheEntries && Entries[E].getPhysReg() == PhysReg" means that the initial values of the array don't matter, and so the only reason for clearing it is to shut up tools like valgrind. That's why it can simply be allocated with calloc() and not cleared in reinitPhysRegEntries().<br><br>Clearly, this needs to be explained in a comment if we're going to bypass the zeroing.<br><br></blockquote><br>Aha. That makes total sense. And please :)<br><br>-eric<br><br></div></blockquote><div><br></div><div>Zeroing explanation added:</div><div><br></div><div></div></div><span><CompileTimeReductionSmall2.patch></span><div style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div></div><div><br></div><div>Eric, Jakob: does this patch now look good to you?</div></div></blockquote><div><br></div><div>Hi Puyan,</div><div><br></div><div>It looks like you have 3 unrelated patches in one. Please split it up. I’ll just comment on the InterferenceCache changes below:</div><div><br></div><div><div> InterferenceCache::Entry *InterferenceCache::get(unsigned PhysReg) {</div><div>+ // Call reinitPhysRegEntries here because PhysRegEntries may need to be</div><div>+ // resized in case a pass manager is reused for multiple targets.</div><div>+ reinitPhysRegEntries();</div><div> unsigned E = PhysRegEntries[PhysReg];</div><div><br></div><div>That doesn’t sound right at all. Isn’t InterferenceCache::init() called per function?</div><div><br></div><div>Thanks,</div><div>/jakob</div><div><br></div></div></div></div></blockquote></div><div><br></div><div><br></div>Hi Jakob,<div><br><div>I was trying to reinitialized the PhysRegEntries prior to its use in InterferenceCache::get, like we had discussed.</div><div>I also call it in InterferenceCache::init since that’s where the assign(N, 0) was.</div><div>But I think there’s probably no need to call reinitPhysRegEntries in InterferenceCache::get.</div><div><br></div><div>Do you think all this needs is to remove the call in InterferenceCache::get?</div><div><br></div><div>These are the 3 separated patches:</div><div><br></div><div></div></div></body></html>