This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Unreviewed patch^3
Hi,
thanks for reviewing.
>> http://gcc.gnu.org/ml/gcc-patches/2005-04/msg00659.html
>
> The big if block at the front replicates much of the code that
> follows it. Since offset is only accumulated, it doesn't seem
> that you'd lose anything by moving that adjustment later and
> not duplicating code.
The code I've duplicated works on USEDREG/SETREG with all subreg expressions
already stripped. I've copied that code to the very beginning of the
function because I needed the outermost modes for adjusting OFFSET.
So it is a different calculation done in the copied code.
> More seriously,
>
> ! /* Do not combine registers if they overlap. */
> ! || (ureg - offset <= sreg && ureg + usize > sreg + MIN (offset,
> 0))
> ! || (ureg - offset >= sreg && ureg - MAX (offset, 0) < sreg +
> ssize)
>
> Comparisons of ureg and sreg like this make no sense if they are
> pseudo register numbers. What's wrong with the original offset check?
- /* Do not combine registers unless one fits within the other. */
- || (offset > 0 && usize + offset > ssize)
- || (offset < 0 && usize + offset < ssize)
I must admit that it is not completely clear to me what the purpose of
these checks is. To my mind they unnecessarily circumvent combining registers
if the setreg is a subreg. e.g:
setreg: (subreg:SI (reg:DI x) 4)
usedreg: (reg:SI y)
the offset would be -1, ssize=2 and usize=1 (32bit system)
Hence the second part of the check would make the function to be left.
I ran into that problem because I wanted a TImode setreg to be combined with
a DImode usedreg which is similar to the case above:
setreg: (reg:TI x) (regarding offset this is handled like
(subreg:DI (reg:TI x) 8) on 64bit big endian)
usedreg: (reg:DI y)
offset would be -1 (set by the initial IF I've added)
ssize=2 and usize=1
Again the second part of this check would circumvent the two regs from being
combined.
Besides the cases above I didn't found a case to trigger these checks so I've
simply taken them out.
Regarding the new checks I've added I must agree that they are indeed pointless.
They are wrong for pseudos and hard registers aren't combined anyway.
So I've removed these too.
So here is the revised version of the patch.
Bootstrapped without testsuite regressions on s390 and s390x.
i686 bootstrap still running
OK? (provided there are no i686 regressions)
Bye,
-Andreas-
2005-05-11 Andreas Krebbel <krebbel1@de.ibm.com>
* local-alloc.c (combine_regs): Adjust reg_offset if regs with different mode
sizes are combined on big endian machines.
Index: gcc/local-alloc.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/local-alloc.c,v
retrieving revision 1.147
diff -p -c -r1.147 local-alloc.c
*** gcc/local-alloc.c 28 Apr 2005 05:38:32 -0000 1.147
--- gcc/local-alloc.c 11 May 2005 14:10:27 -0000
*************** combine_regs (rtx usedreg, rtx setreg, i
*** 1751,1756 ****
--- 1751,1780 ----
int usize, ssize;
int sqty;
+ if (WORDS_BIG_ENDIAN
+ && SCALAR_INT_MODE_P (GET_MODE (setreg))
+ && SCALAR_INT_MODE_P (GET_MODE (usedreg))
+ && GET_MODE_SIZE (GET_MODE (setreg)) != GET_MODE_SIZE (GET_MODE (usedreg))
+ && (GET_MODE_SIZE (GET_MODE (setreg)) > UNITS_PER_WORD
+ || GET_MODE_SIZE (GET_MODE (usedreg)) > UNITS_PER_WORD))
+ {
+ if (REG_P (setreg) && REGNO (setreg) < FIRST_PSEUDO_REGISTER)
+ ssize = hard_regno_nregs[REGNO (setreg)][GET_MODE (setreg)];
+ else
+ ssize = ((GET_MODE_SIZE (GET_MODE (setreg))
+ + (REGMODE_NATURAL_SIZE (GET_MODE (setreg)) - 1))
+ / REGMODE_NATURAL_SIZE (GET_MODE (setreg)));
+
+ if (REG_P (usedreg) && REGNO (usedreg) < FIRST_PSEUDO_REGISTER)
+ usize = hard_regno_nregs[REGNO (usedreg)][GET_MODE (usedreg)];
+ else
+ usize = ((GET_MODE_SIZE (GET_MODE (usedreg))
+ + (REGMODE_NATURAL_SIZE (GET_MODE (usedreg)) - 1))
+ / REGMODE_NATURAL_SIZE (GET_MODE (usedreg)));
+
+ offset = usize - ssize;
+ }
+
/* Determine the numbers and sizes of registers being used. If a subreg
is present that does not change the entire register, don't consider
this a copy insn. */
*************** combine_regs (rtx usedreg, rtx setreg, i
*** 1825,1833 ****
quantity number, it means that it is not local to this block or dies
more than once. In either event, we can't do anything with it. */
if ((ureg >= FIRST_PSEUDO_REGISTER && reg_qty[ureg] < 0)
- /* Do not combine registers unless one fits within the other. */
- || (offset > 0 && usize + offset > ssize)
- || (offset < 0 && usize + offset < ssize)
/* Do not combine with a smaller already-assigned object
if that smaller object is already combined with something bigger. */
|| (ssize > usize && ureg >= FIRST_PSEUDO_REGISTER
--- 1849,1854 ----