efriedma added inline comments.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4337
+ // Collect the (promoted) operands
+ SDValue Ops[] = { GetPromotedInteger(InOp0), BaseIdx };
+
----------------
In general, there are four possibilities for legalization of the operand vector:
1. Legal. In this case, I guess the operation you want is essentially
ANY_EXTEND_VECTOR_INREG. You're currently handling this in target-specific
code; I guess that's okay for now.
2. Promote. In this case, you can promote both the operand and the result, what
you're doing here.
3. Widen. In this case, you just widen the operand.
4. Split. In this case, you pick the correct half (or construct it with a
shuffle), and extract from that.
-----
Currently, you're assuming the the result of GetPromotedInteger() has an
element type narrower or equal to the element type of NOutVT. I'm not sure
that's true on all targets, but I guess it's true for SVE. Better to
explicitly check, I think.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4344
+ }
+ }
+
----------------
Please put a report_fatal_error here for scalable vectors, so we don't end up
with some obscure invalid BUILD_VECTOR error.
Alternatively, I guess you could call ExpandExtractFromVectorThroughStack().
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:906
+ if (VT.getVectorElementType() != MVT::i1)
+ setOperationAction(ISD::EXTRACT_SUBVECTOR, VT, Custom);
}
----------------
Can you restrict this to specifically the types you're interested in handling?
It looks like you only implemented handling for the following types: nxv8i8,
nxv4i16, nxv2i32.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8559
+ if (Op.getValueType().isScalableVector())
+ return SDValue();
+
----------------
I think you can just assert the type isn't scalable? It shouldn't be possible
to get here. You're only marking EXTRACT_SUBVECTOR Custom for illegal types.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676
+ ConstantSDNode *C = dyn_cast<ConstantSDNode>(Dup->getOperand(0));
+ uint64_t ExtVal = C->getZExtValue();
+
----------------
Do you need to truncate ExtVal somewhere, so you don't end up with a DUP with
an over-wide constant?
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13300
+ // is another unpack:
+ // 4i32 sign_extend_inreg (4i32 uunpklo(8i16 uunpklo (16i8 opnd)), from
4i8)
+ // ->
----------------
It seems a little fragile to assume the inner VT of the SIGN_EXTEND_INREG is
exactly the type you're expecting here. Probably worth at least adding an
assertion to encode the assumptions you're making.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13816
+ if (!isTypeLegal(InVT)) {
+ // Bubble truncates to illegal types to the surface.
+ if (In->getOpcode() == ISD::TRUNCATE) {
----------------
"Bubble truncates to illegal types to the surface" is an optimization?
================
Comment at: llvm/test/CodeGen/AArch64/sve-sext-zext.ll:195
+; CHECK-DAG: sunpklo z2.h, z0.b
+; CHECK-DAG: sunpkhi z1.h, z0.b
+; CHECK-DAG: mov z0.d, z2.d
----------------
Please just generate the checks with update_llc_test_checks.py.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
https://reviews.llvm.org/D79587
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
