Segher Boessenkool <seg...@kernel.crashing.org> writes: > On Thu, Feb 12, 2015 at 03:54:21PM +0000, Richard Sandiford wrote: >> "Hale Wang" <hale.w...@arm.com> writes: >> > Ping? > > It's not a regression (or is it?), so it is not appropriate for stage4. > > >> >> diff --git a/gcc/combine.c b/gcc/combine.c index 5c763b4..6901ac2 100644 >> >> --- a/gcc/combine.c >> >> +++ b/gcc/combine.c >> >> @@ -1904,6 +1904,12 @@ 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 combine if dest contains a user specified register, because >> > the >> >> + user specified register (same with dest) in i3 would be replaced by >> > the >> >> + src of insn which might be different with the user's expectation. >> >> + */ if (REG_P (dest) && REG_USERVAR_P (dest) && HARD_REGISTER_P >> >> (dest)) >> >> + return 0; >> >> I suppose this is similar to Andrew's comment, but I think the rule >> is that it's invalid to replace a REG_USERVAR_P operand in an inline asm. > > Why not? You probably mean register asm, not all user variables?
Yeah, meant hard REG_USERVAR_P, sorry, as for the patch. >> Outside of an inline asm we make no guarantee about whether something is >> stored in a particular register or not. >> >> So IMO we should be checking whether either INSN or I3 is an asm as well >> as the above. > > [ INSN can never be an asm, that is already refused by can_combine_p. ] > > We do not guarantee things will end up in the specified reg (except for asm), > but will it hurt to leave things in the reg the user said it should be in, > even > if we do not guarantee this behaviour? Whether it does not, making the test unnecessarily wide is at best only going to paper over problems elsewhere. I really think we should test for i3 being an asm. Thanks, Richard
