[PATCH] D94097: [AArch64] Legalize MVT::i64x8 in DAG isel lowering.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:347
+    if (OpInfo.ConstraintVT == MVT::i64x8)
+      return false;
+
----------------
efriedma wrote:
> It's a little unfortunate we're sticking this in target-independent code, but not sure there's a better place.
Not, I am afraid.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:251
+    addRegisterClass(MVT::i64x8, &AArch64::GPR64x8ClassRegClass);
+    setOperationAction(ISD::BITCAST, MVT::i64x8, Legal);
+    setOperationAction(ISD::UNDEF, MVT::i64x8, Legal);
----------------
efriedma wrote:
> Not sure why you're marking BITCAST Legal; there isn't any legal type to bitcast from.
I think that's a leftover from previous efforts, so as the UNDEF just below. I'll remove them.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:254
+    setOperationAction(ISD::LOAD, MVT::i64x8, Custom);
+    setOperationAction(ISD::STORE, MVT::i64x8, Custom);
+  }
----------------
efriedma wrote:
> I guess LOAD/STORE come out of type legalization?
These are the custom lowering operations for the `load/store i512` instruction that clang inserts pre/post inline assembly for accessing the asm operands (see lines 4569 and 4595).


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8254
+
+  return TargetLowering::getValueType(DL, Ty, AllowUnknown);
+}
----------------
This should be TargetLowering::getAsmOperandValueType. I'll fix it.


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

https://reviews.llvm.org/D94097