RFR 8164632: Node indices should be treated as unsigned integers

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Aug 13 22:58:44 UTC 2020


Yes, it is sloppy :(

Mostly it bases on value of MaxNodeLimit = 80000 by default and as result node's idx will never reach MAX_INT.

For EA we need 2 special types TOP and BOTTOM as Paul correctly pointed in RFE.
We can make InstanceTop == max_juint and node_idx_t type for _instance_id . We don't do arithmetic on it, see 
TypeOopPtr::meet_instance_id(). But we can't use assert in this case to check incoming idx because max_juint will be 
valid value - InstanceTop.

And I agree that we should use node_idx_t everywhere.

For example, Node::Init(), init_node_notes(), node_notes_at() and set_node_notes_at() should use it.

Same goes for req and other Node's methods arguments. All Node fields defined as node_idx_t but we have mix of int and 
uint when referencing them.

Warning: it is not small change.

Regards,
Vladimir

On 8/13/20 2:51 PM, Hohensee, Paul wrote:
> Shouldn't all the uint type uses that represent node indices actually be node_idx_t?
> 
> Thanks,
> Paul
> 
> On 8/13/20, 12:34 AM, "hotspot-compiler-dev on behalf of Tobias Hartmann" <hotspot-compiler-dev-retn at openjdk.java.net on behalf of tobias.hartmann at oracle.com> wrote:
> 
>      Hi Eric,
> 
>      there are other places where Node::_idx is casted to int (and a potential overflow might happen).
>      For example, calls to Compile::node_notes_at.
> 
>      The purpose of this RFE was to replace all Node::_idx uint -> int casts and consistently use uint
>      for the node index. If that's not feasible, we should at least add a guarantee (not only an assert)
>      checking that _idx is always <= MAX_INT.
> 
>      Best regards,
>      Tobias
> 
>      On 12.08.20 00:41, Eric, Chan wrote:
>      > Hi,
>      >
>      > Requesting review for
>      >
>      > Webrev : http://cr.openjdk.java.net/~xliu/eric/8164632/00/webrev/
>      > JBS : https://bugs.openjdk.java.net/browse/JDK-8164632
>      >
>      > The change cast uint ni to integer so that the parameter that pass to method TypeOopPtr::cast_to_instance_id is a integer.
>      >
>      > I have tested this builds successfully .
>      >
>      > Ensured that there are no regressions in hotspot : tier1 tests.
>      >
>      > Regards,
>      > Eric Chen
>      >
> 


More information about the hotspot-compiler-dev mailing list