[PATCH] LoopVectorizer: Don't attempt to vectorize extractelement instructions

[Hal Finkel]

----- Original Message -----
> 
> 
> 
> On Oct 25, 2013, at 1:29 PM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- Original Message -----
> 
> 
> You are disabling vectorization for any extractelement. Do they all
> not work or only the once with a vector operand? (I am not sure we
> care whether we vectorize code with extractelement for this to be
> important, I guess).
> 
> // Check that the instruction return type is vectorizable.
> - if (!VectorType::isValidElementType(it->getType()) &&
> - !it->getType()->isVoidTy()) {
> + // Also, we can't vectorize extractelement instructions.
> + if ((!VectorType::isValidElementType(it->getType()) &&
> + !it->getType()->isVoidTy()) ||
> isa<ExtractElementInst>(it)) {
> 
> All valid extractelement instructions have a vector operand. Maybe
> you're thinking about extractvalue (which extracts from aggregates)?
> 
> 
> 
> 
> 
> I think if you specify a triple you must put the test in the targets
> subdirectory:
> 
> +++ b/test/Transforms/LoopVectorize/ee-crash.ll
> @@ -0,0 +1,35 @@
> +; RUN: opt < %s -loop-vectorize -force-vector-unroll=1
> -force-vector-width=4 -S | FileCheck %s
> +
> +target datalayout =
> "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
> +target triple = "x86_64-apple-macosx10.8.0”
> 
> Otherwise, the test might fail when llvm is not compiled with x86_64
> support.
> 
> I don't think that is a problem; or, if it is, then
> test/Transforms/LoopVectorize/vectorize-once.ll has a problem too.
> 
> 
> 
> 
> It is a problem for people who don’t build the x86 target. The
> different targets have lit config files that require the different
> targets to be built.

Yes, but I don't see how specifying a target triple will cause that to be a problem. I'm forcing the vector width here, so we're not dependent on x86's TTI implementation. Also 43 of the 65 tests in that directory (test/Transforms/LoopVectorize) specify a target triple. I thought only those tests that depend on x86's TTI need to be in the X86 directory. no?

 -Hal

> 
> 
> 
> 
> 
> Thanks again,
> Hal
> 
> 
> 
> 
> 
> Thanks,
> Arnold
> 
> On Oct 25, 2013, at 3:13 PM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> [With the patch this time].
> 
> ----- Original Message -----
> 
> 
> Nadav, Arnold,
> 
> The loop vectorizer does not currently understand how to vectorize
> extractelement instructions. The existing check, which excluded
> all
> vector-valued instructions, did not catch extractelement
> instructions
> because
> it checked only the return value. As a result, vectorization would
> proceed,
> producing illegal instructions like this:
> 
> %58 = extractelement <2 x i32> %15, i32 0
> %59 = extractelement i32 %58, i32 0
> 
> where the second extractelement is illegal because its first
> operand
> is not a vector. Later passes (or the verifier) would then crash.
> 
> We could teach the vectorizer to handle this case when
> vectorizing,
> but I think that might be a task better associated with a larger
> effort in vectorizing vectorized loops. Please review.
> 
> Thanks again,
> Hal
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> <lv-ee-crash.patch>
> 
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory