[PATCH] D25062: [x86][inline-asm][AVX512][llvm][PART-2] Introducing "k" and "Yk" constraints for extended inline assembly, enabling use of AVX512 masked vectorized instructions.

[Reid Kleckner via cfe-commits]

rnk added a comment.

Looks good functionally with some surface level nits



> X86ISelLowering.cpp:31979
>        break;
>      }
>    }

Let's check size 2 after size 1, it seems more logical, and is 1 is probably the common case.

> X86ISelLowering.cpp:32025
>    case 'Y':
> +    // Impliment Y<x> (two letters variant) constraints handle here.
> +    if (constraint[1] == 'k') {

Typo on "Impliment". Also the comment could be reworded.

> X86ISelLowering.cpp:32321
> +        //  Only supported in AVX512 or later.
> +        if (VT == MVT::i32)
> +          return std::make_pair(0U, &X86::VK32RegClass);

Seems like a good place to use a switch on VT

> X86ISelLowering.cpp:32429
> +    case 'k':
> +      // This regiter class doesn't allocate k0 for masked vector operation.
> +      if (Subtarget.hasAVX512()) { // Only supported in AVX512.

"register"

> X86ISelLowering.cpp:32431
> +      if (Subtarget.hasAVX512()) { // Only supported in AVX512.
> +        if (VT == MVT::i32)
> +          return std::make_pair(0U, &X86::VK32WMRegClass);

Maybe do a switch?

Repository:
  rL LLVM

https://reviews.llvm.org/D25062