Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Units for BitOps.TrailingZeroCount - #35193

Merged
jkotas merged 26 commits into
dotnet:masterfrom
grant-d:grant-d.tzcnt.units
Feb 18, 2019
Merged

Units for BitOps.TrailingZeroCount#35193
jkotas merged 26 commits into
dotnet:masterfrom
grant-d:grant-d.tzcnt.units

Conversation

@grant-d

@grant-d grant-d commented Feb 9, 2019

Copy link
Copy Markdown

Units for BitOps from related PRs: LZCNT, LOG2, TZCNT, ROTL/R, POPCNT

cc @tannergooding, @jkotas

Grant Dickinson added 2 commits February 8, 2019 18:09
Comment thread src/Common/tests/Common.Tests.csproj Outdated
@grant-d

grant-d commented Feb 11, 2019

Copy link
Copy Markdown
Author

@stephentoub it's failing due to this line in the CoreCLR sources - I am not familiar with what that line imports. Any advice on how to fix this, perhaps that line can be removed? Or I need a suitable #if.

...BitOps.cs(9,7): error CS0246: The type or namespace name 'Internal' could not be found...

using Internal.Runtime.CompilerServices;

[Edit] It needs that line for access to Unsafe.AddByteOffset.
Not sure how to mitigate the issue on this side.

@grant-d

grant-d commented Feb 11, 2019

Copy link
Copy Markdown
Author

Tests are failing due to this code in BitOps. It's missing the explicit conversion to IntPtr, which was apparently fine on Windows but not Linux et al.
I will fix in the next push I make in coreclr.

public static int TrailingZeroCount(uint value)
    ....
    return Unsafe.AddByteOffset(
        ref MemoryMarshal.GetReference(s_TrailingZeroCountDeBruijn),
        (IntPtr)(((value & -value) * 0x077CB531u) >> 27)); // <--- (IntPtr)

@stephentoub

Copy link
Copy Markdown
Member

Any advice on how to fix this, perhaps that line can be removed?

Your workaround of adding an empty namespace to the test file seems fine. Thanks.

Comment thread src/Common/src/CoreLib/System/BitOps.cs Outdated
@grant-d grant-d changed the title Units for BitOps.TrailingZeroCount Units for BitOps.TrailingZeroCount (WIP) Feb 11, 2019
@grant-d grant-d changed the title Units for BitOps.TrailingZeroCount (WIP) Units for BitOps.TrailingZeroCount Feb 14, 2019
@grant-d

grant-d commented Feb 14, 2019

Copy link
Copy Markdown
Author

Jumped the gun. These won't pass until the sync from coreclr

@grant-d grant-d changed the title Units for BitOps.TrailingZeroCount Units for BitOps.TrailingZeroCount (WIP) Feb 15, 2019
@grant-d grant-d changed the title Units for BitOps.TrailingZeroCount (WIP) Units for BitOps.TrailingZeroCount Feb 15, 2019
<Compile Include="$(CommonPath)\System\Security\IdentityHelper.cs">
<Link>Common\System\Security\IdentityHelper.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\CoreLib\System\BitOps.cs">

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this once BitOps is exposed

namespace Internal.Runtime.CompilerServices
{
// Dummy namespace needed for compilation of BitOps
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this once BitOps is exposed

@grant-d

grant-d commented Feb 15, 2019

Copy link
Copy Markdown
Author

@dotnet-bot test corefx-ci (Linux x64_Release) please

@grant-d

grant-d commented Feb 15, 2019

Copy link
Copy Markdown
Author

/azp run corefx-ci

@azure-pipelines

Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@grant-d

grant-d commented Feb 15, 2019

Copy link
Copy Markdown
Author

/azp test corefx-ci (Linux x64_Release)

@azure-pipelines

This comment has been minimized.

@grant-d

grant-d commented Feb 15, 2019

Copy link
Copy Markdown
Author

/azp run corefx-ci (Linux x64_Release)

@azure-pipelines

Copy link
Copy Markdown

No pipelines are associated with this pull request.

@grant-d

grant-d commented Feb 15, 2019

Copy link
Copy Markdown
Author

Can we merge?

@grant-d

grant-d commented Feb 18, 2019

Copy link
Copy Markdown
Author

Thanks for approving Jan, LGTM?

@jkotas
jkotas merged commit 23f36ed into dotnet:master Feb 18, 2019
@grant-d
grant-d deleted the grant-d.tzcnt.units branch February 18, 2019 19:50
@karelz karelz added this to the 3.0 milestone Mar 18, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants