* [RS6000] "G" and "H" constraints
@ 2018-11-25 12:14 Alan Modra
2018-11-28 17:55 ` Segher Boessenkool
0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2018-11-25 12:14 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool
The patch fixes two cases where the "G" and "H" constraints were used
incorrectly. Their purpose is calculating insn lengths. Thus it
never makes sense to put "GH" together or with "F" in an insn
alternative.
movdi_internal32 used "GHF" in an alternative so I replaced that with
"F", and added length attributes for the insn, which were missing.
There are some formatting changes too, to make alternatives line up
with attributes.
The FMOVE128 version of mov<mode>_softfloat also had "GHF" in an
alternative, so "GH" is dropped and "F" moved to a separate
alternative in order to get insn lengths correct. Well by correct I
mean we need to choose the maximum insn length otherwise branches
might not reach. Note the very large length for "F". I think it
would be better to force soft-float long double constants to memory in
64-bit mode, but that's a patch for another day.
Bootstrapped etc. powerpc64le-linux.
* config/rs6000/constraints.md (G, H): Comment on purpose of
constraint. Correct mode comments.
* config/rs6000/rs6000.md (movdi_internal32): Remove "GH" from
alternative handling "F". Add length attr. Formatting.
(mov<mode>_softfloat128): Renamed FMOVE128 mov<mode>_softfloat.
Delete "GH" from alternative, and move "F" to separate alternative.
Correct insn lengths.
diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
index 90aba1e77d3..284d3fedc3a 100644
--- a/gcc/config/rs6000/constraints.md
+++ b/gcc/config/rs6000/constraints.md
@@ -252,17 +252,18 @@ (define_constraint "P"
(and (match_code "const_int")
(match_test "((- (unsigned HOST_WIDE_INT) ival) + 0x8000) < 0x10000")))
-;; Floating-point constraints
+;; Floating-point constraints. These two are defined so that insn
+;; length attributes can be calculated exactly.
(define_constraint "G"
- "Constant that can be copied into GPR with two insns for DF/DI
- and one for SF."
+ "Constant that can be copied into GPR with two insns for DF/DD
+ and one for SF/SD."
(and (match_code "const_double")
(match_test "num_insns_constant (op, mode)
== (mode == SFmode ? 1 : 2)")))
(define_constraint "H"
- "DF/DI constant that takes three insns."
+ "DF/DD constant that takes three insns."
(and (match_code "const_double")
(match_test "num_insns_constant (op, mode) == 3")))
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 8b640b1334c..c4d697f856b 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -7711,9 +7711,9 @@ (define_insn_and_split "*mov<mode>_32bit"
{ rs6000_split_multireg_move (operands[0], operands[1]); DONE; }
[(set_attr "length" "8,8,8,8,20,20,16")])
-(define_insn_and_split "*mov<mode>_softfloat"
- [(set (match_operand:FMOVE128 0 "nonimmediate_operand" "=Y,r,r")
- (match_operand:FMOVE128 1 "input_operand" "r,YGHF,r"))]
+(define_insn_and_split "*mov<mode>_softfloat128"
+ [(set (match_operand:FMOVE128 0 "nonimmediate_operand" "=Y,r,r,r")
+ (match_operand:FMOVE128 1 "input_operand" "r,Y,F,r"))]
"TARGET_SOFT_FLOAT
&& (gpc_reg_operand (operands[0], <MODE>mode)
|| gpc_reg_operand (operands[1], <MODE>mode))"
@@ -7721,7 +7721,19 @@ (define_insn_and_split "*mov<mode>_softfloat"
"&& reload_completed"
[(pc)]
{ rs6000_split_multireg_move (operands[0], operands[1]); DONE; }
- [(set_attr "length" "20,20,16")])
+ [(set_attr_alternative "length"
+ [(if_then_else (match_test "TARGET_POWERPC64")
+ (const_string "8")
+ (const_string "16"))
+ (if_then_else (match_test "TARGET_POWERPC64")
+ (const_string "8")
+ (const_string "16"))
+ (if_then_else (match_test "TARGET_POWERPC64")
+ (const_string "40")
+ (const_string "32"))
+ (if_then_else (match_test "TARGET_POWERPC64")
+ (const_string "8")
+ (const_string "16"))])])
(define_expand "extenddf<mode>2"
[(set (match_operand:FLOAT128 0 "gpc_reg_operand")
@@ -8656,22 +8668,22 @@ (define_insn_and_split "reload_gpr_from_vsxsf"
;; Use of fprs is disparaged slightly otherwise reload prefers to reload
;; a gpr into a fpr instead of reloading an invalid 'Y' address
-;; GPR store GPR load GPR move FPR store FPR load FPR move
-;; GPR const AVX store AVX store AVX load AVX load VSX move
-;; P9 0 P9 -1 AVX 0/-1 VSX 0 VSX -1 P9 const
+;; GPR store GPR load GPR move FPR store FPR load FPR move
+;; GPR const AVX store AVX store AVX load AVX load VSX move
+;; P9 0 P9 -1 AVX 0/-1 VSX 0 VSX -1 P9 const
;; AVX const
(define_insn "*movdi_internal32"
[(set (match_operand:DI 0 "nonimmediate_operand"
- "=Y, r, r, m, ^d, ^d,
- r, wY, Z, ^wb, $wv, ^wi,
- *wo, *wo, *wv, *wi, *wi, *wv,
+ "=Y, r, r, m, ^d, ^d,
+ r, wY, Z, ^wb, $wv, ^wi,
+ *wo, *wo, *wv, *wi, *wi, *wv,
*wv")
(match_operand:DI 1 "input_operand"
- "r, Y, r, ^d, m, ^d,
- IJKnGHF, ^wb, $wv, wY, Z, ^wi,
- Oj, wM, OjwM, Oj, wM, wS,
+ "r, Y, r, ^d, m, ^d,
+ IJKnF, ^wb, $wv, wY, Z, ^wi,
+ Oj, wM, OjwM, Oj, wM, wS,
wB"))]
"! TARGET_POWERPC64
@@ -8698,11 +8710,16 @@ (define_insn "*movdi_internal32"
#
#"
[(set_attr "type"
- "store, load, *, fpstore, fpload, fpsimple,
- *, fpstore, fpstore, fpload, fpload, veclogical,
- vecsimple, vecsimple, vecsimple, veclogical, veclogical, vecsimple,
- vecsimple")
- (set_attr "size" "64")])
+ "store, load, *, fpstore, fpload, fpsimple,
+ *, fpstore, fpstore, fpload, fpload, veclogical,
+ vecsimple, vecsimple, vecsimple, veclogical, veclogical, vecsimple,
+ vecsimple")
+ (set_attr "size" "64")
+ (set_attr "length"
+ "8, 8, 8, 4, 4, 4,
+ 16, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 8,
+ 4")])
(define_split
[(set (match_operand:DI 0 "gpc_reg_operand")
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RS6000] "G" and "H" constraints
2018-11-25 12:14 [RS6000] "G" and "H" constraints Alan Modra
@ 2018-11-28 17:55 ` Segher Boessenkool
2018-11-28 23:27 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: Segher Boessenkool @ 2018-11-28 17:55 UTC (permalink / raw)
To: Alan Modra; +Cc: gcc-patches
Hi!
On Sun, Nov 25, 2018 at 10:44:00PM +1030, Alan Modra wrote:
> The patch fixes two cases where the "G" and "H" constraints were used
> incorrectly. Their purpose is calculating insn lengths. Thus it
> never makes sense to put "GH" together or with "F" in an insn
> alternative.
>
> movdi_internal32 used "GHF" in an alternative so I replaced that with
> "F", and added length attributes for the insn, which were missing.
> There are some formatting changes too, to make alternatives line up
> with attributes.
>
> The FMOVE128 version of mov<mode>_softfloat also had "GHF" in an
> alternative, so "GH" is dropped and "F" moved to a separate
> alternative in order to get insn lengths correct. Well by correct I
> mean we need to choose the maximum insn length otherwise branches
> might not reach. Note the very large length for "F". I think it
> would be better to force soft-float long double constants to memory in
> 64-bit mode, but that's a patch for another day.
>
> Bootstrapped etc. powerpc64le-linux.
>
> * config/rs6000/constraints.md (G, H): Comment on purpose of
> constraint. Correct mode comments.
> * config/rs6000/rs6000.md (movdi_internal32): Remove "GH" from
> alternative handling "F". Add length attr. Formatting.
> (mov<mode>_softfloat128): Renamed FMOVE128 mov<mode>_softfloat.
> Delete "GH" from alternative, and move "F" to separate alternative.
> Correct insn lengths.
> -;; Floating-point constraints
> +;; Floating-point constraints. These two are defined so that insn
> +;; length attributes can be calculated exactly.
>
> (define_constraint "G"
> - "Constant that can be copied into GPR with two insns for DF/DI
> - and one for SF."
> + "Constant that can be copied into GPR with two insns for DF/DD
> + and one for SF/SD."
> (and (match_code "const_double")
> (match_test "num_insns_constant (op, mode)
> == (mode == SFmode ? 1 : 2)")))
The code does 2 for SDmode, the comment says 1. Which is correct?
> -(define_insn_and_split "*mov<mode>_softfloat"
> - [(set (match_operand:FMOVE128 0 "nonimmediate_operand" "=Y,r,r")
> - (match_operand:FMOVE128 1 "input_operand" "r,YGHF,r"))]
> +(define_insn_and_split "*mov<mode>_softfloat128"
> + [(set (match_operand:FMOVE128 0 "nonimmediate_operand" "=Y,r,r,r")
> + (match_operand:FMOVE128 1 "input_operand" "r,Y,F,r"))]
There is <mode> as part of the name already; what does adding "128" help?
> "TARGET_SOFT_FLOAT
> && (gpc_reg_operand (operands[0], <MODE>mode)
> || gpc_reg_operand (operands[1], <MODE>mode))"
> @@ -7721,7 +7721,19 @@ (define_insn_and_split "*mov<mode>_softfloat"
> "&& reload_completed"
> [(pc)]
> { rs6000_split_multireg_move (operands[0], operands[1]); DONE; }
> - [(set_attr "length" "20,20,16")])
> + [(set_attr_alternative "length"
> + [(if_then_else (match_test "TARGET_POWERPC64")
> + (const_string "8")
> + (const_string "16"))
> + (if_then_else (match_test "TARGET_POWERPC64")
> + (const_string "8")
> + (const_string "16"))
> + (if_then_else (match_test "TARGET_POWERPC64")
> + (const_string "40")
> + (const_string "32"))
> + (if_then_else (match_test "TARGET_POWERPC64")
> + (const_string "8")
> + (const_string "16"))])])
Does -mpowerpc64 really use more insns for F than -mno-powerpc64 does? Huh.
> @@ -8656,22 +8668,22 @@ (define_insn_and_split "reload_gpr_from_vsxsf"
> ;; Use of fprs is disparaged slightly otherwise reload prefers to reload
> ;; a gpr into a fpr instead of reloading an invalid 'Y' address
>
> -;; GPR store GPR load GPR move FPR store FPR load FPR move
> -;; GPR const AVX store AVX store AVX load AVX load VSX move
> -;; P9 0 P9 -1 AVX 0/-1 VSX 0 VSX -1 P9 const
> +;; GPR store GPR load GPR move FPR store FPR load FPR move
> +;; GPR const AVX store AVX store AVX load AVX load VSX move
> +;; P9 0 P9 -1 AVX 0/-1 VSX 0 VSX -1 P9 const
> ;; AVX const
>
> (define_insn "*movdi_internal32"
> [(set (match_operand:DI 0 "nonimmediate_operand"
> - "=Y, r, r, m, ^d, ^d,
> - r, wY, Z, ^wb, $wv, ^wi,
> - *wo, *wo, *wv, *wi, *wi, *wv,
> + "=Y, r, r, m, ^d, ^d,
> + r, wY, Z, ^wb, $wv, ^wi,
> + *wo, *wo, *wv, *wi, *wi, *wv,
> *wv")
>
> (match_operand:DI 1 "input_operand"
> - "r, Y, r, ^d, m, ^d,
> - IJKnGHF, ^wb, $wv, wY, Z, ^wi,
> - Oj, wM, OjwM, Oj, wM, wS,
> + "r, Y, r, ^d, m, ^d,
> + IJKnF, ^wb, $wv, wY, Z, ^wi,
> + Oj, wM, OjwM, Oj, wM, wS,
> wB"))]
>
> "! TARGET_POWERPC64
> @@ -8698,11 +8710,16 @@ (define_insn "*movdi_internal32"
> #
> #"
> [(set_attr "type"
> - "store, load, *, fpstore, fpload, fpsimple,
> - *, fpstore, fpstore, fpload, fpload, veclogical,
> - vecsimple, vecsimple, vecsimple, veclogical, veclogical, vecsimple,
> - vecsimple")
> - (set_attr "size" "64")])
> + "store, load, *, fpstore, fpload, fpsimple,
> + *, fpstore, fpstore, fpload, fpload, veclogical,
> + vecsimple, vecsimple, vecsimple, veclogical, veclogical, vecsimple,
> + vecsimple")
> + (set_attr "size" "64")
> + (set_attr "length"
> + "8, 8, 8, 4, 4, 4,
> + 16, 4, 4, 4, 4, 4,
> + 4, 4, 4, 4, 4, 8,
> + 4")])
Please don't make some columns wider than others. You could just remove
the space after "veclogical,", maybe?
Looks good otherwise. Please fix that SDmode thing and the 128 thing?
Okay for trunk with that, thanks!
Segher
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RS6000] "G" and "H" constraints
2018-11-28 17:55 ` Segher Boessenkool
@ 2018-11-28 23:27 ` Alan Modra
0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2018-11-28 23:27 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On Wed, Nov 28, 2018 at 11:55:21AM -0600, Segher Boessenkool wrote:
> > (define_constraint "G"
> > - "Constant that can be copied into GPR with two insns for DF/DI
> > - and one for SF."
> > + "Constant that can be copied into GPR with two insns for DF/DD
> > + and one for SF/SD."
> > (and (match_code "const_double")
> > (match_test "num_insns_constant (op, mode)
> > == (mode == SFmode ? 1 : 2)")))
>
> The code does 2 for SDmode, the comment says 1. Which is correct?
The comment is correct. I'll fix the code to do
"mode == SFmode || mode == SDmode".
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-28 23:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-25 12:14 [RS6000] "G" and "H" constraints Alan Modra
2018-11-28 17:55 ` Segher Boessenkool
2018-11-28 23:27 ` Alan Modra
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).