RFR: 8366702: C2 SuperWord: refactor VTransform vector nodes

Emanuel Peter epeter at openjdk.org
Mon Sep 8 06:16:13 UTC 2025


On Mon, 8 Sep 2025 06:05:02 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/superwordVTransformBuilder.cpp line 154:
>> 
>>> 152:   Node* p0 = pack->at(0);
>>> 153:   const VTransformVectorNodePrototype prototype = VTransformVectorNodePrototype::make_from_pack(pack, _vloop_analyzer);
>>> 154:   const int  sopc = prototype.scalar_opcode();
>> 
>> Suggestion:
>> 
>>   const int sopc = prototype.scalar_opcode();
>> 
>> Nit: whitespace
>> Or were you trying to align with the line below? Personally, I find this a bit too much, but up to you.
>
> Yes, I was trying to get alignment. I'll try some alternatives.

Variant 1: no alignment

~  787   uint vlen = vector_length();
~  788   int vopc = _vector_opcode;
   789   BasicType bt = element_basic_type();
   790   const TypeVect* vt = TypeVect::make(bt, vlen);

It looks a bit noisy to me.

Variant 2: align on assignment operator

~  787   uint vlen          = vector_length();                                                                                                                                                                                                                                                                                                                                                                                              
~  788   int vopc           = _vector_opcode;
~  789   BasicType bt       = element_basic_type();
   790   const TypeVect* vt = TypeVect::make(bt, vlen);

Better. But somehow I'd still prefer if the names were also aligned. Question if left or right aligned looks better.

3a

~  787   uint            vlen = vector_length();                                                                                                                                                                                                                                                                                                                                                                                            
~  788   int             vopc = _vector_opcode;
~  789   BasicType       bt   = element_basic_type();
~  790   const TypeVect* vt   = TypeVect::make(bt, vlen);

3b

~  787   uint          vlen = vector_length();
~  788   int           vopc = _vector_opcode;
~  789   BasicType       bt = element_basic_type();                                                                                                                                                                                                                                                                                                                                                                                         
   790   const TypeVect* vt = TypeVect::make(bt, vlen);


Personally the last one looks the calmest to me. But it's a bit of a funky choice compared to the rest of hotspot, so it is probably just my own brain that thinks it is good.
I think I'm going with variant 2, since that is a little less controversial I think, and still a bit better than no alignment at all.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27056#discussion_r2329234097


More information about the hotspot-compiler-dev mailing list