RFR: 8341411: C2: remove slice parameter from GraphKit::make_load() and GraphKit::store_to_memory()

[Roland Westrelin]

On Fri, 1 Nov 2024 14:50:31 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

> This patch removes the address type from `GraphKit::make_load()` and `GraphKit::store_to_memory()`
> 
> As https://github.com/openjdk/jdk/pull/21303 introduced asserts that check that the address type agrees with  `C->get_alias_index(_gvn.type(adr)->isa_ptr()`, passing the address type is redundant and it can be computed internally from the address.

src/hotspot/share/gc/shared/c2/barrierSetC2.cpp line 220:

> 218:       load = kit->gvn().transform(load);
> 219:     } else {
> 220:       load = kit->make_load(control, adr, val_type, access.type(), adr_type, mo,

No similar change to `BarrierSetC2::store_at_resolved()`?

src/hotspot/share/opto/graphKit.cpp line 1561:

> 1559:                           bool unsafe,
> 1560:                           uint8_t barrier_data) {
> 1561:   assert(adr_idx == C->get_alias_index(_gvn.type(adr)->isa_ptr()), "slice of address and input slice don't match");

This assert (and the other one in `store_to_memory`) were added because there are 2 ways to compute the slice for a memory operation. One is from `_gvn.type(adr)->isa_ptr()`. The other is from `C->alias_type(field)->adr_type()` in case of fields accesses (see `Parse::do_get_xxx()` and `Parse::do_put_xxx()`). They should give the same result but in one bug we ran into that wasn't the case (thus the assert). I don't think we want to remove this assert entirely but rather push it up the call chain maybe to `BarrierSetC2::store_at_resolved()`/`BarrierSetC2::load_at_resolved` or all the way to where `C->alias_type(field)->adr_type()` is called.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21834#discussion_r1830926706
PR Review Comment: https://git.openjdk.org/jdk/pull/21834#discussion_r1830901217