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