<div>+ // Use to store the smallest APSInt size that can represent all the elements.</div><div>+ bool signedness = false;</div><div>+ unsigned bitwidth = 0;</div><div>+</div><div>+ // Skip diagnostic if previous error were found with the enum.</div>
<div>+ for (unsigned i = 0; i != NumElements; ++i) {</div><div>+ EnumConstantDecl *ECD = cast<EnumConstantDecl>(Elements[i]);</div><div>+ if (!ECD)</div><div>+ return;</div><div>+</div><div>+ const llvm::APSInt& Val = ECD->getInitVal();</div>
<div>+ if (!signedness && Val.isSigned()) {</div><div>+ signedness = true;</div><div>+ ++bitwidth;</div><div>+ }</div><div>+</div><div>+ unsigned ValWidth;</div><div>+ if (Val.isUnsigned())</div>
<div>+ ValWidth = Val.getActiveBits() + signedness;</div><div>+ else if (Val.isNonNegative())</div><div>+ ValWidth = Val.getActiveBits() + 1;</div><div>+ else</div><div>+ ValWidth = Val.getBitWidth() - Val.countLeadingOnes() + 1;</div>
<div>+</div><div>+ if (bitwidth < ValWidth)</div><div>+ bitwidth = ValWidth;</div><div>+ }</div><div><br></div><div>You can get these directly from the EnumDecl: see getNumPositiveBits, getNumNegativeBits.</div>
<div><br></div><div><div>+ // Store a map of values to decls. Values are extended to a common size</div><div>+ // first to for comparisons.</div><div>+ std::map<llvm::APSInt, EnumConstantDecl*> ValueMap;</div></div>
<div><br></div><div>How about building a SmallVector of EnumConstantDecl*, sorting it by init value then by whether there is an init expression, and finally performing your check as a single linear pass over the vector? That should be a bit quicker than the repeated map lookups and heap allocations you're currently doing.</div>
<div><br></div><div class="gmail_quote">On Wed, Jul 25, 2012 at 2:37 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">Ok. That's still a scary number. Do you have numbers for realistic examples? For example, we know Clang has some particularly large enums. This micro benchmark is useful, but it may be overly pessimistic.<div>
<div class="h5"><div><br><div><div>On Jul 25, 2012, at 2:34 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:</div><br><blockquote type="cite">Yes, that is the slowdown for the entire -fsyntax-only time for a source file with only an enum in it.<br>
<br><div class="gmail_quote">On Wed, Jul 25, 2012 at 1:48 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Richard,<div><br></div><div>If I am reading that right, the 6-10% slowdown is for the entire -fsyntax-only time? If so, that's definitely cost prohibitive.</div>
<div><br></div><div>Ted</div><div><br><div><div><div><div>On Jul 19, 2012, at 8:25 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:</div><br></div></div>
<blockquote type="cite"><div><div><div class="gmail_quote">On Wed, Jul 18, 2012 at 9:14 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div><div>On Jul 18, 2012, at 6:34 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:</div><br><blockquote type="cite">
<span style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;display:inline!important;float:none">A set could work for detecting the values, but both EnumConstantDecls are needed for the diagnostic, not just the values. Possibly a map from APSInt->EnumConstantDecl* would work. But either way, I would be dealing with getting APSInts to play nice with each other.</span></blockquote>
</div><br></div><div>That seems reasonable to me. The primary performance issue I see is the quadratic algorithmic complexity. If the APSInt comparisons are an issue, we can see if we can find ways to optimize that further.</div>
</div></blockquote></div><br><div>I created two more variations on and measured some timings. Both used a map, one with a custom compare function and one that extended the APSInt value before insertion. The APSInt extension had the better time, so I'll be giving the number for that one.</div>
<div><br></div><div>At 10,000 elements, there was a 6-10% slow down. This amounts to .01-.03 seconds difference on .13-.27 second runtime.</div><div><br></div><div>At 100,000 elements, 8-12% slow down. .2-.3 seconds on 1.34 to 2.66 second run time.</div>
<div><br></div><div>At 1,000,000 elements, 7-14% slow down. Around 2 second difference for runs of 13.6 to 26.7 seconds.</div><div><br></div><div>A new patch has been attached which has the APSInt bit extension before adding to the map.</div>
</div></div><span><duplicate-enum-bit-extension.patch></span></blockquote></div><br></div></div>
</blockquote></div><br>
</blockquote></div><br></div></div></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br>