On Wed, 8 Apr 2026 12:36:40 GMT, Stefan Karlsson <[email protected]> wrote:
>> @caspernorrbin Started cleaning up markWord.hpp with >> https://github.com/openjdk/valhalla/pull/2238, but while reviewing that >> patch I realized that it would be good to make a number of simplifications >> before proceeding. I talked that through with Casper and @Arraying. Those >> changes have now been done, and I've taken a pass through markWord.hpp to >> updated the code in the spirit of what Casper did and some of the things we >> three talked about. >> >> I've also made updates and clarifications to the comments. >> >> One main thing about this proposal is that I want it to be crystal clear >> when we talk about bit and when we talk about a mask. To make sure that the >> bit constants only ever refers to one bit I've added helpers to verify that >> this is the case. > > src/hotspot/share/oops/markWord.hpp line 145: > >> 143: static_assert((bits) == 1 NOT_LP64(|| (bits) == 0)); \ >> 144: return mask_in_place((bits), (shift)); \ >> 145: }() > > I want to be crystal clear when we're talking about one bit. So, > `bit_in_place` is used when we have only one bit, and `mask_in_place` is used > when have a consecutive set of bits. This disambiguates usage sites, IMHO. A brief comment about this in the source code would make it clearer, it wasn't immediately obvious to me. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2310#discussion_r3057791709
