On Wed, Nov 13, 2013 at 01:21:03PM +0100, Jakub Jelinek wrote:
> Sergey has kindly tested the patch on SPEC2k6, but on 4 tests it revealed
> an ICE. Here is an incremental fix for that, for now it just punts on
> those. In theory the invariant conditional loads could be handled e.g.
> using gather, or perhaps V{,P}MOVMSKP{S,D,B} on the mask followed by
> conditional scalar load of the value if any bits in the mask are set,
> then broadcasting it.
And another issue, ICE in predcom. The problem is that the patch creates
DR_READ data ref for the MASK_LOAD internal call and !DR_READ data ref
for the MASK_STORE, which is handled properly in the vectorizer, but
apparently other data ref consumers aren't prepared for it.
So, in the spirit of the recommended change to do some dataref processing
privately in tree-vect-data-refs.c, the first patch (bootstrapped/regtested
on x86_64-linux and i686-linux) handles MASK_LOAD and MASK_STORE solely in
the vectorizer and leaves those as failing the data ref analysis otherwise.
Or, alternatively, we would need to handle those or punt in the other
consumers. Untested second patch does the punting in predcom and phiopt,
but there are others still unverified. Apparently e.g. predcom isn't
prepared to handle data refs on any calls at all, which can happen already
before my patches, and apparently other passes aren't either.
E.g. I've tried to create a testcase for pcom (which asserts that DR_STMT
is either gimple assign or PHI):
struct S { int i; };
__attribute__((const, noinline, noclone))
struct S foo (int x)
{
struct S s;
s.i = x;
return s;
}
int a[2048], b[2048], c[2048], d[2048];
struct S e[2048];
__attribute__((noinline, noclone)) void
bar (void)
{
int i;
for (i = 0; i < 1024; i++)
{
e[i] = foo (i);
a[i+2] = a[i] + a[i+1];
b[10] = b[10] + i;
c[i] = c[2047 - i];
d[i] = d[i + 1];
}
}
int
main ()
{
int i;
bar ();
for (i = 0; i < 1024; i++)
if (e[i].i != i)
__builtin_abort ();
return 0;
}
but to my surprise pcom didn't fail on it (because the data reference
doesn't have a gimple type and thus isn't suitable_reference_p),
but e.g. ldist miscompiled it (completely ignored it as if there wasn't
e[i] = foo (i); and let it out of the loop, so now it fails at runtime).
Richard, any preferences on whether you'd prefer MASK_LOAD/MASK_STORE
to be handled as data references only for the vectorization, or also
other passes? Even in the former case, we need to investigate the
non-vectorizer data ref users how they handle calls and fix the ldist
bug.
Jakub
2013-11-13 Jakub Jelinek <ja...@redhat.com>
* tree-vect-data-refs.c (vect_analyze_data_refs): Inline by hand
find_data_references_in_loop and find_data_references_in_bb, if
find_data_references_in_stmt fails, still allow calls to MASK_LOAD
and MASK_STORE internal calls.
--- gcc/tree-vect-data-refs.c.jj 2013-11-13 19:19:07.433383289 +0100
+++ gcc/tree-vect-data-refs.c 2013-11-13 20:27:11.209181089 +0100
@@ -3191,10 +3191,11 @@ vect_analyze_data_refs (loop_vec_info lo
if (loop_vinfo)
{
+ basic_block *bbs = LOOP_VINFO_BBS (loop_vinfo);
+
loop = LOOP_VINFO_LOOP (loop_vinfo);
- if (!find_loop_nest (loop, &LOOP_VINFO_LOOP_NEST (loop_vinfo))
- || find_data_references_in_loop
- (loop, &LOOP_VINFO_DATAREFS (loop_vinfo)))
+ datarefs = LOOP_VINFO_DATAREFS (loop_vinfo);
+ if (!find_loop_nest (loop, &LOOP_VINFO_LOOP_NEST (loop_vinfo)))
{
if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
@@ -3203,7 +3204,62 @@ vect_analyze_data_refs (loop_vec_info lo
return false;
}
- datarefs = LOOP_VINFO_DATAREFS (loop_vinfo);
+ for (i = 0; i < loop->num_nodes; i++)
+ {
+ gimple_stmt_iterator gsi;
+
+ for (gsi = gsi_start_bb (bbs[i]); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple stmt = gsi_stmt (gsi);
+ if (!find_data_references_in_stmt (loop, stmt, &datarefs))
+ {
+ bool is_read = false;
+ tree ref;
+ data_reference_p drt;
+
+ if (is_gimple_call (stmt) && gimple_call_internal_p (stmt))
+ switch (gimple_call_internal_fn (stmt))
+ {
+ case IFN_GOMP_SIMD_LANE:
+ {
+ struct loop *cloop = loop_containing_stmt (stmt);
+ tree uid = gimple_call_arg (stmt, 0);
+ gcc_assert (TREE_CODE (uid) == SSA_NAME);
+ if (cloop && cloop->simduid == SSA_NAME_VAR (uid))
+ continue;
+ break;
+ }
+ case IFN_MASK_LOAD:
+ is_read = true;
+ /* FALLTHRU */
+ case IFN_MASK_STORE:
+ ref = build2 (MEM_REF,
+ is_read
+ ? TREE_TYPE (gimple_call_lhs (stmt))
+ : TREE_TYPE (gimple_call_arg (stmt, 3)),
+ gimple_call_arg (stmt, 0),
+ gimple_call_arg (stmt, 1));
+ drt = create_data_ref (loop,
+ loop_containing_stmt (stmt),
+ ref, stmt, is_read);
+ gcc_assert (drt);
+ datarefs.safe_push (drt);
+ continue;
+ default:
+ break;
+ }
+ LOOP_VINFO_DATAREFS (loop_vinfo) = datarefs;
+ if (dump_enabled_p ())
+ dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+ "not vectorized: loop contains function "
+ "calls or data references that cannot "
+ "be analyzed\n");
+ return false;
+ }
+ }
+ }
+
+ LOOP_VINFO_DATAREFS (loop_vinfo) = datarefs;
}
else
{
--- gcc/tree-data-ref.c.jj 2013-11-13 19:19:07.000000000 +0100
+++ gcc/tree-data-ref.c 2013-11-13 20:28:16.753878385 +0100
@@ -4313,8 +4313,8 @@ compute_all_dependences (vec<data_refere
typedef struct data_ref_loc_d
{
- /* The memory reference. */
- tree ref;
+ /* Position of the memory reference. */
+ tree *pos;
/* True if the memory reference is read. */
bool is_read;
@@ -4329,7 +4329,7 @@ get_references_in_stmt (gimple stmt, vec
{
bool clobbers_memory = false;
data_ref_loc ref;
- tree op0, op1;
+ tree *op0, *op1;
enum gimple_code stmt_code = gimple_code (stmt);
/* ASM_EXPR and CALL_EXPR may embed arbitrary side effects.
@@ -4339,26 +4339,16 @@ get_references_in_stmt (gimple stmt, vec
&& !(gimple_call_flags (stmt) & ECF_CONST))
{
/* Allow IFN_GOMP_SIMD_LANE in their own loops. */
- if (gimple_call_internal_p (stmt))
- switch (gimple_call_internal_fn (stmt))
- {
- case IFN_GOMP_SIMD_LANE:
- {
- struct loop *loop = gimple_bb (stmt)->loop_father;
- tree uid = gimple_call_arg (stmt, 0);
- gcc_assert (TREE_CODE (uid) == SSA_NAME);
- if (loop == NULL
- || loop->simduid != SSA_NAME_VAR (uid))
- clobbers_memory = true;
- break;
- }
- case IFN_MASK_LOAD:
- case IFN_MASK_STORE:
- break;
- default:
+ if (gimple_call_internal_p (stmt)
+ && gimple_call_internal_fn (stmt) == IFN_GOMP_SIMD_LANE)
+ {
+ struct loop *loop = gimple_bb (stmt)->loop_father;
+ tree uid = gimple_call_arg (stmt, 0);
+ gcc_assert (TREE_CODE (uid) == SSA_NAME);
+ if (loop == NULL
+ || loop->simduid != SSA_NAME_VAR (uid))
clobbers_memory = true;
- break;
- }
+ }
else
clobbers_memory = true;
}
@@ -4372,15 +4362,15 @@ get_references_in_stmt (gimple stmt, vec
if (stmt_code == GIMPLE_ASSIGN)
{
tree base;
- op0 = gimple_assign_lhs (stmt);
- op1 = gimple_assign_rhs1 (stmt);
+ op0 = gimple_assign_lhs_ptr (stmt);
+ op1 = gimple_assign_rhs1_ptr (stmt);
- if (DECL_P (op1)
- || (REFERENCE_CLASS_P (op1)
- && (base = get_base_address (op1))
+ if (DECL_P (*op1)
+ || (REFERENCE_CLASS_P (*op1)
+ && (base = get_base_address (*op1))
&& TREE_CODE (base) != SSA_NAME))
{
- ref.ref = op1;
+ ref.pos = op1;
ref.is_read = true;
references->safe_push (ref);
}
@@ -4389,35 +4379,16 @@ get_references_in_stmt (gimple stmt, vec
{
unsigned i, n;
- ref.is_read = false;
- if (gimple_call_internal_p (stmt))
- switch (gimple_call_internal_fn (stmt))
- {
- case IFN_MASK_LOAD:
- ref.is_read = true;
- case IFN_MASK_STORE:
- ref.ref = build2 (MEM_REF,
- ref.is_read
- ? TREE_TYPE (gimple_call_lhs (stmt))
- : TREE_TYPE (gimple_call_arg (stmt, 3)),
- gimple_call_arg (stmt, 0),
- gimple_call_arg (stmt, 1));
- references->safe_push (ref);
- return false;
- default:
- break;
- }
-
- op0 = gimple_call_lhs (stmt);
+ op0 = gimple_call_lhs_ptr (stmt);
n = gimple_call_num_args (stmt);
for (i = 0; i < n; i++)
{
- op1 = gimple_call_arg (stmt, i);
+ op1 = gimple_call_arg_ptr (stmt, i);
- if (DECL_P (op1)
- || (REFERENCE_CLASS_P (op1) && get_base_address (op1)))
+ if (DECL_P (*op1)
+ || (REFERENCE_CLASS_P (*op1) && get_base_address (*op1)))
{
- ref.ref = op1;
+ ref.pos = op1;
ref.is_read = true;
references->safe_push (ref);
}
@@ -4426,11 +4397,11 @@ get_references_in_stmt (gimple stmt, vec
else
return clobbers_memory;
- if (op0
- && (DECL_P (op0)
- || (REFERENCE_CLASS_P (op0) && get_base_address (op0))))
+ if (*op0
+ && (DECL_P (*op0)
+ || (REFERENCE_CLASS_P (*op0) && get_base_address (*op0))))
{
- ref.ref = op0;
+ ref.pos = op0;
ref.is_read = false;
references->safe_push (ref);
}
@@ -4457,7 +4428,7 @@ find_data_references_in_stmt (struct loo
FOR_EACH_VEC_ELT (references, i, ref)
{
dr = create_data_ref (nest, loop_containing_stmt (stmt),
- ref->ref, stmt, ref->is_read);
+ *ref->pos, stmt, ref->is_read);
gcc_assert (dr != NULL);
datarefs->safe_push (dr);
}
@@ -4486,7 +4457,7 @@ graphite_find_data_references_in_stmt (l
FOR_EACH_VEC_ELT (references, i, ref)
{
- dr = create_data_ref (nest, loop, ref->ref, stmt, ref->is_read);
+ dr = create_data_ref (nest, loop, *ref->pos, stmt, ref->is_read);
gcc_assert (dr != NULL);
datarefs->safe_push (dr);
}
--- gcc/tree-predcom.c.jj 2013-11-12 23:10:09.000000000 +0100
+++ gcc/tree-predcom.c 2013-11-13 19:47:04.701663975 +0100
@@ -722,6 +722,9 @@ split_data_refs_to_components (struct lo
just fail. */
goto end;
}
+ /* predcom pass isn't prepared to handle calls with data references. */
+ if (is_gimple_call (DR_STMT (dr)))
+ goto end;
dr->aux = (void *) (size_t) i;
comp_father[i] = i;
comp_size[i] = 1;
--- gcc/tree-ssa-phiopt.c.jj 2013-11-12 23:10:09.000000000 +0100
+++ gcc/tree-ssa-phiopt.c 2013-11-13 20:09:28.385656663 +0100
@@ -1714,6 +1714,8 @@ cond_if_else_store_replacement (basic_bl
then_store = DR_STMT (then_dr);
then_lhs = gimple_get_lhs (then_store);
+ if (then_lhs == NULL_TREE)
+ continue;
found = false;
FOR_EACH_VEC_ELT (else_datarefs, j, else_dr)
@@ -1723,6 +1725,8 @@ cond_if_else_store_replacement (basic_bl
else_store = DR_STMT (else_dr);
else_lhs = gimple_get_lhs (else_store);
+ if (else_lhs == NULL_TREE)
+ continue;
if (operand_equal_p (then_lhs, else_lhs, 0))
{
