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

L/Q again, step two #414

Closed
wants to merge 7 commits into from
Closed
Changes from 4 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
@@ -310,12 +310,12 @@ void NewTypeArrayStub::emit_code(LIR_Assembler* ce) {
// Implementation of NewObjectArrayStub

NewObjectArrayStub::NewObjectArrayStub(LIR_Opr klass_reg, LIR_Opr length, LIR_Opr result,
CodeEmitInfo* info, bool is_inline_type) {
CodeEmitInfo* info, bool is_null_free) {
Copy link
Member

@nick-arm nick-arm May 19, 2021

Choose a reason for hiding this comment

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

Would you mind making the same change in c1_CodeStubs_aarch64.cpp to keep them in sync?

Copy link
Collaborator Author

@fparain fparain May 19, 2021

Choose a reason for hiding this comment

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

Sure, sorry for the breakage of the aarch64 platform.

Fred

_klass_reg = klass_reg;
_result = result;
_length = length;
_info = new CodeEmitInfo(info);
_is_inline_type = is_inline_type;
_is_null_free = is_null_free;
}


@@ -324,7 +324,7 @@ void NewObjectArrayStub::emit_code(LIR_Assembler* ce) {
__ bind(_entry);
assert(_length->as_register() == rbx, "length must in rbx,");
assert(_klass_reg->as_register() == rdx, "klass_reg must in rdx");
if (_is_inline_type) {
if (_is_null_free) {
__ call(RuntimeAddress(Runtime1::entry_for(Runtime1::new_flat_array_id)));
} else {
__ call(RuntimeAddress(Runtime1::entry_for(Runtime1::new_object_array_id)));
@@ -384,9 +384,9 @@ class NewObjectArrayStub: public CodeStub {
LIR_Opr _length;
LIR_Opr _result;
CodeEmitInfo* _info;
bool _is_inline_type;
bool _is_null_free;
public:
NewObjectArrayStub(LIR_Opr klass_reg, LIR_Opr length, LIR_Opr result, CodeEmitInfo* info, bool is_inline_type);
NewObjectArrayStub(LIR_Opr klass_reg, LIR_Opr length, LIR_Opr result, CodeEmitInfo* info, bool is_null_free);
virtual void emit_code(LIR_Assembler* e);
virtual CodeEmitInfo* info() const { return _info; }
virtual void visit(LIR_OpVisitState* visitor) {
@@ -268,7 +268,7 @@ ciType* LoadIndexed::declared_type() const {
bool StoreIndexed::is_exact_flattened_array_store() const {
if (array()->is_loaded_flattened_array() && value()->as_Constant() == NULL && value()->declared_type() != NULL) {
ciKlass* element_klass = array()->declared_type()->as_flat_array_klass()->element_klass();
ciKlass* actual_klass = value()->declared_type()->as_klass();
ciKlass* actual_klass = value()->declared_type()->unwrap()->as_klass();

// The following check can fail with inlining:
// void test45_inline(Object[] oa, Object o, int index) { oa[index] = o; }
@@ -290,7 +290,7 @@ ciType* NewTypeArray::exact_type() const {
}

ciType* NewObjectArray::exact_type() const {
return ciArrayKlass::make(klass());
return ciArrayKlass::make(klass(), is_null_free());
}

ciType* NewMultiArray::exact_type() const {
@@ -30,6 +30,7 @@
#include "ci/ciTypeArrayKlass.hpp"
#include "ci/ciUtilities.hpp"
#include "ci/ciUtilities.inline.hpp"
#include "oops/inlineKlass.inline.hpp"

// ciArrayKlass
//
@@ -104,10 +105,36 @@ bool ciArrayKlass::is_leaf_type() {
ciArrayKlass* ciArrayKlass::make(ciType* element_type) {
if (element_type->is_primitive_type()) {
return ciTypeArrayKlass::make(element_type->basic_type());
} else if (element_type->flatten_array()) {
return ciFlatArrayKlass::make(element_type->as_klass());
} else {
return ciObjArrayKlass::make(element_type->as_klass());
return make(element_type->as_klass(), element_type->is_null_free());
}
}

ciArrayKlass* ciArrayKlass::make(ciKlass* klass, bool null_free) {
if (null_free) {
if (klass->is_loaded()) {
bool is_array_flattened = false;
GUARDED_VM_ENTRY(
EXCEPTION_CONTEXT;
Klass* ak = InlineKlass::cast(klass->get_Klass())->null_free_inline_array_klass(THREAD);
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION;
} else {
if (ak != NULL && ak->is_flatArray_klass()) {
is_array_flattened = true;
Copy link
Member

@TobiHartmann TobiHartmann May 19, 2021

Choose a reason for hiding this comment

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

Why is that boolean needed? Can't we simply return ciFlatArrayKlass::make(klass) here?

Copy link
Collaborator Author

@fparain fparain May 19, 2021

Choose a reason for hiding this comment

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

Boolean removed in the latest commit.

}
}
)
if (is_array_flattened) {
return ciFlatArrayKlass::make(klass);
} else {
return ciObjArrayKlass::make(klass, true);
}
} else {
return ciEnv::unloaded_ciobjarrayklass();
Copy link
Member

@TobiHartmann TobiHartmann May 19, 2021

Choose a reason for hiding this comment

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

Can't we delegate handling the unloaded case to ciObjArrayKlass::make(klass) below? I.e. check for if (null_free && klass->is_loaded()) above.

Copy link
Collaborator Author

@fparain fparain May 19, 2021

Choose a reason for hiding this comment

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

I can change that. This was the way it was implemented initially, then I changed it to have the whole logic in a single method, but I don't have a strong opinion about this one.
Updated in the latest commit.

}
} else {
return ciObjArrayKlass::make(klass);
}
}

@@ -60,6 +60,7 @@ class ciArrayKlass : public ciKlass {
virtual ciKlass* element_klass() { return NULL; }

static ciArrayKlass* make(ciType* element_type);
static ciArrayKlass* make(ciKlass* klass, bool null_free);

int array_header_in_bytes();
ciInstance* component_mirror_instance() const;
@@ -475,7 +475,8 @@ ciKlass* ciEnv::get_klass_by_name_impl(ciKlass* accessing_klass,
require_local);
if (elem_klass != NULL && elem_klass->is_loaded()) {
// Now make an array for it
return ciArrayKlass::make(elem_klass);
bool null_free_array = sym->is_Q_array_signature() && sym->char_at(1) == JVM_SIGNATURE_INLINE_TYPE;
Copy link
Member

@TobiHartmann TobiHartmann May 19, 2021

Choose a reason for hiding this comment

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

Why do we need the sym->char_at(1) == JVM_SIGNATURE_INLINE_TYPE check? Isn't that part of is_Q_array_signature()?

Copy link
Collaborator Author

@fparain fparain May 19, 2021

Choose a reason for hiding this comment

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

The problem with is_Q_array_signature() is that it returns true for arrays of any dimension with the last dimension element klass being a null-free primitive class. It returns true for [QFoo; but also for [[[QFoo;. But for the [[[QFoo; array, the first two dimensions are regular nullable object arrays, only the last array is a null-free array (possibly flattened).

Copy link
Member

@TobiHartmann TobiHartmann May 19, 2021

Choose a reason for hiding this comment

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

Ah, right. Thanks for the explanation!

return ciArrayKlass::make(elem_klass, null_free_array);
}
}

@@ -31,6 +31,7 @@
#include "ci/ciUtilities.hpp"
#include "ci/ciUtilities.inline.hpp"
#include "oops/flatArrayKlass.hpp"
#include "oops/inlineKlass.inline.hpp"

// ciFlatArrayKlass
//
@@ -129,11 +130,11 @@ ciSymbol* ciFlatArrayKlass::construct_array_name(ciSymbol* element_name,
// Implementation of make.
ciArrayKlass* ciFlatArrayKlass::make_impl(ciKlass* element_klass) {
assert(UseFlatArray, "should only be used for flat arrays");
assert(element_klass->is_inlinetype(), "element type must be an inline type");
assert(element_klass->is_loaded(), "unloaded inline klasses are represented by ciInstanceKlass");
assert(element_klass->is_inlinetype(), "element type must be an inline type");
{
EXCEPTION_CONTEXT;
Klass* array = element_klass->get_Klass()->array_klass(THREAD);
Klass* array = InlineKlass::cast(element_klass->get_Klass())->null_free_inline_array_klass(THREAD);
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION;
CURRENT_THREAD_ENV->record_out_of_memory_failure();
@@ -47,6 +47,7 @@ class ciKlass : public ciType {
friend class ciReceiverTypeData;
friend class ciSignature;
friend class ciFlatArrayKlass;
friend class ciArrayKlass;

private:
ciSymbol* _name;
@@ -27,6 +27,7 @@
#include "ci/ciObjArrayKlass.hpp"
#include "ci/ciSymbol.hpp"
#include "ci/ciUtilities.inline.hpp"
#include "oops/inlineKlass.inline.hpp"
#include "oops/objArrayKlass.hpp"
#include "runtime/signature.hpp"

@@ -135,11 +136,17 @@ ciSymbol* ciObjArrayKlass::construct_array_name(ciSymbol* element_name,
// ciObjArrayKlass::make_impl
//
// Implementation of make.
ciObjArrayKlass* ciObjArrayKlass::make_impl(ciKlass* element_klass) {
ciObjArrayKlass* ciObjArrayKlass::make_impl(ciKlass* element_klass, bool null_free) {
if (element_klass->is_loaded()) {
EXCEPTION_CONTEXT;
// The element klass is loaded
Klass* array = element_klass->get_Klass()->array_klass(THREAD);
Klass* array;
if (null_free) {
assert(element_klass->get_Klass()->is_inline_klass(), "Only inline classes can have null free arrays");
array = InlineKlass::cast(element_klass->get_Klass())->null_free_inline_array_klass(THREAD);
} else {
array = element_klass->get_Klass()->array_klass(THREAD);
}
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION;
CURRENT_THREAD_ENV->record_out_of_memory_failure();
@@ -162,8 +169,8 @@ ciObjArrayKlass* ciObjArrayKlass::make_impl(ciKlass* element_klass) {
// ciObjArrayKlass::make
//
// Make an array klass corresponding to the specified primitive type.
ciObjArrayKlass* ciObjArrayKlass::make(ciKlass* element_klass) {
GUARDED_VM_ENTRY(return make_impl(element_klass);)
ciObjArrayKlass* ciObjArrayKlass::make(ciKlass* element_klass, bool null_free) {
GUARDED_VM_ENTRY(return make_impl(element_klass, null_free);)
}

ciKlass* ciObjArrayKlass::exact_klass() {
@@ -49,7 +49,7 @@ class ciObjArrayKlass : public ciArrayKlass {
return (ObjArrayKlass*)get_Klass();
}

static ciObjArrayKlass* make_impl(ciKlass* element_klass);
static ciObjArrayKlass* make_impl(ciKlass* element_klass, bool null_free);
static ciSymbol* construct_array_name(ciSymbol* element_name,
int dimension);

@@ -72,7 +72,7 @@ class ciObjArrayKlass : public ciArrayKlass {
// What kind of ciObject is this?
bool is_obj_array_klass() const { return true; }

static ciObjArrayKlass* make(ciKlass* element_klass);
static ciObjArrayKlass* make(ciKlass* element_klass, bool null_free = false);

virtual ciKlass* exact_klass();

@@ -49,8 +49,7 @@ public static void main(String[] args) {
} catch(Error e) {
ex = e;
}
Asserts.assertNotNull(ex, "An ICCE should have been thrown");
Asserts.assertEquals(ex.getClass(), IncompatibleClassChangeError.class, "Error is not an ICCE");
Asserts.assertNull(ex, "No error should have been thrown");
ex = null;
}
}