RFR: 8322694: C1: Handle Constant and IfOp in NullCheckEliminator

Aleksey Shipilev shade at openjdk.org
Wed Jan 3 12:25:49 UTC 2024


On Mon, 25 Dec 2023 15:43:52 GMT, Denghui Dong <ddong at openjdk.org> wrote:

> This patch added the support for Constant and IfOn in NullCheckEliminator to eliminate more null check.
> 
> testing: tier1-4 in progress

Nice corner case!

src/hotspot/share/c1/c1_Optimizer.cpp line 888:

> 886:     mark_visitable(instr);
> 887:     if (instr->is_pinned() || instr->can_trap() || (instr->as_NullCheck() != nullptr)
> 888:         || (instr->as_Constant() != nullptr && instr->as_Constant()->type()->is_object())) {

Is this just `instr->as_ObjectConstant() != nullptr`?

src/hotspot/share/c1/c1_Optimizer.cpp line 1206:

> 1204: void NullCheckEliminator::handle_Constant(Constant *x) {
> 1205:   ObjectType* ot = x->type()->as_ObjectType();
> 1206:   if (ot && ot->is_loaded()) {

Hotspot style guide insists we avoid implicit bool conversions. Check `ot != nullptr` explicitly.

src/hotspot/share/c1/c1_Optimizer.cpp line 1208:

> 1206:   if (ot && ot->is_loaded()) {
> 1207:     ObjectConstant* oc = ot->as_ObjectConstant();
> 1208:     if (!oc || !oc->value()->is_null_object()) {

Ditto, check `oc == nullptr`.

Now, the fact that `as_ObjectConstant` returns `nullptr` means this is not an _object constant_, but some other constant, right? I think this is similar to what other places in C1 do, so while awkward, this looks okay.

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

PR Review: https://git.openjdk.org/jdk/pull/17191#pullrequestreview-1801885674
PR Review Comment: https://git.openjdk.org/jdk/pull/17191#discussion_r1440392602
PR Review Comment: https://git.openjdk.org/jdk/pull/17191#discussion_r1440383548
PR Review Comment: https://git.openjdk.org/jdk/pull/17191#discussion_r1440391525


More information about the hotspot-compiler-dev mailing list