From ff73b678ee69670eaf3b77a8ffe483efb8aeffa4 Mon Sep 17 00:00:00 2001 From: Nigel Yu Date: Tue, 29 Jan 2019 16:36:35 -0500 Subject: [PATCH] Fix vector load/store instruction large displacement bug 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 --- compiler/z/codegen/OMRMemoryReference.cpp | 24 ++- .../z/codegen/S390GenerateInstructions.cpp | 60 +++++-- .../z/codegen/S390GenerateInstructions.hpp | 18 +- compiler/z/codegen/S390Instruction.hpp | 161 ++++++++++++++---- 4 files changed, 200 insertions(+), 63 deletions(-) diff --git a/compiler/z/codegen/OMRMemoryReference.cpp b/compiler/z/codegen/OMRMemoryReference.cpp index 55020e85763..3dbd6f99602 100644 --- a/compiler/z/codegen/OMRMemoryReference.cpp +++ b/compiler/z/codegen/OMRMemoryReference.cpp @@ -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) { @@ -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 * @@ -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); diff --git a/compiler/z/codegen/S390GenerateInstructions.cpp b/compiler/z/codegen/S390GenerateInstructions.cpp index d1bd2918c7e..ad7e49ff71a 100644 --- a/compiler/z/codegen/S390GenerateInstructions.cpp +++ b/compiler/z/codegen/S390GenerateInstructions.cpp @@ -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 * @@ -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); + 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 ******/ @@ -2096,7 +2121,7 @@ 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); @@ -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 ************************************************************/ diff --git a/compiler/z/codegen/S390GenerateInstructions.hpp b/compiler/z/codegen/S390GenerateInstructions.hpp index 905f73802aa..44c75fe7362 100644 --- a/compiler/z/codegen/S390GenerateInstructions.hpp +++ b/compiler/z/codegen/S390GenerateInstructions.hpp @@ -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 , @@ -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 , @@ -1188,7 +1190,8 @@ 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 , @@ -1196,7 +1199,8 @@ TR::Instruction * generateVRSdInstruction( 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( @@ -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( @@ -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( diff --git a/compiler/z/codegen/S390Instruction.hpp b/compiler/z/codegen/S390Instruction.hpp index b0e91e7d934..8a21ed6e0fb 100644 --- a/compiler/z/codegen/S390Instruction.hpp +++ b/compiler/z/codegen/S390Instruction.hpp @@ -5768,6 +5768,21 @@ class S390VRSaInstruction : public S390VStorageInstruction { setPrintMaskField(getOpCode().usesM4()); } + + S390VRSaInstruction( + TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op , + TR::Node * n , + TR::Register * targetReg, /* VRF */ + TR::Register * sourceReg, /* VRF */ + TR::MemoryReference * mr , + uint8_t mask4 , /* 4 bits */ + TR::Instruction * preced) + : S390VStorageInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced) + { + setPrintMaskField(getOpCode().usesM4()); + } + Kind getKind() { return IsVRSa; } virtual char *description() { return "S390VRSaInstruction"; } }; @@ -5795,6 +5810,20 @@ class S390VRSbInstruction : public S390VStorageInstruction setPrintMaskField(getOpCode().usesM4()); } + S390VRSbInstruction( + TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op , + TR::Node * n , + TR::Register * targetReg, /* VRF */ + TR::Register * sourceReg, /* GPR */ + TR::MemoryReference * mr , + uint8_t mask4 , /* 4 bits */ + TR::Instruction * preced) + : S390VStorageInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced) + { + setPrintMaskField(getOpCode().usesM4()); + } + Kind getKind() { return IsVRSb; } virtual char *description() { return "S390VRSbInstruction"; } }; @@ -5821,12 +5850,25 @@ class S390VRScInstruction : public S390VStorageInstruction { setPrintMaskField(getOpCode().usesM4()); } + + S390VRScInstruction( + TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op , + TR::Node * n , + TR::Register * targetReg, /* GPR */ + TR::Register * sourceReg, /* VRF */ + TR::MemoryReference * mr , + uint8_t mask4 , /* 4 bits */ + TR::Instruction * preced) + : S390VStorageInstruction(cg, op, n, targetReg, sourceReg, mr, mask4, preced) + { + setPrintMaskField(getOpCode().usesM4()); + } + Kind getKind() { return IsVRSc; } virtual char *description() { return "S390VRScInstruction"; } }; - - /** * VRS-d * _________________________________________________________ @@ -5852,6 +5894,29 @@ class S390VRSdInstruction : public S390VStorageInstruction TR::Register * v1Reg = NULL, /* VRF */ TR::MemoryReference * mr = NULL) : S390VStorageInstruction(cg, op, n) + { + initVRSd(r3Reg, v1Reg, mr); + } + + S390VRSdInstruction( + TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op, + TR::Node * n , + TR::Register * r3Reg, /* GPR */ + TR::Register * v1Reg, /* VRF */ + TR::MemoryReference * mr , + TR::Instruction* preced ) + : S390VStorageInstruction(cg, op, n, r3Reg, NULL, mr, 0, preced) + { + initVRSd(r3Reg, v1Reg, mr); + } + + Kind getKind() { return IsVRSd; } + uint8_t * generateBinaryEncoding(); + char *description() { return "S390VRSdInstruction"; } + +private: + void initVRSd(TR::Register* r3Reg, TR::Register* v1Reg, TR::MemoryReference* mr) { if(getOpCode().isStore()) { @@ -5868,10 +5933,6 @@ class S390VRSdInstruction : public S390VStorageInstruction } setPrintMaskField(false); } - - Kind getKind() { return IsVRSd; } - uint8_t * generateBinaryEncoding(); - char *description() { return "S390VRSdInstruction"; } }; @@ -5898,6 +5959,20 @@ class S390VRVInstruction : public S390VStorageInstruction { setPrintMaskField(getOpCode().usesM3()); } + + S390VRVInstruction( + TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op , + TR::Node * n , + TR::Register * sourceReg, /* VRF */ + TR::MemoryReference * mr , + uint8_t mask3 , /* 4 bits */ + TR::Instruction * preced) + : S390VStorageInstruction(cg, op, n, sourceReg, sourceReg, mr, mask3, preced) + { + setPrintMaskField(getOpCode().usesM3()); + } + Kind getKind() { return IsVRV; } uint8_t * generateBinaryEncoding(); }; @@ -5955,44 +6030,60 @@ class S390VRXInstruction : public S390VStorageInstruction */ class S390VSIInstruction : public S390VStorageInstruction { - uint8_t _constantImm3; - public: - S390VSIInstruction( - TR::CodeGenerator * cg = NULL, - TR::InstOpCode::Mnemonic op = TR::InstOpCode::BAD, - TR::Node * n = NULL, - TR::Register * v1Reg = NULL, - TR::MemoryReference * memRef = NULL, - uint8_t constantImm3 = 0) - : S390VStorageInstruction(cg, op, n), - _constantImm3(constantImm3) + S390VSIInstruction(TR::CodeGenerator * cg = NULL, + TR::InstOpCode::Mnemonic op = TR::InstOpCode::BAD, + TR::Node * n = NULL, + TR::Register * v1Reg = NULL, + TR::MemoryReference * memRef = NULL, + uint8_t constantImm3 = 0) + : S390VStorageInstruction(cg, op, n), + _constantImm3(constantImm3) { - if(getOpCode().setsOperand1()) - { - useTargetRegister(v1Reg); - } - else - { - useSourceRegister(v1Reg); - } - - // memrefs are always named source memory reference regardless of what they actually are. - useSourceMemoryReference(memRef); - setupThrowsImplicitNullPointerException(n, memRef); - - if (memRef->getUnresolvedSnippet() != NULL) - { - (memRef->getUnresolvedSnippet())->setDataReferenceInstruction(this); - } + initVSI(n, v1Reg, memRef); + } - setPrintMaskField(false); + S390VSIInstruction(TR::CodeGenerator * cg , + TR::InstOpCode::Mnemonic op , + TR::Node * n , + TR::Register * v1Reg , + TR::MemoryReference * memRef, + uint8_t constantImm3, + TR::Instruction * preced) + : S390VStorageInstruction(cg, op, n, v1Reg, NULL, memRef, 0, preced), + _constantImm3(constantImm3) + { } Kind getKind() { return IsVSI; } uint8_t getImmediateField3() { return _constantImm3; } char *description() { return "S390VSIInstruction"; } uint8_t * generateBinaryEncoding(); +private: + uint8_t _constantImm3; + + void initVSI(TR::Node* n, TR::Register* v1Reg, TR::MemoryReference* memRef) + { + if(getOpCode().setsOperand1()) + { + useTargetRegister(v1Reg); + } + else + { + useSourceRegister(v1Reg); + } + + // memrefs are always named source memory reference regardless of what they actually are. + useSourceMemoryReference(memRef); + setupThrowsImplicitNullPointerException(n, memRef); + + if (memRef->getUnresolvedSnippet() != NULL) + { + (memRef->getUnresolvedSnippet())->setDataReferenceInstruction(this); + } + + setPrintMaskField(false); + } }; ////////////////////////////////////////////////////////////////////////////////