This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Remove SLOW_BYTE_ACCESS from get_best_mode


"H. J. Lu" <hjl@lucon.org> writes:

> 2006-09-13  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	* config/i386/i386.h (SLOW_BYTE_ACCESS): Update comment.
> 	(WIDEN_MODE_ACCESS_BITFIELD): New.
> 	(SLOW_SHORT_ACCESS): Removed.
> 
> 	* doc/tm.texi (SLOW_BYTE_ACCESS): Updated.
> 	(WIDEN_MODE_ACCESS_BITFIELD): Document.
> 
> 	* defaults.h (WIDEN_MODE_ACCESS_BITFIELD): New. Default to
> 	SLOW_BYTE_ACCESS.
> 
> 	* stor-layout.c (SLOW_BYTE_ACCESS): Renamed to ...
> 	(WIDEN_MODE_ACCESS_BITFIELD): This.

I would like to see a different approach here.

It seems clear to me that the use of SLOW_BYTE_ACCESS in stor-layout.c
conforms to the documentation of SLOW_BYTE_ACCESS: SLOW_BYTE_ACCESS
should be defined as non-zero if accessing a byte *is no faster* than
accessing a word.  By that definition, very few modern processors
should define SLOW_BYTE_ACCESS as zero.  And the use in stor-layout.c
conforms to that definition.

The use of SLOW_BYTE_ACCESS in dojump.c, on the other hand, does not
conform to the documentation.  In dojump.c the code is trying to test
whether the target supports a comparison in a smaller mode.  That use
of SLOW_BYTE_ACCESS was introduced here:

Wed Jun 24 19:15:14 1992  Richard Kenner  (kenner@vlsi1.ultra.nyu.edu)

        * expr.c (do_jump, case BIT_AND_EXPR, COMPONENT_REF): Don't narrow
        comparison unless byte accesses are not slow and we have a
        comparison in the new mode.

That code tests both SLOW_BYTE_ACCESS and cmp_optab.

The following targets set SLOW_BYTE_ACCESS to 1: alpha, arc, crx,
fr30, frv, h8300 (if -mslowbyte is used), ia64, iq2000, m32c, m32r,
mcore (if -mslow-bytes is used), mips, mn10300, mt, pa, rs6000, s390,
sh, sparc, v850, xtensa.

Of those targets, the following have small cmpMM insns: crx, h8300,
m32c.  As far as I can see, those targets could benefit from the
optimization in dojump.c even though they define SLOW_BYTE_ACCESS to
1.

So I think that we should simply remove the test of SLOW_BYTE_ACCESS
in dojump.c.  I don't think it conforms to the documentation of
SLOW_BYTE_ACCESS, and I don't think it helps for any actual target.

That will leave us with a single use of SLOW_BYTE_ACCESS, in
stor-layout.c.  The following targets set SLOW_BYTE_ACCESS to 0: arm,
avr, bfin, c4x, cris, h8300 (if -mslowbyte is not used), i386,
m68hc11, m68k, mcore (if -mslow-bytes is not used), mmix, pdp11,
stormy16, vax.  I'm pretty sure that at least arm and i386 should be
definining SLOW_BYTE_ACCESS as 1.

So I think we should have two separate patches.  The first should
remove the test of SLOW_BYTE_ACCESS from the two locations in
dojump.c.  The second should rename SLOW_BYTE_ACCESS to, e.g.,
PREFER_WORD_ACCESS, and should ideally put it into the target
structure.  And as long as we are going to rename it, we should
probably pass in MODE, which is the narrowest mode that contains the
bitfield, and have the new target function return the preferred access
mode.  Then the code in get_best_mode should return a mode between
those two modes that fits the alignment constraints.

This is, of course, stage 1 material, not stage 3.

Thanks.

Incidentally, while looking into I discovered that the entire file
dojump.c was created without a ChangeLog entry.  Wow.  The ChangeLog
entry is in the CVS log, but the ChangeLog file was not committed:
    http://gcc.gnu.org/ml/gcc-cvs/2003-03/msg00706.html

Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]