Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag #5202

ashu-mehra · 2020-05-15T22:11:53Z

In OpenJ9 the JProfiling framework is used to create profiled code
which contains references to internal data structures like block
frequencies stored in TR_BlockFrequencInfo class, or the flag for
checking if the method is already in recompilation queue.
To support AOT compilation for such code, we need to generate relocation
records for such data structures referenced by the compiled code.
This commit adds the code for generating the relocation records
for block frequency and the recompilation queued flag.
Currently it only handles x and p architectures.

Signed-off-by: Ashutosh Mehra mehra.ashutosh@ibm.com

dsouzai

Overall looks ok to me.

dsouzai · 2020-05-19T14:46:16Z

compiler/p/codegen/OMRCodeGenerator.cpp

@@ -2970,6 +2970,14 @@ OMR::Power::CodeGenerator::addMetaDataForLoadAddressConstantFixed(
         TR::DebugCounter::generateRelocation(comp, firstInstruction, node, counter, seqKind);
         return;
         }
+
+      case TR_BlockFrequency:
+	 {


Formatting.

vijaysun-omr · 2020-05-28T16:41:39Z

I don't have questions related to the "how" in this PR but I did have a question around "why" we feel that relocating this data is important ?

vijaysun-omr · 2020-05-28T17:13:26Z

@genie-omr build all

Leonardo2718 · 2020-05-28T17:24:22Z

I don't have questions related to the "how" in this PR but I did have a question around "why" we feel that relocating this data is important ?

It would be good if that information was in the commit message too.

vijaysun-omr · 2020-06-17T19:21:00Z

Ping @ashu-mehra this is probably only awaiting some more description from you before it can be considered for merging.

vijaysun-omr · 2020-08-19T19:18:59Z

Ping again to see if this can be pushed forward in the next few days

vijaysun-omr · 2020-08-20T11:59:38Z

The request for a more descriptive commit message in prior comments probably needs to be addressed.

ashu-mehra · 2020-08-20T14:05:43Z

~~This PR depends on OpenJ9 PR #5202.~~ There is some more work needed and hence it is still in draft mode.

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for recompilation queue flag. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

ashu-mehra · 2020-08-20T14:07:12Z

I have updated the commit messages to provide more information.

vijaysun-omr · 2020-08-20T18:30:50Z

Okay if you don't mind, please make it WIP until you are ready to have me consider it for merging. That way, I don't mistakenly take action when you did not want me to.

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for recompilation queue flag. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for recompilation queue flag. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

vijaysun-omr · 2020-10-05T12:56:31Z

Any update on this ?

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for recompilation queue flag. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for recompilation queue flag. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records specifically for block frequency. Signed-off-by: Ashutosh Mehra <mehra.ashutosh@ibm.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records for block frequency and the recompilation queued flag. Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. OpenJ9 PR eclipse-omr/omr#5202 creates the necessary structures representing the relocation record for this data. This commit adds the code for generating the relocation records for block frequency and the recompilation queued flag. Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2021-01-08T21:45:12Z

Windows failure is due to SegFault in ThreadCreateTest that runs as part of threadtest. It cannot be related to the changes in this PR.

ashu-mehra · 2021-01-08T21:46:03Z

@dsouzai another pr for your review.

dsouzai

I noticed that you you've only created the relo records for x and p; I know that this stuff isn't supported on the other platforms, but you might as well add them for the others. That way when it's finally enabled on the other platforms, the infra will already be in place.

dsouzai · 2021-01-08T22:06:38Z

compiler/p/codegen/OMRCodeGenerator.cpp

+            (uint8_t *)recordInfo,
+            TR_BlockFrequency, self());
+         break;
+         }


How come you don't also need a relo record for TR_RecompQueuedFlag here?

TR_RecompQueuedFlag gets taken care by the default case towards the end of the method:

if (!relo) { relo = new (self()->trHeapMemory()) TR::BeforeBinaryEncodingExternalRelocation( firstInstruction, (uint8_t *)value, (uint8_t *)seqKind, (TR_ExternalRelocationTargetKind)typeAddress, self()); }

dsouzai · 2021-01-08T22:06:48Z

