RFR: 8313672: C2: PhaseCCP does not correctly track analysis dependencies involving shift, convert, and mask [v8]
Daniel Lundén
duke at openjdk.org
Mon Nov 6 16:25:09 UTC 2023
On Mon, 6 Nov 2023 14:06:05 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> Yes, it is true that we sometimes may visit nodes multiple times. However, is it worth the extra bookkeeping overhead to ensure uses are visited only once? At least for the current applications of `visit_uses`, it's not unsound to (quite rarely, I think) add a node twice or more times to worklists.
>>
>> I'm fine with either option, let me know what you think.
>
> I agree, it will be rare. But we don't know what it will be used for. One might want to count all uses or whatever, and then duplicates could matter.
>
> You can also leave a clear comment, that uses may be called multiple times, then it is up to the user.
>
> I would simply have renamed `internals` -> `worklist`.
> And then push all nodes to it.
> But when you look at a node, you first check it `is_boundary` -> `callback`.
> Else put all its outputs on `worklist`.
> I think the overhead is not that much.
You are right, of course; we can use the `Unique_Node_List` to ensure we only visit uses once (I somehow managed to not read the `Unique` part of the class name and thought it was a basic stack). Then, the overhead is already there in any case and we can easily guarantee only one use. I'll make the change.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1383602913
More information about the hotspot-compiler-dev
mailing list