Skip to content

Commit 3cc0d71

Browse files
k-sareenk-sareen
authored
Fix MarkCompact C2 fastpath and refactor allocator offset calculation (#158)
MarkCompactAllocator mistakenly used the offset for `bump_allocator` in the Allocators struct instead of the offset for `markcompact` in its C2 fastpath. This commit fixes the above issue and refactors the duplicated offset calculation code in the C1 and C2 fastpaths into a new function.
1 parent e6c9b94 commit 3cc0d71

File tree

4 files changed

+68
-73
lines changed

4 files changed

+68
-73
lines changed

‎openjdk/mmtkBarrierSet.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,44 @@
3535
#include "mmtkBarrierSetC2.hpp"
3636
#endif
3737

38+
MMTkAllocatorOffsets get_tlab_top_and_end_offsets(AllocatorSelector selector) {
39+
int tlab_top_offset, tlab_end_offset;
40+
int allocators_base_offset = in_bytes(JavaThread::third_party_heap_mutator_offset())
41+
+ in_bytes(byte_offset_of(MMTkMutatorContext, allocators));
42+
43+
if (selector.tag == TAG_IMMIX) {
44+
int allocator_base_offset = allocators_base_offset
45+
+ in_bytes(byte_offset_of(Allocators, immix))
46+
+ selector.index * sizeof(ImmixAllocator);
47+
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, cursor));
48+
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, limit));
49+
} else if (selector.tag == TAG_BUMP_POINTER) {
50+
int allocator_base_offset = allocators_base_offset
51+
+ in_bytes(byte_offset_of(Allocators, bump_pointer))
52+
+ selector.index * sizeof(BumpAllocator);
53+
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor));
54+
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit));
55+
} else if (selector.tag == TAG_MARK_COMPACT) {
56+
int allocator_base_offset = allocators_base_offset
57+
+ in_bytes(byte_offset_of(Allocators, markcompact))
58+
+ selector.index * sizeof(MarkCompactAllocator)
59+
+ in_bytes(byte_offset_of(MarkCompactAllocator, bump_allocator));
60+
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor));
61+
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit));
62+
} else {
63+
fatal("Unimplemented allocator fastpath\n");
64+
// Setting values to make compiler happy about unitialized variables. This
65+
// case should never be reached in practice, however.
66+
tlab_top_offset = 0;
67+
tlab_end_offset = 0;
68+
}
69+
70+
MMTkAllocatorOffsets alloc_offsets;
71+
alloc_offsets.tlab_top_offset = tlab_top_offset;
72+
alloc_offsets.tlab_end_offset = tlab_end_offset;
73+
return alloc_offsets;
74+
}
75+
3876
MMTkBarrierBase* get_selected_barrier() {
3977
static MMTkBarrierBase* selected_barrier = NULL;
4078
if (selected_barrier) return selected_barrier;

‎openjdk/mmtkBarrierSet.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,20 @@
4040

4141
const intptr_t ALLOC_BIT_BASE_ADDRESS = GLOBAL_ALLOC_BIT_ADDRESS;
4242

43+
struct MMTkAllocatorOffsets {
44+
int tlab_top_offset;
45+
int tlab_end_offset;
46+
};
47+
48+
/**
49+
* Return the offset (from the start of the mutator) for the TLAB top (cursor)
50+
* and end (limit) for an MMTk Allocator.
51+
*
52+
* @param selector The current MMTk Allocator being used
53+
* @return the offsets to the top and end of the TLAB
54+
*/
55+
MMTkAllocatorOffsets get_tlab_top_and_end_offsets(AllocatorSelector selector);
56+
4357
class MMTkBarrierSetRuntime: public CHeapObj<mtGC> {
4458
public:
4559
virtual void record_modified_node(oop object) {};

‎openjdk/mmtkBarrierSetAssembler_x86.cpp

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -71,39 +71,13 @@ void MMTkBarrierSetAssembler::eden_allocate(MacroAssembler* masm, Register threa
7171
return;
7272
}
7373

74-
// Only bump pointer allocator is implemented.
75-
if (selector.tag != TAG_BUMP_POINTER && selector.tag != TAG_MARK_COMPACT && selector.tag != TAG_IMMIX) {
76-
fatal("unimplemented allocator fastpath\n");
77-
}
78-
79-
// Calculat offsets of top and end. We now assume we are using bump pointer.
80-
int allocator_base_offset;
74+
// Calculate offsets of TLAB top and end
8175
Address cursor, limit;
76+
MMTkAllocatorOffsets alloc_offsets = get_tlab_top_and_end_offsets(selector);
77+
78+
cursor = Address(r15_thread, alloc_offsets.tlab_top_offset);
79+
limit = Address(r15_thread, alloc_offsets.tlab_end_offset);
8280

83-
if (selector.tag == TAG_IMMIX) {
84-
allocator_base_offset = in_bytes(JavaThread::third_party_heap_mutator_offset())
85-
+ in_bytes(byte_offset_of(MMTkMutatorContext, allocators))
86-
+ in_bytes(byte_offset_of(Allocators, immix))
87-
+ selector.index * sizeof(ImmixAllocator);
88-
cursor = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, cursor)));
89-
limit = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, limit)));
90-
} else if(selector.tag == TAG_BUMP_POINTER) {
91-
allocator_base_offset = in_bytes(JavaThread::third_party_heap_mutator_offset())
92-
+ in_bytes(byte_offset_of(MMTkMutatorContext, allocators))
93-
+ in_bytes(byte_offset_of(Allocators, bump_pointer))
94-
+ selector.index * sizeof(BumpAllocator);
95-
cursor = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor)));
96-
limit = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit)));
97-
} else {
98-
// markcompact allocator
99-
allocator_base_offset = in_bytes(JavaThread::third_party_heap_mutator_offset())
100-
+ in_bytes(byte_offset_of(MMTkMutatorContext, allocators))
101-
+ in_bytes(byte_offset_of(Allocators, markcompact))
102-
+ selector.index * sizeof(MarkCompactAllocator)
103-
+ in_bytes(byte_offset_of(MarkCompactAllocator, bump_allocator));
104-
cursor = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor)));
105-
limit = Address(r15_thread, allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit)));
106-
}
10781
// obj = load lab.cursor
10882
__ movptr(obj, cursor);
10983
if (selector.tag == TAG_MARK_COMPACT) __ addptr(obj, extra_header);
@@ -126,8 +100,7 @@ void MMTkBarrierSetAssembler::eden_allocate(MacroAssembler* masm, Register threa
126100
#ifdef MMTK_ENABLE_GLOBAL_ALLOC_BIT
127101
enable_global_alloc_bit = true;
128102
#endif
129-
// #ifdef MMTK_ENABLE_GLOBAL_ALLOC_BIT
130-
if(enable_global_alloc_bit || selector.tag == TAG_MARK_COMPACT) {
103+
if (enable_global_alloc_bit || selector.tag == TAG_MARK_COMPACT) {
131104
Register tmp3 = rdi;
132105
Register tmp2 = rscratch1;
133106
assert_different_registers(obj, tmp2, tmp3, rcx);
@@ -152,9 +125,8 @@ void MMTkBarrierSetAssembler::eden_allocate(MacroAssembler* masm, Register threa
152125
__ movptr(tmp3, obj);
153126
__ shrptr(tmp3, 6);
154127
__ movptr(rcx, ALLOC_BIT_BASE_ADDRESS);
155-
__ movb(Address(rcx, tmp3), tmp2);
128+
__ movb(Address(rcx, tmp3), tmp2);
156129
}
157-
// #endif
158130

159131
// BarrierSetAssembler::incr_allocated_bytes
160132
if (var_size_in_bytes->is_valid()) {

‎openjdk/mmtkBarrierSetC2.cpp

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -177,39 +177,12 @@ void MMTkBarrierSetC2::expand_allocate(PhaseMacroExpand* x,
177177
Node* eden_end_adr;
178178

179179
{
180-
// Only bump pointer allocator fastpath is implemented.
181-
if (selector.tag != TAG_BUMP_POINTER && selector.tag != TAG_MARK_COMPACT && selector.tag != TAG_IMMIX) {
182-
fatal("unimplemented allocator fastpath\n");
183-
}
180+
// Calculate offsets of TLAB top and end
181+
MMTkAllocatorOffsets alloc_offsets = get_tlab_top_and_end_offsets(selector);
184182

185-
// Calculat offsets of top and end. We now assume we are using bump pointer.
186-
int allocators_base_offset = in_bytes(JavaThread::third_party_heap_mutator_offset())
187-
+ in_bytes(byte_offset_of(MMTkMutatorContext, allocators));
188-
int tlab_top_offset, tlab_end_offset;
189-
if (selector.tag == TAG_IMMIX) {
190-
int allocator_base_offset = allocators_base_offset
191-
+ in_bytes(byte_offset_of(Allocators, immix))
192-
+ selector.index * sizeof(ImmixAllocator);
193-
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, cursor));
194-
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(ImmixAllocator, limit));
195-
} else if (selector.tag == TAG_BUMP_POINTER) {
196-
int allocator_base_offset = allocators_base_offset
197-
+ in_bytes(byte_offset_of(Allocators, bump_pointer))
198-
+ selector.index * sizeof(BumpAllocator);
199-
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor));
200-
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit));
201-
} else {
202-
// markcompact allocator
203-
int allocator_base_offset = allocators_base_offset
204-
+ in_bytes(byte_offset_of(Allocators, bump_pointer))
205-
+ selector.index * sizeof(MarkCompactAllocator)
206-
+ in_bytes(byte_offset_of(MarkCompactAllocator, bump_allocator));
207-
tlab_top_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, cursor));
208-
tlab_end_offset = allocator_base_offset + in_bytes(byte_offset_of(BumpAllocator, limit));
209-
}
210183
Node* thread = x->transform_later(new ThreadLocalNode());
211-
eden_top_adr = x->basic_plus_adr(x->top()/*not oop*/, thread, tlab_top_offset);
212-
eden_end_adr = x->basic_plus_adr(x->top()/*not oop*/, thread, tlab_end_offset);
184+
eden_top_adr = x->basic_plus_adr(x->top()/*not oop*/, thread, alloc_offsets.tlab_top_offset);
185+
eden_end_adr = x->basic_plus_adr(x->top()/*not oop*/, thread, alloc_offsets.tlab_end_offset);
213186
}
214187

