[PATCH] D31405: [CodeGen] Pass SDAG an ORE, and replace FastISel stats with remarks.
Ahmed Bougacha via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 29 09:57:15 PDT 2017
On Wed, Mar 29, 2017 at 4:12 AM, Justin Bogner <mail at justinbogner.com> wrote:
> 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.
'allowsExtraAnalysis' would be the appropriate API, but that doesn't
know about which remarks are enabled (http://llvm.org/PR32352).
In the meantime, we can definitely use isEnabled. You'll need
'-Rpass-missed=isel -mllvm -pass-remarks-missed=isel' to get the
remarks from clang, but I suppose that's no worse than
'-Rpass-missed=isel -mllvm -fast-isel-verbose'.
> 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.
Done
>> +
>> + 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.
Good point, done
>> +}
>> +
>> 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.
Done
>> - 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.
Eh, we'd have some very basic remarks with just '-Rpass-missed=isel',
but these are obscure enough that it's fine; removed.
-Ahmed
>> + }
>> +
>> + 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