Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On Thu, Jul 04, 2019 at 07:19:56AM +0900, Stafford Horne wrote:
> On Wed, Jul 03, 2019 at 09:09:51PM +0200, Richard Henderson wrote:
> > On 7/3/19 5:43 PM, Segher Boessenkool wrote:
> > >> @@ -212,6 +214,7 @@ enum reg_class
> > >> #define REG_CLASS_CONTENTS \
> > >> { { 0x, 0x }, \
> > >>{ SIBCALL_REGS_MASK, 0 }, \
> > >> + { 0x7efe, 0x }, \
> > >
> > > Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> > > in GCC register numbers, 0, 8, and 31? You probably should mention r8
> > > somewhere (it's because it is the last arg, this avoid problems, I
> > > guess?),
> > > and the 30/31 thing is confused some way. Maybe it is all just that one
> > > documentation line :-)
> >
> > ... and if r8 is excluded because of arguments, I suspect that this is the
> > wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a
> > pair,
> > at least that I can see.
> >
> > Perhaps function_arg and/or function_arg_advance is the right place for a
> > fix?
> > The calling convention says that 64-bit arguments are not split across
> > registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
>
> I will double check, the mask may be wrong. It should not matter about the
> function args.
>
> I didn't see any issue that caused me to add r8. So I may have just masked
> thw
> rong bit thinking it's r31. Is there something worng with what I did?
>
> The mask is 0x7efe, and names should corresbond to this name list?
>
> #define REGISTER_NAMES {
> "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", # 7e, excl r0
> "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", # ff, excl
> none
> "r17", "r19", "r21", "r23", "r25", "r27", "r29", "r31", # fe, excl
> r31
> "r16", "r18", "r20", "r22", "r24", "r26", "r28", "r30", # fe, excl
> r30
> "?ap", "?fp", "?sr_f" }
>
> Do I have it backwards? With an endian issue?
Yes :-) 0x0001 is reg 0 (r0), 0x8000 is reg 31 (r30).
Segher
Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On Wed, Jul 03, 2019 at 09:09:51PM +0200, Richard Henderson wrote:
> On 7/3/19 5:43 PM, Segher Boessenkool wrote:
> >> @@ -212,6 +214,7 @@ enum reg_class
> >> #define REG_CLASS_CONTENTS \
> >> { { 0x, 0x }, \
> >>{ SIBCALL_REGS_MASK, 0 }, \
> >> + { 0x7efe, 0x }, \
> >
> > Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> > in GCC register numbers, 0, 8, and 31? You probably should mention r8
> > somewhere (it's because it is the last arg, this avoid problems, I guess?),
> > and the 30/31 thing is confused some way. Maybe it is all just that one
> > documentation line :-)
>
> ... and if r8 is excluded because of arguments, I suspect that this is the
> wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a pair,
> at least that I can see.
>
> Perhaps function_arg and/or function_arg_advance is the right place for a fix?
> The calling convention says that 64-bit arguments are not split across
> registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
I will double check, the mask may be wrong. It should not matter about the
function args.
I didn't see any issue that caused me to add r8. So I may have just masked thw
rong bit thinking it's r31. Is there something worng with what I did?
The mask is 0x7efe, and names should corresbond to this name list?
#define REGISTER_NAMES {
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", # 7e, excl r0
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", # ff, excl none
"r17", "r19", "r21", "r23", "r25", "r27", "r29", "r31", # fe, excl r31
"r16", "r18", "r20", "r22", "r24", "r26", "r28", "r30", # fe, excl r30
"?ap", "?fp", "?sr_f" }
Do I have it backwards? With an endian issue?
-Stafford
Re: [PATCH v2 4/5] or1k: Initial support for FPU
On Wed, Jul 03, 2019 at 10:43:01AM -0500, Segher Boessenkool wrote:
> Hi Stafford,
>
> On Wed, Jul 03, 2019 at 12:33:50PM +0900, Stafford Horne wrote:
> > +case 'd':
> > + if (REG_P (x))
> > + if (GET_MODE (x) == DFmode || GET_MODE (x) == DImode)
> > + fprintf (file, "%s,%s", reg_names[REGNO (operand)],
> > + reg_names[REGNO (operand) + 1]);
> > + else
> > + fprintf (file, "%s", reg_names[REGNO (operand)]);
> > + else
>
> The coding conventions says to use braces around nested conditionals.
Right I will fix that. Interestingly the indentation is correct just missing
the braces.
> > @@ -212,6 +214,7 @@ enum reg_class
> > #define REG_CLASS_CONTENTS \
> > { { 0x, 0x }, \
> >{ SIBCALL_REGS_MASK, 0 }, \
> > + { 0x7efe, 0x }, \
>
> Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> in GCC register numbers, 0, 8, and 31? You probably should mention r8
> somewhere (it's because it is the last arg, this avoid problems, I guess?),
> and the 30/31 thing is confused some way. Maybe it is all just that one
> documentation line :-)
>
> > +; d - double pair base registers (excludes r0, r30 and r31 which overflow)
Hmm, maybe I messed up the mask. It should be r0, r30 and r31. Register pairs
can be a base register (rX) with a +1 or +2 offset second register.
Registers not allowed
- r0, because its reserved for hardwired zero and doesn't work as a double
zero when paired with a general register.
- r31, because it cant pair with r32 or r33 (those are overflows)
- r30, because it cant work when paried with r32 (its an overflow), it would
work with r31, but GCC will not generate that pair anyway.
-Stafford
Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On 7/3/19 5:43 PM, Segher Boessenkool wrote:
>> @@ -212,6 +214,7 @@ enum reg_class
>> #define REG_CLASS_CONTENTS \
>> { { 0x, 0x }, \
>>{ SIBCALL_REGS_MASK, 0 }, \
>> + { 0x7efe, 0x }, \
>
> Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> in GCC register numbers, 0, 8, and 31? You probably should mention r8
> somewhere (it's because it is the last arg, this avoid problems, I guess?),
> and the 30/31 thing is confused some way. Maybe it is all just that one
> documentation line :-)
... and if r8 is excluded because of arguments, I suspect that this is the
wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a pair,
at least that I can see.
Perhaps function_arg and/or function_arg_advance is the right place for a fix?
The calling convention says that 64-bit arguments are not split across
registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
r~
Re: [PATCH v2 4/5] or1k: Initial support for FPU
Hi Stafford,
On Wed, Jul 03, 2019 at 12:33:50PM +0900, Stafford Horne wrote:
> +case 'd':
> + if (REG_P (x))
> + if (GET_MODE (x) == DFmode || GET_MODE (x) == DImode)
> + fprintf (file, "%s,%s", reg_names[REGNO (operand)],
> + reg_names[REGNO (operand) + 1]);
> + else
> + fprintf (file, "%s", reg_names[REGNO (operand)]);
> + else
The coding conventions says to use braces around nested conditionals.
> @@ -212,6 +214,7 @@ enum reg_class
> #define REG_CLASS_CONTENTS \
> { { 0x, 0x },\
>{ SIBCALL_REGS_MASK, 0 },\
> + { 0x7efe, 0x },\
Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
in GCC register numbers, 0, 8, and 31? You probably should mention r8
somewhere (it's because it is the last arg, this avoid problems, I guess?),
and the 30/31 thing is confused some way. Maybe it is all just that one
documentation line :-)
> +; d - double pair base registers (excludes r0, r30 and r31 which overflow)
Segher
[PATCH v2 4/5] or1k: Initial support for FPU
This adds support for OpenRISC hardware floating point instructions.
This is enabled with the -mhard-float option.
Double-prevision floating point operations work using register pairing as
specified in: https://openrisc.io/proposals/orfpx64a32. This has just been
added in the OpenRISC architecture specification 1.3.
This is enabled with the -mdouble-float option.
Not all architectures support unordered comparisons so an option,
-munordered-float is added.
Currently OpenRISC does not support sf/df or df/sf conversions, but this has
also just been added in architecture specification 1.3.
gcc/ChangeLog:
* config.gcc (or1k*-*-*): Add mhard-float, mdouble-float, msoft-float
and munordered-float validations.
* config/or1k/constraints.md (d): New register constraint.
* config/or1k/predicates.md (fp_comparison_operator): New.
* config/or1k/or1k.c (or1k_print_operand): Add support for printing 'd'
operands.
(or1k_expand_compare): Normalize unordered comparisons.
* config/or1k/or1k.h (reg_class): Define DOUBLE_REGS.
(REG_CLASS_NAMES): Add "DOUBLE_REGS".
(REG_CLASS_CONTENTS): Add contents for DOUBLE_REGS.
* config/or1k/or1k.md (type): Add fpu.
(fpu): New instruction reservation.
(F, f, fr, fi, FI, FOP, fop): New.
(3): New ALU instruction definition.
(float2): New conversion instruction definition.
(fix_trunc2): New conversion instruction definition.
(fpcmpcc): New code iterator.
(*sf_fp_insn): New instruction definition.
(cstore4): New expand definition.
(cbranch4): New expand definition.
* config/or1k/or1k.opt (msoft-float, mhard-float, mdouble-float,
munordered-float): New options.
* doc/invoke.texi: Document msoft-float, mhard-float, mdouble-float and
munordered-float.
---
Changes since v1:
- User register pairs for 64-bit ops. See or1k_print_operand
gcc/config.gcc | 1 +
gcc/config/or1k/constraints.md | 4 ++
gcc/config/or1k/or1k.c | 36 ++-
gcc/config/or1k/or1k.h | 3 +
gcc/config/or1k/or1k.md| 111 -
gcc/config/or1k/or1k.opt | 22 +++
gcc/config/or1k/predicates.md | 5 ++
gcc/doc/invoke.texi| 21 +++
8 files changed, 199 insertions(+), 4 deletions(-)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index aeab8b4544e..1678109131f 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -2579,6 +2579,7 @@ or1k*-*-*)
case ${or1k_multilib} in
mcmov | msext | msfimm | \
mror | mrori | \
+ mhard-float | mdouble-float | munordered-float | msoft-float | \
mhard-div | mhard-mul | \
msoft-div | msoft-mul )
TM_MULTILIB_CONFIG="${TM_MULTILIB_CONFIG},${or1k_multilib}"
diff --git a/gcc/config/or1k/constraints.md b/gcc/config/or1k/constraints.md
index 93da8c058c6..8cac7eb5329 100644
--- a/gcc/config/or1k/constraints.md
+++ b/gcc/config/or1k/constraints.md
@@ -24,6 +24,7 @@
; We use:
; c - sibcall registers
+; d - double pair base registers (excludes r0, r30 and r31 which overflow)
; I - constant signed 16-bit
; K - constant unsigned 16-bit
; M - constant signed 16-bit shifted left 16-bits (l.movhi)
@@ -32,6 +33,9 @@
(define_register_constraint "c" "SIBCALL_REGS"
"Registers which can hold a sibling call address")
+(define_register_constraint "d" "DOUBLE_REGS"
+ "Registers which can be used for double reg pairs.")
+
;; Immediates
(define_constraint "I"
"A signed 16-bit immediate in the range -32768 to 32767."
diff --git a/gcc/config/or1k/or1k.c b/gcc/config/or1k/or1k.c
index 54c9e804ea5..d90826b75ca 100644
--- a/gcc/config/or1k/or1k.c
+++ b/gcc/config/or1k/or1k.c
@@ -1226,6 +1226,17 @@ or1k_print_operand (FILE *file, rtx x, int code)
output_operand_lossage ("invalid %%H value");
break;
+case 'd':
+ if (REG_P (x))
+ if (GET_MODE (x) == DFmode || GET_MODE (x) == DImode)
+ fprintf (file, "%s,%s", reg_names[REGNO (operand)],
+ reg_names[REGNO (operand) + 1]);
+ else
+ fprintf (file, "%s", reg_names[REGNO (operand)]);
+ else
+ output_operand_lossage ("invalid %%d value");
+ break;
+
case 'h':
print_reloc (file, x, 0, RKIND_HI);
break;
@@ -1435,21 +1446,42 @@ void
or1k_expand_compare (rtx *operands)
{
rtx sr_f = gen_rtx_REG (BImode, SR_F_REGNUM);
+ rtx_code cmp_code = GET_CODE (operands[0]);
+ bool flag_check_ne = true;
/* The RTL may receive an immediate in argument 1 of the compare, this is not
supported unless we have l.sf*i instructions, force them into registers.
*/
if (!TARGET_SFIMM)
XEXP (operands[0], 1) = force_reg (SImode, XEXP (operands[0], 1));
+ /* Normalize comparison operators to ones OpenRISC support. */
+
