Skip to content

Commit

Permalink
Fix ordering of buffer loads when threading is used
Browse files Browse the repository at this point in the history
  • Loading branch information
@MikePopoloski
MikePopoloski committed Aug 1, 2023
1 parent 7258ad1 commit 4d84bc9
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 77 deletions.
14 changes: 10 additions & 4 deletions include/slang/driver/SourceLoader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,21 @@ class SLANG_EXPORT SourceLoader {
library(library), libraryRank(libraryRank), isLibraryFile(isLibraryFile) {}
};

// The result of a loadAndParse call.
// 0: A parsed syntax tree
// 1: A loaded source buffer + bool that indicates whether it's a library
// 2: A file entry + error code if the load fails
using LoadResult =
std::variant<std::shared_ptr<syntax::SyntaxTree>, std::pair<SourceBuffer, bool>,
std::pair<const FileEntry*, std::error_code>>;

const SourceLibrary* getOrAddLibrary(std::string_view name);
void addFilesInternal(std::string_view pattern, const std::filesystem::path& basePath,
bool isLibraryFile, const SourceLibrary* library, bool expandEnvVars);
void createLibrary(const syntax::LibraryDeclarationSyntax& syntax,
const std::filesystem::path& basePath);
void loadAndParse(const FileEntry& fileEntry, const Bag& optionBag,
const SourceOptions& srcOptions, std::vector<SourceBuffer>& singleUnitBuffers,
std::vector<SourceBuffer>& deferredLibBuffers, SyntaxTreeList& syntaxTrees,
std::mutex* errorMutex);
LoadResult loadAndParse(const FileEntry& fileEntry, const Bag& optionBag,
const SourceOptions& srcOptions);

SourceManager& sourceManager;

