[X86][AMX] Verify illegal types or instructions for x86_amx.

[LuoYuanke]

Summary

This patch is related to https://reviews.llvm.org/D100032 which define
some illegal types or operations for x86_amx. There are no arguments,
arrays, pointers, vectors or constants of x86_amx.

[LuoYuanke]

Remove attribute in test case.

[lebedev.ri]

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

2414–2416

Not the bitcast, the load/store is the problem.

[LuoYuanke]

Address Roman's comments.

[LuoYuanke]

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

2414–2416

Since x86_amx pointer is invalid (llvm/test/Verifier/x86_amx8.ll), the load/store form x86_amx is also invalid. I change it as transform in the comments.

[pengfei]

LGTM, let's see if others have objections.

[fhahn]

llvm/test/Verifier/x86_amx1.ll

3

can all those tests be in a single test file? I think the other verifier tests also group multiple checks in a single file

[LuoYuanke]

llvm/test/Verifier/x86_amx1.ll

3

Probably not. As llvm-as stops on the first error.

; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s

define dso_local void @f() {
entry:
  %a.addr = alloca <2 x x86_amx>, align 4
  ret void
}

; CHECK: invalid vector element type


define void @f(x86_amx %A, x86_amx %B) {
entry:
  alloca x86_amx
; CHECK: invalid type for alloca
  ret void
}

[fhahn]

llvm/test/Verifier/x86_amx1.ll

3

I'm not sure if there's a problem with the test matrix verifier tests or another difference, but I *think* the matrix verifier tests check multiple errors in a single file for example: https://github.com/llvm/llvm-project/blob/main/llvm/test/Verifier/matrix-intrinsics.ll

[LuoYuanke]

llvm/test/Verifier/x86_amx1.ll

3

I tested llvm/test/Verifier/matrix-intrinsics.ll. Yes this case is able to be grouped in a single file, but it is not the same to x86_amx test case.

[llvm]$ cat test/Verifier/x86_amx.ll
; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s

define dso_local void @f() {
entry:
  %a.addr = alloca <2 x x86_amx>, align 4
  ret void
}

; CHECK: invalid vector element type

define void @f(x86_amx %A, x86_amx %B) {
entry:
  alloca x86_amx
; CHECK: invalid type for alloca
  ret void
}
[llvm]$ llvm-as test/Verifier/x86_amx.ll
llvm-as: test/Verifier/x86_amx.ll:5:25: error: invalid vector element type
  %a.addr = alloca <2 x x86_amx>, align 4
                        ^
[llvm]$

[LuoYuanke]

llvm/test/Verifier/x86_amx1.ll

3

It seems the token test cases are also not able to be grouped.
test/Verifier/token1.ll
test/Verifier/token2.ll
test/Verifier/token3.ll

[craig.topper]

llvm/lib/IR/Verifier.cpp

2462

Should this be Function rather than Functions? Same for the message it was copied from

[LuoYuanke]

Address Craig's comments.

[LuoYuanke]

llvm/lib/IR/Verifier.cpp

2462

Thanks! It's very careful of you. :)

llvm/lib/IR/Type.cpp

