Message ID | 87a96ryrsf.fsf@googlemail.com |
---|---|
State | New |
Headers | show |
On 08/26/14 13:28, Richard Sandiford wrote: > [Jeff, sorry for the duplicate, sent the original from an account that > adds disclaimers.] No worries. Given the 3000+ messages that were waiting for me when I got back from PTO, what's another duplicate here and there :-) [ Big snip. ] >> invalid and cope with it if we ever find one. Perhaps a bit of >> ENABLE_CHECKING to detect if we ever create such a note? > > I suppose an assert means that it'd be up to each piece of code that > creates a note to check whether the equivalent value has autoinc addresses. > How about just dropping those REG_EQUAL and REG_EQUIV notes instead, > like we already do for ASM_OPERANDS? I can live with that. > > Here I've extended it to all notes with side effects. The additional > cases are: > > case CLOBBER: > /* Reject CLOBBER with a non-VOID mode. These are made by combine.c > when some combination can't be done. If we see one, don't think > that we can simplify the expression. */ > return (GET_MODE (x) != VOIDmode); > > [...snip autoincs...] > case CALL: > case UNSPEC_VOLATILE: > return 1; > > case MEM: > case ASM_INPUT: > case ASM_OPERANDS: > if (MEM_VOLATILE_P (x)) > return 1; > > The combine clobbers shouldn't make their way into a note anyway, > since it represents a failed optimisation. ASM_INPUT is a top-level > rtx rather than a SET_SRC, so isn't important. Checking for volatile > ASM_OPERANDS is just a subset of the current: > > /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes. > It serves no useful purpose and breaks eliminate_regs. */ > if (GET_CODE (datum) == ASM_OPERANDS) > return NULL_RTX; > > So the remaining cases are CALL, UNSPEC_VOLATILE and volatile MEMs. > I think UNSPEC_VOLATILE and volatile MEMs really do fall into the same > category as auto-inc/dec. Agreed. Const CALLs should be OK in practice, > but I'm not sure why they'd ever need to be treated as having > side effects. They probably don't, but I think that's independent of your changes. There may be subtle things that depend on the const calls having "side effects" -- for example, they might be considered as clobbering argument saveback aeras on the stack and other such nonsense. Other CALLs are more dangerous. In practice the only > interesting notes for calls are (a) that the call address is equal > to some other rtx, which is recorded in the insn that sets the > address register rather than on the call itself and (b) that the result > of the call is equivalent to some non-CALL rtx (e.g. after a libcall). Clearly dropping these is safe as well. And in the case of a libcall, the note shouldn't look like a call, the note should be the "obvious" RTL form that the libcall is implementing. ie instead of a call to mulsi the note looks like (mult (a) (b)) > > Tested on x86_64-linux-gnu. OK to install? > > Thanks, > Richard > > > gcc/ > * emit-rtl.c (set_unique_reg_note): Discard notes with side effects. OK. Jeff
Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c 2014-08-26 12:09:28.242659157 +0100 +++ gcc/emit-rtl.c 2014-08-26 12:09:50.694400018 +0100 @@ -5160,6 +5160,14 @@ set_unique_reg_note (rtx insn, enum reg_ It serves no useful purpose and breaks eliminate_regs. */ if (GET_CODE (datum) == ASM_OPERANDS) return NULL_RTX; + + /* Notes with side effects are dangerous. Even if the side-effect + initially mirrors one in PATTERN (INSN), later optimizations + might alter the way that the final register value is calculated + and so move or alter the side-effect in some way. The note would + then no longer be a valid substitution for SET_SRC. */ + if (side_effects_p (datum)) + return NULL_RTX; break; default: