Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: Implement aladdEvaluator() #4118

Merged
merged 1 commit into from
Jul 22, 2019
Merged

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Jul 9, 2019

This commit implements aladdEvaluator() for AArch64.

Signed-off-by: KONNO Kazuhiro konno@jp.ibm.com

@knn-k knn-k requested a review from 0xdaryl as a code owner July 9, 2019 09:52
@knn-k knn-k force-pushed the aarch64eval44 branch 2 times, most recently from 1cb7de3 to fad8eca Compare July 18, 2019 07:15
@knn-k knn-k changed the title WIP: AArch64: Implement aladdEvaluator() AArch64: Implement aladdEvaluator() Jul 18, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Jul 18, 2019

Ready for review.

trgReg->setPinningArrayPointer(firstChild->getRegister()->getPinningArrayPointer());
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will accomplish the same thing for the code under the isInternalPointer() guard but with fewer calls and indirections. Correct me if I'm wrong though. This wouldn't work, for example, if the pinning array pointer symbol could ever be NULL, but I don't think that's possible.

   if (node->isInternalPointer())
      {
      TR::AutomaticSymbol *pinningArrayPointerSymbol = NULL;

      if (node->getPinningArrayPointer())
         {
         pinningArrayPointerSymbol = node->getPinningArrayPointer();
         }
      else
         {
         TR::Node *firstChild = node->getFirstChild();
   
         if ((firstChild->getOpCodeValue() == TR::aload) &&
             firstChild->getSymbolReference()->getSymbol()->isAuto() &&
             firstChild->getSymbolReference()->getSymbol()->isPinningArrayPointer())
            {
            TR::Symbol *firstChildSymbol = firstChild->getSymbolReference()->getSymbol();
            
            pinningArrayPointerSymbol = firstChildSymbol->isInternalPointer() ?
               firstChildSymbol->castToInternalPointerAutoSymbol()->getPinningArrayPointer() :
               firstChildSymbol->castToAutoSymbol();
            }
         else if (firstChild->getRegister() &&
                  firstChild->getRegister()->containsInternalPointer())
            {
            pinningArrayPointerSymbol = firstChild->getRegister()->getPinningArrayPointer();
            }
         }

      if (pinningArrayPointerSymbol)
         {
         trgReg->setContainsInternalPointer();
         trgReg->setPinningArrayPointer(pinningArrayPointerSymbol);
         }
      }

@@ -612,3 +613,46 @@ OMR::ARM64::TreeEvaluator::lxorEvaluator(TR::Node *node, TR::CodeGenerator *cg)
// boolean xor of 2 integers
return genericBinaryEvaluator(node, TR::InstOpCode::eorx, TR::InstOpCode::eorimmx, true, cg);
}

// Also handles TR::aluadd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove comment. TR::aluadd has been deprecated.

@0xdaryl 0xdaryl self-assigned this Jul 20, 2019
This commit implements aladdEvaluator() for AArch64.

Signed-off-by: KONNO Kazuhiro <konno@jp.ibm.com>
@knn-k
Copy link
Contributor Author

knn-k commented Jul 22, 2019

Updated BinaryEvaluator.cpp reflecting the review comments.

@0xdaryl
Copy link
Contributor

0xdaryl commented Jul 22, 2019

@genie-omr build aarch64

@0xdaryl
Copy link
Contributor

0xdaryl commented Jul 22, 2019

Code changes seem fine. I don't fully understand all the implications of compressed references on this evaluator (and if any support is missing) but I think what you have is a good start.

@0xdaryl 0xdaryl merged commit f90c0be into eclipse-omr:master Jul 22, 2019
@knn-k knn-k deleted the aarch64eval44 branch August 19, 2019 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants