This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
gcse fix
- To: gcc-patches at gcc dot gnu dot org
- Subject: gcse fix
- From: Ulf Carlsson <ulfc at calypso dot engr dot sgi dot com>
- Date: Mon, 3 Jul 2000 01:10:12 -0700 (PDT)
- Cc: Andreas Jaeger <aj at suse dot de>
Hi,
There was a problem with Linux kernel compiles when we used GCC 2.96,
and I have a patch that solves this problem. The fix is rather
obvious - we verify that the copy register instruction we generate
when optimize really is valid.
I sent the bug report[1] to the gcc-bugs last week. Keith Wesolowski
has also written a detailed description of the problem:
[Beginning of description by Keith M Wesolowski]
The function of GCSE is to eliminate pieces of code that end up
looking the same. So for example,
x = y + 4;
z = y + 4;
ends up looking like
z = y + 4;
x = z;
Provided that x is not changed between the original two statements, of
course. And this is key: if the compiler thinks the expression hasn't
changed, it will be removed.
In our case, we have
__asm__("":"=r" (foo)); __asm__("":"=r" (bar));
The rtl corresponding to the two asm sets are:
(insn 14 27 16 (set (reg:QI 83)
(asm_operands ("") ("=r") 0[ ]
[ ] ("foo.c") 7)) -1 (nil)
(nil))
and
(insn 19 18 21 (set (reg/v:SI 82)
(asm_operands ("") ("=r") 0[ ]
[ ] ("foo.c") 7)) -1 (nil)
(nil))
Note that both include a 7 in the instruction description - this is
the line number of the inline asm. Since they're on the same line, and
contain the same code and arguments, *** they are the same
instruction. *** The compiler is not smart enough to know that the
instruction operates on operands of two different modes and thus
should be considered different - all it knows is the text inside
__asm__() and the line number.
So the pre_gcse pass removes the first copy of the __asm__ and copies
the value of the output of the last instance of it (word) into the
output register for the first instance of it (byte). Copying an SI (32
bits) into a QI (8 bits) requires a subreg, but the compiler doesn't
generate one. ICE.
Consistent with my explanation, any of the following eliminates the ICE:
- using __asm__ volatile (...) instead of __asm__ (...)
- switching the order of dirty (byte) and dirty (word)
- making sure only one instance of dirty() is ever on a line
- compiling with -fno-gcse
This is not to say there is no bug; it seems to be that if
dirty (word); dirty (byte);
is valid and generates valid (even if incorrect - the flip side of
subreg is zero/sign extension) code, then placing them in the opposite
order should do so as well. This requires that gcc be taught to check
the mode of operands in common subexpressions and either a) recognize
them as different if the operands' modes differ, or b) use subreg or
zero/sign extension as appropriate, or c) distinguish among instances
of inline asm even if the instances occur on the same line.
[End of description by Keith M Wesolowski]
We have verified that the patch doesn't break things by boot strapping
the compiler itself on Sparc. We have also built functional kernels
and glibc with this patch.
Ulf
[1] http://gcc.gnu.org/ml/gcc-bugs/2000-06/msg00646.html
Index: gcse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/gcse.c,v
retrieving revision 1.89
diff -p -r1.89 gcse.c
*** gcse.c 2000/06/19 01:40:32 1.89
--- gcse.c 2000/07/03 08:09:18
*************** static struct obstack gcse_obstack;
*** 302,308 ****
/* Non-zero for each mode that supports (set (reg) (reg)).
This is trivially true for integer and floating point values.
It may or may not be true for condition codes. */
! static char can_copy_p[(int) NUM_MACHINE_MODES];
/* Non-zero if can_copy_p has been initialized. */
static int can_copy_init_p;
--- 302,308 ----
/* Non-zero for each mode that supports (set (reg) (reg)).
This is trivially true for integer and floating point values.
It may or may not be true for condition codes. */
! static char can_copy_p[(int) NUM_MACHINE_MODES][(int) NUM_MACHINE_MODES];
/* Non-zero if can_copy_p has been initialized. */
static int can_copy_init_p;
*************** gcse_main (f, file)
*** 810,838 ****
static void
compute_can_copy ()
{
! int i;
! #ifndef AVOID_CCMODE_COPIES
! rtx reg,insn;
! #endif
char *free_point = (char *) oballoc (1);
! bzero (can_copy_p, NUM_MACHINE_MODES);
start_sequence ();
for (i = 0; i < NUM_MACHINE_MODES; i++)
! if (GET_MODE_CLASS (i) == MODE_CC)
! {
#ifdef AVOID_CCMODE_COPIES
! can_copy_p[i] = 0;
! #else
! reg = gen_rtx_REG ((enum machine_mode) i, LAST_VIRTUAL_REGISTER + 1);
! insn = emit_insn (gen_rtx_SET (VOIDmode, reg, reg));
! if (recog (PATTERN (insn), insn, NULL_PTR) >= 0)
! can_copy_p[i] = 1;
#endif
! }
! else
! can_copy_p[i] = 1;
end_sequence ();
--- 810,838 ----
static void
compute_can_copy ()
{
! int i, j;
! rtx reg1,reg2,insn;
char *free_point = (char *) oballoc (1);
! bzero (can_copy_p, NUM_MACHINE_MODES * NUM_MACHINE_MODES);
start_sequence ();
for (i = 0; i < NUM_MACHINE_MODES; i++)
! for (j = 0; j < NUM_MACHINE_MODES; j++)
#ifdef AVOID_CCMODE_COPIES
! if (GET_MODE_CLASS (i) == MODE_CC)
! can_copy_p[i][j] = 0;
! else
#endif
! {
! reg1 = gen_rtx_REG ((enum machine_mode) i,
! LAST_VIRTUAL_REGISTER + 1);
! reg2 = gen_rtx_REG ((enum machine_mode) j,
! LAST_VIRTUAL_REGISTER + 2);
! insn = emit_insn (gen_rtx_SET (VOIDmode, reg1, reg2));
! if (recog (PATTERN (insn), insn, NULL_PTR) >= 0)
! can_copy_p[i][j] = 1;
! }
end_sequence ();
*************** hash_scan_set (pat, insn, set_p)
*** 1875,1881 ****
if (! set_p
&& regno >= FIRST_PSEUDO_REGISTER
/* Don't GCSE something if we can't do a reg/reg copy. */
! && can_copy_p [GET_MODE (dest)]
/* Is SET_SRC something we want to gcse? */
&& want_to_gcse_p (src))
{
--- 1875,1881 ----
if (! set_p
&& regno >= FIRST_PSEUDO_REGISTER
/* Don't GCSE something if we can't do a reg/reg copy. */
! && can_copy_p [GET_MODE (dest)][GET_MODE (src)]
/* Is SET_SRC something we want to gcse? */
&& want_to_gcse_p (src))
{
*************** hash_scan_set (pat, insn, set_p)
*** 1894,1900 ****
&& regno >= FIRST_PSEUDO_REGISTER
&& ((GET_CODE (src) == REG
&& REGNO (src) >= FIRST_PSEUDO_REGISTER
! && can_copy_p [GET_MODE (dest)])
|| GET_CODE (src) == CONST_INT
|| GET_CODE (src) == SYMBOL_REF
|| GET_CODE (src) == CONST_DOUBLE)
--- 1894,1900 ----
&& regno >= FIRST_PSEUDO_REGISTER
&& ((GET_CODE (src) == REG
&& REGNO (src) >= FIRST_PSEUDO_REGISTER
! && can_copy_p [GET_MODE (dest)][GET_MODE (src)])
|| GET_CODE (src) == CONST_INT
|| GET_CODE (src) == SYMBOL_REF
|| GET_CODE (src) == CONST_DOUBLE)