<div dir="ltr">Thanks Hans for the tip!<div><br></div><div>I was following CoveredLookupTable.ll. I checked in r213880. If that does not work, I will move it to X86.</div><div><br></div><div>Thanks,</div><div>Manman</div></div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 24, 2014 at 10:23 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="">On Thu, Jul 24, 2014 at 8:16 AM, Manman <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
> Sorry for the breakage. It seems simplifycfg (switch to table conversion) depends on the target.<br>
<br>
</div>Yes, the switch to table conversion only runs on targets that opt in<br>
to it via TargetTransformInfo.<br>
<div class=""><br>
> I will update the testing case.<br>
<br>
</div>You can just move it to test/Transforms/SimplifyCFG/X86/ where the<br>
other switch to table conversion tests live.<br>
<span class="HOEnZb"><font color="#888888"><br>
 - Hans<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
>> On Jul 24, 2014, at 5:56 AM, Tilmann Scheller <<a href="mailto:t.scheller@samsung.com">t.scheller@samsung.com</a>> wrote:<br>
>><br>
>> Hi Manman,<br>
>><br>
>> seems like this commit broke the clang-native-arm-cortex-a9 buildbot, can<br>
>> you please have a look?<br>
>><br>
>> <a href="http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/20308" target="_blank">http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/20308</a><br>
>><br>
>> Regards,<br>
>><br>
>> Tilmann<br>
>><br>
>> -----Original Message-----<br>
>> From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a><br>
>> [mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Manman Ren<br>
>> Sent: Thursday, July 24, 2014 1:13 AM<br>
>> To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> Subject: [llvm] r213815 - SimplifyCFG: fix a bug in switch to table<br>
>> conversion<br>
>><br>
>> Author: mren<br>
>> Date: Wed Jul 23 18:13:23 2014<br>
>> New Revision: 213815<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=213815&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=213815&view=rev</a><br>
>> Log:<br>
>> SimplifyCFG: fix a bug in switch to table conversion<br>
>><br>
>> We use gep to access the global array "switch.table", and the table index<br>
>> should be treated as unsigned. When the highest bit is 1, this commit<br>
>> zero-extends the index to an integer type with larger size.<br>
>><br>
>> For a switch on i2, we used to generate:<br>
>> %switch.tableidx = sub i2 %0, -2<br>
>> getelementptr inbounds [4 x i64]* @switch.table, i32 0, i2 %switch.tableidx<br>
>><br>
>> It is incorrect when %switch.tableidx is 2 or 3. The fix is to generate<br>
>> %switch.tableidx = sub i2 %0, -2 %switch.tableidx.zext = zext i2<br>
>> %switch.tableidx to i3 getelementptr inbounds [4 x i64]* @switch.table, i32<br>
>> 0, i3 %switch.tableidx.zext<br>
>><br>
>> rdar://17735071<br>
>><br>
>> Added:<br>
>>    llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll<br>
>> Modified:<br>
>>    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Simplify" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Simplify</a><br>
>> CFG.cpp?rev=213815&r1=213814&r2=213815&view=diff<br>
>> ============================================================================<br>
>> ==<br>
>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)<br>
>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Jul 23 18:13:23<br>
>> +++ 2014<br>
>> @@ -3477,7 +3477,7 @@ namespace {<br>
>><br>
>>     /// BuildLookup - Build instructions with Builder to retrieve the value<br>
>> at<br>
>>     /// the position given by Index in the lookup table.<br>
>> -    Value *BuildLookup(Value *Index, IRBuilder<> &Builder);<br>
>> +    Value *BuildLookup(Value *Index, uint64_t TableSize, IRBuilder<><br>
>> + &Builder);<br>
>><br>
>>     /// WouldFitInRegister - Return true if a table with TableSize elements<br>
>> of<br>
>>     /// type ElementType would fit in a target-legal register.<br>
>> @@ -3598,7 +3598,8 @@ SwitchLookupTable::SwitchLookupTable(Mod<br>
>>   Kind = ArrayKind;<br>
>> }<br>
>><br>
>> -Value *SwitchLookupTable::BuildLookup(Value *Index, IRBuilder<> &Builder) {<br>
>> +Value *SwitchLookupTable::BuildLookup(Value *Index, uint64_t TableSize,<br>
>> +                                      IRBuilder<> &Builder) {<br>
>>   switch (Kind) {<br>
>>     case SingleValueKind:<br>
>>       return SingleValue;<br>
>> @@ -3624,6 +3625,14 @@ Value *SwitchLookupTable::BuildLookup(Va<br>
>>                                  "switch.masked");<br>
>>     }<br>
>>     case ArrayKind: {<br>
>> +      // Make sure the table index will not overflow when treated as<br>
>> signed.<br>
>> +      if (IntegerType *IT = dyn_cast<IntegerType>(Index->getType()))<br>
>> +        if (TableSize > (1 << (IT->getBitWidth() - 1)))<br>
>> +          Index = Builder.CreateZExt(Index,<br>
>> +                                     IntegerType::get(IT->getContext(),<br>
>> +                                                      IT->getBitWidth() +<br>
>> 1),<br>
>> +                                     "switch.tableidx.zext");<br>
>> +<br>
>>       Value *GEPIndices[] = { Builder.getInt32(0), Index };<br>
>>       Value *GEP = Builder.CreateInBoundsGEP(Array, GEPIndices,<br>
>>                                              "switch.gep"); @@ -3823,7<br>
>> +3832,7 @@ static bool SwitchToLookupTable(SwitchIn<br>
>>     SI->getDefaultDest()->removePredecessor(SI->getParent());<br>
>>   } else {<br>
>>     Value *Cmp = Builder.CreateICmpULT(TableIndex, ConstantInt::get(<br>
>> -                                         MinCaseVal->getType(),<br>
>> TableSize));<br>
>> +                                       MinCaseVal->getType(),<br>
>> + TableSize));<br>
>>     Builder.CreateCondBr(Cmp, LookupBB, SI->getDefaultDest());<br>
>>   }<br>
>><br>
>> @@ -3878,7 +3887,7 @@ static bool SwitchToLookupTable(SwitchIn<br>
>>     SwitchLookupTable Table(Mod, TableSize, MinCaseVal, ResultLists[PHI],<br>
>>                             DV, DL);<br>
>><br>
>> -    Value *Result = Table.BuildLookup(TableIndex, Builder);<br>
>> +    Value *Result = Table.BuildLookup(TableIndex, TableSize, Builder);<br>
>><br>
>>     // If the result is used to return immediately from the function, we<br>
>> want to<br>
>>     // do that right here.<br>
>><br>
>> Added: llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/s" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/s</a><br>
>> witch-table-bug.ll?rev=213815&view=auto<br>
>> ============================================================================<br>
>> ==<br>
>> --- llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll (added)<br>
>> +++ llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll Wed Jul<br>
>> +++ 23 18:13:23 2014<br>
>> @@ -0,0 +1,41 @@<br>
>> +; RUN: opt -S -simplifycfg < %s | FileCheck %s ; rdar://17735071 target<br>
>> +datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"<br>
>> +target triple = "x86_64-apple-darwin12.0.0"<br>
>> +<br>
>> +; When tableindex can't fit into i2, we should extend the type to i3.<br>
>> +; CHECK-LABEL: @_TFO6reduce1E5toRawfS0_FT_Si ; CHECK: entry:<br>
>> +; CHECK-NEXT: sub i2 %0, -2<br>
>> +; CHECK-NEXT: zext i2 %switch.tableidx to i3 ; CHECK-NEXT:<br>
>> +getelementptr inbounds [4 x i64]* @switch.table, i32 0, i3<br>
>> +%switch.tableidx.zext ; CHECK-NEXT: load i64* %switch.gep ; CHECK-NEXT:<br>
>> +ret i64 %switch.load define i64 @_TFO6reduce1E5toRawfS0_FT_Si(i2) {<br>
>> +entry:<br>
>> +  switch i2 %0, label %1 [<br>
>> +    i2 0, label %2<br>
>> +    i2 1, label %3<br>
>> +    i2 -2, label %4<br>
>> +    i2 -1, label %5<br>
>> +  ]<br>
>> +<br>
>> +; <label>:1                                       ; preds = %entry<br>
>> +  unreachable<br>
>> +<br>
>> +; <label>:2                                       ; preds = %2<br>
>> +  br label %6<br>
>> +<br>
>> +; <label>:3                                       ; preds = %4<br>
>> +  br label %6<br>
>> +<br>
>> +; <label>:4                                       ; preds = %6<br>
>> +  br label %6<br>
>> +<br>
>> +; <label>:5                                       ; preds = %8<br>
>> +  br label %6<br>
>> +<br>
>> +; <label>:6                                      ; preds = %3, %5, %7, %9<br>
>> +  %7 = phi i64 [ 3, %5 ], [ 2, %4 ], [ 1, %3 ], [ 0, %2 ]<br>
>> +  ret i64 %7<br>
>> +}<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>