On 7/27/2021 2:50 AM, Bin.Cheng wrote:
On Mon, Jul 26, 2021 at 11:07 PM Jeff Law <jeffreya...@gmail.com> 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
<gcc-patches@gcc.gnu.org> 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?
Yes. Go ahead and commit it.
Thanks for your patience,
Jeff
