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

Roland Westrelin roland at openjdk.org
Tue Nov 12 10:21:48 UTC 2024


On Fri, 8 Nov 2024 09:16:03 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.
>
> theoweidmannoracle has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix asserts

Looks good to me.
I'm wondering it we can go further and in:

Node* LoadNode::make(PhaseGVN& gvn, Node* ctl, Node* mem, Node* adr, const TypePtr* adr_type, const Type* rt, BasicType bt, MemOrd mo,
                     ControlDependency control_dependency, bool require_atomic_access, bool unaligned, bool mismatched, bool unsafe, uint8_t barrier_data) {

remove the adr_type parameter (because it should be `gvn.type(adr)`). Something similar must be possible for `Store`. Then in the gc api:

  C2AccessValuePtr addr(adr, adr_type);
  C2ParseAccess access(this, decorators | C2_READ_ACCESS, bt, obj, addr);

would we even need to pass `adr_type`?
Do we want to file a bug about to, at least, give it a try?

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

Marked as reviewed by roland (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21834#pullrequestreview-2429164805


More information about the hotspot-dev mailing list