Hi Olivier, On Thu, Apr 05, 2012 at 12:36:01PM +0200, Olivier Hainque wrote: > On Apr 4, 2012, at 03:22 , Alan Modra wrote: > > I'll see whether my approach fixes pr30282 for gcc-4.4 which has known > > deficiencies in alias analysis. Olivier, would you be kind enough to > > backport and test against other versions of gcc that needed your > > bigger hammer?
Well it turns out that gcc-4.4 still gets this testcase from pr30282 wrong. int find_num(int i) { int arr[5] = {0, 1, 2, 3, 4}; return arr[i]; } The read from arr[i] is scheduled past the frame teardown. [snip] > There are lots of places in the emit_epilogue code that assign > frame_reg_rtx with different possible values, (hard_fp, sp, r11) r12 too > before we first get to points where we emit ties. There are also > multiple places where we do emit ties. > > It is not at all obvious to me that the all places where we emit ties > have an accurate perception of what frame_reg's were possibly used before. > > IOW, I am not 100% convinced that we cannot have a bad case where > we emit a tie that misses a reg reference. The various paths are all > controlled by intricate conditions, so getting that 100% conviction > (at least on paper) isn't easy to me. > > Is it clearer to you ? I spent quite some time looking at it. ;) Have you spotted an error somewhere, apart from spe_save_area_ptr not being mentioned in the stack tie? I left that one for a later fix since the SPE reg saves use alias set 0 mems (which is another bug). Hmm, I see I accidentally left out an assert from the stack tie patch. This one may make you feel a little better. :) /* Update stack and set back pointer unless this is V.4, for which it was done previously. */ if (!WORLD_SAVE_P (info) && info->push_p && !(DEFAULT_ABI == ABI_V4 || crtl->calls_eh_return)) { rtx copy_reg = NULL; /* If using some other frame reg previously, then we ought to ensure it is mentioned in the stack tie emitted below. */ gcc_checking_assert (REGNO (frame_reg_rtx) == 1 || REGNO (frame_reg_rtx) == 12); > FWIW, I had made an experiment at trying to extract subfunctions, > which might help such reasoning. Set of patches attached. This doesn't > apply to the current mainline, would need to refined, and we probably > couldn't/shouldn't put something like this on the path to the resolution > of our current issue, so this is JIC you are curious. I think this is worthwhile doing, but more important to try to make the logic simpler. For example, this /* If we need to save CR, put it into r12 or r11. */ if (!WORLD_SAVE_P (info) && info->cr_save_p && frame_reg_rtx != frame_ptr_rtx) { rtx set; cr_save_rtx = gen_rtx_REG (SImode, DEFAULT_ABI == ABI_AIX && !saving_GPRs_inline ? 11 : 12); is better written as /* If we need to save CR, put it into r12 or r11. */ cr_save_regno = DEFAULT_ABI == ABI_AIX && !saving_GPRs_inline ? 11 : 12; if (!WORLD_SAVE_P (info) && info->cr_save_p && REGNO (frame_reg_rtx) != cr_save_regno) { rtx set; cr_save_rtx = gen_rtx_REG (SImode, cr_save_regno); This way it's obvious why there is a test of frame_reg_rtx, and that the test is correct. ie. you aren't clobbering frame_reg_rtx with cr. -- Alan Modra Australia Development Lab, IBM