On Fri, 20 Nov 2020 22:39:19 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ci/ciObjectFactory.java
>> line 82:
>>
>>> 80: public GrowableArray<ciSymbol> symbols() {
>>> 81: Address addr = getAddress().addOffsetTo(symbolsField.getOffset());
>>> 82: return GrowableArray.create(addr, ciSymbolConstructor);
>>
>> It's unclear to me why these two changes were needed. Don't they produce the
>> same result before and after?
>
> When changing the fields from `GrowableArray<..>*` to `GrowableArray<..>` I
> looked around and found another use of an embedded `GrowableArray` in
> `InlineTree.java` and took my cues from that. If you're certain these
> approaches are semantically identical I can drop these changes, but as I'm
> not exactly sure how to verify and test this I went the safe(?) route of
> leaning on prior art here.
Ah, I see now. I didn't pick up on the change from a pointer type to an
embedded type. Yes, I think your changes are correct and necessary. Rather than
plucking the pointer value out of the field, you are now computing the address
of the field, which seems like the right thing to be doing when changing the
field from a pointer type to an embedded type.
Consider the SA changes reviewed. I'm not reviewing any of the hotspot changes.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1346
