[llvm] r274233 - [InstCombine] shrink switch conditions better (PR24766)
Aditya Nandakumar via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 9 13:17:19 PST 2016
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/07acc19e/attachment.html>
More information about the llvm-commits
mailing list