if ((isX86_AMXTy() && isa<FixedVectorType>(Ty)) &&
       Ty->getPrimitiveSizeInBits().getFixedSize() == 8192)
     return true;
 
   // At this point we have only various mismatches of the first class types
   // remaining and ptr->ptr. Just select the lossless conversions. Everything
   // else is not lossless. Conservatively assume we can't losslessly convert
   // between pointers with different address spaces.
   if (auto *PTy = dyn_cast<PointerType>(this)) {
-    if (auto *OtherPTy = dyn_cast<PointerType>(Ty)) {
-      // Don't bitcast "load <256 x i32>, <256 x i32>*" to
-      // "load x86_amx, x86_amx*", because we don't have a corresponding
-      // instruction to load x86_amx. Doing the transform causes trouble
-      // to lower "load x86_amx" instruction in backend.
-      if (OtherPTy->getElementType()->isX86_AMXTy())
-        return false;
+    if (auto *OtherPTy = dyn_cast<PointerType>(Ty))
       return PTy->getAddressSpace() == OtherPTy->getAddressSpace();
-    }
     return false;
   }
   return false;  // Other types have no identity values
 }
 
 bool Type::isEmptyTy() const {
   if (auto *ATy = dyn_cast<ArrayType>(this)) {
     unsigned NumElements = ATy->getNumElements();
ArrayType *ArrayType::get(Type *ElementType, uint64_t NumElements) {
   if (!Entry)
     Entry = new (pImpl->Alloc) ArrayType(ElementType, NumElements);
   return Entry;
 }
 
 bool ArrayType::isValidElementType(Type *ElemTy) {
   return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
          !ElemTy->isMetadataTy() && !ElemTy->isFunctionTy() &&
-         !ElemTy->isTokenTy() && !isa<ScalableVectorType>(ElemTy);
+         !ElemTy->isTokenTy() && !ElemTy->isX86_AMXTy() &&
+         !isa<ScalableVectorType>(ElemTy);
 }
 
 //===----------------------------------------------------------------------===//
 //                          VectorType Implementation
 //===----------------------------------------------------------------------===//
 
 VectorType::VectorType(Type *ElType, unsigned EQ, Type::TypeID TID)
     : Type(ElType->getContext(), TID), ContainedType(ElType),

 }
 
 PointerType *Type::getPointerTo(unsigned addrs) const {
   return PointerType::get(const_cast<Type*>(this), addrs);
 }
 
 bool PointerType::isValidElementType(Type *ElemTy) {
   return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
-         !ElemTy->isMetadataTy() && !ElemTy->isTokenTy();
+         !ElemTy->isMetadataTy() && !ElemTy->isTokenTy() &&
+         !ElemTy->isX86_AMXTy();
 }
 
 bool PointerType::isLoadableOrStorableType(Type *ElemTy) {
   return isValidElementType(ElemTy) && !ElemTy->isFunctionTy();
 }

llvm/lib/IR/Verifier.cpp

Assert(Arg.getType() == FT->getParamType(i),
            FT->getParamType(i));
     Assert(Arg.getType()->isFirstClassType(),
            "Function arguments must have first-class types!", &Arg);
     if (!isLLVMdotName) {
       Assert(!Arg.getType()->isMetadataTy(),
              "Function takes metadata but isn't an intrinsic", &Arg, &F);
       Assert(!Arg.getType()->isTokenTy(),
              "Function takes token but isn't an intrinsic", &Arg, &F);
+      Assert(!Arg.getType()->isX86_AMXTy(),
+             "Function takes x86_amx but isn't an intrinsic", &Arg, &F);
     }
 
     // Check that swifterror argument is only used by loads and stores.
     if (Attrs.hasParamAttribute(i, Attribute::SwiftError)) {
       verifySwiftErrorValue(&Arg);
     }
     ++i;
   }
 
-  if (!isLLVMdotName)
+  if (!isLLVMdotName) {
     Assert(!F.getReturnType()->isTokenTy(),
            "Functions returns a token but isn't an intrinsic", &F);
+    Assert(!F.getReturnType()->isX86_AMXTy(),
+           "Functions returns a x86_amx but isn't an intrinsic", &F);
+  }
 
   // Get the function metadata attachments.
   SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
   F.getAllMetadata(MDs);
   assert(F.hasMetadata() != MDs.empty() && "Bit out-of-sync");
   verifyFunctionMetadata(MDs);
 
   // Check validity of the personality function
for (Type *ParamTy : FTy->params()) {
       Assert(!ParamTy->isMetadataTy(),
              "Function has metadata parameter but isn't an intrinsic", Call);
       Assert(!ParamTy->isTokenTy(),
              "Function has token parameter but isn't an intrinsic", Call);
     }
   }
 
   // Verify that indirect calls don't return tokens.
-  if (!Call.getCalledFunction())
+  if (!Call.getCalledFunction()) {
     Assert(!FTy->getReturnType()->isTokenTy(),
            "Return type cannot be token for indirect call!");
+    Assert(!FTy->getReturnType()->isX86_AMXTy(),
+           "Return type cannot be x86_amx for indirect call!");
+  }
 
   if (Function *F = Call.getCalledFunction())
     if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID())
       visitIntrinsicCall(ID, Call);
 
   // Verify that a callsite has at most one "deopt", at most one "funclet", at
   // most one "gc-transition", at most one "cfguardtarget",
   // and at most one "preallocated" operand bundle.
