ImageSharp fails to build on UWP Release x86

[Sergio0694]

Prerequisites

• I have written a descriptive issue title
• I have verified that I am running the latest version of ImageSharp
• I have verified if the problem exist in both DEBUG and RELEASE mode
• I have searched open and closed issues to ensure it has not already been reported

Description

So, it looks like I might have claimed victory too soon on this one 😅
This is a follow up from #828 (pinging @MattWhilden again, I'm so sorry).
With the latest .NETCore.UWP NuGet package (version 6.2.9) it looks like ImageSharp can now run just fine in Release x64, but not in Release x86.
Whenever I try to build in Release x86, I get the following:

ILT0005: 'C:\Users\Sergio.nuget\packages\runtime.win10-x86.microsoft.net.native.compiler\2.2.7-rel-27913-00\tools\x86\ilc\Tools\nutc_driver.exe @"C:\Users\Sergio\Documents\GitHub\Legere\src\Legere\obj\x86\Release\ilc\intermediate\MDIL\Legere.rsp"' returned exit code -1073740791

Once again, this looks like some issue with the .NET Native compiler itself, due to that ILT0005 error code, which is typically associated with internal compiler errors (as per .NET Native docs).

This happens both in my app and in an empty repro project (see point below).

@JimBobSquarePants @antonfirsov Looks like we'll have to hold off on considering ImageSharp as fully UWP-compatible for a little longer 😕

Steps to Reproduce

• Create an empty UWP project (I'm targeting SDK17763)
• Reference ImageSharp (I'm using the latest version, 1.0.0-beta0006)
• Build in Release x86

System Configuration

• ImageSharp version: 1.0.0-beta0006
• Other ImageSharp packages and versions: n/a
• Environment (Operating system, version and so on): Windows 10 Pro 18362.365 x64
• .NET Framework version: n/a
• Additional information: n/a

[MattWhilden]

That error code is 0xc0000409 and means STATUS_STACK_BUFFER_OVERRUN. So likely something sideways in either the optimizer or type system. I'll take a look on Monday.

[Sergio0694]

Hey @MattWhilden - thank you for taking a look during the weekend, I wasn't expecting a reply until Monday at least! Glad to hear you already have a couple ideas of what that could be, looking forward for an update on this, and as always I really appreciate your help with all these issues 😊
Cheers!

[MattWhilden]

The method we're failing to build this one.

It's got refs, vector, Span. A veritable who's who of compiler intrinsic and optimization things to think about. I'll likely need to route this to some other folks for review but I'll try to keep us all on top of it. Hopefully it's something that a little code jiggling can get everyone unblocked.

[Sergio0694]

Hey @MattWhilden - thank you for following up on this, glad to hear you've already pinpointed the exact method that's causing the issue.
And yeah let's hope it's something that can be fixed easily enough, after all it's already building just fine in x64 at least (for some reason), so let's say we're already halfway there on this 😄

[MattWhilden]

Definitely. The vast majority of the x86/x64 code paths are identical but you could imagine the different number of registers available can cause various things to kick in etc.

Won't know until we dig some more, but progress is progress. Thanks for your perseverance!

[MattWhilden]

I've been informed by my esteemed colleagues who work on the back-end optimizer that this is in fact a bug in vector handing. It's particularly insidious because it's not part of an optimization phase so there's not a trivial way to work around it (for example marking it with System.Runtime.CompilerServices.MethodImplOptionsAttribute).

They're going to work on this rapidly but it's usually a few weeks to get various design considerations worked through.

In the mean time, it's possible that tweaking the method in question by breaking it up, shuffling the Span usage around etc could get things jiggled enough to make it happy.

Last, it would make me feel better if we could work more closely when a fix does become available. Given we've already had a couple shots at this it'd be nice to get someone actually using this library to kick the tires on a new package before we jam it in to production. Let me know if that's the kind of thing you'd be interested in. Would hate to get another thing pushed out just to have to turn the crank again. ☹️

[Sergio0694]

Hi @MattWhilden - oof that definitely isn't the good news I was hoping for 😆

I figured it'd take at least a few weeks to get a new update for the package though, so that's fine. The idea of being able to test preview packages sounds great though, in fact that was my idea as well back when I reported the issue with ImageSharp for the first time.
If you'd like someone working with ImageSharp on UWP to help out, I'd be happy to do that!
I'd be using that anyway, and I'd love to be able to test preview packages to help you guys investigate this and make sure new patches do solve all the issues with this library.

As for trying to split that method up as a temporary workaround (possibly for the next ImageSharp preview package which would be coming out shortly anyway), @JimBobSquarePants @antonfirsov what'd be the best way to try this out?
Can I just do that in a branch and grab a NuGet package to install from one of the CIs, or would I have to manually create a package from my local branch?

[MattWhilden]

@Sergio0694 Sounds good. I'll let you know when we have something to try out.

The compiler tends to fail noisily when it's confused but there's always the potentially codegen issues at runtime. As you can imagine it's way easier for someone with a bit of experience with the library to be able to say, "hey, this behavior isn't crashing but also isn't quite correct".

[Sergio0694]

@MattWhilden awesome, looking forward to it! 😊

@JimBobSquarePants @antonfirsov So, I'm not sure how much of an interest the UWP support is for you, but if you'd like to "officially" support it I think we could look into creating a UWP Unit Test project (see here), with at least some general tests (to avoid just having to copy-paste the entire tests codebase).
It should be able to run in a CI, and that way we could automate the tests for the UWP Release mode. Even something like just loading a couple images and applying a few effects would be enough to make sure the library does work at least to some degree on UWP.
What do you think?

Also, let me know what you guys think would be the best way to do some refactoring tests for the failing method, to see if we could use that as a temporary workaround when publishing 1.0.0-beta7 👍

[antonfirsov]

@Sergio0694 it would be great if you could define a simple, self-contained ImageSharp test project that could be pulled and executed by the UWP team. Maybe they can even integrate some parts into their test suite. (We kick-ass, when it's about triggering bugs in all kinds of .NET runtimes & AOT compilers!)

Based on those experiences later we can try to build & run our default tests project on UWP. (This is far from being trivial though.)

@MattWhilden does it sound like a plan? Are there any other considerations we should be aware of?

[MattWhilden]

I couldn't help myself so spent my lunch fiddling with things a bit. Good news! I can now get a successful build of WuFrameQuantizer. It turns out removing one AgressiveInlining call here relieves enough pressure on things downstream to let the compile succeed. I've isolated out this class to play with it so I don't know if there will be other issues if you build the entire project.

One mitigating from this change (at least for UWP) is that the whole program optimizer is actually quite well tuned for making inlining decisions. It would be interesting to know if there's a noticeable performance issue from this change. I'd be a bit surprised but you never know.

Let me know how your investigations go. With any luck, that gets things unblocked for ya.

[MattWhilden]

@antonfirsov We can certainly head down that path. Just trying to keep things balanced with "get things unblocked cheaply" on one side and "oh golly everything is broken let's do some real heavy lifting". Given we're a few issues deep, I'm drifting closer to the latter but I don't think I'll pull the trigger just yet.

[antonfirsov]

@MattWhilden great job! That call is not critical at all, I can make a PR removing that inlining.

However, since we don't have any CI configured for UWP, we can break stuff any time with our changes without noticing it, and same is true for the UWP team I guess. Having a basic test suite would really worth the efforts. Maybe you would be able to isolate powerful regression tests from it.