Skip to content

Commit

Permalink
Fix vector load/store instruction large displacement bug
Browse files Browse the repository at this point in the history 
Fix a problem in VRS, VSI, and VRV vector-storage instruction
formats to handle large displacements correctly.

The fix consists of two parts: an instruction selection phase change and
a binary encoding change. Both of these changes aim to use a scratch register
to hold the base+displacement value if a vector load/store instruction has a
displacement that exceeds the 12-bit signed integer range.

Signed-off-by: Nigel Yu <yunigel@ca.ibm.com>
  • Loading branch information
@NigelYiboYu
NigelYiboYu committed Feb 5, 2019
1 parent d85304f commit f8042f4
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 78 deletions.
41 changes: 19 additions & 22 deletions compiler/z/codegen/OMRMemoryReference.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,11 @@ OMR::Z::MemoryReference::alignmentBumpMayRequire4KFixup(TR::Node * node, TR::Cod
return false;
}

/**
* \brief
* Enforces format limits on several instruction formats that have a displacement and base register without
* an index register. This routine was originally meant for SS formats and is now used on SS, VRS, VRV and VSI formats.
*/
TR::Instruction *
OMR::Z::MemoryReference::enforceSSFormatLimits(TR::Node * node, TR::CodeGenerator * cg, TR::Instruction *preced)
{
Expand All @@ -2047,11 +2052,7 @@ OMR::Z::MemoryReference::enforceSSFormatLimits(TR::Node * node, TR::CodeGenerato
TR::Instruction *
OMR::Z::MemoryReference::enforceRSLFormatLimits(TR::Node * node, TR::CodeGenerator * cg, TR::Instruction *preced)
{
bool forceDueToAlignmentBump = self()->alignmentBumpMayRequire4KFixup(node, cg);
// call separateIndexRegister first so any large offset is handled along with the index register folding
preced = self()->separateIndexRegister(node, cg, true, preced, forceDueToAlignmentBump); // enforce4KDisplacementLimit=true
preced = self()->enforce4KDisplacementLimit(node, cg, preced, false, forceDueToAlignmentBump);
return preced;
return enforceSSFormatLimits(node, cg, preced);
}

TR::Instruction *
Expand All @@ -2076,9 +2077,7 @@ OMR::Z::MemoryReference::enforce4KDisplacementLimit(TR::Node * node, TR::CodeGen
if (self()->ignoreNegativeOffset())
return preced;

TR_ASSERT(!forcePLXFixup, "This logic is only used for markAndAdjustForLongDisplacementIfNeeded. You probably want the other flag.");

if ((_offset < 0 || _offset >= MAXDISP || forcePLXFixup || forceFixup) &&
if ((_offset < 0 || _offset >= MAXDISP || forceFixup) &&
!self()->isAdjustedForLongDisplacement())
{
TR::Register * tempTargetRegister = NULL;
Expand All @@ -2087,11 +2086,6 @@ OMR::Z::MemoryReference::enforce4KDisplacementLimit(TR::Node * node, TR::CodeGen
else
tempTargetRegister = cg->allocateRegister();
TR::MemoryReference * interimMemoryReference = generateS390MemoryReference(cg);
if (forcePLXFixup) // avoid double counting of the TotalUseCount on the VRs
{
if (_baseRegister) _baseRegister->decTotalUseCount();
if (_indexRegister) _indexRegister->decTotalUseCount();
}
interimMemoryReference->setBaseRegister(_baseRegister, cg);
interimMemoryReference->setIndexRegister(_indexRegister);
TR::SymbolReference * interinSymRef = interimMemoryReference->getSymbolReference();
Expand All @@ -2106,14 +2100,6 @@ OMR::Z::MemoryReference::enforce4KDisplacementLimit(TR::Node * node, TR::CodeGen
self()->setConstantDataSnippet(NULL);
}
preced = self()->handleLargeOffset(node, interimMemoryReference, tempTargetRegister, cg, preced);

if (forcePLXFixup)
{
tempTargetRegister->incTotalUseCount();
if (self()->getStorageReference() && interimMemoryReference->getSymbolReference()->isTempVariableSizeSymRef())
{
}
}
cg->stopUsingRegister(tempTargetRegister);

_baseNode = NULL;
Expand Down Expand Up @@ -3032,7 +3018,18 @@ OMR::Z::MemoryReference::generateBinaryEncoding(uint8_t * cursor, TR::CodeGenera
instructionFormat == TR::Instruction::IsSS1 ||
instructionFormat == TR::Instruction::IsSS2 ||
instructionFormat == TR::Instruction::IsSS4 ||
instructionFormat == TR::Instruction::IsS)
instructionFormat == TR::Instruction::IsS ||
instructionFormat == TR::Instruction::IsRIS ||
instructionFormat == TR::Instruction::IsRRS ||
instructionFormat == TR::Instruction::IsSSE ||
instructionFormat == TR::Instruction::IsSSF ||
// Vector instruction formats that have 12-bit displacements and no index registers
instructionFormat == TR::Instruction::IsVSI ||
instructionFormat == TR::Instruction::IsVRV ||
instructionFormat == TR::Instruction::IsVRSa||
instructionFormat == TR::Instruction::IsVRSb||
instructionFormat == TR::Instruction::IsVRSc||
instructionFormat == TR::Instruction::IsVRSd)
{
TR_ASSERT_FATAL(base != NULL, "Expected non-NULL base register for long displacement on %s [%p] instruction", instr->getOpCode().getMnemonicName(), instr);

Expand Down
58 changes: 43 additions & 15 deletions compiler/z/codegen/S390GenerateInstructions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2051,24 +2051,38 @@ TR::Instruction * generateVRRiInstruction(
/* Note subtle differences between register types and optionality of masks between these 3*/
TR::Instruction *
generateVRSaInstruction(TR::CodeGenerator * cg, TR::InstOpCode::Mnemonic op, TR::Node * n, TR::Register * targetReg, TR::Register * sourceReg, TR::MemoryReference * mr,
uint8_t mask4 /* 4 bits */)
uint8_t mask4 /* 4 bits */, TR::Instruction* preced)
{
return new (INSN_HEAP) TR::S390VRSaInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

if (preced)
return new (INSN_HEAP) TR::S390VRSaInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced);
else
return new (INSN_HEAP) TR::S390VRSaInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
}

TR::Instruction *
generateVRSbInstruction(TR::CodeGenerator * cg, TR::InstOpCode::Mnemonic op, TR::Node * n, TR::Register * targetReg, TR::Register * sourceReg, TR::MemoryReference * mr,
uint8_t mask4 /* 4 bits */)
uint8_t mask4 /* 4 bits */, TR::Instruction* preced)
{
mr->separateIndexRegister(n, cg, true, NULL);
return new (INSN_HEAP) TR::S390VRSbInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

if (preced)
return new (INSN_HEAP) TR::S390VRSbInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced);
else
return new (INSN_HEAP) TR::S390VRSbInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
}

TR::Instruction *
generateVRScInstruction(TR::CodeGenerator * cg, TR::InstOpCode::Mnemonic op, TR::Node * n, TR::Register * targetReg, TR::Register * sourceReg, TR::MemoryReference * mr,
uint8_t mask4 /* 4 bits */)
uint8_t mask4 /* 4 bits */, TR::Instruction* preced)
{
return new (INSN_HEAP) TR::S390VRScInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

if (preced)
return new (INSN_HEAP) TR::S390VRScInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced);
else
return new (INSN_HEAP) TR::S390VRScInstruction(cg, op, n, targetReg, sourceReg, mr, mask4);
}

TR::Instruction *
Expand All @@ -2078,17 +2092,28 @@ generateVRSdInstruction(
TR::Node * n ,
TR::Register * targetReg , /* VRF */
TR::Register * sourceReg3 , /* GPR R3 */
TR::MemoryReference * mr )
TR::MemoryReference * mr ,
TR::Instruction * preced)
{
return new (INSN_HEAP) TR::S390VRSdInstruction(cg, op, n, targetReg, sourceReg3, mr);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

if (preced)
return new (INSN_HEAP) TR::S390VRSdInstruction(cg, op, n, targetReg, sourceReg3, mr, preced);
else
return new (INSN_HEAP) TR::S390VRSdInstruction(cg, op, n, targetReg, sourceReg3, mr);
}

