Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Compilation without Collecting Constants #2246

Merged
merged 1 commit into from
Jan 16, 2019
Merged

Allow Compilation without Collecting Constants #2246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/glow/Backends/Backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class Backend {
/// Generate code for input function \param F.
virtual std::unique_ptr<CompiledFunction> compile(Function *F) const = 0;

/// Generate code for input function \param F but do not collect constants.
virtual std::unique_ptr<CompiledFunction>
compileWithoutConstants(Function *F) const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

where is it invoked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently nowhere but it will be called from the Provisioner.


/// Save the bundle for \p F for a later standalone execution
/// in \p outputDir. Make \p networkName the function name for
/// the entry point of the network and prepend all generated
Expand Down
4 changes: 3 additions & 1 deletion include/glow/Backends/BackendUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class RuntimeBundle {
size_t getActivationsSize() const { return activationsMemSize_; }
/// Get pointer to memory block of constants.
uint8_t *getConstants() const { return constants_; }
/// Set pointer to memory block of constants.
void setConstants(uint8_t *constants) { constants_ = constants; }
/// Helper function, gets offset of \p v.
size_t getValueOffset(const Named *v) const;
/// Helper function, gets symbol info for \p v.
Expand All @@ -67,7 +69,7 @@ class RuntimeBundle {
RuntimeBundle() = default;
RuntimeBundle(std::unordered_map<std::string, RuntimeSymbolInfo> &symbolTable,
size_t constWeight, size_t mutableWeight, size_t activations)
: symbolTable_(std::move(symbolTable)),
: symbolTable_(std::move(symbolTable)), constants_(nullptr),
constantWeightVarsMemSize_(constWeight),
mutableWeightVarsMemSize_(mutableWeight),
activationsMemSize_(activations) {}
Expand Down
6 changes: 2 additions & 4 deletions lib/Backends/BackendUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ glow::generateRuntimeBundle(const IRFunction &F,
}
auto activationsMaxSize = activationsAllocator.getMaxMemoryUsage();

runtime::RuntimeBundle info(symbolTable, constantMaxSize, placeholderMaxSize,
activationsMaxSize);
info.collectConstants(&F);
return info;
return runtime::RuntimeBundle(symbolTable, constantMaxSize,
placeholderMaxSize, activationsMaxSize);
}
17 changes: 15 additions & 2 deletions lib/Backends/CPU/CPUBackend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,21 @@ CPUBackend::createIRGen(IRFunction *IR,

std::unique_ptr<CompiledFunction>
CPUBackend::compileIR(std::unique_ptr<IRFunction> IR) const {
auto function = compileIRWithoutConstants(IR.get());
static_cast<CPUFunction *>(function.get())->collectConstants(IR.get());
return function;
}

std::unique_ptr<CompiledFunction>
CPUBackend::compileIRWithoutConstants(IRFunction *IR) const {
AllocationsInfo allocationsInfo;
std::unique_ptr<LLVMIRGen> irgen = createIRGen(IR.get(), allocationsInfo);
std::unique_ptr<LLVMIRGen> irgen = createIRGen(IR, allocationsInfo);
irgen->initTargetMachine(target.empty() ? "" : target.getValue(),
llvm::CodeModel::Model::Large);
irgen->initCodeGen();
// Perform the address assignment for activations and WeightVars.

allocateJITMemory(IR.get(), irgen->getAllocationsInfo());
allocateJITMemory(IR, irgen->getAllocationsInfo());
// Create the jitmain function to be invoked by JIT.
emitJitMain(*irgen);
// Emit the code for the body of the entry function.
Expand All @@ -128,6 +135,12 @@ std::unique_ptr<CompiledFunction> CPUBackend::compile(Function *F) const {
return compileIR(std::move(IR));
}

std::unique_ptr<CompiledFunction>
CPUBackend::compileWithoutConstants(Function *F) const {
auto IR = generateAndOptimizeIR(F, shouldShareBuffers());
return compileIRWithoutConstants(IR.get());
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we come to a conclusion about how collectConstants is going to work for the CPU backend yesterday? Since compileWithoutConstants throws away the IR, we don't have anything to pass to collectConstants...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we decided to make expose collect constants as a method on each compiled function, and keep the bundle_ private. We call collectConstants in CompileIR where we still have the IR.

}

void CPUBackend::save(Function *F, llvm::StringRef outputDir,
llvm::StringRef networkName) const {
std::string tgt = target.empty() ? "" : target.getValue();
Expand Down
6 changes: 6 additions & 0 deletions lib/Backends/CPU/CPUBackend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ class CPUBackend : public BackendUsingGlowIR {
std::unique_ptr<CompiledFunction>
compileIR(std::unique_ptr<IRFunction> IR) const override;

std::unique_ptr<CompiledFunction>
compileIRWithoutConstants(IRFunction *IR) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

vertical space b/w top level declarations.


std::unique_ptr<CompiledFunction> compile(Function *F) const override;

std::unique_ptr<CompiledFunction>
Copy link
Contributor

Choose a reason for hiding this comment

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

vertical space b/w top level declarations.

compileWithoutConstants(Function *F) const override;

void save(Function *F, llvm::StringRef outputDir,
llvm::StringRef networkName) const override;

Expand Down
4 changes: 4 additions & 0 deletions lib/Backends/CPU/CPUFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ void CPUFunction::setupRuns() {
}
}

void CPUFunction::collectConstants(IRFunction *F) {
runtimeBundle_.collectConstants(F);
}

void CPUFunction::beforeRun(const Context &ctx) {
// Copy Placeholders into allocated memory.
for (auto PH : ctx.pairs()) {
Expand Down
4 changes: 4 additions & 0 deletions lib/Backends/CPU/CPUFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class CPUFunction final : public CompiledFunction {
/// Ctor.
CPUFunction(std::unique_ptr<llvm::orc::GlowJIT> JIT,
const runtime::RuntimeBundle &runtimeBundle);

/// Collects constants for runtime.
void collectConstants(IRFunction *F);

/// Allocate Mutable buffers on device this includes Activations and
/// Placeholders.
void setupRuns() override;
Expand Down
14 changes: 14 additions & 0 deletions lib/Backends/Interpreter/Interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,22 @@ std::unique_ptr<CompiledFunction> Interpreter::compile(Function *F) const {
return compileIR(std::move(IR));
}

std::unique_ptr<CompiledFunction>
Interpreter::compileWithoutConstants(Function *F) const {
auto IR = generateAndOptimizeIR(F, shouldShareBuffers());
return compileIRWithoutConstants(std::move(IR));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Blank line between definitions.


std::unique_ptr<CompiledFunction>
Interpreter::compileIR(std::unique_ptr<IRFunction> IR) const {
auto function = compileIRWithoutConstants(std::move(IR));
auto IFunction = static_cast<InterpreterFunction *>(function.get());
IFunction->collectConstants(IFunction->getIR());
return function;
}

std::unique_ptr<CompiledFunction>
Interpreter::compileIRWithoutConstants(std::unique_ptr<IRFunction> IR) const {
MemoryAllocator constantWeightsAllocator("ConstantWeights", 0);
MemoryAllocator placeholderWeightsAllocator("PlaceholderWeights", 0);
MemoryAllocator activationsAllocator("Activations", 0);
Expand Down
6 changes: 6 additions & 0 deletions lib/Backends/Interpreter/Interpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,14 @@ class Interpreter final : public BackendUsingGlowIR {
std::unique_ptr<CompiledFunction>
compileIR(std::unique_ptr<IRFunction> IR) const override;

std::unique_ptr<CompiledFunction>
compileIRWithoutConstants(std::unique_ptr<IRFunction> IR) const;

std::unique_ptr<CompiledFunction> compile(Function *F) const override;

std::unique_ptr<CompiledFunction>
Copy link
Contributor

Choose a reason for hiding this comment

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

More blank lines plz :)

compileWithoutConstants(Function *F) const override;

bool isOpSupported(Kinded::Kind opKind, ElemKind elementTy) const override;

bool shouldLower(const Node *N) const override;
Expand Down
4 changes: 4 additions & 0 deletions lib/Backends/Interpreter/InterpreterFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ InterpreterFunction::~InterpreterFunction() {
tearDownRuns();
}

void InterpreterFunction::collectConstants(IRFunction *F) {
runtimeBundle_.collectConstants(F);
}

void InterpreterFunction::setupRuns() {
if (!runsSetup_) {
if (runtimeBundle_.getConstantWeightSize()) {
Expand Down
6 changes: 6 additions & 0 deletions lib/Backends/Interpreter/InterpreterFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ class InterpreterFunction final : public CompiledFunction {

/// Does any needed initialization work for the Backend, creates tensors from
/// constants.

/// Collects constants for runtime.
void collectConstants(IRFunction *F);

void setupRuns() override;

/// Per run setup, adds references for tensors from \p ctx to
Expand All @@ -76,6 +80,8 @@ class InterpreterFunction final : public CompiledFunction {
void tearDownRuns() override;

void execute() override;
/// Get reference to IR function.
IRFunction *getIR() { return F_.get(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, I think adding a collectConstants would be better than adding getIR just to pass it back to runtime bundle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The challenge here is the CPUFunction, it doesn't keep the IR around so it would still need it passed in.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is a good point. Which makes me wonder, how could we even use collectConstants with the CPU backend? Since compileIR takes ownership of the IR and then destroys it, how can we later pass it to collect? (Right now we only collect inside the compile function, but that's going to change soon... right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the device manager will have a reference to the module so that is where it can collect constants from.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at collectConstants, does it even need to take an IRFunction? It looks like it pulls all the constants from the Module via F->getGraph()->getParent()->getConstants(). It then uses getWeightForNode to turn those back into WeightVars but I don't see a reason for that, since all we do is call getSizeInBytes, which we can get without getting the WeightVar. Am I missing something?

///@}

private:
Expand Down
18 changes: 17 additions & 1 deletion lib/Backends/OpenCL/OpenCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1525,15 +1525,31 @@ cl_mem OpenCLFunction::allocDeviceBuffer(uint64_t size) {

void OpenCLFunction::freeDeviceBuffer(cl_mem buf) { clReleaseMemObject(buf); }

void OpenCLFunction::collectConstants(IRFunction *F) {
runtimeBundle_.collectConstants(F);
}
std::unique_ptr<CompiledFunction>
OCLBackend::compileIR(std::unique_ptr<IRFunction> IR) const {
auto function = compileIRWithoutConstants(std::move(IR));
auto OCLFunction = static_cast<OpenCLFunction *>(function.get());
OCLFunction->collectConstants(OCLFunction->getIR());
return function;
}

std::unique_ptr<CompiledFunction>
OCLBackend::compileIRWithoutConstants(std::unique_ptr<IRFunction> IR) const {
MemoryAllocator allocator("GPU", 0xFFFFFFFF);
runtime::RuntimeBundle bundle =
generateRuntimeBundle(*IR, allocator, allocator, allocator);
return llvm::make_unique<OpenCLFunction>(std::move(IR), bundle);
}

std::unique_ptr<CompiledFunction> OCLBackend::compile(Function *F) const {
auto IR = generateAndOptimizeIR(F, shouldShareBuffers());
return compileIR(std::move(IR));
}

std::unique_ptr<CompiledFunction>
OCLBackend::compileWithoutConstants(Function *F) const {
auto IR = generateAndOptimizeIR(F, shouldShareBuffers());
return compileIRWithoutConstants(std::move(IR));
}
10 changes: 10 additions & 0 deletions lib/Backends/OpenCL/OpenCL.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class OpenCLFunction final : public CompiledFunction {
/// Final cleanup, currently an empty function in OpenCL.
void tearDownRuns() override;

/// Returns IR function pointer.
IRFunction *getIR() { return F_.get(); }

/// Collects constants for runtime.
void collectConstants(IRFunction *F);

private:
/// Copy the value from a device to a provided buffer.
/// \returns number of copied bytes.
Expand Down Expand Up @@ -161,8 +167,12 @@ class OCLBackend final : public BackendUsingGlowIR {

std::unique_ptr<CompiledFunction>
compileIR(std::unique_ptr<IRFunction> IR) const override;
std::unique_ptr<CompiledFunction>
compileIRWithoutConstants(std::unique_ptr<IRFunction> IR) const;

std::unique_ptr<CompiledFunction> compile(Function *F) const override;
std::unique_ptr<CompiledFunction>
compileWithoutConstants(Function *F) const override;

bool transformPostLowering(Function *F, CompilationMode mode) const override;

Expand Down
5 changes: 5 additions & 0 deletions tests/unittests/BackendCorrectnessTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ class MockCPUBackend : public BackendUsingGlowIR {
std::unique_ptr<CompiledFunction> compile(Function *F) const override {
return backend_->compile(F);
}

std::unique_ptr<CompiledFunction>
compileWithoutConstants(Function *F) const override {
return backend_->compile(F);
}
std::unique_ptr<CompiledFunction>
compileIR(std::unique_ptr<IRFunction> IR) const override {
return backend_->compileIR(std::move(IR));
Expand Down
16 changes: 16 additions & 0 deletions tests/unittests/BackendTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ TEST_P(BackendTest, debugPrint) {
function->tearDownRuns();
}

/// Test the compileWithoutConstants method on the backend completes without
/// error.
TEST_P(BackendTest, CompileWithoutConstants) {
Module mod;
Context ctx;
Function *F = mod.createFunction("main");
auto *X = mod.createPlaceholder(ElemKind::FloatTy, {3}, "X", false);
auto *XTensor = ctx.allocate(X);
XTensor->getHandle() = {1., 2., 3.};
auto *pow = F->createPow("Pow1", X, 2.0);
auto *save = F->createSave("save", pow);
ctx.allocate(save->getPlaceholder());
std::unique_ptr<Backend> backend(createBackend(GetParam()));
auto function = backend->compileWithoutConstants(F);
}

/// This test checks that we can compile a function without depending on the
/// graph representation. We compile some function and then delete the function.
/// Later we execute the code and check that things work.
Expand Down
4 changes: 4 additions & 0 deletions tests/unittests/BackendTestUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class MockBackend : public Backend {
std::unique_ptr<CompiledFunction> compile(Function *F) const override {
return llvm::make_unique<MockFunction>();
}
std::unique_ptr<CompiledFunction>
compileWithoutConstants(Function *F) const override {
return llvm::make_unique<MockFunction>();
}
bool isOpSupported(Kinded::Kind opKind, ElemKind elementTy) const override {
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions tests/unittests/QuantizationTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,11 @@ class MockQuantBackend : public Backend {
std::unique_ptr<CompiledFunction> compile(Function *F) const override {
return backend_->compile(F);
}

std::unique_ptr<CompiledFunction>
compileWithoutConstants(Function *F) const override {
return backend_->compile(F);
}
bool isOpSupported(Kinded::Kind opKind, ElemKind elementTy) const override {
if (opKind == Kinded::Kind::SoftMaxNodeKind ||
opKind == Kinded::Kind::LocalResponseNormalizationNodeKind) {
Expand Down