[lworld] RFR: 8311219: [lworld] VM option "InlineFieldMaxFlatSize" cannot work well [v3]
Tobias Hartmann
thartmann at openjdk.org
Thu Sep 7 07:07:04 UTC 2023
On Thu, 7 Sep 2023 03:56:37 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:
> This change seems extended the buffer check for users besides InlineType, didn't it?
My change does two things:
(1) Remove the `u->Opcode() != Op_Return || !tf()->returns_inline_type_as_fields()` condition because if the user would be a ReturnNode and we would return as fields, the InlineTypeNode should be buffered.
(2) For InlineType users, never require buffering because the InlineType user will be replaced by its oop as well (detailed explanation below).
> So if the current InlineTypeNode is used in a method call which expects the InlineType is scalarized as arguments/return, is the buffer for the InlineType still needed?
No, it would not be needed but the InlineTypeNode would not be connected to the call anymore at this point. Scalarization of arguments/returns happens much earlier, we would already have wired the field values directly to the call. But note that the field value itself could be an InlineTypeNode and **that one** would need to be buffered.
> Conversely, if an InlineType is a non-flattened field of another InlineType, why is the buffer not needed although the user InlineType will be replaced with its oop? Per my understanding, the oop is the class instance of the InlineType, which also contains the fields.
It's not needed because we will replace the user separately and then assert that it's buffered (if required). It might well be that the user InlineType does not need to be allocated (it might itself be a flat field value of another InlineType node or something). Asserting that a non-flattened field value is always buffered "itself" is too strong because there can be cases where neither the holder nor the field value need to be buffered. It's sufficient to assert buffering for the InlineTypeNode that has a non-InlineType user.
> With another thinking, I assumed maybe all the InlineType have been scalarized to their users if it can be before this final replacement (i.e. replacement with its oop). So for almost all cases, the remaining InlineTypeNode must be buffered at this point, right?
Ideally yes but there will be chains of InlineTypeNodes and there is no need for the parent nodes to be buffered when their users are going to be replaced anyway. Also, there's an issue with PhiNodes not being properly replaced (see `TODO 8302217`).
I run this through all our testing and the assert never triggered.
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/888#issuecomment-1709587001
More information about the valhalla-dev
mailing list