On 05/18/2016 05:29 AM, Nathan Sidwell wrote:
> On 05/17/16 17:30, Cesar Philippidis wrote:
>> On 05/17/2016 02:22 PM, Andrew Pinski wrote:

>>>> gcc.sum
>>>> Tests that now fail, but worked before:
>>>>
>>>> nvptx-none-run: gcc.c-torture/execute/20100316-1.c   -Os  execution
>>>> test
>>>> nvptx-none-run: gcc.c-torture/execute/20100708-1.c   -O1  execution
>>>> test
>>>> nvptx-none-run: gcc.c-torture/execute/20100805-1.c   -O0  execution
>>>> test
>>>> nvptx-none-run: gcc.dg/torture/pr52028.c   -O3 -fomit-frame-pointer
>>>> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
>>>> nvptx-none-run: gcc.dg/torture/pr52028.c   -O3 -g  execution test
>>>>
> 
> Please determine why these now fail.

Those were failing intermittently, at least on my desktop. I'll look
into that it next.

>> +(define_expand "sincossf3"
>> +  [(set (match_operand:SF 0 "nvptx_register_operand" "=R")
>> +    (unspec:SF [(match_operand:SF 2 "nvptx_register_operand" "R")]
>> +               UNSPEC_COS))
>> +   (set (match_operand:SF 1 "nvptx_register_operand" "=R")
>> +    (unspec:SF [(match_dup 2)] UNSPEC_SIN))]
>> +  "flag_unsafe_math_optimizations"
>> +{
>> +  emit_insn (gen_sinsf2 (operands[1], operands[2]));
>> +  emit_insn (gen_cossf2 (operands[0], operands[2]));
>> +
>> +  DONE;
>> +})
> 
> Why the emit_insn code?  that seems to be replicating the RTL
> representation -- you're saying the same thing twice.
> 
> Doesn't operands[2] need (conditionally) copying to a new register --
> what if it aliases operands[1]?

This patch does that now.

>> +++ b/gcc/testsuite/gcc.target/nvptx/sincos-2.c
>> @@ -0,0 +1,30 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -ffast-math" } */
>> +
> 
> What is this test trying to test?  I'm puzzled by it.  (btw, don't use
> assert, either abort, exit(1) or return from main.)

My intent was to verify that I got the sin and cos arguments right,
i.e., make sure that this sincos expansion didn't mix up sin(x) with
cos(x). I guess I can create a test that uses vprintf and scans
dg-output for the proper results. But in this patch I just omitted that
test case altogether.

Is this patch ok for trunk?

Cesar