Skip to content

Commit

Permalink
List modules explicitly in SegmentRange, rename to SegmentInfo
Browse files Browse the repository at this point in the history 
Summary:
Allows CompilerDriver to pass a list of possibly non-contiguous CommonJS module IDs to IR generation.

Following the cjsModuleOffset -> segmentID change (D23940758) we no long rely on the lowest module ID in a segment to serve as a segment identifier. Here we change `Module::getFunctionsInSegment` slightly to iterate over the given list instead of a contiguous range of IDs.

Reviewed By: avp

Differential Revision: D23941626

fbshipit-source-id: 453677add22af2fa2f60bef2d54d6da14f3b685a
  • Loading branch information
@motiz88 @facebook-github-bot
motiz88 authored and facebook-github-bot committed Oct 8, 2020
1 parent c57fde2 commit 18e834a
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 74 deletions.
25 changes: 12 additions & 13 deletions include/hermes/AST/Context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,14 @@ class Context {
using ResolutionTable = llvh::DenseMap<llvh::StringRef, ResolutionTableEntry>;

/// Represents a range of modules used in a given segment.
struct SegmentRange {
struct SegmentInfo {
/// ID of the segment this range represents.
uint32_t segment;

/// ID of the first module in this segment, inclusive.
uint32_t first;

/// ID of the last module in this segment, inclusive.
uint32_t last;
/// The module IDs to include in this segment. This is a subset, specified
/// in an arbitrary order, of the IDs of modules added to the IR. No two
/// segments may include the same module ID.
std::vector<uint32_t> moduleIDs;
};

private:
Expand Down Expand Up @@ -197,7 +196,7 @@ class Context {
/// on the user's metadata input to the compiler when splitting the bundle.
/// Determines which CJS modules are placed into which segment when splitting
/// the result bundle.
const std::vector<SegmentRange> segmentRanges_;
const std::vector<SegmentInfo> segmentInfos_;

/// The level of debug information we should emit. Defaults to
/// DebugInfoSetting::THROWING.
Expand All @@ -220,22 +219,22 @@ class Context {
CodeGenerationSettings codeGenOpts = CodeGenerationSettings(),
OptimizationSettings optimizationOpts = OptimizationSettings(),
std::unique_ptr<ResolutionTable> resolutionTable = nullptr,
std::vector<SegmentRange> segmentRanges = {})
std::vector<SegmentInfo> segmentInfos = {})
: sm_(sm),
resolutionTable_(std::move(resolutionTable)),
segmentRanges_(std::move(segmentRanges)),
segmentInfos_(std::move(segmentInfos)),
codeGenerationSettings_(std::move(codeGenOpts)),
optimizationSettings_(std::move(optimizationOpts)) {}

explicit Context(
CodeGenerationSettings codeGenOpts = CodeGenerationSettings(),
OptimizationSettings optimizationOpts = OptimizationSettings(),
std::unique_ptr<ResolutionTable> resolutionTable = nullptr,
std::vector<SegmentRange> segmentRanges = {})
std::vector<SegmentInfo> segmentInfos = {})
: ownSm_(new SourceErrorManager()),
sm_(*ownSm_),
resolutionTable_(std::move(resolutionTable)),
segmentRanges_(std::move(segmentRanges)),
segmentInfos_(std::move(segmentInfos)),
codeGenerationSettings_(std::move(codeGenOpts)),
optimizationSettings_(std::move(optimizationOpts)) {}

Expand All @@ -261,8 +260,8 @@ class Context {
}

/// \return the table for static require resolution, nullptr if not supplied.
const std::vector<SegmentRange> &getSegmentRanges() const {
return segmentRanges_;
const std::vector<SegmentInfo> &getSegmentInfos() const {
return segmentInfos_;
}

/// \return the table for static require resolution, nullptr if not supplied.
Expand Down
6 changes: 3 additions & 3 deletions include/hermes/BCGen/HBC/HBC.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ std::unique_ptr<BytecodeModule> generateBytecodeModule(
Module *M,
Function *entryPoint,
const BytecodeGenerationOptions &options,
OptValue<Context::SegmentRange> range = llvh::None,
const Context::SegmentInfo *segmentInfo = nullptr,
SourceMapGenerator *sourceMap = nullptr,
std::unique_ptr<BCProviderBase> baseBCProvider = nullptr);

Expand All @@ -47,7 +47,7 @@ std::unique_ptr<BytecodeModule> generateBytecodeModule(
Function *lexicalTopLevel,
Function *entryPoint,
const BytecodeGenerationOptions &options,
OptValue<Context::SegmentRange> range = llvh::None,
const Context::SegmentInfo *segmentInfo = nullptr,
SourceMapGenerator *sourceMap = nullptr,
std::unique_ptr<BCProviderBase> baseBCProvider = nullptr);

Expand All @@ -63,7 +63,7 @@ std::unique_ptr<BytecodeModule> generateBytecode(
raw_ostream &OS,
const BytecodeGenerationOptions &options,
const SHA1 &sourceHash,
OptValue<Context::SegmentRange> range = llvh::None,
const Context::SegmentInfo *segmentInfo = nullptr,
SourceMapGenerator *sourceMap = nullptr,
std::unique_ptr<BCProviderBase> baseBCProvider = nullptr);
} // namespace hbc
Expand Down
7 changes: 4 additions & 3 deletions include/hermes/IR/IR.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,9 +1913,10 @@ class Module : public Value {
}

/// \return the set of functions which are used by the modules in the segment
/// specified by \p range. Order is unspecified, so the return value should
/// not be used for iteration, only for checking membership.
llvh::DenseSet<Function *> getFunctionsInSegment(Context::SegmentRange range);
/// specified by \p segmentInfo. Order is unspecified, so the return value
/// should not be used for iteration, only for checking membership.
llvh::DenseSet<Function *> getFunctionsInSegment(
const Context::SegmentInfo &segmentInfo);

/// Given a list of raw strings from a template literal, get its unique id.
uint32_t getTemplateObjectID(RawStringList &&rawStrings);
Expand Down
21 changes: 11 additions & 10 deletions lib/BCGen/HBC/HBC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,15 @@ std::unique_ptr<BytecodeModule> hbc::generateBytecodeModule(
Module *M,
Function *entryPoint,
const BytecodeGenerationOptions &options,
OptValue<Context::SegmentRange> range,
const Context::SegmentInfo *segmentInfo,
SourceMapGenerator *sourceMapGen,
std::unique_ptr<BCProviderBase> baseBCProvider) {
return generateBytecodeModule(
M,
entryPoint,
entryPoint,
options,
range,
segmentInfo,
sourceMapGen,
std::move(baseBCProvider));
}
Expand All @@ -159,7 +159,7 @@ std::unique_ptr<BytecodeModule> hbc::generateBytecodeModule(
Function *lexicalTopLevel,
Function *entryPoint,
const BytecodeGenerationOptions &options,
OptValue<Context::SegmentRange> range,
const Context::SegmentInfo *segmentInfo,
SourceMapGenerator *sourceMapGen,
std::unique_ptr<BCProviderBase> baseBCProvider) {
PerfSection perf("Bytecode Generation");
Expand All @@ -170,18 +170,19 @@ std::unique_ptr<BytecodeModule> hbc::generateBytecodeModule(

BytecodeModuleGenerator BMGen(options);

if (range) {
BMGen.setSegmentID(range->segment);
if (segmentInfo) {
BMGen.setSegmentID(segmentInfo->segment);
}

// Empty if all functions should be generated (i.e. bundle splitting was not
// requested).
llvh::DenseSet<Function *> functionsToGenerate =
range ? M->getFunctionsInSegment(*range) : llvh::DenseSet<Function *>{};
llvh::DenseSet<Function *> functionsToGenerate = segmentInfo
? M->getFunctionsInSegment(*segmentInfo)
: llvh::DenseSet<Function *>{};

/// \return true if we should generate function \p f.
std::function<bool(const Function *)> shouldGenerate;
if (range) {
if (segmentInfo) {
shouldGenerate = [entryPoint, &functionsToGenerate](const Function *f) {
return f == entryPoint || functionsToGenerate.count(f) > 0;
};
Expand Down Expand Up @@ -315,14 +316,14 @@ std::unique_ptr<BytecodeModule> hbc::generateBytecode(
raw_ostream &OS,
const BytecodeGenerationOptions &options,
const SHA1 &sourceHash,
OptValue<Context::SegmentRange> range,
const Context::SegmentInfo *segmentInfo,
SourceMapGenerator *sourceMapGen,
std::unique_ptr<BCProviderBase> baseBCProvider) {
auto BM = generateBytecodeModule(
M,
M->getTopLevelFunction(),
options,
range,
segmentInfo,
sourceMapGen,
std::move(baseBCProvider));
if (options.format == OutputFormatKind::EmitBundle) {
Expand Down
67 changes: 33 additions & 34 deletions lib/CompilerDriver/CompilerDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ bool validateFlags() {
/// \return the Context.
std::shared_ptr<Context> createContext(
std::unique_ptr<Context::ResolutionTable> resolutionTable,
std::vector<Context::SegmentRange> segmentRanges) {
std::vector<Context::SegmentInfo> segmentInfos) {
CodeGenerationSettings codeGenOpts;
codeGenOpts.enableTDZ = cl::EnableTDZ;
codeGenOpts.dumpOperandRegisters = cl::DumpOperandRegisters;
Expand Down Expand Up @@ -1009,7 +1009,7 @@ std::shared_ptr<Context> createContext(
codeGenOpts,
optimizationOpts,
std::move(resolutionTable),
std::move(segmentRanges));
std::move(segmentInfos));

// Default is non-strict mode.
context->setStrictMode(!cl::NonStrictMode && cl::StrictMode);
Expand Down Expand Up @@ -1124,7 +1124,7 @@ std::unique_ptr<llvh::MemoryBuffer> getFileFromDirectoryOrZip(
::hermes::parser::JSONObject *readInputFilenamesFromDirectoryOrZip(
llvh::StringRef inputPath,
SegmentTable &fileBufs,
std::vector<Context::SegmentRange> &segmentRanges,
std::vector<Context::SegmentInfo> &segmentInfos,
::hermes::parser::JSLexer::Allocator &alloc,
struct zip_t *zip) {
auto metadataBuf = getFileFromDirectoryOrZip(zip, inputPath, "metadata.json");
Expand Down Expand Up @@ -1159,8 +1159,8 @@ ::hermes::parser::JSONObject *readInputFilenamesFromDirectoryOrZip(
uint32_t moduleIdx = 0;

for (auto it : *segments) {
Context::SegmentRange range;
if (it.first->str().getAsInteger(10, range.segment)) {
Context::SegmentInfo segmentInfo;
if (it.first->str().getAsInteger(10, segmentInfo.segment)) {
// getAsInteger returns true to signal error.
llvh::errs()
<< "Metadata segment indices must be unsigned integers: Found "
Expand All @@ -1174,8 +1174,6 @@ ::hermes::parser::JSONObject *readInputFilenamesFromDirectoryOrZip(
return nullptr;
}

range.first = moduleIdx;

SegmentTableEntry segmentBufs{};
for (auto val : *segment) {
auto *relPath = llvh::dyn_cast_or_null<parser::JSONString>(val);
Expand All @@ -1190,20 +1188,20 @@ ::hermes::parser::JSONObject *readInputFilenamesFromDirectoryOrZip(
auto mapBuf = getFileFromDirectoryOrZip(
zip, inputPath, llvh::Twine(relPath->str(), ".map"), true);
// mapBuf is optional, so simply pass it through if it's null.
segmentBufs.push_back(
{moduleIdx++, std::move(fileBuf), std::move(mapBuf)});
auto moduleID = moduleIdx++;
segmentInfo.moduleIDs.push_back(moduleID);
segmentBufs.push_back({moduleID, std::move(fileBuf), std::move(mapBuf)});
}

range.last = moduleIdx - 1;

auto emplaceRes = fileBufs.emplace(range.segment, std::move(segmentBufs));
auto emplaceRes =
fileBufs.emplace(segmentInfo.segment, std::move(segmentBufs));
if (!emplaceRes.second) {
llvh::errs() << "Duplicate segment entry in metadata: " << range.segment
<< "\n";
llvh::errs() << "Duplicate segment entry in metadata: "
<< segmentInfo.segment << "\n";
return nullptr;
}

segmentRanges.push_back(std::move(range));
segmentInfos.push_back(std::move(segmentInfo));
}

return metadata;
Expand Down Expand Up @@ -1571,13 +1569,13 @@ CompileResult generateBytecodeForSerialization(
Module &M,
const BytecodeGenerationOptions &genOptions,
const SHA1 &sourceHash,
OptValue<Context::SegmentRange> range,
const Context::SegmentInfo *segmentInfo,
SourceMapGenerator *sourceMapGenOrNull,
BaseBytecodeMap &baseBytecodeMap) {
// Serialize the bytecode to the file.
if (cl::BytecodeFormat == cl::BytecodeFormatKind::HBC) {
std::unique_ptr<hbc::BCProviderFromBuffer> baseBCProvider = nullptr;
auto itr = baseBytecodeMap.find(range ? range->segment : 0);
auto itr = baseBytecodeMap.find(segmentInfo ? segmentInfo->segment : 0);
if (itr != baseBytecodeMap.end()) {
baseBCProvider = std::move(itr->second);
// We want to erase it from the map because unique_ptr can only
Expand All @@ -1589,7 +1587,7 @@ CompileResult generateBytecodeForSerialization(
OS,
genOptions,
sourceHash,
range,
segmentInfo,
sourceMapGenOrNull,
std::move(baseBCProvider));

Expand Down Expand Up @@ -1818,7 +1816,7 @@ CompileResult processSourceFiles(

CompileResult result{Success};
StringRef base = cl::BytecodeOutputFilename;
if (context->getSegmentRanges().size() < 2) {
if (context->getSegmentInfos().size() < 2) {
OutputStream fileOS{llvh::outs()};
if (!base.empty() && !fileOS.open(base, F_None)) {
return OutputFileError;
Expand All @@ -1828,7 +1826,7 @@ CompileResult processSourceFiles(
M,
genOptions,
sourceHash,
llvh::None,
nullptr,
sourceMapGen ? sourceMapGen.getPointer() : nullptr,
baseBytecodeMap);
if (result.status != Success) {
Expand All @@ -1847,12 +1845,13 @@ CompileResult processSourceFiles(
JSONEmitter manifest{manifestOS.os(), /* pretty */ true};
manifest.openArray();

for (const auto &range : context->getSegmentRanges()) {
for (const auto &segmentInfo : context->getSegmentInfos()) {
std::string filename = base.str();
if (range.segment != 0) {
filename += "." + oscompat::to_string(range.segment);
if (segmentInfo.segment != 0) {
filename += "." + oscompat::to_string(segmentInfo.segment);
}
std::string flavor = "hbc-seg-" + oscompat::to_string(range.segment);
std::string flavor =
"hbc-seg-" + oscompat::to_string(segmentInfo.segment);

OutputStream fileOS{llvh::outs()};
if (!base.empty() && !fileOS.open(filename, F_None)) {
Expand All @@ -1863,7 +1862,7 @@ CompileResult processSourceFiles(
M,
genOptions,
sourceHash,
range,
&segmentInfo,
sourceMapGen ? sourceMapGen.getPointer() : nullptr,
baseBytecodeMap);
if (segResult.status != Success) {
Expand Down Expand Up @@ -1961,15 +1960,15 @@ CompileResult compileFromCommandLineOptions() {
std::unique_ptr<Context::ResolutionTable> resolutionTable = nullptr;

// Segment table in metadata.
std::vector<Context::SegmentRange> segmentRanges;
std::vector<Context::SegmentInfo> segmentInfos;

// Attempt to open the first file as a Zip file.
struct zip_t *zip = zip_open(cl::InputFilenames[0].data(), 0, 'r');

if (llvh::sys::fs::is_directory(cl::InputFilenames[0]) || zip) {
::hermes::parser::JSONObject *metadata =
readInputFilenamesFromDirectoryOrZip(
cl::InputFilenames[0], fileBufs, segmentRanges, metadataAlloc, zip);
cl::InputFilenames[0], fileBufs, segmentInfos, metadataAlloc, zip);

if (zip) {
zip_close(zip);
Expand All @@ -1981,19 +1980,19 @@ CompileResult compileFromCommandLineOptions() {
resolutionTable = readResolutionTable(metadata);
} else {
// If we aren't reading from a dir or a zip, we have only one segment.
Context::SegmentRange range;
range.first = 0;
range.last = cl::InputFilenames.size();
range.segment = 0;
segmentRanges.push_back(std::move(range));
Context::SegmentInfo segmentInfo;
segmentInfo.segment = 0;
segmentInfos.push_back(std::move(segmentInfo));

uint32_t id = 0;
SegmentTableEntry entry{};
for (const std::string &filename : cl::InputFilenames) {
auto fileBuf = memoryBufferFromFile(filename, true);
if (!fileBuf)
return InputFileError;
entry.push_back({id++, std::move(fileBuf), nullptr});
auto moduleID = id++;
segmentInfo.moduleIDs.push_back(moduleID);
entry.push_back({moduleID, std::move(fileBuf), nullptr});
}

// Read input source map if available.
Expand All @@ -2020,7 +2019,7 @@ CompileResult compileFromCommandLineOptions() {
return processBytecodeFile(std::move(fileBufs[0][0].file));
} else {
std::shared_ptr<Context> context =
createContext(std::move(resolutionTable), std::move(segmentRanges));
createContext(std::move(resolutionTable), std::move(segmentInfos));
return processSourceFiles(context, std::move(fileBufs));
}
}
Expand Down
8 changes: 4 additions & 4 deletions lib/IR/IR.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ void Module::populateCJSModuleUseGraph() {
}

llvh::DenseSet<Function *> Module::getFunctionsInSegment(
Context::SegmentRange range) {
const Context::SegmentInfo &segmentInfo) {
populateCJSModuleUseGraph();

// Final set of functions which must be output when generating this segment.
Expand All @@ -590,9 +590,9 @@ llvh::DenseSet<Function *> Module::getFunctionsInSegment(
llvh::SetVector<Function *> worklist{};

// Populate the worklist initially with the wrapper functions for each module
// in the given range.
for (auto i = range.first; i <= range.last; ++i) {
worklist.insert(cjsModules_[i].function);
// in the given segment.
for (const auto &moduleID : segmentInfo.moduleIDs) {
worklist.insert(cjsModules_[moduleID].function);
}

while (!worklist.empty()) {
Expand Down
Loading

0 comments on commit 18e834a

Please sign in to comment.