Re: PING: Fwd: Re: [patch] implement Cilk Plus simd loops on trunk

[Aldy Hernandez <aldyh at redhat dot com>]

On 11/01/13 14:21, Jason Merrill wrote:

> Out of curiosity, how are OpenMP and Cilk+ for loops so different that
> they can't share a parsing function?

Ok, this changes everything for the better.  I honestly can't think of a 
good reason why we can't share the parsing with OpenMP.

I have redesigned everything to share the parsing (both for the C and 
C++ FE's), thus making most of your review irrelevant (in a good way 
:)), with the exception of...

> > +/* Cilk Plus - #pragma simd [clause1 ... clauseN]
> > +   Operands like for OMP_FOR.  */
> > +DEFTREECODE (CILK_SIMD, "cilk_simd", tcc_statement, 6)
>
> Could this just be a flag on OMP_SIMD?

The GF_OMP_FOR_* flags are set at gimplification, so unavailable before. 
 I suppose I could rearrange all the calls within omp-low.c/etc to pass 
a flag, but it seems to work cleaner with a CILK_SIMD code.

Jakub: I did find an existing ICE in c_finish_omp_for for the following 
testcase on trunk:

	int *a, *b;
	void foo()
	{
	#pragma simd
	  for (const int ci=0; ci<1000; ++ci)
	    a[ci] = b[ci];
	}

basically the following asserts will fail because the build_modify_expr 
preceding them can return error_mark_node for the above invalid testcase:

-      gcc_assert (TREE_CODE (init) == MODIFY_EXPR);
-      gcc_assert (TREE_OPERAND (init, 0) == decl);

I have fixed it by allowing error_mark_node.

In the above testcase I also found that readonly_error() was diagnosing 
the wrong line and fixed it to accept a location.  These are the only 
two functionality changes I believe I introduced, ahem...fixed.

Theoretically I should submit the above fix as a separate patch, but 
since I'm touching all this already...

Jason/Jakub: I have also left the abstractions I previously introduced 
(c_omp_for_incr_canonicalize_ptr() and cp_parser_omp_for_loop_init()) in 
my previous versions of this patch, since IMHO it cleans up these big 
functions.  I can inline them back in, if you fell strongly about it.

Phew... much cleaner.

How does this look?

Attachment: curr
Description: Text document