[llvm] r355865 - [GlobalISel][AArch64] Always fall back on aarch64.neon.addp.*
Jessica Paquette via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 11 13:51:17 PDT 2019
Author: paquette
Date: Mon Mar 11 13:51:17 2019
New Revision: 355865
URL: http://llvm.org/viewvc/llvm-project?rev=355865&view=rev
Log:
[GlobalISel][AArch64] Always fall back on aarch64.neon.addp.*
Overloaded intrinsics aren't necessarily safe for instruction selection. One
such intrinsic is aarch64.neon.addp.*.
This is a temporary workaround to ensure that we always fall back on that
intrinsic. Eventually this will be replaced with a proper solution.
https://bugs.llvm.org/show_bug.cgi?id=40968
Differential Revision: https://reviews.llvm.org/D59062
Added:
llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir
Modified:
llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp
llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h
llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h?rev=355865&r1=355864&r2=355865&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h Mon Mar 11 13:51:17 2019
@@ -1055,6 +1055,8 @@ public:
const MachineRegisterInfo &MRI) const;
bool isLegal(const MachineInstr &MI, const MachineRegisterInfo &MRI) const;
+ bool isLegalOrCustom(const MachineInstr &MI,
+ const MachineRegisterInfo &MRI) const;
virtual bool legalizeCustom(MachineInstr &MI, MachineRegisterInfo &MRI,
MachineIRBuilder &MIRBuilder,
Modified: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp?rev=355865&r1=355864&r2=355865&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp Mon Mar 11 13:51:17 2019
@@ -59,10 +59,30 @@ raw_ostream &LegalityQuery::print(raw_os
}
#ifndef NDEBUG
+// Make sure the rule won't (trivially) loop forever.
+static bool hasNoSimpleLoops(const LegalizeRule &Rule, const LegalityQuery &Q,
+ const std::pair<unsigned, LLT> &Mutation) {
+ switch (Rule.getAction()) {
+ case Custom:
+ case Lower:
+ case MoreElements:
+ case FewerElements:
+ break;
+ default:
+ return Q.Types[Mutation.first] != Mutation.second;
+ }
+ return true;
+}
+
// Make sure the returned mutation makes sense for the match type.
static bool mutationIsSane(const LegalizeRule &Rule,
const LegalityQuery &Q,
std::pair<unsigned, LLT> Mutation) {
+ // If the user wants a custom mutation, then we can't really say much about
+ // it. Return true, and trust that they're doing the right thing.
+ if (Rule.getAction() == Custom)
+ return true;
+
const unsigned TypeIdx = Mutation.first;
const LLT OldTy = Q.Types[TypeIdx];
const LLT NewTy = Mutation.second;
@@ -133,12 +153,7 @@ LegalizeActionStep LegalizeRuleSet::appl
<< Mutation.first << ", " << Mutation.second << "\n");
assert(mutationIsSane(Rule, Query, Mutation) &&
"legality mutation invalid for match");
-
- assert((Query.Types[Mutation.first] != Mutation.second ||
- Rule.getAction() == Lower ||
- Rule.getAction() == MoreElements ||
- Rule.getAction() == FewerElements) &&
- "Simple loop detected");
+ assert(hasNoSimpleLoops(Rule, Query, Mutation) && "Simple loop detected");
return {Rule.getAction(), Mutation.first, Mutation.second};
} else
LLVM_DEBUG(dbgs() << ".. no match\n");
@@ -435,6 +450,14 @@ bool LegalizerInfo::isLegal(const Machin
return getAction(MI, MRI).Action == Legal;
}
+bool LegalizerInfo::isLegalOrCustom(const MachineInstr &MI,
+ const MachineRegisterInfo &MRI) const {
+ auto Action = getAction(MI, MRI).Action;
+ // If the action is custom, it may not necessarily modify the instruction,
+ // so we have to assume it's legal.
+ return Action == Legal || Action == Custom;
+}
+
bool LegalizerInfo::legalizeCustom(MachineInstr &MI, MachineRegisterInfo &MRI,
MachineIRBuilder &MIRBuilder,
GISelChangeObserver &Observer) const {
@@ -644,7 +667,8 @@ const MachineInstr *llvm::machineFunctio
const MachineRegisterInfo &MRI = MF.getRegInfo();
for (const MachineBasicBlock &MBB : MF)
for (const MachineInstr &MI : MBB)
- if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI))
+ if (isPreISelGenericOpcode(MI.getOpcode()) &&
+ !MLI->isLegalOrCustom(MI, MRI))
return &MI;
}
return nullptr;
Modified: llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp?rev=355865&r1=355864&r2=355865&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp Mon Mar 11 13:51:17 2019
@@ -64,6 +64,11 @@ AArch64LegalizerInfo::AArch64LegalizerIn
return std::make_pair(0, EltTy);
});
+ // HACK: Check that the intrinsic isn't ambiguous.
+ // (See: https://bugs.llvm.org/show_bug.cgi?id=40968)
+ getActionDefinitionsBuilder(G_INTRINSIC)
+ .custom();
+
getActionDefinitionsBuilder(G_PHI)
.legalFor({p0, s16, s32, s64})
.clampScalar(0, s16, s64)
@@ -500,11 +505,30 @@ bool AArch64LegalizerInfo::legalizeCusto
return false;
case TargetOpcode::G_VAARG:
return legalizeVaArg(MI, MRI, MIRBuilder);
+ case TargetOpcode::G_INTRINSIC:
+ return legalizeIntrinsic(MI, MRI, MIRBuilder);
}
llvm_unreachable("expected switch to return");
}
+bool AArch64LegalizerInfo::legalizeIntrinsic(
+ MachineInstr &MI, MachineRegisterInfo &MRI,
+ MachineIRBuilder &MIRBuilder) const {
+ // HACK: Don't allow faddp/addp for now. We don't pass down the type info
+ // necessary to get this right today.
+ //
+ // It looks like addp/faddp is the only intrinsic that's impacted by this.
+ // All other intrinsics fully describe the required types in their names.
+ //
+ // (See: https://bugs.llvm.org/show_bug.cgi?id=40968)
+ const MachineOperand &IntrinOp = MI.getOperand(1);
+ if (IntrinOp.isIntrinsicID() &&
+ IntrinOp.getIntrinsicID() == Intrinsic::aarch64_neon_addp)
+ return false;
+ return true;
+}
+
bool AArch64LegalizerInfo::legalizeVaArg(MachineInstr &MI,
MachineRegisterInfo &MRI,
MachineIRBuilder &MIRBuilder) const {
Modified: llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h?rev=355865&r1=355864&r2=355865&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h Mon Mar 11 13:51:17 2019
@@ -34,6 +34,9 @@ public:
private:
bool legalizeVaArg(MachineInstr &MI, MachineRegisterInfo &MRI,
MachineIRBuilder &MIRBuilder) const;
+
+ bool legalizeIntrinsic(MachineInstr &MI, MachineRegisterInfo &MRI,
+ MachineIRBuilder &MIRBuilder) const;
};
} // End llvm namespace.
#endif
Added: llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir?rev=355865&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir (added)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir Mon Mar 11 13:51:17 2019
@@ -0,0 +1,32 @@
+# RUN: llc -mtriple aarch64-unknown-unknown -O0 -start-before=legalizer -pass-remarks-missed=gisel* %s -o - 2>&1 | FileCheck %s
+#
+# Check that we fall back on @llvm.aarch64.neon.addp and ensure that we get the
+# correct instruction.
+# https://bugs.llvm.org/show_bug.cgi?id=40968
+
+--- |
+ define <2 x float> @foo(<2 x float> %v1, <2 x float> %v2) {
+ entry:
+ %v3 = call <2 x float> @llvm.aarch64.neon.addp.v2f32(<2 x float> %v1, <2 x float> %v2)
+ ret <2 x float> %v3
+ }
+ declare <2 x float> @llvm.aarch64.neon.addp.v2f32(<2 x float>, <2 x float>)
+...
+---
+name: foo
+alignment: 2
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $d0, $d1
+ ; CHECK: remark:
+ ; CHECK-SAME: unable to legalize instruction: %2:_(<2 x s32>) = G_INTRINSIC intrinsic(@llvm.aarch64.neon.addp), %0:_(<2 x s32>), %1:_(<2 x s32>)
+ ; CHECK: faddp
+ ; CHECK-NOT: addp
+ %0:_(<2 x s32>) = COPY $d0
+ %1:_(<2 x s32>) = COPY $d1
+ %2:_(<2 x s32>) = G_INTRINSIC intrinsic(@llvm.aarch64.neon.addp), %0(<2 x s32>), %1(<2 x s32>)
+ $d0 = COPY %2(<2 x s32>)
+ RET_ReallyLR implicit $d0
+
+...
Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir?rev=355865&r1=355864&r2=355865&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir Mon Mar 11 13:51:17 2019
@@ -151,7 +151,7 @@
# DEBUG: .. the first uncovered type index: 1, OK
#
# DEBUG-NEXT: G_INTRINSIC (opcode {{[0-9]+}}): 0 type indices
-# DEBUG: .. type index coverage check SKIPPED: no rules defined
+# DEBUG: .. type index coverage check SKIPPED: user-defined predicate detected
#
# DEBUG-NEXT: G_INTRINSIC_W_SIDE_EFFECTS (opcode {{[0-9]+}}): 0 type indices
# DEBUG: .. type index coverage check SKIPPED: no rules defined
More information about the llvm-commits
mailing list