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

Optimizer hints - Unsafe.Assume #4966

Open
jkotas opened this issue Jan 19, 2016 · 15 comments
Open

Optimizer hints - Unsafe.Assume #4966

jkotas opened this issue Jan 19, 2016 · 15 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions optimization tenet-performance Performance related issue
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Jan 19, 2016

C/C++ compilers allows passing hints to the optimizer using __assume or similar constructs. It would be useful to have same capability in .NET.

One use case was discussed in https://github.com/dotnet/corefx/issues/5474#issuecomment-172653606:

    Unsafe.Assume((p & 0x1F) == 0);
    Unsafe.Write<Vector<double>>(p, v);

category:proposal
theme:optimization
skill-level:expert
cost:large

@jkotas
Copy link
Member Author

jkotas commented Jan 19, 2016

cc @CarolEidt

@CarolEidt
Copy link
Contributor

It would be great to design such a feature to capture both the assertion about the expected behavior as well as allowing that behavior to be validated. That is, perhaps in Debug one might check the condition, while when generating optimized code (e.g. in a Release build), one might assume it to be true. We've already got Debug.Assert and Trace.Assert. Maybe this should be Unsafe.Assert, and it would check the condition in Debug builds, but assume the condition for optimization purposes in the Release build.

@nietras
Copy link
Contributor

nietras commented Jan 21, 2016

Sounds interesting. How would this be expressed in IL?

@mikedn
Copy link
Contributor

mikedn commented Jan 21, 2016

Just like any other method call. It's just that the JIT assumes that the expression passed as argument always evaluates to true and can attempt to infer useful information from that.

That said, the "infer" part is a bit of a can of worms. Any bool expression can be used but only the simplest ones will be useful.

@GSPP
Copy link

GSPP commented Aug 6, 2016

This can be tremendously useful because it's a way turn off any JIT safety. x != null turns off a null check. index >= 0 && index < array.Length turns off a bounds check.

This design is very extensible because with one API addition more and more assumptions can be made to work in the future.

There should be a small catalog of patterns that are recognized.

(Update: Removed the builtin_expect thought and posted them elsewhere.)

@nietras
Copy link
Contributor

nietras commented Aug 6, 2016

@GSPP elsewhere would then be https://github.com/dotnet/coreclr/issues/6024 just so we have the link :)

@Daniel-Svensson
Copy link
Contributor

Sounds like you want the exising Contract.Assume but without the tooling.

https://msdn.microsoft.com/en-us/library/system.diagnostics.contracts.contract.assume(v=vs.110).aspx

@redknightlois
Copy link

@Daniel-Svensson Using Contract.Assume is not a bad idea at all (it is there already and there are many annotations that the JIT could understand if properly stated).

Another interesting use of Assume is hinting that a particular store on an array should be a non-temporal write. Therefore, you can use non-temporal instructions and avoiding polutting the cache if you are going to write and never read that information in that method. For example:

public int[] Sum ( int[] op1, int[] op2 )
{
    var result = new int[op1.Lenght];
    Contract.Assume( Performance.NonTemporal(result) );

    /// here goes result[i] = op1[i] + op2[i]
}

And that can be done to achieve very interesting results

public int[] Sum ( int[] op1, int[] op2 )
{
    if ( op1.Length > 4096 ) // or whatever it makes sense for the kernel
    {
       Contract.Assume( Performance.NonTemporal(op1) );
       Contract.Assume( Performance.NonTemporal(op2) );
    }
    var result = new int[op1.Lenght];
    Contract.Assume( Performance.NonTemporal(result) );

    /// here goes result[i] = op1[i] + op2[i]
}

This is pretty handy optimization because by the time the kernel ends the cache is poluted with only the last batch of data.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 30, 2020
@msftgits msftgits added this to the Future milestone Jan 30, 2020
@GrabYourPitchforks
Copy link
Member

As an extension to this, it might be useful to specify contracts on return values from methods. A strawman:

[return: AssumeNotNull]
public static object MyMethod();

[return: AssumeNonNegative]
public static int MyOtherMethod();

For example, the expression new Span<T>(Array.Empty<T>()) still performs the null check at the beginning of the ctor because the JIT doesn't have enough information to prove that the return value of Array.Empty<T>() is non-null.

@GSPP
Copy link

GSPP commented Feb 21, 2020

@GrabYourPitchforks The caller can already specify anything like this:

var result = MyMethod();
Assume(Anything(result));

The advantage of this would be that a single unified system (the assume method call) suffices for anything. This would be per callsite customizable as well.

On the other hand, those attributes would help all callsites and can be library provided.

@GSPP
Copy link

GSPP commented Feb 21, 2020

A related idea would be to automatically analyze methods and discover invariants such as non-null return value. This seems very expensive to do in the JIT. It could be implemented as a tool that can optionally be run on any assembly or application. The tool would produce a file that records all discovered invariants and data flow information. The JIT would consume that data. This could discover all kinds of useful information such as non-nullness, more precise types, integer ranges and constants.

