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
