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

8283775: better dump: VM support for graph querying in debugger with BFS traversal and node filtering #8468

Closed
wants to merge 42 commits into from

Conversation

eme64
Copy link
Contributor

@eme64 eme64 commented Apr 29, 2022

What this gives you for the debugger

  • BFS traversal (inputs / outputs)
  • node filtering by category
  • shortest path between nodes
  • all paths between nodes
  • readability in terminal: alignment, sorting by node idx, distance to start, and colors (optional)
  • and more

Some usecases

  • more readable dump
  • follow only nodes of some categories (only control, only data, etc)
  • find which control nodes depend on data node (visit data nodes, include control in boundary)
  • how two nodes relate (shortest / all paths, following input/output nodes, or both)
  • find loops (control / memory / data: call all paths with node as start and target)

Description
I implemented VM support for BFS traversal of IR nodes, where one can filter for input/output edges, and the node-type (control / data / memory). If one specifies the target node, we find a shortest path between this and target. With the options string one can easily select which node types to visit (cdmxo) and which to include only in the boundary (CDMXO). To find all paths between two nodes, include the letter A in the options string.

void Node::dump_bfs(const int max_distance, Node* target, char const* options)

To get familiar with the many options, run this to get help:
find_node(0)->dump_bfs(0,0,"h")

While a sufficient summary and description is in the comments above the function definition, I want to explain some useful use-cases here.

Please let me know if you would find this helpful, or if you have any feedback to improve it.
Thanks, Emanuel

PS: I do plan to refactor the dump code in node.cpp to use my new infrastructure. I will also remove Node::related and dump_related, since it has not been properly extended and maintained. But that refactoring would risk messing with tools that depend on dump, which I would like to avoid for now, and do that in a second step.

Better dump()
The function is similar to dump(), we can also follow input / output edges up to a certain distance. There are a few improvements/added features:

  1. Display the distance to the specified node. This way I can quickly assess how many nodes I have at a certain distance, and I can traverse edges from one distance to the next.
  2. Choose if you want to traverse only input + or output - edges, or both +-.
  3. Node filtering with types taken from type->category() help one limit traversal to control, data or memory flow. The categories are the same as in IGV.
  4. Separate visit / boundary filters by node type: traverse graph visiting only some node types (eg. data). On the boundary, also display but do not traverse nodes allowed by boundary filter (eg. control). This can be useful to traverse outputs of a data node recursively, and see what control nodes depend on it. Use dcmxo for visit filter, and DCMXO for boundary filter.
  5. Probably controversial: coloring of node types in terminal. By default it is off. May not work on all terminals. But I find it very useful to quickly separate memory, data and control - just as in IGV - but in my terminal! Highly recommend putting the # in the options string! To more easily trace chains of nodes, I highlight the node idx of all nodes that are displayed in their respective colors.
  6. After matching, we create Mach nodes from the IR nodes. For most Mach nodes, there is an associated old node. Often an assert is triggered for Mach nodes, but the graph already breaks in the IR graph. It is thus very helpful to have the old node idx displayed. Use @ in options string.
  7. I also display the head node idx of the block a Mach node is scheduled for. This can be helpful if one gets bad dominance asserts, and needs to see why the blocks were scheduled in a bad way. Use B in options string.
  8. Some people like the displayed nodes to be sorted by node idx. Simply add an S to the option string!

Example (BFS inputs):

(rr) p find_node(161)->dump_bfs(2,0,"dcmxo+")
dist dump
---------------------------------------------
   2  159  CmpI  === _ 137 40  [[ 160 ]]  !orig=[144] !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   2  147  IfTrue  === 161  [[ 166 ]] #1 !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   2  165  OuterStripMinedLoop  === 165 93 164  [[ 165 166 ]] 
   1  160  Bool  === _ 159  [[ 161 ]] [lt] !orig=[145] !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   1  166  CountedLoop  === 166 165 147  [[ 166 161 102 103 ]] stride: 1  strip mined !orig=[157],[99] !jvms: StringLatin1::hashCode @ bci:16 (line 193)
   0  161  CountedLoopEnd  === 166 160  [[ 162 147 ]] [lt] P=0.957374, C=19675.000000 !orig=[146] !jvms: StringLatin1::hashCode @ bci:13 (line 193)

