Re: [PATCH] Proto-patch to make insn-attrtab.c much smaller (was Re: [PATCH] Remove define_function_unit)

[Steven Bosscher <stevenb at suse dot de>]

On Tuesday 20 July 2004 12:17, Steven Bosscher wrote:
> On Tuesday 20 July 2004 10:22, Paolo Bonzini wrote:
> > > I would say it's definitely worth finding out whether that time and
> > > space buys us anything.
> >
> > If anybody wants to experiment, here's a patch.  It does not remove
> > *all* optimizations, only those based in optimize_attrs (250 lines of
> > code), but it's enough for...
> >
> > wc insn-attrtab.c.*
> >    50293  245411 2026436 insn-attrtab.c.opt
> >                   795200 insn-attrtab.o.opt
> >    13140   71926  563531 insn-attrtab.c.noopt
> >                   228328 insn-attrtab.o.noopt
>
> It's also enough for:
> ../../mainline/gcc/crtstuff.c: In function `__do_global_dtors_aux':
> ../../mainline/gcc/crtstuff.c:288: internal compiler error:
> in ix86_attr_length_immediate_default, at config/i386/i386.c:12134

The problem here seems to be that a SYMBOL_REF is a RTX_CONST_OBJ.

The insn that triggers this is:
(gdb) p debug_rtx(insn)
(call_insn 28 27 93 (call (mem:QI (symbol_ref/i:SI ("__cxa_finalize") [flags 
0x41] <function_decl 0x40417984 __cxa_finalize>) [0 S1 A8])
        (const_int 4 [0x4])) 363 {*call_0} (insn_list:REG_DEP_ANTI 27 
(insn_list:REG_DEP_ANTI 26 (nil)))
    (nil)
    (expr_list (use (reg:SI 3 bx))
        (nil)))
$10 = void
(gdb) p debug_rtx (recog_data.operand[0])
(symbol_ref/i:SI ("__cxa_finalize") [flags 0x41] <function_decl 0x40417984 
__cxa_finalize>)
$11 = void
(gdb) p debug_rtx (recog_data.operand[1])
(const_int 4 [0x4])
$12 = void
(gdb)

So CONSTANT_P returns true for both operand[0] and operand[1], and
len is already set to nonzero for operand[1] which is a CONST_INT.
Then you go do the body of the loop for the SYMBOL_REF and you die:

  for (i = recog_data.n_operands - 1; i >= 0; --i)
    if (CONSTANT_P (recog_data.operand[i]))
      {
        if (len)  /* Already set here for operand[1]  */
          abort ();

I don't understand why SYMBOL_REF (and LABEL_REF and HIGH) would be in
the RTX_CONST_OBJ class instead of just RTX_OBJ.  Is that a bug?  Or
should the above loop just be

  for (i = recog_data.n_operands - 1; i >= 0; --i)
    if (CONSTANT_P (recog_data.operand[i]))
      {
	if (GET_CODE (recog_data.operand[i]) == LABEL_REF
	    || GET_CODE (recog_data.operand[i]) == SYMBOL_REF
	    || GET_CODE (recog_data.operand[i]) == HIGH)
	  continue;
        if (len)  /* Already set here for operand[1]  */
          abort ();

instead because those three are not immediates?  In that case I wonder
if there are other places where this check would be necessary.

Or something else entirely?

Gr.
Steven