215188
// set_eden_pointers(eden_top_adr, eden_end_adr);
@@ -239,7 +212,7 @@ void MMTkBarrierSetC2::expand_allocate(PhaseMacroExpand* x,
239212

240213
// Load(-locked) the heap top.
241214
// See note above concerning the control input when using a TLAB
242-
Node *old_eden_top;
215+
Node *old_eden_top;
243216

244217
if (selector.tag == TAG_MARK_COMPACT) {
245218
Node *offset = ConLNode::make(extra_header);
@@ -303,15 +276,14 @@ void MMTkBarrierSetC2::expand_allocate(PhaseMacroExpand* x,
303276
#ifdef MMTK_ENABLE_GLOBAL_ALLOC_BIT
304277
enable_global_alloc_bit = true;
305278
#endif
306-
// #ifdef MMTK_ENABLE_GLOBAL_ALLOC_BIT
307-
if(enable_global_alloc_bit || selector.tag == TAG_MARK_COMPACT) {
308-
// set the alloc bit:
279+
if (enable_global_alloc_bit || selector.tag == TAG_MARK_COMPACT) {
280+
// set the alloc bit:
309281
// intptr_t addr = (intptr_t) (void*) fast_oop;
310282
// uint8_t* meta_addr = (uint8_t*) (ALLOC_BIT_BASE_ADDRESS + (addr >> 6));
311283
// intptr_t shift = (addr >> 3) & 0b111;
312284
// uint8_t byte_val = *meta_addr;
313285
// uint8_t new_byte_val = byte_val | (1 << shift);
314-
// *meta_addr = new_byte_val;
286+
// *meta_addr = new_byte_val;
315287
Node *obj_addr = new CastP2XNode(fast_oop_ctrl, fast_oop);
316288
x->transform_later(obj_addr);
317289

@@ -348,7 +320,7 @@ void MMTkBarrierSetC2::expand_allocate(PhaseMacroExpand* x,
348320
Node *const_one = ConINode::make(1);
349321
x->transform_later(const_one);
350322

351-
Node *shifted_masked_addr_i = new ConvL2INode(shifted_masked_addr);
323+
Node *shifted_masked_addr_i = new ConvL2INode(shifted_masked_addr);
352324
x->transform_later(shifted_masked_addr_i);
353325

354326
Node *set_bit = new LShiftINode(const_one, shifted_masked_addr_i);
@@ -362,7 +334,6 @@ void MMTkBarrierSetC2::expand_allocate(PhaseMacroExpand* x,
362334

363335
fast_oop_rawmem = set_alloc_bit;
364336
}
365-
// #endif
366337

367338
InitializeNode* init = alloc->initialization();
368339
fast_oop_rawmem = x->initialize_object(alloc,

0 commit comments

Comments
 (0)