This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]