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