Skip to content

Optimize AdvSimd.Extract() when passed variable that can be const propagated #36070

Description

@kunalspathak
public byte DoExtract(Vector64<byte> vector) {
  int index = 5;
  AdvSimd.Extract(vector, index);
}

I was expecting it to generate:

G_M20657_IG02:
        3DC007B0          ldr     q16, [fp,#16]
        0E0B3E00          umov    w0, v16.b[5]

But instead we generate the following.

G_M20657_IG02:
        3DC007A0          ldr     q0, [fp,#16]
        528000A0          mov     w0, #5
        97FFF7F5          bl      System.Runtime.Intrinsics.Arm.AdvSimd:Extract(System.Runtime.Intrinsics.Vector128`1[Byte],ubyte):ubyte
        53001C00          uxtb    w0, w0

It happens because we decide whether to fallback or not depending on the index operand. If it is const, we generate the optimize code however this decision happens during importing and we won't know if the operand is constant or not until we do constant propagation which is in later phase.

We should also investigate if there are more scenarios in which we miss optimizing opportunity because of this dependency and evaluate if we should do the decision after constant propagation is done probably in lower.

category:cq
theme:hardware-intrinsics
skill-level:intermediate
cost:medium

Activity

  1. added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on May 7, 2020
  2. kunalspathak commented on May 7, 2020

    @kunalspathak
    ContributorAuthor

    //cc : @BruceForstall , @echesakovMSFT , @tannergooding

  3. added this to the 5.0 milestone on May 7, 2020
  4. removed
    untriagedNew issue has not been triaged by the area owner
    on May 7, 2020
  5. BruceForstall commented on May 7, 2020

    @BruceForstall
    Contributor
  6. echesakov commented on May 7, 2020

    @echesakov
    Contributor

    This is a case for any intrinsic that has an immediate operand - not only for Extract
    I wonder if it's possible to move an intrinsic call transofrmation from importer to a later phase.

  7. EgorBo commented on May 8, 2020

    @EgorBo
    Member

    Doesn't Roslyn propagate constants for such simple cases (without inlining) ?

  8. tannergooding commented on May 8, 2020

    @tannergooding
    Member

    This is basically a dupe of #11062 and possibly a couple other issues iirc. It is a problem on both x86/x64 and ARM64.

    Ideally we would delay the decision for this to be a call or constant until lowering but that isn't necessarily "easy" to do today.
    We already have minimal support for rewriting intrinsics to calls in Rationalization (which happens just before lowering) and are using that today for GT_INTRINSIC.

    We could presumably extend that to GenTreeJitIntrinsic as well, but it would require a few changes including making it a LARGE_NODE.
    I left some comments from my initial investigation here: #11062 (comment) including some of the issues I ran into with the RwriteIntrinsicAsUserCall method.

  9. tannergooding commented on May 8, 2020

    @tannergooding
    Member

    There is also another case that might be related where we miss some optimizations on x86 due to GT_CAST nodes: #35857 (comment)

    Basically the tree might look like:

                   [000038] -----+------                 +--*  CAST      int <- ubyte <- int
                   [000000] -----+------                 |  \--*  LCL_VAR   int    V01 arg0
    

    and certain instructions might be able to contain or consume the underlying value directly, but we aren't smart enough to contain and handle the cast today.

  10. modified the milestones: 5.0.0, 6.0.0 on Jun 22, 2020
  11. BruceForstall commented on Jun 22, 2020

    @BruceForstall
    Contributor

    Since it appears unlikely we will work on this for 5.0, I've moved it out to 6.0.

    cc @AndyAyersMS who might be interested in the phase ordering aspect of this.

  12. added
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    on Mar 23, 2021
  13. added
    Priority:3Work that is nice to have
    and removed
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    on Jun 3, 2021
  14. modified the milestones: 6.0.0, Future on Jun 3, 2021
  15. removed their assignment
    on Mar 15, 2022
  16. echesakov commented on Mar 15, 2022

    @echesakov
    Contributor

    Un-assigning myself
    cc @BruceForstall

  17. BruceForstall commented on Mar 16, 2022

    @BruceForstall
    Contributor

    Closing as a dup of #11062

  18. ghost locked as resolved and limited conversation to collaborators on Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority:3Work that is nice to havearea-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions