RFR: 8310190: C2 SuperWord: AlignVector is broken, generates misaligned packs [v21]

Christian Hagedorn chagedorn at openjdk.org
Thu Dec 7 07:22:47 UTC 2023


On Wed, 6 Dec 2023 16:04:22 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/superword.cpp line 1628:
>> 
>>> 1626:   int element_size  = mem_ref->memory_size();
>>> 1627:   int vw            = pack_size * element_size;         // vector_width
>>> 1628:   int aw            = MIN2(vw, ObjectAlignmentInBytes); // alignment_width
>> 
>> Is there a specific reason to go with `vw` and `aw` instead of directly naming the variable `vector_width` and `alighment_width`?
>
> I replaced `vw` with `vector_width`, there were relatively few occurances. But if I replace `aw` with `alighment_width` then everything becomes much more "wordy", all lines become much longer. I would like to keep `aw`, I think it is much more readable then the long form. What do you think?

Thanks. Okay, I see your point. I agree that we can go with `aw` in the local scope of `pack_alignment_solution`/`AlignmentSolver`  for readability purposes (and you explain what it is in the comments/print statements). However, I think the `AlignmentSolution` class should use the full name `alignment_width` as a general property to store/return. What are your thoughts about that?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14785#discussion_r1418488407


More information about the hotspot-compiler-dev mailing list