diff mbox

cilkplus fails without pthreads for me

Message ID 6CDCA042-220B-4F95-A92B-F0CA82B6EECA@comcast.net
State New
Headers show

Commit Message

Mike Stump Jan. 4, 2016, 7:17 p.m. UTC 
On Jan 4, 2016, at 7:22 AM, Nathan Sidwell <nathan@acm.org> wrote:
> On 01/01/16 13:13, Mike Stump wrote:
>> cilkplus fails without pthreads for me:
>> 
>> xg++: error: unrecognized command line option '-pthread'
>> compiler exited with status 1
>> output is:
>> xg++: error: unrecognized command line option '-pthread'
>> 
>> FAIL: c-c++-common/attr-simd-3.c  -std=gnu++14 PR68158 (test for errors, line 5)
>> 
>> I suspect pthreads is a fairly hard requirement.  Either a test compile and link needs to be done, or we need to be able to whack out the tests on non-pthread systems.
>> 
>> Ok?
> 
> Probably not.  See  the discussion at https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01882.html  Admittedly, that was annotating the test directly,  but Rainer's comment suggests to me that requiring pthreads would be too great a hammer.
> 
> You don't say what target -- is it a system where a target triplet is insufficient for this check?

That was on purpose.  All non-pthreads targets.  One cannot ascertain if a system has pthreads by checking a target triplet.  This is the problem I want fixed.  Adding a clause for one such target doesn’t fix all such targets.  I didn’t read Rainer’s comments as authoritative for the design of cilk.  I also don’t read them as inconsistent with my proposed patch.

Since Bernd Ok it, and that is consistent with the apparent design to me, I’m going with his approval.  Here is my take, the runtime is written to require pthreads, that’s just how it is.  Since it is, the testing for it is going to require pthreads.  That’s just how it is.  We gate off all tests that require cilk on systems that don’t have pthreads.  Special escapes from the general rule can happen before or after the newly added clause on a per target or some other metric.

The next proposed patch is:


I believe this is now, not required nor desirable.  The attr-simd-3.c test case on NVPTX should be able to show if this is on the right track.

Ok?

Comments

Nathan Sidwell Jan. 4, 2016, 7:29 p.m. UTC | #1 
On 01/04/16 14:17, Mike Stump wrote:

> The next proposed patch is:
>
> Index: target-supports.exp
> ===================================================================
> --- target-supports.exp	(revision 232062)
> +++ target-supports.exp	(working copy)
> @@ -1442,11 +1442,6 @@ proc check_effective_target_cilkplus { }
>   	return 0;
>       }
>
> -    # No pthreads on NVPTX
> -    if { [istarget nvptx-*-*] } {
> -	return 0;
> -    }
> -
>       if { ! [check_effective_target_pthread] } {
>   	return 0;
>       }
>
> I believe this is now, not required nor desirable.  The attr-simd-3.c test case on NVPTX should be able to show if this is on the right track.
>
> Ok?

works for me, thanks.
diff mbox

Patch

Index: target-supports.exp
===================================================================
--- target-supports.exp	(revision 232062)
+++ target-supports.exp	(working copy)
@@ -1442,11 +1442,6 @@  proc check_effective_target_cilkplus { }
 	return 0;
     }
 
-    # No pthreads on NVPTX
-    if { [istarget nvptx-*-*] } {
-	return 0;
-    }
-
     if { ! [check_effective_target_pthread] } {
 	return 0;
     }