<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">This is the new version:</div><div class=""><br class=""></div><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><span class="" style="float: none; display: inline !important;">The way I would do this is I would first look at two values to figure</span><br class=""><span class="" style="float: none; display: inline !important;">out the offset and multiplier, and then iterate through the rest of</span><br class=""><span class="" style="float: none; display: inline !important;">the values and see if they fit.</span><br class=""></blockquote><div class=""><br class=""></div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On 13 Nov 2014, at 22:29, Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">On Thu, Nov 13, 2014 at 12:20 PM, Erik Eckstein <</span><a href="mailto:eeckstein@apple.com" style="font-family: Helvetica; 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;" class="">eeckstein@apple.com</a><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">> wrote:</span><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class="">Thanks for reviewing!<br class=""><br class=""><blockquote type="cite" class="">Do you have any numbers on how often this fires in a Clang bootstrap<br class="">or other benchmark?<br class=""></blockquote><br class="">29 in the test-suite (including External).<br class=""></blockquote><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Nice.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class=""><blockquote type="cite" class="">This enum is sorted in a "cheapest first" order, which matches the<br class="">order that the SwitchLookupTable tries things. LinearMapKind should go<br class="">before ArrayKind.<br class=""></blockquote><br class="">In this case I would put it even before BitMapKind.<br class=""></blockquote><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Sounds good.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class=""><blockquote type="cite" class="">I think undef is probably the only possible case here, right?<br class=""></blockquote><br class="">I'm not sure.<br class=""><br class=""><blockquote type="cite" class="">For undef, it shouldn't matter what result we return (someone please<br class="">correct me if I'm wrong here), so it would be nice if we could ignore<br class="">undef values when trying to find the linear relationship. If it makes<br class="">things more complicated we could do it in a follow-up patch.<br class=""></blockquote><br class="">Let's do it in a follow-up patch. I'll add a TODO.<br class=""><br class=""><blockquote type="cite" class="">Should we handle the cases where the multiplier is 1 or offset is 0<br class="">and generate simpler code for that?<br class=""></blockquote><br class="">I didn't do it to keep the code simpler. But if there is a good reason to simplify it here and not in follow-up optimizations, it's not a problem.<br class=""></blockquote><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Yeah, I think we should do it here.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">A few more thoughts about the patch:</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class="">+ // Check if we can derive the value with a linear transformation from the<br class="">+ // table index.<br class="">+ IntegerType *ValueIntType = dyn_cast<IntegerType>(ValueType);<br class="">+ if (ValueIntType) {<br class="">+ bool LinearMappingPossible = true;<br class="">+ APInt PrevVal;<br class="">+ APInt DistToPrev;<br class="">+ // Check if there is the same distance between two consecutive values.<br class="">+ for (uint64_t I = 0; I < TableSize; ++I) {<br class="">+ ConstantInt *ResConst = dyn_cast<ConstantInt>(TableContents[I]);<br class="">+ if (!ResConst) {<br class="">+ // E.g. if it is an undef.<br class="">+ LinearMappingPossible = false;<br class="">+ break;<br class="">+ }<br class="">+ APInt Val = ResConst->getValue();<br class="">+ if (I > 0) {<br class=""></blockquote><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Not a big deal, but checking "I != 0" seems like a more clear way to</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">check that it's not the first iteration.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class="">+ APInt Dist = Val - PrevVal;<br class="">+ if (I == 1) {<br class="">+ DistToPrev = Dist;<br class="">+ } else if (Dist != DistToPrev) {<br class="">+ LinearMappingPossible = false;<br class="">+ break;<br class="">+ }<br class="">+ }<br class="">+ PrevVal = Val;<br class="">+ }<br class=""></blockquote><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">The way I would do this is I would first look at two values to figure</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">out the offset and multiplier, and then iterate through the rest of</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">the values and see if they fit.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">I think that might be a little easier to read, and it would also make</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">it more obvious that all the results are correct even in the case of</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">overflows etc., because we've checked the same computation that will</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">happen at run-time. It will also make handling undefs easier, i.e. we</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">can just ignore them.</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Thanks,</span><br style="font-family: Helvetica; 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;" class=""><span style="font-family: Helvetica; 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; float: none; display: inline !important;" class="">Hans</span><br style="font-family: Helvetica; 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;" class=""><br style="font-family: Helvetica; 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;" class=""><blockquote type="cite" style="font-family: Helvetica; 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;" class=""><blockquote type="cite" class="">On 13 Nov 2014, at 19:40, Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:<br class=""><br class="">On Thu, Nov 13, 2014 at 7:26 AM, Erik Eckstein <<a href="mailto:eeckstein@apple.com" class="">eeckstein@apple.com</a>> wrote:<br class=""><blockquote type="cite" class="">Hi Hans,<br class=""><br class="">this patch adds a simple optimization for switch table lookup: It computes the output value directly with an (optional) mul and add if there is a linear mapping between index and output. Example:<br class=""></blockquote><br class="">Thanks! This has been on my mental todo list for switch transformation<br class="">stuff for a while, so I'm very happy to review it.<br class=""><br class="">Do you have any numbers on how often this fires in a Clang bootstrap<br class="">or other benchmark?<br class=""><br class=""><blockquote type="cite" class="">Index: lib/Transforms/Utils/SimplifyCFG.cpp<br class="">===================================================================<br class="">--- lib/Transforms/Utils/SimplifyCFG.cpp (revision 221882)<br class="">+++ lib/Transforms/Utils/SimplifyCFG.cpp (working copy)<br class="">@@ -70,6 +70,7 @@<br class=""> cl::desc("Hoist conditional stores if an unconditional store precedes"));<br class=""><br class="">STATISTIC(NumBitMaps, "Number of switch instructions turned into bitmaps");<br class="">+STATISTIC(NumLinearMaps, "Number of switch instructions turned into linear mapping");<br class="">STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup tables");<br class="">STATISTIC(NumLookupTablesHoles, "Number of switch instructions turned into lookup tables (holes checked)");<br class="">STATISTIC(NumSinkCommons, "Number of common instructions sunk down to the end block");<br class="">@@ -3663,7 +3664,12 @@<br class=""><br class=""> // The table is stored as an array of values. Values are retrieved by load<br class=""> // instructions from the table.<br class="">- ArrayKind<br class="">+ ArrayKind,<br class="">+<br class="">+ // For tables where there is a linear relationship between table index<br class="">+ // and values. We calculate the result with a simple multiplication<br class="">+ // and addition instead of a table lookup.<br class="">+ LinearMapKind<br class=""></blockquote><br class="">This enum is sorted in a "cheapest first" order, which matches the<br class="">order that the SwitchLookupTable tries things. LinearMapKind should go<br class="">before ArrayKind.<br class=""><br class=""><br class=""><blockquote type="cite" class=""> } Kind;<br class=""><br class=""> // For SingleValueKind, this is the single value.<br class="">@@ -3673,6 +3679,10 @@<br class=""> ConstantInt *BitMap;<br class=""> IntegerType *BitMapElementTy;<br class=""><br class="">+ // For LinearMapKind, these are the constants used to derive the value.<br class="">+ Constant *ValueOffset;<br class="">+ Constant *Multiplier;<br class=""></blockquote><br class="">Maybe LinearOffset and LinearMultiplier, to tie the names even tighter<br class="">to the LinearMapKind?<br class=""><br class=""><blockquote type="cite" class="">+<br class=""> // For ArrayKind, this is the array.<br class=""> GlobalVariable *Array;<br class=""> };<br class="">@@ -3685,7 +3695,7 @@<br class=""> Constant *DefaultValue,<br class=""> const DataLayout *DL)<br class=""> : SingleValue(nullptr), BitMap(nullptr), BitMapElementTy(nullptr),<br class="">- Array(nullptr) {<br class="">+ ValueOffset(nullptr), Multiplier(nullptr), Array(nullptr) {<br class=""> assert(Values.size() && "Can't build lookup table without values!");<br class=""> assert(TableSize >= Values.size() && "Can't fit values in table!");<br class=""><br class="">@@ -3730,6 +3740,42 @@<br class=""> return;<br class=""> }<br class=""><br class="">+ // Check if we can derive the value with a linear transformation from the<br class="">+ // table index.<br class="">+ IntegerType *ValueIntType = dyn_cast<IntegerType>(ValueType);<br class="">+ if (ValueIntType) {<br class=""></blockquote><br class="">In LLVM style it's more common to do:<br class=""><br class="">if (IntegerType *ValueIntType = dyn_cast<...<br class=""><br class="">This saves a line and reduces the scope of ValueIntType to only the if<br class="">statement, which is where it's used.<br class=""><br class=""><blockquote type="cite" class="">+ bool LinearMappingPossible = true;<br class="">+ APInt PrevVal;<br class="">+ APInt DistToPrev;<br class="">+ // Check if there is the same distance between two consecutive values.<br class="">+ for (uint64_t I = 0; I < TableSize; ++I) {<br class="">+ ConstantInt *ResConst = dyn_cast<ConstantInt>(TableContents[I]);<br class=""></blockquote><br class="">ResConst feels a little backwards to me. Why not ConstRes, as in<br class="">"constant result"?<br class=""><br class=""><blockquote type="cite" class="">+ if (!ResConst) {<br class="">+ // E.g. if it is an undef.<br class=""></blockquote><br class="">I think undef is probably the only possible case here, right?<br class=""><br class="">For undef, it shouldn't matter what result we return (someone please<br class="">correct me if I'm wrong here), so it would be nice if we could ignore<br class="">undef values when trying to find the linear relationship. If it makes<br class="">things more complicated we could do it in a follow-up patch.<br class=""><br class=""><blockquote type="cite" class="">+ LinearMappingPossible = false;<br class="">+ break;<br class="">+ }<br class="">+ APInt Val = ResConst->getValue();<br class="">+ if (I > 0) {<br class="">+ APInt Dist = Val - PrevVal;<br class="">+ if (I == 1) {<br class="">+ DistToPrev = Dist;<br class="">+ } else if (Dist != DistToPrev) {<br class="">+ LinearMappingPossible = false;<br class="">+ break;<br class="">+ }<br class="">+ }<br class="">+ PrevVal = Val;<br class="">+ }<br class="">+ if (LinearMappingPossible) {<br class="">+ ValueOffset = TableContents[0];<br class="">+ Multiplier = ConstantInt::get(ValueIntType, DistToPrev);<br class="">+ Kind = LinearMapKind;<br class="">+ ++NumLinearMaps;<br class="">+ return;<br class="">+ }<br class="">+ }<br class="">+<br class=""> // If the type is integer and the table fits in a register, build a bitmap.<br class=""> if (WouldFitInRegister(DL, TableSize, ValueType)) {<br class=""> IntegerType *IT = cast<IntegerType>(ValueType);<br class="">@@ -3802,6 +3848,14 @@<br class=""> "switch.gep");<br class=""> return Builder.CreateLoad(GEP, "switch.load");<br class=""> }<br class="">+ case LinearMapKind: {<br class="">+ // Derive the result value from the input value.<br class="">+ Value *Result = Builder.CreateIntCast(Index, Multiplier->getType(), false,<br class="">+ "switch.idx.cast");<br class="">+ Result = Builder.CreateMul(Result, Multiplier, "switch.idx.mult");<br class="">+ Result = Builder.CreateAdd(Result, ValueOffset, "switch.offset");<br class=""></blockquote><br class="">Should we handle the cases where the multiplier is 1 or offset is 0<br class="">and generate simpler code for that? It seems like it would be easy to<br class="">handle here, so we might as well do it and save some churn.</blockquote></blockquote></div></blockquote></div><br class=""></body></html>