diff mbox

[gcc/MIPS] Make loongson3a use fused madd.d

Message ID CAKjxQHk5dKJxPJfYZ6XXhyX8J2bpeBF-ijXUt9uRXhrVXzQg6A@mail.gmail.com
State New
Headers show

Commit Message

Paul Hua Nov. 3, 2016, 11:58 a.m. UTC 
Hi Matthew,

Thanks for your comments, update the patch.

*** gcc/ChangeLog ***

2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>

        * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
        TARGET_LOONGSON_3A.
        (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.

Thanks,
Paul

On Thu, Nov 3, 2016 at 6:31 PM, Matthew Fortune
<Matthew.Fortune@imgtec.com> wrote:
> Paul Hua <paul.hua.gm@gmail.com> writes:
>> Loongson3a has 4 operand fused madd instrcution. This patch set
>> loongson3a use fused madd.d.
>
> Hi Paul,
>
> Thanks for the fix. I was vaguely aware that this was wrong for
> loongson-3a but never confirmed it.
>
> I suspect this change is mechanical enough that it can bypass
> copyright assignment but I'd need a global maintainer to comment.
>
> I've sent you copyright assignment paperwork separately.
>
> Two comments on the patch:
>
>> ChangeLog :
>>
>> *** gcc/ChangeLog ***
>>
>> 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>
>>
>>     config/mips/
>>     * mips.h: Set loongson3a use fused madd.d.
>
> The changelog needs to reference what was changed rather than the
> effect of the change:
>
>         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
>         TARGET_LOONGSON_3A.
>         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.
>
>
>>diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
>>index 81862a9..5076a2b 100644
>>--- a/gcc/config/mips/mips.h
>>+++ b/gcc/config/mips/mips.h
>>@@ -1056,11 +1056,11 @@ struct mips_cpu_info {
>>
>> /* ISA has 4 operand fused madd instructions of the form
>>    'd = [+-] (a * b [+-] c)'.  */
>>-#define ISA_HAS_FUSED_MADD4   TARGET_MIPS8000
>>+#define ISA_HAS_FUSED_MADD4   (TARGET_MIPS8000 || TARGET_LOONGSON_3A)
>>
>> /* ISA has 4 operand unfused madd instructions of the form
>>    'd = [+-] (a * b [+-] c)'.  */
>>-#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000)
>>+#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000 && !TARGET_LOONGSON_3A)
>
> Please split this line and move && !TARGET_LOONGSON_3A to the next line
> under ISA_HAS_FP4.
>
>>
>> /* ISA has 3 operand r6 fused madd instructions of the form
>>    'c = c [+-] (a * b)'.  */
>
> Thanks,
> Matthew
>

Comments

Paul Hua Nov. 17, 2016, 3 a.m. UTC | #1 
ping...

On Thu, Nov 3, 2016 at 7:58 PM, Paul Hua <paul.hua.gm@gmail.com> wrote:
> Hi Matthew,
>
> Thanks for your comments, update the patch.
>
> *** gcc/ChangeLog ***
>
> 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>
>
>         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
>         TARGET_LOONGSON_3A.
>         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.
>
> Thanks,
> Paul
>
> On Thu, Nov 3, 2016 at 6:31 PM, Matthew Fortune
> <Matthew.Fortune@imgtec.com> wrote:
>> Paul Hua <paul.hua.gm@gmail.com> writes:
>>> Loongson3a has 4 operand fused madd instrcution. This patch set
>>> loongson3a use fused madd.d.
>>
>> Hi Paul,
>>
>> Thanks for the fix. I was vaguely aware that this was wrong for
>> loongson-3a but never confirmed it.
>>
>> I suspect this change is mechanical enough that it can bypass
>> copyright assignment but I'd need a global maintainer to comment.
>>
>> I've sent you copyright assignment paperwork separately.
>>
>> Two comments on the patch:
>>
>>> ChangeLog :
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>
>>>
>>>     config/mips/
>>>     * mips.h: Set loongson3a use fused madd.d.
>>
>> The changelog needs to reference what was changed rather than the
>> effect of the change:
>>
>>         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
>>         TARGET_LOONGSON_3A.
>>         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.
>>
>>
>>>diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
>>>index 81862a9..5076a2b 100644
>>>--- a/gcc/config/mips/mips.h
>>>+++ b/gcc/config/mips/mips.h
>>>@@ -1056,11 +1056,11 @@ struct mips_cpu_info {
>>>
>>> /* ISA has 4 operand fused madd instructions of the form
>>>    'd = [+-] (a * b [+-] c)'.  */
>>>-#define ISA_HAS_FUSED_MADD4   TARGET_MIPS8000
>>>+#define ISA_HAS_FUSED_MADD4   (TARGET_MIPS8000 || TARGET_LOONGSON_3A)
>>>
>>> /* ISA has 4 operand unfused madd instructions of the form
>>>    'd = [+-] (a * b [+-] c)'.  */
>>>-#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000)
>>>+#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000 && !TARGET_LOONGSON_3A)
>>
>> Please split this line and move && !TARGET_LOONGSON_3A to the next line
>> under ISA_HAS_FP4.
>>
>>>
>>> /* ISA has 3 operand r6 fused madd instructions of the form
>>>    'c = c [+-] (a * b)'.  */
>>
>> Thanks,
>> Matthew
>>
Matthew Fortune Nov. 17, 2016, 11:14 a.m. UTC | #2 
Hi Jeff,

Am I OK to accept this change without copyright assignment from Paul?

The change is small and there is no other way it could be implemented
anyway if I had someone write it from scratch.

Thanks,
Matthew

> -----Original Message-----

> From: Paul Hua [mailto:paul.hua.gm@gmail.com]

> Sent: 17 November 2016 03:01

> To: Matthew Fortune

> Cc: gcc-patches@gcc.gnu.org; catherine_moore@mentor.com

> Subject: Re: [PATCH,gcc/MIPS] Make loongson3a use fused madd.d

> 

> ping...

> 

> On Thu, Nov 3, 2016 at 7:58 PM, Paul Hua <paul.hua.gm@gmail.com> wrote:

> > Hi Matthew,

> >

> > Thanks for your comments, update the patch.

> >

> > *** gcc/ChangeLog ***

> >

> > 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>

> >

> >         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for

> >         TARGET_LOONGSON_3A.

> >         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.

> >

> > Thanks,

> > Paul

> >

> > On Thu, Nov 3, 2016 at 6:31 PM, Matthew Fortune

> > <Matthew.Fortune@imgtec.com> wrote:

> >> Paul Hua <paul.hua.gm@gmail.com> writes:

> >>> Loongson3a has 4 operand fused madd instrcution. This patch set

> >>> loongson3a use fused madd.d.

> >>

> >> Hi Paul,

> >>

> >> Thanks for the fix. I was vaguely aware that this was wrong for

> >> loongson-3a but never confirmed it.

> >>

> >> I suspect this change is mechanical enough that it can bypass

> >> copyright assignment but I'd need a global maintainer to comment.

> >>

> >> I've sent you copyright assignment paperwork separately.

> >>

> >> Two comments on the patch:

> >>

> >>> ChangeLog :

> >>>

> >>> *** gcc/ChangeLog ***

> >>>

> >>> 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>

> >>>

> >>>     config/mips/

> >>>     * mips.h: Set loongson3a use fused madd.d.

> >>

> >> The changelog needs to reference what was changed rather than the

> >> effect of the change:

> >>

> >>         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for

> >>         TARGET_LOONGSON_3A.

> >>         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.

> >>

> >>

> >>>diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index

> >>>81862a9..5076a2b 100644

> >>>--- a/gcc/config/mips/mips.h

> >>>+++ b/gcc/config/mips/mips.h

> >>>@@ -1056,11 +1056,11 @@ struct mips_cpu_info {

> >>>

> >>> /* ISA has 4 operand fused madd instructions of the form

> >>>    'd = [+-] (a * b [+-] c)'.  */

> >>>-#define ISA_HAS_FUSED_MADD4   TARGET_MIPS8000

> >>>+#define ISA_HAS_FUSED_MADD4   (TARGET_MIPS8000 ||

> TARGET_LOONGSON_3A)

> >>>

> >>> /* ISA has 4 operand unfused madd instructions of the form

> >>>    'd = [+-] (a * b [+-] c)'.  */

> >>>-#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000)

> >>>+#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000 &&

> >>>+!TARGET_LOONGSON_3A)

> >>

> >> Please split this line and move && !TARGET_LOONGSON_3A to the next

> >> line under ISA_HAS_FP4.

> >>

> >>>

> >>> /* ISA has 3 operand r6 fused madd instructions of the form

> >>>    'c = c [+-] (a * b)'.  */

> >>

> >> Thanks,

> >> Matthew

> >>
Paul Hua Dec. 13, 2016, 4 a.m. UTC | #3 
Hi:

I get the copyright assignment, it's ok for commit.
but recently, The gcc mainline trunk are fail to building on
mips64el-unknown-linux,
the bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 show the problem.

Thanks,
Paul

On Thu, Nov 17, 2016 at 7:14 PM, Matthew Fortune
<Matthew.Fortune@imgtec.com> wrote:
> Hi Jeff,
>
> Am I OK to accept this change without copyright assignment from Paul?
>
> The change is small and there is no other way it could be implemented
> anyway if I had someone write it from scratch.
>
> Thanks,
> Matthew
>
>> -----Original Message-----
>> From: Paul Hua [mailto:paul.hua.gm@gmail.com]
>> Sent: 17 November 2016 03:01
>> To: Matthew Fortune
>> Cc: gcc-patches@gcc.gnu.org; catherine_moore@mentor.com
>> Subject: Re: [PATCH,gcc/MIPS] Make loongson3a use fused madd.d
>>
>> ping...
>>
>> On Thu, Nov 3, 2016 at 7:58 PM, Paul Hua <paul.hua.gm@gmail.com> wrote:
>> > Hi Matthew,
>> >
>> > Thanks for your comments, update the patch.
>> >
>> > *** gcc/ChangeLog ***
>> >
>> > 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>
>> >
>> >         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
>> >         TARGET_LOONGSON_3A.
>> >         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.
>> >
>> > Thanks,
>> > Paul
>> >
>> > On Thu, Nov 3, 2016 at 6:31 PM, Matthew Fortune
>> > <Matthew.Fortune@imgtec.com> wrote:
>> >> Paul Hua <paul.hua.gm@gmail.com> writes:
>> >>> Loongson3a has 4 operand fused madd instrcution. This patch set
>> >>> loongson3a use fused madd.d.
>> >>
>> >> Hi Paul,
>> >>
>> >> Thanks for the fix. I was vaguely aware that this was wrong for
>> >> loongson-3a but never confirmed it.
>> >>
>> >> I suspect this change is mechanical enough that it can bypass
>> >> copyright assignment but I'd need a global maintainer to comment.
>> >>
>> >> I've sent you copyright assignment paperwork separately.
>> >>
>> >> Two comments on the patch:
>> >>
>> >>> ChangeLog :
>> >>>
>> >>> *** gcc/ChangeLog ***
>> >>>
>> >>> 2016-11-03 Chenghua Xu <paul.hua.gm@gmail.com>
>> >>>
>> >>>     config/mips/
>> >>>     * mips.h: Set loongson3a use fused madd.d.
>> >>
>> >> The changelog needs to reference what was changed rather than the
>> >> effect of the change:
>> >>
>> >>         * config/mips/mips.h (ISA_HAS_FUSED_MADD4): Enable for
>> >>         TARGET_LOONGSON_3A.
>> >>         (ISA_HAS_UNFUSED_MADD4): Exclude TARGET_LOONGSON_3A.
>> >>
>> >>
>> >>>diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
>> >>>81862a9..5076a2b 100644
>> >>>--- a/gcc/config/mips/mips.h
>> >>>+++ b/gcc/config/mips/mips.h
>> >>>@@ -1056,11 +1056,11 @@ struct mips_cpu_info {
>> >>>
>> >>> /* ISA has 4 operand fused madd instructions of the form
>> >>>    'd = [+-] (a * b [+-] c)'.  */
>> >>>-#define ISA_HAS_FUSED_MADD4   TARGET_MIPS8000
>> >>>+#define ISA_HAS_FUSED_MADD4   (TARGET_MIPS8000 ||
>> TARGET_LOONGSON_3A)
>> >>>
>> >>> /* ISA has 4 operand unfused madd instructions of the form
>> >>>    'd = [+-] (a * b [+-] c)'.  */
>> >>>-#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000)
>> >>>+#define ISA_HAS_UNFUSED_MADD4 (ISA_HAS_FP4 && !TARGET_MIPS8000 &&
>> >>>+!TARGET_LOONGSON_3A)
>> >>
>> >> Please split this line and move && !TARGET_LOONGSON_3A to the next
>> >> line under ISA_HAS_FP4.
>> >>
>> >>>
>> >>> /* ISA has 3 operand r6 fused madd instructions of the form
>> >>>    'c = c [+-] (a * b)'.  */
>> >>
>> >> Thanks,
>> >> Matthew
>> >>
Matthew Fortune Dec. 19, 2016, 12:55 p.m. UTC | #4 
Hi Paul,

Apologies for the delay in responding.

> I get the copyright assignment, it's ok for commit.


Thanks for going through copyright assignment, I can see you listed and
also you have commit access now. Is the trunk build failure still
present for you, if it is now resolved then please go ahead and commit.

Thanks,
Matthew
Matthew Fortune Jan. 19, 2017, 4:29 p.m. UTC | #5 
Hi Paul,

Your latest version of the patch is now committed.  I have been doing some
work on the recursive build failure but the issue is complex and involves
LRA so I went ahead with committing your change independently.  It also
turns out that (at least when targeting loongson3a) there are stage2/3
object comparison issues even after resolving the LRA bug with an initial
fix.

r244641

Thanks,
Matthew
diff mbox

Patch

diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 81862a9..2a7a3f2 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -1056,11 +1056,13 @@  struct mips_cpu_info {
 
 /* ISA has 4 operand fused madd instructions of the form
    'd = [+-] (a * b [+-] c)'.  */
-#define ISA_HAS_FUSED_MADD4	TARGET_MIPS8000
+#define ISA_HAS_FUSED_MADD4	(TARGET_MIPS8000 || TARGET_LOONGSON_3A)
 
 /* ISA has 4 operand unfused madd instructions of the form
    'd = [+-] (a * b [+-] c)'.  */
-#define ISA_HAS_UNFUSED_MADD4	(ISA_HAS_FP4 && !TARGET_MIPS8000)
+#define ISA_HAS_UNFUSED_MADD4	(ISA_HAS_FP4				\
+				 && !TARGET_MIPS8000			\
+				 && !TARGET_LOONGSON_3A)
 
 /* ISA has 3 operand r6 fused madd instructions of the form
    'c = c [+-] (a * b)'.  */