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: [cilkplus] misc cleanups for <#pragma simd> implementation

On 04/08/13 08:59, Iyer, Balaji V wrote:
Hi Aldy,
	Here are the things I  found with the patch. All my comments have "BVI:" in front of them.

BTW, it would be nice if you could use standard mailer quotation when responding (">", etc).

-  return;

BVI: I am OK with removing this return, but the reason why I put it there is because it gets easier for me to set the break point there.

This is not standard practice in GCC source code. It will have to be removed if we ever merge.

-      if (context == pragma_external)
-	{
-	  c_parser_error (parser,"pragma grainsize must be inside a function");
-	  return false;
-	}
-      if (flag_enable_cilk)
-	c_parser_cilk_grainsize (parser);
-      else
-	{
-	  warning (0, "pragma grainsize ignored");
-	  c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
-	}
+      if (!c_parser_pragma_simd_ok_p (parser, context))
+	return false;
+      cilkplus_local_simd_values.loc = loc;
+      c_parser_cilk_grainsize (parser);

BVI: This is incorrect. #pragma grainsize is part of cilk keywords. It has no relation to the pragma simd and it will work wthout vectorization support.

Fixed, let me know if the current implementation is correct.

+// FIXME: We should really rewrite all this psv* business to use vectors.
+/* Given an index into the pragma simd list (PSV_INDEX), find its
+   entry and return it.  */

BVI: I am in the process of doing so. I will send out that patch as soon as I get some free time.

Ok, I have left all the FIXMEs so we don't miss any of them.

    for (ps_iter = psv_head; ps_iter->ptr_next != NULL;
         ps_iter = ps_iter->ptr_next)
-    {
-      ;
-    }
+    ;

BVI: Are you sure the compiler let you get away this this? It gave me a warning once (in stage2 I believe).

Sure, I've done it for years.

BVI: I have fixed these scripts already: the correct notation that I have used is "cilkplus_<type>_<language>_<compile/execute/errors>.exp

Ok, I have removed them from my patch.
BVI: Why did you replace a space with a tab?

Whoops, removed the space (and the tab).

How about this?

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]