Re: [PATCH] [4.3 projects] Verctorizer - versioning for alias

[Dorit Nuzman <DORIT@il.ibm.com>]

> Please find below updated patch after implementation of comments
> from Dorit and Zdenek.
>
>   Here is the list of changes from previous patch:
>   - A new testcase when IN and OUT overlap in such a way that
> the vectorized copy of the loop would produce wrong results if
> it were used added(vect-vfa-04.c).
>   - Fixed the size of segment accessed by data reference in cases
> when realignment loads are used.

Is there a testcase that catches this case?

>   - POINTER_PLUS_EXPR used insted of PLUS_EXPR for pointer
> arithmentics.
>   - Parameters "vect-max-version-for-alignment-checks" and
> "vect-max-version-for-alias-checks" documented in invoke.texi.

(make sure you pass make info/make dvi, if you haven't yet)

>   - Fixed typos and unclear comments.
>

So you're going to address the rest of the comments in a separate patch
(cost-model updates, improved handling of interleaved-accesses, adding
testcases that combine interleaving and versioning-for-aliasing, factoring
out the versioning related stuff from vect_transform_loop)? (that would be
fine with me, just want to make sure you're planning to address these
issues too).

>
> Bootstrapped with vectorization enabled on x86_64, regtested on
> x86_64.
> Okay for mainline after boostrap and regression testing on PPC?
>
>
>
> 2007-08-14  Victor Kaplansky <victork@il.ibm.com>
>
> ChangeLog:
>
...
>              (vect_enhance_data_refs_alignment): Define local variable
>              vect_versioning_for_alias_required, don't perform
>              versioning for alignment if versioning for alias is
>              required.

so I asked this before: I think you mean "don't perform **peeling** for
alignment if versioning for alias is required", right?

Lastly, about:

+static tree
+vect_vfa_segment_size (struct data_reference *dr, tree vect_factor)
+{
+  tree segment_length;
+
+  if (vect_supportable_dr_alignment (dr) ==
dr_unaligned_software_pipeline)
+    {
+      tree vector_size =
+        build_int_cst (integer_type_node,
+          GET_MODE_SIZE (TYPE_MODE (STMT_VINFO_VECTYPE

I think you can use TYPE_VECTOR_SUBPARTS() instead of
GET_MODE_SIZE (TYPE_MODE ()) (just a tiny bit more compact)

Also, for more compact code, you could compute the vf*step once and reuse
it for both then and else. For your consideration.

+         (vinfo_for_stmt (DR_STMT (dr))))));
+
+      segment_length =
+     fold_convert (sizetype,
+       fold_build2 (PLUS_EXPR, integer_type_node,
+         fold_build2 (MULT_EXPR, integer_type_node, DR_STEP (dr),
+                  vect_factor),
+         vector_size));
+
+
+    }
+  else
+    {
+      segment_length =
+     fold_convert (sizetype,
+       fold_build2 (MULT_EXPR, integer_type_node, DR_STEP (dr),
+                  vect_factor));
+    }
+
+    return segment_length;
+}

OK otherwise,

thanks,
dorit

> (See attached file: vfa-n.txt)
> [attachment "vfa-n.txt" deleted by Dorit Nuzman/Haifa/IBM]