Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JIT] Backport 8318446: C2: optimize stores into primitive arrays by … #921

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kuaiwei
Copy link
Collaborator

@kuaiwei kuaiwei commented Jan 9, 2025

…combining values into larger store

Summary: include these patches for merge stores optimization
8318446: C2: optimize stores into primitive arrays by combining values into larger store
8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug"
8335390: C2 MergeStores: wrong result with Unsafe
8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
8331085: Crash in MergePrimitiveArrayStores::is_compatible_store()
8331252: C2: MergeStores: handle negative shift values
8331054: C2 MergeStores: assert failed: unexpected basic type after JDK-8318446 and JDK-8329555
8335392: C2 MergeStores: enhanced pointer parsing
8334342: Add MergeStore JMH benchmarks
8226411: C2: Avoid memory barriers around off-heap unsafe accesses
Fix is_ConI() query after port 8318446

Testing: CI/CD

Reviewers: zhuoren.wz, MaxXSoft

Issue: #920

…combining values into larger store

Summary: include these patches for merge stores optimization
 8318446: C2: optimize stores into primitive arrays by combining values into larger store
 8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug"
 8335390: C2 MergeStores: wrong result with Unsafe
 8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
 8331085: Crash in MergePrimitiveArrayStores::is_compatible_store()
 8331252: C2: MergeStores: handle negative shift values
 8331054: C2 MergeStores: assert failed: unexpected basic type after JDK-8318446 and JDK-8329555
 8335392: C2 MergeStores: enhanced pointer parsing
 8334342: Add MergeStore JMH benchmarks
 8226411: C2: Avoid memory barriers around off-heap unsafe accesses
 Fix is_ConI() query after port 8318446

Testing: CI/CD

Reviewers: zhuoren.wz, MaxXSoft

Issue: dragonwell-project#920
@sandlerwang
Copy link
Collaborator

LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件在 JDK-8335392 的 patch 中叫 test_no_overflow_int.cpp 而非 test_no_overflow.cppopenjdk/jdk@f3671be#diff-65c7ade01f9f7b974da5242a049166feed0da47c34f7656a357039b4842bc2b5

修改文件名的原因是什么呢?

@@ -312,6 +312,7 @@ bool LibraryCallKit::try_to_inline(int predicate) {
case vmIntrinsics::_equalsU: return inline_string_equals(StrIntrinsicNode::UU);

case vmIntrinsics::_toBytesStringU: return inline_string_toBytesU();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这行更改在 PR 描述中提到的所有 patch 中都没出现,且在上游最新的代码中也没出现。

参考 OpenJDK 中的相关提交:openjdk/jdk@eabb5cc#diff-1929ace9ae6df116e2fa2a718ed3924d9dae9a2daea454ca9a78177c21477aa3

return Status::make(def_store, cfg_status_for_pair(use_store, def_store));
}

static int round_down_power_of_2(uint value) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这部分看起来本来应该是在 src/hotspot/share/utilities/powerOfTwo.hpp 里的,但 dw11 没这个文件。

此外,这部分的语义和原有的相比缺了一个 assert


#ifndef PRODUCT
bool is_trace_basic() const {
return _trace;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看上去这里和 compilerDirectives.hppTraceMergeStores做了简化处理,改成了统一的 boolean flag。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的,简化为用TraceMergeStores来控制trace,原patch中加了多个标志位,需要在compilerOracle中引入更多的改动

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants