This is the mail archive of the
mailing list for the GCC project.
Re: Fix PR middle-end/57370
- From: Easwaran Raman <eraman at google dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 30 Aug 2013 09:49:59 -0700
- Subject: Re: Fix PR middle-end/57370
- Authentication-results: sourceware.org; auth=none
- References: <CAPK5YPbNvp9g-9O=DUb06S3yNEvXiBrYeTbbsW2cDJuwd2_UXQ at mail dot gmail dot com> <CAPK5YPaXwtv6O41hgk4gdfdU0H2d=H3Ac5YUJsmYDQWLKc7VGg at mail dot gmail dot com> <CAPK5YPYXHtxs=+LpJNeZuKU+hmx80zjG1qQgeetxLt6qtGBSJA at mail dot gmail dot com> <CAFiYyc1_zfQzzJZQpTh+di=Q1RVrEMXnNtksTRb9UaGeDZCjvQ at mail dot gmail dot com> <CAPK5YPY4f8zr6O4h+FKs9_9Wn5-LRzBc0CQp742nQ9gF3bM0Sg at mail dot gmail dot com> <CAFiYyc2uR6G1UUJYGfa-s8Tz08b_cfw6cYJ9g8bG4qUZPEVcxw at mail dot gmail dot com> <CAPK5YPYZLSBNED3+7c802U=ixU2EUJhQu8yOzyHTy6P8czR=yQ at mail dot gmail dot com> <20130830162611 dot GR21876 at tucnak dot zalov dot cz>
On Fri, Aug 30, 2013 at 9:26 AM, Jakub Jelinek <firstname.lastname@example.org> wrote:
> On Fri, Aug 30, 2013 at 09:13:34AM -0700, Easwaran Raman wrote:
>> >> There are more similar failures reported in
>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
>> >> the updated patch there. Shall I send that for review? Apart from the
>> >> debug statement issue, almost all the bugs are due to dependence
>> >> violation because certain newly inserted statements do not have the
>> >> right UID. Instead of trying to catch all of them, will it be better
>> >> if I check if the stmt has a proper uid (non-zero if it is not the
>> >> first stmt) and assign a sensible value at the point where it is used
>> >> (find_insert_point and appears_later_in_bb) instead of where the stmt
>> >> is created? I think that would be less brittle.
>> > In the end all this placement stuff should be reverted and done as
>> > post-processing after reassoc is finished. You can remember the
>> > inserted stmts that are candidates for moving (just set a pass-local
>> > flag on them) and assign UIDs for the whole function after all stmts
>> > are inserted.
>> The problem is we need sane UID values as reassoc is happening - not
>> after it is finished. As it process each stmt in reassoc_bb, it might
>> generate new stmts which might have to be compared in
>> find_insert_point or appears_later_in_bb.
> A new stmt will be created with UID 0 and in case there is stmt movement,
> you could zero the UID during movement. Then, you can just special case
> UID zero when you are looking at UIDs. As on trunk/4.8 gsi_for_stmt is
> O(1), you can easily walk a couple of previous and/or later stmts and look
> for the first non-zero UID around it, and treat it as if the UID was
> previous non-zero UID + 0.5 or next non-zero UID - 0.5. And, if you see
> too many consecutive statements with UID 0 (more than some constant, 32?),
> just reassign UIDs to the whole bb. Or you could initially assign UIDs
> with some gaps, then keep filling those gaps and once you fill them,
> renumber again with new gaps.
Yes, this is pretty much what I was proposing. The current
implementation doesn't rely on UIDs being unique - they only have to
be monotonically non-decreasing. So, when a UID of 0 is encountered,
go up till a non-zero UID is found and then go down and assign this
non-zero UID. This effectively implies that each UID-0 stmt is visited
at most twice. I don't think we need to check if I see more than
certain number of UID-0 stmts and redo the entire BB.