replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[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

llvm-svn: 355865
This commit is contained in:
Jessica Paquette 2019-03-11 20:51:17 +00:00
parent 2c6c84e52c
commit 42d16501e6
6 changed files with 93 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
View File

@ -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,

38
llvm/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
View File

@ -59,10 +59,30 @@ raw_ostream &LegalityQuery::print(raw_ostream &OS) const {
}
#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::apply(const LegalityQuery &Query) const {
<< 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 MachineInstr &MI,
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::machineFunctionIsIllegal(const MachineFunction &MF) {
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;

24
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
View File

@ -64,6 +64,11 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST) {
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::legalizeCustom(MachineInstr &MI,
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 {

3
llvm/lib/Target/AArch64/AArch64LegalizerInfo.h
View File

@ -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

32
llvm/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir Normal file
View File

@ -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
...

2
llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
View File

@ -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