public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RS6000] Fix 65576 regression
@ 2015-04-02  5:22 Alan Modra
  2015-04-02 17:03 ` David Edelsohn
  2015-04-02 23:41 ` Michael Meissner
  0 siblings, 2 replies; 7+ messages in thread
From: Alan Modra @ 2015-04-02  5:22 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Edelsohn, Michael Meissner

This cures an unrecognizable insn ICE by modifying a predicate of
extenddftf2_internal (the only place this predicate is used) to ensure
that rtl optimization passes do not substitute 0.0 for a register with
known 0.0 value, except when VSX is enabled.  ie. Don't undo the
necessary register move emitted by the extenddftf2_fprs expander.
Bootstrapped and regression tested powerpc64le-linux, powerpc64-linux
and powerpc-linux.  OK to apply? 

	PR target/65576
	PR target/65240
	* config/rs6000/predicates.md (zero_reg_mem_operand): Exclude
	0.0 constant unless TARGET_VSX.

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 221805)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -964,7 +964,8 @@
 
 ;; Return 1 if the operand is CONST_DOUBLE 0, register or memory operand.
 (define_predicate "zero_reg_mem_operand"
-  (ior (match_operand 0 "zero_fp_constant")
+  (ior (and (match_test "TARGET_VSX")
+	    (match_operand 0 "zero_fp_constant"))
        (match_operand 0 "reg_or_mem_operand")))
 
 ;; Return 1 if the operand is a CONST_INT and it is the element for 64-bit

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-02  5:22 [RS6000] Fix 65576 regression Alan Modra
@ 2015-04-02 17:03 ` David Edelsohn
  2015-04-02 23:19   ` Alan Modra
  2015-04-02 23:41 ` Michael Meissner
  1 sibling, 1 reply; 7+ messages in thread
From: David Edelsohn @ 2015-04-02 17:03 UTC (permalink / raw)
  To: GCC Patches, Michael Meissner, Alan Modra

On Thu, Apr 2, 2015 at 1:22 AM, Alan Modra <amodra@gmail.com> wrote:
> This cures an unrecognizable insn ICE by modifying a predicate of
> extenddftf2_internal (the only place this predicate is used) to ensure
> that rtl optimization passes do not substitute 0.0 for a register with
> known 0.0 value, except when VSX is enabled.  ie. Don't undo the
> necessary register move emitted by the extenddftf2_fprs expander.
> Bootstrapped and regression tested powerpc64le-linux, powerpc64-linux
> and powerpc-linux.  OK to apply?
>
>         PR target/65576
>         PR target/65240
>         * config/rs6000/predicates.md (zero_reg_mem_operand): Exclude
>         0.0 constant unless TARGET_VSX.

This change seems to disable the final alternative in the
extenddftf2_internal pattern.

- David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-02 17:03 ` David Edelsohn
@ 2015-04-02 23:19   ` Alan Modra
  0 siblings, 0 replies; 7+ messages in thread
From: Alan Modra @ 2015-04-02 23:19 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches, Michael Meissner

On Thu, Apr 02, 2015 at 01:03:19PM -0400, David Edelsohn wrote:
> On Thu, Apr 2, 2015 at 1:22 AM, Alan Modra <amodra@gmail.com> wrote:
> > This cures an unrecognizable insn ICE by modifying a predicate of
> > extenddftf2_internal (the only place this predicate is used) to ensure
> > that rtl optimization passes do not substitute 0.0 for a register with
> > known 0.0 value, except when VSX is enabled.  ie. Don't undo the
> > necessary register move emitted by the extenddftf2_fprs expander.
> > Bootstrapped and regression tested powerpc64le-linux, powerpc64-linux
> > and powerpc-linux.  OK to apply?
> >
> >         PR target/65576
> >         PR target/65240
> >         * config/rs6000/predicates.md (zero_reg_mem_operand): Exclude
> >         0.0 constant unless TARGET_VSX.
> 
> This change seems to disable the final alternative in the
> extenddftf2_internal pattern.

It looks to me like it was already disabled, by r96390.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-02  5:22 [RS6000] Fix 65576 regression Alan Modra
  2015-04-02 17:03 ` David Edelsohn
@ 2015-04-02 23:41 ` Michael Meissner
  2015-04-03  0:03   ` David Edelsohn
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Meissner @ 2015-04-02 23:41 UTC (permalink / raw)
  To: gcc-patches, David Edelsohn, Michael Meissner

On Thu, Apr 02, 2015 at 03:52:35PM +1030, Alan Modra wrote:
> This cures an unrecognizable insn ICE by modifying a predicate of
> extenddftf2_internal (the only place this predicate is used) to ensure
> that rtl optimization passes do not substitute 0.0 for a register with
> known 0.0 value, except when VSX is enabled.  ie. Don't undo the
> necessary register move emitted by the extenddftf2_fprs expander.
> Bootstrapped and regression tested powerpc64le-linux, powerpc64-linux
> and powerpc-linux.  OK to apply? 
> 
> 	PR target/65576
> 	PR target/65240
> 	* config/rs6000/predicates.md (zero_reg_mem_operand): Exclude
> 	0.0 constant unless TARGET_VSX.
> 
> Index: gcc/config/rs6000/predicates.md
> ===================================================================
> --- gcc/config/rs6000/predicates.md	(revision 221805)
> +++ gcc/config/rs6000/predicates.md	(working copy)
> @@ -964,7 +964,8 @@
> 
>  ;; Return 1 if the operand is CONST_DOUBLE 0, register or memory operand.
>  (define_predicate "zero_reg_mem_operand"
> -  (ior (match_operand 0 "zero_fp_constant")
> +  (ior (and (match_test "TARGET_VSX")
> +	    (match_operand 0 "zero_fp_constant"))
>         (match_operand 0 "reg_or_mem_operand")))
> 
>  ;; Return 1 if the operand is a CONST_INT and it is the element for 64-bit
> 

I definately prefer Alan's patch over mine.

In looking at extenddftf2_internal, I believe you are correct David, in that
final alternative will never match because 0.0 will not be valid (pre-VSX 0.0
won't be allowed as operand2, VSX 0.0 will not match "n" constraint).  For the
VSX case, the "n" should be either "j" or "E".

If you are doing a double->long double conversion, and it happens to pick GPRs
for some reason, the GPR side will do a load/move of 0.0 instead of using li to
load 0 on pre-VSX systems.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-02 23:41 ` Michael Meissner
@ 2015-04-03  0:03   ` David Edelsohn
  2015-04-03 12:51     ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: David Edelsohn @ 2015-04-03  0:03 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches, Michael Meissner, Alan Modra

On Thu, Apr 2, 2015 at 7:41 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> On Thu, Apr 02, 2015 at 03:52:35PM +1030, Alan Modra wrote:
>> This cures an unrecognizable insn ICE by modifying a predicate of
>> extenddftf2_internal (the only place this predicate is used) to ensure
>> that rtl optimization passes do not substitute 0.0 for a register with
>> known 0.0 value, except when VSX is enabled.  ie. Don't undo the
>> necessary register move emitted by the extenddftf2_fprs expander.
>> Bootstrapped and regression tested powerpc64le-linux, powerpc64-linux
>> and powerpc-linux.  OK to apply?
>>
>>       PR target/65576
>>       PR target/65240
>>       * config/rs6000/predicates.md (zero_reg_mem_operand): Exclude
>>       0.0 constant unless TARGET_VSX.
>>
>> Index: gcc/config/rs6000/predicates.md
>> ===================================================================
>> --- gcc/config/rs6000/predicates.md   (revision 221805)
>> +++ gcc/config/rs6000/predicates.md   (working copy)
>> @@ -964,7 +964,8 @@
>>
>>  ;; Return 1 if the operand is CONST_DOUBLE 0, register or memory operand.
>>  (define_predicate "zero_reg_mem_operand"
>> -  (ior (match_operand 0 "zero_fp_constant")
>> +  (ior (and (match_test "TARGET_VSX")
>> +         (match_operand 0 "zero_fp_constant"))
>>         (match_operand 0 "reg_or_mem_operand")))
>>
>>  ;; Return 1 if the operand is a CONST_INT and it is the element for 64-bit
>>
>
> I definately prefer Alan's patch over mine.
>
> In looking at extenddftf2_internal, I believe you are correct David, in that
> final alternative will never match because 0.0 will not be valid (pre-VSX 0.0
> won't be allowed as operand2, VSX 0.0 will not match "n" constraint).  For the
> VSX case, the "n" should be either "j" or "E".
>
> If you are doing a double->long double conversion, and it happens to pick GPRs
> for some reason, the GPR side will do a load/move of 0.0 instead of using li to
> load 0 on pre-VSX systems.

If the final alternative cannot occur, it should be removed as well.
Alan, would you please test that change also?

Thanks, David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-03  0:03   ` David Edelsohn
@ 2015-04-03 12:51     ` Alan Modra
  2015-04-03 13:35       ` David Edelsohn
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2015-04-03 12:51 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Michael Meissner, GCC Patches, Michael Meissner

On Thu, Apr 02, 2015 at 08:02:57PM -0400, David Edelsohn wrote:
> If the final alternative cannot occur, it should be removed as well.
> Alan, would you please test that change also?

Tested powerpc64-linux and powerpc-linux no regressions.

	* config/rs6000/rs6000.md (extenddftf2_internal): Remove last
	alternative.

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 221805)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -8369,9 +8369,9 @@
 })
 
 (define_insn_and_split "*extenddftf2_internal"
-  [(set (match_operand:TF 0 "nonimmediate_operand" "=m,Y,ws,d,&d,r")
-       (float_extend:TF (match_operand:DF 1 "input_operand" "d,r,md,md,md,rm")))
-   (use (match_operand:DF 2 "zero_reg_mem_operand" "d,r,j,m,d,n"))]
+  [(set (match_operand:TF 0 "nonimmediate_operand" "=m,Y,ws,d,&d")
+       (float_extend:TF (match_operand:DF 1 "input_operand" "d,r,md,md,md")))
+   (use (match_operand:DF 2 "zero_reg_mem_operand" "d,r,j,m,d"))]
   "!TARGET_IEEEQUAD
    && TARGET_HARD_FLOAT && TARGET_FPRS && TARGET_DOUBLE_FLOAT 
    && TARGET_LONG_DOUBLE_128"

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RS6000] Fix 65576 regression
  2015-04-03 12:51     ` Alan Modra
@ 2015-04-03 13:35       ` David Edelsohn
  0 siblings, 0 replies; 7+ messages in thread
From: David Edelsohn @ 2015-04-03 13:35 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches, Michael Meissner, Alan Modra

On Fri, Apr 3, 2015 at 8:51 AM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 02, 2015 at 08:02:57PM -0400, David Edelsohn wrote:
>> If the final alternative cannot occur, it should be removed as well.
>> Alan, would you please test that change also?
>
> Tested powerpc64-linux and powerpc-linux no regressions.
>
>         * config/rs6000/rs6000.md (extenddftf2_internal): Remove last
>         alternative.

Okay with the two patches combined.

Thanks, David

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-04-03 13:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-02  5:22 [RS6000] Fix 65576 regression Alan Modra
2015-04-02 17:03 ` David Edelsohn
2015-04-02 23:19   ` Alan Modra
2015-04-02 23:41 ` Michael Meissner
2015-04-03  0:03   ` David Edelsohn
2015-04-03 12:51     ` Alan Modra
2015-04-03 13:35       ` David Edelsohn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).