RFR: 8327109: Refactor data graph cloning used in create_new_if_for_predicate() into separate class

Christian Hagedorn chagedorn at openjdk.org
Mon Mar 4 08:13:59 UTC 2024


On Fri, 1 Mar 2024 13:58:42 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Suggestion for better name `CloneDataNodeGraph`. Do you assert that only data nodes are cloned, and no CFG nodes?
>
> Yes, you do verify it, great!

> You could have a typedef for `ResizeableResourceHashtable<Node*, Node*, AnyObj::RESOURCE_AREA, mtCompiler>`. Then you don't need to use `auto` for it elsewhere, and it is clear what it is. Suggestion: `OrigToNewHashtable`.

Good idea. I'll add one.

> The name could mention that we are cloning. And maybe you could do the work in the constructor, and just have accessors for the finished products, such as `_orig_to_new`. Suggestion for better name CloneDataNodeGraph.

As mentioned earlier, we are later gonna reuse this class when refactoring `create_bool_from_template_assertion_predicate()`. For template assertion predicates we not only need to clone nodes but also need to transform the `OpaqueLoop*Nodes`. Therefore, I went with keeping the name of this class as `DataNodeGraph` and use `_orig_to_new` and not use `_orig_to_clone` since we could be transforming `OpaqueLoop*Nodes` in such a way that we replace it with existing nodes and not clones.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18080#discussion_r1510704146


More information about the hotspot-compiler-dev mailing list