Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix asserts in Vector<> #78765

Merged
merged 4 commits into from
Nov 25, 2022
Merged

Fix asserts in Vector<> #78765

merged 4 commits into from
Nov 25, 2022

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Nov 23, 2022

Fixes #78717 (and unblocks jit-diff)

This fixes two asserts:

  1. This used to assert while it is expected to early out in importer as an unsupported type.
Int128 Foo(Vector<Int128> v) => v[0];
  1. Prejitting of
        [Intrinsic]
        [CLSCompliant(false)]
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static Vector<T> Load<T>(T* source)
            where T : unmanaged => LoadUnsafe(ref *source);

Fails in that assert assert(genActualType(arg->gtType) == genActualType(argType));
My understanding it's completely fine that we pass ULONG (representing pointer) to BYREF

PTAL @tannergooding

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 23, 2022
@ghost ghost assigned EgorBo Nov 23, 2022
@ghost
Copy link

ghost commented Nov 23, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes (and unblocks jit-diff)

This fixes two asserts:
1)

Int128 Foo(Vector<Int128> v) => v[0];

this used to assert while is expected to early out in importer as an unsupported type.

Prejitting of

        [Intrinsic]
        [CLSCompliant(false)]
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static Vector<T> Load<T>(T* source)
            where T : unmanaged => LoadUnsafe(ref *source);

Fails in that assert assert(genActualType(arg->gtType) == genActualType(argType));
where argType is BYREF and arg is ULONG.

PTAL @tannergooding

Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo
Copy link
Member Author

EgorBo commented Nov 25, 2022

@jakobbotsch does it look good otherwise?

Comment on lines +564 to +567
if (!impCheckImplicitArgumentCoercion(argType, arg->gtType))
{
BADCODE("the hwintrinsic argument has a type that can't be implicitly converted to the signature type");
}
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we should have this check outside the else but let's not rerun CI for it

@EgorBo EgorBo merged commit 4f53c2f into dotnet:main Nov 25, 2022
@EgorBo EgorBo deleted the fix-vector-asserts branch November 25, 2022 22:27
@ghost ghost locked as resolved and limited conversation to collaborators Dec 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NativeAOT] System.Numerics.Vectors test fails to compile with an assert.
2 participants