diff mbox series

cselib: Discard useless locs of preserved VALUEs [PR116627]

Message ID ZuILA6hGlnVQq5ws@tucnak
State New
Headers show
Series cselib: Discard useless locs of preserved VALUEs [PR116627] | expand

Commit Message

Jakub Jelinek Sept. 11, 2024, 9:26 p.m. UTC 
Hi!

remove_useless_values iteratively discards useless locs (locs of
cselib_val which refer to non-preserved VALUEs with no locations),
which in turn can make further values useless until no further VALUEs
are made useless and then discards the useless VALUEs.

Preserved VALUEs (something done during var-tracking only I think)
live in a different hash table, cselib_preserved_hash_table rather
than cselib_hash_table.  cselib_find_slot first looks up slot in
cselib_preserved_hash_table and only if not found looks it up in
cselib_hash_table (and INSERTs only into the latter), whereas preservation
of a VALUE results in move of a cselib_val from the latter to the former
hash table.

The testcase in the PR (apparently too fragile, it only reproduces on 14
branch with various flags on a single arch, not on trunk) ICEs, because
we have a preserved VALUE (QImode with (const_int 0) as one of the locs).
In a different BB SImode r2 is looked up, a non-preserved VALUE is created
for it, and the r13-2916 added code attempts to lookup also SUBREGs of that
in narrower modes, among those QImode, so adds to that SImode r2
non-preserve VALUE a new loc of (subreg:QI (value:SI) 0).  That SImode
value is considered useless, so remove_useless_value discards it, but
nothing discarded it from the preserved VALUE's loc_list, so when looking
something up in the hash table we ICE trying to derevence CSELIB_VAL
of the discarded VALUE.

I think we need to discuard useless locs even from the preserved VALUEs.
That IMHO shouldn't create any further useless VALUEs, the preserved
VALUEs are never useless, so we don't need to iterate with it, can do it
just once, but IMHO it needs to be done because actually
discard_useless_values.

The following patch does that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-09-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/116627
	* cselib.cc (remove_useless_values): Discard useless locs
	even from preserved cselib_vals in cselib_preserved_hash_table
	hash table.


	Jakub

Comments

Jakub Jelinek Sept. 12, 2024, 6:43 a.m. UTC | #1 
On Wed, Sep 11, 2024 at 11:26:27PM +0200, Jakub Jelinek wrote:
> I think we need to discuard useless locs even from the preserved VALUEs.
> That IMHO shouldn't create any further useless VALUEs, the preserved
> VALUEs are never useless, so we don't need to iterate with it, can do it
> just once, but IMHO it needs to be done because actually
> discard_useless_values.
> 
> The following patch does that.

Note, I've verified the patch on x86_64-linux cc1plus didn't change
anything at all on the resulting cc1plus binary (compared it to one
bootstrapped without this patch with the patch later applied and
make cc1plus done in the stage3, the only change in the binary was
16 bytes of executable_checksum).

	Jakub
diff mbox series

Patch

--- gcc/cselib.cc.jj	2024-04-26 11:46:54.960269768 +0200
+++ gcc/cselib.cc	2024-09-11 10:54:05.018242593 +0200
@@ -751,6 +751,11 @@  remove_useless_values (void)
       }
   *p = &dummy_val;
 
+  if (cselib_preserve_constants)
+    cselib_preserved_hash_table->traverse <void *,
+					   discard_useless_locs> (NULL);
+  gcc_assert (!values_became_useless);
+
   n_useless_values += n_useless_debug_values;
   n_debug_values -= n_useless_debug_values;
   n_useless_debug_values = 0;