Skip to content

Vector256<Integer>.AllBitsSet emits illegal instructions on a machine without AVX2 #48283

Description

@SingleAccretion

Description

On a machine without AVX2 but with AVX (Sandy Bridge / Ivy Bridge), Vector256<IntegralType>.AllBitsSet will emit illegal instructions and cause the program to crash.

In particular, this method:

public static Vector256<byte> Problem() => Vector256<byte>.AllBitsSet;

Will emit the following:

       vzeroupper 
G_M65241_IG02:
       vpcmpeqd ymm0, ymm0, ymm0
       vmovupd  ymmword ptr[rcx], ymm0
       mov      rax, rcx
G_M65241_IG03:
       vzeroupper 
       ret

According to https://www.felixcloutier.com/x86/pcmpeqb:pcmpeqw:pcmpeqd, vpcmpeqd with ymm operands is only available when AVX2 is.

This is also a problem for Vector256.Create(-1).

Configuration

This applies to .NET 5 and .NET 6, tested on an Ivy Bridge machine running Windows 10 x64.

Regression?

This is not a regression in case of AllBitsSet (the API was introduced in .NET 5), but it is a regression from .NET Core 3.1 for the Create(-1) call (lowering now folds it to the same get_AllBitsSet node, see also #47385).

Other information

During importation, Vector256<byte>.AllBitsSet depends on the Vector256 ISA, presumably because it is declared like so:

HARDWARE_INTRINSIC(Vector256, get_AllBitsSet, 32, 0, {INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_pcmpeqd, INS_cmpps, INS_cmppd}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_NoContainment|HW_Flag_NoRMWSemantics)

This ISA is available when AVX is available, and the intrinsic is recognized:

Named Intrinsic System.Runtime.Intrinsics.Vector256`1.get_AllBitsSet: Notify VM instruction set (Vector256) must be supported.

And the node is later emitted as vpcmpeqd:

case NI_Vector128_get_AllBitsSet:
case NI_Vector256_get_AllBitsSet:
{
assert(op1 == nullptr);
if (varTypeIsFloating(baseType) && compiler->compOpportunisticallyDependsOn(InstructionSet_AVX))
{
// The immediate 8 means Equal (unordered, non-signaling)
// This is not available without VEX prefix.
emit->emitIns_SIMD_R_R_R_I(ins, attr, targetReg, targetReg, targetReg, 8);
}
else
{
assert(varTypeIsIntegral(baseType) || !compiler->compIsaSupportedDebugOnly(InstructionSet_AVX));
emit->emitIns_SIMD_R_R_R(INS_pcmpeqd, attr, targetReg, targetReg, targetReg);
}
break;

cc @tannergooding

Activity

  1. ghost added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on Feb 14, 2021
  2. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Feb 14, 2021
  3. EgorBo commented on Feb 15, 2021

    @EgorBo
    Member

    but it is a regression from .NET Core 3.1 for the Create(-1) call

    As far as I remember that optimization (Create(-1) to get_AllBitsSet) was introduced in .NET 5.0

  4. SingleAccretion commented on Feb 15, 2021

    @SingleAccretion
    ContributorAuthor

    Indeed, that is my understanding as well and why I pointed it out as a regression.

    I. e. this application works fine when targeting netcoreapp3.1 and but throws SEHException when targeting net5.0:

    using System;
    using System.Runtime.
    using System.Runtime.Intrinsics;
    
    Console.WriteLine(Vector256.Create(-1));
  5. JulieLeeMSFT commented on Feb 16, 2021

    @JulieLeeMSFT
    Member

    CC @echesakovMSFT

  6. added
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    and removed
    untriagedNew issue has not been triaged by the area owner
    on Feb 16, 2021
  7. echesakov commented on Feb 16, 2021

    @echesakov
    Contributor

    I will take a look

  8. added and removed
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    on Feb 17, 2021
  9. added this to the 6.0.0 milestone on Feb 17, 2021
  10. echesakov commented on Feb 17, 2021

    @echesakov
    Contributor

    Setting 6.0 milestone for now

  11. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Feb 17, 2021
  12. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Feb 19, 2021
  13. echesakov commented on Feb 19, 2021

    @echesakov
    Contributor

    Reopened to track 5.0 backporting

  14. modified the milestones: 6.0.0, 5.0.x on Feb 19, 2021
  15. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Feb 22, 2021
  16. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Mar 10, 2021
  17. SingleAccretion commented on Mar 11, 2021

    @SingleAccretion
    ContributorAuthor

    Backported in #48630.

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

Metadata

Metadata

Assignees

Labels

arch-x64area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIbug

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions