RFR: 8257069: C2: Clarify and sanity test RegMask/RegMaskIterator
Vladimir Kozlov
kvn at openjdk.java.net
Wed Nov 25 19:47:56 UTC 2020
On Wed, 25 Nov 2020 14:23:53 GMT, Claes Redestad <redestad at openjdk.org> wrote:
> This patch adds a sanity test to RegMask. It's not (yet) exhaustive, but it covers most common operations and iteration.
>
> While implementing this test I noticed that the code have a strong, implied assumption that RM_SIZE is even on 64-bit platforms, and a few things could have broken badly if RM_SIZE was odd (mismatch between set_AllStack/is_AllStack, extra bits after CHUNK_SIZE). That RM_SIZE is even is thankfully an invariant, since the AD preprocessor aligns up RM_SIZE. Add a static assert to this effect, and prefer _RM_SIZE for clarity.
>
> The iteration algorithm in RegMaskIterator, which I borrowed from IndexSetIterator, has garnered a few raised eyebrows. I think the quirky scheme of not shifting out the bit of interest but instead clear is mainly explained by the need to avoid undefined behavior in the corner case when only the highest bit is set. I've added some commentary to try and clarify the quirks.
src/hotspot/share/opto/regmask.hpp line 170:
> 168: void set_AllStack() {
> 169: _RM_UP[_RM_SIZE - 1U] |= (uintptr_t(1) << (_WordBits - 1U));
> 170: }
`_WordBits - 1U` is used in several places in this file. Should we add and use new _WordBitsM1 value instead?
-------------
PR: https://git.openjdk.java.net/jdk/pull/1437
More information about the hotspot-compiler-dev
mailing list