Example (BFS control inputs):

(rr) p find_node(163)->dump_bfs(5,0,"c+")
dist dump
---------------------------------------------
   5  147  IfTrue  === 161  [[ 166 ]] #1 !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   5  165  OuterStripMinedLoop  === 165 93 164  [[ 165 166 ]] 
   4  166  CountedLoop  === 166 165 147  [[ 166 161 102 103 ]] stride: 1  strip mined !orig=[157],[99] !jvms: StringLatin1::hashCode @ bci:16 (line 193)
   3  161  CountedLoopEnd  === 166 160  [[ 162 147 ]] [lt] P=0.957374, C=19675.000000 !orig=[146] !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   2  162  IfFalse  === 161  [[ 167 168 ]] #0 !orig=148 !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   1  167  SafePoint  === 162 1 7 1 1 168 1 136 37 40 137 1  [[ 163 ]]  SafePoint  !orig=138 !jvms: StringLatin1::hashCode @ bci:37 (line 193)
   0  163  OuterStripMinedLoopEnd  === 167 22  [[ 164 148 ]] P=0.957374, C=19675.000000

We see the control flow of a strip mined loop.

Experiment (BFS only data, but display all nodes on boundary)

(rr) p find_node(102)->dump_bfs(10,0,"dCDMOX-")
dist dump
---------------------------------------------
   0  102  Phi  === 166 22 136  [[ 133 132 ]]  #int !jvms: StringLatin1::hashCode @ bci:16 (line 193)
   1  133  SubI  === _ 132 102  [[ 136 ]]  !jvms: StringLatin1::hashCode @ bci:25 (line 194)
   1  132  LShiftI  === _ 102 131  [[ 133 ]]  !jvms: StringLatin1::hashCode @ bci:25 (line 194)
   2  136  AddI  === _ 133 155  [[ 153 167 102 ]]  !jvms: StringLatin1::hashCode @ bci:32 (line 194)
   3  153  Phi  === 53 136 22  [[ 154 ]]  #int !jvms: StringLatin1::hashCode @ bci:13 (line 193)
   3  167  SafePoint  === 162 1 7 1 1 168 1 136 37 40 137 1  [[ 163 ]]  SafePoint  !orig=138 !jvms: StringLatin1::hashCode @ bci:37 (line 193)
   4  154  Return  === 53 6 7 8 9 returns 153  [[ 0 ]] 

We see the dependent output nodes of the data-phi 102, we see that a SafePoint and the Return depend on it. Here colors are really helpful, as it makes it easy to separate the data-nodes (blue) from the boundary-nodes (other colors).

Example with Mach nodes:

(rr) p find_node(280)->dump_bfs(2,0,"cdmxo+@B")
dist [block  head  idom depth]   old dump
---------------------------------------------
   2     B6   379   377     4   o118   38  sarI_rReg_CL  === _ 39 40  [[ 41 36 31 31 71 75 66 82 86 103 116 148 152 161 119 119 184 186 170 281 268 ]]  !jvms: String::length @ bci:9 (line 1487) ByteVector::putUTF8 @ bci:1 (line 285)
   2    B52   441   277    23   o738  283  incI_rReg  === _ 285  [[ 284 285 281 ]] #1/0x00000001 !jvms: ByteVector::putUTF8 @ bci:131 (line 300)
   2    B50   277   439    22   o756  282  IfTrue  === 273  [[ 441 ]] #1 !jvms: ByteVector::putUTF8 @ bci:100 (line 302)
   1    B52   441   277    23   o737  281  compI_rReg  === _ 283 38  [[ 280 ]] 
   1    B52   441   277    23      _  441  Region  === 441 282  [[ 441 280 290 ]] 
   0    B52   441   277    23   o757  280  jmpLoopEnd  === 441 281  [[ 279 347 ]] P=0.500000, C=21462.000000 !jvms: ByteVector::putUTF8 @ bci:79 (line 300)

And the query on the old nodes:

(rr) p find_old_node(741)->dump_bfs(2,0,"cdmxo+#")
dist dump
---------------------------------------------
   2 o1871  AddI  === _ o79 o1872  [[ o739 o1948 o761 o1477 ]] 
   2  o186  AddI  === _ o1756 o1714  [[ o1756 o739 o1055 ]] 
   2  o178  If  === o1159 o177 o176  [[ o179 o180 ]] P=0.800503, C=7153.000000
   1  o739  CmpI  === _ o186 o1871  [[ o740 o741 ]] 
   1  o740  Bool  === _ o739  [[ o741 ]] [lt]
   1  o179  IfTrue  === o178  [[ o741 ]] #1
   0  o741  CountedLoopEnd  === o179 o740 o739  [[ o742 o190 ]] [lt] P=0.993611, C=7200.000000

Exploring loop body
When I find myself in a loop, I try to localize the loop head and end, and map out at least one path between them.
loop_end->print_bfs(20, loop_head, "c+")
This provides us with a shortest control path, given this path has a distance of at most 20.

Example (shortest path over control nodes):

(rr) p find_node(741)->dump_bfs(20,find_node(746),"c+")
dist dump
---------------------------------------------
   5  746  CountedLoop  === 746 745 190  [[ 746 148 141 ]] stride: 1  strip mined !orig=[735],[138] !jvms: StringLatin1::replace @ bci:22 (line 304)
   4  148  RangeCheck  === 746 147  [[ 149 152 ]] P=0.999999, C=-1.000000 !jvms: StringLatin1::replace @ bci:25 (line 304)
   3  149  IfTrue  === 148  [[ 178 166 ]] #1 !orig=170 !jvms: StringLatin1::replace @ bci:25 (line 304)
   2  178  If  === 149 177  [[ 179 180 ]] P=0.800503, C=7153.000000 !jvms: StringLatin1::replace @ bci:28 (line 304)
   1  179  IfTrue  === 178  [[ 741 ]] #1 !jvms: StringLatin1::replace @ bci:28 (line 304)
   0  741  CountedLoopEnd  === 179 740  [[ 742 190 ]] [lt] P=0.993611, C=7200.000000 !orig=[189] !jvms: StringLatin1::replace @ bci:19 (line 303)

Once we see this single path in the loop, we may want to see more of the body. For this, we can run an all paths query, with the additional character A in the options string. We see all nodes that lay on a path between the start and target node, with at most the specified path length.

Example (all paths between two nodes):

(rr) p find_node(741)->dump_bfs(8,find_node(746),"cdmxo+A")
dist apd dump
---------------------------------------------
   6   8  146  CmpU  === _ 141 79  [[ 147 ]]  !jvms: StringLatin1::replace @ bci:25 (line 304)
   5   8  166  LoadB  === 149 7 164  [[ 176 747 ]]  @byte[int:>=0]:exact+any *, idx=5; #byte !jvms: StringLatin1::replace @ bci:25 (line 304)
   5   8  147  Bool  === _ 146  [[ 148 ]] [lt] !jvms: StringLatin1::replace @ bci:25 (line 304)
   5   5  746  CountedLoop  === 746 745 190  [[ 746 148 141 ]] stride: 1  strip mined !orig=[735],[138] !jvms: StringLatin1::replace @ bci:22 (line 304)
   4   5  141  Phi  === 746 36 186  [[ 185 186 162 146 154 154 747 ]]  #int:0..max-1:www #tripcount !orig=[161] !jvms: StringLatin1::replace @ bci:22 (line 304)
   4   8  176  CmpI  === _ 166 169  [[ 177 ]]  !jvms: StringLatin1::replace @ bci:28 (line 304)
   4   5  148  RangeCheck  === 746 147  [[ 149 152 ]] P=0.999999, C=-1.000000 !jvms: StringLatin1::replace @ bci:25 (line 304)
   3   5  186  AddI  === _ 141 51  [[ 185 739 141 ]]  !orig=[738],... !jvms: StringLatin1::replace @ bci:13 (line 303)
   3   8  177  Bool  === _ 176  [[ 178 ]] [ne] !jvms: StringLatin1::replace @ bci:28 (line 304)
   3   5  149  IfTrue  === 148  [[ 178 166 ]] #1 !orig=170 !jvms: StringLatin1::replace @ bci:25 (line 304)
   2   5  739  CmpI  === _ 186 79  [[ 740 ]]  !orig=[187] !jvms: StringLatin1::replace @ bci:19 (line 303)
   2   5  178  If  === 149 177  [[ 179 180 ]] P=0.800503, C=7153.000000 !jvms: StringLatin1::replace @ bci:28 (line 304)
   1   5  740  Bool  === _ 739  [[ 741 ]] [lt] !orig=[188] !jvms: StringLatin1::replace @ bci:19 (line 303)
   1   5  179  IfTrue  === 178  [[ 741 ]] #1 !jvms: StringLatin1::replace @ bci:28 (line 304)
   0   5  741  CountedLoopEnd  === 179 740  [[ 742 190 ]] [lt] P=0.993611, C=7200.000000 !orig=[189] !jvms: StringLatin1::replace @ bci:19 (line 303)

