Skip to content

Commit 73e2aa2

Browse files
pshiptonpshipton
authored
Merge pull request #20595 from eclipse-openj9/revert-20530-minmax
Revert "Support Java Behaviour w.r.t Math.max and Math.min for Floating Points"
2 parents 27fba55 + 962df50 commit 73e2aa2

File tree

8 files changed

+102
-485
lines changed

8 files changed

+102
-485
lines changed

‎runtime/compiler/codegen/J9CodeGenerator.hpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -512,16 +512,6 @@ void addMonClass(TR::Node* monNode, TR_OpaqueClassBlock* clazz);
512512
*/
513513
void setSupportsInlineVectorizedHashCode() { _j9Flags.set(SupportsInlineVectorizedHashCode); }
514514

515-
/** \brief
516-
* Determines whether the code generator supports inlining of java_lang_Math_max/min_F/D
517-
*/
518-
bool getSupportsInlineMath_MaxMin_FD() { return _j9Flags.testAny(SupportsInlineMath_MaxMin_FD); }
519-
520-
/** \brief
521-
* The code generator supports inlining of java_lang_Math_max/min_F/D
522-
*/
523-
void setSupportsInlineMath_MaxMin_FD() { _j9Flags.set(SupportsInlineMath_MaxMin_FD); }
524-
525515
/**
526516
* \brief
527517
* The number of nodes between a monext and the next monent before
@@ -687,7 +677,6 @@ void addMonClass(TR::Node* monNode, TR_OpaqueClassBlock* clazz);
687677
SavesNonVolatileGPRsForGC = 0x00000800,
688678
SupportsInlineVectorizedMismatch = 0x00001000,
689679
SupportsInlineVectorizedHashCode = 0x00002000,
690-
SupportsInlineMath_MaxMin_FD = 0x00004000,
691680
};
692681

693682
flags32_t _j9Flags;

‎runtime/compiler/env/j9method.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5069,10 +5069,6 @@ TR_ResolvedJ9Method::setRecognizedMethodInfo(TR::RecognizedMethod rm)
50695069
case TR::java_lang_Math_min_I:
50705070
case TR::java_lang_Math_max_L:
50715071
case TR::java_lang_Math_min_L:
5072-
case TR::java_lang_Math_max_F:
5073-
case TR::java_lang_Math_min_F:
5074-
case TR::java_lang_Math_max_D:
5075-
case TR::java_lang_Math_min_D:
50765072
case TR::java_lang_Math_abs_I:
50775073
case TR::java_lang_Math_abs_L:
50785074
case TR::java_lang_Math_abs_F:

‎runtime/compiler/optimizer/J9RecognizedCallTransformer.cpp

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,11 +1367,6 @@ bool J9::RecognizedCallTransformer::isInlineable(TR::TreeTop* treetop)
13671367
case TR::java_lang_Math_max_L:
13681368
case TR::java_lang_Math_min_L:
13691369
return !comp()->getOption(TR_DisableMaxMinOptimization);
1370-
case TR::java_lang_Math_max_F:
1371-
case TR::java_lang_Math_min_F:
1372-
case TR::java_lang_Math_max_D:
1373-
case TR::java_lang_Math_min_D:
1374-
return !comp()->getOption(TR_DisableMaxMinOptimization) && cg()->getSupportsInlineMath_MaxMin_FD();
13751370
case TR::java_lang_Math_multiplyHigh:
13761371
return cg()->getSupportsLMulHigh();
13771372
case TR::java_lang_StringUTF16_toBytes:
@@ -1500,18 +1495,6 @@ void J9::RecognizedCallTransformer::transform(TR::TreeTop* treetop)
15001495
case TR::java_lang_Math_min_L:
15011496
processIntrinsicFunction(treetop, node, TR::lmin);
15021497
break;
1503-
case TR::java_lang_Math_max_F:
1504-
processIntrinsicFunction(treetop, node, TR::fmax);
1505-
break;
1506-
case TR::java_lang_Math_min_F:
1507-
processIntrinsicFunction(treetop, node, TR::fmin);
1508-
break;
1509-
case TR::java_lang_Math_max_D:
1510-
processIntrinsicFunction(treetop, node, TR::dmax);
1511-
break;
1512-
case TR::java_lang_Math_min_D:
1513-
processIntrinsicFunction(treetop, node, TR::dmin);
1514-
break;
15151498
case TR::java_lang_Math_multiplyHigh:
15161499
processIntrinsicFunction(treetop, node, TR::lmulh);
15171500
break;

‎runtime/compiler/z/codegen/J9CodeGenerator.cpp

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,6 @@ J9::Z::CodeGenerator::initialize()
133133
cg->setSupportsInlineEncodeASCII();
134134
}
135135

136-
static bool disableInlineMath_MaxMin_FD = feGetEnv("TR_disableInlineMaxMin") != NULL;
137-
if (!disableInlineMath_MaxMin_FD)
138-
{
139-
cg->setSupportsInlineMath_MaxMin_FD();
140-
}
141-
142136
static bool disableInlineVectorizedMismatch = feGetEnv("TR_disableInlineVectorizedMismatch") != NULL;
143137
if (cg->getSupportsArrayCmpLen() &&
144138
#if defined(J9VM_GC_SPARSE_HEAP_ALLOCATION)
@@ -4132,24 +4126,20 @@ J9::Z::CodeGenerator::inlineDirectCall(
41324126
}
41334127
}
41344128

4135-
if (!self()->comp()->getOption(TR_DisableMaxMinOptimization) && cg->getSupportsInlineMath_MaxMin_FD()) {
4136-
switch (methodSymbol->getRecognizedMethod()) {
4129+
if (!comp->getOption(TR_DisableSIMDDoubleMaxMin) && cg->getSupportsVectorRegisters())
4130+
{
4131+
switch (methodSymbol->getRecognizedMethod())
4132+
{
41374133
case TR::java_lang_Math_max_D:
4138-
resultReg = J9::Z::TreeEvaluator::dmaxEvaluator(node, cg);
4134+
resultReg = TR::TreeEvaluator::inlineDoubleMax(node, cg);
41394135
return true;
41404136
case TR::java_lang_Math_min_D:
4141-
resultReg = J9::Z::TreeEvaluator::dminEvaluator(node, cg);
4142-
return true;
4143-
case TR::java_lang_Math_max_F:
4144-
resultReg = J9::Z::TreeEvaluator::fmaxEvaluator(node, cg);
4145-
return true;
4146-
case TR::java_lang_Math_min_F:
4147-
resultReg = J9::Z::TreeEvaluator::fminEvaluator(node, cg);
4137+
resultReg = TR::TreeEvaluator::inlineDoubleMin(node, cg);
41484138
return true;
41494139
default:
41504140
break;
4141+
}
41514142
}
4152-
}
41534143

41544144
switch (methodSymbol->getRecognizedMethod())
41554145
{

‎runtime/compiler/z/codegen/J9TreeEvaluator.cpp

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -906,48 +906,76 @@ allocateWriteBarrierInternalPointerRegister(TR::CodeGenerator * cg, TR::Node * s
906906
}
907907

908908

909-
TR::Register *
910-
J9::Z::TreeEvaluator::dmaxEvaluator(TR::Node * node, TR::CodeGenerator * cg)
909+
extern TR::Register *
910+
doubleMaxMinHelper(TR::Node *node, TR::CodeGenerator *cg, bool isMaxOp)
911911
{
912-
if (cg->getSupportsVectorRegisters())
913-
{
914-
cg->generateDebugCounter("z13/simd/doubleMax", 1, TR::DebugCounter::Free);
915-
return OMR::Z::TreeEvaluator::fpMinMaxVectorHelper(node, cg);
916-
}
917-
return OMR::Z::TreeEvaluator::xmaxxminHelper(node, cg);
918-
}
912+
TR_ASSERT(node->getNumChildren() >= 1 || node->getNumChildren() <= 2, "node has incorrect number of children");
919913

920-
TR::Register *
921-
J9::Z::TreeEvaluator::dminEvaluator(TR::Node * node, TR::CodeGenerator * cg)
922-
{
923-
if (cg->getSupportsVectorRegisters())
924-
{
925-
cg->generateDebugCounter("z13/simd/doubleMin", 1, TR::DebugCounter::Free);
926-
return OMR::Z::TreeEvaluator::fpMinMaxVectorHelper(node, cg);
927-
}
928-
return OMR::Z::TreeEvaluator::xmaxxminHelper(node, cg);
929-
}
914+
/* ===================== Allocating Registers ===================== */
930915

931-
TR::Register *
932-
J9::Z::TreeEvaluator::fmaxEvaluator(TR::Node * node, TR::CodeGenerator * cg)
933-
{
934-
if (cg->getSupportsVectorRegisters())
916+
TR::Register * v16 = cg->allocateRegister(TR_VRF);
917+
TR::Register * v17 = cg->allocateRegister(TR_VRF);
918+
TR::Register * v18 = cg->allocateRegister(TR_VRF);
919+
920+
/* ===================== Generating instructions ===================== */
921+
922+
/* ====== LD FPR0,16(GPR5) Load a ====== */
923+
TR::Register * v0 = cg->fprClobberEvaluate(node->getFirstChild());
924+
925+
/* ====== LD FPR2, 0(GPR5) Load b ====== */
926+
TR::Register * v2 = cg->evaluate(node->getSecondChild());
927+
928+
/* ====== WFTCIDB V16,V0,X'F' a == NaN ====== */
929+
generateVRIeInstruction(cg, TR::InstOpCode::VFTCI, node, v16, v0, 0xF, 8, 3);
930+
931+
/* ====== For Max: WFCHE V17,V0,V2 Compare a >= b ====== */
932+
if(isMaxOp)
935933
{
936-
cg->generateDebugCounter("z13/simd/floatMax", 1, TR::DebugCounter::Free);
937-
return OMR::Z::TreeEvaluator::fpMinMaxVectorHelper(node, cg);
934+
generateVRRcInstruction(cg, TR::InstOpCode::VFCH, node, v17, v0, v2, 0, 8, 3);
938935
}
939-
return OMR::Z::TreeEvaluator::xmaxxminHelper(node, cg);
940-
}
941-
942-
TR::Register *
943-
J9::Z::TreeEvaluator::fminEvaluator(TR::Node * node, TR::CodeGenerator * cg)
944-
{
945-
if (cg->getSupportsVectorRegisters())
936+
/* ====== For Min: WFCHE V17,V0,V2 Compare a <= b ====== */
937+
else
946938
{
947-
cg->generateDebugCounter("z13/simd/floatMin", 1, TR::DebugCounter::Free);
948-
return OMR::Z::TreeEvaluator::fpMinMaxVectorHelper(node, cg);
939+
generateVRRcInstruction(cg, TR::InstOpCode::VFCH, node, v17, v2, v0, 0, 8, 3);
949940
}
950-
return OMR::Z::TreeEvaluator::xmaxxminHelper(node, cg);
941+
942+
/* ====== VO V16,V16,V17 (a >= b) || (a == NaN) ====== */
943+
generateVRRcInstruction(cg, TR::InstOpCode::VO, node, v16, v16, v17, 0, 0, 0);
944+
945+
/* ====== For Max: WFTCIDB V17,V0,X'800' a == +0 ====== */
946+
if(isMaxOp)
947+
{
948+
generateVRIeInstruction(cg, TR::InstOpCode::VFTCI, node, v17, v0, 0x800, 8, 3);
949+
}
950+
/* ====== For Min: WFTCIDB V17,V0,X'400' a == -0 ====== */
951+
else
952+
{
953+
generateVRIeInstruction(cg, TR::InstOpCode::VFTCI, node, v17, v0, 0x400, 8, 3);
954+
}
955+
/* ====== WFTCIDB V18,V2,X'C00' b == 0 ====== */
956+
generateVRIeInstruction(cg, TR::InstOpCode::VFTCI, node, v18, v2, 0xC00, 8, 3);
957+
958+
/* ====== VN V17,V17,V18 (a == -0) && (b == 0) ====== */
959+
generateVRRcInstruction(cg, TR::InstOpCode::VN, node, v17, v17, v18, 0, 0, 0);
960+
961+
/* ====== VO V16,V16,V17 (a >= b) || (a == NaN) || ((a == -0) && (b == 0)) ====== */
962+
generateVRRcInstruction(cg, TR::InstOpCode::VO, node, v16, v16, v17, 0, 0, 0);
963+
964+
/* ====== VSEL V0,V0,V2,V16 ====== */
965+
generateVRReInstruction(cg, TR::InstOpCode::VSEL, node, v0, v0, v2, v16);
966+
967+
/* ===================== Deallocating Registers ===================== */
968+
cg->stopUsingRegister(v2);
969+
cg->stopUsingRegister(v16);
970+
cg->stopUsingRegister(v17);
971+
cg->stopUsingRegister(v18);
972+
973+
node->setRegister(v0);
974+
975+
cg->decReferenceCount(node->getFirstChild());
976+
cg->decReferenceCount(node->getSecondChild());
977+
978+
return node->getRegister();
951979
}
952980