compiler/p/codegen/OMRCodeGenerator.cpp

+                                                                                          (uint8_t *)recordInfo,
+                                                                                          (TR_ExternalRelocationTargetKind)typeAddress, self()),
+                                                                                          __FILE__, __LINE__, node);
+      }


How come you don't also need a relo record for TR_RecompQueuedFlag here?

TR_RecompQueuedFlag gets taken care by the last else if block:

else if (typeAddress != -1) { TR_RelocationRecordInformation *recordInfo = ( TR_RelocationRecordInformation *)comp->trMemory()->allocateMemory(sizeof( TR_RelocationRecordInformation), heapAlloc); recordInfo->data1 = (uintptr_t)value; recordInfo->data3 = orderedPairSequence2; self()->addExternalRelocation(new (self()->trHeapMemory()) TR::ExternalOrderedPair32BitRelocation((uint8_t *)firstInstruction, (uint8_t *)secondInstruction, (uint8_t *)recordInfo, (TR_ExternalRelocationTargetKind)typeAddress, self()), __FILE__, __LINE__, node); }

dsouzai · 2021-01-08T22:08:46Z

compiler/x/amd64/codegen/OMRMemoryReference.cpp

+            //TR::StaticSymbol *staticSym = sr.getSymbol()->getStaticSymbol();
+            //TR_ASSERT(staticSym, "Expected static symbol for block frequency\n");
+            //TR::Relocation *relocation = new (cg->trHeapMemory()) TR::ExternalRelocation(displacementLocation, (uint8_t *)staticSym->getStaticAddress(), TR_BlockFrequency, cg);


Can these comments be removed?

dsouzai · 2021-01-08T22:11:03Z

compiler/x/amd64/codegen/OMRMemoryReference.cpp

+            TR::Relocation *relocation = new (cg->trHeapMemory()) TR::ExternalRelocation(displacementLocation, (uint8_t *)recordInfo, TR_BlockFrequency, cg);
+            cg->addExternalRelocation(relocation, __FILE__, __LINE__, containingInstruction->getNode());
+            }
+         }


How come you don't also need a relo record for TR_RecompQueuedFlag here?

hmm it does look like TR_RecompQueuedFlag should be handled here, but during my testing I noticed relocation records are getting generated for TR_RecompQueuedFlag. Probably they are getting generated from other locations.
Let me check why it is working without the relo record for TR_RecompQueuedFlag here.

Looking at the relocation records generated with this change set I see TR_BlockFrequency is getting generated twice for the same offset, eg:

TR_BlockFrequency (102) /home/asmehra/data/ashu-mehra/openj9-openjdk-jdk11/omr/compiler/x/amd64/codegen/OMRMemoryReference.cpp 524 0191 0151 00007F9E174BFD80 TargetAddress1:00007F9E15DE4D10, TargetAddress2:0000000000000000 TR_BlockFrequency (102) /home/asmehra/data/ashu-mehra/openj9-openjdk-jdk11/omr/compiler/x/codegen/X86BinaryEncoding.cpp 2937 0191 0151 00007F9E174BFD80 TargetAddress1:00007F9E15DE4DA0, TargetAddress2:0000000000000000

but TR_RecompQueuedFlag is generated only once.

OMR::X86::AMD64::MemoryReference::generateBinaryEncoding creates a new instruction to move the addr to reg [1].
Then we add metadata for the address as part of which relocation records are generated [2].
This is the place where this patch has code for creating relo record for TR_BlockFrequency but not for TR_RecompQueuedFlag.
Then further down in OMR::X86::AMD64::MemoryReference::generateBinaryEncoding when the binary encoding for the new instruction is generated [3], we again generate metadata [4] where this patch creates relo records for TR_BlockFrequency and TR_RecompQueuedFlag.
So effectively we end up creating duplicate relo records for TR_BlockFrequency. We should be generating the relo record only at one place, which I feel should be when adding metadata for the instruction. Going by that logic, the relo record generated at [2] for TR_BlockFrequency should be avoided.

On similar note, probably TR_DebugCounter also has similar issue of duplicate records getting generated.

@dsouzai your thoughts on this?

