* [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264]
@ 2020-03-20 10:04 Jakub Jelinek
2020-03-20 10:17 ` Richard Biener
2020-03-25 22:33 ` Jeff Law
0 siblings, 2 replies; 3+ messages in thread
From: Jakub Jelinek @ 2020-03-20 10:04 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
With this simple patch, on i686-linux and x86_64-linux with -m32 (no change
for -m64), the find_base_term visited_vals.length () > 100 find_base_term
statistics changed (fbt is before this patch, fbt2 with this patch):
for k in '' '1$'; do for i in 32 64; do for j in fbt fbt2; do \
echo -n "$j $i $k "; LC_ALL=C grep ^$i.*"$k" $j | wc -l; done; done; done
fbt 32 5313355
fbt2 32 4229854
fbt 64 217523
fbt2 64 217523
fbt 32 1$ 1296
fbt2 32 1$ 407
fbt 64 1$ 0
fbt2 64 1$ 0
For frame_pointer_needed functions, we need to wait until we see the
fp_setter insn in the prologue at which point we disassociate the fp based
VALUEs from sp based VALUEs, but for !frame_pointer_needed functions,
we IMHO don't need to wait anything. For ACCUMULATE_OUTGOING_ARGS it isn't
IMHO worth doing anything, as there is a single sp adjustment and so there
is no risk of many thousands deep VALUE chains, but for
!ACCUMULATE_OUTGOING_ARGS the sp keeps changing constantly.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Perhaps even better would be if we'd be able already in cselib somehow
figure out these sp based VALUEs and be able to constantly reuse them and in
loc list for them remember (apart from the registers etc. that hold the
VALUE ATM) have just (plus:P (value:P sp0) (const_int NNN)) where sp0 would
be a VALUE of stack pointer at the start of !frame_pointer_needed functions
and for frame_pointer_needed functions say sp VALUE at the point of
fp_setter insn. Then if we have
sp -= 4
sp -= 4
sp -= 4
sp += 8
sp -= 4
sp += 8
sp -= 4
sp -= 4
sp += 8
we wouldn't need 10 sp based VALUEs, but only one for orig sp, one for
orig_sp-4, another one for orig_sp-8, another one for orig_sp-12 and that's
it.
2020-03-19 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/92264
* var-tracking.c (add_stores): Call cselib_set_value_sp_based even
for sp based values in !frame_pointer_needed
&& !ACCUMULATE_OUTGOING_ARGS functions.
--- gcc/var-tracking.c.jj 2020-01-12 11:54:38.532381439 +0100
+++ gcc/var-tracking.c 2020-03-19 15:49:19.457340470 +0100
@@ -6112,7 +6112,8 @@ add_stores (rtx loc, const_rtx expr, voi
}
if (loc == stack_pointer_rtx
- && maybe_ne (hard_frame_pointer_adjustment, -1)
+ && (maybe_ne (hard_frame_pointer_adjustment, -1)
+ || (!frame_pointer_needed && !ACCUMULATE_OUTGOING_ARGS))
&& preserve)
cselib_set_value_sp_based (v);
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264]
2020-03-20 10:04 [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264] Jakub Jelinek
@ 2020-03-20 10:17 ` Richard Biener
2020-03-25 22:33 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2020-03-20 10:17 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, 20 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> With this simple patch, on i686-linux and x86_64-linux with -m32 (no change
> for -m64), the find_base_term visited_vals.length () > 100 find_base_term
> statistics changed (fbt is before this patch, fbt2 with this patch):
> for k in '' '1$'; do for i in 32 64; do for j in fbt fbt2; do \
> echo -n "$j $i $k "; LC_ALL=C grep ^$i.*"$k" $j | wc -l; done; done; done
> fbt 32 5313355
> fbt2 32 4229854
> fbt 64 217523
> fbt2 64 217523
> fbt 32 1$ 1296
> fbt2 32 1$ 407
> fbt 64 1$ 0
> fbt2 64 1$ 0
> For frame_pointer_needed functions, we need to wait until we see the
> fp_setter insn in the prologue at which point we disassociate the fp based
> VALUEs from sp based VALUEs, but for !frame_pointer_needed functions,
> we IMHO don't need to wait anything. For ACCUMULATE_OUTGOING_ARGS it isn't
> IMHO worth doing anything, as there is a single sp adjustment and so there
> is no risk of many thousands deep VALUE chains, but for
> !ACCUMULATE_OUTGOING_ARGS the sp keeps changing constantly.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Perhaps even better would be if we'd be able already in cselib somehow
> figure out these sp based VALUEs and be able to constantly reuse them and in
> loc list for them remember (apart from the registers etc. that hold the
> VALUE ATM) have just (plus:P (value:P sp0) (const_int NNN)) where sp0 would
> be a VALUE of stack pointer at the start of !frame_pointer_needed functions
> and for frame_pointer_needed functions say sp VALUE at the point of
> fp_setter insn. Then if we have
> sp -= 4
> sp -= 4
> sp -= 4
> sp += 8
> sp -= 4
> sp += 8
> sp -= 4
> sp -= 4
> sp += 8
> we wouldn't need 10 sp based VALUEs, but only one for orig sp, one for
> orig_sp-4, another one for orig_sp-8, another one for orig_sp-12 and that's
> it.
That would indeed be great - and I'd expect value-numbering to do such,
why doesn't it work like this in cselib?
I'm not knowledgable enough to review the actual patch, in particular
whether we can rely on frame_pointer_needed or ACCUMULATE_OUTGOING_ARGS
to actually reflect what the target does.
> 2020-03-19 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/92264
> * var-tracking.c (add_stores): Call cselib_set_value_sp_based even
> for sp based values in !frame_pointer_needed
> && !ACCUMULATE_OUTGOING_ARGS functions.
>
> --- gcc/var-tracking.c.jj 2020-01-12 11:54:38.532381439 +0100
> +++ gcc/var-tracking.c 2020-03-19 15:49:19.457340470 +0100
> @@ -6112,7 +6112,8 @@ add_stores (rtx loc, const_rtx expr, voi
> }
>
> if (loc == stack_pointer_rtx
> - && maybe_ne (hard_frame_pointer_adjustment, -1)
> + && (maybe_ne (hard_frame_pointer_adjustment, -1)
> + || (!frame_pointer_needed && !ACCUMULATE_OUTGOING_ARGS))
> && preserve)
> cselib_set_value_sp_based (v);
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264]
2020-03-20 10:04 [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264] Jakub Jelinek
2020-03-20 10:17 ` Richard Biener
@ 2020-03-25 22:33 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Law @ 2020-03-25 22:33 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: gcc-patches
On Fri, 2020-03-20 at 11:04 +0100, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> With this simple patch, on i686-linux and x86_64-linux with -m32 (no change
> for -m64), the find_base_term visited_vals.length () > 100 find_base_term
> statistics changed (fbt is before this patch, fbt2 with this patch):
> for k in '' '1$'; do for i in 32 64; do for j in fbt fbt2; do \
> echo -n "$j $i $k "; LC_ALL=C grep ^$i.*"$k" $j | wc -l; done; done; done
> fbt 32 5313355
> fbt2 32 4229854
> fbt 64 217523
> fbt2 64 217523
> fbt 32 1$ 1296
> fbt2 32 1$ 407
> fbt 64 1$ 0
> fbt2 64 1$ 0
> For frame_pointer_needed functions, we need to wait until we see the
> fp_setter insn in the prologue at which point we disassociate the fp based
> VALUEs from sp based VALUEs, but for !frame_pointer_needed functions,
> we IMHO don't need to wait anything. For ACCUMULATE_OUTGOING_ARGS it isn't
> IMHO worth doing anything, as there is a single sp adjustment and so there
> is no risk of many thousands deep VALUE chains, but for
> !ACCUMULATE_OUTGOING_ARGS the sp keeps changing constantly.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Perhaps even better would be if we'd be able already in cselib somehow
> figure out these sp based VALUEs and be able to constantly reuse them and in
> loc list for them remember (apart from the registers etc. that hold the
> VALUE ATM) have just (plus:P (value:P sp0) (const_int NNN)) where sp0 would
> be a VALUE of stack pointer at the start of !frame_pointer_needed functions
> and for frame_pointer_needed functions say sp VALUE at the point of
> fp_setter insn. Then if we have
> sp -= 4
> sp -= 4
> sp -= 4
> sp += 8
> sp -= 4
> sp += 8
> sp -= 4
> sp -= 4
> sp += 8
> we wouldn't need 10 sp based VALUEs, but only one for orig sp, one for
> orig_sp-4, another one for orig_sp-8, another one for orig_sp-12 and that's
> it.
>
> 2020-03-19 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/92264
> * var-tracking.c (add_stores): Call cselib_set_value_sp_based even
> for sp based values in !frame_pointer_needed
> && !ACCUMULATE_OUTGOING_ARGS functions.
OK.
jeff
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-25 22:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 10:04 [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264] Jakub Jelinek
2020-03-20 10:17 ` Richard Biener
2020-03-25 22:33 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).