<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 4, 2014, at 11:41 PM, 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 Tue, Feb 4, 2014 at 10:26 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:<br><blockquote type="cite"><br>On Feb 4, 2014, at 10:16 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><br><blockquote type="cite">On Tue, Feb 4, 2014 at 9:47 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:<br><blockquote type="cite"><br>On Feb 4, 2014, at 6:52 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><br><blockquote type="cite">Hey cool,<br><br>+/// reinitPhysRegEntries - Initializes PhysRegEntries to new<br>+/// zeroed out memory of size NumPhysRegs. This is faster than using<br>+/// llvm::SmallVector::assign(N, 0). It's used in the below code to<br>+/// also supports when a pass manager could be reused for different<br>+/// but similar targets that could have varying numbers of PhysRegs.<br>+void InterferenceCache::reinitPhysRegEntries() {<br>+ if (PhysRegEntriesCount == TRI->getNumRegs()) return;<br>+ free(PhysRegEntries);<br>+ PhysRegEntriesCount = TRI->getNumRegs();<br>+ PhysRegEntries = (unsigned char*) calloc(PhysRegEntriesCount,<br>sizeof(unsigned char));<br>+}<br><br>Looks to be faster since we're not actually zeroing out the array if<br>it's the same size as the previous allocation?<br><br></blockquote><br>Not exactly; the compile time improvement gained here over the current implementation comes from the fact that we're using a SmallVector:<br><br>SmallVector<unsigned char, 2> PhysRegEntries;<br><br>that we zero out using assign:<br><br>PhysRegEntries.assign(TRI->getNumRegs(), 0);<br><br>when the SmallVector might not actually be that small at all.<br><br>Compile time was being spent in zeroing out the SmallVector when calling llvm::SmallVector::assign, which uses an iterator to assign each element in the SmallVector to zero.<br>I found that just by changing the SmallVector to a std::vector I got a compile time reduction since std::vector ends up using __platform_bzero to do the job.<br><br>Jakob suggested I just calloc the memory in the first place, rather than using a std::vector or calling bzero.<br>And actually, it would be nice if SmallVector had some platform specific speed improvements eventually, the way that std::vector does.<br><br></blockquote><br>Yeah, it would be. That's a bit crazy.<br><br>At any rate, I was wondering why reinit didn't 0 the mem if it<br>happened to be the same size?<br><br></blockquote><br>Hmm, I am actually not sure what is correct in this case. I'll have to ask Jakob.<br>But the reinit here is more or less a precaution.<br>I believe the existing code would have used the same already populated SmallVector memory if the pass manager we to be reused too.<br><br></blockquote><br>Right, I'm just confused as to the seeming (?) assumption that a<br>reused pass manager with a different machine would have the same<br>registers in the phys reg map? Or maybe I'm confused :)<br></div></blockquote><div><br></div><div>If the pass is reused for a machine with a different number of physregs, then the PhysRegEntries array needs to be resized.</div><div><br></div><div>If the pass is reused for the same machine (or a machine with same # physregs), it does not need to be resized.</div><div><br></div><div>PhysRegEntries does not need to be zeroed between passes, because it is safe for the entries to contain garbage. It works like a SparseSet. The entry is only valid if its corresponding CacheEntries bucket has a matching register assignment.</div><div><br></div><div>Puyan, it would be good to add a small comment explaining why the PhysRegEntries do not need to be zero upon initialization.</div><div><br></div><div>As for why we do the calloc the first time, it is just good form. And probably prevents valgrind from complaining.</div><div><br></div><div>-Andy</div><br><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;"><br>-eric<br><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br><blockquote type="cite">+ for (auto RegI = PhysRegs.begin(), E = PhysRegs.end(); RegI != E; ++RegI)<br>+ if (!MRI->reg_nodbg_empty(*RegI))<br>+ MRI->setPhysRegUsed(*RegI);<br><br>Can't use auto quite yet :)<br><br></blockquote><br>Doe! I was under the impression that C++11 support had been enabled several weeks ago.<br></blockquote><br>Almost there :)<br><br>-eric<br><br><blockquote type="cite"><br><blockquote type="cite"><br>-eric<br><br></blockquote><br>-Thanks<br><br>Puyan<br><br><blockquote type="cite">On Tue, Feb 4, 2014 at 3:44 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:<br><blockquote type="cite"><br>On Feb 4, 2014, at 2:52 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><br><blockquote type="cite">On Tue, Feb 4, 2014 at 2:47 PM, Andrew Trick <<a href="mailto:atrick@apple.com">atrick@apple.com</a>> wrote:<br><blockquote type="cite">On Feb 4, 2014, at 2:41 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:<br><br><blockquote type="cite">Here is a patch for reducing compile time on small programs, especially if the target has a large register file.<br>I've already done sanity checks to determine that it doesn't regress compile time on large programs.<br></blockquote><br>LGTM. (mainly I'm trusting Jakob's informal review earlier).<br></blockquote><br>Knowing this helps :)<br><br>It'd be nice to get some comments in the code as to what's going on<br>where. There aren't a lot in there as it is, but getting some more<br>would be nice.<br><br>-eric<br><br></blockquote><br>Here's a better commented version, Eric:<br><br><br><br><br>-Puyan<br><br><br><blockquote type="cite"><blockquote type="cite">-Andy<br><br><blockquote type="cite"><br>-Puyan<br><br><CompileTimeReductionSmall.patch>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></div></blockquote></div><br></body></html>