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

SIMD problems when mixing llvm and non-llvm code #73454

Closed
Tracked by #43051
vargaz opened this issue Aug 5, 2022 · 12 comments · Fixed by #74582
Closed
Tracked by #43051

SIMD problems when mixing llvm and non-llvm code #73454

vargaz opened this issue Aug 5, 2022 · 12 comments · Fixed by #74582
Assignees
Milestone

Comments

@vargaz
Copy link
Contributor

vargaz commented Aug 5, 2022

Description

Currently on mono, SIMD is only supported when using LLVM compiled code. This can lead to various problems when mixing llvm and non-llvm compiled code. This mixing can happen for various reasons:

  • profiled AOT
  • LLVM doesn't support some IL constructs like filter clauses
  1. Different calling conventions
    LLVM code passes SIMD types in SIMD registers, while JITted code passes them on the stack.
    Testcase:
using System;
using System.Numerics;

public class Tests
{
    public static void Main () {
        foo (new Vector<int> (1));
    }

    static void foo (Vector<int> v) {
        try {
            Console.WriteLine (v);
        } catch (Exception ex) when (ex is OverflowException) {
            }
    }
}
  1. The IsSupported methods return different values in LLVM and non-LLVM methods, leading to subtle logic problems:
using System;
using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.Arm;

public class Tests
{
    public static void Main () {
        if (AdvSimd.Arm64.IsSupported)
            foo (new Vector128<double> ());
    }

    static void foo (Vector128<double> v) {
        try {
        } catch (Exception ex) when (ex is OverflowException) {
            }
        AdvSimd.Arm64.Abs (v);
    }
}

This will fail when AOT-ed using --aot=llvm

Reproduction Steps

.

Expected behavior

.

Actual behavior

.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 5, 2022
@vargaz vargaz added this to the 8.0.0 milestone Aug 5, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 5, 2022
@vargaz
Copy link
Contributor Author

vargaz commented Aug 5, 2022

@fanyang-mono

@lambdageek
Copy link
Member

This is another instance of the same problem #73003

Basically as long as we have a fallback execution engine (JIT or interpreter) that doesn't support some intrinsic (or returns IsSupported == false) we can't really support that intrinsic in an AOT mode (or return IsSupported == true)

@SamMonoRT
Copy link
Member

@lambdageek @fanyang-mono -- with #74357 merged(backported), I think we can move this to 8.0.0 ?

@fanyang-mono
Copy link
Member

I think so. If @vargaz has some idea of a quick fix for this, he could move it back later.

@lambdageek
Copy link
Member

we need to also do something with calling conventions

lambdageek added a commit to lambdageek/runtime that referenced this issue Aug 25, 2022
Partially reverts dotnet#68991

When LLVM code calls non-LLVM code it will pass arguments in SIMD
registers which the non-LLVM code doesn't expect

Fixes dotnet#73454 (issue 1)
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 25, 2022
github-actions bot pushed a commit that referenced this issue Aug 25, 2022
Partially reverts #68991

When LLVM code calls non-LLVM code it will pass arguments in SIMD
registers which the non-LLVM code doesn't expect

Fixes #73454 (issue 1)
carlossanlop pushed a commit that referenced this issue Aug 25, 2022
Partially reverts #68991

When LLVM code calls non-LLVM code it will pass arguments in SIMD
registers which the non-LLVM code doesn't expect

Fixes #73454 (issue 1)

Co-authored-by: Aleksey Kliger <alklig@microsoft.com>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2022
@vargaz vargaz reopened this Oct 23, 2022
@vargaz vargaz reopened this Oct 27, 2022
@MichalStrehovsky
Copy link
Member

MichalStrehovsky closed this as completed in bflattened/runtime@b9e2214

@vargaz Sorry about that - created a dummy default branch in that repo so that this never happens again.

@fanyang-mono
Copy link
Member

Moving this issue to .NET8, since we didn't had the bandwidth to handle it during .NET7.

@SamMonoRT
Copy link
Member

@fanyang-mono with recent intrinsic implementations and reverting for removing fallback paths, can this be closed?

@SamMonoRT SamMonoRT assigned fanyang-mono and unassigned lambdageek and SamMonoRT Aug 3, 2023
@matouskozak
Copy link
Member

I tested both cases with miniJIT and normal AOT with LLVM and both are working. However, I encountered failures during running fullAOT with LLVM versions of both cases. I'll investigate further what is causing these failures.

@matouskozak
Copy link
Member

I ran additional tests on osx-arm64 and these are the results:

  1. Different calling conventions:
    Tested with miniJIT and AOT/fullAOT (llvm) and both seem to be passing the Vector type using stack.

  2. The IsSupported methods return different values in LLVM and non-LLVM methods, leading to subtle logic problems:
    Tested with AOT/fullAOT (llvm) and both are working (not failing). My previously reported failures using fullAOT (llvm) were due to misconfigured build script.

@fanyang-mono
Copy link
Member

Based on @matouskozak's testing, the SIMD behavior between various codegen engines are aligned now. Closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

6 participants