reorg.c messes up register liveness info by deleting its own placeholder insns. Further analysis with forthcoming patch. First seen in 3.2.x. On 3.3-branch, 3.4-branch the problem is exposed by the to-be-attached test-case but it's hidden on trunk.
Created attachment 6226 [details] executable test-case Will be committed as gcc.c-torture/execute/pr15296.c
<URL:http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00246.html>
Subject: Bug 15296 CVSROOT: /cvs/gcc Module name: gcc Changes by: hp@gcc.gnu.org 2004-05-07 03:20:25 Modified files: gcc : ChangeLog reorg.c Log message: PR optimization/15296 * reorg.c (fill_simple_delay_slots): Use next_real_insn when getting last consecutive label at a branch. (relax_delay_slots): Similar, near top of loop. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3593&r2=2.3594 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/reorg.c.diff?cvsroot=gcc&r1=1.93&r2=1.94
Subject: Bug 15296 CVSROOT: /cvs/gcc Module name: gcc Changes by: hp@gcc.gnu.org 2004-05-07 03:22:34 Modified files: gcc/testsuite : ChangeLog Log message: PR optimization/15296 * gcc.c-torture/execute/pr15296.c: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3723&r2=1.3724
Committed on trunk, awaiting resolution for 3.3 and 3.4 branches.
(In reply to comment #2) > <URL:http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00246.html> This is OK for gcc-3_3-branch.
Subject: Bug 15296 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_3-branch Changes by: hp@gcc.gnu.org 2004-05-26 13:19:22 Modified files: gcc : ChangeLog reorg.c Log message: PR optimization/15296 * reorg.c (fill_simple_delay_slots): Use next_real_insn when getting last consecutive label at a branch. (relax_delay_slots): Similar, near top of loop. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.984&r2=1.16114.2.985 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/reorg.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.82.4.1&r2=1.82.4.2
Subject: Bug 15296 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_3-branch Changes by: hp@gcc.gnu.org 2004-05-26 13:24:07 Modified files: gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: pr15296.c Log message: PR optimization/15296 * gcc.c-torture/execute/pr15296.c: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.375&r2=1.2261.2.376 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/pr15296.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.10.1
Subject: Re: [3.4 only] Delayed branch scheduling causing invalid code on cris-* On Wed, 26 May 2004, pinskia at gcc dot gnu dot org wrote: > What |Removed |Added > ---------------------------------------------------------------------------- > Known to fail|3.3.3 3.4.0 |3.4.0 > Known to work|3.5.0 |3.5.0 3.3.3 No, 3.3.3 is still known to fail. brgds, H-P
I also removed 3.5.0 from "known to work" since it isn't released: we can't "know" anything about it. Please don't change anything.
Maybe I misunderstand the use of known to fail, since I thought your patch fixed it in 3.3.4 since you applied it there, likewise for 3.5.0 as that is what the known to fail is for. Yes I did mess up by including 3.3.3 that should have been 3.3.4 woops but you can test 3.5.0 but you said it was fixed by your patch already, there might be a different bug which effects the now merged tree-ssa but that should be a different bug as none of the tree-ssa merge touched this code.
I really think so. How can you *know* that the test-case succeeds in 3.5.0 when until it's released? These "known to" fields are an indication from a user, not a statement from a developer: that's what "target milestone" is for. With your interpretation one of "known to" and "target milestone" fields would be redundant, and there would not be a field to specify what *released* versions are known to work/fail. The "reported against" is just the first spotting. Now please *do not* change it again without proper references. If you think my interpretation is incorrect, do take it up on gcc@ (please CC: me, I'm behind on the list reading) and/or point me to the proper documentation. I can't find anything specific for the "known to" fields at <URL:http://gcc.gnu.org/management.html> or <URL:http://www.bugzilla.org/docs/html/> (but target milestone is documented).
All of us bugzilla people quite frequently use the known-to-{work,fail} fields to fill which _branches_ work. For example, if something is known to fail with the 3.4.0 release but to be fixed on mainline, then 3.5.0 will be listed in the known-to-work field. This actually makes some sense, since if it is fixed on the mainline, then there needs to be a testcase in the testsuite which (at least ideally) should prevent this problem from re-appearing on mainline, thus guaranteeing that the actual 3.5.0 release will also work. This procedure may not be documented, but us bugzilla guys all seem to use it. W.
Subject: Re: [3.4 only] Delayed branch scheduling causing invalid code on cris-* On Wed, 26 May 2004, bangerth at dealii dot org wrote: > All of us bugzilla people quite frequently use the known-to-{work,fail} > fields to fill which _branches_ work. For example, if something is > known to fail with the 3.4.0 release but to be fixed on mainline, then > 3.5.0 will be listed in the known-to-work field. This actually makes some > sense, since if it is fixed on the mainline, then there needs to be a > testcase in the testsuite which (at least ideally) should prevent this > problem from re-appearing on mainline, thus guaranteeing that the actual > 3.5.0 release will also work. You may think it makes sense, but it is still confusing: consider the bug re-appearing; perhaps because the fix was reverted. Then later, someone looks at bugzilla to choose a "safe" version regarding the bug. That person would be fooled by the assertion that a bug is fixed in a version where the testcase provingly fails. Hence "known to work/fail" should only be used for released versions. That is, unless extra work is done at the release (or for any change that could possibly affect the "known to" facts), checking the "known to work" fields, adjusting them when necessary. Besides, how do you compare the "known to work" field with the "target version" field? What about the apparent redundancy? > > This procedure may not be documented, but us bugzilla guys all seem to use > it. Which doesn't make it correct or any less confusing. Anyway, if you stand by that usage, please document it for the benefit of everyone else. I'd like to be able to explain the person reporting the bug to me (or anyone, for example a manager), without refering to folklore. 1/2 :-) brgds, H-P
Subject: Re: [3.4 only] Delayed branch scheduling causing invalid code on cris-* On Wed, 26 May 2004, Hans-Peter Nilsson wrote: > Besides, how do you compare the "known to work" field with the > "target version" field? What about the apparent redundancy? Sorry, should've been "target milestone". brgds, H-P
If a patch is reverted, the PR must be reopened anyway, and the person doing so should ideally also adjust the fields that this changes. Note that bugs should really not re-appear in other circumstances, since patches are supposed to come with testcases. The target milestone is always the first upcoming version on the first active branch on which the bug appears, unless the release manager for that branch decides that this bug can't be fixed for this release. Note that the target milestone describes an _intent_: "we'd like to have this bug fixed by that version". The known-to-work field describes a _fact_: "This bug has been tested against these versions/branches and has been verified to trigger/to not trigger the bug there". Thus, the known-to-* fields are not meant to indicate something to a user, but rather to fellow developers, namely where we have checked the bug and for example whether a patch has to be applied to the 3.4 or 3.3 branch as well since the bug exists there as well. If we have verified that the bug doesn't exist there, that means that the developer does not have to spend cycles on re-checking this. I understand that the fact that we also use branch-names and not only release names in these fields can in a few occasions be confusing. However, I cannot imagine a significant number of cases where us setting the known-to-work field for branch x.y will be wrong for release x.y because the bug has reappeared. I believe that these should be very infrequent cases, and that we are much better served if we use this field to indicate where people have tested against a certain bug. W.
Subject: Re: [3.4 only] Delayed branch scheduling causing invalid code on cris-* On Wed, 26 May 2004, bangerth at dealii dot org wrote: > Note > that the target milestone describes an _intent_: "we'd like to have this > bug fixed by that version". The known-to-work field describes a _fact_: > "This bug has been tested against these versions/branches and has been > verified to trigger/to not trigger the bug there". Well, that's what I've been trying to say... The known-to fields should state facts, but your current usage does not follow the intent you state: when you write release identifiers rather than branch identifiers, it can't be a *fact* until the release is done. Can you consider e.g. writing 3.3-branch, not 3.3.4 in the known-to-work field? It doesn't have to be an exact branch identifier as long as it's unique. Perhaps also consider using the known-to-fail field for releases only? If not, you'd have to iterate over all bugs to fix known-to-fail fields for bugs that "disappear" for that field to state a fact! > I understand that the fact that we also use branch-names and not only > release names in these fields can in a few occasions be confusing. The problem seems to be that you use non-unique terms in those fields: your refer to the 3.3 branch as 3.3.4, which will hopefully be the name of a release as well. Still, please document whatever usage of yours, supposedly in management.html. Maybe I should open a bug report for that? (No, *I* don't want to document it since I still consider this usage confusing if not wrong, if nothing else than that it's easy to mistakenly remove a "known to fail" field for a release known to fail (QED). At least until you stop using the same name for releases as branches.) brgds, H-P
I understand your reasoning, though I fail to see where it will actually bite us in practice. The fact that we don't document could be improved, though. Please open a a PR for this, and either copy the contents of our discussion there, or put a link to this PR. I myself will now have to leave for a nice Memorial Day weekend :) W.
> I fail to see where it will actually bite us in practice. Lack of imagination? :-) Consider the time put into this issue: enough evidence of a problem, methinks. 8-) > Please open a a PR for this bug 15669 (gotta love those generated links)
Mark, there is a proposed patch for this which already went into mainline and 3.3 as far as I can see. Is it OK for 3.4 as well?
This patch is OK for 3.4.1; please apply it.
See Comment #20.
Subject: Bug 15296 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: giovannibajo@gcc.gnu.org 2004-06-13 01:58:58 Modified files: gcc : ChangeLog reorg.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: pr15296.c Log message: PR rtl-optimization/15296 * reorg.c (fill_simple_delay_slots): Use next_real_insn when getting last consecutive label at a branch. (relax_delay_slots): Similar, near top of loop. PR rtl-optimization/15296 * gcc.c-torture/execute/pr15296.c: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.498&r2=2.2326.2.499 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/reorg.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.90.4.1&r2=1.90.4.2 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.205&r2=1.3389.2.206 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/pr15296.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.18.1
Fixed by applying HP's patch to 3.4 branch.