We see there are multiple paths. We can quickly see that there are paths with length 5 (apd = 5): the control flow, but also the data flow for the loop-back condition. We also see some paths with length 8, which feed into 178 If and 148 Rangecheck. Node that the distance d is the distance to the start node 741 CountedLoopEnd. The all paths distance apd computes the sum of the shortest path from the current node to the start plus the shortest path to the target node. Thus, we can easily compute the distance to the target node with apd - d.

An alternative to detect loops quickly, is running an all paths query from a node to itself:

Example (loop detection with all paths):

(rr) p find_node(741)->dump_bfs(7,find_node(741),"c+A")
dist apd dump
---------------------------------------------
   6   7  190  IfTrue  === 741  [[ 746 ]] #1 !jvms: StringLatin1::replace @ bci:19 (line 303)
   5   7  746  CountedLoop  === 746 745 190  [[ 746 148 141 ]] stride: 1  strip mined !orig=[735],[138] !jvms: StringLatin1::replace @ bci:22 (line 304)
   4   7  148  RangeCheck  === 746 147  [[ 149 152 ]] P=0.999999, C=-1.000000 !jvms: StringLatin1::replace @ bci:25 (line 304)
   3   7  149  IfTrue  === 148  [[ 178 166 ]] #1 !orig=170 !jvms: StringLatin1::replace @ bci:25 (line 304)
   2   7  178  If  === 149 177  [[ 179 180 ]] P=0.800503, C=7153.000000 !jvms: StringLatin1::replace @ bci:28 (line 304)
   1   7  179  IfTrue  === 178  [[ 741 ]] #1 !jvms: StringLatin1::replace @ bci:28 (line 304)
   0   0  741  CountedLoopEnd  === 179 740  [[ 742 190 ]] [lt] P=0.993611, C=7200.000000 !orig=[189] !jvms: StringLatin1::replace @ bci:19 (line 303)

We get the loop control, plus the loop-back 190 IfTrue.

Example (loop detection with all paths for phi):

(rr) p find_node(141)->dump_bfs(4,find_node(141),"cdmxo+A")
dist apd dump
---------------------------------------------
   1   2  186  AddI  === _ 141 51  [[ 185 739 141 ]]  !orig=[738],... !jvms: StringLatin1::replace @ bci:13 (line 303)
   0   0  141  Phi  === 746 36 186  [[ 185 186 162 146 154 154 747 ]]  #int:0..max-1:www #tripcount !orig=[161] !jvms: StringLatin1::replace @ bci:22 (line 304)