void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
   Assert(ExpectedName == IF->getName(),
          "Intrinsic name not mangled correctly for type arguments! "
          "Should be: " +
              ExpectedName,
          IF);
 
   // If the intrinsic takes MDNode arguments, verify that they are either global
   // or are local to *this* function.
-  for (Value *V : Call.args())
+  for (Value *V : Call.args()) {
     if (auto *MD = dyn_cast<MetadataAsValue>(V))
       visitMetadataAsValue(*MD, Call.getCaller());
+    if (auto *Const = dyn_cast<Constant>(V))
+      Assert(!Const->getType()->isX86_AMXTy(),
+             "const x86_amx is not allowed in argument!");
+  }
 
   switch (ID) {
   default:
     break;
   case Intrinsic::assume: {
     for (auto &Elem : Call.bundle_op_infos()) {
       Assert(Elem.Tag->getKey() == "ignore" ||
                  Attribute::isExistingAttribute(Elem.Tag->getKey()),

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

for (Value *IncValue : OldPN->incoming_values()) {
       if (auto *LI = dyn_cast<LoadInst>(IncValue)) {
         // If there is a sequence of one or more load instructions, each loaded
         // value is used as address of later load instruction, bitcast is
         // necessary to change the value type, don't optimize it. For
         // simplicity we give up if the load address comes from another load.
         Value *Addr = LI->getOperand(0);
         if (Addr == &CI || isa<LoadInst>(Addr))
           return nullptr;
-        // If there is any loss for the pointer bitcast, abandon.
-        auto *DestPtrTy = DestTy->getPointerTo(LI->getPointerAddressSpace());
-        auto *SrcPtrTy = Addr->getType();
-        if (!SrcPtrTy->canLosslesslyBitCastTo(DestPtrTy))
+        // Don't tranform "load <256 x i32>, <256 x i32>*" to
+        // "load x86_amx, x86_amx*", because x86_amx* is invalid.
+        // TODO: Remove this check when bitcast between vector and x86_amx
+        // is replaced with a specific intrinsic.
+        if (DestTy->isX86_AMXTy())
           return nullptr;
         if (LI->hasOneUse() && LI->isSimple())
           continue;
         // If a LoadInst has more than one use, changing the type of loaded
         // value may create another bitcast.
         return nullptr;
       }
 

llvm/test/Verifier/x86_amx1.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+@GV = dso_local global [10 x x86_amx] zeroinitializer, align 16
+; CHECK: invalid array element type

llvm/test/Verifier/x86_amx2.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define dso_local void @f() {
+entry:
+  %a.addr = alloca <2 x x86_amx>, align 4
+  ret void
+}
+
+; CHECK: invalid vector element type

llvm/test/Verifier/x86_amx3.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @f(x86_amx %A, x86_amx %B) {
+entry:
+  alloca x86_amx
+; CHECK: invalid type for alloca
+  ret void
+}

llvm/test/Verifier/x86_amx4.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+@GV = external global x86_amx
+; CHECK: invalid type for global variable

llvm/test/Verifier/x86_amx5.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @f(x86_amx %A) {
+entry:
+  ret void
+}
+; CHECK: Function takes x86_amx but isn't an intrinsic

llvm/test/Verifier/x86_amx6.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define x86_amx @f() {
+entry:
+  ret x86_amx undef
+}
+; CHECK: Functions returns a x86_amx but isn't an intrinsic

llvm/test/Verifier/x86_amx7.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @f() {
+entry:
+  call x86_amx () undef ()
+  ret void
+}
+; CHECK: Return type cannot be x86_amx for indirect call!

llvm/test/Verifier/x86_amx8.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+@GV = dso_local global x86_amx* null
+; CHECK: pointer to this type is invalid

llvm/test/Verifier/x86_amx9.ll

+; RUN: not llc %s -o /dev/null 2>&1 | FileCheck %s
+
+@buf = dso_local global [1024 x i8] zeroinitializer, align 16
+
+define dso_local void @test_tile_init(i16 signext %row, i16 signext %col) {
+entry:
+  tail call void @llvm.x86.tilestored64.internal(i16 %row, i16 %col, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @buf, i64 0, i64 0), i64 64, x86_amx bitcast (<256 x i32> <i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0> to x86_amx))
+  ret void
+}
+; CHECK: const x86_amx is not allowed in argument!
+
+declare void @llvm.x86.tilestored64.internal(i16, i16, i8*, i64, x86_amx)