Merge pull request #4804 from aviansie-ben/power-issue-4765

Fix issues with the Power system linkage

compiler/p/codegen/OMRLinkage.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -166,7 +166,6 @@ TR::Instruction *OMR::Power::Linkage::saveArguments(TR::Instruction *cursor, boo
       TR::DataType type = paramCursor->getType();
       int32_t dtype = type.getDataType();
 
-
       // TODO: Is there an accurate assume to insert here ?
       if (lri >= 0)
          {
@@ -191,6 +190,9 @@ TR::Instruction *OMR::Power::Linkage::saveArguments(TR::Instruction *cursor, boo
                {
                case TR::Int8:
                case TR::Int16:
+                  if (properties.getSmallIntParmsAlignedRight())
+                     offset &= ~3;
+
                case TR::Int32:
                   op = TR::InstOpCode::stw;
                   length = 4;

compiler/p/codegen/OMRLinkage.hpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -68,11 +68,12 @@ class PPCMemoryArgument
 
 
 // linkage properties
-#define CallerCleanup       0x01
-#define RightToLeft         0x02
-#define IntegersInRegisters 0x04
-#define LongsInRegisters    0x08
-#define FloatsInRegisters   0x10
+#define CallerCleanup             0x01
+#define RightToLeft               0x02
+#define IntegersInRegisters       0x04
+#define LongsInRegisters          0x08
+#define FloatsInRegisters         0x10
+#define SmallIntParmsAlignedRight 0x20
 
 // register flags
 #define Preserved                   0x01
@@ -139,6 +140,8 @@ struct PPCLinkageProperties
 
    uint32_t getFloatsInRegisters() const {return (_properties & FloatsInRegisters);}
 
+   uint32_t getSmallIntParmsAlignedRight() const { return (_properties & SmallIntParmsAlignedRight); }
+
    uint32_t getRegisterFlags(TR::RealRegister::RegNum regNum) const
       {
       return _registerFlags[regNum];

compiler/p/codegen/PPCSystemLinkage.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -90,6 +90,9 @@ TR::PPCSystemLinkage::PPCSystemLinkage(TR::CodeGenerator *cg)
    {
    int i = 0;
    _properties._properties = IntegersInRegisters|FloatsInRegisters|RightToLeft;
+   if (comp()->target().cpu.isBigEndian())
+      _properties._properties |= SmallIntParmsAlignedRight;
+
    _properties._registerFlags[TR::RealRegister::NoReg] = 0;
    _properties._registerFlags[TR::RealRegister::gr0]   = 0;
    _properties._registerFlags[TR::RealRegister::gr1]   = Preserved|PPC_Reserved; // system sp
@@ -436,6 +439,23 @@ TR::PPCSystemLinkage::hasToBeOnStack(TR::ParameterSymbol *parm)
    }
 
 
+static bool
+hasParmsPassedOnStack(List<TR::ParameterSymbol> &parmList)
+   {
+   ListIterator<TR::ParameterSymbol> parmIt(&parmList);
+   TR::ParameterSymbol *parmCursor = parmIt.getFirst();
+
+   while (parmCursor)
+      {
+      if (parmCursor->getLinkageRegisterIndex() < 0)
+         return true;
+
+      parmCursor = parmIt.getNext();
+      }
+
+   return false;
+   }
+
 void
 TR::PPCSystemLinkage::mapParameters(
       TR::ResolvedMethodSymbol *method,
@@ -449,14 +469,12 @@ TR::PPCSystemLinkage::mapParameters(
    int32_t                          offsetToFirstParm = linkage.getOffsetToFirstParm();
    int32_t offset_from_top = 0;
    int32_t slot_size = sizeof(uintptrj_t);
-#ifdef __LITTLE_ENDIAN__
-   // XXX: This hack fixes ppc64le by saving params in the current stack frame, rather than the caller's parameter save area, which may not exist
-   //      This code needs to be refactored to accomodate ABIs that don't guarantee a parameter save area
-   // XXX: Also, for some reason all these system linkages are marked passing parms right-to-left when they should be left-to-right
-   const bool saveParmsInLocalArea = true;
-#else
-   const bool saveParmsInLocalArea = false;
-#endif
+
+   // The 64-bit ELF V2 ABI Specification makes having a parameter save area optional if all parameters can be passed in
+   // registers. As a result, we cannot use the caller's parameter save area on 64-bit Linux if all parameters were
+   // passed in registers. However, we *must* use the caller's parameter save area if one or more parameters were passed
+   // on the stack to load those values correctly.
+   bool saveParmsInLocalArea = comp()->target().isLinux() && comp()->target().is64Bit() && !hasParmsPassedOnStack(parmList);
 
    if (linkage.getRightToLeft())
       {
@@ -466,6 +484,10 @@ TR::PPCSystemLinkage::mapParameters(
             parmCursor->setParameterOffset(offset_from_top + stackIndex);
          else
             parmCursor->setParameterOffset(offset_from_top + offsetToFirstParm + stackIndex);
+
+         if (linkage.getSmallIntParmsAlignedRight() && parmCursor->getType().isIntegral() && parmCursor->getSize() < slot_size)
+            parmCursor->setParameterOffset(parmCursor->getParameterOffset() + slot_size - parmCursor->getSize());
+
          offset_from_top += (parmCursor->getSize() + slot_size - 1) & (~(slot_size - 1));
          parmCursor = parameterIterator.getNext();
          }

fvtest/compilertriltest/ArithmeticTest.cpp

@@ -320,8 +320,6 @@ TEST_P(Int16Arithmetic, UsingConst) {
     std::string arch = omrsysinfo_get_CPU_architecture();
     SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
         << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-    SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-        << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
 
     auto param = TRTest::to_struct(GetParam());
 
@@ -355,8 +353,6 @@ TEST_P(Int16Arithmetic, UsingLoadParam) {
     std::string arch = omrsysinfo_get_CPU_architecture();
     SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
         << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-    SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-        << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
 
     auto param = TRTest::to_struct(GetParam());
 
@@ -386,8 +382,6 @@ TEST_P(Int8Arithmetic, UsingConst) {
     std::string arch = omrsysinfo_get_CPU_architecture();
     SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
         << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-    SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-        << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
 
     auto param = TRTest::to_struct(GetParam());
 
@@ -421,8 +415,6 @@ TEST_P(Int8Arithmetic, UsingLoadParam) {
     std::string arch = omrsysinfo_get_CPU_architecture();
     SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
         << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-    SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-        << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
     auto param = TRTest::to_struct(GetParam());
 
     char inputTrees[1024] = {0};

fvtest/compilertriltest/BitPermuteTest.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2017, 2019 IBM Corp. and others
+ * Copyright (c) 2017, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -343,8 +343,6 @@ TEST_P(sBitPermuteTest, ConstAddressLengthTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[16];
@@ -385,8 +383,6 @@ TEST_P(sBitPermuteTest, ConstAddressTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[16];
@@ -426,8 +422,6 @@ TEST_P(sBitPermuteTest, NoConstTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[16];
@@ -468,8 +462,6 @@ TEST_P(bBitPermuteTest, ConstAddressLengthTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[8];
@@ -510,8 +502,6 @@ TEST_P(bBitPermuteTest, ConstAddressTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[8];
@@ -551,8 +541,6 @@ TEST_P(bBitPermuteTest, NoConstTest)
    std::string arch = omrsysinfo_get_CPU_architecture();
    SKIP_IF(OMRPORT_ARCH_S390 == arch || OMRPORT_ARCH_S390X == arch, KnownBug)
       << "The Z code generator incorrectly spills sub-integer types arguments (see issue #3525)";
-   SKIP_IF(OMRPORT_ARCH_PPC64 == arch || OMRPORT_ARCH_PPC == arch, KnownBug)
-      << "The Power code generator incorrectly spills sub-integer type arguments on big-endian machines (see issue #3525)";
    auto param = to_struct(GetParam());
 
    uint8_t maskedIndices[8];