Skip to content

Commit

Permalink
[nfc][mlgo][regalloc] Stop warnings about unused function
Browse files Browse the repository at this point in the history
Added a `NoopSavedModelImpl` type which can be used as a mock AOT-ed
saved model, and further minimize conditional compilation cases. This
also removes unused function warnings on gcc.
  • Loading branch information
mtrofin committed Feb 8, 2022
1 parent 24a1869 commit 5a50ab4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
17 changes: 17 additions & 0 deletions llvm/include/llvm/Analysis/ReleaseModeModelRunner.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define LLVM_ANALYSIS_RELEASEMODEMODELRUNNER_H

#include "llvm/Analysis/MLModelRunner.h"
#include "llvm/Support/ErrorHandling.h"

#include <memory>
#include <vector>
Expand Down Expand Up @@ -73,6 +74,22 @@ class ReleaseModeModelRunner final : public MLModelRunner {
int32_t ResultIndex = -1;
std::unique_ptr<TGen> CompiledModel;
};

/// A mock class satisfying the interface expected by ReleaseModeModelRunner for
/// its `TGen` parameter. Useful to avoid conditional compilation complexity, as
/// a compile-time replacement for a real AOT-ed model.
class NoopSavedModelImpl final {
const char *ErrMsg = "The mock AOT-ed saved model is a compile-time stub and "
"should not be called.";

public:
NoopSavedModelImpl() = default;
int LookupArgIndex(const std::string &) { llvm_unreachable(ErrMsg); }
int LookupResultIndex(const std::string &) { llvm_unreachable(ErrMsg); }
void Run() { llvm_unreachable(ErrMsg); }
void *result_data(int) { llvm_unreachable(ErrMsg); }
void *arg_data(int) { llvm_unreachable(ErrMsg); }
};
} // namespace llvm

#endif // LLVM_ANALYSIS_RELEASEMODEMODELRUNNER_H
11 changes: 5 additions & 6 deletions llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ using namespace llvm;
// Generated header in release (AOT) mode
#if defined(LLVM_HAVE_TF_AOT_REGALLOCEVICTMODEL)
#include "RegallocEvictModel.h"
using CompiledModelType = RegallocEvictModel;
#else
using CompiledModelType = NoopSavedModelImpl;
#endif

// Options that only make sense in development mode
Expand Down Expand Up @@ -318,7 +321,6 @@ class MLEvictAdvisor : public RegAllocEvictionAdvisor {
// ===================================
// Release (AOT) - specifics
// ===================================
#if defined(LLVM_HAVE_TF_AOT_REGALLOCEVICTMODEL)
const std::array<std::string, FeatureIDs::FeatureCount> FeatureNames{
#define _GETNAME(_, NAME, __, ___) #NAME,
RA_EVICT_FEATURES_LIST(_GETNAME)
Expand All @@ -344,15 +346,14 @@ class ReleaseModeEvictionAdvisorAnalysis final
std::unique_ptr<RegAllocEvictionAdvisor>
getAdvisor(const MachineFunction &MF, const RAGreedy &RA) override {
if (!Runner)
Runner = std::make_unique<ReleaseModeModelRunner<RegallocEvictModel>>(
Runner = std::make_unique<ReleaseModeModelRunner<CompiledModelType>>(
MF.getFunction().getContext(), FeatureNames, DecisionName);
return std::make_unique<MLEvictAdvisor>(
MF, RA, Runner.get(), getAnalysis<MachineBlockFrequencyInfo>(),
getAnalysis<MachineLoopInfo>());
}
std::unique_ptr<ReleaseModeModelRunner<RegallocEvictModel>> Runner;
std::unique_ptr<ReleaseModeModelRunner<CompiledModelType>> Runner;
};
#endif

// ===================================
// Development mode-specifics
Expand Down Expand Up @@ -901,11 +902,9 @@ bool RegAllocScoring::runOnMachineFunction(MachineFunction &MF) {
}
#endif // #ifdef LLVM_HAVE_TF_API

#if defined(LLVM_HAVE_TF_AOT_REGALLOCEVICTMODEL)
RegAllocEvictionAdvisorAnalysis *llvm::createReleaseModeAdvisor() {
return new ReleaseModeEvictionAdvisorAnalysis();
}
#endif

// In all cases except development mode, we don't need scoring.
#if !defined(LLVM_HAVE_TF_API)
Expand Down

2 comments on commit 5a50ab4

@mikaelholmen
Copy link
Collaborator

@mikaelholmen mikaelholmen commented on 5a50ab4 Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Sorry for being a pain, but when compiling without asserts (-DLLVM_ENABLE_ASSERTIONS=OFF) I get a warning with clang8 on this patch:

In file included from ../lib/CodeGen/MLRegallocEvictAdvisor.cpp:20:
../include/llvm/Analysis/ReleaseModeModelRunner.h:82:15: error: private field 'ErrMsg' is not used [-Werror,-Wunused-private-field]
const char *ErrMsg = "The mock AOT-ed saved model is a compile-time stub and "
^
1 error generated.

Seems the llvm_unreachable macro doesn't use the provided string in NDEBUG builds.

Maybe just make ErrMsg public as well?

@mtrofin
Copy link
Member Author

@mtrofin mtrofin commented on 5a50ab4 Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgot llvm_unreachable is itself a macro. Fixed.

Please sign in to comment.