Z codegen does not handle sub-integer compares as the first child of an integral select node #5499
Labels
Comments
|
@r30shah and I looked into this today. This might be more hairy to fix than expected. Address "Unhandled compare node type" assertPatchdiff --git a/compiler/z/codegen/ControlFlowEvaluator.cpp b/compiler/z/codegen/ControlFlowEvaluator.cpp
index be2ff651f..0bda5fbce 100644
--- a/compiler/z/codegen/ControlFlowEvaluator.cpp
+++ b/compiler/z/codegen/ControlFlowEvaluator.cpp
@@ -2410,7 +2422,7 @@ TR::InstOpCode::Mnemonic OMR::Z::TreeEvaluator::getCompareOpFromNode(TR::CodeGen
return TR::InstOpCode::CGR;
}
}
- else if (node->getFirstChild()->getSize() == 4)
+ else
{
if (node->getOpCode().isUnsigned())
{
@@ -2421,10 +2433,6 @@ TR::InstOpCode::Mnemonic OMR::Z::TreeEvaluator::getCompareOpFromNode(TR::CodeGen
return TR::InstOpCode::CR;
}
}
-
- TR_ASSERT(0, "Unhandled compare node type\n");
- return TR::InstOpCode::CR;
-
}The original failing assert is hit here when Resultspencer@zbox:omr/build/fvtest/compilertriltest$ ./comptest --gtest_filter=SelectCompareTest*
Note: Google Test filter = SelectCompareTest*
[==========] Running 5130 tests from 11 test cases.
[----------] 450 tests from SelectCompareTest/Int8SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int8SelectInt32CompareTest (679 ms total)
[----------] 450 tests from SelectCompareTest/Int16SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int16SelectInt32CompareTest (737 ms total)
[----------] 450 tests from SelectCompareTest/Int32SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int32SelectInt32CompareTest (675 ms total)
[----------] 450 tests from SelectCompareTest/Int64SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int64SelectInt32CompareTest (820 ms total)
[----------] 450 tests from SelectCompareTest/FloatSelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/FloatSelectInt32CompareTest (796 ms total)
[----------] 450 tests from SelectCompareTest/DoubleSelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/DoubleSelectInt32CompareTest (808 ms total)
[----------] 450 tests from SelectCompareTest/Int32SelectInt8CompareTest
Assertion failed at /jit/team/spencer/omr/compiler/z/codegen/ControlFlowEvaluator.cpp:2343: 0
Unsupported compare type under select
compiling file:line:name at level: warm
#0: ./comptest(_ZN25TR_LinuxCallStackIterator19printStackBacktraceEPN2TR11CompilationE+0x56) [0x2aa32abcdbe]
#1: ./comptest(_ZN8TR_Debug19printStackBacktraceEv+0x4c) [0x2aa3279e564]
#2: ./comptest(+0x1a4b846) [0x2aa3254b846]
#3: ./comptest(+0x1a4b9b8) [0x2aa3254b9b8]
#4: ./comptest(+0x1a4bbb8) [0x2aa3254bbb8]
#5: ./comptest(_ZN3OMR1Z13TreeEvaluator35getBranchConditionFromCompareOpCodeEN2TR9ILOpCodesE+0x92) [0x2aa32ae4512]
#6: ./comptest(_ZN3OMR1Z13TreeEvaluator15selectEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x75a) [0x2aa32ae52a2]
#7: ./comptest(_ZN3OMR1Z13TreeEvaluator16iselectEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x2e) [0x2aa32b9d74e]
#8: ./comptest(_ZN3OMR13CodeGenerator8evaluateEPN2TR4NodeE+0x222) [0x2aa3283239a]
#9: ./comptest(_ZN3OMR1Z13TreeEvaluator15returnEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x13e) [0x2aa32ade39e]
#10: ./comptest(_ZN3OMR1Z13TreeEvaluator16ireturnEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x2e) [0x2aa32b9af5e]
#11: ./comptest(_ZN3OMR13CodeGenerator8evaluateEPN2TR4NodeE+0x222) [0x2aa3283239a]
#12: ./comptest(_ZN3OMR13CodeGenerator22doInstructionSelectionEv+0x598) [0x2aa32836df8]
#13: ./comptest(_ZN3OMR1Z13CodeGenerator22doInstructionSelectionEv+0x26) [0x2aa327e4a8e]
#14: ./comptest(_ZN3OMR12CodeGenPhase32performInstructionSelectionPhaseEPN2TR13CodeGeneratorEPNS1_12CodeGenPhaseE+0x12a) [0x2aa3284b76a]
#15: ./comptest(_ZN3OMR12CodeGenPhase10performAllEv+0x1d8) [0x2aa32849e20]
#16: ./comptest(_ZN3OMR13CodeGenerator12generateCodeEv+0x104) [0x2aa3283a0f4]
#17: ./comptest(_ZN3OMR11Compilation7compileEv+0xe3e) [0x2aa3244eb56]
#18: ./comptest(_Z24compileMethodFromDetailsP12OMR_VMThreadRN2TR24IlGeneratorMethodDetailsE10TR_HotnessRi+0x7da) [0x2aa32480c62]
#19: ./comptest(_ZN4Tril14SimpleCompiler19compileWithVerifierEPN2TR10IlVerifierE+0x400) [0x2aa323a45c8]
#20: ./comptest(_ZN4Tril14SimpleCompiler7compileEv+0x26) [0x2aa323a41ae]
#21: ./comptest(_ZN46Int32SelectInt8CompareTest_UsingLoadParam_Test8TestBodyEv+0x500) [0x2aa3228bb70]
#22: ./comptest(_ZN7testing8internal38HandleSehExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc+0x84) [0x2aa323f4d3c]
#23: ./comptest(_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc+0x8c) [0x2aa323ec6ac]
#24: ./comptest(_ZN7testing4Test3RunEv+0x100) [0x2aa323c4e20]
#25: ./comptest(_ZN7testing8TestInfo3RunEv+0x15a) [0x2aa323c5a62]
#26: ./comptest(_ZN7testing8TestCase3RunEv+0x162) [0x2aa323c645a]
#27: ./comptest(_ZN7testing8internal12UnitTestImpl11RunAllTestsEv+0x402) [0x2aa323d0152]
#28: ./comptest(_ZN7testing8internal38HandleSehExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc+0x84) [0x2aa323f670c]
#29: ./comptest(_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc+0x8c) [0x2aa323edbb4]
Trace/breakpoint trap (core dumped)Another assert hit Address "Unsupported compare type under select" assertPatchdiff --git a/compiler/z/codegen/ControlFlowEvaluator.cpp b/compiler/z/codegen/ControlFlowEvaluator.cpp
index be2ff651f..0bda5fbce 100644
--- a/compiler/z/codegen/ControlFlowEvaluator.cpp
+++ b/compiler/z/codegen/ControlFlowEvaluator.cpp
@@ -2292,6 +2292,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
{
switch (opCode)
{
+ case TR::bcmpeq:
+ case TR::scmpeq:
case TR::icmpeq:
case TR::acmpeq:
case TR::lcmpeq:
@@ -2299,6 +2301,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
return TR::InstOpCode::COND_BE;
case TR::lcmpne:
@@ -2306,6 +2310,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
return TR::InstOpCode::COND_BNE;
}
break;
+ case TR::bcmplt:
+ case TR::scmplt:
case TR::icmplt:
case TR::iucmplt:
case TR::lcmplt:
@@ -2314,6 +2320,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
return TR::InstOpCode::COND_BL;
}
break;
+ case TR::bcmple:
+ case TR::scmple:
case TR::icmple:
case TR::iucmple:
case TR::lcmple:
@@ -2322,6 +2330,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
return TR::InstOpCode::COND_BNH;
}
break;
+ case TR::bcmpgt:
+ case TR::scmpgt:
case TR::icmpgt:
case TR::iucmpgt:
case TR::lcmpgt:
@@ -2330,6 +2340,8 @@ TR::InstOpCode::S390BranchCondition OMR::Z::TreeEvaluator::getBranchConditionFro
return TR::InstOpCode::COND_BH;
}
break;
+ case TR::bcmpge:
+ case TR::scmpge:
case TR::icmpge:
case TR::iucmpge:
case TR::lcmpge:Addressing this assert is a simple fix; it appears that the byte and short opcodes were left out of the switch statement and simply needed adding in. Resultspencer@zbox:omr/build/fvtest/compilertriltest$ ./comptest --gtest_filter=SelectCompareTest*
Note: Google Test filter = SelectCompareTest*
[==========] Running 5130 tests from 11 test cases.
[----------] 450 tests from SelectCompareTest/Int8SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int8SelectInt32CompareTest (702 ms total)
[----------] 450 tests from SelectCompareTest/Int16SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int16SelectInt32CompareTest (834 ms total)
[----------] 450 tests from SelectCompareTest/Int32SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int32SelectInt32CompareTest (627 ms total)
[----------] 450 tests from SelectCompareTest/Int64SelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/Int64SelectInt32CompareTest (673 ms total)
[----------] 450 tests from SelectCompareTest/FloatSelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/FloatSelectInt32CompareTest (641 ms total)
[----------] 450 tests from SelectCompareTest/DoubleSelectInt32CompareTest
[----------] 450 tests from SelectCompareTest/DoubleSelectInt32CompareTest (647 ms total)
[----------] 450 tests from SelectCompareTest/Int32SelectInt8CompareTest
Assertion failed at /jit/team/spencer/omr/compiler/z/codegen/OMRTreeEvaluator.cpp:5086: 0
generateS390CompareOps: Unexpected child type (Int8) with size = 1
compiling file:line:name at level: warm
#0: ./comptest(_ZN25TR_LinuxCallStackIterator19printStackBacktraceEPN2TR11CompilationE+0x56) [0x2aa3263cdbe]
#1: ./comptest(_ZN8TR_Debug19printStackBacktraceEv+0x4c) [0x2aa3231e564]
#2: ./comptest(+0x1a4b846) [0x2aa320cb846]
#3: ./comptest(+0x1a4b9b8) [0x2aa320cb9b8]
#4: ./comptest(+0x1a4bbb8) [0x2aa320cbbb8]
#5: ./comptest(_Z37generateS390CompareAndBranchOpsHelperPN2TR4NodeEPNS_13CodeGeneratorEN3OMR1Z10InstOpCode19S390BranchConditionES7_RS7_PNS_11LabelSymbolE+0x1d28) [0x2aa3272ae70]
#6: ./comptest(_Z22generateS390CompareOpsPN2TR4NodeEPNS_13CodeGeneratorEN3OMR1Z10InstOpCode19S390BranchConditionES7_PNS_11LabelSymbolE+0x86) [0x2aa3272b1ce]
#7: ./comptest(_Z23generateS390CompareBoolPN2TR4NodeEPNS_13CodeGeneratorEN3OMR10InstOpCode8MnemonicENS4_1Z10InstOpCode19S390BranchConditionES9_b+0x230) [0x2aa3272b440]
#8: ./comptest(_ZN3OMR1Z13TreeEvaluator15icmpltEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x2ec) [0x2aa32660754]
#9: ./comptest(_ZN3OMR1Z13TreeEvaluator15bcmpltEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x2e) [0x2aa32660e46]
#10: ./comptest(_ZN3OMR13CodeGenerator8evaluateEPN2TR4NodeE+0x222) [0x2aa323b239a]
#11: ./comptest(_ZN3OMR1Z13TreeEvaluator15returnEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x13e) [0x2aa3265e39e]
#12: ./comptest(_ZN3OMR1Z13TreeEvaluator16ireturnEvaluatorEPN2TR4NodeEPNS2_13CodeGeneratorE+0x2e) [0x2aa3271af5e]
#13: ./comptest(_ZN3OMR13CodeGenerator8evaluateEPN2TR4NodeE+0x222) [0x2aa323b239a]
#14: ./comptest(_ZN3OMR13CodeGenerator22doInstructionSelectionEv+0x598) [0x2aa323b6df8]
#15: ./comptest(_ZN3OMR1Z13CodeGenerator22doInstructionSelectionEv+0x26) [0x2aa32364a8e]
#16: ./comptest(_ZN3OMR12CodeGenPhase32performInstructionSelectionPhaseEPN2TR13CodeGeneratorEPNS1_12CodeGenPhaseE+0x12a) [0x2aa323cb76a]
#17: ./comptest(_ZN3OMR12CodeGenPhase10performAllEv+0x1d8) [0x2aa323c9e20]
#18: ./comptest(_ZN3OMR13CodeGenerator12generateCodeEv+0x104) [0x2aa323ba0f4]
#19: ./comptest(_ZN3OMR11Compilation7compileEv+0xe3e) [0x2aa31fceb56]
#20: ./comptest(_Z24compileMethodFromDetailsP12OMR_VMThreadRN2TR24IlGeneratorMethodDetailsE10TR_HotnessRi+0x7da) [0x2aa32000c62]
#21: ./comptest(_ZN4Tril14SimpleCompiler19compileWithVerifierEPN2TR10IlVerifierE+0x400) [0x2aa31f245c8]
#22: ./comptest(_ZN4Tril14SimpleCompiler7compileEv+0x26) [0x2aa31f241ae]
#23: ./comptest(_ZN48Int32SelectInt8CompareTest_UsingConstValues_Test8TestBodyEv+0x51a) [0x2aa31e0d002]
#24: ./comptest(_ZN7testing8internal38HandleSehExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc+0x84) [0x2aa31f74d3c]
#25: ./comptest(_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc+0x8c) [0x2aa31f6c6ac]
#26: ./comptest(_ZN7testing4Test3RunEv+0x100) [0x2aa31f44e20]
#27: ./comptest(_ZN7testing8TestInfo3RunEv+0x15a) [0x2aa31f45a62]
#28: ./comptest(_ZN7testing8TestCase3RunEv+0x162) [0x2aa31f4645a]
#29: ./comptest(_ZN7testing8internal12UnitTestImpl11RunAllTestsEv+0x402) [0x2aa31f50152]
Trace/breakpoint trap (core dumped)Another assert hit Address "generateS390CompareOps: Unexpected child type (Int8) with size = 1" assertThis is where it gets complicated. This assert is in the following switch statement: switch (TR::DataType::getSize(firstChild->getDataType()))
{
case 8:
temp.genericAnalyser (node, isUnsignedCmp ? TR::InstOpCode::CLGR : TR::InstOpCode::CGR,
isUnsignedCmp ? TR::InstOpCode::CLG : TR::InstOpCode::CG,
TR::InstOpCode::LGR, true, branchTarget, fBranchOpCond, rBranchOpCond);
returnInstruction = cg->getAppendInstruction();
isBranchGenerated = true;
break;
case 4:
temp.genericAnalyser (node, isUnsignedCmp ? TR::InstOpCode::CLR : TR::InstOpCode::CR,
isUnsignedCmp ? TR::InstOpCode::CL : TR::InstOpCode::C,
TR::InstOpCode::LR, true, branchTarget, fBranchOpCond, rBranchOpCond);
returnInstruction = cg->getAppendInstruction();
isBranchGenerated = true;
break;
case 2:
temp.genericAnalyser(node, isUnsignedCmp ? TR::InstOpCode::CLR : TR::InstOpCode::CR,
isUnsignedCmp ? TR::InstOpCode::CL : TR::InstOpCode::CH,
TR::InstOpCode::LR, true, branchTarget, fBranchOpCond, rBranchOpCond);
returnInstruction = cg->getAppendInstruction();
isBranchGenerated = true;
break;
default:
TR_ASSERT_FATAL( 0, "generateS390CompareOps: Unexpected child type (%s) with size = %d\n", firstChild->getDataType().toString(), TR::DataType::getSize(firstChild->getDataType()));
break;
}The assert is hit with byte nodes as there is no |
|
Even in |
|
I'm assuming a 16-bit value is extended to 32 bits when it comes into a register, so 32-bit compares between registers should be fine. The |
|
@Spencer-Comin , it would be interesting to see how |
|
The only test I'm seeing failing with the assert at [1] is the following https://github.com/eclipse/omr/blob/efe8caf43537270e978f504215342bfba466f0b0/fvtest/compilertriltest/SelectTest.cpp#L1428-L1463 (method return=Int32 args=[Int8, Int8]
(block
(ireturn
(iselect
(bcmpeq
(bload parm=0)
(bload parm=1))
(iconst 1)
(iconst 0)))))Which generates the following instructions with the assert removed stg %r15,120(%r15)
lay %r15,-160(%r15)
stg %r2,176(%r15)
stg %r3,184(%r15)
lhi %r2,1
clc 183(1,%r15),191(%r15)
je L1
xr %r2,%r2
L1:
lgfr %r2,%r2
lg %r15,280(%r15)
br %r14These instructions seem fine to me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As part of refactoring the
*selectevaluators on Power in #5497, new Tril tests were added to exercise previously untested codegen optimizations for when the first child is a compare. When running these tests on Z, the following assertion was observed inSelectCompareTest/Int32SelectInt8CompareTest:From looking at the code, it appears that the Z codegen's evaluator for integral select nodes has an optimization for when the first child is a compare node, but it does not handle
Int8orInt16compares even though Z does have evaluators for these opcodes.The text was updated successfully, but these errors were encountered: