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 10/02/13 16:53, Jason Merrill wrote:

Sorry for the delay, and thank you for the review.

On 08/27/2013 04:03 PM, Aldy Hernandez wrote:
+  /* First, try to parse as an initialized declaration.  See
+     cp_parser_condition, from whence the bulk of this is copied.  */

You didn't do this copy, but I'd appreciate it if you could reintegrate
this with cp_parser_condition.  I notice that there's already been
significant drift: this copied code doesn't recognize {} initialization.

This has the potential of throwing my mind for a spin. Can I do this as a followup, and keep it simple for now?

+          else if (!TREE_TYPE (e) || !TREE_CONSTANT (e)
+               || !INTEGRAL_TYPE_P (TREE_TYPE (e)))
+        cp_parser_error (parser,
+                 "step size must be an integer constant");

Can't the step size be a value-dependent expression like a template
non-type parameter?

Fixed. I also found some nuances in how we were (NOT) handling variables as the standard mentioned, both for C and C++. Fixed as well, and added testcases when appropriate.

+  if (cp_lexer_next_token_is_keyword (parser->lexer, RID_STATIC)
+      || cp_lexer_next_token_is_keyword (parser->lexer, RID_REGISTER)
+      || cp_lexer_next_token_is_keyword (parser->lexer, RID_EXTERN)
+      || cp_lexer_next_token_is_keyword (parser->lexer, RID_MUTABLE)
+      || cp_lexer_next_token_is_keyword (parser->lexer, RID_THREAD))
+    {
+      error_at (loc, "storage class is not allowed");
+      cp_lexer_consume_token (parser->lexer);
+    }

How is this different from any other for-loop?

Sigh, frustratingly because the standard says so.

+  gcc_assert (for_keyword == RID_FOR);
+  if (!cp_lexer_next_token_is_keyword (parser->lexer, for_keyword))
+    {
+      if (for_keyword == RID_FOR)

A lot of code in this function tests for_keyword after the assert.  A
comment lower down says

+      /* Handle _Cilk_for here when implemented.  */
+      gcc_unreachable ();

So do we need the earlier assert?  At least add a comment to it.


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]