Skip to content

Commit afd27c2

Browse files
lamb-jlamb-j
authored
Revert "[Comgr] Remove silent undbundle path from LINK_BC_TO_BC" (#373)
This reverts commit eb04fc5. We should patch up CLR to forbid passing bundles to LINK_BC_TO_BC, then reapply this
1 parent e63b57a commit afd27c2

File tree

3 files changed

+344
-23
lines changed

3 files changed

+344
-23
lines changed

‎amd/comgr/include/amd_comgr.h.in‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,11 +1634,17 @@ typedef enum amd_comgr_action_kind_s {
16341634
*/
16351635
AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC = 0x2,
16361636
/**
1637-
* Link a collection of bitcodes in @p into a single composite bitcode @p.
1637+
* Link a collection of bitcodes, bundled bitcodes, and bundled bitcode
1638+
* archives in @p into a single composite (unbundled) bitcode @p.
16381639
* Any device library bc data object must be explicitly added to @p input if
16391640
* needed.
16401641
*
1641-
* Return @p AMD_COMGR_STATUS_ERROR if the link fails.
1642+
* Return @p AMD_COMGR_STATUS_ERROR if the link or unbundling fails.
1643+
*
1644+
* Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
1645+
* if IsaName is not set in @p info and does not match the isa name
1646+
* of all bc data objects in @p input, or if the Name field is not set for
1647+
* any DataObject in the input set.
16421648
*/
16431649
AMD_COMGR_ACTION_LINK_BC_TO_BC = 0x3,
16441650
/**

‎amd/comgr/src/comgr-compiler.cpp‎

Lines changed: 263 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,6 +1374,13 @@ amd_comgr_status_t AMDGPUCompiler::unbundle() {
13741374
size_t Index = OutputPrefix.find_last_of(".");
13751375
OutputPrefix = OutputPrefix.substr(0, Index);
13761376

1377+
// TODO: Log Command (see linkBitcodeToBitcode() unbundling)
1378+
if (env::shouldEmitVerboseLogs()) {
1379+
LogS << " Extracting Bundle:\n"
1380+
<< " Input Filename: " << BundlerConfig.InputFileNames[0] << "\n"
1381+
<< " Unbundled Files Extension: ." << FileExtension << "\n";
1382+
}
1383+
13771384
for (StringRef Entry : ActionInfo->BundleEntryIDs) {
13781385
// Add an output file for each target
13791386
SmallString<128> OutputFilePath = OutputDir;
@@ -1463,31 +1470,266 @@ amd_comgr_status_t AMDGPUCompiler::linkBitcodeToBitcode() {
14631470
}
14641471
}
14651472

1466-
if (Input->DataKind != AMD_COMGR_DATA_KIND_BC) {
1467-
continue;
1468-
}
1473+
if (Input->DataKind == AMD_COMGR_DATA_KIND_BC) {
1474+
if (env::shouldEmitVerboseLogs()) {
1475+
LogS << "\t Linking Bitcode: " << InputDir << path::get_separator() << Input->Name
1476+
<< "\n";
1477+
}
14691478

1470-
if (env::shouldEmitVerboseLogs()) {
1471-
LogS << "\t Linking Bitcode: " << InputDir << path::get_separator() << Input->Name
1472-
<< "\n";
1473-
}
1479+
// The data in Input outlives Mod, and the linker destructs Mod after
1480+
// linking it into composite (i.e. ownership is not transferred to the
1481+
// composite) so MemoryBuffer::getMemBuffer is sufficient.
1482+
auto Mod =
1483+
getLazyIRModule(MemoryBuffer::getMemBuffer(
1484+
StringRef(Input->Data, Input->Size), "", false),
1485+
SMDiag, Context, true);
14741486

1475-
// The data in Input outlives Mod, and the linker destructs Mod after
1476-
// linking it into composite (i.e. ownership is not transferred to the
1477-
// composite) so MemoryBuffer::getMemBuffer is sufficient.
1478-
auto Mod =
1479-
getLazyIRModule(MemoryBuffer::getMemBuffer(
1480-
StringRef(Input->Data, Input->Size), "", false),
1481-
SMDiag, Context, true);
1487+
if (!Mod) {
1488+
SMDiag.print(Input->Name, LogS, /* ShowColors */ false);
1489+
return AMD_COMGR_STATUS_ERROR;
1490+
}
1491+
if (verifyModule(*Mod, &LogS))
1492+
return AMD_COMGR_STATUS_ERROR;
1493+
if (L.linkInModule(std::move(Mod), ApplicableFlags))
1494+
return AMD_COMGR_STATUS_ERROR;
1495+
} else if (Input->DataKind == AMD_COMGR_DATA_KIND_BC_BUNDLE) {
1496+
if (env::shouldEmitVerboseLogs()) {
1497+
LogS << " Linking Bundle: " << InputDir << path::get_separator() << Input->Name
1498+
<< "\n";
1499+
}
14821500

1483-
if (!Mod) {
1484-
SMDiag.print(Input->Name, LogS, /* ShowColors */ false);
1485-
return AMD_COMGR_STATUS_ERROR;
1501+
// Determine desired bundle entry ID
1502+
// TODO: Move away from using ActionInfo->IsaName
1503+
// Use ActionInfo->BundleEntryIDs instead
1504+
if (!ActionInfo->IsaName)
1505+
return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
1506+
1507+
std::string IsaName = ActionInfo->IsaName;
1508+
size_t Index = IsaName.find("gfx");
1509+
std::string BundleEntryId =
1510+
"hip-amdgcn-amd-amdhsa--gfx" + IsaName.substr(Index + 3);
1511+
1512+
// Write data to file system so that Offload Bundler can process, assuming
1513+
// we didn't already write due to shouldSaveTemps() conditional above
1514+
// TODO: Switch write to VFS
1515+
if (!env::shouldSaveTemps()) {
1516+
if (auto Status = outputToFile(Input, getFilePath(Input, InputDir))) {
1517+
return Status;
1518+
}
1519+
}
1520+
1521+
// Configure Offload Bundler
1522+
OffloadBundlerConfig BundlerConfig;
1523+
BundlerConfig.AllowMissingBundles = true;
1524+
BundlerConfig.FilesType = "bc";
1525+
1526+
BundlerConfig.TargetNames.push_back(BundleEntryId);
1527+
std::string InputFilePath = getFilePath(Input, InputDir).str().str();
1528+
BundlerConfig.InputFileNames.push_back(InputFilePath);
1529+
1530+
// Generate prefix for output files
1531+
std::string OutputPrefix = std::string(Input->Name);
1532+
Index = OutputPrefix.find_last_of(".");
1533+
OutputPrefix = OutputPrefix.substr(0, Index);
1534+
std::string OutputFileName = OutputPrefix + '-' + BundleEntryId + ".bc";
1535+
1536+
// ISA name may contain ':', which is an invalid character in file names
1537+
// on Windows. Replace with '_'
1538+
std::replace(OutputFileName.begin(), OutputFileName.end(), ':', '_');
1539+
1540+
std::string OutputFilePath = OutputDir.str().str() + path::get_separator().str() + OutputFileName;
1541+
BundlerConfig.OutputFileNames.push_back(OutputFilePath);
1542+
1543+
OffloadBundler Bundler(BundlerConfig);
1544+
1545+
// Execute unbundling
1546+
if (env::shouldEmitVerboseLogs()) {
1547+
LogS << "Extracting Bitcode Bundle:\n"
1548+
<< "\t Bundle Entry ID: " << BundlerConfig.TargetNames[0] << "\n"
1549+
<< "\t Input Filename: " << BundlerConfig.InputFileNames[0]
1550+
<< "\n"
1551+
<< "\t Output Filename: " << BundlerConfig.OutputFileNames[0]
1552+
<< "\n";
1553+
LogS << "\t Command: clang-offload-bundler -unbundle -type=bc"
1554+
" -targets="
1555+
<< BundleEntryId << " -input=" << InputFilePath
1556+
<< " -output=" << OutputFilePath << "\n";
1557+
LogS.flush();
1558+
}
1559+
1560+
llvm::Error Err = Bundler.UnbundleFiles();
1561+
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
1562+
"UnbundleFiles error: ");
1563+
1564+
// Read unbundled bitcode from file system in order to pass to linker
1565+
amd_comgr_data_t ResultT;
1566+
if (auto Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_BC, &ResultT))
1567+
return Status;
1568+
1569+
// ResultT can be released after addition to the data_set
1570+
ScopedDataObjectReleaser SDOR(ResultT);
1571+
1572+
DataObject *Result = DataObject::convert(ResultT);
1573+
if (auto Status = inputFromFile(Result, StringRef(OutputFilePath)))
1574+
return Status;
1575+
1576+
Result->Name = strdup(OutputFileName.c_str());
1577+
1578+
auto Mod =
1579+
getLazyIRModule(MemoryBuffer::getMemBuffer(
1580+
StringRef(Result->Data, Result->Size), "", false),
1581+
SMDiag, Context, true);
1582+
1583+
if (!Mod) {
1584+
SMDiag.print(Result->Name, LogS, /* ShowColors */ false);
1585+
return AMD_COMGR_STATUS_ERROR;
1586+
}
1587+
if (verifyModule(*Mod, &LogS))
1588+
return AMD_COMGR_STATUS_ERROR;
1589+
if (L.linkInModule(std::move(Mod), ApplicableFlags))
1590+
return AMD_COMGR_STATUS_ERROR;
14861591
}
1487-
if (verifyModule(*Mod, &LogS))
1488-
return AMD_COMGR_STATUS_ERROR;
1489-
if (L.linkInModule(std::move(Mod), ApplicableFlags))
1490-
return AMD_COMGR_STATUS_ERROR;
1592+
// Unbundle bitcode archive
1593+
else if (Input->DataKind == AMD_COMGR_DATA_KIND_AR_BUNDLE) {
1594+
if (env::shouldEmitVerboseLogs()) {
1595+
LogS << "\t Linking Archive: " << InputDir << path::get_separator() << Input->Name
1596+
<< "\n";
1597+
}
1598+
1599+
// Determine desired bundle entry ID
1600+
// TODO: Move away from using ActionInfo->IsaName
1601+
// Use ActionInfo->BundleEntryIDs instead
1602+
if (!ActionInfo->IsaName)
1603+
return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
1604+
1605+
std::string IsaName = ActionInfo->IsaName;
1606+
size_t Index = IsaName.find("gfx");
1607+
std::string BundleEntryId =
1608+
"hip-amdgcn-amd-amdhsa--gfx" + IsaName.substr(Index + 3);
1609+
1610+
// Write data to file system so that Offload Bundler can process, assuming
1611+
// we didn't already write due to shouldSaveTemps() conditional above
1612+
// TODO: Switch write to VFS
1613+
if (!env::shouldSaveTemps()) {
1614+
if (auto Status = outputToFile(Input, getFilePath(Input, InputDir))) {
1615+
return Status;
1616+
}
1617+
}
1618+
1619+
// Configure Offload Bundler
1620+
OffloadBundlerConfig BundlerConfig;
1621+
BundlerConfig.AllowMissingBundles = true;
1622+
BundlerConfig.FilesType = "a";
1623+
BundlerConfig.HipOpenmpCompatible = 1;
1624+
BundlerConfig.AllowNoHost = 1;
1625+
1626+
BundlerConfig.TargetNames.push_back(BundleEntryId);
1627+
std::string InputFilePath = getFilePath(Input, InputDir).str().str();
1628+
BundlerConfig.InputFileNames.push_back(InputFilePath);
1629+
1630+
// Generate prefix for output files
1631+
std::string OutputPrefix = std::string(Input->Name);
1632+
Index = OutputPrefix.find_last_of(".");
1633+
OutputPrefix = OutputPrefix.substr(0, Index);
1634+
1635+
std::string OutputFileName = OutputPrefix + '-' + BundleEntryId + ".a";
1636+
1637+
// ISA name may contain ':', which is an invalid character in file names
1638+
// on Windows. Replace with '_'
1639+
std::replace(OutputFileName.begin(), OutputFileName.end(), ':', '_');
1640+
1641+
std::string OutputFilePath = OutputDir.str().str() + path::get_separator().str() + OutputFileName;
1642+
BundlerConfig.OutputFileNames.push_back(OutputFilePath);
1643+
1644+
OffloadBundler Bundler(BundlerConfig);
1645+
1646+
// Execute unbundling
1647+
if (env::shouldEmitVerboseLogs()) {
1648+
LogS << " Extracting Bitcode Archive:\n"
1649+
<< "\t Bundle Entry ID: " << BundlerConfig.TargetNames[0] << "\n"
1650+
<< "\t Input Filename: " << BundlerConfig.InputFileNames[0]
1651+
<< "\n"
1652+
<< "\t Output Filename: " << BundlerConfig.OutputFileNames[0]
1653+
<< "\n";
1654+
LogS << "\t Command: clang-offload-bundler -unbundle -type=a "
1655+
" -targets="
1656+
<< BundleEntryId << " -input=" << InputFilePath
1657+
<< " -output=" << OutputFilePath << "\n";
1658+
LogS.flush();
1659+
}
1660+
llvm::Error Err = Bundler.UnbundleArchive();
1661+
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
1662+
"UnbundleArchive error: ");
1663+
1664+
// Read archive back into Comgr
1665+
amd_comgr_data_t ResultT;
1666+
if (auto Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_AR, &ResultT))
1667+
return Status;
1668+
1669+
// ResultT can be released after addition to the data_set
1670+
ScopedDataObjectReleaser SDOR(ResultT);
1671+
1672+
DataObject *Result = DataObject::convert(ResultT);
1673+
if (auto Status = inputFromFile(Result, StringRef(OutputFilePath)))
1674+
return Status;
1675+
1676+
// Get memory buffer for each bitcode in archive file
1677+
// Modeled after static loadArFile in llvm-link.cpp
1678+
std::string ArchiveName = "comgr.ar";
1679+
llvm::StringRef ArchiveBuf = StringRef(Result->Data, Result->Size);
1680+
auto ArchiveOrError =
1681+
object::Archive::create(MemoryBufferRef(ArchiveBuf, ArchiveName));
1682+
1683+
if (!ArchiveOrError) {
1684+
llvm::logAllUnhandledErrors(ArchiveOrError.takeError(), llvm::errs(),
1685+
"Unpack Archives error: ");
1686+
return AMD_COMGR_STATUS_ERROR;
1687+
}
1688+
1689+
auto Archive = std::move(ArchiveOrError.get());
1690+
1691+
Err = Error::success();
1692+
for (const object::Archive::Child &C : Archive->children(Err)) {
1693+
1694+
// Get child name
1695+
Expected<StringRef> Ename = C.getName();
1696+
if (Error E = Ename.takeError()) {
1697+
errs() << ": ";
1698+
WithColor::error() << " failed to read name of archive member"
1699+
<< ArchiveName << "'\n";
1700+
return AMD_COMGR_STATUS_ERROR;
1701+
}
1702+
std::string ChildName = Ename.get().str();
1703+
1704+
// Get memory buffer
1705+
SMDiagnostic ParseErr;
1706+
Expected<MemoryBufferRef> MemBuf = C.getMemoryBufferRef();
1707+
if (Error E = MemBuf.takeError()) {
1708+
errs() << ": ";
1709+
WithColor::error()
1710+
<< " loading memory for member '"
1711+
<< "' of archive library failed'" << ArchiveName << "'\n";
1712+
return AMD_COMGR_STATUS_ERROR;
1713+
};
1714+
1715+
// Link memory buffer into composite
1716+
auto Mod = getLazyIRModule(MemoryBuffer::getMemBuffer(MemBuf.get()),
1717+
SMDiag, Context, true);
1718+
1719+
if (!Mod) {
1720+
SMDiag.print(ChildName.c_str(), LogS, /* ShowColors */ false);
1721+
return AMD_COMGR_STATUS_ERROR;
1722+
}
1723+
if (verifyModule(*Mod, &LogS))
1724+
return AMD_COMGR_STATUS_ERROR;
1725+
if (L.linkInModule(std::move(Mod), ApplicableFlags))
1726+
return AMD_COMGR_STATUS_ERROR;
1727+
}
1728+
1729+
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
1730+
"Unpack Archives error: ");
1731+
} else
1732+
continue;
14911733
}
14921734

14931735
if (verifyModule(*Composite, &LogS)) {

0 commit comments

Comments
 (0)