On Wed, Mar 13, 2019 at 10:08:09PM -0400, Jason Merrill wrote: > > The following testcase ICEs since my recent cxx_eval_loop_expr changes. > > The problem is that the Forget saved values of SAVE_EXPRs. inside of the > > loop can remove SAVE_EXPRs from new_ctx.values and if that is the last > > iteration, we can also do the loop at the end of function (which has been > > added there mainly to handle cases where the main loop breaks earlier) > > and new_ctx.values->remove ICEs because *iter is already not in the table. > > It shouldn't ICE, remove_elt_with_hash is documented to do nothing if the > element is not in the table. > > Does this untested patch fix the test?
This changed in https://gcc.gnu.org/ml/gcc-patches/2000-02/msg00988.html Before that change, it has been documented that: "Naturally the hash table must already exist." That patch changed it to do slot = htab_find_slot (htab, element, 0); if (*slot == EMPTY_ENTRY) return; but htab_find_slot actually never returns *slot == EMPTY_ENTRY if called with insert 0: + if (!insert) + return NULL; It can return *slot == EMPTY_ENTRY only if insert is non-zero, and otherwise it must be (for both cases) a non-deleted non-empty entry equal to the one we are looking for. Thus, I believe what is documented since that patch has never worked and we are wasting cycles (if not optimized out) testing something impossible. So, I think our options are either to make it work, like below (untested), or remove those ifs altogether and put back the "Naturally the hash table must already exist." comment. 2019-03-14 Jason Merrill <ja...@redhat.com> Jakub Jelinek <ja...@redhat.com> * hash-table.h (remove_elt_with_hash): Return if slot is NULL rather than if is_empty (*slot). * hashtab.c (htab_remove_elt_with_hash): Return if slot is NULL rather than if *slot is HTAB_EMPTY_ENTRY. --- gcc/hash-table.h.jj 2019-01-01 12:37:17.446970209 +0100 +++ gcc/hash-table.h 2019-03-14 08:48:18.861131498 +0100 @@ -940,7 +940,7 @@ hash_table<Descriptor, Allocator> ::remove_elt_with_hash (const compare_type &comparable, hashval_t hash) { value_type *slot = find_slot_with_hash (comparable, hash, NO_INSERT); - if (is_empty (*slot)) + if (slot == NULL) return; Descriptor::remove (*slot); --- libiberty/hashtab.c.jj 2019-01-01 12:38:46.868503025 +0100 +++ libiberty/hashtab.c 2019-03-14 08:47:49.172612001 +0100 @@ -725,7 +725,7 @@ htab_remove_elt_with_hash (htab_t htab, PTR *slot; slot = htab_find_slot_with_hash (htab, element, hash, NO_INSERT); - if (*slot == HTAB_EMPTY_ENTRY) + if (slot == NULL) return; if (htab->del_f) Jakub