Color examples
Colors are especially useful to see chains between nodes (options character #).
The input and output node idx are also colored if the node is displayed somewhere in the list. This should help you find chains of nodes.
Tip: it can be worth it to configure the colors of your terminal to be more appealing.

Example (find control dependency of data node):
image
We see data nodes in blue, and find a SafePoint in red and the Return in yellow.

Example (find memory dependency of data node):
image

Example (loop detection):
image
We find the control and some data loop paths.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8283775: better dump: VM support for graph querying in debugger with BFS traversal and node filtering

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8468/head:pull/8468
$ git checkout pull/8468

Update a local copy of the PR:
$ git checkout pull/8468
$ git pull https://git.openjdk.java.net/jdk pull/8468/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8468

View PR using the GUI difftool:
$ git pr show -t 8468

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8468.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 29, 2022

👋 Welcome back eme64! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Apr 29, 2022

@eme64 The following label will be automatically applied to this pull request:

  • hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Apr 29, 2022
@openjdk openjdk bot added the rfr Pull request is ready for review label May 2, 2022
@robcasloz
Copy link
Contributor

I tried the functionality and found it useful, thanks! Just a few comments:

  • What does dir stand for in the table header? Whether the node is input/output to its parent? It would perhaps be more intuitive to decouple this information with the node category and put this in a separate column (cat?)
  • A whitespace is missing between the dis and dir columns when a target node is given.
  • If this functionality subsumes that provided by Node::dump(int) and Node::dump_ctrl(int), could you replace them to avoid code duplication? (perhaps leaving both interfaces to avoid large changes across the code base).

@eme64
Copy link
Contributor Author

eme64 commented May 2, 2022

In conversation with @TobiHartmann we decided to make it a member function of Node, and also expose it in node.hpp, so that it can be used for print-debugging.

@eme64
Copy link
Contributor Author

eme64 commented May 2, 2022

Thanks for looking at it, @robcasloz !
In reply to your coments:

I tried the functionality and found it useful, thanks! Just a few comments:

  • What does dir stand for in the table header? Whether the node is input/output to its parent? It would perhaps be more intuitive to decouple this information with the node category and put this in a separate column (cat?)
  • A whitespace is missing between the dis and dir columns when a target node is given.

I like the idea of making a cat column.
And I will have +/- in a separate column, but only displayed if both directions are enabled to save space.

  • If this functionality subsumes that provided by Node::dump(int) and Node::dump_ctrl(int), could you replace them to avoid code duplication? (perhaps leaving both interfaces to avoid large changes across the code base).

Yes, it would be nice to avoid code duplication. The question is if people will be ok with having their tools replaced.
Maybe we can do it as you say: leave the interfaces identical, and have them redirect to print_bfs with the relevant inputs.

@openjdk openjdk bot removed the rfr Pull request is ready for review label May 2, 2022
@openjdk openjdk bot added the rfr Pull request is ready for review label May 3, 2022
Copy link
Contributor

@vnkozlov vnkozlov left a comment

Choose a reason for hiding this comment

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

Nice.

@openjdk
Copy link

openjdk bot commented May 3, 2022

@eme64 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8283775: better dump: VM support for graph querying in debugger with BFS traversal and node filtering

Reviewed-by: kvn, chagedorn, thartmann, rcastanedalo

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 411 new commits pushed to the master branch:

  • d959c22: 8288000: compiler/loopopts/TestOverUnrolling2.java fails with release VMs
  • 230726e: 8287735: Provide separate event category for dll operations
  • ecf0078: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
  • 5c39a36: 8287522: StringConcatFactory: Add in prependers and mixers in batches
  • 47d3c2a: 8287980: Build is broken due to SuperWordMaxVectorSize when C2 is disabled after JDK-8287697
  • bf0e625: 8286451: C2: assert(nb == 1) failed: only when the head is not shared
  • 0960ecc: 8287700: C2 Crash running eclipse benchmark from Dacapo
  • 590337e: 8286625: C2 fails with assert(!n->is_Store() && !n->is_LoadStore()) failed: no node with a side effect
  • 4662e06: 8277307: Pre shared key sent under both session_ticket and pre_shared_key extensions
  • 7df48f9: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
  • ... and 401 more: https://git.openjdk.java.net/jdk/compare/82aa04558434f60f3b308e4da164cf44120efa67...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label May 3, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 4, 2022
@openjdk openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 4, 2022
@eme64
Copy link
Contributor Author

eme64 commented May 10, 2022

Feedback from @chhagedorn : refactor print_bfs into a class with methods, rather than one large function with lambdas.
I agree with this idea, will do it soon.

@openjdk
Copy link

openjdk bot commented May 12, 2022

@eme64 this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8283775
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

@openjdk openjdk bot added the merge-conflict Pull request has merge conflict with target branch label May 12, 2022
@eme64 eme64 changed the title 8283775: VM support for graph querying in debugger with BFS traversal and node filtering 8283775: better dump: VM support for graph querying in debugger with BFS traversal and node filtering May 31, 2022
@eme64 eme64 marked this pull request as ready for review May 31, 2022 16:41
@openjdk openjdk bot added the rfr Pull request is ready for review label May 31, 2022
Copy link
Member

@chhagedorn chhagedorn left a comment

Choose a reason for hiding this comment

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

These are cool new dumping features! I especially like the coloring and the filtering which follow the coloring and filtering of IGV. The implementation looks good apart from some minor code styles things. Thanks for addressing all the offline update suggestions!

@vnkozlov should also review it again as his review applies to an old state which is now quite different.

src/hotspot/share/opto/callnode.hpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
Copy link
Member

@chhagedorn chhagedorn left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing the updates.

Copy link
Contributor

@vnkozlov vnkozlov left a comment

Choose a reason for hiding this comment

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

Update looks good.

Copy link
Member

@TobiHartmann TobiHartmann left a comment

Choose a reason for hiding this comment

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

Looks great!

src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
Thank you @TobiHartmann

Co-authored-by: Tobias Hartmann <tobias.hartmann@oracle.com>
Copy link
Contributor

@robcasloz robcasloz left a comment

Choose a reason for hiding this comment

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

Thanks for adding this functionality, Emanuel! I tried it out and works fine for my regular use cases. The code itself looks good, too. I have a few minor comments, please take them as suggestions only.

src/hotspot/share/opto/node.hpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Outdated Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Show resolved Hide resolved
src/hotspot/share/opto/node.cpp Show resolved Hide resolved
Copy link
Contributor

@robcasloz robcasloz left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments, looks good!

@eme64
Copy link
Contributor Author

eme64 commented Jun 13, 2022

/integrate
@robcasloz @TobiHartmann @chhagedorn @vnkozlov Thank you for the reviews, and the many suggestions, here and in person.

@openjdk
Copy link

openjdk bot commented Jun 13, 2022

Going to push as commit 33ed036.
Since your change was applied there have been 471 commits pushed to the master branch:

  • ac28be7: 8283612: IGV: Remove Graal module
  • 0cb0ecf: 8209935: Test to cover CodeSource.getCodeSigners()
  • f1143b1: 8287696: Avoid redundant Hashtable.containsKey call in JarVerifier.doneWithMeta
  • d46f404: 8279047: Remove expired flags in JDK 20
  • f7a4be7: 8288270: Tier1 build failures after JDK-8287178
  • d4b473d: 8287178: IntegerModuloP::multiplicativeInverse returns 0 for 0
  • da2339c: 8288173: JDK-8202449 fix causes conformance test failure : api/java_util/Random/RandomGenerator/NextFloat.html
  • f2e10dc: 8288238: Add missing file jdk.incubator.concurrent-J.sym.txt
  • fa564e9: Merge
  • 0164145: 8288222: ProblemList serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java
  • ... and 461 more: https://git.openjdk.org/jdk/compare/82aa04558434f60f3b308e4da164cf44120efa67...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Jun 13, 2022
@openjdk openjdk bot closed this Jun 13, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 13, 2022
@openjdk
Copy link

openjdk bot commented Jun 13, 2022

@eme64 Pushed as commit 33ed036.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot-compiler hotspot-compiler-dev@openjdk.org integrated Pull request has been integrated
5 participants