[rfc] rs6000: Updated constraint documentation

This is my current work-in-progress version.  There still are rough
edges, and not much is done for the output modifiers yet, but it should
be in much better shape wrt the user manual now.  The internals manual
also is a bit better I think.

md.texi is not automatically kept in synch with constraints.md (let
alone generated from it), so the two diverged.  I tried to correct
that, too.

Please let me know if you have any ideas how to improve it further, or
if I did something terribly wrong, or anything else.  Thanks,


Segher

---
 gcc/config/rs6000/constraints.md | 159 +++++++++++++++++++--------------
 gcc/doc/md.texi                  | 188 +++++++++++++++++++--------------------
 2 files changed, 182 insertions(+), 165 deletions(-)

On 1/30/20 6:17 PM, Segher Boessenkool wrote:
> This is my current work-in-progress version.  There still are rough
> edges, and not much is done for the output modifiers yet, but it should
> be in much better shape wrt the user manual now.  The internals manual
> also is a bit better I think.
>
> md.texi is not automatically kept in synch with constraints.md (let
> alone generated from it), so the two diverged.  I tried to correct
> that, too.
>
> Please let me know if you have any ideas how to improve it further, or
> if I did something terribly wrong, or anything else.  Thanks,
>
>
> Segher
>
> ---
>   gcc/config/rs6000/constraints.md | 159 +++++++++++++++++++--------------
>   gcc/doc/md.texi                  | 188 +++++++++++++++++++--------------------
>   2 files changed, 182 insertions(+), 165 deletions(-)
>
> diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
> index 398c894..bafc22a 100644
> --- a/gcc/config/rs6000/constraints.md
> +++ b/gcc/config/rs6000/constraints.md
> @@ -21,192 +21,214 @@
>
>   ;; Register constraints
>
> -(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
> -  "@internal")
> -
> -(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
> -  "@internal")
> +; Actually defined in common.md:
> +; (define_register_constraint "r" "GENERAL_REGS"
> +;   "A general purpose register (GPR), @code{r0}@dots{}@code{r31}.")
>
>   (define_register_constraint "b" "BASE_REGS"
> -  "@internal")
> +  "A base register.  Like @code{r}, but @code{r0} is not allowed, so
> +   @code{r1}@dots{}@code{r31}.")
>
> -(define_register_constraint "h" "SPECIAL_REGS"
> -  "@internal")
> +(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
> +  "A floating point register (FPR), @code{f0}@dots{}@code{f31}.")
>
> -(define_register_constraint "c" "CTR_REGS"
> -  "@internal")
> -
> -(define_register_constraint "l" "LINK_REGS"
> -  "@internal")
> +(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
> +  "A floating point register.  This is the same as @code{f} nowadays;
> +   historically @code{f} was for single-precision and @code{d} was for
> +   double-precision floating point.")
>
>   (define_register_constraint "v" "ALTIVEC_REGS"
> -  "@internal")
> +  "An Altivec vector register (VR), @code{v0}@dots{}@code{v31}.")
> +
> +(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
> +  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d}
> +   or a @code{v} register.")


Not quite true, as the "d" register is only half of a VSX register.  It 
may or may not be worth including a picture of register overlaps...

> +
> +(define_register_constraint "h" "SPECIAL_REGS"
> +  "@internal @code{vrsave}, @code{ctr}, or @code{lr}.")
> +
> +(define_register_constraint "c" "CTR_REGS"
> +  "The count register, @code{ctr}.")
> +
> +(define_register_constraint "l" "LINK_REGS"
> +  "The link register, @code{lr}.")
>
>   (define_register_constraint "x" "CR0_REGS"
> -  "@internal")
> +  "Condition register field 0, @code{cr0}.")
>
>   (define_register_constraint "y" "CR_REGS"
> -  "@internal")
> +  "Any condition register field, @code{cr0}@dots{}@code{cr7}.")
>
>   (define_register_constraint "z" "CA_REGS"
> -  "@internal")
> -
> -;; Use w as a prefix to add VSX modes
> -;; any VSX register
> -(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
> -  "Any VSX register if the -mvsx option was used or NO_REGS.")
> +  "@internal The carry bit, @code{XER[CA]}.")
>
>   ;; NOTE: For compatibility, "wc" is reserved to represent individual CR bits.
>   ;; It is currently used for that purpose in LLVM.
>
>   (define_register_constraint "we" "rs6000_constraints[RS6000_CONSTRAINT_we]"
> -  "VSX register if the -mpower9-vector -m64 options were used or NO_REGS.")
> +  "@internal VSX register if the -mpower9-vector -m64 options were used
> +   or NO_REGS.")


Suggest changing "used or" to "used, else".

