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] Fix DCE from removing instructions setting hard frame pointer or other artificially used registers (PR middle-end/32758)


On Wed, Aug 29, 2007 at 11:19:25AM -0700, Seongbae Park wrote:
> Can you show me an example of the bad DCE your change prevents ?

Bugzilla has it, but I can repeat it.

void foo (void *x __attribute__((unused)));

__attribute__((noinline))
int bar (unsigned int x)
{
  void *y = __builtin_alloca (x);
  foo (y);
  return 1;
}

with -O2 -fexceptions with the patch generates:

.bar:
.LFB2:
        mflr 0
.LCFI0:
        std 31,-8(1)
.LCFI1:
        addi 3,3,30
        std 0,16(1)
.LCFI2:
        stdu 1,-128(1)
.LCFI3:
        rldicr 3,3,31,32
        rldicr 3,3,33,59
        neg 3,3
        ld 0,0(1)
        mr 31,1			# <----- this one
.LCFI4:				# <----- and this label
        stdux 0,1,3
        addi 3,1,112
        bl .foo
        nop
        ld 1,0(1)
        li 3,1
        ld 0,16(1)
        ld 31,-8(1)
        mtlr 0
        blr
        .long 0
        .byte 0,0,0,1,128,1,0,0
.LFE2:

while without the patch removes the above marked mr 31,1
instruction.  Register 31 is on ppc hard frame pointer, this
function uses alloca and subtracts non-constant amount from
the stack pointer (register 1).  Eventhough the hard frame
pointer isn't visibly used anywhere (except instructions
which save resp. restore it as it is a call saved register),
it is used by the EH, so with -fexceptions it must be live
and have proper value at least on all call instructions
where the stack is decreased by alloca (bl .foo in this case)
and e.g. with -fasynchronous-unwind-tables on every single
instruction when the stack is decreased. 
To make debugging possible it is always needed on all such
instructions, otherwise the debugger can't find out the
size of the stack frame.

> On the face of it, the change looks wrong,
> as the artificial uses for a block aren't meant to be alive everywhere.

Nope, it has to be alive everywhere, at least on the stdux 0,1,3
through ld 1,0(1) (the former decreases stack by reg3 bytes,
the latter loads a value from the stack the former insn saved)
instructions.

> Beside, it's a per-block set, so the patch as it is
> (even if making them live everywhere is what we want to do)
> is fragile - it should really use df_get_artificial_uses(bb_index)
> to initialize a temporary bitmap.

So why df_simulate_fixup_sets uses it directly then?
df_get_artificial_uses(bb_index) contains also other regs
that don't have these properties like hfp, fp, ap
- EH_USES, EH_RETURN_DATA_REGNO.

	Jakub


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