/****** VRV ******/
TR::Instruction *
generateVRVInstruction(TR::CodeGenerator * cg, TR::InstOpCode::Mnemonic op, TR::Node * n, TR::Register *sourceReg, TR::MemoryReference * mr,
uint8_t mask3 /* 4 bits */)
uint8_t mask3 /* 4 bits */, TR::Instruction* preced)
{
return new (INSN_HEAP) TR::S390VRVInstruction(cg, op, n, sourceReg, mr, mask3);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

if (preced != NULL)
return new (INSN_HEAP) TR::S390VRVInstruction(cg, op, n, sourceReg, mr, mask3, preced);
else
return new (INSN_HEAP) TR::S390VRVInstruction(cg, op, n, sourceReg, mr, mask3);
}

/****** VRX ******/
Expand All @@ -2111,12 +2136,15 @@ TR::Instruction * generateVSIInstruction(
TR::Node * n ,
TR::Register * reg, /* VRF */
TR::MemoryReference * mr ,
uint8_t imm3) /* 8 bits */
uint8_t imm3, /* 8 bits */
TR::Instruction * preced)
{
TR_ASSERT_FATAL(mr != NULL, "NULL memory reference for VSI instruction\n");
mr->separateIndexRegister(n, cg, true, NULL);
if (mr != NULL) preced = mr->enforceSSFormatLimits(n, cg, preced);

return new (INSN_HEAP) TR::S390VSIInstruction(cg, op, n, reg, mr, imm3);
if (preced != NULL)
return new (INSN_HEAP) TR::S390VSIInstruction(cg, op, n, reg, mr, imm3, preced);
else
return new (INSN_HEAP) TR::S390VSIInstruction(cg, op, n, reg, mr, imm3);
}

