Skip to content

Commit

Permalink
[globalisel][tablegen] Generate rule coverage and use it to identify …
Browse files Browse the repository at this point in the history
…untested rules

Summary:
This patch adds a LLVM_ENABLE_GISEL_COV which, like LLVM_ENABLE_DAGISEL_COV,
causes TableGen to instrument the generated table to collect rule coverage
information. However, LLVM_ENABLE_GISEL_COV goes a bit further than
LLVM_ENABLE_DAGISEL_COV. The information is written to files
(${CMAKE_BINARY_DIR}/gisel-coverage-* by default). These files can then be
concatenated into ${LLVM_GISEL_COV_PREFIX}-all after which TableGen will
read this information and use it to emit warnings about untested rules.

This technique could also be used by SelectionDAG and can be further
extended to detect hot rules and give them priority over colder rules.

Usage:
* Enable LLVM_ENABLE_GISEL_COV in CMake
* Build the compiler and run some tests
* cat gisel-coverage-[0-9]* > gisel-coverage-all
* Delete lib/Target/*/*GenGlobalISel.inc*
* Build the compiler

Known issues:
* ${LLVM_GISEL_COV_PREFIX}-all must be generated as a manual
  step due to a lack of a portable 'cat' command. It should be the
  concatenation of all ${LLVM_GISEL_COV_PREFIX}-[0-9]* files.
* There's no mechanism to discard coverage information when the ruleset
  changes

Depends on D39742

Reviewers: ab, qcolombet, t.p.northover, aditya_nandakumar, rovka

Reviewed By: rovka

Subscribers: vsk, arsenm, nhaehnle, mgorny, kristof.beyls, javed.absar, igorb, llvm-commits

Differential Revision: https://reviews.llvm.org/D39747

llvm-svn: 318356
  • Loading branch information
dsandersllvm committed Nov 16, 2017
1 parent 8d8a8bb commit f76f315
Show file tree
Hide file tree
Showing 17 changed files with 372 additions and 40 deletions.
4 changes: 4 additions & 0 deletions llvm/CMakeLists.txt
Expand Up @@ -167,6 +167,10 @@ if(LLVM_DEPENDENCY_DEBUGGING)
endif()

option(LLVM_ENABLE_DAGISEL_COV "Debug: Prints tablegen patterns that were used for selecting" OFF)
option(LLVM_ENABLE_GISEL_COV "Enable collection of GlobalISel rule coverage" OFF)
if(LLVM_ENABLE_GISEL_COV)
set(LLVM_GISEL_COV_PREFIX "${CMAKE_BINARY_DIR}/gisel-coverage-" CACHE STRING "Provide a filename prefix to collect the GlobalISel rule coverage")
endif()

# Add path for custom modules
set(CMAKE_MODULE_PATH
Expand Down
7 changes: 7 additions & 0 deletions llvm/cmake/modules/TableGen.cmake
Expand Up @@ -52,6 +52,13 @@ function(tablegen project ofn)
list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-coverage")
endif()
endif()
if (LLVM_ENABLE_GISEL_COV)
list(FIND ARGN "-gen-global-isel" idx)
if( NOT idx EQUAL -1 )
list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-gisel-coverage")
list(APPEND LLVM_TABLEGEN_FLAGS "-gisel-coverage-file=${LLVM_GISEL_COV_PREFIX}all")
endif()
endif()

# We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
# (both the target and the file) to have .inc files rebuilt on
Expand Down
14 changes: 10 additions & 4 deletions llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
Expand Up @@ -17,8 +17,9 @@
#define LLVM_CODEGEN_GLOBALISEL_INSTRUCTIONSELECTOR_H

#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CodeGenCoverage.h"
#include <bitset>
#include <cstddef>
#include <cstdint>
Expand All @@ -33,6 +34,7 @@ class APFloat;
class LLT;
class MachineInstr;
class MachineInstrBuilder;
class MachineFunction;
class MachineOperand;
class MachineRegisterInfo;
class RegisterBankInfo;
Expand Down Expand Up @@ -262,6 +264,10 @@ enum {

/// A successful emission
GIR_Done,

/// Increment the rule coverage counter.
/// - RuleID - The ID of the rule that was covered.
GIR_Coverage,
};

enum {
Expand Down Expand Up @@ -289,7 +295,7 @@ class InstructionSelector {
/// if returns true:
/// for I in all mutated/inserted instructions:
/// !isPreISelGenericOpcode(I.getOpcode())
virtual bool select(MachineInstr &I) const = 0;
virtual bool select(MachineInstr &I, CodeGenCoverage &CoverageInfo) const = 0;

protected:
using ComplexRendererFns =
Expand Down Expand Up @@ -328,8 +334,8 @@ class InstructionSelector {
const MatcherInfoTy<PredicateBitset, ComplexMatcherMemFn> &MatcherInfo,
const int64_t *MatchTable, const TargetInstrInfo &TII,
MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI,
const PredicateBitset &AvailableFeatures) const;
const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
CodeGenCoverage &CoverageInfo) const;

/// Constrain a register operand of an instruction \p I to a specified
/// register class. This could involve inserting COPYs before (for uses) or
Expand Down
14 changes: 12 additions & 2 deletions llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
Expand Up @@ -49,8 +49,8 @@ bool InstructionSelector::executeMatchTable(
const MatcherInfoTy<PredicateBitset, ComplexMatcherMemFn> &MatcherInfo,
const int64_t *MatchTable, const TargetInstrInfo &TII,
MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI,
const PredicateBitset &AvailableFeatures) const {
const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
CodeGenCoverage &CoverageInfo) const {
uint64_t CurrentIdx = 0;
SmallVector<uint64_t, 8> OnFailResumeAt;

Expand Down Expand Up @@ -677,6 +677,16 @@ bool InstructionSelector::executeMatchTable(
break;
}

case GIR_Coverage: {
int64_t RuleID = MatchTable[CurrentIdx++];
CoverageInfo.setCovered(RuleID);

DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs()
<< CurrentIdx << ": GIR_Coverage(" << RuleID << ")");
break;
}

case GIR_Done:
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": GIR_Done");
Expand Down
6 changes: 6 additions & 0 deletions llvm/include/llvm/Config/config.h.cmake
Expand Up @@ -437,4 +437,10 @@
/* Define to a function implementing strdup */
#cmakedefine strdup ${strdup}

/* Whether GlobalISel rule coverage is being collected */
#cmakedefine01 LLVM_GISEL_COV_ENABLED

/* Define to the default GlobalISel coverage file prefix */
#cmakedefine LLVM_GISEL_COV_PREFIX "${LLVM_GISEL_COV_PREFIX}"

#endif
37 changes: 37 additions & 0 deletions llvm/include/llvm/Support/CodeGenCoverage.h
@@ -0,0 +1,37 @@
//== llvm/Support/CodeGenCoverage.h ------------------------------*- C++ -*-==//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
/// \file This file provides rule coverage tracking for tablegen-erated CodeGen.
//===----------------------------------------------------------------------===//

#ifndef LLVM_SUPPORT_CODEGENCOVERAGE_H
#define LLVM_SUPPORT_CODEGENCOVERAGE_H

#include "llvm/ADT/BitVector.h"
#include "llvm/Config/config.h"

namespace llvm {
class LLVMContext;

class CodeGenCoverage {
protected:
BitVector RuleCoverage;

public:
CodeGenCoverage();

void setCovered(uint64_t RuleID);
bool isCovered(uint64_t RuleID);

bool parse(MemoryBuffer &Buffer, StringRef BackendName);
bool emit(StringRef FilePrefix, StringRef BackendName) const;
void reset();
};
} // end namespace llvm

#endif // ifndef LLVM_SUPPORT_CODEGENCOVERAGE_H
21 changes: 20 additions & 1 deletion llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
Expand Up @@ -20,17 +20,28 @@
#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetPassConfig.h"
#include "llvm/Config/config.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Function.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Target/TargetLowering.h"
#include "llvm/Target/TargetSubtargetInfo.h"

#define DEBUG_TYPE "instruction-select"

using namespace llvm;

#ifdef LLVM_GISEL_COV_PREFIX
static cl::opt<std::string>
CoveragePrefix("gisel-coverage-prefix", cl::init(LLVM_GISEL_COV_PREFIX),
cl::desc("Record GlobalISel rule coverage files of this "
"prefix if instrumentation was generated"));
#else
static const std::string CoveragePrefix = "";
#endif

char InstructionSelect::ID = 0;
INITIALIZE_PASS_BEGIN(InstructionSelect, DEBUG_TYPE,
"Select target instructions out of generic instructions",
Expand Down Expand Up @@ -66,6 +77,7 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {

const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
const InstructionSelector *ISel = MF.getSubtarget().getInstructionSelector();
CodeGenCoverage CoverageInfo;
assert(ISel && "Cannot work without InstructionSelector");

// An optimization remark emitter. Used to report failures.
Expand Down Expand Up @@ -127,7 +139,7 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
continue;
}

if (!ISel->select(MI)) {
if (!ISel->select(MI, CoverageInfo)) {
// FIXME: It would be nice to dump all inserted instructions. It's
// not obvious how, esp. considering select() can insert after MI.
reportGISelFailure(MF, TPC, MORE, "gisel-select", "cannot select", MI);
Expand Down Expand Up @@ -187,6 +199,13 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
auto &TLI = *MF.getSubtarget().getTargetLowering();
TLI.finalizeLowering(MF);

CoverageInfo.emit(CoveragePrefix,
MF.getSubtarget()
.getTargetLowering()
->getTargetMachine()
.getTarget()
.getBackendName());

// FIXME: Should we accurately track changes?
return true;
}
1 change: 1 addition & 0 deletions llvm/lib/Support/CMakeLists.txt
Expand Up @@ -48,6 +48,7 @@ add_llvm_library(LLVMSupport
circular_raw_ostream.cpp
Chrono.cpp
COM.cpp
CodeGenCoverage.cpp
CommandLine.cpp
Compression.cpp
ConvertUTF.cpp
Expand Down
118 changes: 118 additions & 0 deletions llvm/lib/Support/CodeGenCoverage.cpp
@@ -0,0 +1,118 @@
//===- lib/Support/CodeGenCoverage.cpp -------------------------------------==//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
/// \file
/// This file implements the CodeGenCoverage class.
//===----------------------------------------------------------------------===//

#include "llvm/Support/CodeGenCoverage.h"

#include "llvm/Support/Endian.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/ToolOutputFile.h"

#if LLVM_ON_UNIX
#include <unistd.h>
#elif LLVM_ON_WIN32
#include <windows.h>
#endif

using namespace llvm;

static sys::SmartMutex<true> OutputMutex;

CodeGenCoverage::CodeGenCoverage() {}

void CodeGenCoverage::setCovered(uint64_t RuleID) {
if (RuleCoverage.size() <= RuleID)
RuleCoverage.resize(RuleID + 1, 0);
RuleCoverage[RuleID] = true;
}

bool CodeGenCoverage::isCovered(uint64_t RuleID) {
if (RuleCoverage.size() <= RuleID)
return false;
return RuleCoverage[RuleID];
}

bool CodeGenCoverage::parse(MemoryBuffer &Buffer, StringRef BackendName) {
const char *CurPtr = Buffer.getBufferStart();

while (CurPtr != Buffer.getBufferEnd()) {
// Read the backend name from the input.
const char *LexedBackendName = CurPtr;
while (*CurPtr++ != 0)
;
if (CurPtr == Buffer.getBufferEnd())
return false; // Data is invalid, expected rule id's to follow.

bool IsForThisBackend = BackendName.equals(LexedBackendName);
while (CurPtr != Buffer.getBufferEnd()) {
if (std::distance(CurPtr, Buffer.getBufferEnd()) < 8)
return false; // Data is invalid. Not enough bytes for another rule id.

uint64_t RuleID = support::endian::read64(CurPtr, support::native);
CurPtr += 8;

// ~0ull terminates the rule id list.
if (RuleID == ~0ull)
break;

// Anything else, is recorded or ignored depending on whether it's
// intended for the backend we're interested in.
if (IsForThisBackend)
setCovered(RuleID);
}
}

return true;
}

bool CodeGenCoverage::emit(StringRef CoveragePrefix,
StringRef BackendName) const {
if (!CoveragePrefix.empty() && !RuleCoverage.empty()) {
sys::SmartScopedLock<true> Lock(OutputMutex);

// We can handle locking within a process easily enough but we don't want to
// manage it between multiple processes. Use the process ID to ensure no
// more than one process is ever writing to the same file at the same time.
std::string Pid =
#if LLVM_ON_UNIX
llvm::to_string(::getpid());
#elif LLVM_ON_WIN32
llvm::to_string(::GetCurrentProcessId());
#else
"";
#endif

std::string CoverageFilename = (CoveragePrefix + Pid).str();

std::error_code EC;
sys::fs::OpenFlags OpenFlags = sys::fs::F_Append;
std::unique_ptr<ToolOutputFile> CoverageFile =
llvm::make_unique<ToolOutputFile>(CoverageFilename, EC, OpenFlags);
if (EC)
return false;

uint64_t Zero = 0;
uint64_t InvZero = ~0ull;
CoverageFile->os() << BackendName;
CoverageFile->os().write((const char *)&Zero, sizeof(unsigned char));
for (uint64_t I : RuleCoverage.set_bits())
CoverageFile->os().write((const char *)&I, sizeof(uint64_t));
CoverageFile->os().write((const char *)&InvZero, sizeof(uint64_t));

CoverageFile->keep();
}

return true;
}

void CodeGenCoverage::reset() { RuleCoverage.resize(0); }
9 changes: 5 additions & 4 deletions llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
Expand Up @@ -48,13 +48,13 @@ class AArch64InstructionSelector : public InstructionSelector {
const AArch64Subtarget &STI,
const AArch64RegisterBankInfo &RBI);

bool select(MachineInstr &I) const override;
bool select(MachineInstr &I, CodeGenCoverage &CoverageInfo) const override;
static const char *getName() { return DEBUG_TYPE; }

private:
/// tblgen-erated 'select' implementation, used as the initial selector for
/// the patterns that don't require complex C++.
bool selectImpl(MachineInstr &I) const;
bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;

bool selectVaStartAAPCS(MachineInstr &I, MachineFunction &MF,
MachineRegisterInfo &MRI) const;
Expand Down Expand Up @@ -609,7 +609,8 @@ bool AArch64InstructionSelector::selectVaStartDarwin(
return true;
}

bool AArch64InstructionSelector::select(MachineInstr &I) const {
bool AArch64InstructionSelector::select(MachineInstr &I,
CodeGenCoverage &CoverageInfo) const {
assert(I.getParent() && "Instruction should be in a basic block!");
assert(I.getParent()->getParent() && "Instruction should be in a function!");

Expand Down Expand Up @@ -667,7 +668,7 @@ bool AArch64InstructionSelector::select(MachineInstr &I) const {
return false;
}

if (selectImpl(I))
if (selectImpl(I, CoverageInfo))
return true;

LLT Ty =
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
Expand Up @@ -402,7 +402,8 @@ bool AMDGPUInstructionSelector::selectG_LOAD(MachineInstr &I) const {
return Ret;
}

bool AMDGPUInstructionSelector::select(MachineInstr &I) const {
bool AMDGPUInstructionSelector::select(MachineInstr &I,
CodeGenCoverage &CoverageInfo) const {

if (!isPreISelGenericOpcode(I.getOpcode()))
return true;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.h
Expand Up @@ -35,7 +35,8 @@ class AMDGPUInstructionSelector : public InstructionSelector {
AMDGPUInstructionSelector(const SISubtarget &STI,
const AMDGPURegisterBankInfo &RBI);

bool select(MachineInstr &I) const override;
bool select(MachineInstr &I, CodeGenCoverage &CoverageInfo) const override;

private:
struct GEPInfo {
const MachineInstr &GEP;
Expand Down

0 comments on commit f76f315

Please sign in to comment.