i386 optimizer bug?
Jim Wilson
wilson@chestnut.cygnus.com
Tue Sep 2 14:38:00 GMT 1997
The problem is a bug in update_equiv_regs. It makes contradictory changes
which result in a reference to a pseudo before it has been initialized.
We start with RTL:
(insn 12 10 14 (set (reg/v:SI 25)
(mem:SI (plus:SI (reg:SI 16 %argp)
(const_int 16)))) 52 {movsi+1} (nil)
(expr_list:REG_EQUIV (mem:SI (plus:SI (reg:SI 16 %argp)
(const_int 16)))
(nil)))
...
(insn 95 92 96 (set (reg:SI 37)
(mem/s:SI (plus:SI (reg:SI 6 %ebp)
(const_int -8)))) 52 {movsi+1} (nil)
(nil))
(insn 96 95 99 (set (mem:SI (reg/v:SI 25))
(reg:SI 37)) 52 {movsi+1} (insn_list 95 (nil))
(expr_list:REG_DEAD (reg:SI 37)
(expr_list:REG_DEAD (reg/v:SI 25)
(nil))))
Reg 25 is set and used only once, so insn 12 is moved immediately before
insn 96 by the reg_equiv_replace[] code. Reg 37 is always equivalent
to (mem (reg 25)), so we add a REG_EQUIV note to insn 95. We end up with
(insn 95 92 197 (set (reg:SI 37)
(mem/s:SI (plus:SI (reg:SI 6 %ebp)
(const_int -8)))) 52 {movsi+1} (nil)
(expr_list:REG_EQUIV (mem:SI (reg/v:SI 25))
(expr_list:REG_EQUIV (mem/s:SI (plus:SI (reg:SI 6 %ebp)
(const_int -8)))
(nil)))
(insn 197 95 96 (set (reg/v:SI 25)
(mem:SI (plus:SI (reg:SI 16 %argp)
(const_int 16)))) 52 {movsi+1} (nil)
(expr_list:REG_EQUIV (mem:SI (plus:SI (reg:SI 16 %argp)
(const_int 16)))
(nil)))
(insn 96 197 99 (set (mem:SI (reg/v:SI 25))
(reg:SI 37)) 52 {movsi+1} (insn_list 95 (nil))
(expr_list:REG_DEAD (reg:SI 37)
(expr_list:REG_DEAD (reg/v:SI 25)
(nil))))
And now note that insn 95 references reg 25 before the instruction that
initializes it. If reload decides to use this REG_EQUIV note, then we
get code that fails at runtime.
I see two problems here:
1) It is pointless to add more than one REG_EQUIV note to an insn.
2) We can add a REG_EQUIV note using a insn that sets a MEM if the MEM
uses any registers which might be later replaced via the reg_equiv_replace
code.
The following patch fixes both problems. It isn't very pretty, but I think
it is very safe.
Tue Sep 2 14:22:43 1997 Jim Wilson <wilson@cygnus.com>
* local-alloc.c (contains_place_regs): New function.
(update_equiv_regs): When adding a REG_EQUIV note for a set of a MEM,
verify that there is no existing REG_EQUIV note, and add a call to
contains_place_regs.
Index: local-alloc.c
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/local-alloc.c,v
retrieving revision 1.3
diff -p -r1.3 local-alloc.c
*** local-alloc.c 1997/08/25 15:00:35 1.3
--- local-alloc.c 1997/09/02 21:22:10
*************** static void alloc_qty PROTO((int, enum
*** 246,251 ****
--- 246,252 ----
static void alloc_qty_for_scratch PROTO((rtx, int, rtx, int, int));
static void validate_equiv_mem_from_store PROTO((rtx, rtx));
static int validate_equiv_mem PROTO((rtx, rtx, rtx));
+ static int contains_replace_regs PROTO((rtx, char *));
static int memref_referenced_p PROTO((rtx, rtx));
static int memref_used_between_p PROTO((rtx, rtx, rtx));
static void optimize_reg_copy_1 PROTO((rtx, rtx, rtx));
*************** validate_equiv_mem (start, reg, memref)
*** 600,605 ****
--- 601,652 ----
return 0;
}
+
+ /* TRUE if X uses any registers for which reg_equiv_replace is true. */
+
+ static int
+ contains_replace_regs (x, reg_equiv_replace)
+ rtx x;
+ char *reg_equiv_replace;
+ {
+ int i, j;
+ char *fmt;
+ enum rtx_code code = GET_CODE (x);
+
+ switch (code)
+ {
+ case CONST_INT:
+ case CONST:
+ case LABEL_REF:
+ case SYMBOL_REF:
+ case CONST_DOUBLE:
+ case PC:
+ case CC0:
+ case HIGH:
+ case LO_SUM:
+ return 0;
+
+ case REG:
+ return reg_equiv_replace[REGNO (x)];
+ }
+
+ fmt = GET_RTX_FORMAT (code);
+ for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+ switch (fmt[i])
+ {
+ case 'e':
+ if (contains_replace_regs (XEXP (x, i), reg_equiv_replace))
+ return 1;
+ break;
+ case 'E':
+ for (j = XVECLEN (x, i) - 1; j >= 0; j--)
+ if (contains_replace_regs (XVECEXP (x, i, j), reg_equiv_replace))
+ return 1;
+ break;
+ }
+
+ return 0;
+ }
/* TRUE if X references a memory location that would be affected by a store
to MEMREF. */
*************** update_equiv_regs ()
*** 1005,1016 ****
in a single basic block, see if the register is always equivalent
to that memory location and if moving the store from INSN to the
insn that set REG is safe. If so, put a REG_EQUIV note on the
! initializing insn. */
if (GET_CODE (dest) == MEM && GET_CODE (SET_SRC (set)) == REG
&& (regno = REGNO (SET_SRC (set))) >= FIRST_PSEUDO_REGISTER
&& REG_BASIC_BLOCK (regno) >= 0
&& reg_equiv_init_insn[regno] != 0
&& validate_equiv_mem (reg_equiv_init_insn[regno], SET_SRC (set),
dest)
&& ! memref_used_between_p (SET_DEST (set),
--- 1052,1075 ----
in a single basic block, see if the register is always equivalent
to that memory location and if moving the store from INSN to the
insn that set REG is safe. If so, put a REG_EQUIV note on the
! initializing insn.
!
! Don't add a REG_EQUIV note if the insn already has one. The existing
! REG_EQUIV is likely more useful than the one we are adding.
!
! If one of the regs in the address is marked as reg_equiv_replace,
! then we can't add this REG_EQUIV note. The reg_equiv_replace
! optimization may move the set of this register immediately before
! insn, which puts it after reg_equiv_init_insn[regno], and hence
! the mention in the REG_EQUIV note would be to an uninitialized
! pseudo. */
if (GET_CODE (dest) == MEM && GET_CODE (SET_SRC (set)) == REG
&& (regno = REGNO (SET_SRC (set))) >= FIRST_PSEUDO_REGISTER
&& REG_BASIC_BLOCK (regno) >= 0
&& reg_equiv_init_insn[regno] != 0
+ && ! find_reg_note (insn, REG_EQUIV, NULL_RTX)
+ && ! contains_replace_regs (XEXP (dest, 0), reg_equiv_replace)
&& validate_equiv_mem (reg_equiv_init_insn[regno], SET_SRC (set),
dest)
&& ! memref_used_between_p (SET_DEST (set),
Testcase:
/*
* i386-linux-gnulibc1 optimizer bug? both egcs snapshots to date have
* this problem.
*
* the pointer "iflag" in rbeam_ becomes garbage and segfaults.
*/
#include <stdio.h>
#include <string.h>
#define DIM1 25001
#define DIM2 18
int rbeam18_ (
const char* fort_fname,
double ray[DIM1][DIM2],
int *ncol, int *npoint, int *iflag, int *ierr,
unsigned long fname_len
) {
FILE *fp;
int i;
int io_dummy[5];
int reclen;
char fname[256];
/* The following is wrong here, but needs to be here for segfaulting
to happen. The result fname is *not* used later on, so no harm is
done. */
for(i = fname_len; fort_fname[i-1] == ' '; --i)
;
strncpy(fname, fort_fname, i);
fname[i] = '\0';
*ierr = 0;
/* note the fopen uses "/dev/null", not the passed in one */
fp = fopen("/dev/null", "rb");
fread(io_dummy, sizeof(int), 5, fp);
/* set a breakpoint here and see that the parameter iflag is changed
after fread. */
*ncol = io_dummy[1];
*npoint = io_dummy[2];
*iflag = io_dummy[3]; /* <<<<<<<<<<< LOSE */
/* force these to read in a fixed set of numbers from /dev/null for
testing purposes. */
*ncol = 10;
*npoint = 10;
/* if I lose the following statement, it works! */
reclen = *ncol * sizeof(double);
for (i = 0; i < *npoint; ++i) {
fread(&reclen, sizeof(int), 1, fp);
fread(&reclen, sizeof(int), 1, fp);
}
fclose(fp);
return 0;
}
int main () {
int ncol, npoint, ierr, iflag;
static double ray[DIM1][DIM2];
rbeam18_("/dev/null", ray, &ncol, &npoint, &iflag, &ierr, 9L);
return 0;
}
More information about the Gcc-bugs
mailing list