From 2f00786128f190a4ad7a0819a84026cd8be19ad4 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Mon, 30 Sep 2019 18:36:11 +0000 Subject: [PATCH] [x86] Cache result of expensive_function_p between frame layouts ix86_compute_frame_layout sets use_fast_prologue_epilogue if the function isn't more expensive than a certain threshold, where the threshold depends on the number of saved registers. However, the RA is allowed to insert and delete instructions as it goes along, which can change whether this threshold is crossed or not. I hit this with an RA change I'm working on. Rematerialisation was able to remove an instruction and avoid a spill, which happened to bring the size of the function below the threshold. But since nothing legitimately frame-related had changed, there was no need for the RA to lay out the frame again. We then failed the final sanity check in lra_eliminate. 2019-09-30 Richard Sandiford gcc/ * config/i386/i386.h (ix86_frame::expensive_p): New field. (ix86_frame::expensive_count): Likewise. * config/i386/i386.c (ix86_compute_frame_layout): Make the choice of use_fast_prologue_epilogue robust against incidental changes in function size. From-SVN: r276361 --- gcc/ChangeLog | 8 ++++++++ gcc/config/i386/i386.c | 19 ++++++++++++++++--- gcc/config/i386/i386.h | 5 +++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 28296d5..f665a5c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2019-09-30 Richard Sandiford + + * config/i386/i386.h (ix86_frame::expensive_p): New field. + (ix86_frame::expensive_count): Likewise. + * config/i386/i386.c (ix86_compute_frame_layout): Make the choice + of use_fast_prologue_epilogue robust against incidental changes + in function size. + 2019-09-30 Ilya Leoshkevich PR target/77918 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 8af4bc5..083e228 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -5877,7 +5877,14 @@ ix86_compute_frame_layout (void) case function is known to be outside hot spot (this is known with feedback only). Weight the size of function by number of registers to save as it is cheap to use one or two push instructions but very - slow to use many of them. */ + slow to use many of them. + + Calling this hook multiple times with the same frame requirements + must produce the same layout, since the RA might otherwise be + unable to reach a fixed point or might fail its final sanity checks. + This means that once we've assumed that a function does or doesn't + have a particular size, we have to stick to that assumption + regardless of how the function has changed since. */ if (count) count = (count - 1) * FAST_PROLOGUE_INSN_COUNT; if (node->frequency < NODE_FREQUENCY_NORMAL @@ -5885,8 +5892,14 @@ ix86_compute_frame_layout (void) && node->frequency < NODE_FREQUENCY_HOT)) m->use_fast_prologue_epilogue = false; else - m->use_fast_prologue_epilogue - = !expensive_function_p (count); + { + if (count != frame->expensive_count) + { + frame->expensive_count = count; + frame->expensive_p = expensive_function_p (count); + } + m->use_fast_prologue_epilogue = !frame->expensive_p; + } } frame->save_regs_using_mov diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 9fe1f45..4e37336 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2643,6 +2643,11 @@ struct GTY(()) ix86_frame /* When save_regs_using_mov is set, emit prologue using move instead of push instructions. */ bool save_regs_using_mov; + + /* Assume without checking that: + EXPENSIVE_P = expensive_function_p (EXPENSIVE_COUNT). */ + bool expensive_p; + int expensive_count; }; /* Machine specific frame tracking during prologue/epilogue generation. All -- 2.7.4