On Tue, Jan 27, 2015 at 11:49:55AM +0800, Hale Wang wrote:
Hi Hale,
> > > diff --git a/gcc/testsuite/gcc.target/arm/pr46164.c
> > > b/gcc/testsuite/gcc.target/arm/pr46164.c
> > > new file mode 100644
> > > index 0000000..ad3b7cb
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/arm/pr46164.c
> > > @@ -0,0 +1,26 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-mcpu=cortex-m3 -mthumb -O1" } */
> >
> > Just "-O1" reproduces the problem here, FWIW.
>
> You are correct. Just "-O1" reproduces this problem.
> However it's a combine bug which is related to the combing user specified
> register into inline-asm.
Yes, it is. But the registers the testcase uses exist on any ARM version
there is as far as I know, so not specifying specific model and ABI should
give wider test coverage (if anyone actually builds and/or tests more than
the default, of course :-) )
> > Could you try this patch please?
>
> Your patch rejected the combine 98+43, that's correct.
Excellent, thanks for testing.
> However, Jakub
> pointed out that preventing that to be combined would be a serious
> regression on code quality.
I know; I needed to think of some good way to detect register variables
(they aren't marked specially in RTL). I think I found one, for combine
that is; if we need to detect it in other passes too, we probably need
to put another flag on it, or something.
> Andrew Pinski suggested: can_combine_p would reject combing into an
> inline-asm to prevent this issue. And I have updated the patch. What do you
> think about this change?
That will regress combining anything else into an asm. It will disallow
combining asms _at all_, if we really wanted that we should simply not build
LOG_LINKS for them. But it hurts optimisation (for simple "r" constraints
it is not a real problem, RA should take care of it, but for anything else
it is).
Updated patch below. A user variable that is also a hard register can only
happen in a few cases: 1) a register variable, the case we are after; 2) an
argument for the current function that was propagated into a user variable
(something combine should not do at all, it hinders good register allocation,
but it does anyway on most targets).
Do you want to take this or shall I? This is not a regression, so it
probably should wait for stage1 :-(
Segher
diff --git a/gcc/combine.c b/gcc/combine.c
index 58de157..9cba594 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1928,6 +1928,10 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn
*pred ATTRIBUTE_UNUSED,
set = expand_field_assignment (set);
src = SET_SRC (set), dest = SET_DEST (set);
+ /* Don't eliminate a register variable. */
+ if (REG_P (dest) && REG_USERVAR_P (dest) && HARD_REGISTER_P (dest))
+ return 0;
+
/* Don't eliminate a store in the stack pointer. */
if (dest == stack_pointer_rtx
/* Don't combine with an insn that sets a register to itself if it has
