Index: llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h =================================================================== --- llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h @@ -1055,6 +1055,8 @@ 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, Index: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp =================================================================== --- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp +++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp @@ -59,10 +59,30 @@ } #ifndef NDEBUG +// Make sure the rule won't (trivially) loop forever. +static bool hasNoSimpleLoops(const LegalizeRule &Rule, const LegalityQuery &Q, + const std::pair &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 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 @@ << 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 @@ 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 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; Index: llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h =================================================================== --- llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h +++ llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.h @@ -34,6 +34,9 @@ private: bool legalizeVaArg(MachineInstr &MI, MachineRegisterInfo &MRI, MachineIRBuilder &MIRBuilder) const; + + bool legalizeIntrinsic(MachineInstr &MI, MachineRegisterInfo &MRI, + MachineIRBuilder &MIRBuilder) const; }; } // End llvm namespace. #endif Index: llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp =================================================================== --- llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp +++ llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp @@ -64,6 +64,11 @@ 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 @@ 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 { Index: llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir =================================================================== --- llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir +++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/fallback-ambiguous-addp-intrinsic.mir @@ -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 + +... Index: llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir =================================================================== --- llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir +++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir @@ -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