RFR: 8326438: C2: assert(ld->in(1)->Opcode() == Op_LoadN) failed: Assumption invalid: input to DecodeN is not LoadN

[Roberto Castañeda Lozano]

On Thu, 21 Mar 2024 15:31:58 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

> The [`assert`](https://github.com/openjdk/jdk/blob/6f2676dc5f09d350c359f906b07f6f6d0d17f030/src/hotspot/share/opto/graphKit.cpp#L1567) added in [JDK-8310524](https://bugs.openjdk.org/browse/JDK-8310524) is too strong and may sometimes not hold due to the [GVN transformation in `LoadNode::make`](https://github.com/openjdk/jdk/blob/8cb9b479c529c058aee50f83920db650b0c18045/src/hotspot/share/opto/memnode.cpp#L973).
> 
> ### Changeset
> Remove the `assert`.
> 
> ### Testing
> N/A

Changes requested by rcastanedalo (Reviewer).

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

> 1564:     record_for_igvn(ld);
> 1565:     if (ld->is_DecodeN()) {
> 1566:       // Also record the actual load (LoadN) in case ld is DecodeN

This comment still reads as if we expect that `ld->in(1)` is necessarily a `LoadN`, which might mislead the reader. Could you extend it with a note that clarifies that in some corner cases `ld->in(1)` might be something else, e.g. a `Phi` like in this issue, but that is OK because it only means we might do unnecessary work during IGVN?

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

PR Review: https://git.openjdk.org/jdk/pull/18434#pullrequestreview-1954126431
PR Review Comment: https://git.openjdk.org/jdk/pull/18434#discussion_r1535166821