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

8256406: G1 x86 C1/Interpreter post write barrier always uses 32 bit to access variable sized PtrQueue::_index #1257

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -32,6 +32,7 @@
#include "gc/g1/heapRegion.hpp"
#include "interpreter/interp_masm.hpp"
#include "runtime/sharedRuntime.hpp"
#include "utilities/debug.hpp"
#include "utilities/macros.hpp"
#ifdef COMPILER1
#include "c1/c1_LIRAssembler.hpp"
@@ -264,6 +265,8 @@ void G1BarrierSetAssembler::g1_write_barrier_post(MacroAssembler* masm,
Register thread,
Register tmp,
Register tmp2) {
// Generated code assumes that buffer index is pointer sized.
STATIC_ASSERT(in_bytes(SATBMarkQueue::byte_width_of_index()) == sizeof(intptr_t));
#ifdef _LP64
assert(thread == r15_thread, "must be");
#endif // _LP64
@@ -314,18 +317,13 @@ void G1BarrierSetAssembler::g1_write_barrier_post(MacroAssembler* masm,

__ movb(Address(card_addr, 0), (int)G1CardTable::dirty_card_val());

__ cmpl(queue_index, 0);
__ jcc(Assembler::equal, runtime);
__ subl(queue_index, wordSize);
__ movptr(tmp2, buffer);
#ifdef _LP64
__ movslq(rscratch1, queue_index);
__ addq(tmp2, rscratch1);
__ movq(Address(tmp2, 0), card_addr);
#else
__ addl(tmp2, queue_index);
__ movl(Address(tmp2, 0), card_addr);
#endif
__ movptr(tmp2, queue_index);
__ testptr(tmp2, tmp2);
__ jcc(Assembler::zero, runtime);
__ subptr(tmp2, wordSize);
__ movptr(queue_index, tmp2);
__ addptr(tmp2, buffer);
__ movptr(Address(tmp2, 0), card_addr);
__ jmp(done);

__ bind(runtime);
@@ -453,6 +451,9 @@ void G1BarrierSetAssembler::gen_post_barrier_stub(LIR_Assembler* ce, G1PostBarri
#define __ sasm->

void G1BarrierSetAssembler::generate_c1_pre_barrier_runtime_stub(StubAssembler* sasm) {
// Generated code assumes that buffer index is pointer sized.
STATIC_ASSERT(in_bytes(SATBMarkQueue::byte_width_of_index()) == sizeof(intptr_t));

__ prologue("g1_pre_barrier", false);
// arg0 : previous value of memory

@@ -178,7 +178,7 @@ class PtrQueue {
return byte_offset_of(Derived, _index);
}

static ByteSize byte_width_of_index() { return in_ByteSize(sizeof(size_t)); }
static constexpr ByteSize byte_width_of_index() { return in_ByteSize(sizeof(size_t)); }
Copy link
Contributor

@shipilev shipilev Nov 17, 2020

Choose a reason for hiding this comment

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

Do we really have to constexpr this for STATIC_ASSERT? No objection, just curious.

Copy link
Contributor Author

@tschatzl tschatzl Nov 17, 2020

Choose a reason for hiding this comment

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

My compiler (gcc 9.3.0) complains with

[...] g1BarrierSetAssembler_x86.cpp:269:60: error: call to non-'constexpr' function 'static ByteSize PtrQueue::byte_width_of_index()' [...]

otherwise. Probably because of the called in_ByteSize() method.

Copy link
Contributor

@shipilev shipilev Nov 17, 2020

Choose a reason for hiding this comment

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

Okay.


template<typename Derived>
static ByteSize byte_offset_of_buf() {