This is the mail archive of the gcc-patches@gcc.gnu.org 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,
> 	* expmed.c (expand_mult_const): Force operand to constant.
> 	* rs6000.c (rs6000_emit_set_const, rs6000_emit_set_long_const): Add
> 	copy_rtx to arguments.

I think this is much better; OK for mainline.

> I am attaching the updated patch (this time with changelog ;).  I
> don't think the code would be more effecient (rather we would end up
> generating more garbage RTL and CSE will put subregs back), but it
> is probably all down in the noise, so I don't think it matters.

I'd be interested to see if you could investigate this further as
I really think we'll generate better code.  Imagine if this line
wasn't in expand_mult_const, we'd re-use a memory again and again,
or if this was a SUBREG to re-interpret a floating point constant
as an integer, etc...  It's the role of CSE, to spot such common
sub-expressions and factor them out of repeated code, so I doubt it
would ever decide to replace multiple uses of a "pseudo" (which has
the cheapest rtx_cost you can get) with SUBREGs which may require
code generation.  And let's not forget all the other possible values
of op0 which are neither REGs, SUBREGs or MEMs.  It's because CSE,
GCSE and cselib only track the values of pseudos, the RTL expanders
try and place repeated expressions in their own registers, otherwise
they wouldn't be able to detect that "SUBREG:SI (REG:SF)" was used
repeatedly.

There is the possibility that fwprop may make some poor/good un-CSE
decisions, but without a concrete example to stare at or some resulting
assembly to ponder over, this is all theoretical.  As for Junk RTL,
you're the man with the extensive memory checking scripts.


Thanks for all of your good work detecting and correcting RTL sharing
violations.  Historically, limited sharing has been "acceptable" (i.e.
hasn't broken anything), but the current rule is that we shouldn't
share trees or RTL, and having verifiers that enforce this is a
good thing.  However, its often the case that there's a better way
to fix a sharing problem than copy_rtx.  Blind use of copy_rtx can
itself leak junk RTL, and once in the source it's difficult to verify
later that we can safely remove a call to copy_rtx.

[As a software architect I think there are significant memory savings
that can be realized through canonicalization and *enforced* sharing
of immutable directed-graph data structures, but that's not the design
that GCC follows, and therefore all sharing violations are bugs].

Thanks again for fixing these issues,

Roger
--


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