On 05/06/2013 10:27 AM, Steven Bosscher wrote:
On Mon, May 6, 2013 at 5:28 PM, Jeff Law wrote:
On 05/05/2013 04:18 AM, Steven Bosscher wrote:
On Fri, May 3, 2013 at 3:10 PM, Julian Brown wrote:
gcc/
* gcse.c (compute_ld_motion_mems): Invalidate non-simple mem refs
in REG_EQUAL notes.
Makes sense to me. Looking at REG_EQUAL notes in hash_scan_set is
something relatively new. Your patch fixes something we appear to have
overlooked.
I'd still like to see the before/after dumps. While I think the patch is
reasonable as well, those dumps should make it clearer to anyone looking at
this in the future what was going on -- particularly important since we
don't have an in-tree port which exhibits this problem.
The dumps are attached to the PR,
I must have missed that notification.
and I know what is going on: Paolo
and I added support for hashing REG_EQUAL notes, to recover most (if
not all) of the PRE/HOIST opportunities lost along with libcall notes.
But what's important here is that anyone be able to look at this issue
in the future and figure out what's going on.
Before that change, the worst that could happen would have been
incorrect REG_EQUAL notes. Now that values in notes are considered as
PRE/HOIST candidates, MEMs within notes have to be invalidated. The
patch fixes something Paolo and I simply overlooked.
Understood. My point is this needs to be clearer to folks other than
Paolo and yourself. For some folks, being able to look at the dumps and
implementation side by side along with the textual explanation really helps.
As the original author of most of the RTL PRE code it certainly wasn't
clear to me what was happening -- and that's an indication more
information was needed.
I never did much/anything with the load/store motion bits, so I wasn't
aware of the overall structure where we have items in the table, and
mark them as invalid. As far as I know load/store motion is the only
PRE code which allows such things in the tables at all. Now that I can
see the dumps & follow the load/store specific bits of the
implementation it's pretty clear now this patch is OK.
Jeff
