Re: [PATCH] Fix a bug in bt-load.c
On 11/24/14 20:56, Segher Boessenkool wrote: This caused ICEs on sh64. `min_cost' and `def' here are supposed to refer to the same element; removing it from the heap before asking the heap for the key doesn't work (and at the end of the loop here we will ask for min_key on an empty heap, which then does gcc_unreachable). Bootstrapped and tested on powerpc64-linux, but I doubt it exercises this code at all; only sh64 did ICE, and does not anymore. Okay for trunk? Segher 2014-11-24 Segher Boessenkool seg...@kernel.crashing.org gcc/ * bt-load.c (migrate_btr_defs): Get the key of a heap entry before removing it, not after. OK. Did sh64 ICE during a build, or was it during testing or something else? Trying to figure out if we need a distinct test in the suite or not. jeff
Re: [PATCH] Fix a bug in bt-load.c
On Tue, Nov 25, 2014 at 09:41:40AM -0700, Jeff Law wrote: On 11/24/14 20:56, Segher Boessenkool wrote: This caused ICEs on sh64. `min_cost' and `def' here are supposed to refer to the same element; removing it from the heap before asking the heap for the key doesn't work (and at the end of the loop here we will ask for min_key on an empty heap, which then does gcc_unreachable). Did sh64 ICE during a build, or was it during testing or something else? Trying to figure out if we need a distinct test in the suite or not. During libgcc build, pretty much all files. The libiberty fibheap code returns 0 for min_key on an empty heap; the new fibonacci_heap code ICEs. This bt-load code will always fail if there is any work to do, so I don't think any other test is needed :-) Segher
Re: [PATCH] Fix a bug in bt-load.c
On 11/25/14 10:26, Segher Boessenkool wrote: On Tue, Nov 25, 2014 at 09:41:40AM -0700, Jeff Law wrote: On 11/24/14 20:56, Segher Boessenkool wrote: This caused ICEs on sh64. `min_cost' and `def' here are supposed to refer to the same element; removing it from the heap before asking the heap for the key doesn't work (and at the end of the loop here we will ask for min_key on an empty heap, which then does gcc_unreachable). Did sh64 ICE during a build, or was it during testing or something else? Trying to figure out if we need a distinct test in the suite or not. During libgcc build, pretty much all files. The libiberty fibheap code returns 0 for min_key on an empty heap; the new fibonacci_heap code ICEs. This bt-load code will always fail if there is any work to do, so I don't think any other test is needed :-) Ok. Thanks. jeff
[PATCH] Fix a bug in bt-load.c
This caused ICEs on sh64. `min_cost' and `def' here are supposed to refer to the same element; removing it from the heap before asking the heap for the key doesn't work (and at the end of the loop here we will ask for min_key on an empty heap, which then does gcc_unreachable). Bootstrapped and tested on powerpc64-linux, but I doubt it exercises this code at all; only sh64 did ICE, and does not anymore. Okay for trunk? Segher 2014-11-24 Segher Boessenkool seg...@kernel.crashing.org gcc/ * bt-load.c (migrate_btr_defs): Get the key of a heap entry before removing it, not after. --- gcc/bt-load.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/bt-load.c b/gcc/bt-load.c index 3002b62..53c4db3 100644 --- a/gcc/bt-load.c +++ b/gcc/bt-load.c @@ -1434,8 +1434,8 @@ migrate_btr_defs (enum reg_class btr_class, int allow_callee_save) while (!all_btr_defs.empty ()) { - btr_def def = all_btr_defs.extract_min (); int min_cost = -all_btr_defs.min_key (); + btr_def def = all_btr_defs.extract_min (); if (migrate_btr_def (def, min_cost)) { all_btr_defs.insert (-def-cost, def); -- 1.8.1.4