Hi again, On Thu, Feb 06, 2020 at 08:51:06PM +0100, Jakub Jelinek wrote: > On Thu, Feb 06, 2020 at 01:15:25PM -0600, Segher Boessenkool wrote: > > On Thu, Jan 30, 2020 at 05:14:08PM +0100, Jakub Jelinek wrote: > > > Here is what I meant as the alternative, i.e. don't check any predicates, > > > just gen_add3_insn, if that fails, force rs into register and retry. > > > > I don't like gen_add3_insn here *at all*, as I said, but okay, you're > > only fixing existing code. But as long as it is there, this code will > > be a problem child. > > gen_add3_insn is used 25 times elsewhere in the rs6000 backend when not > counting these 2 calls that were just slightly moved around by the patch.
Yes, and almost none of those cases check for errors. If they really cannot error, they can probably just call one of the actual patterns for the machine instructions directly (like we already do in many more cases). > > > And, add REG_FRAME_RELATED_EXPR note always when we haven't emitted a > > > single > > > insn that has rtl exactly matching what we'd add the > > > REG_FRAME_RELATED_EXPR > > > with (in that case, dwarf2cfi.c is able to figure it out by itself, no > > > need > > > to waste compile time memory). > > > > I would say "just always emit that note", but that is what the patch > > does, already :-) > > No, the patch doesn't emit it always, see below. So move the comment *before* "if (add_note)" then? :-) (I don't think it would be terrible to do it actually always either, fwiw, but this is fine). > 2020-02-06 Jakub Jelinek <ja...@redhat.com> > > PR target/93122 > * config/rs6000/rs6000-logue.c > (rs6000_emit_probe_stack_range_stack_clash): Always use gen_add3_insn, > if it fails, move rs into end_addr and retry. Add > REG_FRAME_RELATED_EXPR note whenever it returns more than one insn or > the insn pattern doesn't describe well what exactly happens to > dwarf2cfi.c. > > * gcc.target/powerpc/pr93122.c: New test. Okay for trunk (and backports if you want those). Thanks for the patch, and thanks for bearing with me. Segher
