[PATCH] D31405: [CodeGen] Pass SDAG an ORE, and replace FastISel stats with remarks.
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 29 04:12:25 PDT 2017
Ahmed Bougacha via Phabricator <reviews at reviews.llvm.org> writes:
> ab created this revision.
> Herald added a subscriber: fhahn.
>
> In the long-term, we want to replace statistics with something
> finer-grained that lets us gather per-function data.
> Remarks are that replacement.
>
> Create an ORE instance in SelectionDAGISel, and pass it to
> SelectionDAG.
>
> SelectionDAG was used so that we can emit remarks from all
> SelectionDAG-related code, including TargetLowering and DAGCombiner.
> This isn't used in the current patch but Adam tells me he's interested
> for the fp-contract combines.
>
> Use the ORE instance to emit FastISel failures as remarks (instead of
> the mix of dbgs() dumps and statistics that we currently have).
>
> This does mean that the replacement for '-fast-isel-verbose' is now
> '-fast-isel-verbose -pass-remarks-missed=isel'.
>
> Having to specify both command line options is awkward, but is only
> necessary because we don't currently have a way to only emit a remark
> if it's enabled (and we can't afford to emit unnecessary remarks all
> the time).
What about Remark.isEnabled()? I think we should just do away with
-fast-isel-verbose completely and replace it with -pass-remarks-missed
here.
A couple of comments below.
> This also removes '-fast-isel-verbose2': there are no static statistics
> that we want to only enable in asserts builds, so we can always use
> '-fast-isel-verbose' regardless of the build type.
...
> ab updated this revision to Diff 93175.
> ab added a comment.
>
> - Tweak parameter names
> - Add a getORE method
>
>
> https://reviews.llvm.org/D31405
>
> Files:
> include/llvm/CodeGen/SelectionDAG.h
> include/llvm/CodeGen/SelectionDAGISel.h
> lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> test/CodeGen/AArch64/fast-isel-tail-call.ll
> test/CodeGen/Mips/Fast-ISel/check-disabled-mcpus.ll
> test/CodeGen/Mips/Fast-ISel/fastcc-miss.ll
>
> Index: test/CodeGen/Mips/Fast-ISel/fastcc-miss.ll
> ===================================================================
> --- test/CodeGen/Mips/Fast-ISel/fastcc-miss.ll
> +++ test/CodeGen/Mips/Fast-ISel/fastcc-miss.ll
> @@ -1,5 +1,5 @@
> ; RUN: llc < %s -march=mipsel -mcpu=mips32r2 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose 2>&1 | FileCheck %s
>
> ; CHECK: FastISel missed call:
> ; CHECK-SAME: %call = call fastcc i32 @foo(i32 signext %a, i32 signext %b)
> Index: test/CodeGen/Mips/Fast-ISel/check-disabled-mcpus.ll
> ===================================================================
> --- test/CodeGen/Mips/Fast-ISel/check-disabled-mcpus.ll
> +++ test/CodeGen/Mips/Fast-ISel/check-disabled-mcpus.ll
> @@ -1,40 +1,40 @@
> ; Targets where we should not enable FastISel.
> ; RUN: llc -march=mips -mcpu=mips2 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips3 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips4 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
>
> ; RUN: llc -march=mips -mcpu=mips32r6 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
>
> ; RUN: llc -march=mips -mattr=mips16 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
>
> ; RUN: llc -march=mips -mcpu=mips32r2 -mattr=+micromips -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips32r3 -mattr=+micromips -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips32r5 -mattr=+micromips -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
>
> ; RUN: llc -march=mips -mcpu=mips64 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips64r2 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips64r3 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips64r5 -O0 -relocation-model=pic -target-abi n64 \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
> ; RUN: llc -march=mips -mcpu=mips32r6 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s
>
> ; Valid targets for FastISel.
> ; RUN: llc -march=mips -mcpu=mips32r0 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
> ; RUN: llc -march=mips -mcpu=mips32r2 -O0 -relocation-model=pic \
> -; RUN: -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
> +; RUN: -pass-remarks-missed=isel -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
>
> ; The CHECK prefix is being used by those targets that do not support FastISel.
> ; By checking that we don't emit the "FastISel missed terminator..." message,
> Index: test/CodeGen/AArch64/fast-isel-tail-call.ll
> ===================================================================
> --- test/CodeGen/AArch64/fast-isel-tail-call.ll
> +++ test/CodeGen/AArch64/fast-isel-tail-call.ll
> @@ -1,4 +1,5 @@
> -; RUN: llc -fast-isel -fast-isel-verbose -mtriple arm64-- < %s 2> %t | FileCheck %s
> +; RUN: llc -fast-isel -fast-isel-verbose -pass-remarks-missed=isel \
> +; RUN: -mtriple arm64-- < %s 2> %t | FileCheck %s
> ; RUN: cat %t | FileCheck %s --check-prefix MISSED
>
> %struct = type { [4 x i32] }
> @@ -10,7 +11,7 @@
>
> ; Here, the %struct extractvalue should fail FastISel.
>
> -; MISSED: FastISel miss: %tmp1 = extractvalue %struct %tmp0, 0
> +; MISSED: FastISel missed: %tmp1 = extractvalue %struct %tmp0, 0
>
> ; CHECK-LABEL: test:
> ; CHECK: b external
> Index: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> +++ lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> @@ -26,6 +26,7 @@
> #include "llvm/Analysis/AliasAnalysis.h"
> #include "llvm/Analysis/BranchProbabilityInfo.h"
> #include "llvm/Analysis/CFG.h"
> +#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
> #include "llvm/Analysis/TargetLibraryInfo.h"
> #include "llvm/CodeGen/FastISel.h"
> #include "llvm/CodeGen/FunctionLoweringInfo.h"
> @@ -104,100 +105,6 @@
> STATISTIC(NumFastIselFailLowerArguments,
> "Number of entry blocks where fast isel failed to lower arguments");
>
> -#ifndef NDEBUG
> -static cl::opt<bool>
> -EnableFastISelVerbose2("fast-isel-verbose2", cl::Hidden,
> - cl::desc("Enable extra verbose messages in the \"fast\" "
> - "instruction selector"));
> -
> - // Terminators
> -STATISTIC(NumFastIselFailRet,"Fast isel fails on Ret");
> -STATISTIC(NumFastIselFailBr,"Fast isel fails on Br");
> -STATISTIC(NumFastIselFailSwitch,"Fast isel fails on Switch");
> -STATISTIC(NumFastIselFailIndirectBr,"Fast isel fails on IndirectBr");
> -STATISTIC(NumFastIselFailInvoke,"Fast isel fails on Invoke");
> -STATISTIC(NumFastIselFailResume,"Fast isel fails on Resume");
> -STATISTIC(NumFastIselFailUnreachable,"Fast isel fails on Unreachable");
> -
> - // Standard binary operators...
> -STATISTIC(NumFastIselFailAdd,"Fast isel fails on Add");
> -STATISTIC(NumFastIselFailFAdd,"Fast isel fails on FAdd");
> -STATISTIC(NumFastIselFailSub,"Fast isel fails on Sub");
> -STATISTIC(NumFastIselFailFSub,"Fast isel fails on FSub");
> -STATISTIC(NumFastIselFailMul,"Fast isel fails on Mul");
> -STATISTIC(NumFastIselFailFMul,"Fast isel fails on FMul");
> -STATISTIC(NumFastIselFailUDiv,"Fast isel fails on UDiv");
> -STATISTIC(NumFastIselFailSDiv,"Fast isel fails on SDiv");
> -STATISTIC(NumFastIselFailFDiv,"Fast isel fails on FDiv");
> -STATISTIC(NumFastIselFailURem,"Fast isel fails on URem");
> -STATISTIC(NumFastIselFailSRem,"Fast isel fails on SRem");
> -STATISTIC(NumFastIselFailFRem,"Fast isel fails on FRem");
> -
> - // Logical operators...
> -STATISTIC(NumFastIselFailAnd,"Fast isel fails on And");
> -STATISTIC(NumFastIselFailOr,"Fast isel fails on Or");
> -STATISTIC(NumFastIselFailXor,"Fast isel fails on Xor");
> -
> - // Memory instructions...
> -STATISTIC(NumFastIselFailAlloca,"Fast isel fails on Alloca");
> -STATISTIC(NumFastIselFailLoad,"Fast isel fails on Load");
> -STATISTIC(NumFastIselFailStore,"Fast isel fails on Store");
> -STATISTIC(NumFastIselFailAtomicCmpXchg,"Fast isel fails on AtomicCmpXchg");
> -STATISTIC(NumFastIselFailAtomicRMW,"Fast isel fails on AtomicRWM");
> -STATISTIC(NumFastIselFailFence,"Fast isel fails on Frence");
> -STATISTIC(NumFastIselFailGetElementPtr,"Fast isel fails on GetElementPtr");
> -
> - // Convert instructions...
> -STATISTIC(NumFastIselFailTrunc,"Fast isel fails on Trunc");
> -STATISTIC(NumFastIselFailZExt,"Fast isel fails on ZExt");
> -STATISTIC(NumFastIselFailSExt,"Fast isel fails on SExt");
> -STATISTIC(NumFastIselFailFPTrunc,"Fast isel fails on FPTrunc");
> -STATISTIC(NumFastIselFailFPExt,"Fast isel fails on FPExt");
> -STATISTIC(NumFastIselFailFPToUI,"Fast isel fails on FPToUI");
> -STATISTIC(NumFastIselFailFPToSI,"Fast isel fails on FPToSI");
> -STATISTIC(NumFastIselFailUIToFP,"Fast isel fails on UIToFP");
> -STATISTIC(NumFastIselFailSIToFP,"Fast isel fails on SIToFP");
> -STATISTIC(NumFastIselFailIntToPtr,"Fast isel fails on IntToPtr");
> -STATISTIC(NumFastIselFailPtrToInt,"Fast isel fails on PtrToInt");
> -STATISTIC(NumFastIselFailBitCast,"Fast isel fails on BitCast");
> -
> - // Other instructions...
> -STATISTIC(NumFastIselFailICmp,"Fast isel fails on ICmp");
> -STATISTIC(NumFastIselFailFCmp,"Fast isel fails on FCmp");
> -STATISTIC(NumFastIselFailPHI,"Fast isel fails on PHI");
> -STATISTIC(NumFastIselFailSelect,"Fast isel fails on Select");
> -STATISTIC(NumFastIselFailCall,"Fast isel fails on Call");
> -STATISTIC(NumFastIselFailShl,"Fast isel fails on Shl");
> -STATISTIC(NumFastIselFailLShr,"Fast isel fails on LShr");
> -STATISTIC(NumFastIselFailAShr,"Fast isel fails on AShr");
> -STATISTIC(NumFastIselFailVAArg,"Fast isel fails on VAArg");
> -STATISTIC(NumFastIselFailExtractElement,"Fast isel fails on ExtractElement");
> -STATISTIC(NumFastIselFailInsertElement,"Fast isel fails on InsertElement");
> -STATISTIC(NumFastIselFailShuffleVector,"Fast isel fails on ShuffleVector");
> -STATISTIC(NumFastIselFailExtractValue,"Fast isel fails on ExtractValue");
> -STATISTIC(NumFastIselFailInsertValue,"Fast isel fails on InsertValue");
> -STATISTIC(NumFastIselFailLandingPad,"Fast isel fails on LandingPad");
> -
> -// Intrinsic instructions...
> -STATISTIC(NumFastIselFailIntrinsicCall, "Fast isel fails on Intrinsic call");
> -STATISTIC(NumFastIselFailSAddWithOverflow,
> - "Fast isel fails on sadd.with.overflow");
> -STATISTIC(NumFastIselFailUAddWithOverflow,
> - "Fast isel fails on uadd.with.overflow");
> -STATISTIC(NumFastIselFailSSubWithOverflow,
> - "Fast isel fails on ssub.with.overflow");
> -STATISTIC(NumFastIselFailUSubWithOverflow,
> - "Fast isel fails on usub.with.overflow");
> -STATISTIC(NumFastIselFailSMulWithOverflow,
> - "Fast isel fails on smul.with.overflow");
> -STATISTIC(NumFastIselFailUMulWithOverflow,
> - "Fast isel fails on umul.with.overflow");
> -STATISTIC(NumFastIselFailFrameaddress, "Fast isel fails on Frameaddress");
> -STATISTIC(NumFastIselFailSqrt, "Fast isel fails on sqrt call");
> -STATISTIC(NumFastIselFailStackMap, "Fast isel fails on StackMap call");
> -STATISTIC(NumFastIselFailPatchPoint, "Fast isel fails on PatchPoint call");
> -#endif
> -
> static cl::opt<bool>
> EnableFastISelVerbose("fast-isel-verbose", cl::Hidden,
> cl::desc("Enable verbose messages in the \"fast\" "
> @@ -500,7 +407,9 @@
>
> SplitCriticalSideEffectEdges(const_cast<Function &>(Fn));
>
> - CurDAG->init(*MF);
> + ORE = make_unique<OptimizationRemarkEmitter>(&Fn);
I'd probably put this a little earlier in the function, with all of the
other member initializations. I think that's a little more consistent.
> +
> + CurDAG->init(*MF, *ORE);
> FuncInfo->set(Fn, *MF, CurDAG);
>
> if (UseMBPI && OptLevel != CodeGenOpt::None)
> @@ -708,6 +617,21 @@
> return true;
> }
>
> +static void reportFastISelFailure(MachineFunction &MF,
> + OptimizationRemarkEmitter &ORE,
> + OptimizationRemarkMissed &R,
> + bool ShouldAbort) {
> + // Print the function name explicitly if we don't have a debug location (which
> + // makes the diagnostic less useful) or if we're going to emit a raw error.
> + if (!R.getLocation().isValid() || ShouldAbort)
> + R << (" (in function: " + MF.getName() + ")").str();
> +
> + if (ShouldAbort)
> + report_fatal_error(R.getMsg());
> + else
> + ORE.emit(R);
Since report_fatal_error is noreturn I think omitting the else is
appropriate. This is fine either way though.
> +}
> +
> void SelectionDAGISel::SelectBasicBlock(BasicBlock::const_iterator Begin,
> BasicBlock::const_iterator End,
> bool &HadTailCall) {
> @@ -1164,116 +1088,6 @@
> !FuncInfo->isExportedInst(I); // Exported instrs must be computed.
> }
>
> -#ifndef NDEBUG
> -// Collect per Instruction statistics for fast-isel misses. Only those
> -// instructions that cause the bail are accounted for. It does not account for
> -// instructions higher in the block. Thus, summing the per instructions stats
> -// will not add up to what is reported by NumFastIselFailures.
> -static void collectFailStats(const Instruction *I) {
> - switch (I->getOpcode()) {
> - default: assert(false && "<Invalid operator>");
> -
> - // Terminators
> - case Instruction::Ret: NumFastIselFailRet++; return;
> - case Instruction::Br: NumFastIselFailBr++; return;
> - case Instruction::Switch: NumFastIselFailSwitch++; return;
> - case Instruction::IndirectBr: NumFastIselFailIndirectBr++; return;
> - case Instruction::Invoke: NumFastIselFailInvoke++; return;
> - case Instruction::Resume: NumFastIselFailResume++; return;
> - case Instruction::Unreachable: NumFastIselFailUnreachable++; return;
> -
> - // Standard binary operators...
> - case Instruction::Add: NumFastIselFailAdd++; return;
> - case Instruction::FAdd: NumFastIselFailFAdd++; return;
> - case Instruction::Sub: NumFastIselFailSub++; return;
> - case Instruction::FSub: NumFastIselFailFSub++; return;
> - case Instruction::Mul: NumFastIselFailMul++; return;
> - case Instruction::FMul: NumFastIselFailFMul++; return;
> - case Instruction::UDiv: NumFastIselFailUDiv++; return;
> - case Instruction::SDiv: NumFastIselFailSDiv++; return;
> - case Instruction::FDiv: NumFastIselFailFDiv++; return;
> - case Instruction::URem: NumFastIselFailURem++; return;
> - case Instruction::SRem: NumFastIselFailSRem++; return;
> - case Instruction::FRem: NumFastIselFailFRem++; return;
> -
> - // Logical operators...
> - case Instruction::And: NumFastIselFailAnd++; return;
> - case Instruction::Or: NumFastIselFailOr++; return;
> - case Instruction::Xor: NumFastIselFailXor++; return;
> -
> - // Memory instructions...
> - case Instruction::Alloca: NumFastIselFailAlloca++; return;
> - case Instruction::Load: NumFastIselFailLoad++; return;
> - case Instruction::Store: NumFastIselFailStore++; return;
> - case Instruction::AtomicCmpXchg: NumFastIselFailAtomicCmpXchg++; return;
> - case Instruction::AtomicRMW: NumFastIselFailAtomicRMW++; return;
> - case Instruction::Fence: NumFastIselFailFence++; return;
> - case Instruction::GetElementPtr: NumFastIselFailGetElementPtr++; return;
> -
> - // Convert instructions...
> - case Instruction::Trunc: NumFastIselFailTrunc++; return;
> - case Instruction::ZExt: NumFastIselFailZExt++; return;
> - case Instruction::SExt: NumFastIselFailSExt++; return;
> - case Instruction::FPTrunc: NumFastIselFailFPTrunc++; return;
> - case Instruction::FPExt: NumFastIselFailFPExt++; return;
> - case Instruction::FPToUI: NumFastIselFailFPToUI++; return;
> - case Instruction::FPToSI: NumFastIselFailFPToSI++; return;
> - case Instruction::UIToFP: NumFastIselFailUIToFP++; return;
> - case Instruction::SIToFP: NumFastIselFailSIToFP++; return;
> - case Instruction::IntToPtr: NumFastIselFailIntToPtr++; return;
> - case Instruction::PtrToInt: NumFastIselFailPtrToInt++; return;
> - case Instruction::BitCast: NumFastIselFailBitCast++; return;
> -
> - // Other instructions...
> - case Instruction::ICmp: NumFastIselFailICmp++; return;
> - case Instruction::FCmp: NumFastIselFailFCmp++; return;
> - case Instruction::PHI: NumFastIselFailPHI++; return;
> - case Instruction::Select: NumFastIselFailSelect++; return;
> - case Instruction::Call: {
> - if (auto const *Intrinsic = dyn_cast<IntrinsicInst>(I)) {
> - switch (Intrinsic->getIntrinsicID()) {
> - default:
> - NumFastIselFailIntrinsicCall++; return;
> - case Intrinsic::sadd_with_overflow:
> - NumFastIselFailSAddWithOverflow++; return;
> - case Intrinsic::uadd_with_overflow:
> - NumFastIselFailUAddWithOverflow++; return;
> - case Intrinsic::ssub_with_overflow:
> - NumFastIselFailSSubWithOverflow++; return;
> - case Intrinsic::usub_with_overflow:
> - NumFastIselFailUSubWithOverflow++; return;
> - case Intrinsic::smul_with_overflow:
> - NumFastIselFailSMulWithOverflow++; return;
> - case Intrinsic::umul_with_overflow:
> - NumFastIselFailUMulWithOverflow++; return;
> - case Intrinsic::frameaddress:
> - NumFastIselFailFrameaddress++; return;
> - case Intrinsic::sqrt:
> - NumFastIselFailSqrt++; return;
> - case Intrinsic::experimental_stackmap:
> - NumFastIselFailStackMap++; return;
> - case Intrinsic::experimental_patchpoint_void: // fall-through
> - case Intrinsic::experimental_patchpoint_i64:
> - NumFastIselFailPatchPoint++; return;
> - }
> - }
> - NumFastIselFailCall++;
> - return;
> - }
> - case Instruction::Shl: NumFastIselFailShl++; return;
> - case Instruction::LShr: NumFastIselFailLShr++; return;
> - case Instruction::AShr: NumFastIselFailAShr++; return;
> - case Instruction::VAArg: NumFastIselFailVAArg++; return;
> - case Instruction::ExtractElement: NumFastIselFailExtractElement++; return;
> - case Instruction::InsertElement: NumFastIselFailInsertElement++; return;
> - case Instruction::ShuffleVector: NumFastIselFailShuffleVector++; return;
> - case Instruction::ExtractValue: NumFastIselFailExtractValue++; return;
> - case Instruction::InsertValue: NumFastIselFailInsertValue++; return;
> - case Instruction::LandingPad: NumFastIselFailLandingPad++; return;
> - }
> -}
> -#endif // NDEBUG
> -
> /// Set up SwiftErrorVals by going through the function. If the function has
> /// swifterror argument, it will be the first entry.
> static void setupSwiftErrorVals(const Function &Fn, const TargetLowering *TLI,
> @@ -1491,8 +1305,13 @@
> FastISelFailed = true;
> // Fast isel failed to lower these arguments
> ++NumFastIselFailLowerArguments;
> - if (EnableFastISelAbort > 1)
> - report_fatal_error("FastISel didn't lower all arguments");
> +
> + OptimizationRemarkMissed R("sdagisel", "FastISelFailure",
> + Fn.getSubprogram(),
> + &Fn.getEntryBlock());
> + R << "FastISel didn't lower all arguments: "
> + << ore::NV("Prototype", Fn.getType());
> + reportFastISelFailure(*MF, *ORE, R, EnableFastISelAbort > 1);
>
> // Use SelectionDAG argument lowering
> LowerArguments(Fn);
> @@ -1601,22 +1420,22 @@
> continue;
> }
>
> -#ifndef NDEBUG
> - if (EnableFastISelVerbose2)
> - collectFailStats(Inst);
> -#endif
> -
> // Then handle certain instructions as single-LLVM-Instruction blocks.
> if (isa<CallInst>(Inst)) {
> + OptimizationRemarkMissed R("sdagisel", "FastISelFailure",
> + Inst->getDebugLoc(), LLVMBB);
> +
> + R << "FastISel missed call";
>
> if (EnableFastISelVerbose || EnableFastISelAbort) {
As I alluded to earlier, I think this should just be R.isEnabled() here
when you want to avoid doing the extra work.
> - dbgs() << "FastISel missed call: ";
> - Inst->print(dbgs());
> + std::string InstStrStorage;
> + raw_string_ostream InstStr(InstStrStorage);
> + InstStr << *Inst;
> +
> + R << ": " << InstStr.str();
> }
> - if (EnableFastISelAbort > 2)
> - // FastISel selector couldn't handle something and bailed.
> - // For the purpose of debugging, just abort.
> - report_fatal_error("FastISel didn't select the entire block");
> +
> + reportFastISelFailure(*MF, *ORE, R, EnableFastISelAbort > 2);
>
> if (!Inst->getType()->isVoidTy() && !Inst->getType()->isTokenTy() &&
> !Inst->use_empty()) {
> @@ -1645,22 +1464,29 @@
> continue;
> }
>
> + OptimizationRemarkMissed R("sdagisel", "FastISelFailure",
> + Inst->getDebugLoc(), LLVMBB);
> +
> bool ShouldAbort = EnableFastISelAbort;
> - if (EnableFastISelVerbose || EnableFastISelAbort) {
> - if (isa<TerminatorInst>(Inst)) {
> - // Use a different message for terminator misses.
> - dbgs() << "FastISel missed terminator: ";
> - // Don't abort unless for terminator unless the level is really high
> - ShouldAbort = (EnableFastISelAbort > 2);
> - } else {
> - dbgs() << "FastISel miss: ";
> - }
> - Inst->print(dbgs());
> + if (isa<TerminatorInst>(Inst)) {
> + // Use a different message for terminator misses.
> + R << "FastISel missed terminator";
> + // Don't abort for terminator unless the level is really high
> + ShouldAbort = (EnableFastISelAbort > 2);
> + } else {
> + R << "FastISel missed";
> }
> - if (ShouldAbort)
> - // FastISel selector couldn't handle something and bailed.
> - // For the purpose of debugging, just abort.
> - report_fatal_error("FastISel didn't select the entire block");
> +
> + if (EnableFastISelVerbose || EnableFastISelAbort) {
Same here.
> + std::string InstStrStorage;
> + raw_string_ostream InstStr(InstStrStorage);
> + InstStr << *Inst;
> + R << ": " << InstStr.str();
> + } else {
> + R << ": " << ore::NV("Instruction", Inst);
I guess this is just a cheaper to build (but less informative version)
of the remark? With the isEnabled checks I guess we'd just drop this.
> + }
> +
> + reportFastISelFailure(*MF, *ORE, R, ShouldAbort);
>
> NumFastIselFailures += NumFastIselRemaining;
> break;
> Index: lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> +++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> @@ -871,11 +871,13 @@
> DbgInfo = new SDDbgInfo();
> }
>
> -void SelectionDAG::init(MachineFunction &mf) {
> - MF = &mf;
> +void SelectionDAG::init(MachineFunction &NewMF,
> + OptimizationRemarkEmitter &NewORE) {
> + MF = &NewMF;
> + ORE = &NewORE;
> TLI = getSubtarget().getTargetLowering();
> TSI = getSubtarget().getSelectionDAGInfo();
> - Context = &mf.getFunction()->getContext();
> + Context = &MF->getFunction()->getContext();
> }
>
> SelectionDAG::~SelectionDAG() {
> Index: include/llvm/CodeGen/SelectionDAGISel.h
> ===================================================================
> --- include/llvm/CodeGen/SelectionDAGISel.h
> +++ include/llvm/CodeGen/SelectionDAGISel.h
> @@ -20,6 +20,7 @@
> #include "llvm/IR/BasicBlock.h"
> #include "llvm/Pass.h"
> #include "llvm/Target/TargetSubtargetInfo.h"
> +#include <memory>
>
> namespace llvm {
> class FastISel;
> @@ -29,6 +30,7 @@
> class MachineBasicBlock;
> class MachineFunction;
> class MachineInstr;
> + class OptimizationRemarkEmitter;
> class TargetLowering;
> class TargetLibraryInfo;
> class FunctionLoweringInfo;
> @@ -56,6 +58,10 @@
> bool FastISelFailed;
> SmallPtrSet<const Instruction *, 4> ElidedArgCopyInstrs;
>
> + /// Current optimization remark emitter.
> + /// Used to report things like combines and FastISel failures.
> + std::unique_ptr<OptimizationRemarkEmitter> ORE;
> +
> static char ID;
>
> explicit SelectionDAGISel(TargetMachine &tm,
> Index: include/llvm/CodeGen/SelectionDAG.h
> ===================================================================
> --- include/llvm/CodeGen/SelectionDAG.h
> +++ include/llvm/CodeGen/SelectionDAG.h
> @@ -36,6 +36,7 @@
> class MachineConstantPoolValue;
> class MachineFunction;
> class MDNode;
> +class OptimizationRemarkEmitter;
> class SDDbgValue;
> class TargetLowering;
> class SelectionDAGTargetInfo;
> @@ -171,6 +172,10 @@
> LLVMContext *Context;
> CodeGenOpt::Level OptLevel;
>
> + /// The function-level optimization remark emitter. Used to emit remarks
> + /// whenever manipulating the DAG.
> + OptimizationRemarkEmitter *ORE;
> +
> /// The starting token.
> SDNode EntryNode;
>
> @@ -318,7 +323,7 @@
> ~SelectionDAG();
>
> /// Prepare this SelectionDAG to process code in the given MachineFunction.
> - void init(MachineFunction &mf);
> + void init(MachineFunction &NewMF, OptimizationRemarkEmitter &NewORE);
>
> /// Clear state and free memory necessary to make this
> /// SelectionDAG ready to process a new block.
> @@ -331,6 +336,7 @@
> const TargetLowering &getTargetLoweringInfo() const { return *TLI; }
> const SelectionDAGTargetInfo &getSelectionDAGInfo() const { return *TSI; }
> LLVMContext *getContext() const {return Context; }
> + OptimizationRemarkEmitter &getORE() const { return *ORE; }
>
> /// Pop up a GraphViz/gv window with the DAG rendered using 'dot'.
> void viewGraph(const std::string &Title);
More information about the llvm-commits
mailing list