Skip to content

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented May 12, 2024

When LowerStoreIndirCoalescing fails to merge two stores it should at least make the stores STP-friendly (when it's legal). Example:

static void Test(ref Vector128<byte> g)
{
    Unsafe.Add(ref g, 0) = default;
    Unsafe.Add(ref g, 1) = default;
}

Codegen diff (on arm64):

; Method Bench:Test(byref) (FullOpts)
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
            movi    v16.4s, #0
-           str     q16, [x0]
-           movi    v16.4s, #0
-           str     q16, [x0, #0x10]
+           movi    v17.4s, #0
+           stp     q16, q17, [x0]
            ldp     fp, lr, [sp], #0x10
            ret     lr
-; Total bytes of code: 32
+; Total bytes of code: 28
  • remove quirk from assertprop.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 12, 2024
@dotnet-policy-service
Copy link
Contributor

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

@jakobbotsch
Copy link
Member

Have you looked into generalizing TryMakeIndirsAdjacent? I think it would be a fair bit more general than this.

@EgorBo
Copy link
Member Author

EgorBo commented May 12, 2024

Have you looked into generalizing TryMakeIndirsAdjacent? I think it would be a fair bit more general than this.

I have, but that was just a lot more work while here it's a few lines of code for -30kb diff, I guess this hack can be removed once OptimizeForLdp learns how to make code STP friendly

@jakobbotsch
Copy link
Member

Hmm, let me give it a quick try. I think the only hard part is extending the smarter interference check, but perhaps that's not necessary to get most cases.

@EgorBo
Copy link
Member Author

EgorBo commented May 12, 2024

Hmm, let me give it a quick try. I think the only hard part is extending the smarter interference check, but perhaps that's not necessary to get most cases.

Sure, go ahead 🙂

@jakobbotsch
Copy link
Member

It took a bit of finesse (and let's see what CI says), but #102133 has a prototype

@EgorBo
Copy link
Member Author

EgorBo commented May 12, 2024

It took a bit of finesse (and let's see what CI says), but #102133 has a prototype

Hoping to see https://dev.azure.com/dnceng-public/public/_build/results?buildId=672905&view=ms.vss-build-web.run-extensions-tab diffs there as well 😉

@EgorBo EgorBo closed this May 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
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.

2 participants