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
