<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="">Hello<div class=""><br class=""></div><div class="">I have a question about this.</div><div class="">Instcombine can now end up creating illegal types in switch statements with a safety net of CGP which should expand the switch statement back into legal types.</div><div class="">I have a few examples of IR where running a sequence of passes (instcombine + simplifycfg) results in other instructions using the illegal type(besides the switch inst).</div><div class="">For e.g., I see the following IR just before CGP</div><div class=""><br class=""></div><div class=""><div class=""> %trunc = trunc i32 %shr35.i9 to i4</div><div class=""> switch i4 %trunc, label %if.else.77.i [</div><div class=""> i4 6, label %for.body.i.i</div><div class=""> i4 2, label %for.body.i.i</div><div class=""> i4 1, label %for.body.i.i</div><div class=""> i4 5, label %for.body.i.362</div><div class=""> ]</div><div class=""><br class=""></div><div class="">for.body.i.i: ; preds = %for.body.i.362, %for.body.i.362, %for.body.i.362</div><div class=""> %cond = icmp eq i4 %trunc, 6</div></div><div class=""><br class=""></div><div class=""><a href="https://reviews.llvm.org/rL251857" class="">rL251857</a> - only fixes up the switch statement but not the icmp.</div><div class="">Should it be left to the backend to fix up these illegal types or should the instcombine + other passes not generate illegal types in the first place?</div><div class=""><br class=""></div><div class="">Thanks</div><div class="">Aditya</div><div class=""><div><blockquote type="cite" class=""><div class="">On Jun 30, 2016, at 7:51 AM, Sanjay Patel via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: spatel<br class="">Date: Thu Jun 30 09:51:21 2016<br class="">New Revision: 274233<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274233&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=274233&view=rev</a><br class="">Log:<br class="">[InstCombine] shrink switch conditions better (PR24766)<br class=""><br class=""><a href="https://llvm.org/bugs/show_bug.cgi?id=24766#c2" class="">https://llvm.org/bugs/show_bug.cgi?id=24766#c2</a><br class=""><br class="">This removes a hack that was added for the benefit of x86 codegen. <br class="">It prevented shrinking the switch condition even to smaller legal (DataLayout) types.<br class="">We have a safety mechanism in CGP after:<br class="">http://reviews.llvm.org/rL251857<br class="">...so we're free to use the optimal (smallest) IR type now.<br class=""><br class="">Differential Revision: http://reviews.llvm.org/D12965<br class=""><br class=""><br class="">Modified:<br class=""> llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br class=""> llvm/trunk/test/Transforms/InstCombine/narrow-switch.ll<br class=""> llvm/trunk/test/Transforms/InstCombine/pr21651.ll<br class=""><br class="">Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=274233&r1=274232&r2=274233&view=diff<br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Thu Jun 30 09:51:21 2016<br class="">@@ -2185,17 +2185,15 @@ Instruction *InstCombiner::visitSwitchIn<br class=""><br class=""> unsigned NewWidth = BitWidth - std::max(LeadingKnownZeros, LeadingKnownOnes);<br class=""><br class="">- // Truncate the condition operand if the new type is equal to or larger than<br class="">- // the largest legal integer type. We need to be conservative here since<br class="">- // x86 generates redundant zero-extension instructions if the operand is<br class="">- // truncated to i8 or i16.<br class="">+ // Shrink the condition operand if the new type is smaller than the old type.<br class="">+ // This may produce a non-standard type for the switch, but that's ok because<br class="">+ // the backend should extend back to a legal type for the target.<br class=""> bool TruncCond = false;<br class="">- if (NewWidth > 0 && BitWidth > NewWidth &&<br class="">- NewWidth >= DL.getLargestLegalIntTypeSizeInBits()) {<br class="">+ if (NewWidth > 0 && NewWidth < BitWidth) {<br class=""> TruncCond = true;<br class=""> IntegerType *Ty = IntegerType::get(SI.getContext(), NewWidth);<br class=""> Builder->SetInsertPoint(&SI);<br class="">- Value *NewCond = Builder->CreateTrunc(SI.getCondition(), Ty, "trunc");<br class="">+ Value *NewCond = Builder->CreateTrunc(Cond, Ty, "trunc");<br class=""> SI.setCondition(NewCond);<br class=""><br class=""> for (auto &C : SI.cases())<br class=""><br class="">Modified: llvm/trunk/test/Transforms/InstCombine/narrow-switch.ll<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/narrow-switch.ll?rev=274233&r1=274232&r2=274233&view=diff<br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/InstCombine/narrow-switch.ll (original)<br class="">+++ llvm/trunk/test/Transforms/InstCombine/narrow-switch.ll Thu Jun 30 09:51:21 2016<br class="">@@ -3,20 +3,16 @@<br class=""> ; RUN: opt < %s -instcombine -S -default-data-layout=n32 | FileCheck %s --check-prefix=ALL --check-prefix=CHECK32<br class=""> ; RUN: opt < %s -instcombine -S -default-data-layout=n32:64 | FileCheck %s --check-prefix=ALL --check-prefix=CHECK64<br class=""><br class="">+; In all cases, the data-layout is irrelevant. We should shrink as much as possible in InstCombine<br class="">+; and allow the backend to expand as much as needed to ensure optimal codegen for any target.<br class="">+<br class=""> define i32 @positive1(i64 %a) {<br class="">-; CHECK32-LABEL: @positive1(<br class="">-; CHECK32: switch i32<br class="">-; CHECK32-NEXT: i32 10, label %return<br class="">-; CHECK32-NEXT: i32 100, label %sw.bb1<br class="">-; CHECK32-NEXT: i32 1001, label %sw.bb2<br class="">-; CHECK32-NEXT: ]<br class="">-;<br class="">-; CHECK64-LABEL: @positive1(<br class="">-; CHECK64: switch i64<br class="">-; CHECK64-NEXT: i64 10, label %return<br class="">-; CHECK64-NEXT: i64 100, label %sw.bb1<br class="">-; CHECK64-NEXT: i64 1001, label %sw.bb2<br class="">-; CHECK64-NEXT: ]<br class="">+; ALL-LABEL: @positive1(<br class="">+; ALL: switch i32<br class="">+; ALL-NEXT: i32 10, label %return<br class="">+; ALL-NEXT: i32 100, label %sw.bb1<br class="">+; ALL-NEXT: i32 1001, label %sw.bb2<br class="">+; ALL-NEXT: ]<br class=""> ;<br class=""> entry:<br class=""> %and = and i64 %a, 4294967295<br class="">@@ -41,19 +37,12 @@ return:<br class=""> }<br class=""><br class=""> define i32 @negative1(i64 %a) {<br class="">-; CHECK32-LABEL: @negative1(<br class="">-; CHECK32: switch i32<br class="">-; CHECK32-NEXT: i32 -10, label %return<br class="">-; CHECK32-NEXT: i32 -100, label %sw.bb1<br class="">-; CHECK32-NEXT: i32 -1001, label %sw.bb2<br class="">-; CHECK32-NEXT: ]<br class="">-;<br class="">-; CHECK64-LABEL: @negative1(<br class="">-; CHECK64: switch i64<br class="">-; CHECK64-NEXT: i64 -10, label %return<br class="">-; CHECK64-NEXT: i64 -100, label %sw.bb1<br class="">-; CHECK64-NEXT: i64 -1001, label %sw.bb2<br class="">-; CHECK64-NEXT: ]<br class="">+; ALL-LABEL: @negative1(<br class="">+; ALL: switch i32<br class="">+; ALL-NEXT: i32 -10, label %return<br class="">+; ALL-NEXT: i32 -100, label %sw.bb1<br class="">+; ALL-NEXT: i32 -1001, label %sw.bb2<br class="">+; ALL-NEXT: ]<br class=""> ;<br class=""> entry:<br class=""> %or = or i64 %a, -4294967296<br class="">@@ -115,17 +104,11 @@ return:<br class=""> ; to the recomputed condition.<br class=""><br class=""> define void @trunc64to59(i64 %a) {<br class="">-; CHECK32-LABEL: @trunc64to59(<br class="">-; CHECK32: switch i59<br class="">-; CHECK32-NEXT: i59 0, label %sw.bb1<br class="">-; CHECK32-NEXT: i59 18717182647723699, label %sw.bb2<br class="">-; CHECK32-NEXT: ]<br class="">-;<br class="">-; CHECK64-LABEL: @trunc64to59(<br class="">-; CHECK64: switch i64<br class="">-; CHECK64-NEXT: i64 0, label %sw.bb1<br class="">-; CHECK64-NEXT: i64 18717182647723699, label %sw.bb2<br class="">-; CHECK64-NEXT: ]<br class="">+; ALL-LABEL: @trunc64to59(<br class="">+; ALL: switch i59<br class="">+; ALL-NEXT: i59 0, label %sw.bb1<br class="">+; ALL-NEXT: i59 18717182647723699, label %sw.bb2<br class="">+; ALL-NEXT: ]<br class=""> ;<br class=""> entry:<br class=""> %tmp0 = and i64 %a, 15<br class=""><br class="">Modified: llvm/trunk/test/Transforms/InstCombine/pr21651.ll<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr21651.ll?rev=274233&r1=274232&r2=274233&view=diff<br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/InstCombine/pr21651.ll (original)<br class="">+++ llvm/trunk/test/Transforms/InstCombine/pr21651.ll Thu Jun 30 09:51:21 2016<br class="">@@ -6,9 +6,9 @@ target datalayout = "n8:16:32:64"<br class=""><br class=""> define void @PR21651() {<br class=""> ; CHECK-LABEL: @PR21651(<br class="">-; CHECK-NEXT: switch i2 0, label %out [<br class="">-; CHECK-NEXT: i2 0, label %out<br class="">-; CHECK-NEXT: i2 1, label %out<br class="">+; CHECK-NEXT: switch i1 false, label %out [<br class="">+; CHECK-NEXT: i1 false, label %out<br class="">+; CHECK-NEXT: i1 true, label %out<br class=""> ; CHECK-NEXT: ]<br class=""> ; CHECK: out:<br class=""> ; CHECK-NEXT: ret void<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class="">llvm-commits@lists.llvm.org<br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></div></blockquote></div><br class=""></div></body></html>