This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix RTL sharing found during PPC bootstrap

> Hi Jan,
> On Sat, 16 Dec 2006, Jan Hubicka wrote:
> > this patch brings PPC into bootstrapland with RTL sharing verifier.
> > The first part is fixing expand_synth_mult that is called from
> > unroll-loops and producing shared SUBREGS.
> Instead of a calling copy_rtx on op0 repeatedly in expand_mult_const
> how about generalizing the code:
>   /* Avoid referencing memory over and over.
>      For speed, but also for correctness when mem is volatile.  */
>   if (MEM_P (op0))
>     op0 = force_reg (mode, op0);
> and instead always call force_reg on op0.  I disagree that RTL

We can definitly do that.  We would end up producing slightly more RTL
as CSE/forward propagation will re-insert subreg in.  But I will re-test
with this fix.
> expansion assumes that sharing is OK, rather I think the problem
> is that RTL expansion often assumes that it's operands are pseudos.

Well, if RTL expansion didn't assumed sharing is OK, we would not need
unsare_rtl and unshare_all_rtl passes at first place.  The shared RTL is
introduced for a lot more stuble reasons than just subregs in operands.
As Iant explained it to me, historically sharing was allowed and used,
later declared invalid, but apparently no one bothered to write verifier
for it.

There seems to be a lot of confusion related to sharing, many of the
problems are obviously assumptions done by various GCC developers about
sharing that are simply in conflict...
We will likely keep finding new bugs in this area for a while :(
> This no only prevent the sharing issue, but may generate more
> efficient code where by avoiding repeated evaluation of a general
> operand.
> I much prefer the alternate fix if it bootstraps and regression
> tests without problems.  And if you're planning on regularly
> contributing to GCC, you should remember to include a ChangeLog
> with your patch submissions :-)
Oops, I wrote it but probably just inserted the orignial file instead of
edited patch. Sorry for that.

> Roger
> --

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]