/************************************************************ Misc Instructions ************************************************************/
Expand Down
18 changes: 12 additions & 6 deletions compiler/z/codegen/S390GenerateInstructions.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,8 @@ TR::Instruction * generateVRSaInstruction(
TR::Register * targetReg ,
TR::Register * sourceReg ,
TR::MemoryReference * mr ,
uint8_t mask4 ); /* 4 bits */
uint8_t mask4 , /* 4 bits */
TR::Instruction * preced = NULL);

TR::Instruction * generateVRSbInstruction(
TR::CodeGenerator * cg ,
Expand All @@ -1179,7 +1180,8 @@ TR::Instruction * generateVRSbInstruction(
TR::Register * targetReg , /* VRF */
TR::Register * sourceReg ,
TR::MemoryReference * mr ,
uint8_t mask4 = 0 ); /* 4 bits */
uint8_t mask4 = 0 , /* 4 bits */
TR::Instruction * preced = NULL);

TR::Instruction * generateVRScInstruction(
TR::CodeGenerator * cg ,
Expand All @@ -1188,15 +1190,17 @@ TR::Instruction * generateVRScInstruction(
TR::Register * targetReg ,
TR::Register * sourceReg ,
TR::MemoryReference * mr ,
uint8_t mask4 ); /* 4 bits */
uint8_t mask4 , /* 4 bits */
TR::Instruction * preced = NULL);

TR::Instruction * generateVRSdInstruction(
TR::CodeGenerator * cg ,
TR::InstOpCode::Mnemonic op ,
TR::Node * n ,
TR::Register * targetReg , /* VRF */
TR::Register * sourceReg3 , /* GPR R3 */
TR::MemoryReference * mr );
TR::MemoryReference * mr ,
TR::Instruction * preced = NULL);

/****** VRV ******/
TR::Instruction * generateVRVInstruction(
Expand All @@ -1205,7 +1209,8 @@ TR::Instruction * generateVRVInstruction(
TR::Node * n ,
TR::Register * sourceReg ,
TR::MemoryReference * mr ,
uint8_t mask3 ); /* 4 bits */
uint8_t mask3 , /* 4 bits */
TR::Instruction* preced = NULL);

/****** VRX ******/
TR::Instruction * generateVRXInstruction(
Expand All @@ -1225,7 +1230,8 @@ TR::Instruction * generateVSIInstruction(
TR::Node * n ,
TR::Register * reg , /* VRF */
TR::MemoryReference * mr ,
uint8_t imm3 = 0); /* 8 bits */
uint8_t imm3 = 0, /* 8 bits */
TR::Instruction * preced = NULL);

/************************************************************ Misc Instructions ************************************************************/
TR::Instruction *generateS390ImmSymInstruction(
Expand Down
Loading

0 comments on commit f8042f4

Please sign in to comment.