On Mon, Dec 3, 2012 at 9:19 PM, Steven Bosscher wrote:
>> So the compiler doesn't bootstrap with the gcse.c patch you posted earlier in
>> the thread? Still this seems too bold to me, the note datum could be a
>> constant and should be preserved in this case.
>
> You mean the patch at
> http://gcc
> You mean the patch at
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02275.html right?
>
> I haven't tried that other patch. I'll test that one.
Yes, thanks.
> I suppose so. But this was all added before RTL fwprop and way before
> GIMPLE optimizations. Avoiding the self-referential case is ju
On Mon, Dec 3, 2012 at 7:23 PM, Eric Botcazou wrote:
>> >> 2. gcse.c: gcse_emit_move_after added notes, but none of them was very
>> >> useful as far as I could tell, and almost all of them turned
>> >> self-referencing after CPROP. So I propose we just never add notes in
>> >> this case.
>> >
>> >
Il 01/12/2012 15:54, Eric Botcazou ha scritto:
>> Attached is a different fix. It splits DF_REF_IN_NOTE in two: One flag
>> > for each kind of note. This allows the dead note removal code to
>> > distinguish the source note for the EQ_USES. I needed to remove one
>> > flag to keep the df_ref_flags
> >> 2. gcse.c: gcse_emit_move_after added notes, but none of them was very
> >> useful as far as I could tell, and almost all of them turned
> >> self-referencing after CPROP. So I propose we just never add notes in
> >> this case.
> >
> > gcse_emit_move_after also preserves existing notes. Are
On Sat, Dec 1, 2012 at 3:54 PM, Eric Botcazou wrote:
>> The patch also includes all places I've found so far where the
>> compiler could create self-referencing notes:
>>
>> 1. optabs.c: Not sure what it was trying to do, but now it just
>> refuses to add a note if TARGET is mentioned in one of the
> Attached is a different fix. It splits DF_REF_IN_NOTE in two: One flag
> for each kind of note. This allows the dead note removal code to
> distinguish the source note for the EQ_USES. I needed to remove one
> flag to keep the df_ref_flags 16-bit, but the DF_REF_SUBREG flag looks
> completely un
On Wed, Nov 28, 2012 at 11:10 PM, Eric Botcazou wrote:
>> Well, I'm not sure I agree that they are wrong. Consider:
>>
>> S0: r1 = ...
>> S1: r2 = r1 + 10
>> S2: r1 = r2 + 10 { REG_EQUAL r1 + 20 }
>> S3: r3 = r1 + 10
>>
>> Clearly it would be wrong to use the REG_EQUAL note on S2 to optimize S3 to:
> Well, I'm not sure I agree that they are wrong. Consider:
>
> S0: r1 = ...
> S1: r2 = r1 + 10
> S2: r1 = r2 + 10 { REG_EQUAL r1 + 20 }
> S3: r3 = r1 + 10
>
> Clearly it would be wrong to use the REG_EQUAL note on S2 to optimize S3 to:
>
> S0: r1 = ...
> S1: r2 = r1 + 10
> S2: r1 = r1 + 20
> S3
On Wed, Nov 28, 2012 at 8:44 AM, Eric Botcazou wrote:
>> Again: Is this really the direction we should be going in? (I haven't
>> any better ideas...)
>
> Do you mean for self-referencing notes? If so, definitely, these notes are
> either wrong or almost always redundant so we should eliminate th
> Again: Is this really the direction we should be going in? (I haven't
> any better ideas...)
Do you mean for self-referencing notes? If so, definitely, these notes are
either wrong or almost always redundant so we should eliminate them.
As for the more general problem of REG_EQ* notes, perhap
On Wed, Nov 28, 2012 at 12:58 AM, Steven Bosscher wrote:
> On Wed, Nov 28, 2012 at 12:53 AM, Steven Bosscher wrote:
>> On Wed, Nov 28, 2012 at 12:47 AM, Eric Botcazou wrote:
Count me in, too. So let's avoid it:
* gcse.c (struct reg_use): Remove unused struct.
(
On Wed, Nov 28, 2012 at 12:53 AM, Steven Bosscher wrote:
> On Wed, Nov 28, 2012 at 12:47 AM, Eric Botcazou wrote:
>>> Count me in, too. So let's avoid it:
>>>
>>> * gcse.c (struct reg_use): Remove unused struct.
>>> (gcse_emit_move_after): Do not create REG_EQUAL notes that referen
On Wed, Nov 28, 2012 at 12:47 AM, Eric Botcazou wrote:
>> Count me in, too. So let's avoid it:
>>
>> * gcse.c (struct reg_use): Remove unused struct.
>> (gcse_emit_move_after): Do not create REG_EQUAL notes that reference
>> the SET_DEST of the instruction the note would be attache
> Count me in, too. So let's avoid it:
>
> * gcse.c (struct reg_use): Remove unused struct.
> (gcse_emit_move_after): Do not create REG_EQUAL notes that reference
> the SET_DEST of the instruction the note would be attached to.
OK (with PR rtl-optimization/55006) if it passes boo
On Tue, Nov 27, 2012 at 5:57 PM, Eric Botcazou wrote:
>> This note seems very very weird. For one thing, it becomes invalid on
>> the very instruction where it is created. I would say that it should
>> not be there.
>
> Agreed.
Count me in, too. So let's avoid it:
* gcse.c (struct reg_
> This note seems very very weird. For one thing, it becomes invalid on
> the very instruction where it is created. I would say that it should
> not be there.
Agreed.
--
Eric Botcazou
On Tue, Nov 27, 2012 at 3:25 PM, Dominique Dhumieres wrote:
>> And more band-aid, ...
>
> The gcc_assert triggers at bootstrap when compiling gcc/ada/ali.adb:
>
> +===GNAT BUG DETECTED==+
> | 4.8.0 20121127 (experimental) [trunk revision 193848p1
Il 27/11/2012 13:00, Steven Bosscher ha scritto:
> It all
> starts with PRE creating a REG_EQUAL note that references the register
> that's SET in the insn the note is attached to, but the register is
> live after the insn so from that point of view the note is not
> invalid.
This note seems very
> And more band-aid, ...
The gcc_assert triggers at bootstrap when compiling gcc/ada/ali.adb:
+===GNAT BUG DETECTED==+
| 4.8.0 20121127 (experimental) [trunk revision 193848p10]
(x86_64-apple-darwin10.8.0) GCC error:|
| in df_remove_dead_eq_not
> Dominique, could you give this a try and see if it helps?
With the patch, the miscompilation of aermod.f90 is gone.
Thanks,
Dominique
On Tue, Nov 27, 2012 at 1:28 PM, Steven Bosscher wrote:
> Dominique, could you give this a try and see if it helps?
> (But as I said up-thread: I'm not sure this is a proper fix, or just
> another band-aid...)
And more band-aid, but this time I'm not even sure where things go
wrong. In any case, w
On Tue, Nov 27, 2012 at 1:00 PM, Steven Bosscher wrote:
> Now that USE of r719 is a use of a dead register, rendering the
> REG_EQUAL invalid. From there on the problem is the same as the ones I
> had to "fix" in loop-unroll.c. First the webizer puts the USE in a
> different web and renames the USE
On Tue, Nov 27, 2012 at 11:34 AM, Steven Bosscher wrote:
>>> gcc/
>>> * loop-unroll.c (struct iv_to_split): Add new 'orig_var' member.
>>> (analyze_iv_to_split_insn): Record it.
>>> (maybe_strip_eq_note_for_split_iv): New function to remove REG_EQUAL
>>> notes that refer to
On Tue, Nov 27, 2012 at 10:55 AM, Eric Botcazou wrote:
>> Or rather this one. Same hammer, different color. It turns out that
>> the rtlanal.c change caused problems, so I've got to use a home-brewn
>> equivalent of remove_reg_equal_equiv_notes_for_regno...
>>
>> Test case is unchanged so I'm omit
> Or rather this one. Same hammer, different color. It turns out that
> the rtlanal.c change caused problems, so I've got to use a home-brewn
> equivalent of remove_reg_equal_equiv_notes_for_regno...
>
> Test case is unchanged so I'm omitting it here.
>
> Ciao!
> Steven
>
>
> gcc/
> * loo
On Mon, Nov 26, 2012 at 3:38 PM, Dominique Dhumieres wrote:
>> Or rather this one. Same hammer, different color. It turns out that
>> the rtlanal.c change caused problems, so I've got to use a home-brewn
>> equivalent of remove_reg_equal_equiv_notes_for_regno...
>
> This does not fix pr55006 on x8
> Or rather this one. Same hammer, different color. It turns out that
> the rtlanal.c change caused problems, so I've got to use a home-brewn
> equivalent of remove_reg_equal_equiv_notes_for_regno...
This does not fix pr55006 on x86_64-apple-darwin10;-(while the patch in
http://gcc.gnu.org/ml/gcc-
On Sun, Nov 25, 2012 at 9:44 PM, Steven Bosscher wrote:
> On Sat, Nov 24, 2012 at 2:09 AM, Steven Bosscher wrote:
>> On Fri, Nov 23, 2012 at 11:45 PM, Steven Bosscher wrote:
>>> Removing the note is easier but it may hurt optimization later on:
>>> CSE2 puts the note back in but this introduces a p
On Sat, Nov 24, 2012 at 2:09 AM, Steven Bosscher wrote:
> On Fri, Nov 23, 2012 at 11:45 PM, Steven Bosscher wrote:
>> Removing the note is easier but it may hurt optimization later on:
>> CSE2 puts the note back in but this introduces a pass ordering
>> dependency. Perhaps that's not a big problem,
On Fri, Nov 23, 2012 at 11:45 PM, Steven Bosscher wrote:
> Removing the note is easier but it may hurt optimization later on:
> CSE2 puts the note back in but this introduces a pass ordering
> dependency. Perhaps that's not a big problem, but I'm not sure...
Hmm, actually CSE2 fails to put the not
On Mon, Nov 19, 2012 at 11:42 PM, Eric Botcazou wrote:
>> That could be done, yes. Cleaning up the REG_EQ* notes requires
>> liveness at the insn level, so it'd require a bigger re-organization
>> of the code. Perhaps adding a new pass (conditional on DF_EQ_NOTES)
>> over all insn in df_lr_finalize
> That could be done, yes. Cleaning up the REG_EQ* notes requires
> liveness at the insn level, so it'd require a bigger re-organization
> of the code. Perhaps adding a new pass (conditional on DF_EQ_NOTES)
> over all insn in df_lr_finalize, tracking liveness and calling
> df_remove_dead_eq_notes o
On Mon, Nov 19, 2012 at 10:50 PM, Eric Botcazou wrote:
>> The root cause is the bad REG_EQUAL note. I think the most robust
>> solution is to make the webizer re-compute notes before renaming.
>> Patch for that is attached.
>
> Thanks for the analysis. However...
>
>> Ciao!
>> Steven
>>
>>
>>
> The root cause is the bad REG_EQUAL note. I think the most robust
> solution is to make the webizer re-compute notes before renaming.
> Patch for that is attached.
Thanks for the analysis. However...
> Ciao!
> Steven
>
>
> PR rtl-optimization/55006
> * web.c (web_main): Add t
> Thanks for this reduced test case, that's saving me a lot of work!
You're welcome.
> Can you please try and see if the following C test case also fails?
Yup, with the same compilation options, there are the same dreadful lines
ld [%g0+0], %fn
in the assembly file, which translat
On Mon, Nov 19, 2012 at 9:34 PM, Steven Bosscher wrote:
> On Mon, Nov 19, 2012 at 12:24 PM, Eric Botcazou wrote:
>>> Yes, I'll be looking into this soon.
>>
>> We have a related regression on SPARC:
>>
>> FAIL: gfortran.dg/minmaxloc_5.f90 -O3 -fomit-frame-pointer -funroll-loops
>> execution test
>
On Mon, Nov 19, 2012 at 12:24 PM, Eric Botcazou wrote:
>> Yes, I'll be looking into this soon.
>
> We have a related regression on SPARC:
>
> FAIL: gfortran.dg/minmaxloc_5.f90 -O3 -fomit-frame-pointer -funroll-loops
> execution test
> FAIL: gfortran.dg/minmaxloc_5.f90 -O3 -fomit-frame-pointer -f
> Yes, I'll be looking into this soon.
We have a related regression on SPARC:
FAIL: gfortran.dg/minmaxloc_5.f90 -O3 -fomit-frame-pointer -funroll-loops
execution test
FAIL: gfortran.dg/minmaxloc_5.f90 -O3 -fomit-frame-pointer -funroll-all-loops
-finline-functions execution test
FAIL: gfortr
On Mon, Nov 19, 2012 at 12:15 AM, Dominique Dhumieres wrote:
>> I think this should fix it. Can't test it right now, so help
>> appreciated (Honza: hint hint! ;-)
>
> The change at
> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01511/remove_dead_eq_notes.diff
> (revision 192526) caused
>
> http://
> I think this should fix it. Can't test it right now, so help
> appreciated (Honza: hint hint! ;-)
The change at
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01511/remove_dead_eq_notes.diff
(revision 192526) caused
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55006
Dominique
On Wed, Oct 17, 2012 at 6:54 AM, Steven Bosscher wrote:
> On Tue, Oct 16, 2012 at 1:38 PM, Paolo Bonzini wrote:
Bottom line: This is a bug in df_kill_notes.
>>
>> Yep, and it could cause wrong code generation, couldn't it? Because the
>> new (reg:DI 72) is obviously not equal to (plus:DI (re
On Tue, Oct 16, 2012 at 1:38 PM, Paolo Bonzini wrote:
>>> Bottom line: This is a bug in df_kill_notes.
>
> Yep, and it could cause wrong code generation, couldn't it? Because the
> new (reg:DI 72) is obviously not equal to (plus:DI (reg:DI 72)
> (const_int 24)), but it could be propagated to subse
Il 16/10/2012 12:35, Steven Bosscher ha scritto:
> On Tue, Oct 16, 2012 at 12:15 PM, Steven Bosscher wrote:
>> On Mon, Oct 15, 2012 at 3:26 PM, Steven Bosscher wrote:
>>> On Mon, Oct 15, 2012 at 3:21 PM, Paolo Bonzini wrote:
Il 15/10/2012 14:53, Steven Bosscher ha scritto:
> I think I've s
On Tue, Oct 16, 2012 at 12:15 PM, Steven Bosscher wrote:
> On Mon, Oct 15, 2012 at 3:26 PM, Steven Bosscher wrote:
>> On Mon, Oct 15, 2012 at 3:21 PM, Paolo Bonzini wrote:
>>> Il 15/10/2012 14:53, Steven Bosscher ha scritto:
I think I've shown above that we're all looking at the wrong pass...
On Mon, Oct 15, 2012 at 3:26 PM, Steven Bosscher wrote:
> On Mon, Oct 15, 2012 at 3:21 PM, Paolo Bonzini wrote:
>> Il 15/10/2012 14:53, Steven Bosscher ha scritto:
>>> I think I've shown above that we're all looking at the wrong pass...
>>
>> I think you have... so we want a patch like this?
>
> I
On Mon, Oct 15, 2012 at 3:21 PM, Paolo Bonzini wrote:
> Il 15/10/2012 14:53, Steven Bosscher ha scritto:
>> I think I've shown above that we're all looking at the wrong pass...
>
> I think you have... so we want a patch like this?
I don't think so. df_kill_notes is already supposed to take care o
Il 15/10/2012 14:53, Steven Bosscher ha scritto:
> I think I've shown above that we're all looking at the wrong pass...
I think you have... so we want a patch like this?
Index: df-problems.c
===
--- df-problems.c (revisione 183
On Mon, Oct 15, 2012 at 1:49 PM, Paolo Bonzini wrote:
>> I strongly disagree with this approach though. It destroys information
>> that is correct, that we had before DF_RD_PRUNE_DEAD_DEFS, that we can
>> update, and that helps with optimization. With renaming these notes
>> are valid, and do not r
Il 15/10/2012 10:37, Steven Bosscher ha scritto:
>> I prefer to declare the notes invalid and drop the notes.
> Then, afaic, our only option is to drop them all in web, as per attached
> patch.
>
> I strongly disagree with this approach though. It destroys information
> that is correct, that we ha
On Mon, Oct 15, 2012 at 10:37 AM, Steven Bosscher wrote:
> On Mon, Oct 15, 2012 at 10:19 AM, Paolo Bonzini wrote:
>> I prefer to declare the notes invalid and drop the notes.
>
> I strongly disagree with this approach though. It destroys information
> that is correct, that we had before DF_RD_PRUN
On Mon, Oct 15, 2012 at 10:19 AM, Paolo Bonzini wrote:
> I prefer to declare the notes invalid and drop the notes.
Then, afaic, our only option is to drop them all in web, as per attached patch.
I strongly disagree with this approach though. It destroys information
that is correct, that we had be
Il 15/10/2012 10:13, Steven Bosscher ha scritto:
> > I disagree that it is valid. At least it is risky to consider it valid,
> > because a pass that simulates liveness might end up doing something
> > wrong because of that note. If simulation is done backwards, it doesn't
> > even require any int
On Mon, Oct 15, 2012 at 9:38 AM, Paolo Bonzini wrote:
>> I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
>> pseudo that isn't live and still be valid. Consider a simple example
>> like this:
>>
>> a = b + 3
>> // b dies here
>> c = a {REG_EQUAL b+3}
>>
>> The REG_EQUAL note i
Il 14/10/2012 22:59, Steven Bosscher ha scritto:
> On Sun, Oct 14, 2012 at 9:02 AM, Paolo Bonzini wrote:
>> Can we just simulate liveness for web, and drop REG_EQUAL/REG_EQUIV
>> notes that refer to a dead pseudo?
>
> I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
> pseudo
On Sun, Oct 14, 2012 at 11:25 PM, Eric Botcazou wrote:
>> I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
>> pseudo that isn't live and still be valid. Consider a simple example
>> like this:
>>
>> a = b + 3
>> // b dies here
>> c = a {REG_EQUAL b+3}
>>
>> The REG_EQUAL note
> I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
> pseudo that isn't live and still be valid. Consider a simple example
> like this:
>
> a = b + 3
> // b dies here
> c = a {REG_EQUAL b+3}
>
> The REG_EQUAL note is valid and may help optimization. Removing it
> just because
On Sun, Oct 14, 2012 at 9:02 AM, Paolo Bonzini wrote:
> Can we just simulate liveness for web, and drop REG_EQUAL/REG_EQUIV
> notes that refer to a dead pseudo?
I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
pseudo that isn't live and still be valid. Consider a simple examp
Il 13/10/2012 00:25, Steven Bosscher ha scritto:
> On Fri, Oct 12, 2012 at 11:16 PM, Jan Hubicka wrote:
>>> On Fri, Oct 12, 2012 at 10:44 PM, Jan Hubicka wrote:
1) computing liveness with REG_EQUAL included prior RD that means a lot
of shuffling of REG_DEAD notes
>>>
>>> I was alre
On Fri, Oct 12, 2012 at 11:16 PM, Jan Hubicka wrote:
>> On Fri, Oct 12, 2012 at 10:44 PM, Jan Hubicka wrote:
>> > 1) computing liveness with REG_EQUAL included prior RD that means a lot
>> > of shuffling of REG_DEAD notes
>>
>> I was already working on a patch for this. I'll send it here lat
> On Fri, Oct 12, 2012 at 10:44 PM, Jan Hubicka wrote:
> > 1) computing liveness with REG_EQUAL included prior RD that means a lot
> > of shuffling of REG_DEAD notes
>
> I was already working on a patch for this. I'll send it here later tonight.
Great, thanks! This is probably most sensibl
On Fri, Oct 12, 2012 at 10:44 PM, Jan Hubicka wrote:
> 1) computing liveness with REG_EQUAL included prior RD that means a lot
> of shuffling of REG_DEAD notes
I was already working on a patch for this. I'll send it here later tonight.
Ciao!
Steven
> On Fri, Oct 12, 2012 at 9:43 PM, Jan Hubicka wrote:
> > Hi,
> > I finally tracked down twolf misoptimization triggered by my loop-unroll.c
> > changes. It has turned out to be semi-latent wrong code issue in webizer.
> > What happens is:
> >
> > 1) gcse.c drop REG_EQUAL note on the induction va
Hi,
On your test case, I have this:
Web oldreg=72 newreg=111
Web oldreg=72 newreg=112
Web oldreg=72 newreg=139
Web oldreg=72 newreg=145
Web oldreg=72 newreg=151
Web oldreg=72 newreg=157
Web oldreg=72 newreg=163
Web oldreg=72 newreg=169
--- t.c.182r.loop2_done 2012-10-12 22:23:59.0 +0200
On Fri, Oct 12, 2012 at 9:43 PM, Jan Hubicka wrote:
> Hi,
> I finally tracked down twolf misoptimization triggered by my loop-unroll.c
> changes. It has turned out to be semi-latent wrong code issue in webizer.
> What happens is:
>
> 1) gcse.c drop REG_EQUAL note on the induction variable
> 2) lo
On 2012.10.12 at 21:43 +0200, Jan Hubicka wrote:
> I finally tracked down twolf misoptimization triggered by my loop-unroll.c
> changes. It has turned out to be semi-latent wrong code issue in webizer.
> What happens is:
>
> 1) gcse.c drop REG_EQUAL note on the induction variable
> 2) loop optimi
Hi,
I finally tracked down twolf misoptimization triggered by my loop-unroll.c
changes. It has turned out to be semi-latent wrong code issue in webizer.
What happens is:
1) gcse.c drop REG_EQUAL note on the induction variable
2) loop optimizer unrolls the loop enabling webizer to cleanup
3) webiz
67 matches
Mail list logo
