RFR: 8366702: C2 SuperWord: refactor VTransform vector nodes [v2]
Christian Hagedorn
chagedorn at openjdk.org
Mon Sep 8 08:45:19 UTC 2025
On Mon, 8 Sep 2025 08:21:29 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> Personally, I'd like to keep p0. An alternative is first. Or something even much longer that just inflates the code and does not make it more readable either.
"first_node_in_pack" would be more understandable I think. But it's much longer than `p0` indeed. Does it matter here if we pick the first, second or just any other node? If not, than maybe "pack_node" would just be expressive enough? But anyways, as you point out, we already use `p0` all over the place. And doing an extensive renaming should be done in a separate task in one go and more people should agree to it before doing it.
> We also have t0 and s0 all over the SuperWord code. And honestly we do the same in all sorts of IGVN code as well, right?
Yes, I personally would prefer to have more names than abbreviations. But that's subjective again :-)
> We could make a uint req = p0->req() but I don't think that is more helpful. req is not a very great name but we are stuck with it because of the definition in Node. Detaching it from p0 would probably not help but rather make it harder to read.
What if you just name it `p0_req`? It was more about sharing and making it `const` since `p0` does not change. But feel free to leave it as it is.
> All of this is rather subjective though :/
Indeed...
>> src/hotspot/share/opto/vtransform.hpp line 617:
>>
>>> 615:
>>> 616: public:
>>> 617: static VTransformVectorNodePrototype make_from_pack(const Node_List* pack, const VLoopAnalyzer& vloop_analyzer) {
>>
>> When switching to "Properties", you could also rename this to something like "fetch_from_pack" since `make` also suggests to actually creating a dummy-kind node when it's only trying to fetch useful information.
>
> I prefer the `make_...` naming. I think it is generally used as a "factory" prefix all over the place. `fetch` means we would be "loading" if from somewhere, and that's not what we do here - rather we just construct the `Properties` given the pack.
I guess with `Properties` in the name, it's more clear now 👍
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27056#discussion_r2329554430
PR Review Comment: https://git.openjdk.org/jdk/pull/27056#discussion_r2329553671
More information about the hotspot-compiler-dev
mailing list