optimizing: Build HConstructorFence for HNewArray/HNewInstance nodes
Also fixes:
* LSE, code_sinking to keep optimizing new-instance if it did so before
* Various tests to expect constructor fences after new-instance
Sidenote: new-instance String does not get a ConstructorFence; the
special StringFactory calls are assumed to be self-fencing.
Metric changes on go/lem:
* CodeSize -0.262% in ART-Compile (ARMv8)
* RunTime -0.747% for all (linux-armv8)
(No changes expected to x86, constructor fences are no-op).
The RunTime regression is temporary until art_quick_alloc_* entrypoints have their
DMBs removed in a follow up CL.
Test: art/test.py
Bug: 36656456
Change-Id: I6a936a6e51c623e1c6b5b22eee5c3c72bebbed35
diff --git a/compiler/optimizing/code_sinking.cc b/compiler/optimizing/code_sinking.cc
index 0b4dcd3..e598e19 100644
--- a/compiler/optimizing/code_sinking.cc
+++ b/compiler/optimizing/code_sinking.cc
@@ -56,6 +56,17 @@
return true;
}
+ // Check it is safe to move ConstructorFence.
+ // (Safe to move ConstructorFence for only protecting the new-instance but not for finals.)
+ if (instruction->IsConstructorFence()) {
+ HConstructorFence* ctor_fence = instruction->AsConstructorFence();
+
+ // A fence with "0" inputs is dead and should've been removed in a prior pass.
+ DCHECK_NE(0u, ctor_fence->InputCount());
+
+ return ctor_fence->GetAssociatedAllocation() != nullptr;
+ }
+
// All other instructions that can throw cannot be moved.
if (instruction->CanThrow()) {
return false;
@@ -134,11 +145,11 @@
HInstruction* user,
const ArenaBitVector& post_dominated) {
if (instruction->IsNewInstance()) {
- return user->IsInstanceFieldSet() &&
+ return (user->IsInstanceFieldSet() || user->IsConstructorFence()) &&
(user->InputAt(0) == instruction) &&
!post_dominated.IsBitSet(user->GetBlock()->GetBlockId());
} else if (instruction->IsNewArray()) {
- return user->IsArraySet() &&
+ return (user->IsArraySet() || user->IsConstructorFence()) &&
(user->InputAt(0) == instruction) &&
!post_dominated.IsBitSet(user->GetBlock()->GetBlockId());
}
@@ -372,7 +383,9 @@
// Step (3): Try to move sinking candidates.
for (HInstruction* instruction : move_in_order) {
HInstruction* position = nullptr;
- if (instruction->IsArraySet() || instruction->IsInstanceFieldSet()) {
+ if (instruction->IsArraySet()
+ || instruction->IsInstanceFieldSet()
+ || instruction->IsConstructorFence()) {
if (!instructions_that_can_move.IsBitSet(instruction->InputAt(0)->GetId())) {
// A store can trivially move, but it can safely do so only if the heap
// location it stores to can also move.
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 8b79da8..40fafb0 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -962,7 +962,7 @@
false /* is_unresolved */);
}
-bool HInstructionBuilder::BuildNewInstance(dex::TypeIndex type_index, uint32_t dex_pc) {
+HNewInstance* HInstructionBuilder::BuildNewInstance(dex::TypeIndex type_index, uint32_t dex_pc) {
ScopedObjectAccess soa(Thread::Current());
HLoadClass* load_class = BuildLoadClass(type_index, dex_pc);
@@ -986,14 +986,65 @@
// Consider classes we haven't resolved as potentially finalizable.
bool finalizable = (klass == nullptr) || klass->IsFinalizable();
- AppendInstruction(new (arena_) HNewInstance(
+ HNewInstance* new_instance = new (arena_) HNewInstance(
cls,
dex_pc,
type_index,
*dex_compilation_unit_->GetDexFile(),
finalizable,
- entrypoint));
- return true;
+ entrypoint);
+ AppendInstruction(new_instance);
+
+ return new_instance;
+}
+
+void HInstructionBuilder::BuildConstructorFenceForAllocation(HInstruction* allocation) {
+ DCHECK(allocation != nullptr &&
+ allocation->IsNewInstance() ||
+ allocation->IsNewArray()); // corresponding to "new" keyword in JLS.
+
+ if (allocation->IsNewInstance()) {
+ // STRING SPECIAL HANDLING:
+ // -------------------------------
+ // Strings have a real HNewInstance node but they end up always having 0 uses.
+ // All uses of a String HNewInstance are always transformed to replace their input
+ // of the HNewInstance with an input of the invoke to StringFactory.
+ //
+ // Do not emit an HConstructorFence here since it can inhibit some String new-instance
+ // optimizations (to pass checker tests that rely on those optimizations).
+ HNewInstance* new_inst = allocation->AsNewInstance();
+ HLoadClass* load_class = new_inst->GetLoadClass();
+
+ Thread* self = Thread::Current();
+ ScopedObjectAccess soa(self);
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> klass = load_class->GetClass();
+ if (klass != nullptr && klass->IsStringClass()) {
+ return;
+ // Note: Do not use allocation->IsStringAlloc which requires
+ // a valid ReferenceTypeInfo, but that doesn't get made until after reference type
+ // propagation (and instruction builder is too early).
+ }
+ // (In terms of correctness, the StringFactory needs to provide its own
+ // default initialization barrier, see below.)
+ }
+
+ // JLS 17.4.5 "Happens-before Order" describes:
+ //
+ // The default initialization of any object happens-before any other actions (other than
+ // default-writes) of a program.
+ //
+ // In our implementation the default initialization of an object to type T means
+ // setting all of its initial data (object[0..size)) to 0, and setting the
+ // object's class header (i.e. object.getClass() == T.class).
+ //
+ // In practice this fence ensures that the writes to the object header
+ // are visible to other threads if this object escapes the current thread.
+ // (and in theory the 0-initializing, but that happens automatically
+ // when new memory pages are mapped in by the OS).
+ HConstructorFence* ctor_fence =
+ new (arena_) HConstructorFence(allocation, allocation->GetDexPc(), arena_);
+ AppendInstruction(ctor_fence);
}
static bool IsSubClass(mirror::Class* to_test, mirror::Class* super_class)
@@ -1522,15 +1573,15 @@
graph_->SetHasBoundsChecks(true);
}
-void HInstructionBuilder::BuildFilledNewArray(uint32_t dex_pc,
- dex::TypeIndex type_index,
- uint32_t number_of_vreg_arguments,
- bool is_range,
- uint32_t* args,
- uint32_t register_index) {
+HNewArray* HInstructionBuilder::BuildFilledNewArray(uint32_t dex_pc,
+ dex::TypeIndex type_index,
+ uint32_t number_of_vreg_arguments,
+ bool is_range,
+ uint32_t* args,
+ uint32_t register_index) {
HInstruction* length = graph_->GetIntConstant(number_of_vreg_arguments, dex_pc);
HLoadClass* cls = BuildLoadClass(type_index, dex_pc);
- HInstruction* object = new (arena_) HNewArray(cls, length, dex_pc);
+ HNewArray* const object = new (arena_) HNewArray(cls, length, dex_pc);
AppendInstruction(object);
const char* descriptor = dex_file_->StringByTypeIdx(type_index);
@@ -1550,6 +1601,8 @@
AppendInstruction(aset);
}
latest_result_ = object;
+
+ return object;
}
template <typename T>
@@ -2534,10 +2587,12 @@
}
case Instruction::NEW_INSTANCE: {
- if (!BuildNewInstance(dex::TypeIndex(instruction.VRegB_21c()), dex_pc)) {
- return false;
- }
+ HNewInstance* new_instance =
+ BuildNewInstance(dex::TypeIndex(instruction.VRegB_21c()), dex_pc);
+ DCHECK(new_instance != nullptr);
+
UpdateLocal(instruction.VRegA(), current_block_->GetLastInstruction());
+ BuildConstructorFenceForAllocation(new_instance);
break;
}
@@ -2545,8 +2600,11 @@
dex::TypeIndex type_index(instruction.VRegC_22c());
HInstruction* length = LoadLocal(instruction.VRegB_22c(), Primitive::kPrimInt);
HLoadClass* cls = BuildLoadClass(type_index, dex_pc);
- AppendInstruction(new (arena_) HNewArray(cls, length, dex_pc));
+
+ HNewArray* new_array = new (arena_) HNewArray(cls, length, dex_pc);
+ AppendInstruction(new_array);
UpdateLocal(instruction.VRegA_22c(), current_block_->GetLastInstruction());
+ BuildConstructorFenceForAllocation(new_array);
break;
}
@@ -2555,7 +2613,13 @@
dex::TypeIndex type_index(instruction.VRegB_35c());
uint32_t args[5];
instruction.GetVarArgs(args);
- BuildFilledNewArray(dex_pc, type_index, number_of_vreg_arguments, false, args, 0);
+ HNewArray* new_array = BuildFilledNewArray(dex_pc,
+ type_index,
+ number_of_vreg_arguments,
+ /* is_range */ false,
+ args,
+ /* register_index */ 0);
+ BuildConstructorFenceForAllocation(new_array);
break;
}
@@ -2563,8 +2627,13 @@
uint32_t number_of_vreg_arguments = instruction.VRegA_3rc();
dex::TypeIndex type_index(instruction.VRegB_3rc());
uint32_t register_index = instruction.VRegC_3rc();
- BuildFilledNewArray(
- dex_pc, type_index, number_of_vreg_arguments, true, nullptr, register_index);
+ HNewArray* new_array = BuildFilledNewArray(dex_pc,
+ type_index,
+ number_of_vreg_arguments,
+ /* is_range */ true,
+ /* args*/ nullptr,
+ register_index);
+ BuildConstructorFenceForAllocation(new_array);
break;
}
diff --git a/compiler/optimizing/instruction_builder.h b/compiler/optimizing/instruction_builder.h
index 2fb5c7b..e968760 100644
--- a/compiler/optimizing/instruction_builder.h
+++ b/compiler/optimizing/instruction_builder.h
@@ -194,12 +194,12 @@
uint32_t register_index);
// Builds a new array node and the instructions that fill it.
- void BuildFilledNewArray(uint32_t dex_pc,
- dex::TypeIndex type_index,
- uint32_t number_of_vreg_arguments,
- bool is_range,
- uint32_t* args,
- uint32_t register_index);
+ HNewArray* BuildFilledNewArray(uint32_t dex_pc,
+ dex::TypeIndex type_index,
+ uint32_t number_of_vreg_arguments,
+ bool is_range,
+ uint32_t* args,
+ uint32_t register_index);
void BuildFillArrayData(const Instruction& instruction, uint32_t dex_pc);
@@ -288,7 +288,11 @@
REQUIRES_SHARED(Locks::mutator_lock_);
// Build a HNewInstance instruction.
- bool BuildNewInstance(dex::TypeIndex type_index, uint32_t dex_pc);
+ HNewInstance* BuildNewInstance(dex::TypeIndex type_index, uint32_t dex_pc);
+
+ // Build a HConstructorFence for HNewInstance and HNewArray instructions. This ensures the
+ // happens-before ordering for default-initialization of the object referred to by new_instance.
+ void BuildConstructorFenceForAllocation(HInstruction* allocation);
// Return whether the compiler can assume `cls` is initialized.
bool IsInitialized(Handle<mirror::Class> cls) const
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 8d8cc93..76c9d23 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -578,9 +578,7 @@
}
}
for (HInstruction* new_array : singleton_new_arrays_) {
- // TODO: Delete constructor fences for new-array
- // In the future HNewArray instructions will have HConstructorFence's for them.
- // HConstructorFence::RemoveConstructorFences(new_array);
+ HConstructorFence::RemoveConstructorFences(new_array);
if (!new_array->HasNonEnvironmentUses()) {
new_array->RemoveEnvironmentUsers();
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index f250c1a..1460b26 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1234,6 +1234,20 @@
}
}
+HInstruction* HConstructorFence::GetAssociatedAllocation() {
+ HInstruction* new_instance_inst = GetPrevious();
+ // Check if the immediately preceding instruction is a new-instance/new-array.
+ // Otherwise this fence is for protecting final fields.
+ if (new_instance_inst != nullptr &&
+ (new_instance_inst->IsNewInstance() || new_instance_inst->IsNewArray())) {
+ // TODO: Need to update this code to handle multiple inputs.
+ DCHECK_EQ(InputCount(), 1u);
+ return new_instance_inst;
+ } else {
+ return nullptr;
+ }
+}
+
#define DEFINE_ACCEPT(name, super) \
void H##name::Accept(HGraphVisitor* visitor) { \
visitor->Visit##name(this); \
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e40361e..72521fd 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6650,6 +6650,14 @@
// still considered live.
static void RemoveConstructorFences(HInstruction* instruction);
+ // Check if this constructor fence is protecting
+ // an HNewInstance or HNewArray that is also the immediate
+ // predecessor of `this`.
+ //
+ // Returns the associated HNewArray or HNewInstance,
+ // or null otherwise.
+ HInstruction* GetAssociatedAllocation();
+
DECLARE_INSTRUCTION(ConstructorFence);
private:
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index c3c141b..aa42fd6 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -168,7 +168,39 @@
}
void PrepareForRegisterAllocation::VisitConstructorFence(HConstructorFence* constructor_fence) {
- // Delete all the inputs to the constructor fence;
+ // Trivially remove redundant HConstructorFence when it immediately follows an HNewInstance
+ // to an uninitialized class. In this special case, the art_quick_alloc_object_resolved
+ // will already have the 'dmb' which is strictly stronger than an HConstructorFence.
+ //
+ // The instruction builder always emits "x = HNewInstance; HConstructorFence(x)" so this
+ // is effectively pattern-matching that particular case and undoing the redundancy the builder
+ // had introduced.
+ //
+ // TODO: Move this to a separate pass.
+ HInstruction* allocation_inst = constructor_fence->GetAssociatedAllocation();
+ if (allocation_inst != nullptr && allocation_inst->IsNewInstance()) {
+ HNewInstance* new_inst = allocation_inst->AsNewInstance();
+ // This relies on the entrypoint already being set to the more optimized version;
+ // as that happens in this pass, this redundancy removal also cannot happen any earlier.
+ if (new_inst != nullptr && new_inst->GetEntrypoint() == kQuickAllocObjectResolved) {
+ // If this was done in an earlier pass, we would want to match that `previous` was an input
+ // to the `constructor_fence`. However, since this pass removes the inputs to the fence,
+ // we can ignore the inputs and just remove the instruction from its block.
+ DCHECK_EQ(1u, constructor_fence->InputCount());
+ // TODO: GetAssociatedAllocation should not care about multiple inputs
+ // if we are in prepare_for_register_allocation pass only.
+ constructor_fence->GetBlock()->RemoveInstruction(constructor_fence);
+ return;
+ // TODO: actually remove the dmb from the .S entrypoints (initialized variants only).
+ }
+
+ // HNewArray does not need this check because the art_quick_alloc_array does not itself
+ // have a dmb in any normal situation (i.e. the array class is never exactly in the
+ // "resolved" state). If the array class is not yet loaded, it will always go from
+ // Unloaded->Initialized state.
+ }
+
+ // Remove all the inputs to the constructor fence;
// they aren't used by the InstructionCodeGenerator and this lets us avoid creating a
// LocationSummary in the LocationsBuilder.
constructor_fence->RemoveAllInputs();
diff --git a/test/476-checker-ctor-memory-barrier/src/Main.java b/test/476-checker-ctor-memory-barrier/src/Main.java
index a538f52..70c5121 100644
--- a/test/476-checker-ctor-memory-barrier/src/Main.java
+++ b/test/476-checker-ctor-memory-barrier/src/Main.java
@@ -35,7 +35,7 @@
/// CHECK-START: void ClassWithFinals.<init>() inliner (after)
/// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK-NOT: ConstructorFence
/*
* Check that the correct assembly instructions are selected for a Store/Store fence.
@@ -65,11 +65,15 @@
}
/// CHECK-START: void ClassWithFinals.<init>(int) inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<This>>]
+ /// CHECK-NOT: ConstructorFence
public ClassWithFinals(int x) {
- // This should have exactly two barriers:
+ // This should have exactly three barriers:
+ // - one for the new-instance
// - one for the constructor
// - one for the `new` which should be inlined.
obj = new ClassWithFinals();
@@ -79,8 +83,9 @@
class InheritFromClassWithFinals extends ClassWithFinals {
/// CHECK-START: void InheritFromClassWithFinals.<init>() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK: ConstructorFence [<<This>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void InheritFromClassWithFinals.<init>() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
@@ -101,17 +106,21 @@
}
/// CHECK-START: void InheritFromClassWithFinals.<init>(int) inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NOT: ConstructorFence
- /// CHECK: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK-DAG: <<NewHere:l\d+>> NewInstance klass:InheritFromClassWithFinals
+ /// CHECK-DAG: ConstructorFence [<<This>>]
+ /// CHECK-DAG: ConstructorFence [<<NewHere>>]
+ /// CHECK-DAG: ConstructorFence [<<NewHere>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void InheritFromClassWithFinals.<init>(int) inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
public InheritFromClassWithFinals(int unused) {
- // Should inline the super constructor and insert a memory barrier.
+ // super(); // implicitly the first invoke in this constructor.
+ // Should inline the super constructor and insert a constructor fence there.
- // Should inline the new instance call and insert another memory barrier.
+ // Should inline the new instance call (barrier); and add another one
+ // because the superclass has finals.
new InheritFromClassWithFinals();
}
}
@@ -120,9 +129,10 @@
final int y;
/// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK: ConstructorFence [<<This>>]
+ /// CHECK: ConstructorFence [<<This>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
@@ -132,9 +142,10 @@
}
/// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>(boolean) inliner (after)
- /// CHECK: InvokeStaticOrDirect
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK: InvokeStaticOrDirect
+ /// CHECK: ConstructorFence [<<This>>]
+ /// CHECK-NOT: ConstructorFence
public HaveFinalsAndInheritFromClassWithFinals(boolean cond) {
super(cond);
// should not inline the super constructor
@@ -142,23 +153,35 @@
}
/// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>(int) inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<This:l\d+>> ParameterValue
+ /// CHECK-DAG: <<NewHF:l\d+>> NewInstance klass:HaveFinalsAndInheritFromClassWithFinals
+ /// CHECK-DAG: <<NewIF:l\d+>> NewInstance klass:InheritFromClassWithFinals
+ /// CHECK-DAG: ConstructorFence [<<This>>]
+ /// CHECK-DAG: ConstructorFence [<<NewHF>>]
+ /// CHECK-DAG: ConstructorFence [<<NewHF>>]
+ /// CHECK-DAG: ConstructorFence [<<NewHF>>]
+ /// CHECK-DAG: ConstructorFence [<<NewIF>>]
+ /// CHECK-DAG: ConstructorFence [<<NewIF>>]
+ /// CHECK-DAG: ConstructorFence [<<This>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>(int) inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
public HaveFinalsAndInheritFromClassWithFinals(int unused) {
- // Should inline the super constructor and keep keep both memory barriers.
+ // super()
+ // -- Inlined super constructor, insert memory barrier here.
y = 0;
// Should inline new instance and keep both memory barriers.
+ // One more memory barrier for new-instance.
+ // (3 total for this new-instance #1)
new HaveFinalsAndInheritFromClassWithFinals();
// Should inline new instance and have exactly one barrier.
+ // One more barrier for new-instance.
+ // (2 total for this new-instance #2)
new InheritFromClassWithFinals();
+
+ // -- End of constructor, insert memory barrier here to freeze 'y'.
}
}
@@ -168,25 +191,33 @@
/// CHECK: InvokeStaticOrDirect
/// CHECK-START: ClassWithFinals Main.noInlineNoConstructorBarrier() inliner (after)
- /// CHECK-NOT: ConstructorFence
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK: ConstructorFence [<<NewInstance>>]
+ /// CHECK-NOT: ConstructorFence
public static ClassWithFinals noInlineNoConstructorBarrier() {
+ // Exactly one barrier for the new-instance.
return new ClassWithFinals(false);
// should not inline the constructor
}
/// CHECK-START: void Main.inlineNew() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void Main.inlineNew() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
public static void inlineNew() {
+ // Exactly 2 barriers. One for new-instance, one for constructor with finals.
new ClassWithFinals();
}
/// CHECK-START: void Main.inlineNew1() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void Main.inlineNew1() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
@@ -195,9 +226,11 @@
}
/// CHECK-START: void Main.inlineNew2() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void Main.inlineNew2() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
@@ -206,11 +239,16 @@
}
/// CHECK-START: void Main.inlineNew3() inliner (after)
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK: ConstructorFence
- /// CHECK-NEXT: ReturnVoid
+ /// CHECK: <<NewInstance:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance>>]
+ /// CHECK-NOT: ConstructorFence
+ /// CHECK: <<NewInstance2:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence [<<NewInstance2>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance2>>]
+ /// CHECK-DAG: ConstructorFence [<<NewInstance2>>]
+ /// CHECK-NOT: ConstructorFence
/// CHECK-START: void Main.inlineNew3() inliner (after)
/// CHECK-NOT: InvokeStaticOrDirect
@@ -219,5 +257,20 @@
new HaveFinalsAndInheritFromClassWithFinals();
}
+ static int[] mCodePointsEmpty = new int[0];
+
+ /// CHECK-START: void Main.testNewString() inliner (after)
+ /// CHECK-NOT: ConstructorFence
+ /// CHECK: InvokeStaticOrDirect method_load_kind:string_init
+ /// CHECK-NOT: ConstructorFence
+ /// CHECK-NOT: InvokeStaticOrDirect
+ public static void testNewString() {
+ // Strings are special because of StringFactory hackeries.
+ //
+ // Assume they handle their own fencing internally in the StringFactory.
+ int[] codePoints = null;
+ String some_new_string = new String(mCodePointsEmpty, 0, 0);
+ }
+
public static void main(String[] args) {}
}
diff --git a/test/529-checker-unresolved/src/Main.java b/test/529-checker-unresolved/src/Main.java
index 89b9cb4..c2683ac 100644
--- a/test/529-checker-unresolved/src/Main.java
+++ b/test/529-checker-unresolved/src/Main.java
@@ -190,16 +190,18 @@
}
/// CHECK-START: void Main.testLicm(int) licm (before)
- /// CHECK: <<Class:l\d+>> LoadClass loop:B2
- /// CHECK-NEXT: <<Clinit:l\d+>> ClinitCheck [<<Class>>] loop:B2
- /// CHECK-NEXT: <<New:l\d+>> NewInstance [<<Clinit>>] loop:B2
- /// CHECK-NEXT: InvokeUnresolved [<<New>>] loop:B2
+ /// CHECK: <<Class:l\d+>> LoadClass loop:<<LoopLabel:B\d+>>
+ /// CHECK-NEXT: <<Clinit:l\d+>> ClinitCheck [<<Class>>] loop:<<LoopLabel>>
+ /// CHECK-NEXT: <<New:l\d+>> NewInstance [<<Clinit>>] loop:<<LoopLabel>>
+ /// CHECK-NEXT: ConstructorFence [<<New>>] loop:<<LoopLabel>>
+ /// CHECK-NEXT: InvokeUnresolved [<<New>>] loop:<<LoopLabel>>
/// CHECK-START: void Main.testLicm(int) licm (after)
/// CHECK: <<Class:l\d+>> LoadClass loop:none
/// CHECK-NEXT: <<Clinit:l\d+>> ClinitCheck [<<Class>>] loop:none
- /// CHECK: <<New:l\d+>> NewInstance [<<Clinit>>] loop:B2
- /// CHECK-NEXT: InvokeUnresolved [<<New>>] loop:B2
+ /// CHECK: <<New:l\d+>> NewInstance [<<Clinit>>] loop:<<LoopLabel:B\d+>>
+ /// CHECK-NEXT: ConstructorFence [<<New>>] loop:<<LoopLabel>>
+ /// CHECK-NEXT: InvokeUnresolved [<<New>>] loop:<<LoopLabel>>
static public void testLicm(int count) {
// Test to make sure we keep the initialization check after loading an unresolved class.
UnresolvedClass c;
diff --git a/test/563-checker-fakestring/smali/TestCase.smali b/test/563-checker-fakestring/smali/TestCase.smali
index 9f86352..adafb78 100644
--- a/test/563-checker-fakestring/smali/TestCase.smali
+++ b/test/563-checker-fakestring/smali/TestCase.smali
@@ -86,11 +86,11 @@
const v2, 0x0
const v1, 0x1
- new-instance v0, Ljava/lang/String;
+ new-instance v0, Ljava/lang/String; # HNewInstance(String)
# Deoptimize here if the array is too short.
- aget v1, p0, v1
- add-int/2addr v2, v1
+ aget v1, p0, v1 # v1 = int_array[0x1]
+ add-int/2addr v2, v1 # v2 = 0x0 + v1
# Check that we're being executed by the interpreter.
invoke-static {}, LMain;->assertIsInterpreted()V
@@ -98,6 +98,8 @@
# String allocation should succeed.
const-string v3, "UTF8"
invoke-direct {v0, p1, v3}, Ljava/lang/String;-><init>([BLjava/lang/String;)V
+ # Transformed into invoke StringFactory(p1,v3).
+ # The use of v0 is dropped (so HNewInstance(String) ends up having 0 uses and is removed).
# This ArrayGet will throw ArrayIndexOutOfBoundsException.
const v1, 0x4
@@ -125,6 +127,9 @@
const-string v1, "UTF8"
invoke-direct {v0, p0, v1}, Ljava/lang/String;-><init>([BLjava/lang/String;)V
return-object v0
+ # Although it looks like we "use" the new-instance v0 here, the optimizing compiler
+ # transforms all uses of the new-instance into uses of the StringFactory invoke.
+ # therefore the HNewInstance for v0 becomes dead and is removed.
.end method
diff --git a/test/569-checker-pattern-replacement/src/Main.java b/test/569-checker-pattern-replacement/src/Main.java
index 26d87b1..ce7cdb0 100644
--- a/test/569-checker-pattern-replacement/src/Main.java
+++ b/test/569-checker-pattern-replacement/src/Main.java
@@ -330,6 +330,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -346,6 +347,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase(int) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -370,6 +372,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBaseWith0() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -386,6 +389,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: java.lang.String Main.constructBase(java.lang.String) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -410,6 +414,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Null>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: java.lang.String Main.constructBaseWithNullString() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -430,6 +435,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<DValue>>,<<OValue>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase(double, java.lang.Object) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -459,6 +465,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<DValue>>,<<OValue>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase(int, double, java.lang.Object) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -492,6 +499,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<DValue>>,<<OValue>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBaseWith0DoubleNull(double) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -542,6 +550,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase(double) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -566,6 +575,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBaseWith0d() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -604,6 +614,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<JValue>>{{(,[ij]\d+)?}}] method_name:Base.<init>
/// CHECK-START: double Main.constructBase(int, long) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -627,6 +638,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -643,6 +655,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived(int) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -667,6 +680,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerivedWith0() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -683,6 +697,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: java.lang.String Main.constructDerived(java.lang.String) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -701,6 +716,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived(double) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -725,6 +741,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerivedWith0d() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -743,6 +760,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<DValue>>,<<OValue>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived(int, double, java.lang.Object) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -793,6 +811,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived(float) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -820,6 +839,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<DValue>>,<<OValue>>,<<FValue>>{{(,[ij]\d+)?}}] method_name:Derived.<init>
/// CHECK-START: double Main.constructDerived(int, double, java.lang.Object, float) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
@@ -851,6 +871,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>{{(,[ij]\d+)?}}] method_name:BaseWithFinalField.<init>
/// CHECK-START: int Main.constructBaseWithFinalField() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -872,6 +893,7 @@
/// CHECK-START: int Main.constructBaseWithFinalField(int) inliner (after)
/// CHECK-DAG: <<Value:i\d+>> ParameterValue
/// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence
/// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<Value>>]
/// CHECK-DAG: ConstructorFence
@@ -891,6 +913,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:BaseWithFinalField.<init>
/// CHECK-START: int Main.constructBaseWithFinalFieldWith0() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -906,6 +929,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>{{(,[ij]\d+)?}}] method_name:DerivedWithFinalField.<init>
/// CHECK-START: double Main.constructDerivedWithFinalField() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -927,6 +951,7 @@
/// CHECK-START: double Main.constructDerivedWithFinalField(int) inliner (after)
/// CHECK-DAG: <<Value:i\d+>> ParameterValue
/// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence
/// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<Value>>]
/// CHECK-DAG: ConstructorFence
@@ -946,6 +971,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:DerivedWithFinalField.<init>
/// CHECK-START: double Main.constructDerivedWithFinalFieldWith0() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -967,6 +993,7 @@
/// CHECK-START: double Main.constructDerivedWithFinalField(double) inliner (after)
/// CHECK-DAG: <<Value:d\d+>> ParameterValue
/// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence
/// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<Value>>]
/// CHECK-DAG: ConstructorFence
@@ -986,6 +1013,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:DerivedWithFinalField.<init>
/// CHECK-START: double Main.constructDerivedWithFinalFieldWith0d() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -1008,6 +1036,7 @@
/// CHECK-START: double Main.constructDerivedWithFinalField(int, double) inliner (after)
/// CHECK-DAG: <<Value:d\d+>> ParameterValue
/// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: ConstructorFence
/// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<Value>>]
/// CHECK-DAG: ConstructorFence
@@ -1018,6 +1047,7 @@
/// CHECK-START: double Main.constructDerivedWithFinalField(int, double) inliner (after)
/// CHECK-DAG: ConstructorFence
+ /// CHECK-DAG: ConstructorFence
/// CHECK-NOT: ConstructorFence
public static double constructDerivedWithFinalField(int intValue, double doubleValue) {
@@ -1033,6 +1063,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<IValue>>,<<DValue>>{{(,[ij]\d+)?}}] method_name:DerivedWithFinalField.<init>
/// CHECK-START: double Main.constructDerivedWithFinalFieldWith0And0d() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -1048,6 +1079,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>{{(,[ij]\d+)?}}] method_name:DerivedInSecondDex.<init>
/// CHECK-START: int Main.constructDerivedInSecondDex() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -1070,6 +1102,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:DerivedInSecondDex.<init>
/// CHECK-START: int Main.constructDerivedInSecondDex(int) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -1091,6 +1124,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:DerivedInSecondDex.<init>
/// CHECK-START: int Main.constructDerivedInSecondDexWith0() inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
@@ -1106,6 +1140,7 @@
/// CHECK-DAG: InvokeStaticOrDirect [<<Obj>>,<<Value>>{{(,[ij]\d+)?}}] method_name:DerivedInSecondDex.<init>
/// CHECK-START: int Main.constructDerivedInSecondDex(long) inliner (after)
+ /// CHECK: ConstructorFence
/// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: ConstructorFence
/// CHECK-NOT: InstanceFieldSet
diff --git a/test/639-checker-code-sinking/src/Main.java b/test/639-checker-code-sinking/src/Main.java
index 1da19b6..7496925 100644
--- a/test/639-checker-code-sinking/src/Main.java
+++ b/test/639-checker-code-sinking/src/Main.java
@@ -50,7 +50,8 @@
/// CHECK-START: void Main.testSimpleUse() code_sinking (before)
/// CHECK: <<LoadClass:l\d+>> LoadClass class_name:java.lang.Object
- /// CHECK: NewInstance [<<LoadClass>>]
+ /// CHECK: <<New:l\d+>> NewInstance [<<LoadClass>>]
+ /// CHECK: ConstructorFence [<<New>>]
/// CHECK: If
/// CHECK: begin_block
/// CHECK: Throw
@@ -62,7 +63,8 @@
/// CHECK: <<Error:l\d+>> LoadClass class_name:java.lang.Error
/// CHECK: <<LoadClass:l\d+>> LoadClass class_name:java.lang.Object
/// CHECK-NOT: begin_block
- /// CHECK: NewInstance [<<LoadClass>>]
+ /// CHECK: <<New:l\d+>> NewInstance [<<LoadClass>>]
+ /// CHECK: ConstructorFence [<<New>>]
/// CHECK-NOT: begin_block
/// CHECK: NewInstance [<<Error>>]
/// CHECK: Throw