<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="">Thanks! Commited in r<span style="font-family: Menlo; font-size: 11px;" class="">222121</span><div class=""><font face="Menlo" class=""><span style="font-size: 11px;" class=""><br class=""></span></font><div><blockquote type="cite" class=""><div class="">On 14 Nov 2014, at 20:12, 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 Fri, Nov 14, 2014 at 2:50 AM, 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="">Sorry, I sent the previous mail too early :-)<br class=""><br class="">Here is the new version:<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">The way I would do this is I would first look at two values to figure<br class="">out the offset and multiplier, and then iterate through the rest of<br class="">the values and see if they fit.<br class=""><br class=""><br class="">Basically a good idea, but it would require to duplicate the checks, so IMO<br class="">it would make things more complicated.<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="">I'm not sure what you mean by "duplicate the checks", but I guess my</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="">approach would be more inefficient since it would be doing</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="">multiplication to check that each table element fits the linear</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="">relation. Just measuring the distance, as your patch does, seems more</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="">efficient.</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="">even in the case of overflows<br class=""><br class=""><br class="">Overflows are not a problem. I added a test case for it.<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="">Great.</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="">It will also make handling undefs easier, i.e. we can just ignore them.<br class=""><br class=""><br class="">I don't agree. This is only true if you assume that the first two values are<br class="">not undefs.<br class=""><br class="">I decided to not handle undefs because according to my findings, they are<br class="">very seldom in switch tables (even in non-linear ones).<br class="">In the whole test-suite there is only a single case. IMO it's not worth the<br class="">additional complexity it would add to the code.<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="">OK, you've convinced me :)</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="">@@ -3765,6 +3812,16 @@<br class=""> switch (Kind) {<br class=""> case SingleValueKind:<br class=""> return SingleValue;<br class="">+ case LinearMapKind: {<br class="">+ // Derive the result value from the input value.<br class="">+ Value *Result = Builder.CreateIntCast(Index, LinearMultiplier->getType(),<br class="">+ false, "switch.idx.cast");<br class="">+ if (!LinearMultiplier->isOne())<br class="">+ Result = Builder.CreateMul(Result, LinearMultiplier, "switch.idx.mult");<br class="">+ if (!LinearOffset->isZero())<br class="">+ Result = Builder.CreateAdd(Result, LinearOffset, "switch.offset");<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="">Thanks! Can you please add tests that cover the cases that don't</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="">require multiplier and/or offset? Besides that, the patch looks 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=""><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="">to me. Feel free to commit with added tests.</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="">- 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=""><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="">On 13 Nov 2014, at 22:29, Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:<br class=""><br class="">On Thu, Nov 13, 2014 at 12:20 PM, Erik Eckstein <<a href="mailto:eeckstein@apple.com" class="">eeckstein@apple.com</a>> wrote:<br class=""><br class="">Thanks for reviewing!<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=""><br class="">29 in the test-suite (including External).<br class=""><br class=""><br class="">Nice.<br class=""><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="">In this case I would put it even before BitMapKind.<br class=""><br class=""><br class="">Sounds good.<br class=""><br class="">I think undef is probably the only possible case here, right?<br class=""><br class=""><br class="">I'm not sure.<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=""><br class="">Let's do it in a follow-up patch. I'll add a TODO.<br class=""><br 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=""><br class=""><br class="">I didn't do it to keep the code simpler. But if there is a good reason to<br class="">simplify it here and not in follow-up optimizations, it's not a problem.<br class=""><br class=""><br class="">Yeah, I think we should do it here.<br class=""><br class=""><br class="">A few more thoughts about the patch:<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="">+ 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=""><br class=""><br class="">Not a big deal, but checking "I != 0" seems like a more clear way to<br class="">check that it's not the first iteration.<br class=""><br class=""><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=""><br class=""><br class="">The way I would do this is I would first look at two values to figure<br class="">out the offset and multiplier, and then iterate through the rest of<br class="">the values and see if they fit.<br class=""><br class="">I think that might be a little easier to read, and it would also make<br class="">it more obvious that all the results are correct even in the case of<br class="">overflows etc., because we've checked the same computation that will<br class="">happen at run-time. It will also make handling undefs easier, i.e. we<br class="">can just ignore them.<br class=""><br class=""><br class="">Thanks,<br class="">Hans<br class=""><br 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=""><br class="">Hi Hans,<br class=""><br class="">this patch adds a simple optimization for switch table lookup: It computes<br class="">the output value directly with an (optional) mul and add if there is a<br class="">linear mapping between index and output. Example:<br class=""><br class=""><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="">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<br class="">mapping");<br class="">STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup<br class="">tables");<br class="">STATISTIC(NumLookupTablesHoles, "Number of switch instructions turned into<br class="">lookup tables (holes checked)");<br class="">STATISTIC(NumSinkCommons, "Number of common instructions sunk down to the<br class="">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<br class="">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<br class="">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=""><br class=""><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=""> } 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=""><br class=""><br class="">Maybe LinearOffset and LinearMultiplier, to tie the names even tighter<br class="">to the LinearMapKind?<br class=""><br 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=""><br class=""><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="">+ 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=""><br class=""><br class="">ResConst feels a little backwards to me. Why not ConstRes, as in<br class="">"constant result"?<br class=""><br class="">+ if (!ResConst) {<br class="">+ // E.g. if it is an undef.<br class=""><br class=""><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="">+ 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(),<br class="">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=""><br class=""><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></div></blockquote></div><br class=""></div></body></html>