Expand Down
133 changes: 61 additions & 72 deletions source/driver/SourceLoader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,38 @@ std::vector<SourceBuffer> SourceLoader::loadSources() {

SourceLoader::SyntaxTreeList SourceLoader::loadAndParseSources(const Bag& optionBag) {
SyntaxTreeList syntaxTrees;
std::vector<SourceBuffer> singleUnitBuffers;
std::vector<SourceBuffer> deferredLibBuffers;
std::span<const DefineDirectiveSyntax* const> inheritedMacros;

const size_t fileEntryCount = fileEntries.size();
syntaxTrees.reserve(fileEntryCount);
singleUnitBuffers.reserve(fileEntryCount);
deferredLibBuffers.reserve(fileEntryCount);

auto srcOptions = optionBag.getOrDefault<SourceOptions>();

auto handleLoadResult = [&](LoadResult&& result) {
switch (result.index()) {
case 0:
syntaxTrees.emplace_back(std::get<0>(std::move(result)));
break;
case 1: {
auto [buffer, isDeferredLib] = std::get<1>(result);
if (isDeferredLib)
deferredLibBuffers.push_back(buffer);
else
singleUnitBuffers.push_back(buffer);
break;
}
case 2: {
auto [entry, code] = std::get<2>(result);
errors.emplace_back(entry->path, code);
break;
}
}
};

auto parseSingleUnit = [&](std::span<const SourceBuffer> buffers) {
// If we waited to parse direct buffers due to wanting a single unit, parse that unit now.
if (!buffers.empty()) {
Expand All @@ -150,78 +178,50 @@ SourceLoader::SyntaxTreeList SourceLoader::loadAndParseSources(const Bag& option
// the use of threads, do the parsing via a thread pool.
ThreadPool threadPool(srcOptions.numThreads.value_or(0u));

std::vector<SourceBuffer> singleUnitBuffers;
std::vector<SourceBuffer> deferredLibBuffers;
std::mutex mut;
std::vector<LoadResult> loadResults;
loadResults.resize(fileEntries.size());

// Load all source files that were specified on the command line
// or via library maps.
threadPool.pushLoop(size_t(0), fileEntries.size(), [&](size_t start, size_t end) {
SyntaxTreeList localTrees;
std::vector<SourceBuffer> localSingleUnitBufs;
std::vector<SourceBuffer> localDeferredLibBufs;

const size_t count = end - start;
localTrees.reserve(count);
localSingleUnitBufs.reserve(count);
localDeferredLibBufs.reserve(count);

for (size_t i = start; i < end; i++) {
auto& entry = fileEntries[i];
loadAndParse(entry, optionBag, srcOptions, localSingleUnitBufs,
localDeferredLibBufs, localTrees, &mut);
}

// Merge our local results into the shared lists.
std::unique_lock lock(mut);
syntaxTrees.insert(syntaxTrees.end(), localTrees.begin(), localTrees.end());
singleUnitBuffers.insert(singleUnitBuffers.end(), localSingleUnitBufs.begin(),
localSingleUnitBufs.end());
deferredLibBuffers.insert(deferredLibBuffers.end(), localDeferredLibBufs.begin(),
localDeferredLibBufs.end());
for (size_t i = start; i < end; i++)
loadResults[i] = loadAndParse(fileEntries[i], optionBag, srcOptions);
});
threadPool.waitForAll();

for (auto&& result : loadResults)
handleLoadResult(std::move(result));

parseSingleUnit(singleUnitBuffers);

// If we deferred libraries due to wanting to inherit macros, parse them now.
threadPool.pushLoop(size_t(0), deferredLibBuffers.size(), [&](size_t start, size_t end) {
SyntaxTreeList localTrees;
localTrees.reserve(end - start);

for (size_t i = start; i < end; i++) {
auto tree = SyntaxTree::fromBuffer(deferredLibBuffers[i], sourceManager, optionBag,
inheritedMacros);
tree->isLibrary = true;
localTrees.emplace_back(std::move(tree));
}

std::unique_lock lock(mut);
syntaxTrees.insert(syntaxTrees.end(), localTrees.begin(), localTrees.end());
});
threadPool.waitForAll();
if (!deferredLibBuffers.empty()) {
const size_t numTrees = syntaxTrees.size();
syntaxTrees.resize(numTrees + deferredLibBuffers.size());

threadPool.pushLoop(size_t(0), deferredLibBuffers.size(),
[&](size_t start, size_t end) {
for (size_t i = start; i < end; i++) {
auto tree = SyntaxTree::fromBuffer(deferredLibBuffers[i],
sourceManager, optionBag,
inheritedMacros);
tree->isLibrary = true;
syntaxTrees[i + numTrees] = std::move(tree);
}
});
threadPool.waitForAll();
}
}
else {
std::vector<SourceBuffer> singleUnitBuffers;
std::vector<SourceBuffer> deferredLibBuffers;

const size_t count = fileEntries.size();
syntaxTrees.reserve(count);
singleUnitBuffers.reserve(count);
deferredLibBuffers.reserve(count);

// Load all source files that were specified on the command line
// or via library maps.
for (auto& entry : fileEntries) {
loadAndParse(entry, optionBag, srcOptions, singleUnitBuffers, deferredLibBuffers,
syntaxTrees, nullptr);
}
for (auto& entry : fileEntries)
handleLoadResult(loadAndParse(entry, optionBag, srcOptions));

parseSingleUnit(singleUnitBuffers);

// If we deferred libraries due to wanting to inherit macros, parse them now.
if (!deferredLibBuffers.empty()) {
syntaxTrees.reserve(syntaxTrees.size() + deferredLibBuffers.size());
for (auto& buffer : deferredLibBuffers) {
auto tree = SyntaxTree::fromBuffer(buffer, sourceManager, optionBag,
inheritedMacros);
Expand Down Expand Up @@ -404,44 +404,33 @@ void SourceLoader::createLibrary(const LibraryDeclarationSyntax& syntax, const f
// TODO: incdirs
}

void SourceLoader::loadAndParse(const FileEntry& entry, const Bag& optionBag,
const SourceOptions& srcOptions,
std::vector<SourceBuffer>& singleUnitBuffers,
std::vector<SourceBuffer>& deferredLibBuffers,
SyntaxTreeList& syntaxTrees, std::mutex* errorMutex) {
SourceLoader::LoadResult SourceLoader::loadAndParse(const FileEntry& entry, const Bag& optionBag,
const SourceOptions& srcOptions) {
// TODO: error if secondLib is set

auto buffer = sourceManager.readSource(entry.path, entry.library);
if (!buffer) {
// If a mutex is provided we need to lock it before adding the error.
std::unique_lock<std::mutex> lock;
if (errorMutex)
lock = std::unique_lock(*errorMutex);

errors.emplace_back(entry.path, buffer.error());
return;
}
if (!buffer)
return std::pair{&entry, buffer.error()};

if (!entry.isLibraryFile && srcOptions.singleUnit) {
// If this file was directly specified (i.e. not via
// a library mapping) and we're in single-unit mode,
// collect it for later parsing.
singleUnitBuffers.push_back(*buffer);
return std::pair{*buffer, false};
}
else if (srcOptions.librariesInheritMacros) {
// If libraries inherit macros then we can't parse here,
// we need to wait for the main compilation unit to be
// parsed.
// we need to wait for the main compilation unit to be parsed.
SLANG_ASSERT(entry.isLibraryFile);
deferredLibBuffers.push_back(*buffer);
return std::pair{*buffer, true};
}
else {
// Otherwise we can parse right away.
auto tree = SyntaxTree::fromBuffer(*buffer, sourceManager, optionBag);
if (entry.isLibraryFile || srcOptions.onlyLint)
tree->isLibrary = true;

syntaxTrees.emplace_back(std::move(tree));
return std::move(tree);
}
};

Expand Down
4 changes: 3 additions & 1 deletion tests/unittests/DriverTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ TEST_CASE("Driver parsing multiple input files") {
Driver driver;
driver.addStandardArgs();

auto args = fmt::format("testfoo \"{0}test?.sv\"", findTestDir());
auto args = fmt::format("testfoo \"{0}test?.sv\" --single-unit --libraries-inherit-macros -v "
"\"{0}library/libmod.qv\"",
findTestDir());
CHECK(driver.parseCommandLine(args));
CHECK(driver.processOptions());
CHECK(driver.parseAllSources());
Expand Down

0 comments on commit 4d84bc9

Please sign in to comment.