* [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085]
@ 2021-02-16 8:13 Jakub Jelinek
2021-02-23 8:49 ` Patch ping Jakub Jelinek
2021-03-02 23:39 ` [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jeff Law
0 siblings, 2 replies; 62+ messages in thread
From: Jakub Jelinek @ 2021-02-16 8:13 UTC (permalink / raw)
To: Richard Biener, Jan Hubicka; +Cc: gcc-patches
Hi!
fixup_partitions sometimes changes some basic blocks from hot partition to
cold partition, in particular if after unreachable block removal or other
optimizations a hot partition block is dominated by cold partition block(s).
It fixes up the edges and jumps on those edges, but when after reorder
blocks and in rtl (non-cfglayout) mode that is clearly not enough, because
it keeps the block order the same and so we can end up with more than
1 hot/cold section transition in the same function.
So, this patch fixes that up too.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2021-02-15 Jakub Jelinek <jakub@redhat.com>
PR target/99085
* cfgrtl.c (fixup_partitions): When changing some bbs from hot to cold
partitions, if in non-layout mode after reorder_blocks also move
affected blocks to ensure a single partition transition.
* gcc.dg/graphite/pr99085.c: New test.
--- gcc/cfgrtl.c.jj 2021-02-10 19:27:33.000000000 +0100
+++ gcc/cfgrtl.c 2021-02-15 16:47:41.625798717 +0100
@@ -2414,8 +2414,6 @@ find_partition_fixes (bool flag_only)
void
fixup_partitions (void)
{
- basic_block bb;
-
if (!crtl->has_bb_partition)
return;
@@ -2436,10 +2434,61 @@ fixup_partitions (void)
/* Do the partition fixup after all necessary blocks have been converted to
cold, so that we only update the region crossings the minimum number of
places, which can require forcing edges to be non fallthru. */
- while (! bbs_to_fix.is_empty ())
+ if (! bbs_to_fix.is_empty ())
{
- bb = bbs_to_fix.pop ();
- fixup_new_cold_bb (bb);
+ do
+ {
+ basic_block bb = bbs_to_fix.pop ();
+ fixup_new_cold_bb (bb);
+ }
+ while (! bbs_to_fix.is_empty ());
+
+ /* Fix up hot cold block grouping if needed. */
+ if (crtl->bb_reorder_complete && current_ir_type () == IR_RTL_CFGRTL)
+ {
+ basic_block bb, first = NULL, second = NULL;
+ int current_partition = BB_UNPARTITIONED;
+
+ FOR_EACH_BB_FN (bb, cfun)
+ {
+ if (current_partition != BB_UNPARTITIONED
+ && BB_PARTITION (bb) != current_partition)
+ {
+ if (first == NULL)
+ first = bb;
+ else if (second == NULL)
+ second = bb;
+ else
+ {
+ /* If we switch partitions for the 3rd, 5th etc. time,
+ move bbs first (inclusive) .. second (exclusive) right
+ before bb. */
+ basic_block prev_first = first->prev_bb;
+ basic_block prev_second = second->prev_bb;
+ basic_block prev_bb = bb->prev_bb;
+ prev_first->next_bb = second;
+ second->prev_bb = prev_first;
+ prev_second->next_bb = bb;
+ bb->prev_bb = prev_second;
+ prev_bb->next_bb = first;
+ first->prev_bb = prev_bb;
+ rtx_insn *prev_first_insn = PREV_INSN (BB_HEAD (first));
+ rtx_insn *prev_second_insn
+ = PREV_INSN (BB_HEAD (second));
+ rtx_insn *prev_bb_insn = PREV_INSN (BB_HEAD (bb));
+ SET_NEXT_INSN (prev_first_insn) = BB_HEAD (second);
+ SET_PREV_INSN (BB_HEAD (second)) = prev_first_insn;
+ SET_NEXT_INSN (prev_second_insn) = BB_HEAD (bb);
+ SET_PREV_INSN (BB_HEAD (bb)) = prev_second_insn;
+ SET_NEXT_INSN (prev_bb_insn) = BB_HEAD (first);
+ SET_PREV_INSN (BB_HEAD (first)) = prev_bb_insn;
+ second = NULL;
+ }
+ }
+ current_partition = BB_PARTITION (bb);
+ }
+ gcc_assert (!second);
+ }
}
}
--- gcc/testsuite/gcc.dg/graphite/pr99085.c.jj 2021-02-15 16:52:59.313169888 +0100
+++ gcc/testsuite/gcc.dg/graphite/pr99085.c 2021-02-15 16:51:54.589910609 +0100
@@ -0,0 +1,20 @@
+/* PR target/99085 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fgraphite-identity -fsel-sched-pipelining -fselective-scheduling2" } */
+
+void
+foo (int m, int n, int o, int i)
+{
+ long double a2[m];
+ int c2[n][o];
+ int j, k;
+
+ while (i < m)
+ a2[i++] = 13.132L;
+
+ for (j = 0; j < n; j++)
+ for (k = 0; k < o; k++)
+ c2[j][k] = 1;
+
+ __builtin_abort ();
+}
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping
2021-02-16 8:13 [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jakub Jelinek
@ 2021-02-23 8:49 ` Jakub Jelinek
2021-03-01 13:01 ` Patch ping^2 Jakub Jelinek
2021-03-02 23:39 ` [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jeff Law
1 sibling, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2021-02-23 8:49 UTC (permalink / raw)
To: Richard Biener, Jan Hubicka, Jeff Law, Eric Botcazou; +Cc: gcc-patches
Hi!
I'd like to ping the
https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565350.html
patch, P2 PR99085 ice-on-valid-code fix in fixup_partitions.
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085]
2021-02-16 8:13 [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jakub Jelinek
2021-02-23 8:49 ` Patch ping Jakub Jelinek
@ 2021-03-02 23:39 ` Jeff Law
1 sibling, 0 replies; 62+ messages in thread
From: Jeff Law @ 2021-03-02 23:39 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener, Jan Hubicka; +Cc: gcc-patches
On 2/16/21 1:13 AM, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> fixup_partitions sometimes changes some basic blocks from hot partition to
> cold partition, in particular if after unreachable block removal or other
> optimizations a hot partition block is dominated by cold partition block(s).
> It fixes up the edges and jumps on those edges, but when after reorder
> blocks and in rtl (non-cfglayout) mode that is clearly not enough, because
> it keeps the block order the same and so we can end up with more than
> 1 hot/cold section transition in the same function.
>
> So, this patch fixes that up too.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2021-02-15 Jakub Jelinek <jakub@redhat.com>
>
> PR target/99085
> * cfgrtl.c (fixup_partitions): When changing some bbs from hot to cold
> partitions, if in non-layout mode after reorder_blocks also move
> affected blocks to ensure a single partition transition.
>
> * gcc.dg/graphite/pr99085.c: New test.
OK
jeff
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2024-02-26 9:33 Jakub Jelinek
2024-02-26 9:38 ` Uros Bizjak
2024-02-26 16:26 ` Jan Hubicka
0 siblings, 2 replies; 62+ messages in thread
From: Jakub Jelinek @ 2024-02-26 9:33 UTC (permalink / raw)
To: Richard Biener, Jeff Law, Uros Bizjak, Jan Hubicka; +Cc: gcc-patches
Hi!
I'd like to ping 2 patches:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/644580.html
PR113617 P1 - Handle private COMDAT function symbol reference in readonly data section
More details in the
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644121
and
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644486
threads.
and
https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645326.html
i386: Enable _BitInt support on ia32
all the FAILs mentioned in that mail have been fixed by now.
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2024-02-26 9:33 Patch ping^2 Jakub Jelinek
@ 2024-02-26 9:38 ` Uros Bizjak
2024-02-26 16:26 ` Jan Hubicka
1 sibling, 0 replies; 62+ messages in thread
From: Uros Bizjak @ 2024-02-26 9:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, Jeff Law, Jan Hubicka, gcc-patches
On Mon, Feb 26, 2024 at 10:33 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> I'd like to ping 2 patches:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645326.html
> i386: Enable _BitInt support on ia32
>
> all the FAILs mentioned in that mail have been fixed by now.
LGTM, based on HJ's advice.
Uros.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2024-02-26 9:33 Patch ping^2 Jakub Jelinek
2024-02-26 9:38 ` Uros Bizjak
@ 2024-02-26 16:26 ` Jan Hubicka
1 sibling, 0 replies; 62+ messages in thread
From: Jan Hubicka @ 2024-02-26 16:26 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, Jeff Law, Uros Bizjak, gcc-patches
> Hi!
>
> I'd like to ping 2 patches:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2024-January/644580.html
> PR113617 P1 - Handle private COMDAT function symbol reference in readonly data section
>
> More details in the
> https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644121
> and
> https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644486
> threads.
I went through the thread - it looks like an issue that was latent for
quite some time. I think it belongs to cgraph/symtab maintenance, so I
think I can OK this patch.
Honza
>
> and
>
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645326.html
> i386: Enable _BitInt support on ia32
>
> all the FAILs mentioned in that mail have been fixed by now.
>
> Thanks
>
> Jakub
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping
@ 2021-03-19 9:57 Jakub Jelinek
2021-03-29 15:19 ` Patch ping^2 Jakub Jelinek
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2021-03-19 9:57 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
I'd like to ping two patches:
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566324.html
PR99388 dwarf2out half float fix
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566669.html
PR99490 dwarf2out -gsplit-dwarf ranges fixes
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping
@ 2020-03-10 12:28 Jakub Jelinek
2020-03-17 12:10 ` Patch ping^2 Jakub Jelinek
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2020-03-10 12:28 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Hi!
I'd like to ping the
https://gcc.gnu.org/legacy-ml/gcc-patches/2020-03/msg00154.html
P1 PR94015
patch, with the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94015#c5
testcase instead of the one sent in the patch, so that it FAILs without the
fix on more targets and more reliably.
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
2020-03-10 12:28 Patch ping Jakub Jelinek
@ 2020-03-17 12:10 ` Jakub Jelinek
2020-03-17 12:23 ` Richard Biener
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2020-03-17 12:10 UTC (permalink / raw)
To: Jeff Law, Richard Biener; +Cc: gcc-patches
Hi!
I'd like to ping this patch again:
On Tue, Mar 10, 2020 at 01:28:19PM +0100, Jakub Jelinek wrote:
> I'd like to ping the
> https://gcc.gnu.org/legacy-ml/gcc-patches/2020-03/msg00154.html
> P1 PR94015
> patch, with the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94015#c5
> testcase instead of the one sent in the patch, so that it FAILs without the
> fix on more targets and more reliably.
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2020-03-17 12:10 ` Patch ping^2 Jakub Jelinek
@ 2020-03-17 12:23 ` Richard Biener
0 siblings, 0 replies; 62+ messages in thread
From: Richard Biener @ 2020-03-17 12:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, gcc-patches
On Tue, 17 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> I'd like to ping this patch again:
OK.
Thanks,
Richard.
> On Tue, Mar 10, 2020 at 01:28:19PM +0100, Jakub Jelinek wrote:
> > I'd like to ping the
> > https://gcc.gnu.org/legacy-ml/gcc-patches/2020-03/msg00154.html
> > P1 PR94015
> > patch, with the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94015#c5
> > testcase instead of the one sent in the patch, so that it FAILs without the
> > fix on more targets and more reliably.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2018-05-07 8:10 Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2018-05-07 8:10 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: gcc-patches
Hi!
I'd like to ping following patches:
- PR85480 improve AVX512 128-bit insertion into 512-bit zero vector
http://gcc.gnu.org/ml/gcc-patches/2018-04/msg01058.html
- PR85572 implement absv2di2 and absv4di2 expanders for pre-avx512vl
http://gcc.gnu.org/ml/gcc-patches/2018-04/msg01341.html
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2018-04-05 8:35 Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2018-04-05 8:35 UTC (permalink / raw)
To: Richard Biener, Jeff Law, Alexandre Oliva, Uros Bizjak; +Cc: gcc-patches
Hi!
I'd like to ping the
http://gcc.gnu.org/ml/gcc-patches/2018-03/msg01244.html
- PR83157 - improve debug info for x86 setcc peepholes
patch. Thanks.
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2017-12-15 9:09 Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2017-12-15 9:09 UTC (permalink / raw)
To: Jason Merrill, Nathan Sidwell, Richard Biener, Jeff Law; +Cc: gcc-patches
Hi!
I'd like to ping a bunch of patches:
http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02521.html
PR c++/83205 - diagnose invalid std::tuple_size<X>::value for structured
bindings; the follow-up with plural spelling is approved
already
http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02629.html
PR c++/83217 - handle references to non-complete type in structured
bindings
http://gcc.gnu.org/ml/gcc-patches/2017-12/msg00184.html
PR c++/81197 - fix ICE with structured bindings and extend_ref_init_temps
http://gcc.gnu.org/ml/gcc-patches/2017-12/msg00391.html
PR c++/83300 - RFC fix for C++ late attribute handling
http://gcc.gnu.org/ml/gcc-patches/2017-12/msg00507.html
PR c++/80135, c++/81922 - fix ICEs with flexible array member initialization
in nested contexts
http://gcc.gnu.org/ml/gcc-patches/2017-12/msg00395.html
PR sanitizer/81281 - further improvements for match.pd
(T)(P + A) - (T)(P + B) -> (T)A - (T)B optimization
http://gcc.gnu.org/ml/gcc-patches/2017-12/msg00346.html
PR target/41455, target/82935 - use tail calls in aggregate copy or clear
expanded as memcpy or memset call if possible
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping
@ 2017-11-06 16:22 Jakub Jelinek
2017-11-14 17:07 ` Patch ping^2 Jakub Jelinek
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2017-11-06 16:22 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
I'd like to ping the:
http://gcc.gnu.org/ml/gcc-patches/2017-10/msg01895.html
PR debug/82718
Fix DWARF5 .debug_loclist handling with hot/cold partitioning
patch. Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
2017-11-06 16:22 Patch ping Jakub Jelinek
@ 2017-11-14 17:07 ` Jakub Jelinek
2017-11-16 3:07 ` Jim Wilson
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2017-11-14 17:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Mon, Nov 06, 2017 at 05:22:36PM +0100, Jakub Jelinek wrote:
> I'd like to ping the:
>
> http://gcc.gnu.org/ml/gcc-patches/2017-10/msg01895.html
> PR debug/82718
> Fix DWARF5 .debug_loclist handling with hot/cold partitioning
>
> patch. Thanks
Ping^2.
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-11-14 17:07 ` Patch ping^2 Jakub Jelinek
@ 2017-11-16 3:07 ` Jim Wilson
0 siblings, 0 replies; 62+ messages in thread
From: Jim Wilson @ 2017-11-16 3:07 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill; +Cc: gcc-patches
On 11/14/2017 08:29 AM, Jakub Jelinek wrote:
> On Mon, Nov 06, 2017 at 05:22:36PM +0100, Jakub Jelinek wrote:
>> I'd like to ping the:
>>
>> http://gcc.gnu.org/ml/gcc-patches/2017-10/msg01895.html
>> PR debug/82718
>> Fix DWARF5 .debug_loclist handling with hot/cold partitioning
>>
>> patch. Thanks
>
> Ping^2.
The testcase doesn't fail on mainline anymore. It also doesn't fail
with the 2017-11-05 snapshot. It does fail with the 2017-10-22
snapshot. Perhaps something subtle changed in the optimizer. But at
the moment the testcase isn't doing anything useful. Maybe it can be fixed?
The dwarf2out.c patch looks OK to me though.
Jim
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2017-04-18 14:21 Jakub Jelinek
2017-04-18 17:07 ` Jeff Law
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2017-04-18 14:21 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
I'd like to ping following patch:
PR debug/80263
http://gcc.gnu.org/ml/gcc-patches/2017-04/msg00004.html
avoid emitting sizetype artificial name into debug info
Thanks
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2017-02-13 15:53 Jakub Jelinek
2017-02-13 16:45 ` Nathan Sidwell
2017-02-14 15:07 ` Nathan Sidwell
0 siblings, 2 replies; 62+ messages in thread
From: Jakub Jelinek @ 2017-02-13 15:53 UTC (permalink / raw)
To: Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor; +Cc: gcc-patches
Hi!
I'd like to ping a couple of patches:
- C++ P1 PR79232 - ICEs and wrong-code with COMPOUND_EXPR on lhs of assignment
http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02341.html
- C++ P1 PR79288 - wrong default TLS model for __thread static data members
http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02349.html
- small simplification for gimple-ssa-sprintf.c
http://gcc.gnu.org/ml/gcc-patches/2017-02/msg00331.html
- noipa attribute addition
http://gcc.gnu.org/ml/gcc-patches/2016-12/msg01501.html
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-13 15:53 Jakub Jelinek
@ 2017-02-13 16:45 ` Nathan Sidwell
2017-02-13 16:50 ` Jakub Jelinek
2017-02-14 15:07 ` Nathan Sidwell
1 sibling, 1 reply; 62+ messages in thread
From: Nathan Sidwell @ 2017-02-13 16:45 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor
Cc: gcc-patches
On 02/13/2017 10:46 AM, Jakub Jelinek wrote:
> Hi!
>
> I'd like to ping a couple of patches:
>
> - C++ P1 PR79232 - ICEs and wrong-code with COMPOUND_EXPR on lhs of assignment
> http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02341.html
What puzzles me about (and may be an existing orthogonal issue), is the
checking for TYPE(rhs) == VOID. In the current code it's only
explicitly checked in the lhs == COND_EXPR case, which is strange. Why
isn't it a general constraint?
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-13 16:45 ` Nathan Sidwell
@ 2017-02-13 16:50 ` Jakub Jelinek
2017-02-13 17:37 ` Jakub Jelinek
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2017-02-13 16:50 UTC (permalink / raw)
To: Nathan Sidwell
Cc: Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor, gcc-patches
On Mon, Feb 13, 2017 at 11:41:48AM -0500, Nathan Sidwell wrote:
> On 02/13/2017 10:46 AM, Jakub Jelinek wrote:
> > Hi!
> >
> > I'd like to ping a couple of patches:
> >
> > - C++ P1 PR79232 - ICEs and wrong-code with COMPOUND_EXPR on lhs of assignment
> > http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02341.html
>
> What puzzles me about (and may be an existing orthogonal issue), is the
> checking for TYPE(rhs) == VOID. In the current code it's only explicitly
> checked in the lhs == COND_EXPR case, which is strange. Why isn't it a
> general constraint?
I'll double check; copied this from the COND_EXPR case which had the same.
I believe the reason is that this is (or ought to be) checked later on,
but stabilitize_expr wouldn't work well if it is called with a void
expression.
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-13 16:50 ` Jakub Jelinek
@ 2017-02-13 17:37 ` Jakub Jelinek
2017-02-13 19:36 ` Nathan Sidwell
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2017-02-13 17:37 UTC (permalink / raw)
To: Nathan Sidwell
Cc: Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor, gcc-patches
On Mon, Feb 13, 2017 at 05:44:54PM +0100, Jakub Jelinek wrote:
> On Mon, Feb 13, 2017 at 11:41:48AM -0500, Nathan Sidwell wrote:
> > On 02/13/2017 10:46 AM, Jakub Jelinek wrote:
> > > Hi!
> > >
> > > I'd like to ping a couple of patches:
> > >
> > > - C++ P1 PR79232 - ICEs and wrong-code with COMPOUND_EXPR on lhs of assignment
> > > http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02341.html
> >
> > What puzzles me about (and may be an existing orthogonal issue), is the
> > checking for TYPE(rhs) == VOID. In the current code it's only explicitly
> > checked in the lhs == COND_EXPR case, which is strange. Why isn't it a
> > general constraint?
>
> I'll double check; copied this from the COND_EXPR case which had the same.
> I believe the reason is that this is (or ought to be) checked later on,
> but stabilitize_expr wouldn't work well if it is called with a void
> expression.
Ok, so tried
void bar (int);
int
foo (int x, int *y, int z, int w, int u)
{
y[++z] = bar (++u);
(x++, y[++z]) = bar (++u);
(w ? y[++z] : y[z++]) = bar (++u);
(x++, (w ? y[++z] : y[z++])) = bar (++u);
return x + z + u;
}
The reason for that VOID_TYPE_P stuff in COND_EXPR handling is not
that stabilize_expr doesn't like it, but to avoid warning twice.
If I replace all 3 VOID_TYPE_P tests (with the patch) in
cp_build_modify_expr with 0 && VOID_TYPE_P, the above testcase
errors 6 times instead of 4 times as it should.
For the case when lhs is a pre-inc/decrement or modify_expr, we don't have
this problem, rhs is used just one in cp_convert_for_assignment where this
error is diagnosed normally.
Thus, I'll bootstrap/regtest following simplified version of the patch
instead:
2017-02-13 Jakub Jelinek <jakub@redhat.com>
PR c++/79232
* typeck.c (cp_build_modify_expr): Handle properly COMPOUND_EXPRs
on lhs that have {PRE{DEC,INC}REMENT,MODIFY,MIN,MAX,COND}_EXPR
in the rightmost operand.
* g++.dg/cpp1z/eval-order4.C: New test.
* g++.dg/other/pr79232.C: New test.
--- gcc/cp/typeck.c.jj 2017-01-31 22:36:33.762297835 +0100
+++ gcc/cp/typeck.c 2017-02-13 18:22:06.927396122 +0100
@@ -7571,16 +7571,26 @@ tree
cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
tree rhs, tsubst_flags_t complain)
{
- tree result;
+ tree result = NULL_TREE;
tree newrhs = rhs;
tree lhstype = TREE_TYPE (lhs);
+ tree olhs = lhs;
tree olhstype = lhstype;
bool plain_assign = (modifycode == NOP_EXPR);
+ bool compound_side_effects_p = false;
+ tree preeval = NULL_TREE;
/* Avoid duplicate error messages from operands that had errors. */
if (error_operand_p (lhs) || error_operand_p (rhs))
return error_mark_node;
+ while (TREE_CODE (lhs) == COMPOUND_EXPR)
+ {
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
+ compound_side_effects_p = true;
+ lhs = TREE_OPERAND (lhs, 1);
+ }
+
/* Handle control structure constructs used as "lvalues". Note that we
leave COMPOUND_EXPR on the LHS because it is sequenced after the RHS. */
switch (TREE_CODE (lhs))
@@ -7588,20 +7598,41 @@ cp_build_modify_expr (location_t loc, tr
/* Handle --foo = 5; as these are valid constructs in C++. */
case PREDECREMENT_EXPR:
case PREINCREMENT_EXPR:
+ if (compound_side_effects_p)
+ newrhs = rhs = stabilize_expr (rhs, &preeval);
if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
TREE_OPERAND (lhs, 1));
lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+ maybe_add_compound:
+ /* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5;
+ and looked through the COMPOUND_EXPRs, readd them now around
+ the resulting lhs. */
+ if (TREE_CODE (olhs) == COMPOUND_EXPR)
+ {
+ lhs = build2 (COMPOUND_EXPR, lhstype, TREE_OPERAND (olhs, 0), lhs);
+ tree *ptr = &TREE_OPERAND (lhs, 1);
+ for (olhs = TREE_OPERAND (olhs, 1);
+ TREE_CODE (olhs) == COMPOUND_EXPR;
+ olhs = TREE_OPERAND (olhs, 1))
+ {
+ *ptr = build2 (COMPOUND_EXPR, lhstype,
+ TREE_OPERAND (olhs, 0), *ptr);
+ ptr = &TREE_OPERAND (*ptr, 1);
+ }
+ }
break;
case MODIFY_EXPR:
+ if (compound_side_effects_p)
+ newrhs = rhs = stabilize_expr (rhs, &preeval);
if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
TREE_OPERAND (lhs, 1));
lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
- break;
+ goto maybe_add_compound;
case MIN_EXPR:
case MAX_EXPR:
@@ -7629,7 +7660,6 @@ cp_build_modify_expr (location_t loc, tr
except that the RHS goes through a save-expr
so the code to compute it is only emitted once. */
tree cond;
- tree preeval = NULL_TREE;
if (VOID_TYPE_P (TREE_TYPE (rhs)))
{
@@ -7655,14 +7685,31 @@ cp_build_modify_expr (location_t loc, tr
if (cond == error_mark_node)
return cond;
+ /* If we had (e, (a ? b : c)) = d; or (e, (f, (a ? b : c))) = d;
+ and looked through the COMPOUND_EXPRs, readd them now around
+ the resulting cond before adding the preevaluated rhs. */
+ if (TREE_CODE (olhs) == COMPOUND_EXPR)
+ {
+ cond = build2 (COMPOUND_EXPR, TREE_TYPE (cond),
+ TREE_OPERAND (olhs, 0), cond);
+ tree *ptr = &TREE_OPERAND (cond, 1);
+ for (olhs = TREE_OPERAND (olhs, 1);
+ TREE_CODE (olhs) == COMPOUND_EXPR;
+ olhs = TREE_OPERAND (olhs, 1))
+ {
+ *ptr = build2 (COMPOUND_EXPR, TREE_TYPE (cond),
+ TREE_OPERAND (olhs, 0), *ptr);
+ ptr = &TREE_OPERAND (*ptr, 1);
+ }
+ }
/* Make sure the code to compute the rhs comes out
before the split. */
- if (preeval)
- cond = build2 (COMPOUND_EXPR, TREE_TYPE (lhs), preeval, cond);
- return cond;
+ result = cond;
+ goto ret;
}
default:
+ lhs = olhs;
break;
}
@@ -7678,7 +7725,7 @@ cp_build_modify_expr (location_t loc, tr
rhs = convert (lhstype, rhs);
result = build2 (INIT_EXPR, lhstype, lhs, rhs);
TREE_SIDE_EFFECTS (result) = 1;
- return result;
+ goto ret;
}
else if (! MAYBE_CLASS_TYPE_P (lhstype))
/* Do the default thing. */;
@@ -7691,7 +7738,7 @@ cp_build_modify_expr (location_t loc, tr
release_tree_vector (rhs_vec);
if (result == NULL_TREE)
return error_mark_node;
- return result;
+ goto ret;
}
}
else
@@ -7706,7 +7753,7 @@ cp_build_modify_expr (location_t loc, tr
{
result = objc_maybe_build_modify_expr (lhs, rhs);
if (result)
- return result;
+ goto ret;
}
/* `operator=' is not an inheritable operator. */
@@ -7720,7 +7767,7 @@ cp_build_modify_expr (location_t loc, tr
complain);
if (result == NULL_TREE)
return error_mark_node;
- return result;
+ goto ret;
}
lhstype = olhstype;
}
@@ -7765,7 +7812,7 @@ cp_build_modify_expr (location_t loc, tr
{
result = objc_maybe_build_modify_expr (lhs, newrhs);
if (result)
- return result;
+ goto ret;
}
}
gcc_assert (TREE_CODE (lhstype) != REFERENCE_TYPE);
@@ -7861,9 +7908,10 @@ cp_build_modify_expr (location_t loc, tr
from_array = TREE_CODE (TREE_TYPE (newrhs)) == ARRAY_TYPE
? 1 + (modifycode != INIT_EXPR): 0;
- return build_vec_init (lhs, NULL_TREE, newrhs,
- /*explicit_value_init_p=*/false,
- from_array, complain);
+ result = build_vec_init (lhs, NULL_TREE, newrhs,
+ /*explicit_value_init_p=*/false,
+ from_array, complain);
+ goto ret;
}
if (modifycode == INIT_EXPR)
@@ -7902,7 +7950,7 @@ cp_build_modify_expr (location_t loc, tr
result = objc_generate_write_barrier (lhs, modifycode, newrhs);
if (result)
- return result;
+ goto ret;
}
result = build2 (modifycode == NOP_EXPR ? MODIFY_EXPR : INIT_EXPR,
@@ -7912,6 +7960,9 @@ cp_build_modify_expr (location_t loc, tr
if (!plain_assign)
TREE_NO_WARNING (result) = 1;
+ ret:
+ if (preeval)
+ result = build2 (COMPOUND_EXPR, TREE_TYPE (result), preeval, result);
return result;
}
--- gcc/testsuite/g++.dg/cpp1z/eval-order4.C.jj 2017-02-13 18:11:00.744134797 +0100
+++ gcc/testsuite/g++.dg/cpp1z/eval-order4.C 2017-02-13 18:11:00.744134797 +0100
@@ -0,0 +1,80 @@
+// PR c++/79232
+// { dg-do run }
+// { dg-options "-fstrong-eval-order" }
+
+int last = 0;
+
+int
+foo (int i)
+{
+ if (i != last + 1)
+ __builtin_abort ();
+ last = i;
+ return i;
+}
+
+char a, b;
+int c;
+
+char &
+bar (int i, int j)
+{
+ foo (i);
+ return j ? a : b;
+}
+
+int
+main ()
+{
+ (foo (2) ? bar (3, 0) : bar (3, 1)) = foo (1);
+ if (last != 3)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), foo (3) ? bar (4, 0) : bar (4, 1)) = foo (1);
+ if (last != 4)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), (foo (3) ? bar (4, 0) : bar (4, 1))) = foo (1);
+ if (last != 4)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), foo (3), foo (4) ? bar (5, 0) : bar (5, 1)) = foo (1);
+ if (last != 5)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), (foo (3), (foo (4) ? bar (5, 0) : bar (5, 1)))) = foo (1);
+ if (last != 5)
+ __builtin_abort ();
+ last = 0;
+ --c = foo (1);
+ if (c != 1)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), --c) = foo (1);
+ if (last != 2 || c != 1)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), foo (3), --c) = foo (1);
+ if (last != 3 || c != 1)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), (foo (3), --c)) = foo (1);
+ if (last != 3 || c != 1)
+ __builtin_abort ();
+ last = 0;
+ bar (2, 0) = foo (1);
+ if (last != 2)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), bar (3, 0)) = foo (1);
+ if (last != 3)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), foo (3), bar (4, 0)) = foo (1);
+ if (last != 4)
+ __builtin_abort ();
+ last = 0;
+ (foo (2), (foo (3), bar (4, 0))) = foo (1);
+ if (last != 4)
+ __builtin_abort ();
+}
--- gcc/testsuite/g++.dg/other/pr79232.C.jj 2017-02-13 18:11:00.745134784 +0100
+++ gcc/testsuite/g++.dg/other/pr79232.C 2017-02-13 18:11:00.745134784 +0100
@@ -0,0 +1,12 @@
+// PR c++/79232
+// { dg-do compile }
+
+extern char a[];
+int b;
+char c, e;
+
+void
+foo (long d)
+{
+ (0, b ? &c : a)[d] = e;
+}
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-13 17:37 ` Jakub Jelinek
@ 2017-02-13 19:36 ` Nathan Sidwell
0 siblings, 0 replies; 62+ messages in thread
From: Nathan Sidwell @ 2017-02-13 19:36 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor, gcc-patches
On 02/13/2017 12:28 PM, Jakub Jelinek wrote:
>
> The reason for that VOID_TYPE_P stuff in COND_EXPR handling is not
> that stabilize_expr doesn't like it, but to avoid warning twice.
> If I replace all 3 VOID_TYPE_P tests (with the patch) in
> cp_build_modify_expr with 0 && VOID_TYPE_P, the above testcase
> errors 6 times instead of 4 times as it should.
> For the case when lhs is a pre-inc/decrement or modify_expr, we don't have
> this problem, rhs is used just one in cp_convert_for_assignment where this
> error is diagnosed normally.
>
> Thus, I'll bootstrap/regtest following simplified version of the patch
> instead:
ok, thanks
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-13 15:53 Jakub Jelinek
2017-02-13 16:45 ` Nathan Sidwell
@ 2017-02-14 15:07 ` Nathan Sidwell
2017-02-15 16:29 ` Jakub Jelinek
1 sibling, 1 reply; 62+ messages in thread
From: Nathan Sidwell @ 2017-02-14 15:07 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor
Cc: gcc-patches
On 02/13/2017 10:46 AM, Jakub Jelinek wrote:
> Hi!
>
> I'd like to ping a couple of patches:
> - C++ P1 PR79288 - wrong default TLS model for __thread static data members
> http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02349.html
This is ok, but don't you think the changelog is misleading? In your
description you say it needs DECL_EXTERNAL set, but the changelog says
'inline', which isn't something static member vars have (although I can
see how it's involved in DECL_EXTERNAL setting).
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2017-02-14 15:07 ` Nathan Sidwell
@ 2017-02-15 16:29 ` Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2017-02-15 16:29 UTC (permalink / raw)
To: Nathan Sidwell
Cc: Jason Merrill, Jan Hubicka, Jeff Law, Martin Sebor, gcc-patches
On Tue, Feb 14, 2017 at 10:05:04AM -0500, Nathan Sidwell wrote:
> On 02/13/2017 10:46 AM, Jakub Jelinek wrote:
> > Hi!
> >
> > I'd like to ping a couple of patches:
>
> > - C++ P1 PR79288 - wrong default TLS model for __thread static data members
> > http://gcc.gnu.org/ml/gcc-patches/2017-01/msg02349.html
>
> This is ok, but don't you think the changelog is misleading? In your
> description you say it needs DECL_EXTERNAL set, but the changelog says
> 'inline', which isn't something static member vars have (although I can see
> how it's involved in DECL_EXTERNAL setting).
It is something static member vars can have (explicitly or implicitly).
In 6.x we had:
/* Even if there is an in-class initialization, DECL
is considered undefined until an out-of-class
definition is provided. */
DECL_EXTERNAL (decl) = 1;
if (thread_p)
{
CP_DECL_THREAD_LOCAL_P (decl) = true;
if (!processing_template_decl)
set_decl_tls_model (decl, decl_default_tls_model (decl));
if (declspecs->gnu_thread_keyword_p)
SET_DECL_GNU_TLS_P (decl);
}
...
With the addition of C++17 inline vars I changed that to:
if (thread_p)
{
CP_DECL_THREAD_LOCAL_P (decl) = true;
if (!processing_template_decl)
set_decl_tls_model (decl, decl_default_tls_model (decl));
if (declspecs->gnu_thread_keyword_p)
SET_DECL_GNU_TLS_P (decl);
}
...
if (inlinep)
mark_inline_variable (decl);
if (!DECL_VAR_DECLARED_INLINE_P (decl)
&& !(cxx_dialect >= cxx1z && constexpr_p))
/* Even if there is an in-class initialization, DECL
is considered undefined until an out-of-class
definition is provided, unless this is an inline
variable. */
DECL_EXTERNAL (decl) = 1;
because inline static data members, explicit or implicit, really shouldn't
be marked DECL_EXTERNAL, they have in-class definition rather than a mere
declaration.
The patch just changes that back to:
...
if (inlinep)
mark_inline_variable (decl);
if (!DECL_VAR_DECLARED_INLINE_P (decl)
&& !(cxx_dialect >= cxx1z && constexpr_p))
/* Even if there is an in-class initialization, DECL
is considered undefined until an out-of-class
definition is provided, unless this is an inline
variable. */
DECL_EXTERNAL (decl) = 1;
if (thread_p)
{
CP_DECL_THREAD_LOCAL_P (decl) = true;
if (!processing_template_decl)
set_decl_tls_model (decl, decl_default_tls_model (decl));
if (declspecs->gnu_thread_keyword_p)
SET_DECL_GNU_TLS_P (decl);
}
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2012-09-05 16:34 Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2012-09-05 16:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01100.html
- C++ -Wsizeof-pointer-memaccess support (C is already in)
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping #2
@ 2011-05-31 12:25 Jakub Jelinek
2011-05-31 18:19 ` Richard Henderson
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2011-05-31 12:25 UTC (permalink / raw)
To: Jason Merrill, Richard Henderson; +Cc: gcc-patches
Hi!
- http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01182.html
various debug info improvements (typed DWARF stack etc.)
- http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01246.html
optimize away unneeded DW_OP_GNU_convert ops with typed DWARF stack
- http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01669.html
PR48688 optimization, I know Richard asked for trying it during
combine, but that attempt failed due to opposite optimization
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping #2
@ 2011-05-23 9:45 Eric Botcazou
2011-05-23 10:16 ` Richard Guenther
0 siblings, 1 reply; 62+ messages in thread
From: Eric Botcazou @ 2011-05-23 9:45 UTC (permalink / raw)
To: gcc-patches
Not as many as Jakub, but still. :-)
Fix annoying gcov filename handling (gcov.c, 2 lines):
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01380.html
Introduce -Wstack-usage:
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01992.html
Extend TYPE_DECL_IS_STUB trick (dwarf2out.c, 1 line):
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00683.html
Emit DW_AT_artificial for types:
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00700.html
Fix PR lto/48492 (bis) (dwarf2out.c, 1 line):
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01461.html
Thanks in advance.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping #2
2011-05-23 9:45 Eric Botcazou
@ 2011-05-23 10:16 ` Richard Guenther
2011-05-23 12:01 ` Eric Botcazou
0 siblings, 1 reply; 62+ messages in thread
From: Richard Guenther @ 2011-05-23 10:16 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Mon, May 23, 2011 at 9:59 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Not as many as Jakub, but still. :-)
>
>
> Fix annoying gcov filename handling (gcov.c, 2 lines):
> http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01380.html
Ok.
> Introduce -Wstack-usage:
> http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01992.html
I wonder if we can't simply use -Wframe-larger-than but with
the more precise information when -fstack-usage is present.
I'll leave dwarf2out.c bits to others.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping #2
2011-05-23 10:16 ` Richard Guenther
@ 2011-05-23 12:01 ` Eric Botcazou
2011-05-23 13:34 ` Richard Guenther
0 siblings, 1 reply; 62+ messages in thread
From: Eric Botcazou @ 2011-05-23 12:01 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
> > Fix annoying gcov filename handling (gcov.c, 2 lines):
> > Â http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01380.html
>
> Ok.
Thanks!
> > Introduce -Wstack-usage:
> > Â http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01992.html
>
> I wonder if we can't simply use -Wframe-larger-than but with
> the more precise information when -fstack-usage is present.
Do you mean when -fstack-usage is enabled? This will generate the .su file as
a by-product. The advantage of -Wstack-usage is that it has the same name as
its sibling flag so you can pass -fstack-usage -Wstack-usage to the compiler.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping #2
2011-05-23 12:01 ` Eric Botcazou
@ 2011-05-23 13:34 ` Richard Guenther
2011-05-24 23:46 ` Eric Botcazou
0 siblings, 1 reply; 62+ messages in thread
From: Richard Guenther @ 2011-05-23 13:34 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Mon, May 23, 2011 at 12:15 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> > Fix annoying gcov filename handling (gcov.c, 2 lines):
>> > http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01380.html
>>
>> Ok.
>
> Thanks!
>
>> > Introduce -Wstack-usage:
>> > http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01992.html
>>
>> I wonder if we can't simply use -Wframe-larger-than but with
>> the more precise information when -fstack-usage is present.
>
> Do you mean when -fstack-usage is enabled? This will generate the .su file as
> a by-product. The advantage of -Wstack-usage is that it has the same name as
> its sibling flag so you can pass -fstack-usage -Wstack-usage to the compiler.
Yes, I mean when -fstack-usage is enabled. Thus, make
-fstack-usage -Wframe-larger-than behave the same as
-fstack-usage -Wstack-usage. Or do you say that -Wstack-usage
doesn't require -fstack-usage? If it doesn't then I hope -Wstack-usage
does not have effects on code generation? And if not then why
can't -Wframe-larger-than just be more precise on some targets?
Richard.
> --
> Eric Botcazou
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping #2
2011-05-23 13:34 ` Richard Guenther
@ 2011-05-24 23:46 ` Eric Botcazou
2011-05-25 10:24 ` Richard Guenther
0 siblings, 1 reply; 62+ messages in thread
From: Eric Botcazou @ 2011-05-24 23:46 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
> Yes, I mean when -fstack-usage is enabled. Thus, make
> -fstack-usage -Wframe-larger-than behave the same as
> -fstack-usage -Wstack-usage. Or do you say that -Wstack-usage
> doesn't require -fstack-usage?
Yes, -Wstack-usage doesn't require -fstack-usage.
> If it doesn't then I hope -Wstack-usage does not have effects on code
> generation?
Neither -fstack-usage nor -Wstack-usage has any effect on code generation.
The former generates a .su file and the latter issues a warning.
> And if not then why can't -Wframe-larger-than just be more precise on some
> targets?
-Wframe-larger-than is documented to work on any targets and to be imprecise.
So we would need to list the targets for which it is precise (or the targets
for which it isn't precise) and maintain it. By contrast, if -Wstack-usage
returns something, then the answer is always precise. Moreover I think that
the common name is a big advantage.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping #2
2011-05-24 23:46 ` Eric Botcazou
@ 2011-05-25 10:24 ` Richard Guenther
0 siblings, 0 replies; 62+ messages in thread
From: Richard Guenther @ 2011-05-25 10:24 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Wed, May 25, 2011 at 12:00 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Yes, I mean when -fstack-usage is enabled. Thus, make
>> -fstack-usage -Wframe-larger-than behave the same as
>> -fstack-usage -Wstack-usage. Or do you say that -Wstack-usage
>> doesn't require -fstack-usage?
>
> Yes, -Wstack-usage doesn't require -fstack-usage.
>
>> If it doesn't then I hope -Wstack-usage does not have effects on code
>> generation?
>
> Neither -fstack-usage nor -Wstack-usage has any effect on code generation.
> The former generates a .su file and the latter issues a warning.
>
>> And if not then why can't -Wframe-larger-than just be more precise on some
>> targets?
>
> -Wframe-larger-than is documented to work on any targets and to be imprecise.
> So we would need to list the targets for which it is precise (or the targets
> for which it isn't precise) and maintain it. By contrast, if -Wstack-usage
> returns something, then the answer is always precise. Moreover I think that
> the common name is a big advantage.
Thanks for explaining. The patch is ok.
Thanks,
Richard.
> --
> Eric Botcazou
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* patch ping^2
@ 2010-02-23 20:38 Jerry Quinn
0 siblings, 0 replies; 62+ messages in thread
From: Jerry Quinn @ 2010-02-23 20:38 UTC (permalink / raw)
To: gcc-patches
http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00222.html
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping
@ 2006-02-14 17:19 Jakub Jelinek
2006-02-20 19:41 ` Patch ping^2 Jakub Jelinek
0 siblings, 1 reply; 62+ messages in thread
From: Jakub Jelinek @ 2006-02-14 17:19 UTC (permalink / raw)
To: gcc-patches
Hi!
--with{,out}-long-double-128 configure option:
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00664.html
(together with the already approved remaining parts of:
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00274.html
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00271.html
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00263.html).
Ok for trunk?
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
2006-02-14 17:19 Patch ping Jakub Jelinek
@ 2006-02-20 19:41 ` Jakub Jelinek
0 siblings, 0 replies; 62+ messages in thread
From: Jakub Jelinek @ 2006-02-20 19:41 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches
On Tue, Feb 14, 2006 at 12:19:41PM -0500, Jakub Jelinek wrote:
> --with{,out}-long-double-128 configure option:
> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00664.html
>
> (together with the already approved remaining parts of:
> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00274.html
> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00271.html
> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00263.html).
>
> Ok for trunk?
I'm attaching the full patch that includes also the approved
parts.
Ok?
2006-02-20 Jakub Jelinek <jakub@redhat.com>
* configure.ac: Add --with{,out}-long-double-128 configure option.
(TARGET_DEFAULT_LONG_DOUBLE_128): New test.
* configure: Rebuilt.
* config.in: Rebuilt.
* doc/install.texi (Options specification): Document
--with-long-double-128.
* config/rs6000/linux.h [TARGET_DEFAULT_LONG_DOUBLE_128]
(RS6000_DEFAULT_LONG_DOUBLE_SIZE): Define to 128.
* config/rs6000/linux64.h [TARGET_DEFAULT_LONG_DOUBLE_128]
(RS6000_DEFAULT_LONG_DOUBLE_SIZE): Define to 128.
2006-02-20 Aldy Hernandez <aldyh@redhat.com>
* config/s390/s390.c (override_options): Handle
TARGET_DEFAULT_LONG_DOUBLE_128.
* config/alpha/alpha.c (override_options): Handle
TARGET_DEFAULT_LONG_DOUBLE_128.
* config/sparc/sparc.c (sparc_override_options): Handle
TARGET_DEFAULT_LONG_DOUBLE_128.
--- gcc/doc/install.texi.jj 2006-02-20 20:01:07.000000000 +0100
+++ gcc/doc/install.texi 2006-02-20 20:21:09.000000000 +0100
@@ -1251,6 +1251,14 @@ extension. This is enabled by default o
systems. Other systems may also support it, but require the user to
specifically enable it.
+@item --with-long-double-128
+Specify if @code{long double} type should be 128-bit by default on selected
+GNU/Linux architectures. If using @code{--without-long-double-128},
+@code{long double} will be by default 64-bit, the same as @code{double} type.
+When neither of these configure options are used, the default will be
+128-bit @code{long double} when built against GNU C Library 2.4 and later,
+64-bit @code{long double} otherwise.
+
@end table
@subheading Cross-Compiler-Specific Options
--- gcc/config/alpha/alpha.c.jj 2006-02-13 22:21:21.000000000 +0100
+++ gcc/config/alpha/alpha.c 2006-02-20 20:38:27.000000000 +0100
@@ -516,6 +516,11 @@ override_options (void)
REAL_MODE_FORMAT (DFmode) = &vax_g_format;
REAL_MODE_FORMAT (TFmode) = NULL;
}
+
+#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
+ if (!(target_flags_explicit & MASK_LONG_DOUBLE_128))
+ target_flags |= MASK_LONG_DOUBLE_128;
+#endif
}
\f
/* Returns 1 if VALUE is a mask that contains full bytes of zero or ones. */
--- gcc/config/s390/s390.c.jj 2006-02-20 20:01:07.000000000 +0100
+++ gcc/config/s390/s390.c 2006-02-20 20:38:27.000000000 +0100
@@ -1415,6 +1415,11 @@ override_options (void)
}
else if (s390_stack_guard)
error ("-mstack-guard implies use of -mstack-size");
+
+#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
+ if (!(target_flags_explicit & MASK_LONG_DOUBLE_128))
+ target_flags |= MASK_LONG_DOUBLE_128;
+#endif
}
/* Map for smallest class containing reg regno. */
--- gcc/config/sparc/sparc.c.jj 2006-02-13 22:21:22.000000000 +0100
+++ gcc/config/sparc/sparc.c 2006-02-20 20:38:27.000000000 +0100
@@ -791,6 +791,11 @@ sparc_override_options (void)
sparc_costs = &ultrasparc3_costs;
break;
};
+
+#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
+ if (!(target_flags_explicit & MASK_LONG_DOUBLE_128))
+ target_flags |= MASK_LONG_DOUBLE_128;
+#endif
}
\f
#ifdef SUBTARGET_ATTRIBUTE_TABLE
--- gcc/config/rs6000/linux.h.jj 2006-02-18 19:55:52.000000000 +0100
+++ gcc/config/rs6000/linux.h 2006-02-20 20:38:27.000000000 +0100
@@ -120,3 +120,8 @@
#endif
#define POWERPC_LINUX
+
+/* ppc linux has 128-bit long double support in glibc 2.4 and later. */
+#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
+#define RS6000_DEFAULT_LONG_DOUBLE_SIZE 128
+#endif
--- gcc/config/rs6000/linux64.h.jj 2006-02-18 19:55:52.000000000 +0100
+++ gcc/config/rs6000/linux64.h 2006-02-20 20:38:27.000000000 +0100
@@ -584,3 +584,8 @@ while (0)
#endif
#define POWERPC_LINUX
+
+/* ppc{32,64} linux has 128-bit long double support in glibc 2.4 and later. */
+#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
+#define RS6000_DEFAULT_LONG_DOUBLE_SIZE 128
+#endif
--- gcc/configure.ac.jj 2006-02-20 20:01:08.000000000 +0100
+++ gcc/configure.ac 2006-02-20 20:21:09.000000000 +0100
@@ -3176,6 +3176,39 @@ if test x$gcc_cv_libc_provides_ssp = xye
[Define if your target C library provides stack protector support])
fi
+# Check if TFmode long double should be used by default or not.
+# Some glibc targets used DFmode long double, but with glibc 2.4
+# and later they can use TFmode.
+case "$target" in
+ powerpc*-*-*gnu* | \
+ sparc*-*-linux* | \
+ s390*-*-linux* | \
+ alpha*-*-linux*)
+ AC_ARG_WITH(long-double-128,
+[ --with-long-double-128 Use 128-bit long double by default.],
+ gcc_cv_target_ldbl128="$with_long_double_128",
+ [gcc_cv_target_ldbl128=no
+ if test x$host != x$target || test "x$TARGET_SYSTEM_ROOT" != x; then
+ if test "x$with_sysroot" = x; then
+ glibc_header_dir="${exec_prefix}/${target_noncanonical}/sys-include"
+ elif test "x$with_sysroot" = xyes; then
+ glibc_header_dir="${exec_prefix}/${target_noncanonical}/sys-root/usr/include"
+ else
+ glibc_header_dir="${with_sysroot}/usr/include"
+ fi
+ else
+ glibc_header_dir=/usr/include
+ fi
+ grep '^[ ]*#[ ]*define[ ][ ]*__LONG_DOUBLE_MATH_OPTIONAL' \
+ $glibc_header_dir/bits/wordsize.h > /dev/null 2>&1 \
+ && gcc_cv_target_ldbl128=yes])
+ ;;
+esac
+if test x$gcc_cv_target_ldbl128 = xyes; then
+ AC_DEFINE(TARGET_DEFAULT_LONG_DOUBLE_128, 1,
+ [Define if TFmode long double should be the default])
+fi
+
# Find out what GC implementation we want, or may, use.
AC_ARG_WITH(gc,
[ --with-gc={page,zone} choose the garbage collection mechanism to use
--- gcc/configure.jj 2006-02-20 20:01:08.000000000 +0100
+++ gcc/configure 2006-02-20 20:21:27.000000000 +0100
@@ -932,6 +932,7 @@ Optional Packages:
--with-libiconv-prefix[=DIR] search for libiconv in DIR/include and DIR/lib
--without-libiconv-prefix don't search for libiconv in includedir and libdir
--with-system-libunwind use installed libunwind
+ --with-long-double-128 Use 128-bit long double by default.
--with-gc={page,zone} choose the garbage collection mechanism to use
with the compiler
--with-system-zlib use installed libz
@@ -7612,7 +7613,7 @@ if test "${gcc_cv_prog_makeinfo_modern+s
else
ac_prog_version=`$MAKEINFO --version 2>&1 |
sed -n 's/^.*GNU texinfo.* \([0-9][0-9.]*\).*$/\1/p'`
- echo "configure:7615: version of makeinfo is $ac_prog_version" >&5
+ echo "configure:7616: version of makeinfo is $ac_prog_version" >&5
case $ac_prog_version in
'') gcc_cv_prog_makeinfo_modern=no;;
4.[4-9]*)
@@ -15675,6 +15676,46 @@ _ACEOF
fi
+# Check if TFmode long double should be used by default or not.
+# Some glibc targets used DFmode long double, but with glibc 2.4
+# and later they can use TFmode.
+case "$target" in
+ powerpc*-*-*gnu* | \
+ sparc*-*-linux* | \
+ s390*-*-linux* | \
+ alpha*-*-linux*)
+
+# Check whether --with-long-double-128 or --without-long-double-128 was given.
+if test "${with_long_double_128+set}" = set; then
+ withval="$with_long_double_128"
+ gcc_cv_target_ldbl128="$with_long_double_128"
+else
+ gcc_cv_target_ldbl128=no
+ if test x$host != x$target || test "x$TARGET_SYSTEM_ROOT" != x; then
+ if test "x$with_sysroot" = x; then
+ glibc_header_dir="${exec_prefix}/${target_noncanonical}/sys-include"
+ elif test "x$with_sysroot" = xyes; then
+ glibc_header_dir="${exec_prefix}/${target_noncanonical}/sys-root/usr/include"
+ else
+ glibc_header_dir="${with_sysroot}/usr/include"
+ fi
+ else
+ glibc_header_dir=/usr/include
+ fi
+ grep '^ *#[ ]*define[ ][ ]*__LONG_DOUBLE_MATH_OPTIONAL' \
+ $glibc_header_dir/bits/wordsize.h > /dev/null 2>&1 \
+ && gcc_cv_target_ldbl128=yes
+fi;
+ ;;
+esac
+if test x$gcc_cv_target_ldbl128 = xyes; then
+
+cat >>confdefs.h <<\_ACEOF
+#define TARGET_DEFAULT_LONG_DOUBLE_128 1
+_ACEOF
+
+fi
+
# Find out what GC implementation we want, or may, use.
# Check whether --with-gc or --without-gc was given.
--- gcc/config.in.jj 2005-12-27 17:31:45.000000000 +0100
+++ gcc/config.in 2006-02-20 20:21:09.000000000 +0100
@@ -1283,6 +1283,12 @@
#endif
+/* Define if TFmode long double should be the default */
+#ifndef USED_FOR_TARGET
+#undef TARGET_DEFAULT_LONG_DOUBLE_128
+#endif
+
+
/* Define if your target C library provides stack protector support */
#ifndef USED_FOR_TARGET
#undef TARGET_LIBC_PROVIDES_SSP
Jakub
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping**2
@ 2005-12-09 0:05 Ben Elliston
0 siblings, 0 replies; 62+ messages in thread
From: Ben Elliston @ 2005-12-09 0:05 UTC (permalink / raw)
To: gcc-patches
Ping:
http://gcc.gnu.org/ml/gcc-patches/2005-12/msg00472.html
If this patch is too hard to review in its present form, is there
further partitioning that would make it easier for reviewers?
Thanks,
Ben
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2005-08-20 18:32 Uros Bizjak
0 siblings, 0 replies; 62+ messages in thread
From: Uros Bizjak @ 2005-08-20 18:32 UTC (permalink / raw)
To: gcc-patches
Hello!
I would relly appreciate a revew of a simple one-liner patch that
would fix critical regression for i386 in 4.0.x and 4.1 branches. The
patch itself can be found at
http://gcc.gnu.org/ml/gcc-patches/2005-07/msg01490.html. There are also
some (unreviewed) testcases for recent PR fixes at
http://gcc.gnu.org/ml/gcc-patches/2005-08/msg00966.html.
BTW: I may be mistaken, but I was under impression that regressions
(found in real code, not some artifical testcases) that are marked
critical for some reason, have some kind of priority. Otherwise, I see
no point to fix them in as-soon-as-possible way, if the patch is then
left unreviewed for a month.
Thanks,
Uros.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping**2
@ 2005-03-28 1:59 Feng Wang
0 siblings, 0 replies; 62+ messages in thread
From: Feng Wang @ 2005-03-28 1:59 UTC (permalink / raw)
To: Tobias Schlüter, GCC Fortran mailing list, patch
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb2312, Size: 761 bytes --]
> 2005-03-05 [gfortran] More type-safety issues
> http://gcc.gnu.org/ml/fortran/2005-03/msg00071.html
> (the part which removes the fold_convert doesn't work with the g77 calling
> conventions, but I think the should remove support for the NULL rhs anyway,
> and that part is independent)
>
I agree with this one. It's more reasonable.
Best Regards,
Feng Wang
_________________________________________________________
Do You Yahoo!?
150ÍòÇúMP3·è¿ñËÑ£¬´øÄú´³ÈëÒôÀÖµîÌÃ
http://music.yisou.com/
ÃÀÅ®Ã÷ÐÇÓ¦Óо¡ÓУ¬ËѱéÃÀͼ¡¢ÑÞͼºÍ¿áͼ
http://image.yisou.com
1G¾ÍÊÇ1000Õ×£¬ÑÅ»¢µçÓÊ×ÔÖúÀ©ÈÝ£¡
http://cn.rd.yahoo.com/mail_cn/tag/1g/*http://cn.mail.yahoo.com/event/mail_1g/
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping**2
@ 2005-03-27 18:21 Tobias Schlüter
2005-03-28 4:05 ` Steve Kargl
0 siblings, 1 reply; 62+ messages in thread
From: Tobias Schlüter @ 2005-03-27 18:21 UTC (permalink / raw)
To: GCC Fortran mailing list, patch
2005-03-08 [gfortran] Fix PR 20178: Implement g77 / f2c calling conventions
mail here: http://gcc.gnu.org/ml/fortran/2005-03/msg00126.html
patch here: http://gcc.gnu.org/ml/fortran/2005-03/msg00129.html
one-line fix here: http://gcc.gnu.org/ml/fortran/2005-03/msg00163.html
2005-03-05 [gfortran] More type-safety issues
http://gcc.gnu.org/ml/fortran/2005-03/msg00071.html
(the part which removes the fold_convert doesn't work with the g77 calling
conventions, but I think the should remove support for the NULL rhs anyway,
and that part is independent)
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping ^ 2
@ 2004-10-17 20:43 Zdenek Dvorak
2004-10-17 20:50 ` Daniel Berlin
0 siblings, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-17 20:43 UTC (permalink / raw)
To: gcc-patches
Hello,
this patch fixes (at least) two several serious problems in lim store
motion -- misscompilations (PR 17133 and several duplicates) and
problems with compile time (PR 17790).
http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 20:43 Patch ping ^ 2 Zdenek Dvorak
@ 2004-10-17 20:50 ` Daniel Berlin
2004-10-17 21:28 ` Zdenek Dvorak
2004-10-17 21:49 ` Zdenek Dvorak
0 siblings, 2 replies; 62+ messages in thread
From: Daniel Berlin @ 2004-10-17 20:50 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: gcc-patches
On Sun, 2004-10-17 at 22:22 +0200, Zdenek Dvorak wrote:
> Hello,
>
> this patch fixes (at least) two several serious problems in lim store
> motion -- misscompilations (PR 17133 and several duplicates) and
> problems with compile time (PR 17790).
>
> http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
I still have serious problems with this patch.
It simply drops using the SSA form to do store motion, and relies on
variable names and lack of aliasing to decide what to move.
This is a serious pessimization from what we have now, which is only
broken because you move things past must-defs that kill them, and
because you don't verify that you aren't doing bad things with global
variables (you need to do an is_global_var check, and a check like dce
does for stores to global variables, and then do the additional checks,
like reverse reachability from the exit node, to make sure you aren't
making the store invisible to outside users).
If this patch is approved, I'll likely rewrite store motion as part of
the sinking work i'm doing so that it works on SSA form again.
>
> Zdenek
--
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 20:50 ` Daniel Berlin
@ 2004-10-17 21:28 ` Zdenek Dvorak
2004-10-17 21:49 ` Zdenek Dvorak
1 sibling, 0 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-17 21:28 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches
Hello,
> > this patch fixes (at least) two several serious problems in lim store
> > motion -- misscompilations (PR 17133 and several duplicates) and
> > problems with compile time (PR 17790).
> >
> > http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
>
> I still have serious problems with this patch.
> It simply drops using the SSA form to do store motion, and relies on
> variable names and lack of aliasing to decide what to move.
> This is a serious pessimization from what we have now, which is only
> broken because you move things past must-defs that kill them, and
> because you don't verify that you aren't doing bad things with global
> variables (you need to do an is_global_var check, and a check like dce
> does for stores to global variables, and then do the additional checks,
> like reverse reachability from the exit node, to make sure you aren't
> making the store invisible to outside users).
>
> If this patch is approved, I'll likely rewrite store motion as part of
> the sinking work i'm doing so that it works on SSA form again.
I think you are wrong. The rewrite has exactly the same functionality
as the current code, and should give exactly the same results.
My opinion currently is that using SSA form for virtual operands is not the
best choice for this kind of optimization, the reasons being:
1) It does not really bring any advantages over conventional approaches.
2) It is slower than conventional approaches.
3) It is more complicated and harder to understand than conventional
approaches.
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 20:50 ` Daniel Berlin
2004-10-17 21:28 ` Zdenek Dvorak
@ 2004-10-17 21:49 ` Zdenek Dvorak
2004-10-17 23:34 ` Daniel Berlin
1 sibling, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-17 21:49 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches
Hello,
> > this patch fixes (at least) two several serious problems in lim store
> > motion -- misscompilations (PR 17133 and several duplicates) and
> > problems with compile time (PR 17790).
> >
> > http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
>
> I still have serious problems with this patch.
> It simply drops using the SSA form to do store motion, and relies on
> variable names and lack of aliasing to decide what to move.
could you please explain more precisely what you mean by the last two
points? If "relies on variable names" means that it ignores ssa
versions for virtual operands, yes, it does, since virtual operands are
in FUD form, so their ssa versions do not carry any interesting
information (if you do not use them to traverse UD chains, which does
not make much sense in case you need to scan body of the whole loop
anyway). I have no idea what you mean by "relies on ... lack of
aliasing".
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 21:49 ` Zdenek Dvorak
@ 2004-10-17 23:34 ` Daniel Berlin
2004-10-17 23:56 ` Zdenek Dvorak
0 siblings, 1 reply; 62+ messages in thread
From: Daniel Berlin @ 2004-10-17 23:34 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: gcc-patches
On Sun, 2004-10-17 at 23:43 +0200, Zdenek Dvorak wrote:
> Hello,
>
> > > this patch fixes (at least) two several serious problems in lim store
> > > motion -- misscompilations (PR 17133 and several duplicates) and
> > > problems with compile time (PR 17790).
> > >
> > > http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
> >
> > I still have serious problems with this patch.
> > It simply drops using the SSA form to do store motion, and relies on
> > variable names and lack of aliasing to decide what to move.
>
> could you please explain more precisely what you mean by the last two
> points? If "relies on variable names" means that it ignores ssa
> versions for virtual operands, yes, it does, since virtual operands are
> in FUD form,
> so their ssa versions do not carry any interesting
> information
> (if you do not use them to traverse UD chains, which does
> not make much sense in case you need to scan body of the whole loop
> anyway).
This is only sort of true now (it's only true if you exclude must-defs,
and assume that our chains don't bypass non-aliasing uses, which they
admittedly happen to now), and certainly won't be true in the future.
Consider:
a_3 = V_MAY_DEF <a_2, 0, 4>
a_4 = V_MAY_DEF <a_2, 4, 8>
If you only look at names, you will move neither store.
> I have no idea what you mean by "relies on ... lack of
> aliasing".
The whole tag_ref counting thing.
As for your earlier message that:
>1) It does not really bring any advantages over conventional
>approaches.
This is not true.
For example, you can do a neat form of promotion of entire blocks where
the conditional edge is when it is in ssa form that you simply can't do
easily without ssa form.
It would be true to state that your current implementation takes no
special advantage of SSA form, however, it is not true to state that
nobody does store motion better because it's done on SSA form.
"2) It is slower than conventional approaches."
Only if you make it so.
Considering the majority of other commercial compilers (IBM, Intel,
Open64) and aggressively optimizing non-commercial compilers (LLVM, for
example) do store motion on SSA, and do so very very quickly, this leads
me to believe that the problem is in your implementation, not with the
algorithm itself.
"3) It is more complicated and harder to understand than conventional
approaches."
This is only because you are doing it in a somewhat ad-hoc way.
Store motion, is not hard to understand or more complicated in SSA form
that it is elsewhere. You have exactly the same legality conditions,
it's actually easier to test them.
>
> Zdenek
--
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 23:34 ` Daniel Berlin
@ 2004-10-17 23:56 ` Zdenek Dvorak
2004-10-18 1:18 ` Daniel Berlin
0 siblings, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-17 23:56 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches
Hello,
> > > > this patch fixes (at least) two several serious problems in lim store
> > > > motion -- misscompilations (PR 17133 and several duplicates) and
> > > > problems with compile time (PR 17790).
> > > >
> > > > http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
> > >
> > > I still have serious problems with this patch.
> > > It simply drops using the SSA form to do store motion, and relies on
> > > variable names and lack of aliasing to decide what to move.
> >
> > could you please explain more precisely what you mean by the last two
> > points? If "relies on variable names" means that it ignores ssa
> > versions for virtual operands, yes, it does, since virtual operands are
> > in FUD form,
> > so their ssa versions do not carry any interesting
> > information
> > (if you do not use them to traverse UD chains, which does
> > not make much sense in case you need to scan body of the whole loop
> > anyway).
> This is only sort of true now (it's only true if you exclude must-defs,
> and assume that our chains don't bypass non-aliasing uses, which they
> admittedly happen to now), and certainly won't be true in the future.
>
> Consider:
>
>
> a_3 = V_MAY_DEF <a_2, 0, 4>
> a_4 = V_MAY_DEF <a_2, 4, 8>
>
> If you only look at names, you will move neither store.
yes, once you start doing this type of changes you do on your branch
that are completely incompatible with how the system currently works,
the things will have to be changed.
> > I have no idea what you mean by "relies on ... lack of
> > aliasing".
>
> The whole tag_ref counting thing.
This just precisely reflects what information on aliasing is currently
provided. Assuming that your changes will be accepted, this will have
to be changed (and of course, the ssa form based implementation would
have to be changed as well).
> As for your earlier message that:
>
> >1) It does not really bring any advantages over conventional
> >approaches.
>
> This is not true.
> For example, you can do a neat form of promotion of entire blocks where
> the conditional edge is when it is in ssa form that you simply can't do
> easily without ssa form.
I cannot parse this sentence.
> It would be true to state that your current implementation takes no
> special advantage of SSA form, however, it is not true to state that
> nobody does store motion better because it's done on SSA form.
>
> "2) It is slower than conventional approaches."
> Only if you make it so.
> Considering the majority of other commercial compilers (IBM, Intel,
> Open64) and aggressively optimizing non-commercial compilers (LLVM, for
> example) do store motion on SSA, and do so very very quickly, this leads
> me to believe that the problem is in your implementation, not with the
> algorithm itself.
You are wrong at least about LLVM, for sure. LLVM does not represent
aliasing info in ssa form. I do not know much about other compilers,
but I suspect gcc is the only one that does attempt to do it.
> "3) It is more complicated and harder to understand than conventional
> approaches."
> This is only because you are doing it in a somewhat ad-hoc way.
> Store motion, is not hard to understand or more complicated in SSA form
> that it is elsewhere. You have exactly the same legality conditions,
> it's actually easier to test them.
I thought that as well, before I tried to implement it.
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-17 23:56 ` Zdenek Dvorak
@ 2004-10-18 1:18 ` Daniel Berlin
2004-10-18 6:50 ` Zdenek Dvorak
2004-10-18 8:37 ` Zdenek Dvorak
0 siblings, 2 replies; 62+ messages in thread
From: Daniel Berlin @ 2004-10-18 1:18 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: gcc-patches
On Mon, 2004-10-18 at 01:43 +0200, Zdenek Dvorak wrote:
> Hello,
> > Consider:
> >
> >
> > a_3 = V_MAY_DEF <a_2, 0, 4>
> > a_4 = V_MAY_DEF <a_2, 4, 8>
> >
> > If you only look at names, you will move neither store.
>
> yes, once you start doing this type of changes you do on your branch
> that are completely incompatible with how the system currently works,
> the things will have to be changed.
And making the types of assumptions you are proposing building into LIM
make exactly these types of changes much harder. Which is why i said if
we go down the route of applying your patch, I would just rewrite LIM.
> > > I have no idea what you mean by "relies on ... lack of
> > > aliasing".
> >
> > The whole tag_ref counting thing.
>
> This just precisely reflects what information on aliasing is currently
> provided.
> Assuming that your changes will be accepted, this will have
> to be changed (and of course, the ssa form based implementation would
> have to be changed as well)
Not really, the ssa form implementation would just work, unless you've
got some weird stuff hidden in there.
It does just work, actually, AFAIK.
>
> > As for your earlier message that:
> >
> > >1) It does not really bring any advantages over conventional
> > >approaches.
> >
> > This is not true.
> > For example, you can do a neat form of promotion of entire blocks where
> > the conditional edge is when it is in ssa form that you simply can't do
> > easily without ssa form.
>
> I cannot parse this sentence.
>
s/conditional edge is/conditional test is invariant/
> > It would be true to state that your current implementation takes no
> > special advantage of SSA form, however, it is not true to state that
> > nobody does store motion better because it's done on SSA form.
> >
> > "2) It is slower than conventional approaches."
> > Only if you make it so.
> > Considering the majority of other commercial compilers (IBM, Intel,
> > Open64) and aggressively optimizing non-commercial compilers (LLVM, for
> > example) do store motion on SSA, and do so very very quickly, this leads
> > me to believe that the problem is in your implementation, not with the
> > algorithm itself.
>
> You are wrong at least about LLVM, for sure.
Uh, see LICM.cpp.
> LLVM does not represent aliasing info in ssa form.
I never said it did.
You said store motion was slower on SSA than convential approachs,
LLVM's is on ssa and it is not slower.
> I do not know much about other compilers,
> but I suspect gcc is the only one that does attempt to do it.
You'd suspect very wrong.
Open64 has mu and chi nodes to represent aliasing in ssa form, IBM's
compilers have even more types of ssa references for aliasing (they
actually have indirect-ssa uses that also have a pointer to all the ssa
defs that make up the addressing arithmetic for things like
indirect_refs).
>
> > "3) It is more complicated and harder to understand than conventional
> > approaches."
> > This is only because you are doing it in a somewhat ad-hoc way.
> > Store motion, is not hard to understand or more complicated in SSA form
> > that it is elsewhere. You have exactly the same legality conditions,
> > it's actually easier to test them.
>
> I thought that as well, before I tried to implement it.
Did you ever ask for help when you ran into trouble?
I'm sure people would have been more than happy to try to explain how do
to it fast in SSA form.
>
> Zdenek
--
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 1:18 ` Daniel Berlin
@ 2004-10-18 6:50 ` Zdenek Dvorak
2004-10-18 8:37 ` Zdenek Dvorak
1 sibling, 0 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-18 6:50 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches
Hello,
> > > As for your earlier message that:
> > >
> > > >1) It does not really bring any advantages over conventional
> > > >approaches.
> > >
> > > This is not true.
> > > For example, you can do a neat form of promotion of entire blocks where
> > > the conditional test is invariant when it is in ssa form that you simply can't do
> > > easily without ssa form.
could you provide an example of what you mean? I cannot imagine ssa
form for virtual operands would be useful for that.
> > > It would be true to state that your current implementation takes no
> > > special advantage of SSA form, however, it is not true to state that
> > > nobody does store motion better because it's done on SSA form.
> > >
> > > "2) It is slower than conventional approaches."
> > > Only if you make it so.
> > > Considering the majority of other commercial compilers (IBM, Intel,
> > > Open64) and aggressively optimizing non-commercial compilers (LLVM, for
> > > example) do store motion on SSA, and do so very very quickly, this leads
> > > me to believe that the problem is in your implementation, not with the
> > > algorithm itself.
> >
> > You are wrong at least about LLVM, for sure.
>
> Uh, see LICM.cpp.
yes, I did. It uses *exactly* the same algorithm for store motion as
the rewritten version of tree-ssa-loop-im.c -- i.e. it gathers first
all aliasing information about stores and loads inside loop, then uses
it to decide what memory references can be promoted. Even the
representation of alias sets used by LICM.cpp is pretty close to the one
used in new tree-ssa-loop-im.c.
> > LLVM does not represent aliasing info in ssa form.
> I never said it did.
Well, if you did not, I do not know what you are trying to argue about.
Rewrite of LICM still uses SSA form, only it does not use it to decide
which memory references alias.
Zdenek
> You said store motion was slower on SSA than convential approachs,
> LLVM's is on ssa and it is not slower.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 1:18 ` Daniel Berlin
2004-10-18 6:50 ` Zdenek Dvorak
@ 2004-10-18 8:37 ` Zdenek Dvorak
2004-10-18 14:12 ` David Edelsohn
1 sibling, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-18 8:37 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches
Hello,
I have thought about the situation a bit more. My opinion now is:
1) Unless your patch to make voperands represent output dependences again
(http://gcc.gnu.org/ml/gcc-patches/2004-10/msg01437.html) gets into
mainline, the rewrite of store motion
(http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html) is the best
choice.
2) If your patch gets to mainline, it will be still necessary to rewrite
store motion to fix its current compile time problems. It will
however be useful to use SSA form for voperands then. This however
will be just a fairly small change over
http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html, since the
only thing that will need to be changed is the way how the
information about aliasing is determined.
Any of these options is fine with me, although 2) will be easier to
update when your changes to aliasing info representation are included
(it should just work without any further changes, in fact, but of course
one cannot be completely sure).
Note that there are no real dependences between the two patches, i.e.
they can be reviewed independently.
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 8:37 ` Zdenek Dvorak
@ 2004-10-18 14:12 ` David Edelsohn
2004-10-18 14:28 ` Zdenek Dvorak
0 siblings, 1 reply; 62+ messages in thread
From: David Edelsohn @ 2004-10-18 14:12 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: gcc-patches
> 1) Unless your patch to make voperands represent output dependences again
> (http://gcc.gnu.org/ml/gcc-patches/2004-10/msg01437.html) gets into
> mainline, the rewrite of store motion
> (http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html) is the best
> choice.
Regardless whether Dan's patch gets approved for mainline, the
original, unmodified patch is taking GCC in the wrong direction and should
not be accepted.
Developing loop optimizations for the GCC SSA infrastructure in
isolation without utilizing the assistance of other developer is much less
effective and much less productive.
David
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 14:12 ` David Edelsohn
@ 2004-10-18 14:28 ` Zdenek Dvorak
2004-10-18 15:00 ` Daniel Berlin
0 siblings, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-18 14:28 UTC (permalink / raw)
To: David Edelsohn; +Cc: gcc-patches
Hello,
> > 1) Unless your patch to make voperands represent output dependences again
> > (http://gcc.gnu.org/ml/gcc-patches/2004-10/msg01437.html) gets into
> > mainline, the rewrite of store motion
> > (http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html) is the best
> > choice.
>
> Regardless whether Dan's patch gets approved for mainline, the
> original, unmodified patch is taking GCC in the wrong direction and should
> not be accepted.
you cannot be serious. Unless the msg01437.html patch is accepted, the
rewrite is the only sane possibility how to solve PR 17133 (serious
misscompilations...)
And why do you assume that the direction in that the patch takes it is
wrong? It is the way at least one other compiler (LLVM) uses. It is
clean and very easy to understand and extend. Also once the
msg01437.html is accepted (which it should, at least for 4.1, although I
think it is a bit too intrusive for 4.0), it is trivial to make it take
advantage of SSA form for virtual operands.
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 14:28 ` Zdenek Dvorak
@ 2004-10-18 15:00 ` Daniel Berlin
2004-10-18 15:40 ` Zdenek Dvorak
0 siblings, 1 reply; 62+ messages in thread
From: Daniel Berlin @ 2004-10-18 15:00 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: gcc-patches, David Edelsohn
On Oct 18, 2004, at 10:25 AM, Zdenek Dvorak wrote:
> Hello,
>
>>> 1) Unless your patch to make voperands represent output dependences
>>> again
>>> (http://gcc.gnu.org/ml/gcc-patches/2004-10/msg01437.html) gets
>>> into
>>> mainline, the rewrite of store motion
>>> (http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html) is the
>>> best
>>> choice.
>>
>> Regardless whether Dan's patch gets approved for mainline, the
>> original, unmodified patch is taking GCC in the wrong direction and
>> should
>> not be accepted.
>
> you cannot be serious. Unless the msg01437.html patch is accepted, the
> rewrite is the only sane possibility how to solve PR 17133 (serious
> misscompilations...)
This is almost true.
The other option is to disable LIM for 4.0 until it's fixed, if the
must-def patch doesn't go in.
However, you are correct that without that killing information, there
would be no good way to fix 17133 in 4.0 besides disabling or your
patch.
>
> And why do you assume that the direction in that the patch takes it is
> wrong? It is the way at least one other compiler (LLVM) uses.
Not quite. At least, I don't believe it's quite the same.
LLVM will hoist loads/sink stores if the addressing arithmetic for a
given load/store doesn't change.
It doesn't look like you do that anymore, but i can't quite tell.
IE they use the equivalent of single_reachable_address, called
PointerInvalidatedByLoop for loads, and an is_loop_invariant check for
stores, both of which it looks like you've removed (by removing
single_reachable_address).
Also, LLVM does it this way because they *don't* have a virtual form
for memory operands.
Open64 basically does what we were doing, except even more complex
because it's part of SSAPRE on the reverse flowgraph (they nicknamed it
SSUPRE, for single static use. They build an entirely new ssa form for
stores), and SSAPRE is insanely complex.
IBM's compiler does it more or less like we do now (since they have
virtual ssa form), only in ~500 lines of code. :)
> It is
> clean and very easy to understand and extend. Also once the
> msg01437.html is accepted (which it should, at least for 4.1, although
> I
> think it is a bit too intrusive for 4.0), it is trivial to make it take
> advantage of SSA form for virtual operands.
>
I don't see how it would look much different from what we have now if
you did.
I'd appreciate it if you could show me what you have in mind.
> Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping ^ 2
2004-10-18 15:00 ` Daniel Berlin
@ 2004-10-18 15:40 ` Zdenek Dvorak
0 siblings, 0 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-10-18 15:40 UTC (permalink / raw)
To: Daniel Berlin; +Cc: gcc-patches, David Edelsohn
Hello,
> >And why do you assume that the direction in that the patch takes it is
> >wrong? It is the way at least one other compiler (LLVM) uses.
>
> Not quite. At least, I don't believe it's quite the same.
> LLVM will hoist loads/sink stores if the addressing arithmetic for a
> given load/store doesn't change.
> It doesn't look like you do that anymore, but i can't quite tell.
> IE they use the equivalent of single_reachable_address, called
> PointerInvalidatedByLoop for loads, and an is_loop_invariant check for
> stores, both of which it looks like you've removed (by removing
> single_reachable_address).
PointerInvalidatedByLoop is equivalent to what the rewritten lim does using
count_tag_references and the check at the start of determine_lsm_ref,
given the limited amount of aliasing information we have now.
Moving of loads for that there are no aliasing stores is not part of
store motion and it is performed in
determine_invariantness/move_computations (using the virtual ssa).
> > It is
> >clean and very easy to understand and extend. Also once the
> >msg01437.html is accepted (which it should, at least for 4.1, although
> >I
> >think it is a bit too intrusive for 4.0), it is trivial to make it take
> >advantage of SSA form for virtual operands.
> >
>
> I don't see how it would look much different from what we have now if
> you did.
> I'd appreciate it if you could show me what you have in mind.
basically replacing the count_tag_references/determine_lsm_ref cycle by
traversing the UD chains in virtual operands from loads and stores in
the loop body.
Indeed it is fairly similar to what we do now, but it would not need to
scan also through DU chains as we do now (which seems to be one of the
reasons for compile time problems, since many of the uses we look at are
outside of the loop and therefore they are not interesting). It also would
not mix the steps of finding the memory references and determining their aliasing
relationships in a single relatively hard to understand function
(single_reachable_address).
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping ^2
@ 2004-08-02 18:04 Pat Haugen
0 siblings, 0 replies; 62+ messages in thread
From: Pat Haugen @ 2004-08-02 18:04 UTC (permalink / raw)
To: gcc-patches
Ping for crossjump patch.
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00495.html
-Pat
^ permalink raw reply [flat|nested] 62+ messages in thread
* patch ping^2
@ 2004-06-28 5:52 Jerry Quinn
0 siblings, 0 replies; 62+ messages in thread
From: Jerry Quinn @ 2004-06-28 5:52 UTC (permalink / raw)
To: gcc-patches
Still unreviewed:
http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01662.html
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2004-06-10 21:36 Richard Guenther
0 siblings, 0 replies; 62+ messages in thread
From: Richard Guenther @ 2004-06-10 21:36 UTC (permalink / raw)
To: gcc-patches
Ping!
http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00187.html
[PATCH] Add leafify function attribute
Thanks,
Richard.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
@ 2004-03-16 23:21 Zdenek Dvorak
2004-03-17 2:40 ` Roger Sayle
2004-03-19 8:14 ` Zdenek Dvorak
0 siblings, 2 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-03-16 23:21 UTC (permalink / raw)
To: gcc-patches; +Cc: rth
Hello,
http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
-- doloop optimization rewrite. I would like to get this in since
it fixes the problem with interaction of the new loop unroller and
doloop optimization (possibly not fully optimally, as we discussed
with Mircea Namolaru, at least in a way that should be simple to
improve upon).
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2004-03-16 23:21 Zdenek Dvorak
@ 2004-03-17 2:40 ` Roger Sayle
2004-03-17 4:58 ` Zdenek Dvorak
2004-03-19 8:14 ` Roger Sayle
2004-03-19 8:14 ` Zdenek Dvorak
1 sibling, 2 replies; 62+ messages in thread
From: Roger Sayle @ 2004-03-17 2:40 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: rth, gcc-patches
On Wed, 17 Mar 2004, Zdenek Dvorak wrote:
> http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
> -- doloop optimization rewrite.
This is Ok for mainline.
The only reason I didn't approve this patch (with the others) after the
first ping is because you'd Cc'd "rth at redhat dot com" on the original
posting and that reminder. I guessing after two pings, you hadn't
previously arranged with him to examine this patch?
My apologies if I've committed a "faux pas" reviewing this patch
after only two pings, but Cc'ing maintainers creates a etiquette dilema,
which only slows down the patch review process. Perhaps using Bcc is
safer?
Roger
--
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2004-03-17 2:40 ` Roger Sayle
@ 2004-03-17 4:58 ` Zdenek Dvorak
2004-03-19 8:14 ` Zdenek Dvorak
2004-03-19 8:14 ` Roger Sayle
1 sibling, 1 reply; 62+ messages in thread
From: Zdenek Dvorak @ 2004-03-17 4:58 UTC (permalink / raw)
To: Roger Sayle; +Cc: rth, gcc-patches
Hello,
> > http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
> > -- doloop optimization rewrite.
>
> This is Ok for mainline.
thanks a lot.
> The only reason I didn't approve this patch (with the others) after the
> first ping is because you'd Cc'd "rth at redhat dot com" on the original
> posting and that reminder. I guessing after two pings, you hadn't
> previously arranged with him to examine this patch?
>
> My apologies if I've committed a "faux pas" reviewing this patch
> after only two pings, but Cc'ing maintainers creates a etiquette dilema,
> which only slows down the patch review process. Perhaps using Bcc is
> safer?
oops... sorry, I think it is just a reflex from the times when basically
noone but Richard did approve any patches :-)
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2004-03-17 4:58 ` Zdenek Dvorak
@ 2004-03-19 8:14 ` Zdenek Dvorak
0 siblings, 0 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-03-19 8:14 UTC (permalink / raw)
To: Roger Sayle; +Cc: rth, gcc-patches
Hello,
> > http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
> > -- doloop optimization rewrite.
>
> This is Ok for mainline.
thanks a lot.
> The only reason I didn't approve this patch (with the others) after the
> first ping is because you'd Cc'd "rth at redhat dot com" on the original
> posting and that reminder. I guessing after two pings, you hadn't
> previously arranged with him to examine this patch?
>
> My apologies if I've committed a "faux pas" reviewing this patch
> after only two pings, but Cc'ing maintainers creates a etiquette dilema,
> which only slows down the patch review process. Perhaps using Bcc is
> safer?
oops... sorry, I think it is just a reflex from the times when basically
noone but Richard did approve any patches :-)
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: Patch ping^2
2004-03-17 2:40 ` Roger Sayle
2004-03-17 4:58 ` Zdenek Dvorak
@ 2004-03-19 8:14 ` Roger Sayle
1 sibling, 0 replies; 62+ messages in thread
From: Roger Sayle @ 2004-03-19 8:14 UTC (permalink / raw)
To: Zdenek Dvorak; +Cc: rth, gcc-patches
On Wed, 17 Mar 2004, Zdenek Dvorak wrote:
> http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
> -- doloop optimization rewrite.
This is Ok for mainline.
The only reason I didn't approve this patch (with the others) after the
first ping is because you'd Cc'd "rth at redhat dot com" on the original
posting and that reminder. I guessing after two pings, you hadn't
previously arranged with him to examine this patch?
My apologies if I've committed a "faux pas" reviewing this patch
after only two pings, but Cc'ing maintainers creates a etiquette dilema,
which only slows down the patch review process. Perhaps using Bcc is
safer?
Roger
--
^ permalink raw reply [flat|nested] 62+ messages in thread
* Patch ping^2
2004-03-16 23:21 Zdenek Dvorak
2004-03-17 2:40 ` Roger Sayle
@ 2004-03-19 8:14 ` Zdenek Dvorak
1 sibling, 0 replies; 62+ messages in thread
From: Zdenek Dvorak @ 2004-03-19 8:14 UTC (permalink / raw)
To: gcc-patches; +Cc: rth
Hello,
http://gcc.gnu.org/ml/gcc-patches/2004-02/msg01733.html
-- doloop optimization rewrite. I would like to get this in since
it fixes the problem with interaction of the new loop unroller and
doloop optimization (possibly not fully optimally, as we discussed
with Mircea Namolaru, at least in a way that should be simple to
improve upon).
Zdenek
^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2024-02-26 16:26 UTC | newest]
Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 8:13 [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jakub Jelinek
2021-02-23 8:49 ` Patch ping Jakub Jelinek
2021-03-01 13:01 ` Patch ping^2 Jakub Jelinek
2021-03-02 23:39 ` [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jeff Law
-- strict thread matches above, loose matches on Subject: below --
2024-02-26 9:33 Patch ping^2 Jakub Jelinek
2024-02-26 9:38 ` Uros Bizjak
2024-02-26 16:26 ` Jan Hubicka
2021-03-19 9:57 Patch ping Jakub Jelinek
2021-03-29 15:19 ` Patch ping^2 Jakub Jelinek
2020-03-10 12:28 Patch ping Jakub Jelinek
2020-03-17 12:10 ` Patch ping^2 Jakub Jelinek
2020-03-17 12:23 ` Richard Biener
2018-05-07 8:10 Jakub Jelinek
2018-04-05 8:35 Jakub Jelinek
2017-12-15 9:09 Jakub Jelinek
2017-11-06 16:22 Patch ping Jakub Jelinek
2017-11-14 17:07 ` Patch ping^2 Jakub Jelinek
2017-11-16 3:07 ` Jim Wilson
2017-04-18 14:21 Jakub Jelinek
2017-04-18 17:07 ` Jeff Law
2017-02-13 15:53 Jakub Jelinek
2017-02-13 16:45 ` Nathan Sidwell
2017-02-13 16:50 ` Jakub Jelinek
2017-02-13 17:37 ` Jakub Jelinek
2017-02-13 19:36 ` Nathan Sidwell
2017-02-14 15:07 ` Nathan Sidwell
2017-02-15 16:29 ` Jakub Jelinek
2012-09-05 16:34 Jakub Jelinek
2011-05-31 12:25 Patch ping #2 Jakub Jelinek
2011-05-31 18:19 ` Richard Henderson
2011-05-23 9:45 Eric Botcazou
2011-05-23 10:16 ` Richard Guenther
2011-05-23 12:01 ` Eric Botcazou
2011-05-23 13:34 ` Richard Guenther
2011-05-24 23:46 ` Eric Botcazou
2011-05-25 10:24 ` Richard Guenther
2010-02-23 20:38 patch ping^2 Jerry Quinn
2006-02-14 17:19 Patch ping Jakub Jelinek
2006-02-20 19:41 ` Patch ping^2 Jakub Jelinek
2005-12-09 0:05 Patch ping**2 Ben Elliston
2005-08-20 18:32 Patch ping^2 Uros Bizjak
2005-03-28 1:59 Patch ping**2 Feng Wang
2005-03-27 18:21 Tobias Schlüter
2005-03-28 4:05 ` Steve Kargl
2004-10-17 20:43 Patch ping ^ 2 Zdenek Dvorak
2004-10-17 20:50 ` Daniel Berlin
2004-10-17 21:28 ` Zdenek Dvorak
2004-10-17 21:49 ` Zdenek Dvorak
2004-10-17 23:34 ` Daniel Berlin
2004-10-17 23:56 ` Zdenek Dvorak
2004-10-18 1:18 ` Daniel Berlin
2004-10-18 6:50 ` Zdenek Dvorak
2004-10-18 8:37 ` Zdenek Dvorak
2004-10-18 14:12 ` David Edelsohn
2004-10-18 14:28 ` Zdenek Dvorak
2004-10-18 15:00 ` Daniel Berlin
2004-10-18 15:40 ` Zdenek Dvorak
2004-08-02 18:04 Patch ping ^2 Pat Haugen
2004-06-28 5:52 patch ping^2 Jerry Quinn
2004-06-10 21:36 Patch ping^2 Richard Guenther
2004-03-16 23:21 Zdenek Dvorak
2004-03-17 2:40 ` Roger Sayle
2004-03-17 4:58 ` Zdenek Dvorak
2004-03-19 8:14 ` Zdenek Dvorak
2004-03-19 8:14 ` Roger Sayle
2004-03-19 8:14 ` Zdenek Dvorak
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).