[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I only took a quick pass through this so far.  What happens if a bitcast between x86amx and v256i32(or any other 1024-bit vector type) exists in the IR but isn't next to a load/store?



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:5334
   // to x86 amx tile on amx intrinsics.
-  if (MemVT == MVT::v256i32)
-    return false;
+  // if (MemVT == MVT::v256i32)
+  //   return false;
----------------
Should this just be deleted?


================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:63
+        LoadInst *LD = dyn_cast<LoadInst>(Src);
+        assert(LD && "Expected bitcast to x86amx from load");
+        assert(LD->hasOneUser() &&
----------------
Don't use an assert to check the result of a dyn_cast. If it shouldn't fail just use cast<LoadInst> which will assert internally.


================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:71
+        auto *AMXIntrinsic =
+            dyn_cast<IntrinsicInst>(Inst.use_begin()->getUser());
+        auto *Row = AMXIntrinsic->getOperand(0);
----------------
Unchecked dyn_cast


================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:94
+        for (auto UI = Inst.use_begin(), UE = Inst.use_end(); UI != UE;) {
+          StoreInst *ST = dyn_cast<StoreInst>((UI++)->getUser());
+          assert(ST && "Expected bitcast to x86amx for store");
----------------
Use cast.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91927/new/

https://reviews.llvm.org/D91927