Add the macros with the hardcoded prefixes #6945

singh264 · 2023-04-10T15:49:40Z

Add the macros with the hardcoded prefixes
of format O^O [a-z A-Z]+(:)?.

Closes: #4550.
Author: Amarpreet Singh amarpreet1997@gmail.com.

singh264 · 2023-04-10T15:55:27Z

The local test results of the code changes in the GitHub pull request is located at the following url: https://drive.google.com/file/d/1OuVzManwMle7SJqRELQP315tafN6YX3U/view?usp=sharing.

The local test results of the code changes in the GitHub pull request could be displayed with the file that is located at the following url: https://github.com/singh264/scripts/blob/main/display_test_openj9-openjdk-jdk8_results.sh.

singh264 · 2023-04-10T15:59:49Z

@VermaSh

VermaSh · 2023-05-01T16:07:16Z

Sorry just saw this PR, will review the changes later this week

VermaSh

Some minor suggestions. I still have to go through 14 files, will post another comment for those soon.

VermaSh · 2023-05-10T20:30:26Z

compiler/il/OMRNode.cpp

@@ -6472,7 +6476,7 @@ OMR::Node::setArrayChkPrimitiveArray2(bool v)
   {
   TR::Compilation * c = TR::comp();
   TR_ASSERT(self()->getOpCodeValue() == TR::ArrayCHK, "Opcode must be ArrayCHK");
-   if (performNodeTransformation2(c, "O^O NODE FLAGS: Setting arrayChkPrimitiveArray2 flag on node %p to %d\n", self(), v))
+   if (performNodeTransformation3(c, "%s Setting arrayChkPrimitiveArray2 flag on node %p to %d\n", OPT_DETAILS, self(), v))


Extra space here before Setting

VermaSh · 2023-05-10T22:47:13Z

compiler/z/codegen/OMRPeephole.cpp

@@ -1578,7 +1580,7 @@ OMR::Z::Peephole::tryToRemoveDuplicateNILF()
            {
            if (currInst->getSourceImmediate() == nextInst->getSourceImmediate())
               {
-               if (performTransformation(self()->comp(), "O^O S390 PEEPHOLE: deleting duplicate NILF from pair %p %p*\n", currInst, nextInst))
+               if (performTransformation(self()->comp(), "%sdeleting duplicate NILF from pair %p %p*\n", OPT_DETAILS_S390_PEEPHOLE, currInst, nextInst))


To be consistent, can you please capitalize D

VermaSh · 2023-05-10T22:47:29Z

compiler/z/codegen/OMRPeephole.cpp

@@ -1602,7 +1604,7 @@ OMR::Z::Peephole::tryToRemoveDuplicateNILF()
            else if (((currInst->getSourceImmediate() & nextInst->getSourceImmediate()) == currInst->getSourceImmediate()) &&
               ((nextInst->getSourceImmediate() & currInst->getSourceImmediate()) != nextInst->getSourceImmediate()))
               {
-               if (performTransformation(self()->comp(), "O^O S390 PEEPHOLE: deleting unnecessary NILF from pair %p %p*\n", currInst, nextInst))
+               if (performTransformation(self()->comp(), "%sdeleting unnecessary NILF from pair %p %p*\n", OPT_DETAILS_S390_PEEPHOLE, currInst, nextInst))


VermaSh · 2023-05-10T22:47:43Z

compiler/z/codegen/OMRPeephole.cpp

@@ -1633,7 +1635,7 @@ OMR::Z::Peephole::tryToRemoveDuplicateNILH()
               if (currInst->matchesTargetRegister(nextInst->getRegisterOperand(1)) &&
                   nextInst->matchesTargetRegister(currInst->getRegisterOperand(1)))
                  {
-                  if (performTransformation(self()->comp(), "O^O S390 PEEPHOLE: deleting duplicate NILH from pair %p %p*\n", currInst, nextInst))
+                  if (performTransformation(self()->comp(), "%sdeleting duplicate NILH from pair %p %p*\n", OPT_DETAILS_S390_PEEPHOLE, currInst, nextInst))


VermaSh · 2023-05-10T22:47:58Z

compiler/z/codegen/OMRPeephole.cpp

@@ -1771,7 +1773,7 @@ OMR::Z::Peephole::tryToRemoveRedundantShift()
            uint32_t newShift = currRSInst->getSourceImmediate() + nextRSInst->getSourceImmediate();
            if (newShift < 64)
               {
-               if (performTransformation(self()->comp(), "O^O S390 PEEPHOLE: merging SRL/SLL pair [%p] [%p]\n", cursor, cursor->getNext()))
+               if (performTransformation(self()->comp(), "%smerging SRL/SLL pair [%p] [%p]\n", OPT_DETAILS_S390_PEEPHOLE, cursor, cursor->getNext()))


To be consistent, can you please capitalize M.

VermaSh · 2023-05-11T00:07:34Z

compiler/il/Aliases.cpp

@@ -65,6 +65,8 @@
 #include "runtime/RuntimeAssumptions.hpp"
 #endif

+#define OPT_DETAILS "O^O REFINING ALIASES: "


Don't think we need to be specific here. IL ALIASES: should suffice.

VermaSh · 2023-05-11T00:11:19Z

compiler/il/OMRResolvedMethodSymbol.cpp

@@ -70,6 +70,7 @@
 #include "runtime/Runtime.hpp"
 #include "infra/SimpleRegex.hpp"

+#define OPT_DETAILS "O^O OSR: "


We can reword this to be more generic since the message has enough detail to indicate the transformation is related to OSR call. So maybe reword to IL Resolved Method Symbol.

VermaSh · 2023-05-11T00:12:23Z

compiler/optimizer/Inliner.cpp

@@ -1683,7 +1684,7 @@ void TR_InlinerBase::rematerializeCallArguments(TR_TransformInlinedFunction & ti
            if (rematTree == argStoreTree)
               {
               TR::Node *duplicateStore = argStore->duplicateTree();
-               if (performTransformation(comp(), "O^O GUARDED CALL REMAT: Rematerialize [%p] as [%p]\n", argStore, duplicateStore))
+               if (performTransformation(comp(), "%sRematerialize [%p] as [%p]\n", OPT_DETAILS_GUARDED_CALL_REMAT, argStore, duplicateStore))


We can reword the message to indicate that a guarded call is being rematerialized.

VermaSh · 2023-05-11T00:12:43Z

compiler/optimizer/Inliner.cpp

@@ -1693,7 +1694,7 @@ void TR_InlinerBase::rematerializeCallArguments(TR_TransformInlinedFunction & ti
               {
               TR::Node *duplicateStore = TR::Node::createStore(argStore->getSymbolReference(), TR::Node::createLoad(argStore, rematTree->getNode()->getSymbolReference()));
               duplicateStore->setByteCodeInfo(argStore->getByteCodeInfo());
-               if (performTransformation(comp(), "O^O GUARDED CALL REMAT: Partial rematerialize of [%p] as [%p] - load of [%d]\n", argStore, duplicateStore, rematTree->getNode()->getSymbolReference()->getReferenceNumber()))
+               if (performTransformation(comp(), "%sPartial rematerialize of [%p] as [%p] - load of [%d]\n", OPT_DETAILS_GUARDED_CALL_REMAT, argStore, duplicateStore, rematTree->getNode()->getSymbolReference()->getReferenceNumber()))


We can reword the message to indicate that a guarded call is being partially rematerialized.

VermaSh · 2023-05-11T00:13:05Z

compiler/optimizer/Inliner.cpp

@@ -119,6 +119,7 @@ int32_t  *NumInlinedMethods   = NULL;
 int32_t  *InlinedSizes        = NULL;

 #define OPT_DETAILS "O^O INLINER: "
+#define OPT_DETAILS_GUARDED_CALL_REMAT "O^O GUARDED CALL REMAT: "


Don't think we need a specific macro for guarded calls

Add the macros with the hardcoded prefixes of format O\^O [a-z A-Z]+(:)?. Closes: eclipse-omr#4550. Author: Amarpreet Singh amarpreet1997@gmail.com.

The code is refactored based on the following GitHub pull request review: eclipse-omr#6945 (review). Closes: eclipse-omr#4550. Author: Amarpreet Singh amarpreet1997@gmail.com.

singh264 · 2023-05-15T11:49:13Z

The local test results of the code changes in the GitHub pull request is located at the following url: https://drive.google.com/file/d/19lcbXGXhrajZTzfy08-JWLPv3ZffOLMh/view?usp=sharing.

The local test results of the code changes in the GitHub pull request could be displayed with the file that is located at the following url: https://github.com/singh264/scripts/blob/main/display_test_openj9-openjdk-jdk8_results.sh.

github-actions bot added arch:aarch32 arch:aarch64 arch:power arch:x86 arch:z comp:compiler labels Apr 10, 2023

singh264 marked this pull request as ready for review April 10, 2023 15:59

singh264 requested review from fjeremic, aviansie-ben, 0xdaryl, knn-k, vijaysun-omr, Leonardo2718 and mstoodle as code owners April 10, 2023 15:59

VermaSh suggested changes May 11, 2023

View reviewed changes

singh264 added 2 commits May 14, 2023 21:21

Add the macros with the hardcoded prefixes

16b4052

Add the macros with the hardcoded prefixes of format O\^O [a-z A-Z]+(:)?. Closes: eclipse-omr#4550. Author: Amarpreet Singh amarpreet1997@gmail.com.

Refactor the code

8ce9e53

The code is refactored based on the following GitHub pull request review: eclipse-omr#6945 (review). Closes: eclipse-omr#4550. Author: Amarpreet Singh amarpreet1997@gmail.com.

singh264 force-pushed the omr_issues_4550 branch from 54e526d to 8ce9e53 Compare May 15, 2023 11:47

singh264 requested a review from VermaSh May 15, 2023 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the macros with the hardcoded prefixes #6945

Add the macros with the hardcoded prefixes #6945

singh264 commented Apr 10, 2023

singh264 commented Apr 10, 2023

singh264 commented Apr 10, 2023

VermaSh commented May 1, 2023

VermaSh left a comment

VermaSh May 10, 2023

VermaSh May 10, 2023

VermaSh May 10, 2023

VermaSh May 10, 2023

VermaSh May 10, 2023

VermaSh May 11, 2023

VermaSh May 11, 2023

VermaSh May 11, 2023

VermaSh May 11, 2023

VermaSh May 11, 2023

singh264 commented May 15, 2023

Add the macros with the hardcoded prefixes #6945

Are you sure you want to change the base?

Add the macros with the hardcoded prefixes #6945

Conversation

singh264 commented Apr 10, 2023

singh264 commented Apr 10, 2023

singh264 commented Apr 10, 2023

VermaSh commented May 1, 2023

VermaSh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singh264 commented May 15, 2023