Re: [PATCH] PowerPC address support clean, patch 3 of 4
On Thu, May 10, 2018 at 05:20:52PM -0500, Segher Boessenkool wrote:
> On Thu, May 10, 2018 at 05:49:12PM -0400, Michael Meissner wrote:
> > > > -/* Return true if we have D-form addressing in altivec registers. */
> > > > +/* Return true if we have D-form addressing (register+offset) in
> > > > either a
> > > > + specific reload register class or whether some reload register class
> > > > + supports d-form addressing. */
> > > > static inline bool
> > > > -mode_supports_vmx_dform (machine_mode mode)
> > > > +mode_supports_d_form (machine_mode mode,
> > > > + enum rs6000_reload_reg_type rt = RELOAD_REG_ANY)
> > > > {
> > > > - return ((reg_addr[mode].addr_mask[RELOAD_REG_VMX] &
> > > > RELOAD_REG_OFFSET) != 0);
> > > > + return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0);
> > > > }
> > >
> > > Will this overload help anything? It does not look that way, all current
> > > callers use a different argument (and all the same).
> >
> > All current callers just use the ANY option (except for these calls).
> > However
> > in the future, I'm planning on calling these functions with the specific
> > reload
> > register class (hence the change).
>
> No, they use RELOAD_REG_VMX. Unless there is something extra tricky
> about your patch?
Yes, point is to make it more general.
Right now, mode_supports_vmx_dform is only called to see if we have the ISA 3.0
d/ds/dq-form instructions. It is called in secondary reload to see if a
register supports d*-form insns. It is also called in preferred reload class
to say whether we would prefer just a FPR register for a d*-form insn or we can
also tolerate a VMX register.
However, as I contemplate reworking the memory support, I want to clean up a
lot of the code that knows certain modes can do certain address forms. I'm
getting to hate code like:
if (mode == SFmode || mode == DFmode || ...)
So I was just trying to clean things up, and migratate to using the
reg_addr[mode].addr_mask bits. Having the inline functions can also make the
line length smaller.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] PowerPC address support clean, patch 3 of 4
On Thu, May 10, 2018 at 05:49:12PM -0400, Michael Meissner wrote:
> > > -/* Return true if we have D-form addressing in altivec registers. */
> > > +/* Return true if we have D-form addressing (register+offset) in either a
> > > + specific reload register class or whether some reload register class
> > > + supports d-form addressing. */
> > > static inline bool
> > > -mode_supports_vmx_dform (machine_mode mode)
> > > +mode_supports_d_form (machine_mode mode,
> > > + enum rs6000_reload_reg_type rt = RELOAD_REG_ANY)
> > > {
> > > - return ((reg_addr[mode].addr_mask[RELOAD_REG_VMX] & RELOAD_REG_OFFSET)
> > > != 0);
> > > + return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0);
> > > }
> >
> > Will this overload help anything? It does not look that way, all current
> > callers use a different argument (and all the same).
>
> All current callers just use the ANY option (except for these calls). However
> in the future, I'm planning on calling these functions with the specific
> reload
> register class (hence the change).
No, they use RELOAD_REG_VMX. Unless there is something extra tricky
about your patch?
> > Overloads are nice if they make things *easier* for the reader, not harder.
> > Same as with all other syntactic sugar.
Segher
Re: [PATCH] PowerPC address support clean, patch 3 of 4
On Wed, May 09, 2018 at 06:14:27PM -0500, Segher Boessenkool wrote:
> On Thu, May 03, 2018 at 01:22:10PM -0400, Michael Meissner wrote:
> > 2018-05-03 Michael Meissner
> >
> > * config/rs6000/rs6000.c (mode_supports_d_form): Rename
> > mode_supports_vmx_dform to mode_supports_d_form. Add an optional
> > argument to say which reload register class to use. Change all
> > callers to pass in the RELOAD_REG_VMX class explicitly.
> > (rs6000_secondary_reload): Likewise.
> > (rs6000_preferred_reload_class): Likewise.
> > (rs6000_secondary_reload_class): Likewise.
>
> Please don't say "likewise" unless the change is actually similar.
>
> > -/* Return true if we have D-form addressing in altivec registers. */
> > +/* Return true if we have D-form addressing (register+offset) in either a
> > + specific reload register class or whether some reload register class
> > + supports d-form addressing. */
> > static inline bool
> > -mode_supports_vmx_dform (machine_mode mode)
> > +mode_supports_d_form (machine_mode mode,
> > + enum rs6000_reload_reg_type rt = RELOAD_REG_ANY)
> > {
> > - return ((reg_addr[mode].addr_mask[RELOAD_REG_VMX] & RELOAD_REG_OFFSET)
> > != 0);
> > + return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0);
> > }
>
> Will this overload help anything? It does not look that way, all current
> callers use a different argument (and all the same).
All current callers just use the ANY option (except for these calls). However
in the future, I'm planning on calling these functions with the specific reload
register class (hence the change).
> Overloads are nice if they make things *easier* for the reader, not harder.
> Same as with all other syntactic sugar.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] PowerPC address support clean, patch 3 of 4
On Thu, May 03, 2018 at 01:22:10PM -0400, Michael Meissner wrote:
> 2018-05-03 Michael Meissner
>
> * config/rs6000/rs6000.c (mode_supports_d_form): Rename
> mode_supports_vmx_dform to mode_supports_d_form. Add an optional
> argument to say which reload register class to use. Change all
> callers to pass in the RELOAD_REG_VMX class explicitly.
> (rs6000_secondary_reload): Likewise.
> (rs6000_preferred_reload_class): Likewise.
> (rs6000_secondary_reload_class): Likewise.
Please don't say "likewise" unless the change is actually similar.
> -/* Return true if we have D-form addressing in altivec registers. */
> +/* Return true if we have D-form addressing (register+offset) in either a
> + specific reload register class or whether some reload register class
> + supports d-form addressing. */
> static inline bool
> -mode_supports_vmx_dform (machine_mode mode)
> +mode_supports_d_form (machine_mode mode,
> + enum rs6000_reload_reg_type rt = RELOAD_REG_ANY)
> {
> - return ((reg_addr[mode].addr_mask[RELOAD_REG_VMX] & RELOAD_REG_OFFSET) !=
> 0);
> + return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0);
> }
Will this overload help anything? It does not look that way, all current
callers use a different argument (and all the same).
Overloads are nice if they make things *easier* for the reader, not harder.
Same as with all other syntactic sugar.
Segher
Re: [PATCH] PowerPC address support clean, patch 3 of 4
These patches were previously posted in March as a RFC, and I would like to
check them into the trunk. These patches make the mode_supports* functions use
similar names for the functions that return if a mode supports D-FORM, DS-FORM,
and/or DQ-FORM instructions, and add the ability to ask whether a particular
reload register class supports a particular D*-form instruction.
This is patch #3 of 4.
I have done a bootstrap with patches 1-4 and did a make check comparison on a
little endian power8 system, and there were no regressions. Can I check it in?
2018-05-03 Michael Meissner
* config/rs6000/rs6000.c (mode_supports_d_form): Rename
mode_supports_vmx_dform to mode_supports_d_form. Add an optional
argument to say which reload register class to use. Change all
callers to pass in the RELOAD_REG_VMX class explicitly.
(rs6000_secondary_reload): Likewise.
(rs6000_preferred_reload_class): Likewise.
(rs6000_secondary_reload_class): Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 259877)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -553,11 +553,14 @@ mode_supports_pre_modify_p (machine_mode
!= 0);
}
-/* Return true if we have D-form addressing in altivec registers. */
+/* Return true if we have D-form addressing (register+offset) in either a
+ specific reload register class or whether some reload register class
+ supports d-form addressing. */
static inline bool
-mode_supports_vmx_dform (machine_mode mode)
+mode_supports_d_form (machine_mode mode,
+ enum rs6000_reload_reg_type rt = RELOAD_REG_ANY)
{
- return ((reg_addr[mode].addr_mask[RELOAD_REG_VMX] & RELOAD_REG_OFFSET) != 0);
+ return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0);
}
/* Return true if we have D-form addressing in VSX registers. This addressing
@@ -19415,7 +19418,7 @@ rs6000_secondary_reload (bool in_p,
point register, unless we have D-form addressing. Also make sure that
non-zero constants use a FPR. */
if (!done_p && reg_addr[mode].scalar_in_vmx_p
- && !mode_supports_vmx_dform (mode)
+ && !mode_supports_d_form (mode, RELOAD_REG_VMX)
&& (rclass == VSX_REGS || rclass == ALTIVEC_REGS)
&& (memory_p || (GET_CODE (x) == CONST_DOUBLE)))
{
@@ -19978,7 +19981,7 @@ rs6000_preferred_reload_class (rtx x, en
}
/* D-form addressing can easily reload the value. */
- if (mode_supports_vmx_dform (mode)
+ if (mode_supports_d_form (mode, RELOAD_REG_VMX)
|| mode_supports_dq_form (mode))
return rclass;
@@ -20135,7 +20138,7 @@ rs6000_secondary_reload_class (enum reg_
instead of reloading the secondary memory address for Altivec moves. */
if (TARGET_VSX
&& GET_MODE_SIZE (mode) < 16
- && !mode_supports_vmx_dform (mode)
+ && !mode_supports_d_form (mode, RELOAD_REG_VMX)
&& (((rclass == GENERAL_REGS || rclass == BASE_REGS)
&& (regno >= 0 && ALTIVEC_REGNO_P (regno)))
|| ((rclass == VSX_REGS || rclass == ALTIVEC_REGS)
