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 committed Feb 6, 2019
1 parent d68d38c commit ff73b67
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 63 deletions.
24 changes: 18 additions & 6 deletions compiler/z/codegen/OMRMemoryReference.cpp
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 self()->enforceSSFormatLimits(node, cg, preced);
}

TR::Instruction *
Expand Down Expand Up @@ -3032,7 +3033,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
60 changes: 44 additions & 16 deletions compiler/z/codegen/S390GenerateInstructions.cpp
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);
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);
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);
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,25 +2092,36 @@ 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);
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);
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 ******/
TR::Instruction *
generateVRXInstruction(TR::CodeGenerator * cg, TR::InstOpCode::Mnemonic op, TR::Node * n, TR::Register * reg,
TR::MemoryReference * memRef, uint8_t mask3, TR::Instruction * preced)
{
if (memRef) preced = memRef->enforceVRXFormatLimits(n, cg, preced);
preced = memRef->enforceVRXFormatLimits(n, cg, preced);

if (preced)
return new (INSN_HEAP) TR::S390VRXInstruction(cg, op, n, reg, memRef, mask3, preced);
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);
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
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 ff73b67

Please sign in to comment.