@redknightlois
Copy link

redknightlois commented Feb 21, 2020

The tool would produce a file that records all discovered invariants and data flow information. The JIT would consume that data. This could discover all kinds of useful information such as non-nullness, more precise types, integer ranges and constants.

You don't need an external file, you can bake that into the assembly itself when building.

@tannergooding
Copy link
Member

I had provided a similar proposal (these used to be linked, but I guess that was dropped in the migration): #24593

I believe the main difference is that this one is a general Assume(bool expression) which would need to rely on the thing producing IL to not optimize expression to a constant case and for the JIT to recognize specific IL patterns.

While in my proposal, I suggested explicit methods (such as AssumeAlignment(address, alignment)) which could be used to make the intent more obvious. It would also apply to cases like which branch is more likely to be taken or if they are split and a conditional move would be beneficial.

monojenkins pushed a commit to monojenkins/mono that referenced this issue Mar 2, 2020
Just a quick draft for dotnet/runtime#4966 for both RyuJIT CoreCLR and Mono-LLVM (maybe it will help to push it forward since it's 4 years old).

It allows programmers to give hints to JIT about branches' probabilities, e.g. to avoid goto hacks [like this](https://github.com/dotnet/runtime/blob/469ece74b6e5b7e94991dde21b3c2409194ceab9/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs#L523) in perf critical code.

#### Sample:
```csharp
static int Foo(int a)
{
    if (Unsafe.Unlikely(a <= 0))   // hint: `a` is unlikely to be <= 0
        return a * a;

    return 0;
}
```
#### Codegen without `Unlikely`:
```asm
       85D2                 test     edx, edx
       7F06                 jg       SHORT G_M36845_IG05
       8BC2                 mov      eax, edx
       0FAFC2               imul     eax, edx
       C3                   ret  ;; return a * a
G_M36845_IG05:
       8BC2                 mov      eax, edx
       C3                   ret ;; return 0
```
#### Codgen with `Unlikely` (this is what this PR emits)
```asm
       85D2                 test     edx, edx
       7E03                 jle      SHORT G_M4270_IG05
       8BC2                 mov      eax, edx
       C3                   ret  ;; return 0
G_M4270_IG05:
       8BC2                 mov      eax, edx
       0FAFC2               imul     eax, edx
       C3                   ret  ;; return a * a
```

Also, I implemented it for Mono-LLVM (I use `@llvm.expect.i32` intrinsic, which is emitted when you use `__builtin_expect` in C/C++)
#### Mono-LLVM without `Unlikely`
```asm
	mov	ecx, edi
	imul	ecx, ecx   ;; Mono-LLVM emits branchless code for our sample
	xor	eax, eax
	test	edi, edi
	cmovle	eax, ecx
	ret
```
#### Mono-LLVM with `Unlikely`
```asm
	xor	eax, eax
	test	edi, edi
	jle	1 <gram_Foo1__int_+0x7>   ;; branch is better than cmove in this case
	ret                               ;; return 0
	imul	edi, edi
	mov	eax, edi
	ret                               ;; return a * a
```
cc @jkotas
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 24, 2020
@deeprobin
Copy link
Contributor

The common suggestion is:

Unsafe.Assume((p & 0x1F) == 0);

However, wouldn't it make more sense and be more maintainable in our current architecture that we use methods similar to test libraries:

namespace System.Runtime.CompilerServices {
    public static unsafe partial class Unsafe
    {
        [Intrinsic, NonVersionable]
        public static void AssumeEqual<T>(T first, T second);

        [Intrinsic, NonVersionable]
        public static void AssumeNotEqual<T>(T first, T second);

        [Intrinsic, NonVersionable]
        public static void AssumeNull<T>(T? val);

        [Intrinsic, NonVersionable]
        public static void AssumeNotNull<T>(T? val);

        [Intrinsic, NonVersionable]
        public static void AssumeIsType<T>(object? obj);

        [Intrinsic, NonVersionable]
        public static void AssumeIsAssignableFrom<T>(object? obj);
    }
}

I find the variant that uses a bool as parameter (Unsafe.Assume) not so nice, since the method call itself probably needs special handling (correct me if am wrong).

Alternatively, I would simply find a new C# syntax that is introduced useful.
But that would also require a new IL opcode.

public unsafe int MyUnsafeMethod(int a, int b) {
    assume a == 1;
    // or:
    __assume a == 1;
   
    return a + b;
}

@ayende
Copy link
Contributor

ayende commented Sep 2, 2022

I think that compiler intrinsic is much better option. I would also assume that having specific named methods make it easier to figure out specific scenarios. It also avoids the "any boolean expression goes here, only a few are actually recognized".

The downside is that things like:

  • Has specific alignment
  • Greater / less than a value

May create a lot of methods (then again, this is self documenting for what is actually supported).
Another benefit here is that you won't need to deal with avoiding constant folding, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions optimization tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests