This revision was automatically updated to reflect the committed changes.
Closed by commit rG89fc0166f532: [CodeGen][SVE] Legalisation of extends with
scalable types (authored by kmclaughlin).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
https://reviews.llvm.org/D79587
___
cfe-commits mailing list
kmclaughlin updated this revision to Diff 268246.
kmclaughlin added a comment.
- Use APInt::trunc to truncate the constant in performSVEAndCombine
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
https://reviews.llvm.org/D79587
Files:
llvm/include/llvm/CodeGen/ValueTypes.h
efriedma added inline comments.
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10695
+// Truncate to prevent a DUP with an over wide constant
+SDValue Trunc = DAG.getNode(ISD::TRUNCATE, DL, EltTy, Dup->getOperand(0));
+
It's not legal to
kmclaughlin updated this revision to Diff 267919.
kmclaughlin marked 2 inline comments as done.
kmclaughlin added a comment.
- Added a truncate of ExtVal in performSVEAndCombine
- Changed the assert added to performSignExtendInRegCombine in the previous
revision
CHANGES SINCE LAST ACTION
kmclaughlin marked an inline comment as not done.
kmclaughlin added inline comments.
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676
+ConstantSDNode *C = dyn_cast(Dup->getOperand(0));
+uint64_t ExtVal = C->getZExtValue();
+
efriedma
efriedma added inline comments.
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676
+ConstantSDNode *C = dyn_cast(Dup->getOperand(0));
+uint64_t ExtVal = C->getZExtValue();
+
kmclaughlin wrote:
> efriedma wrote:
> > Do you need to truncate
kmclaughlin marked 2 inline comments as done.
kmclaughlin added a comment.
Thanks for taking another look at this, @efriedma!
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676
+ConstantSDNode *C = dyn_cast(Dup->getOperand(0));
+uint64_t ExtVal =
kmclaughlin updated this revision to Diff 267695.
kmclaughlin marked 7 inline comments as done.
kmclaughlin added a comment.
- Restricted the illegal types which should be lowered for EXTRACT_SUBVECTOR to
those handled in this patch (nxv8i8, nxv4i16 & nxv2i32)
- Removed unnecessary changes in
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
kmclaughlin updated this revision to Diff 267280.
kmclaughlin added a comment.
- Replaced uses of getVectorNumElements() with getVectorElementCount()
- Moved the new tests into the existing sve-sext-zext.ll file
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
david-arm added a comment.
Sorry I forgot to mention I think we have an existing test file for extends:
llvm/test/CodeGen/AArch64/sve-sext-zext.ll
It might be worth adding these cases to that file instead of creating a new one?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
david-arm added a comment.
Hi Kerry, just a couple of comments about the use of getVectorNumElements() -
we're trying to remove calls to this function so it would be good if you could
use getVectorElementCount() instead. Thanks!
Comment at:
kmclaughlin updated this revision to Diff 267183.
kmclaughlin edited the summary of this revision.
kmclaughlin added a comment.
- Removed ReplaceExtensionResults and instead try to use extract_subvector as
much as possible to legalise the result
- Added ReplaceExtractSubVectorResults, which
efriedma added a comment.
For sunpckhi... no, not really. You'd need to either add a new opcode, or add
a new shuffle operation of some sort.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79587/new/
https://reviews.llvm.org/D79587
david-arm added a comment.
Hi @efriedma, is there a target-independent equivalent of SUNPKHI? From a quick
glance at the codebase where X86 uses ISD::SIGN_EXTEND_VECTOR_INREG it seems
vector shuffles are still required for the Hi part, which is fine for fixed
length vectors I guess.
david-arm added a comment.
Hi Kerry, I think we do already have some suitable test files where perhaps you
could add your tests instead of creating new files? For example, there are:
CodeGen/AArch64/sve-int-arith.ll (perhaps integer divides and shifts could live
there?)
efriedma added a comment.
Can you explain why target-independent legalization isn't suitable here? I'd
prefer not to have target-specific type legalization for every operation on
vscale'ed types. (The target-independent name of AArch64ISD::SUNPKLO is
ISD::SIGN_EXTEND_VECTOR_INREG.)
kmclaughlin created this revision.
kmclaughlin added reviewers: sdesmalen, efriedma, david-arm.
Herald added subscribers: psnobl, rkruppe, hiraditya, tschuett.
Herald added a project: LLVM.
This patch adds legalisation of extensions where the operand
of the extend is a legal scalable type but the
19 matches
Mail list logo
