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

Conversation

@grant-d
Copy link

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

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
@grant-d
Copy link
Author

grant-d commented Feb 11, 2019

@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
Copy link
Author

grant-d commented Feb 11, 2019

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
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.

@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
Copy link
Author

grant-d commented Feb 14, 2019

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
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
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
Copy link
Author

grant-d commented Feb 15, 2019

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

@grant-d
Copy link
Author

grant-d commented Feb 15, 2019

/azp run corefx-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@grant-d
Copy link
Author

grant-d commented Feb 15, 2019

/azp test corefx-ci (Linux x64_Release)

@azure-pipelines

This comment has been minimized.

@grant-d
Copy link
Author

grant-d commented Feb 15, 2019

/azp run corefx-ci (Linux x64_Release)

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@grant-d
Copy link
Author

grant-d commented Feb 15, 2019

Can we merge?

@grant-d
Copy link
Author

grant-d commented Feb 18, 2019

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