953981
TR::Register*
@@ -2917,7 +2945,19 @@ J9::Z::TreeEvaluator::toLowerIntrinsic(TR::Node *node, TR::CodeGenerator *cg, bo
29172945
return caseConversionHelper(node, cg, false, isCompressedString);
29182946
}
29192947

2948+
TR::Register*
2949+
J9::Z::TreeEvaluator::inlineDoubleMax(TR::Node *node, TR::CodeGenerator *cg)
2950+
{
2951+
cg->generateDebugCounter("z13/simd/doubleMax", 1, TR::DebugCounter::Free);
2952+
return doubleMaxMinHelper(node, cg, true);
2953+
}
29202954

2955+
TR::Register*
2956+
J9::Z::TreeEvaluator::inlineDoubleMin(TR::Node *node, TR::CodeGenerator *cg)
2957+
{
2958+
cg->generateDebugCounter("z13/simd/doubleMin", 1, TR::DebugCounter::Free);
2959+
return doubleMaxMinHelper(node, cg, false);
2960+
}
29212961

29222962
TR::Register *
29232963
J9::Z::TreeEvaluator::inlineMathFma(TR::Node *node, TR::CodeGenerator *cg)

‎runtime/compiler/z/codegen/J9TreeEvaluator.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,8 @@ class OMR_EXTENSIBLE TreeEvaluator: public J9::TreeEvaluator
126126
*/
127127
static TR::Register *inlineVectorizedStringIndexOf(TR::Node *node, TR::CodeGenerator *cg, bool isCompressed);
128128
static TR::Register *inlineIntrinsicIndexOf(TR::Node *node, TR::CodeGenerator *cg, bool isLatin1);
129-
static TR::Register *fminEvaluator(TR::Node *node, TR::CodeGenerator *cg);
130-
static TR::Register *dminEvaluator(TR::Node *node, TR::CodeGenerator *cg);
131-
static TR::Register *fmaxEvaluator(TR::Node *node, TR::CodeGenerator *cg);
132-
static TR::Register *dmaxEvaluator(TR::Node *node, TR::CodeGenerator *cg);
129+
static TR::Register *inlineDoubleMax(TR::Node *node, TR::CodeGenerator *cg);
130+
static TR::Register *inlineDoubleMin(TR::Node *node, TR::CodeGenerator *cg);
133131
static TR::Register *inlineMathFma(TR::Node *node, TR::CodeGenerator *cg);
134132

