> So, the code has this structure > > if (looks safe) > emit in existing order > else if (reverse order looks safe) > emit in reversed order > else > undo_all > > > In this specific case, the existing order is never going to look safe > because set1 uses (sp) as an input argument and use_crosses_set_p is > very conservative when the value is the stack pointer on a PUSH_ROUNDING > machine (such as the m68k)
What a bummer, it seems all things are aligned so as to block the optimization here... It is a bit of a shame, as this is a powerful combination that would eliminate a branch if it succeeded. The combination of the 3 insns yields the (set (pc) (pc)) pattern and it's only because added_sets_1 is true that the annoying PARALLEL is created. > So we could put the verification code that both I3 and I2 are INSNs in > the else if (reverse order looks safe) clause. That would mean for > this testcase, we ultimately undo_all. But I consider that reasonable > given the only reason this instance bled into RTL land was -O1 instead > of -O2 compilation. Although it's probably time to concede defeat in this particular case, I don't think we should use the NONJUMP_INSN_P big hammer because we want to eliminate branches if possible. So I'd just add the minimal test to the two existing conditions so as to prevent the invalid RTL from being created, e.g. && (!JUMP_P (i3) || SET_DEST (set[01]) == pc_rtx) -- Eric Botcazou
