ets.nullcheck BCO optimizations-arkcompiler_runtime

已关闭

ets.nullcheck BCO optimizations #8470

alexanderandreevmir创建于 2025年11月25日关闭于 2月14日

2025年11月25日创建

Due to the lack of ChecksElimination and the lack of ability to handle NullCheck in BCO pipeline properly, the ets.nullcheck is made a simple intrinsic in BCO mode, so it lacks optimization and not handled by the BCO's regalloc properly

.function std.core.Object test.A.getx() <static, access.function=public> {
ldstatic.obj test.A.x
sta.obj v0 // redundant
lda.obj v0 // redundant
ets.nullcheck
lda.obj v0 // redundant
return.obj
}

openharmony_ci成员

2025年11月25日评论：

感谢提交Issue！关于Issue的交互操作，请访问OpenHarmony社区支持命令清单。如果有问题，请联系 [@weng-changcheng](https://gitcode.com/weng-changcheng) [@godmiaozi](https://gitcode.com/godmiaozi) [@liyiming13](https://gitcode.com/liyiming13) [@lijin1039](https://gitcode.com/lijin1039) [@Prof](https://gitcode.com/Prof) [@dingding5](https://gitcode.com/dingding5) [@v-cherkashin](https://gitcode.com/v-cherkashin) 。如果需要调整订阅PR、Issue的变更状态，请访问链接。

Thanks for submitting the issue. For more commands, please visit OpenHarmony Command List. If you have any questions, please refer to committer gitcode for help. If you need to change the subscription of a Pull Request or Issue, please visit the link.

openharmony_ci成员

2025年11月25日添加了label：waiting_for_assign

nehrby

2025年11月27日评论：

The ETS source:
class A {
static x: Object = new String
getx():Object { return A.x; }
}
This source is compiled to .abc, in which A.getx() is disassembled exactly as above. I expected, that ark_aot will do some optimization on that .abc. However, ir dumps show no difference: code is the same on IrBuilder pass and Codegen pass.
For IrBuilder:
Method: std.core.Object test3.A::getx(test3.A) 0x7ffff081a800

BB 1
prop: start, bc: 0x00000000
hotness=0
0.ref Parameter arg 0
1. SafePoint inlining_depth=0 bc: 0x00000000
succs: [bb 0]

BB 0 preds: [bb 1]
prop: bc: 0x00000000
hotness=0
2. SaveState inlining_depth=0 -> (v3) bc: 0x00000000
3.ref LoadAndInitClass 'test3.A' v2 -> (v4) bc: 0x00000000
4.ref LoadStatic 443 test3.A.x v3 -> (v5, v7, v6, v5) bc: 0x00000000
5. SaveState v4(vr0), v4(ACC), inlining_depth=0 -> (v6) bc: 0x00000007
6.ref NullCheck v4, v5 bc: 0x00000007
7.ref Return v4 bc: 0x0000000b
succs: [bb 2]

BB 2 preds: [bb 0]
prop: end, bc: 0x0000000c
hotness=0

For Codegen:
Method: std.core.Object test3.A::getx(test3.A) 0x7ffff081a800

BB 1
prop: start, bc: 0x00000000
hotness=0
succs: [bb 0]

BB 0 preds: [bb 1]
prop: bc: 0x00000000
hotness=0
2. SaveState inlining_depth=0 -> (v3) bc: 0x00000000
3.ref LoadAndInitClass 'test3.A' v2 -> rax (v4) bc: 0x00000000
4.ref LoadStatic 443 test3.A.x v3(rax) -> rax (v5, v7, v6, v5) bc: 0x00000000
5. SaveState v4(vr0), v4(ACC), inlining_depth=0 -> (v6) bc: 0x00000007
6.ref NullCheck v4(rax), v5 bc: 0x00000007
7.ref Return v4(rax) bc: 0x0000000b
succs: [bb 2]

BB 2 preds: [bb 0]
prop: end, bc: 0x0000000c
hotness=0

Ivanov Mikhail

2025年12月3日评论：

Patch to start with edbe02ea9e8d45f0839c161bc78223ef.patch

nehrby

2025年12月8日关联了pull request：ets.nullcheck BCO optimizations

nehrby

2025年12月10日评论：

The PR passes all tests except the unit test Atomics base_operation_biguint, function testSubBigUint, which fails as follows:

RangeError: Index out of bounds
at std.core.RangeError. (Errors.ets:82:0)
at std.core.RangeError. (Errors.ets:81:0)
at std.concurrency.Atomics.sub (AsyncLock.ets:1:0)
at base_operation_biguint.ETSGLOBAL.testSubBigUint (base_operation_biguint.ets:151:0)
at base_operation_biguint.%lambda-lambda_invoke-7.invoke0 (base_operation_biguint.ets:1:0)
at std.testing.arktest.ArkTest.run (arktest.ets:335:0)
at std.testing.arktest.ArkTestsuite.lambda_invoke-76 (arktest.ets:450:0)
at std.testing.%lambda-lambda_invoke-76.invoke1 (arktest.ets:1:0)
at std.core.Lambda1.invoke3 (Function.ets:422:0)
at std.core.Array.forEach (Array.ets:1207:0)
at std.testing.arktest.ArkTestsuite.run (arktest.ets:448:0)
at base_operation_biguint.ETSGLOBAL.main (base_operation_biguint.ets:194:0)

nehrby

2025年12月12日评论：

The .abc files are the same for PR and the mainstream code. However, it is not so simple to find which difference in etsstdlib code produces the error. Atomics::sub code (in Panda asm) is the same in both library versions. The code for Atomics::sub in Atomics.ets starts with :
if (index < 0 || index >= typedArray.length) {
throw new RangeError("Index out of bounds")
However, if we change the conditions order
if (index >= typedArray.length || index < 0) {
throw new RangeError("Index out of bounds")
the test will pass.
The Panda assembler, of course, shows different code, but its both versions seem correct:
.function std.core.BigInt std.concurrency.Atomics.sub({Uescompat.BigInt64Array,escompat.BigUint64Array} a0, f64 a1, std.core.BigInt a2) <static, access.function=public> {
fldai.64 0x0
fcmpl.64 a1
jgtz jump_label_0
ets.ldobj.name a0, etsstdlib.%%union_prop-BigInt64Array|BigUint64Array.length
i32tof64
fcmpg.64 a1
jgtz jump_label_1
jump_label_0:
lda.str "Index out of bounds"

.function std.core.BigInt std.concurrency.Atomics.sub({Uescompat.BigInt64Array,escompat.BigUint64Array} a0, f64 a1, std.core.BigInt a2) <static, access.function=public> {
ets.ldobj.name a0, etsstdlib.%%union_prop-BigInt64Array|BigUint64Array.length
i32tof64
fcmpg.64 a1
jlez jump_label_0
fldai.64 0x0
fcmpl.64 a1
jlez jump_label_1
jump_label_0:
lda.str "Index out of bounds"

There seems to be no other library dependencies except etsstdlib.%%union_prop-BigInt64Array|BigUint64Array.length, but the PR's fix seems to have no effect on this...

nehrby

2025年12月15日评论：

Further localization of the error: if we prohibit the optimization for std.concurrency.AsyncLockMode::values(), the test passes. The code of the method:
.function std.concurrency.AsyncLockMode[] std.concurrency.AsyncLockMode.values() <static, access.function=public> {
ldstatic.obj std.concurrency.AsyncLockMode.#ItemsArray
sta.obj v0 // removed when the optimization is on
lda.obj v0 // removed when the optimization is on
ets.nullcheck
lda.obj v0 // removed when the optimization is on
return.obj
}

The removals seem reasonable and should not produce any bad side effect...

nehrby

2025年12月19日评论：

The minimum test, on which the issue is reproducible is

const BYTE_LENGTH = 128

let ab = new ArrayBuffer(BYTE_LENGTH)
let biguint64array = new BigUint64Array(ab)

function main() {
Atomics.exchange(biguint64array, 0, 12n)
Atomics.sub(biguint64array, 0, 12n)
}

nehrby

2025年12月19日评论：

Both Atomics.exchange and Atomics.sub start with checking the index against the array length. In Atomics.exchange the length returned is 16 (correct). However, in Atomics.sub the length returned is 0, which leads to the RangeError exception.
Debugging showed that in the second case we get wrong getter address returned by LookupGetterByNameShortEntrypoint. The reason is that we have a cached entry, which, however, is somewhat wrong. Why it is wrong - it requires further investigation.

Ivanov Mikhail

2025年12月24日评论：

Filename: runtime_core/static_core/plugins/ets/runtime/ets_stubs-inl.h
Function: GetAccessorByName

Change: bool cacheExists = res != nullptr && ((resUint & METHOD_FLAG_MASK) == 1);
To: bool cacheExists = res != nullptr && ((resUint & METHOD_FLAG_MASK) == 1) && address == entry->pc;

nehrby

2025年12月24日关联了pull request：Interpreter bugfix for not checking pc in cache entry

nehrby

2025年12月24日关联了pull request：ets.nullcheck BCO optimizations

openharmony_dcp成员

2月14日评论：

开发自提单，长时间未处理，经领域审视，issue先关闭

openharmony_dcp成员

2月14日 issue状态由待办的改变为已完成

openharmony_dcp成员

2月14日关闭了 issue