[llvm] r179786 - In the function InstCombiner::visitExtractElementInst() removed the limitation that extract is promoted over a cast only if the cast has only one use.

[Dmitry Babokin]

Hi Anat,

The patch look good to me. It also fixes all the problems, that I
observed with previous patch - I got several fails in ISPC compiler
testing (ispc.github.com).

Thanks for quick turnaround!

-Dmitry

On Mon, Apr 22, 2013 at 8:40 PM, Shemer, Anat <anat.shemer at intel.com> wrote:
> Hi,
>
>
>
> Attached please see a patch with the fix to the bug and the test.
>
>
>
> I changed back the operations executed when extract(cast) is transformed to
> cast(extract). It uses the Builder class as before. In addition the result
> node is added to the Worklist, so all the previous extract users will become
> the new scalar cast users.
>
>
>
> Looking forward to your feedback,
>
> Anat
>
>
>
>
>
> From: Dmitry Babokin [mailto:babokin at gmail.com]
>
> Sent: Monday, April 22, 2013 17:21
> To: Shemer, Anat
> Cc: llvm-commits
> Subject: Re: [llvm] r179786 - In the function
> InstCombiner::visitExtractElementInst() removed the limitation that extract
> is promoted over a cast only if the cast has only one use.
>
>
>
> This commit introduces a regression. Please see bug
> http://llvm.org/bugs/show_bug.cgi?id=15818 for more details.
>
>
>
> The illegal transformation is the following:
>
> Before:
>
>   %retval.i.i = bitcast <8 x float> %floatval.i.i to <8 x i32>
>
>   %vf.i = bitcast <8 x i32> %retval.i.i to <8 x float>
>
>   %aFOO_load20_offset_load_to_int32 = fptosi <8 x float> %vf.i to <8 x i32>
>
>   %b_load28_to_int32 = fptosi float %b to i32
>
>   %sub_b_load28_to_int32_ = add i32 %b_load28_to_int32, -2
>
>   %v.i.i = extractelement <8 x i32> %aFOO_load20_offset_load_to_int32, i32
> %sub_b_load28_to_int32_
>
> After:
>
>   %0 = extractelement <8 x float> %floatval.i.i, i32 %sub_b_load28_to_int32_
>
>   %b_load28_to_int32 = fptosi float %b to i32
>
>   %sub_b_load28_to_int32_ = add i32 %b_load28_to_int32, -2
>
>   %v.i.i = fptosi float %0 to i32
>
>
>
> Note that %sub_b_load28_to_int32_ use move before definition.
>
>
>
> -Dmitry
>
>
>
>
>
> On Thu, Apr 18, 2013 at 9:56 PM, Anat Shemer <anat.shemer at intel.com> wrote:
>
> Author: ashemer
> Date: Thu Apr 18 14:56:44 2013
> New Revision: 179786
>
> URL: http://llvm.org/viewvc/llvm-project?rev=179786&view=rev
> Log:
> In the function InstCombiner::visitExtractElementInst() removed the
> limitation that extract is promoted over a cast only if the cast has only
> one use.
>
>
> Added:
>     llvm/trunk/test/Transforms/InstCombine/vec_extract_2elts.ll
> Modified:
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=179786&r1=179785&r2=179786&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp Thu Apr
> 18 14:56:44 2013
> @@ -278,10 +278,10 @@ Instruction *InstCombiner::visitExtractE
>      } else if (CastInst *CI = dyn_cast<CastInst>(I)) {
>        // Canonicalize extractelement(cast) -> cast(extractelement)
>        // bitcasts can change the number of vector elements and they cost
> nothing
> -      if (CI->hasOneUse() && EI.hasOneUse() &&
> -          (CI->getOpcode() != Instruction::BitCast)) {
> -        Value *EE = Builder->CreateExtractElement(CI->getOperand(0),
> -                                                  EI.getIndexOperand());
> +      if (CI->hasOneUse() && (CI->getOpcode() != Instruction::BitCast)) {
> +        Value *EE = InsertNewInstWith(
> +                 ExtractElementInst::Create(CI->getOperand(0),
> EI.getIndexOperand()),
> +          *CI);
>          return CastInst::Create(CI->getOpcode(), EE, EI.getType());
>        }
>      }
>
> Added: llvm/trunk/test/Transforms/InstCombine/vec_extract_2elts.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_extract_2elts.ll?rev=179786&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/vec_extract_2elts.ll (added)
> +++ llvm/trunk/test/Transforms/InstCombine/vec_extract_2elts.ll Thu Apr 18
> 14:56:44 2013
> @@ -0,0 +1,12 @@
> +; RUN: opt < %s -instcombine -S | FileCheck %s
> +
> +define void @test(<4 x i32> %v, i64 *%r1, i64 *%r2) {
> +;CHECK: %1 = extractelement <4 x i32> %v, i32 0
> +;CHECK: %2 = zext i32 %1 to i64
> +        %1 = zext <4 x i32> %v to <4 x i64>
> +        %2 = extractelement <4 x i64> %1, i32 0
> +        store i64 %2, i64 *%r1
> +        store i64 %2, i64 *%r2
> +        ret void
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.