135133
/* This Evaluator generates the SIMD routine for methods

‎test/functional/JIT_Test/src/jit/test/recognizedMethod/TestJavaLangMath.java

Lines changed: 18 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -23,37 +23,28 @@
2323
package jit.test.recognizedMethod;
2424
import org.testng.AssertJUnit;
2525
import org.testng.annotations.Test;
26-
import java.util.Random;
27-
import org.testng.asserts.SoftAssert;
28-
import static jit.test.recognizedMethod.TestMathUtils.*;
29-
import org.testng.annotations.DataProvider;
30-
import org.testng.annotations.Listeners;
31-
import org.testng.AssertJUnit;
32-
3326

34-
35-
@Test(singleThreaded=true)
3627
public class TestJavaLangMath {
3728

3829
/**
39-
* Tests the constant corner cases defined by the {@link Math.sqrt} method.
40-
* <p>
41-
* The JIT compiler will transform calls to {@link Math.sqrt} within this test
42-
* into the following tree sequence:
43-
*
44-
* <code>
45-
* dsqrt
46-
* dconst <x>
47-
* </code>
48-
*
49-
* Subsequent tree simplification passes will attempt to reduce this constant
50-
* operation to a <code>dsqrt</code> IL by performing the square root at compile
51-
* time. The transformation will be performed when the function get executed
52-
* twice, therefore, the "invocationCount=2" is needed. However we must ensure the
53-
* result of the square root done by the compiler at compile time will be exactly
54-
* the same as the result had it been done by the Java runtime at runtime. This
55-
* test validates the results are the same.
56-
*/
30+
* Tests the constant corner cases defined by the {@link Math.sqrt} method.
31+
* <p>
32+
* The JIT compiler will transform calls to {@link Math.sqrt} within this test
33+
* into the following tree sequence:
34+
*
35+
* <code>
36+
* dsqrt
37+
* dconst <x>
38+
* </code>
39+
*
40+
* Subsequent tree simplification passes will attempt to reduce this constant
41+
* operation to a <code>dsqrt</code> IL by performing the square root at compile
42+
* time. The transformation will be performed when the function get executed
43+
* twice, therefore, the "invocationCount=2" is needed. However we must ensure the
44+
* result of the square root done by the compiler at compile time will be exactly
45+
* the same as the result had it been done by the Java runtime at runtime. This
46+
* test validates the results are the same.
47+
*/
5748
@Test(groups = {"level.sanity"}, invocationCount=2)
5849
public void test_java_lang_Math_sqrt() {
5950
AssertJUnit.assertTrue(Double.isNaN(Math.sqrt(Double.NEGATIVE_INFINITY)));
@@ -64,82 +55,4 @@ public void test_java_lang_Math_sqrt() {
6455
AssertJUnit.assertEquals(Double.POSITIVE_INFINITY, Math.sqrt(Double.POSITIVE_INFINITY));
6556
AssertJUnit.assertTrue(Double.isNaN(Math.sqrt(Double.NaN)));
6657
}
67-
68-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="zeroProviderFD", dataProviderClass=TestMathUtils.class)
69-
public void test_java_lang_Math_min_zeros_FD(Number a, Number b, boolean isFirstArg) {
70-
if (a instanceof Float) {
71-
float f1 = a.floatValue();
72-
float f2 = b.floatValue();
73-
assertEquals(Math.min(f1, f2), isFirstArg ? f1 : f2);
74-
} else {
75-
double f1 = a.doubleValue();
76-
double f2 = b.doubleValue();
77-
assertEquals(Math.min(f1, f2), isFirstArg ? f1 : f2);
78-
}
79-
}
80-
81-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="zeroProviderFD", dataProviderClass=TestMathUtils.class)
82-
public void test_java_lang_Math_max_zeros_FD(Number a, Number b, boolean isFirstArg) {
83-
if (a instanceof Float) {
84-
float f1 = a.floatValue();
85-
float f2 = b.floatValue();
86-
assertEquals(Math.max(f1, f2), isFirstArg ? f2 : f1);
87-
} else {
88-
double f1 = a.doubleValue();
89-
double f2 = b.doubleValue();
90-
assertEquals(Math.max(f1, f2), isFirstArg ? f2 : f1);
91-
}
92-
}
93-
94-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="nanProviderFD", dataProviderClass=TestMathUtils.class)
95-
public void test_java_lang_Math_min_nan_FD(Number a, Number b, Number expected) {
96-
if (a instanceof Float) {
97-
float f1 = a.floatValue();
98-
float f2 = b.floatValue();
99-
AssertJUnit.assertTrue(Float.isNaN(Math.min(f1, f2)));
100-
} else {
101-
double f1 = a.doubleValue();
102-
double f2 = b.doubleValue();
103-
AssertJUnit.assertTrue(Double.isNaN(Math.min(f1, f2)));
104-
}
105-
}
106-
107-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="nanProviderFD", dataProviderClass=TestMathUtils.class)
108-
public void test_java_lang_Math_max_nan_FD(Number a, Number b, Number expected) {
109-
if (a instanceof Float) {
110-
float f1 = a.floatValue();
111-
float f2 = b.floatValue();
112-
AssertJUnit.assertTrue(Float.isNaN(Math.max(f1, f2)));
113-
} else {
114-
double f1 = a.doubleValue();
115-
double f2 = b.doubleValue();
116-
AssertJUnit.assertTrue(Double.isNaN(Math.max(f1, f2)));
117-
}
118-
}
119-
120-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="normalNumberProviderFD", dataProviderClass=TestMathUtils.class)
121-
public void test_java_lang_Math_min_normal_FD(Number a, Number b){
122-
if (a instanceof Float) {
123-
float f1 = a.floatValue();
124-
float f2 = b.floatValue();
125-
AssertJUnit.assertEquals(Math.min(f1, f2), f1 <= f2 ? f1 : f2);
126-
} else {
127-
double f1 = a.doubleValue();
128-
double f2 = b.doubleValue();
129-
AssertJUnit.assertEquals(Math.min(f1, f2), f1 <= f2 ? f1 : f2);
130-
}
131-
}
132-
133-
@Test(groups = {"level.sanity"}, invocationCount=2, dataProvider="normalNumberProviderFD", dataProviderClass=TestMathUtils.class)
134-
public void test_java_lang_Math_max_normal_FD(Number a, Number b){
135-
if (a instanceof Float) {
136-
float f1 = a.floatValue();
137-
float f2 = b.floatValue();
138-
AssertJUnit.assertEquals(Math.max(f1, f2), f1 >= f2 ? f1 : f2);
139-
} else {
140-
double f1 = a.doubleValue();
141-
double f2 = b.doubleValue();
142-
AssertJUnit.assertEquals(Math.max(f1, f2), f1 >= f2 ? f1 : f2);
143-
}
144-
}
14558
}

0 commit comments

Comments
 (0)