>
>   ;; NO_REGs register constraint, used to merge mov{sd,sf}, since movsd can use
>   ;; direct move directly, and movsf can't to move between the register sets.
>   ;; There is a mode_attr that resolves to wa for SDmode and wn for SFmode
> -(define_register_constraint "wn" "NO_REGS" "No register (NO_REGS).")
> +(define_register_constraint "wn" "NO_REGS"
> +  "@internal No register (NO_REGS).")
>
>   (define_register_constraint "wr" "rs6000_constraints[RS6000_CONSTRAINT_wr]"
> -  "General purpose register if 64-bit instructions are enabled or NO_REGS.")
> +  "@internal General purpose register if 64-bit instructions are enabled
> +   or NO_REGS.")


Similar here.

>
>   (define_register_constraint "wx" "rs6000_constraints[RS6000_CONSTRAINT_wx]"
> -  "Floating point register if the STFIWX instruction is enabled or NO_REGS.")
> +  "@internal Floating point register if the STFIWX instruction is enabled
> +   or NO_REGS.")


And here.

>
>   (define_register_constraint "wA" "rs6000_constraints[RS6000_CONSTRAINT_wA]"
> -  "BASE_REGS if 64-bit instructions are enabled or NO_REGS.")
> +  "@internal BASE_REGS if 64-bit instructions are enabled or NO_REGS.")


Etc.

>
>   ;; wB needs ISA 2.07 VUPKHSW
>   (define_constraint "wB"
> -  "Signed 5-bit constant integer that can be loaded into an altivec register."
> +  "@internal Signed 5-bit constant integer that can be loaded into an
> +   Altivec register."
>     (and (match_code "const_int")
> -       (and (match_test "TARGET_P8_VECTOR")
> -	    (match_operand 0 "s5bit_cint_operand"))))
> +       (match_test "TARGET_P8_VECTOR")
> +       (match_operand 0 "s5bit_cint_operand")))
>
>   (define_constraint "wD"
> -  "Int constant that is the element number of the 64-bit scalar in a vector."
> +  "@internal Int constant that is the element number of the 64-bit scalar
> +   in a vector."
>     (and (match_code "const_int")
>          (match_test "TARGET_VSX && (ival == VECTOR_ELEMENT_SCALAR_64BIT)")))
>
>   (define_constraint "wE"
> -  "Vector constant that can be loaded with the XXSPLTIB instruction."
> +  "@internal Vector constant that can be loaded with the XXSPLTIB instruction."
>     (match_test "xxspltib_constant_nosplit (op, mode)"))
>
>   ;; Extended fusion store
>   (define_memory_constraint "wF"
> -  "Memory operand suitable for power8 GPR load fusion"
> +  "@internal Memory operand suitable for power8 GPR load fusion."
>     (match_operand 0 "fusion_addis_mem_combo_load"))
>
>   (define_constraint "wL"
> -  "Int constant that is the element number mfvsrld accesses in a vector."
> +  "@internal Int constant that is the element number mfvsrld accesses in
> +   a vector."
>     (and (match_code "const_int")
> -       (and (match_test "TARGET_DIRECT_MOVE_128")
> -	    (match_test "(ival == VECTOR_ELEMENT_MFVSRLD_64BIT)"))))
> +       (match_test "TARGET_DIRECT_MOVE_128")
> +       (match_test "(ival == VECTOR_ELEMENT_MFVSRLD_64BIT)")))
>
>   ;; Generate the XXORC instruction to set a register to all 1's
>   (define_constraint "wM"
> -  "Match vector constant with all 1's if the XXLORC instruction is available"
> +  "@internal Match vector constant with all 1's if the XXLORC instruction
> +   is available."
>     (and (match_test "TARGET_P8_VECTOR")
>          (match_operand 0 "all_ones_constant")))
>
>   ;; ISA 3.0 vector d-form addresses
>   (define_memory_constraint "wO"
> -  "Memory operand suitable for the ISA 3.0 vector d-form instructions."
> +  "@internal Memory operand suitable for the ISA 3.0 vector d-form instructions."
>     (match_operand 0 "vsx_quad_dform_memory_operand"))
>
>   ;; Lq/stq validates the address for load/store quad
>   (define_memory_constraint "wQ"
> -  "Memory operand suitable for the load/store quad instructions"
> +  "@internal Memory operand suitable for the load/store quad instructions."
>     (match_operand 0 "quad_memory_operand"))
>
>   (define_constraint "wS"
> -  "Vector constant that can be loaded with XXSPLTIB & sign extension."
> +  "@internal Vector constant that can be loaded with XXSPLTIB & sign extension."
>     (match_test "xxspltib_constant_split (op, mode)"))
>
>   ;; ISA 3.0 DS-form instruction that has the bottom 2 bits 0 and no update form.
>   ;; Used by LXSD/STXSD/LXSSP/STXSSP.  In contrast to "Y", the multiple-of-four
>   ;; offset is enforced for 32-bit too.
>   (define_memory_constraint "wY"
> -  "Offsettable memory operand, with bottom 2 bits 0"
> +  "@internal A memory operand for a DS-form instruction."
>     (and (match_code "mem")
>          (not (match_test "update_address_mem (op, mode)"))
>          (match_test "mem_operand_ds_form (op, mode)")))
>
>   ;; Altivec style load/store that ignores the bottom bits of the address
>   (define_memory_constraint "wZ"
> -  "Indexed or indirect memory operand, ignoring the bottom 4 bits"
> +  "@internal Indexed or indirect memory operand, ignoring the bottom 4 bits."
>     (match_operand 0 "altivec_indexed_or_indirect_operand"))
>
>   ;; Integer constraints
>
>   (define_constraint "I"
> -  "A signed 16-bit constant"
> +  "A signed 16-bit constant."
>     (and (match_code "const_int")
>          (match_test "((unsigned HOST_WIDE_INT) ival + 0x8000) < 0x10000")))
>
>   (define_constraint "J"
> -  "high-order 16 bits nonzero"
> +  "An unsigned 16-bit constant shifted left 16 bits (use @code{L} instead
> +   for @code{SImode} constants)."
>     (and (match_code "const_int")
>          (match_test "(ival & (~ (unsigned HOST_WIDE_INT) 0xffff0000)) == 0")))
>
>   (define_constraint "K"
> -  "low-order 16 bits nonzero"
> +  "An unsigned 16-bit constant."
>     (and (match_code "const_int")
>          (match_test "(ival & (~ (HOST_WIDE_INT) 0xffff)) == 0")))
>
>   (define_constraint "L"
> -  "signed 16-bit constant shifted left 16 bits"
> +  "A signed 16-bit constant shifted left 16 bits."
>     (and (match_code "const_int")
>          (match_test "((ival & 0xffff) == 0
>   		      && (ival >> 31 == -1 || ival >> 31 == 0))")))
>
>   (define_constraint "M"
> -  "constant greater than 31"
> +  "@internal A constant greater than 31."
>     (and (match_code "const_int")
>          (match_test "ival > 31")))
>
>   (define_constraint "N"
> -  "positive constant that is an exact power of two"
> +  "@internal An exact power of two."
>     (and (match_code "const_int")
>          (match_test "ival > 0 && exact_log2 (ival) >= 0")))
>
>   (define_constraint "O"
> -  "constant zero"
> +  "@internal The integer constant zero."
>     (and (match_code "const_int")
>          (match_test "ival == 0")))
>
>   (define_constraint "P"
> -  "constant whose negation is signed 16-bit constant"
> +  "@internal A constant whose negation is a signed 16-bit constant."
>     (and (match_code "const_int")
>          (match_test "((- (unsigned HOST_WIDE_INT) ival) + 0x8000) < 0x10000")))
>
>   ;; 34-bit signed integer constant
>   (define_constraint "eI"
> -  "34-bit constant integer that can be loaded with PADDI"
> +  "A signed 34-bit integer constant if prefixed instructions are supported."
>     (match_operand 0 "cint34_operand"))
>
>   ;; 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/DD
> -   and one for SF/SD."
> +  "@internal A floating point constant that can be loaded into a register
> +   with one instruction per word."
>     (and (match_code "const_double")
>          (match_test "num_insns_constant (op, mode)
>   		    == (mode == SFmode || mode == SDmode ? 1 : 2)")))
>
>   (define_constraint "H"
> -  "DF/DD constant that takes three insns."
> +  "@internal A floating point constant that can be loaded into a register
> +   using three instructions."
>     (and (match_code "const_double")
>          (match_test "num_insns_constant (op, mode) == 3")))
>
>   ;; Memory constraints
>
> +; Actually defined in common.md:
> +; (define_memory_constraint "m"
> +;   "A memory operand."
> +
>   (define_memory_constraint "es"
> -  "A ``stable'' memory operand; that is, one which does not include any
> -automodification of the base register.  Unlike @samp{m}, this constraint
> -can be used in @code{asm} statements that might access the operand
> -several times, or that might not access it at all."
> +  "@internal
> +   A ``stable'' memory operand; that is, one which does not include any
> +   automodification of the base register.  This used to be useful when
> +   @code{m} allowed automodification of the base register, but as those


Trailing whitespace here.

> +   are now only allowed when @code{<} or @code{>} is used, @code{es} is
> +   basically the same as @code{m} without @code{<} and @code{>}."
>     (and (match_code "mem")
>          (match_test "GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) != RTX_AUTOINC")))
>
> @@ -216,36 +238,35 @@ (define_memory_constraint "Q"
>          (match_test "REG_P (XEXP (op, 0))")))
>
>   (define_memory_constraint "Y"
> -  "memory operand for 8 byte and 16 byte gpr load/store"
> +  "@internal A memory operand for a DQ-form instruction."
>     (and (match_code "mem")
>          (match_test "mem_operand_gpr (op, mode)")))
>
>   (define_memory_constraint "Z"
> -  "Memory operand that is an indexed or indirect from a register (it is
> -usually better to use @samp{m} or @samp{es} in @code{asm} statements)"
> +  "A memory operand accessed with indexed or indirect addressing."
>     (match_operand 0 "indexed_or_indirect_operand"))
>
>   ;; Address constraints
>
> -(define_address_constraint "a"
> -  "Indexed or indirect address operand"
> -  (match_operand 0 "indexed_or_indirect_address"))
> -
>   (define_constraint "R"
> -  "AIX TOC entry"
> +  "@internal An AIX TOC entry."
>     (match_test "legitimate_constant_pool_address_p (op, QImode, false)"))
>
> +(define_address_constraint "a"
> +  "An indexed or indirect address."
> +  (match_operand 0 "indexed_or_indirect_address"))
> +
>   ;; General constraints
>
>   (define_constraint "U"
> -  "V.4 small data reference"
> +  "@internal A V.4 small data reference."
>     (and (match_test "DEFAULT_ABI == ABI_V4")
>          (match_test "small_data_operand (op, mode)")))
>
>   (define_constraint "W"
> -  "vector constant that does not require memory"
> +  "@internal A vector constant that does not require memory."
>     (match_operand 0 "easy_vector_constant"))
>
>   (define_constraint "j"
> -  "Zero vector constant"
> +  "@internal The zero vector constant."
>     (match_test "op == const0_rtx || op == CONST0_RTX (mode)"))
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index cec74ea..119ed91 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -3187,27 +3187,32 @@ A memory reference that is encoded within the opcode.
>
>   @item PowerPC and IBM RS6000---@file{config/rs6000/constraints.md}
>   @table @code
> -@item b
> -Address base register
> +@item r
> +A general purpose register (GPR), @code{r0}@dots{}@code{r31}.
>
> -@item d
> -Floating point register (containing 64-bit value)
> +@item b
> +A base register.  Like @code{r}, but @code{r0} is not allowed, so
> +@code{r1}@dots{}@code{r31}.
>
>   @item f
> -Floating point register (containing 32-bit value)
> +A floating point register (FPR), @code{f0}@dots{}@code{f31}.
> +
> +@item d
> +A floating point register.  This is the same as @code{f} nowadays;
> +historically @code{f} was for single-precision and @code{d} was for
> +double-precision floating point.
>
>   @item v
> -Altivec vector register
> +An Altivec vector register (VR), @code{v0}@dots{}@code{v31}.
>
>   @item wa
> -Any VSX register if the @option{-mvsx} option was used or NO_REGS.
> +A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d} or a @code{v}
> +register.


Same concern as above.

>
> -When using the register constraint @code{wa}
> -that takes VSX registers, you must use @code{%x<n>} in the template so
> -that the correct register is used.  Otherwise the register number
> -output in the assembly file will be incorrect if an Altivec register
> -is an operand of a VSX instruction that expects VSX register
> -numbering.
> +When using @code{wa}, you must use @code{%x<n>} in the template, so that
> +the correct register number is used for Altivec registers (those are
> +registers @code{vs32}@dots{}@code{vs63}, but they would be printed as @code{0}@dots{}@code{31}
> +without the output modifier).  For example,
>
>   @smallexample
>   asm ("xvadddp %x0,%x1,%x2"
> @@ -3215,20 +3220,7 @@ asm ("xvadddp %x0,%x1,%x2"
>        : "wa" (v2), "wa" (v3));
>   @end smallexample
>
> -@noindent
> -is correct, but:
> -
> -@smallexample
> -asm ("xvadddp %0,%1,%2"
> -     : "=wa" (v1)
> -     : "wa" (v2), "wa" (v3));
> -@end smallexample
> -
> -@noindent
> -is not correct.
> -
> -If an instruction only takes Altivec registers, you do not want to use
> -@code{%x<n>}.
> +You should use @code{%x<n>} only for @code{wa} operands, never for @code{v}:
>
>   @smallexample
>   asm ("xsaddqp %0,%1,%2"
> @@ -3236,22 +3228,29 @@ asm ("xsaddqp %0,%1,%2"
>        : "v" (v2), "v" (v3));
>   @end smallexample
>
> -@noindent
> -is correct because the @code{xsaddqp} instruction only takes Altivec
> -registers, while:
> +@ifset INTERNALS
> +@item h
> +@code{vrsave}, @code{ctr}, or @code{lr}.
> +@end ifset


I don't see vrsave elsewhere in either document (should have noted this 
in constraints.md also).

>
> -@smallexample
> -asm ("xsaddqp %x0,%x1,%x2"
> -     : "=v" (v1)
> -     : "v" (v2), "v" (v3));
> -@end smallexample
> +@item c
> +The count register, @code{ctr}.
>
> -@noindent
> -is incorrect.
> +@item l
> +The link register, @code{lr}.
> +
> +@item x
> +Condition register field 0, @code{cr0}.
> +
> +@item y
> +Any condition register field, @code{cr0}@dots{}@code{cr7}.
> +
> +@ifset INTERNALS
> +@item z
> +The carry bit, @code{XER[CA]}.
>
>   @item we
> -VSX register if the @option{-mcpu=power9} and @option{-m64} options
> -were used or NO_REGS.
> +VSX register if the -mpower9-vector -m64 options were used or NO_REGS.


As above.  I won't call out the rest of these.

>
>   @item wn
>   No register (NO_REGS).
> @@ -3263,10 +3262,10 @@ General purpose register if 64-bit instructions are enabled or NO_REGS.
>   Floating point register if the STFIWX instruction is enabled or NO_REGS.
>
>   @item wA
> -Address base register if 64-bit instructions are enabled or NO_REGS.
> +BASE_REGS if 64-bit instructions are enabled or NO_REGS.
>
>   @item wB
> -Signed 5-bit constant integer that can be loaded into an altivec register.
> +Signed 5-bit constant integer that can be loaded into an Altivec register.
>
>   @item wD
>   Int constant that is the element number of the 64-bit scalar in a vector.
> @@ -3275,90 +3274,78 @@ Int constant that is the element number of the 64-bit scalar in a vector.
>   Vector constant that can be loaded with the XXSPLTIB instruction.
>
>   @item wF
> -Memory operand suitable for power8 GPR load fusion
> -
> -@item wG
> -Memory operand suitable for TOC fusion memory references.
> +Memory operand suitable for power8 GPR load fusion.
>
>   @item wL
> -Int constant that is the element number that the MFVSRLD instruction.
> -targets.
> +Int constant that is the element number mfvsrld accesses in a vector.
>
>   @item wM
>   Match vector constant with all 1's if the XXLORC instruction is available.
>
>   @item wO
> -A memory operand suitable for the ISA 3.0 vector d-form instructions.
> +Memory operand suitable for the ISA 3.0 vector d-form instructions.
>
>   @item wQ
> -A memory address that will work with the @code{lq} and @code{stq}
> -instructions.
> +Memory operand suitable for the load/store quad instructions.
>
>   @item wS
>   Vector constant that can be loaded with XXSPLTIB & sign extension.
>
> -@item h
> -@samp{VRSAVE}, @samp{CTR}, or @samp{LINK} register
> +@item wY
> +A memory operand for a DS-form instruction.
>
> -@item c
> -@samp{CTR} register
> -
> -@item l
> -@samp{LINK} register
> -
> -@item x
> -@samp{CR} register (condition register) number 0
> -
> -@item y
> -@samp{CR} register (condition register)
> -
> -@item z
> -@samp{XER[CA]} carry bit (part of the XER register)
> +@item wZ
> +Indexed or indirect memory operand, ignoring the bottom 4 bits.
> +@end ifset


For consistency, "An indexed..." ?

>
>   @item I
> -Signed 16-bit constant
> +A signed 16-bit constant.
>
>   @item J
> -Unsigned 16-bit constant shifted left 16 bits (use @samp{L} instead for
> -@code{SImode} constants)
> +An unsigned 16-bit constant shifted left 16 bits (use @code{L} instead
> +for @code{SImode} constants).
>
>   @item K
> -Unsigned 16-bit constant
> +An unsigned 16-bit constant.
>
>   @item L
> -Signed 16-bit constant shifted left 16 bits
> +A signed 16-bit constant shifted left 16 bits.
>
> +@ifset INTERNALS
>   @item M
> -Constant larger than 31
> +An integer constant greater than 31.
>
>   @item N
> -Exact power of 2
> +An exact power of 2.
>
>   @item O
> -Zero
> +The integer constant zero.
>
>   @item P
> -Constant whose negation is a signed 16-bit constant
> +A constant whose negation is a signed 16-bit constant.
> +@end ifset
>
>   @item eI
> -Signed 34-bit integer constant if prefixed instructions are supported.
> +A signed 34-bit integer constant if prefixed instructions are supported.
>
> +@ifset INTERNALS
>   @item G
> -Floating point constant that can be loaded into a register with one
> -instruction per word
> +A floating point constant that can be loaded into a register with one
> +instruction per word.
>
>   @item H
> -Integer/Floating point constant that can be loaded into a register using
> -three instructions
> +A floating point constant that can be loaded into a register using
> +three instructions.
> +@end ifset
>
>   @item m
> -Memory operand.
> +A memory operand.
>   Normally, @code{m} does not allow addresses that update the base register.
> -If @samp{<} or @samp{>} constraint is also used, they are allowed and
> +If the @code{<} or @code{>} constraint is also used, they are allowed and
>   therefore on PowerPC targets in that case it is only safe
> -to use @samp{m<>} in an @code{asm} statement if that @code{asm} statement
> +to use @code{m<>} in an @code{asm} statement if that @code{asm} statement
>   accesses the operand exactly once.  The @code{asm} statement must also
> -use @samp{%U@var{<opno>}} as a placeholder for the ``update'' flag in the
> +use @code{%U@var{<opno>}} as a placeholder for the ``update'' flag in the
>   corresponding load or store instruction.  For example:
>
>   @smallexample
> @@ -3373,35 +3360,44 @@ asm ("st %1,%0" : "=m<>" (mem) : "r" (val));
>
>   is not.
>
> +@ifset INTERNALS
>   @item es
>   A ``stable'' memory operand; that is, one which does not include any
>   automodification of the base register.  This used to be useful when
> -@samp{m} allowed automodification of the base register, but as those are now only
> -allowed when @samp{<} or @samp{>} is used, @samp{es} is basically the same
> -as @samp{m} without @samp{<} and @samp{>}.
> +@code{m} allowed automodification of the base register, but as those
> +are now only allowed when @code{<} or @code{>} is used, @code{es} is
> +basically the same as @code{m} without @code{<} and @code{>}.
> +@end ifset
>
>   @item Q
>   A memory operand addressed by just a base register.
>
> -@item Z
> -Memory operand that is an indexed or indirect from a register (it is
> -usually better to use @samp{m} or @samp{es} in @code{asm} statements)
> +@ifset INTERNALS
> +@item Y
> +A memory operand for a DQ-form instruction.
> +@end ifset
>
> +@item Z
> +A memory operand that is an indexed or indirect from a register.


"indexed or indirect access"?

Great improvements!

Thanks,
Bill

> +
> +@ifset INTERNALS
>   @item R
> -AIX TOC entry
> +An AIX TOC entry.
> +@end ifset
>
>   @item a
> -Address operand that is an indexed or indirect from a register (@samp{p} is
> -preferable for @code{asm} statements)
> +An indexed or indirect address.
>
> +@ifset INTERNALS
>   @item U
> -System V Release 4 small data area reference
> +A V.4 small data reference.
>
>   @item W
> -Vector constant that does not require memory
> +A vector constant that does not require memory.
>
>   @item j
> -Vector constant that is all zeros.
> +The zero vector constant.
> +@end ifset
>
>   @end table
>

Hi Bill,

Thanks a lot for looking at this!  :-)

On Fri, Jan 31, 2020 at 08:49:21AM -0600, Bill Schmidt wrote:
> >+(define_register_constraint "wa" 
> >"rs6000_constraints[RS6000_CONSTRAINT_wa]"
> >+  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d}
> >+   or a @code{v} register.")
> 
> Not quite true, as the "d" register is only half of a VSX register.  It 
> may or may not be worth including a picture of register overlaps...

No, the "d" registers are the actual full registers, all 128 bits of it.
You often use them in a mode that uses only 64 bits, sure.

I was planning to update this to

(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  This is either an
   FPR (@code{d}) or a VR (@code{v}).")

Does that improve it?

The numbering thing is also mentioned in the %x output modifier stuff.
There must be a better way to present this, but I don't see it yet.  Hrm.

> >  (define_register_constraint "we" 
> >  "rs6000_constraints[RS6000_CONSTRAINT_we]"
> >-  "VSX register if the -mpower9-vector -m64 options were used or 
> >NO_REGS.")
> >+  "@internal VSX register if the -mpower9-vector -m64 options were used
> >+   or NO_REGS.")
> 
> Suggest changing "used or" to "used, else".

Or just "used."; this is internals documentation only, and all similar
constraints will ideally go away at some point (it just didn't fit in
easily with the "enabled" attribute yet; it probably should be just "p9"
for "isa" and test the TARGET_64BIT in the insn condition, something
like that.  Or maybe there shouldn't be separate handling for 64-bit
at all here).

> >  (define_register_constraint "wr" 
> >  "rs6000_constraints[RS6000_CONSTRAINT_wr]"
> >-  "General purpose register if 64-bit instructions are enabled or 
> >NO_REGS.")
> >+  "@internal General purpose register if 64-bit instructions are enabled
> >+   or NO_REGS.")
> 
> Similar here.

Yup.  I didn't change this, fwiw, just synched up md.texi and
constraints.md where they diverged.

> >  (define_memory_constraint "es"
> >-  "A ``stable'' memory operand; that is, one which does not include any
> >-automodification of the base register.  Unlike @samp{m}, this constraint
> >-can be used in @code{asm} statements that might access the operand
> >-several times, or that might not access it at all."
> >+  "@internal
> >+   A ``stable'' memory operand; that is, one which does not include any
> >+   automodification of the base register.  This used to be useful when
> >+   @code{m} allowed automodification of the base register, but as those
> 
> Trailing whitespace here.

Yeah, I don't know how I missed that, git tends to shout about it.
Fixed.

> >  @item wa
> >-Any VSX register if the @option{-mvsx} option was used or NO_REGS.
> >+A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d} or 
> >a @code{v}
> >+register.
> 
> Same concern as above.

It is literally the same text now (unless I messed up the c'n'p).

> >+@ifset INTERNALS
> >+@item h
> >+@code{vrsave}, @code{ctr}, or @code{lr}.
> >+@end ifset
> 
> 
> I don't see vrsave elsewhere in either document (should have noted this 
> in constraints.md also).

There is no other constraint for vrsave.  constraints.md says

(define_register_constraint "h" "SPECIAL_REGS"
  "@internal @code{vrsave}, @code{ctr}, or @code{lr}.")

(Same text, as should be).  It ends up only in gccint.*, not in gcc.* .

> >  @item we
> >-VSX register if the @option{-mcpu=power9} and @option{-m64} options
> >-were used or NO_REGS.
> >+VSX register if the -mpower9-vector -m64 options were used or NO_REGS.
> 
> As above.  I won't call out the rest of these.

Since this is not new text, and it now only ends up in the internals
documentation, and a lot of it should go away in the short term anyway,
and importantly I don't know a good simple way to write what it does
anyway (because it *isn't* simple), I hoped I could just keep this for
now.

Hrm, I lost markup there, will fix.

> >+@item wZ
> >+Indexed or indirect memory operand, ignoring the bottom 4 bits.
> >+@end ifset
> 
> For consistency, "An indexed..." ?

Yes, thanks!

> >+@item Z
> >+A memory operand that is an indexed or indirect from a register.
> 
> "indexed or indirect access"?

And s/from a register// yeah.

> Great improvements!

Thanks :-)

Somewhere it should say (in the gcc.* doc) that there are other
constraints and output modifiers as well, and some are even supported
for backwards compatibility, but here only the ones you should use are
mentioned.  Not sure where to do that.


Segher

On 1/31/20 9:42 AM, Segher Boessenkool wrote:
> Hi Bill,
>
> Thanks a lot for looking at this!  :-)
>
> On Fri, Jan 31, 2020 at 08:49:21AM -0600, Bill Schmidt wrote:
>>> +(define_register_constraint "wa"
>>> "rs6000_constraints[RS6000_CONSTRAINT_wa]"
>>> +  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d}
>>> +   or a @code{v} register.")
>> Not quite true, as the "d" register is only half of a VSX register.  It
>> may or may not be worth including a picture of register overlaps...
> No, the "d" registers are the actual full registers, all 128 bits of it.
> You often use them in a mode that uses only 64 bits, sure.


Perhaps that would be worth a few words when describing the "d" 
constraint, then.  This is not at all obvious to the casual user. Thanks!

>
> I was planning to update this to
>
> (define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
>    "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  This is either an
>     FPR (@code{d}) or a VR (@code{v}).")
>
> Does that improve it?


Yes, sure.

>
> The numbering thing is also mentioned in the %x output modifier stuff.
> There must be a better way to present this, but I don't see it yet.  Hrm.


I honestly thought that was pretty good as is.

Thanks again!
Bill

>
>>>   (define_register_constraint "we"
>>>   "rs6000_constraints[RS6000_CONSTRAINT_we]"
>>> -  "VSX register if the -mpower9-vector -m64 options were used or
>>> NO_REGS.")
>>> +  "@internal VSX register if the -mpower9-vector -m64 options were used
>>> +   or NO_REGS.")
>> Suggest changing "used or" to "used, else".
> Or just "used."; this is internals documentation only, and all similar
> constraints will ideally go away at some point (it just didn't fit in
> easily with the "enabled" attribute yet; it probably should be just "p9"
> for "isa" and test the TARGET_64BIT in the insn condition, something
> like that.  Or maybe there shouldn't be separate handling for 64-bit
> at all here).
>
>>>   (define_register_constraint "wr"
>>>   "rs6000_constraints[RS6000_CONSTRAINT_wr]"
>>> -  "General purpose register if 64-bit instructions are enabled or
>>> NO_REGS.")
>>> +  "@internal General purpose register if 64-bit instructions are enabled
>>> +   or NO_REGS.")
>> Similar here.
> Yup.  I didn't change this, fwiw, just synched up md.texi and
> constraints.md where they diverged.
>
>>>   (define_memory_constraint "es"
>>> -  "A ``stable'' memory operand; that is, one which does not include any
>>> -automodification of the base register.  Unlike @samp{m}, this constraint
>>> -can be used in @code{asm} statements that might access the operand
>>> -several times, or that might not access it at all."
>>> +  "@internal
>>> +   A ``stable'' memory operand; that is, one which does not include any
>>> +   automodification of the base register.  This used to be useful when
>>> +   @code{m} allowed automodification of the base register, but as those
>> Trailing whitespace here.
> Yeah, I don't know how I missed that, git tends to shout about it.
> Fixed.
>
>>>   @item wa
>>> -Any VSX register if the @option{-mvsx} option was used or NO_REGS.
>>> +A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d} or
>>> a @code{v}
>>> +register.
>> Same concern as above.
> It is literally the same text now (unless I messed up the c'n'p).
>
>>> +@ifset INTERNALS
>>> +@item h
>>> +@code{vrsave}, @code{ctr}, or @code{lr}.
>>> +@end ifset
>>
>> I don't see vrsave elsewhere in either document (should have noted this
>> in constraints.md also).
> There is no other constraint for vrsave.  constraints.md says
>
> (define_register_constraint "h" "SPECIAL_REGS"
>    "@internal @code{vrsave}, @code{ctr}, or @code{lr}.")
>
> (Same text, as should be).  It ends up only in gccint.*, not in gcc.* .
>
>>>   @item we
>>> -VSX register if the @option{-mcpu=power9} and @option{-m64} options
>>> -were used or NO_REGS.
>>> +VSX register if the -mpower9-vector -m64 options were used or NO_REGS.
>> As above.  I won't call out the rest of these.
> Since this is not new text, and it now only ends up in the internals
> documentation, and a lot of it should go away in the short term anyway,
> and importantly I don't know a good simple way to write what it does
> anyway (because it *isn't* simple), I hoped I could just keep this for
> now.
>
> Hrm, I lost markup there, will fix.
>
>>> +@item wZ
>>> +Indexed or indirect memory operand, ignoring the bottom 4 bits.
>>> +@end ifset
>> For consistency, "An indexed..." ?
> Yes, thanks!
>
>>> +@item Z
>>> +A memory operand that is an indexed or indirect from a register.
>> "indexed or indirect access"?
> And s/from a register// yeah.
>
>> Great improvements!
> Thanks :-)
>
> Somewhere it should say (in the gcc.* doc) that there are other
> constraints and output modifiers as well, and some are even supported
> for backwards compatibility, but here only the ones you should use are
> mentioned.  Not sure where to do that.
>
>
> Segher

On Fri, Jan 31, 2020 at 10:56:10AM -0600, Bill Schmidt wrote:
> On 1/31/20 9:42 AM, Segher Boessenkool wrote:
> >On Fri, Jan 31, 2020 at 08:49:21AM -0600, Bill Schmidt wrote:
> >>>+(define_register_constraint "wa"
> >>>"rs6000_constraints[RS6000_CONSTRAINT_wa]"
> >>>+  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a 
> >>>@code{d}
> >>>+   or a @code{v} register.")
> >>Not quite true, as the "d" register is only half of a VSX register.  It
> >>may or may not be worth including a picture of register overlaps...
> >No, the "d" registers are the actual full registers, all 128 bits of it.
> >You often use them in a mode that uses only 64 bits, sure.
> 
> Perhaps that would be worth a few words when describing the "d" 
> constraint, then.  This is not at all obvious to the casual user. Thanks!

"They should read the ISA, it's all explained right at the start of
Chapter 7".

"All instructions that operate on an FPR are redefined to operate on
doubleword element 0 of the corresponding VSR.  The contents of
doubleword element 1 of the VSR corresponding to a source FPR or FPR
pair for these instructions are ignored and the contents of doubleword
element 1 of the VSR corresponding to the target FPR or FPR pair for
these instructions are undefined."

The twist in GCC is that all register numbers always denote the whole
register.  Compares this to (reg:DI 3) vs. (reg:SI 3) (and even HI and
QI); here it is (reg:DF 32) vs. (reg:V2DF 32) (etc.)

None of the VSRs are separate registers, they just *are* the FPRs and
the VRs.  On the machines the FPRs are in some implementations really
only half the width of the VSRs, or the other half does not even exist
on older machines; but inside GCC, such details do not matter.

But I'll add some more words, sure, it is probably useful to explain
some of the basic setup :-)

The tricky part is to have it make sense when you read it in order, but
to also make sense when used as a reference.


Segher