[llvm] f721212 - [SVE] Don't reorder subvector/binop sequences when the resulting binop is not legal.
Paul Walker via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 2 03:04:40 PDT 2020
Author: Paul Walker
Date: 2020-09-02T11:01:33+01:00
New Revision: f72121254da48bf668c35918b53c96cf8c568342
URL: https://github.com/llvm/llvm-project/commit/f72121254da48bf668c35918b53c96cf8c568342
DIFF: https://github.com/llvm/llvm-project/commit/f72121254da48bf668c35918b53c96cf8c568342.diff
LOG: [SVE] Don't reorder subvector/binop sequences when the resulting binop is not legal.
When lowering fixed length vector operations for SVE the subvector
operations are used extensively to marshall data between scalable
and fixed-length vectors. This means that sequences like:
extract_subvec(binop(insert_subvec(a), insert_subvec(b)))
are very common. DAGCombine only checks if the resulting binop is
legal or can be custom lowered when undoing such sequences. When
it's custom lowering that is introducing them the result is an
infinite legalise->combine->legalise loop.
This patch extends the isOperationLegalOr... functions to include
a "LegalOnly" parameter to restrict the check to legal operations
only. Although isOperationLegal could be used it's common for
the affected code paths to be visited pre and post legalisation,
so the extra parameter keeps the code tidy.
Differential Revision: https://reviews.llvm.org/D86450
Added:
Modified:
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index ec6e038d59f6..b8a7a3437915 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -1093,8 +1093,13 @@ class TargetLoweringBase {
/// Return true if the specified operation is legal on this target or can be
/// made legal with custom lowering. This is used to help guide high-level
- /// lowering decisions.
- bool isOperationLegalOrCustom(unsigned Op, EVT VT) const {
+ /// lowering decisions. LegalOnly is an optional convenience for code paths
+ /// traversed pre and post legalisation.
+ bool isOperationLegalOrCustom(unsigned Op, EVT VT,
+ bool LegalOnly = false) const {
+ if (LegalOnly)
+ return isOperationLegal(Op, VT);
+
return (VT == MVT::Other || isTypeLegal(VT)) &&
(getOperationAction(Op, VT) == Legal ||
getOperationAction(Op, VT) == Custom);
@@ -1102,8 +1107,13 @@ class TargetLoweringBase {
/// Return true if the specified operation is legal on this target or can be
/// made legal using promotion. This is used to help guide high-level lowering
- /// decisions.
- bool isOperationLegalOrPromote(unsigned Op, EVT VT) const {
+ /// decisions. LegalOnly is an optional convenience for code paths traversed
+ /// pre and post legalisation.
+ bool isOperationLegalOrPromote(unsigned Op, EVT VT,
+ bool LegalOnly = false) const {
+ if (LegalOnly)
+ return isOperationLegal(Op, VT);
+
return (VT == MVT::Other || isTypeLegal(VT)) &&
(getOperationAction(Op, VT) == Legal ||
getOperationAction(Op, VT) == Promote);
@@ -1111,8 +1121,13 @@ class TargetLoweringBase {
/// Return true if the specified operation is legal on this target or can be
/// made legal with custom lowering or using promotion. This is used to help
- /// guide high-level lowering decisions.
- bool isOperationLegalOrCustomOrPromote(unsigned Op, EVT VT) const {
+ /// guide high-level lowering decisions. LegalOnly is an optional convenience
+ /// for code paths traversed pre and post legalisation.
+ bool isOperationLegalOrCustomOrPromote(unsigned Op, EVT VT,
+ bool LegalOnly = false) const {
+ if (LegalOnly)
+ return isOperationLegal(Op, VT);
+
return (VT == MVT::Other || isTypeLegal(VT)) &&
(getOperationAction(Op, VT) == Legal ||
getOperationAction(Op, VT) == Custom ||
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 60fba1cc4e20..286d54386357 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -758,9 +758,7 @@ namespace {
/// is legal or custom before legalizing operations, and whether is
/// legal (but not custom) after legalization.
bool hasOperation(unsigned Opcode, EVT VT) {
- if (LegalOperations)
- return TLI.isOperationLegal(Opcode, VT);
- return TLI.isOperationLegalOrCustom(Opcode, VT);
+ return TLI.isOperationLegalOrCustom(Opcode, VT, LegalOperations);
}
public:
@@ -19193,7 +19191,8 @@ static SDValue getSubVectorSrc(SDValue V, SDValue Index, EVT SubVT) {
}
static SDValue narrowInsertExtractVectorBinOp(SDNode *Extract,
- SelectionDAG &DAG) {
+ SelectionDAG &DAG,
+ bool LegalOperations) {
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
SDValue BinOp = Extract->getOperand(0);
unsigned BinOpcode = BinOp.getOpcode();
@@ -19207,7 +19206,7 @@ static SDValue narrowInsertExtractVectorBinOp(SDNode *Extract,
SDValue Index = Extract->getOperand(1);
EVT SubVT = Extract->getValueType(0);
- if (!TLI.isOperationLegalOrCustom(BinOpcode, SubVT))
+ if (!TLI.isOperationLegalOrCustom(BinOpcode, SubVT, LegalOperations))
return SDValue();
SDValue Sub0 = getSubVectorSrc(Bop0, Index, SubVT);
@@ -19228,11 +19227,12 @@ static SDValue narrowInsertExtractVectorBinOp(SDNode *Extract,
/// If we are extracting a subvector produced by a wide binary operator try
/// to use a narrow binary operator and/or avoid concatenation and extraction.
-static SDValue narrowExtractedVectorBinOp(SDNode *Extract, SelectionDAG &DAG) {
+static SDValue narrowExtractedVectorBinOp(SDNode *Extract, SelectionDAG &DAG,
+ bool LegalOperations) {
// TODO: Refactor with the caller (visitEXTRACT_SUBVECTOR), so we can share
// some of these bailouts with other transforms.
- if (SDValue V = narrowInsertExtractVectorBinOp(Extract, DAG))
+ if (SDValue V = narrowInsertExtractVectorBinOp(Extract, DAG, LegalOperations))
return V;
// The extract index must be a constant, so we can map it to a concat operand.
@@ -19581,7 +19581,7 @@ SDValue DAGCombiner::visitEXTRACT_SUBVECTOR(SDNode *N) {
N->getOperand(1));
}
- if (SDValue NarrowBOp = narrowExtractedVectorBinOp(N, DAG))
+ if (SDValue NarrowBOp = narrowExtractedVectorBinOp(N, DAG, LegalOperations))
return NarrowBOp;
if (SimplifyDemandedVectorElts(SDValue(N, 0)))
@@ -20945,7 +20945,8 @@ SDValue DAGCombiner::SimplifyVBinOp(SDNode *N) {
SDValue Z = LHS.getOperand(2);
EVT NarrowVT = X.getValueType();
if (NarrowVT == Y.getValueType() &&
- TLI.isOperationLegalOrCustomOrPromote(Opcode, NarrowVT)) {
+ TLI.isOperationLegalOrCustomOrPromote(Opcode, NarrowVT,
+ LegalOperations)) {
// (binop undef, undef) may not return undef, so compute that result.
SDLoc DL(N);
SDValue VecC =
diff --git a/llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll b/llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll
index 857a2f6ff204..348c785ebae2 100644
--- a/llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll
+++ b/llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll
@@ -88,7 +88,6 @@ bb1:
ret void
}
-;
define <8 x i1> @no_warn_dropped_scalable(<8 x i32>* %in) #0 {
; CHECK-LABEL: no_warn_dropped_scalable:
; CHECK: ptrue [[PG:p[0-9]+]].s, vl8
@@ -103,4 +102,33 @@ bb1:
ret <8 x i1> %cond
}
+; binop(insert_subvec(a), insert_subvec(b)) -> insert_subvec(binop(a,b)) like
+; combines remove redundant subvector operations. This test ensures it's not
+; performed when the input idiom is the result of operation legalisation. When
+; not prevented the test triggers infinite combine->legalise->combine->...
+define void @no_subvector_binop_hang(<8 x i32>* %in, <8 x i32>* %out, i1 %cond) #0 {
+; CHECK-LABEL: no_subvector_binop_hang:
+; CHECK: // %bb.0:
+; CHECK-NEXT: ptrue [[PG:p[0-9]+]].s, vl8
+; CHECK-NEXT: ld1w { [[A:z[0-9]+]].s }, [[PG]]/z, [x0]
+; CHECK-NEXT: ld1w { [[B:z[0-9]+]].s }, [[PG]]/z, [x1]
+; CHECK-NEXT: tbz w2, #0, .LBB5_2
+; CHECK-NEXT: // %bb.1: // %bb.1
+; CHECK-NEXT: orr [[OR:z[0-9]+]].d, [[A]].d, [[B]].d
+; CHECK-NEXT: st1w { [[OR]].s }, [[PG]], [x1]
+; CHECK-NEXT: .LBB5_2: // %bb.2
+; CHECK-NEXT: ret
+ %a = load <8 x i32>, <8 x i32>* %in
+ %b = load <8 x i32>, <8 x i32>* %out
+ br i1 %cond, label %bb.1, label %bb.2
+
+bb.1:
+ %or = or <8 x i32> %a, %b
+ store <8 x i32> %or, <8 x i32>* %out
+ br label %bb.2
+
+bb.2:
+ ret void
+}
+
attributes #0 = { "target-features"="+sve" }
More information about the llvm-commits
mailing list