On Mon, Jul 26, 2021 at 11:07 PM Jeff Law <[email protected]> wrote:
>
>
>
> On 7/25/2021 7:47 PM, Bin.Cheng wrote:
> > On Sat, Jul 24, 2021 at 12:30 AM Jeff Law via Gcc-patches
> > <[email protected]> wrote:
> >>
> >>
> >> On 7/14/2021 3:14 AM, bin.cheng via Gcc-patches wrote:
> >>> Hi,
> >>> I ran into a wrong code bug in code with deep template instantiation when
> >>> working on sdx::simd.
> >>> The root cause as described in commit summary is we skip prologue insns
> >>> in init_alias_analysis.
> >>> This simple patch fixes the issue, however, it's hard to reduce a case
> >>> because of heavy use of
> >>> templates.
> >>> Bootstrap and test on x86_64, is it OK?
> >> It's a clear correctness improvement, but what's unclear to me is why
> >> we'd want to skip them in the epilogue either.
> > I can only guess, there is nothing to initialize epilogue for because
> > no code follows.
> Yea, but couldn't the lack of analysis of the epilogue lead to a pass
> mis-optimizing code within the epilogue itself? It's not terribly
> likely, but it just seems wrong to skip the epilogue like this.
> Remember, the aliasing bits are just an analysis phase to find the
> aliasing relationships that exist and we don't necessarily know how that
> data is going to be used. It may in fact be safe now, but may not be
> safe in the future if someone added a late RTL pass that used the
> aliasing info in a new way.
>
> The more I think about it, the more I think we should remove remove this
> hunk of code completely. There is some chance for fallout, but I think
> it's unlikely.
Hi Jeff,
Thanks for the suggestion, here is the simple patch removing all of it.
diff --git a/gcc/alias.c b/gcc/alias.c
index 69e1eb89ac6..099acabca6b 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -3406,14 +3406,6 @@ init_alias_analysis (void)
rpo = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
rpo_cnt = pre_and_rev_post_order_compute (NULL, rpo, false);
- /* The prologue/epilogue insns are not threaded onto the
- insn chain until after reload has completed. Thus,
- there is no sense wasting time checking if INSN is in
- the prologue/epilogue until after reload has completed. */
- bool could_be_prologue_epilogue = ((targetm.have_prologue ()
- || targetm.have_epilogue ())
- && reload_completed);
-
pass = 0;
do
{
@@ -3459,10 +3451,6 @@ init_alias_analysis (void)
{
rtx note, set;
- if (could_be_prologue_epilogue
- && prologue_epilogue_contains (insn))
- continue;
-
/* If this insn has a noalias note, process it, Otherwise,
scan for sets. A simple set will have no side effects
which could change the base value of any other
register. */
No fallouts in bootstrap/test on x86_64. Is it OK?
Thanks,
bin
>
> Jeff
>