[1] https://github.com/eclipse/omr/blob/bded46caf73287830139853961ffacb96cc9758e/compiler/x/amd64/codegen/OMRMemoryReference.cpp#L612
[2] https://github.com/eclipse/omr/blob/bded46caf73287830139853961ffacb96cc9758e/compiler/x/amd64/codegen/OMRMemoryReference.cpp#L644
[3] https://github.com/eclipse/omr/blob/bded46caf73287830139853961ffacb96cc9758e/compiler/x/amd64/codegen/OMRMemoryReference.cpp#L664
[4] https://github.com/eclipse/omr/blob/bded46caf73287830139853961ffacb96cc9758e/compiler/x/codegen/X86BinaryEncoding.cpp#L2796

Hm interesting. Yeah I think you're right; the debug counter relo doesn't look like it belongs in [2] above, especially since there's no TR_DataAddress in [2]. So TR_BlockFrequency doesn't belong in [2] either.

dsouzai · 2021-01-08T22:13:10Z

compiler/x/codegen/X86BinaryEncoding.cpp

+
+      case TR_RecompQueuedFlag:
+         {
+         TR::StaticSymbol *staticSym = symbol->getStaticSymbol();


This seems to be unused.

dsouzai · 2021-01-08T22:13:19Z

compiler/x/codegen/X86BinaryEncoding.cpp

+      }
+   else if (symbol->isRecompQueuedFlag())
+      {
+      TR::StaticSymbol *staticSym = symbol->getStaticSymbol();


This seems to be unused.

In OpenJ9 the JProfiling framework is used to create profiled code which contains references to internal data structures like block frequencies stored in `TR_BlockFrequencInfo` class, or the flag for checking if the method is already in recompilation queue. To support AOT compilation for such code, we need to generate relocation records for such data structures referenced by the compiled code. This commit adds the code for generating the relocation records for block frequency and the recompilation queued flag. Currently it only handles x and p architectures. Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2021-01-18T19:00:17Z

@dsouzai I have updated the pr addressing the review comments and also added the changes for z arch.

Thanks to @dchopra001 for testing the z changes using eclipse-openj9/openj9#11625.

dsouzai

lgtm; thanks @ashu-mehra

vijaysun-omr · 2021-01-25T14:08:12Z

@genie-omr build all

vijaysun-omr · 2021-01-25T23:03:51Z

Checks have passed and code has been reviewed by @dsouzai and @mpirvu. I am merging.

ashu-mehra · 2021-01-29T15:08:19Z

@vijaysun-omr can this be merged?

ashu-mehra mentioned this pull request May 15, 2020

Add new relocation records for block frequency eclipse-openj9/openj9#9591

Merged

0xdaryl added the comp:compiler label May 19, 2020

dsouzai reviewed May 19, 2020

View reviewed changes

vijaysun-omr self-assigned this May 21, 2020

ashu-mehra force-pushed the create_relorecord_block_frequency branch from c4d5d33 to df97e33 Compare August 19, 2020 19:29

ashu-mehra force-pushed the create_relorecord_block_frequency branch from df97e33 to c215809 Compare August 20, 2020 14:06

ashu-mehra mentioned this pull request Aug 20, 2020

Design: Enable JProfiling for JITServer eclipse-openj9/openj9#9974

Open

4 tasks

ashu-mehra changed the title ~~Create relocation records of type TR_BlockFrequency~~ WIP Create relocation records of type TR_BlockFrequency Aug 20, 2020

ashu-mehra force-pushed the create_relorecord_block_frequency branch from c215809 to c16938a Compare August 26, 2020 23:17

ashu-mehra force-pushed the create_relorecord_block_frequency branch 2 times, most recently from deeb63f to f77ef57 Compare November 4, 2020 15:18

ashu-mehra force-pushed the create_relorecord_block_frequency branch from f77ef57 to 831b33e Compare November 4, 2020 16:31

ashu-mehra force-pushed the create_relorecord_block_frequency branch from 831b33e to 3943892 Compare January 8, 2021 21:07

ashu-mehra marked this pull request as ready for review January 8, 2021 21:09

ashu-mehra requested review from 0xdaryl, aviansie-ben and mstoodle as code owners January 8, 2021 21:09

ashu-mehra changed the title ~~WIP Create relocation records of type TR_BlockFrequency~~ Create relocation records of type TR_BlockFrequency Jan 8, 2021

ashu-mehra changed the title ~~Create relocation records of type TR_BlockFrequency~~ Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag Jan 8, 2021

dsouzai requested changes Jan 8, 2021

View reviewed changes

ashu-mehra mentioned this pull request Jan 12, 2021

Enable block frequency profiling in JITServer eclipse-openj9/openj9#11625

Open

ashu-mehra force-pushed the create_relorecord_block_frequency branch from 3943892 to 9475912 Compare January 15, 2021 21:08

ashu-mehra requested a review from fjeremic as a code owner January 15, 2021 21:08

ashu-mehra force-pushed the create_relorecord_block_frequency branch 2 times, most recently from 4d70afa to 27a98fd Compare January 15, 2021 21:35

ashu-mehra force-pushed the create_relorecord_block_frequency branch from 27a98fd to c249f4d Compare January 18, 2021 16:51

dsouzai approved these changes Jan 22, 2021

View reviewed changes

vijaysun-omr merged commit 53ef3b8 into eclipse-omr:master Jan 29, 2021

mpirvu mentioned this pull request May 11, 2021

Checklist for JITServer GA eclipse-openj9/openj9#12666

Open

54 tasks

SajinaKandy mentioned this pull request Jan 4, 2024

WIP:Enable block frequency profiling in JITServer eclipse-openj9/openj9#18516

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag #5202

Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag #5202

ashu-mehra commented May 15, 2020 •

edited

Loading

dsouzai left a comment

dsouzai May 19, 2020

vijaysun-omr commented May 28, 2020

vijaysun-omr commented May 28, 2020

Leonardo2718 commented May 28, 2020

vijaysun-omr commented Jun 17, 2020

vijaysun-omr commented Aug 19, 2020

vijaysun-omr commented Aug 20, 2020

ashu-mehra commented Aug 20, 2020 •

edited

Loading

ashu-mehra commented Aug 20, 2020

vijaysun-omr commented Aug 20, 2020

vijaysun-omr commented Oct 5, 2020

ashu-mehra commented Jan 8, 2021

ashu-mehra commented Jan 8, 2021

dsouzai left a comment

dsouzai Jan 8, 2021

ashu-mehra Jan 8, 2021

dsouzai Jan 8, 2021

ashu-mehra Jan 8, 2021

dsouzai Jan 8, 2021

dsouzai Jan 8, 2021

ashu-mehra Jan 9, 2021

ashu-mehra Jan 11, 2021

dsouzai Jan 11, 2021

dsouzai Jan 8, 2021

dsouzai Jan 8, 2021

ashu-mehra commented Jan 18, 2021

dsouzai left a comment

vijaysun-omr commented Jan 25, 2021

vijaysun-omr commented Jan 25, 2021

ashu-mehra commented Jan 29, 2021

Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag #5202

Create relocation records of type TR_BlockFrequency and TR_RecompQueuedFlag #5202

Conversation

ashu-mehra commented May 15, 2020 • edited Loading

dsouzai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vijaysun-omr commented May 28, 2020

vijaysun-omr commented May 28, 2020

Leonardo2718 commented May 28, 2020

vijaysun-omr commented Jun 17, 2020

vijaysun-omr commented Aug 19, 2020

vijaysun-omr commented Aug 20, 2020

ashu-mehra commented Aug 20, 2020 • edited Loading

ashu-mehra commented Aug 20, 2020

vijaysun-omr commented Aug 20, 2020

vijaysun-omr commented Oct 5, 2020

ashu-mehra commented Jan 8, 2021

ashu-mehra commented Jan 8, 2021

dsouzai 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

Choose a reason for hiding this comment

ashu-mehra commented Jan 18, 2021

dsouzai left a comment

Choose a reason for hiding this comment

vijaysun-omr commented Jan 25, 2021

vijaysun-omr commented Jan 25, 2021

ashu-mehra commented Jan 29, 2021

ashu-mehra commented May 15, 2020 •

edited

Loading

ashu-mehra commented Aug 20, 2020 •

edited

Loading