Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
@rwestrel
rwestrel committed Oct 12, 2020
1 parent 01b0039 commit 60561b7
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 64 deletions.
12 changes: 6 additions & 6 deletions src/hotspot/share/c1/c1_Instruction.hpp
View file
Expand Up @@ -2724,12 +2724,12 @@ LEAF(ProfileACmpTypes, Instruction)

ciMethod* method() const { return _method; }
int bci() const { return _bci; }
Value left() const { return _left; }
Value right() const { return _right; }
bool left_maybe_null() const { return _left_maybe_null; }
bool right_maybe_null() const { return _right_maybe_null; }
void set_left_maybe_null(bool v) { _left_maybe_null = v; }
void set_right_maybe_null(bool v) { _right_maybe_null = v; }
Value left() const { return _left; }
Value right() const { return _right; }
bool left_maybe_null() const { return _left_maybe_null; }
bool right_maybe_null() const { return _right_maybe_null; }
void set_left_maybe_null(bool v) { _left_maybe_null = v; }
void set_right_maybe_null(bool v) { _right_maybe_null = v; }

virtual void input_values_do(ValueVisitor* f) {
if (_left != NULL) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/methodData.hpp
View file
Expand Up @@ -394,7 +394,7 @@ class ProfileData : public ResourceObj {
virtual bool is_ParametersTypeData() const { return false; }
virtual bool is_SpeculativeTrapData()const { return false; }
virtual bool is_ArrayLoadStoreData() const { return false; }
virtual bool is_ACmpData() const { return false; }
virtual bool is_ACmpData() const { return false; }


BitData* as_BitData() const {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/c2_globals.hpp
View file
Expand Up @@ -780,7 +780,7 @@
product(bool, UseArrayLoadStoreProfile, true, \
"Take advantage of profiling at array load/store") \
\
product(bool, UseACmpProfile, false, \
product(bool, UseACmpProfile, true, \
"Take advantage of profiling at acmp") \
\
product(bool, ExpandSubTypeCheckAtParseTime, false, DIAGNOSTIC, \
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/parse.hpp
View file
Expand Up @@ -549,7 +549,7 @@ class Parse : public GraphKit {

void do_ifnull(BoolTest::mask btest, Node* c);
void do_if(BoolTest::mask btest, Node* c, bool new_path = false, Node** ctrl_taken = NULL);
void do_acmp(BoolTest::mask btest, Node* a, Node* b);
void do_acmp(BoolTest::mask btest, Node* left, Node* right);
void acmp_always_null_input(Node* input, const TypeOopPtr* tinput, BoolTest::mask btest, Node* eq_region);
void acmp_known_non_inline_type_input(Node* input, const TypeOopPtr* tinput, ProfilePtrKind input_ptr, ciKlass* input_type, BoolTest::mask btest, Node* eq_region);
Node* acmp_null_check(Node* input, const TypeOopPtr* tinput, ProfilePtrKind input_ptr, Node*& null_ctl);
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/opto/parse2.cpp
View file
Expand Up @@ -2177,7 +2177,7 @@ void Parse::acmp_unknown_non_inline_type_input(Node* input, const TypeOopPtr* ti
}
}

void Parse::do_acmp(BoolTest::mask btest, Node* right, Node* left) {
void Parse::do_acmp(BoolTest::mask btest, Node* left, Node* right) {
ciKlass* left_type = NULL;
ciKlass* right_type = NULL;
ProfilePtrKind left_ptr = ProfileUnknownNull;
Expand Down Expand Up @@ -2310,12 +2310,12 @@ void Parse::do_acmp(BoolTest::mask btest, Node* right, Node* left) {
return;
}
if (!left_inline_type) {
// Comparison with an object of known not to be an inline type
// Comparison with an object known not to be an inline type
acmp_unknown_non_inline_type_input(left, tleft, left_ptr, btest, eq_region);
return;
}
if (!right_inline_type) {
// Comparison with an object of known not to be an inline type
// Comparison with an object known not to be an inline type
acmp_unknown_non_inline_type_input(right, tright, right_ptr, btest, eq_region);
return;
}
Expand Down Expand Up @@ -3546,7 +3546,7 @@ void Parse::do_one_bytecode() {
maybe_add_safepoint(iter().get_dest());
a = pop();
b = pop();
do_acmp(btest, a, b);
do_acmp(btest, b, a);
break;

case Bytecodes::_ifeq: btest = BoolTest::eq; goto handle_ifxx;
Expand Down
4 changes: 1 addition & 3 deletions test/hotspot/jtreg/compiler/valhalla/inlinetypes/InlineTypeTest.java
View file
Expand Up @@ -236,7 +236,7 @@ public abstract class InlineTypeTest {
protected static final String CHECKCAST_ARRAYCOPY = "(.*call_leaf_nofp,runtime checkcast_arraycopy.*" + END;
protected static final String JLONG_ARRAYCOPY = "(.*call_leaf_nofp,runtime jlong_disjoint_arraycopy.*" + END;
protected static final String FIELD_ACCESS = "(.*Field: *" + END;
protected static final String SUBSTITUTABLITY_TEST = START + "CallStaticJava" + MID + "java.lang.invoke.ValueBootstrapMethods::isSubstitutable" + END;
protected static final String SUBSTITUTABILITY_TEST = START + "CallStaticJava" + MID + "java.lang.invoke.ValueBootstrapMethods::isSubstitutable" + END;

public static String[] concat(String prefix[], String... extra) {
ArrayList<String> list = new ArrayList<String>();
Expand Down Expand Up @@ -297,7 +297,6 @@ public String[] getVMParameters(int scenario) {
"-XX:+InlineTypeReturnedAsFields",
"-XX:+StressInlineTypeReturnedAsFields"};
case 3: return new String[] {
"-XX:-UseACmpProfile",
"-DVerifyIR=false",
"-XX:+AlwaysIncrementalInline",
"-XX:FlatArrayElementMaxOops=0",
Expand All @@ -306,7 +305,6 @@ public String[] getVMParameters(int scenario) {
"-XX:+InlineTypePassFieldsAsArgs",
"-XX:+InlineTypeReturnedAsFields"};
case 4: return new String[] {
"-XX:-UseACmpProfile",
"-DVerifyIR=false",
"-XX:FlatArrayElementMaxOops=-1",
"-XX:FlatArrayElementMaxSize=-1",
Expand Down
8 changes: 4 additions & 4 deletions test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java
View file
Expand Up @@ -3385,9 +3385,9 @@ public void test123_verifier(boolean warmup) {
}
}

// acmp doesn't need substitutablity test when one input is known
// acmp doesn't need substitutability test when one input is known
// not to be a value type
@Test(failOn = SUBSTITUTABLITY_TEST)
@Test(failOn = SUBSTITUTABILITY_TEST)
public boolean test124(Integer o1, Object o2) {
return o1 == o2;
}
Expand All @@ -3398,8 +3398,8 @@ public void test124_verifier(boolean warmup) {
test124(42, testValue1);
}

// acmp doesn't need substitutablity test when one input null
@Test(failOn = SUBSTITUTABLITY_TEST)
// acmp doesn't need substitutability test when one input null
@Test(failOn = SUBSTITUTABILITY_TEST)
public boolean test125(Object o1) {
Object o2 = null;
return o1 == o2;
Expand Down
88 changes: 44 additions & 44 deletions test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorldProfiling.java
View file
Expand Up @@ -491,7 +491,7 @@ public void test20_verifier(boolean warmup) {

// branch frequency profiling causes not equal branch to be optimized out
@Warmup(10000)
@Test(failOn = SUBSTITUTABLITY_TEST)
@Test(failOn = SUBSTITUTABILITY_TEST)
public boolean test21(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -504,8 +504,8 @@ public void test21_verifier(boolean warmup) {

// Input profiled non null
@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test22(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -524,9 +524,9 @@ public void test22_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test23(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -545,8 +545,8 @@ public void test23_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test24(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -565,9 +565,9 @@ public void test24_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_ASSERT_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test25(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -587,9 +587,9 @@ public void test25_verifier(boolean warmup) {

// Input profiled not inline type with known type
@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test26(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -610,8 +610,8 @@ public void test26_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test27(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -632,9 +632,9 @@ public void test27_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test28(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -655,8 +655,8 @@ public void test28_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test29(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -677,9 +677,9 @@ public void test29_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test30(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -701,8 +701,8 @@ public void test30_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST + NULL_CHECK_TRAP)
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST + NULL_CHECK_TRAP)
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test31(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -725,8 +725,8 @@ public void test31_verifier(boolean warmup) {

// Input profiled not inline type with unknown type
@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test32(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -748,8 +748,8 @@ public void test32_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test33(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -771,8 +771,8 @@ public void test33_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test34(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -794,8 +794,8 @@ public void test34_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { NULL_CHECK_TRAP, CLASS_CHECK_TRAP }, matchCount = { 1, 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test35(Object o1, Object o2) {
return o1 != o2;
}
Expand All @@ -817,8 +817,8 @@ public void test35_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST + NULL_CHECK_TRAP, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test36(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -840,8 +840,8 @@ public void test36_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST + NULL_CHECK_TRAP)
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1})
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST + NULL_CHECK_TRAP)
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1})
public boolean test37(Object o1, Object o2) {
return o1 == o2;
}
Expand All @@ -865,9 +865,9 @@ public void test37_verifier(boolean warmup) {
// Test that acmp profile data that's unused at the acmp is fed to
// speculation and leverage later
@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1 })
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1 })
public void test38(Object o1, Object o2, Object o3) {
if (o1 == o2) {
test38_helper2();
Expand All @@ -890,9 +890,9 @@ public void test38_verifier(boolean warmup) {
}

@Warmup(10000)
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABLITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABLITY_TEST }, matchCount = { 1 })
@Test(valid = ACmpProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(valid = TypeProfileOn, failOn = SUBSTITUTABILITY_TEST, match = { CLASS_CHECK_TRAP }, matchCount = { 1})
@Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1 })
public void test39(Object o1, Object o2, Object o3) {
if (o1 == o2) {
test39_helper2();
Expand Down

0 comments on commit 60561b7

Please sign in to comment.