bitscan forward/reverse on Windows

Started by John Naylorover 3 years ago3 messageshackers
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

Attached is a quick-and-dirty attempt to add MSVC support for the
rightmost/leftmost-one-pos functions.

0001 adds asserts to the existing coding.
0002 adds MSVC support. Tests pass on CI, but it's of course possible that
there is some bug that prevents hitting the fastpath. I've mostly used
the platform specific types, so some further cleanup might be needed.
0003 tries one way to reduce the duplication that arose in 0002. Maybe
there is a better way.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachments:

v3-0002-Add-MSVC-support-for-bitscan-reverse-forward.patchtext/x-patch; charset=US-ASCII; name=v3-0002-Add-MSVC-support-for-bitscan-reverse-forward.patchDownload+38-1
v3-0001-Add-asserts-to-verify-bitscan-intrinsics.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Add-asserts-to-verify-bitscan-intrinsics.patchDownload+60-27
v3-0003-Add-new-symbols-for-reducing-repetition.patchtext/x-patch; charset=US-ASCII; name=v3-0003-Add-new-symbols-for-reducing-repetition.patchDownload+40-33
#2John Naylor
john.naylor@enterprisedb.com
In reply to: John Naylor (#1)
Re: bitscan forward/reverse on Windows

I wrote:

Attached is a quick-and-dirty attempt to add MSVC support for the

rightmost/leftmost-one-pos functions.

0001 adds asserts to the existing coding.
0002 adds MSVC support. Tests pass on CI, but it's of course possible

that there is some bug that prevents hitting the fastpath. I've mostly used
the platform specific types, so some further cleanup might be needed.

I've cleaned these up and verified on godbolt.org that they work as
intended and still pass CI. I kept the Windows types as does other Winows
code in the tree, but used bool instead of unsigned char because it's used
like a boolean.

0003 is separate because I'm not quite sure how detailed to comment the
#ifdef maze. Could be left out.
0004 simplifies AllocSetFreeIndex() in the course of supporting MSVC. The
output is identical to HEAD in non-assert builds using gcc.

0002 through 0004 could be squashed together.

This plugs a hole in our platform-specific intrinsic support and is fairly
straightforward. Review welcome, but if there is none I intend to commit in
a week or two.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachments:

v4-0003-Add-more-comments-to-else-directives.patchtext/x-patch; charset=US-ASCII; name=v4-0003-Add-more-comments-to-else-directives.patchDownload+5-6
v4-0004-Rationalize-platform-support-in-AllocSetFreeIndex.patchtext/x-patch; charset=US-ASCII; name=v4-0004-Rationalize-platform-support-in-AllocSetFreeIndex.patchDownload+3-4
v4-0002-Add-MSVC-support-for-bitscan-reverse-forward.patchtext/x-patch; charset=US-ASCII; name=v4-0002-Add-MSVC-support-for-bitscan-reverse-forward.patchDownload+67-9
v4-0001-Add-asserts-to-verify-bitscan-intrinsics.patchtext/x-patch; charset=US-ASCII; name=v4-0001-Add-asserts-to-verify-bitscan-intrinsics.patchDownload+60-27
#3John Naylor
john.naylor@enterprisedb.com
In reply to: John Naylor (#2)
Re: bitscan forward/reverse on Windows

On Wed, Feb 8, 2023 at 3:14 PM John Naylor <john.naylor@enterprisedb.com>
wrote:

0001 adds asserts to the existing coding.
0002 adds MSVC support. Tests pass on CI, but it's of course possible

that there is some bug that prevents hitting the fastpath. I've mostly used
the platform specific types, so some further cleanup might be needed.

I've cleaned these up and verified on godbolt.org that they work as

intended and still pass CI. I kept the Windows types as does other Winows
code in the tree, but used bool instead of unsigned char because it's used
like a boolean.

0003 is separate because I'm not quite sure how detailed to comment the

#ifdef maze. Could be left out.

0004 simplifies AllocSetFreeIndex() in the course of supporting MSVC. The

output is identical to HEAD in non-assert builds using gcc.

0002 through 0004 could be squashed together.

This plugs a hole in our platform-specific intrinsic support and is

fairly straightforward. Review welcome, but if there is none I intend to
commit in a week or two.

I've committed 0001 separately, and squashed 0002 and 0004, deciding that
0003 didn't really add to readability.

--
John Naylor
EDB: http://www.enterprisedb.com