Skip to content

Commit ae09dd8

Browse files
francisvmfrancisvm
committed
[Remarks][Driver] Error on -foptimization-record-file with multiple -arch options
This adds a check for the usage of -foptimization-record-file with multiple -arch options. This is not permitted since it would require us to rename the file requested by the user to avoid overwriting it for the second cc1 invocation.
1 parent 479e3b8 commit ae09dd8

File tree

3 files changed

+132
-80
lines changed

3 files changed

+132
-80
lines changed

‎clang/docs/ClangCommandLineReference.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1701,7 +1701,7 @@ Emit OpenMP code only for SIMD-based constructs.
17011701

17021702
.. option:: -foptimization-record-file=<arg>
17031703

1704-
Specify the file name of any generated YAML optimization record
1704+
Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch <arch> options.
17051705

17061706
.. option:: -foptimize-sibling-calls, -fno-optimize-sibling-calls
17071707

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 128 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,6 +1415,132 @@ static bool isNoCommonDefault(const llvm::Triple &Triple) {
14151415
}
14161416
}
14171417

1418+
static bool shouldEmitRemarks(const ArgList &Args) {
1419+
// -fsave-optimization-record enables it.
1420+
if (Args.hasFlag(options::OPT_fsave_optimization_record,
1421+
options::OPT_fno_save_optimization_record, false))
1422+
return true;
1423+
1424+
// -fsave-optimization-record=<format> enables it as well.
1425+
if (Args.hasFlag(options::OPT_fsave_optimization_record_EQ,
1426+
options::OPT_fno_save_optimization_record, false))
1427+
return true;
1428+
1429+
// -foptimization-record-file alone enables it too.
1430+
if (Args.hasFlag(options::OPT_foptimization_record_file_EQ,
1431+
options::OPT_fno_save_optimization_record, false))
1432+
return true;
1433+
1434+
// -foptimization-record-passes alone enables it too.
1435+
if (Args.hasFlag(options::OPT_foptimization_record_passes_EQ,
1436+
options::OPT_fno_save_optimization_record, false))
1437+
return true;
1438+
return false;
1439+
}
1440+
1441+
static bool hasMultipleInvocations(const llvm::Triple &Triple,
1442+
const ArgList &Args) {
1443+
// Supported only on Darwin where we invoke the compiler multiple times
1444+
// followed by an invocation to lipo.
1445+
if (!Triple.isOSDarwin())
1446+
return false;
1447+
// If more than one "-arch <arch>" is specified, we're targeting multiple
1448+
// architectures resulting in a fat binary.
1449+
return Args.getAllArgValues(options::OPT_arch).size() > 1;
1450+
}
1451+
1452+
static bool checkRemarksOptions(const Driver &D, const ArgList &Args,
1453+
const llvm::Triple &Triple) {
1454+
// When enabling remarks, we need to error if:
1455+
// * The remark file is specified but we're targeting multiple architectures,
1456+
// which means more than one remark file is being generated.
1457+
bool hasMultipleInvocations = ::hasMultipleInvocations(Triple, Args);
1458+
bool hasExplicitOutputFile =
1459+
Args.getLastArg(options::OPT_foptimization_record_file_EQ);
1460+
if (hasMultipleInvocations && hasExplicitOutputFile) {
1461+
D.Diag(diag::err_drv_invalid_output_with_multiple_archs)
1462+
<< "-foptimization-record-file";
1463+
return false;
1464+
}
1465+
return true;
1466+
}
1467+
1468+
static void renderRemarksOptions(const ArgList &Args, ArgStringList &CmdArgs,
1469+
const llvm::Triple &Triple,
1470+
const InputInfo &Input, const JobAction &JA) {
1471+
CmdArgs.push_back("-opt-record-file");
1472+
1473+
const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
1474+
if (A) {
1475+
CmdArgs.push_back(A->getValue());
1476+
} else {
1477+
bool hasMultipleArchs =
1478+
Triple.isOSDarwin() && // Only supported on Darwin platforms.
1479+
Args.getAllArgValues(options::OPT_arch).size() > 1;
1480+
SmallString<128> F;
1481+
1482+
if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
1483+
if (Arg *FinalOutput = Args.getLastArg(options::OPT_o))
1484+
F = FinalOutput->getValue();
1485+
}
1486+
1487+
if (F.empty()) {
1488+
// Use the input filename.
1489+
F = llvm::sys::path::stem(Input.getBaseInput());
1490+
1491+
// If we're compiling for an offload architecture (i.e. a CUDA device),
1492+
// we need to make the file name for the device compilation different
1493+
// from the host compilation.
1494+
if (!JA.isDeviceOffloading(Action::OFK_None) &&
1495+
!JA.isDeviceOffloading(Action::OFK_Host)) {
1496+
llvm::sys::path::replace_extension(F, "");
1497+
F += Action::GetOffloadingFileNamePrefix(JA.getOffloadingDeviceKind(),
1498+
Triple.normalize());
1499+
F += "-";
1500+
F += JA.getOffloadingArch();
1501+
}
1502+
}
1503+
1504+
// If we're having more than one "-arch", we should name the files
1505+
// differently so that every cc1 invocation writes to a different file.
1506+
// We're doing that by appending "-<arch>" with "<arch>" being the arch
1507+
// name from the triple.
1508+
if (hasMultipleArchs) {
1509+
// First, remember the extension.
1510+
SmallString<64> OldExtension = llvm::sys::path::extension(F);
1511+
// then, remove it.
1512+
llvm::sys::path::replace_extension(F, "");
1513+
// attach -<arch> to it.
1514+
F += "-";
1515+
F += Triple.getArchName();
1516+
// put back the extension.
1517+
llvm::sys::path::replace_extension(F, OldExtension);
1518+
}
1519+
1520+
std::string Extension = "opt.";
1521+
if (const Arg *A =
1522+
Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
1523+
Extension += A->getValue();
1524+
else
1525+
Extension += "yaml";
1526+
1527+
llvm::sys::path::replace_extension(F, Extension);
1528+
CmdArgs.push_back(Args.MakeArgString(F));
1529+
}
1530+
1531+
if (const Arg *A =
1532+
Args.getLastArg(options::OPT_foptimization_record_passes_EQ)) {
1533+
CmdArgs.push_back("-opt-record-passes");
1534+
CmdArgs.push_back(A->getValue());
1535+
}
1536+
1537+
if (const Arg *A =
1538+
Args.getLastArg(options::OPT_fsave_optimization_record_EQ)) {
1539+
CmdArgs.push_back("-opt-record-format");
1540+
CmdArgs.push_back(A->getValue());
1541+
}
1542+
}
1543+
14181544
namespace {
14191545
void RenderARMABI(const llvm::Triple &Triple, const ArgList &Args,
14201546
ArgStringList &CmdArgs) {
@@ -5412,85 +5538,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
54125538
CmdArgs.push_back("-fapple-pragma-pack");
54135539

54145540
// Remarks can be enabled with any of the `-f.*optimization-record.*` flags.
5415-
if (Args.hasFlag(options::OPT_fsave_optimization_record,
5416-
options::OPT_foptimization_record_file_EQ,
5417-
options::OPT_fno_save_optimization_record, false) ||
5418-
Args.hasFlag(options::OPT_fsave_optimization_record_EQ,
5419-
options::OPT_fno_save_optimization_record, false) ||
5420-
Args.hasFlag(options::OPT_foptimization_record_passes_EQ,
5421-
options::OPT_fno_save_optimization_record, false)) {
5422-
CmdArgs.push_back("-opt-record-file");
5423-
5424-
const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
5425-
if (A) {
5426-
CmdArgs.push_back(A->getValue());
5427-
} else {
5428-
bool hasMultipleArchs =
5429-
Triple.isOSDarwin() && // Only supported on Darwin platforms.
5430-
Args.getAllArgValues(options::OPT_arch).size() > 1;
5431-
SmallString<128> F;
5432-
5433-
if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
5434-
if (Arg *FinalOutput = Args.getLastArg(options::OPT_o))
5435-
F = FinalOutput->getValue();
5436-
}
5437-
5438-
if (F.empty()) {
5439-
// Use the input filename.
5440-
F = llvm::sys::path::stem(Input.getBaseInput());
5441-
5442-
// If we're compiling for an offload architecture (i.e. a CUDA device),
5443-
// we need to make the file name for the device compilation different
5444-
// from the host compilation.
5445-
if (!JA.isDeviceOffloading(Action::OFK_None) &&
5446-
!JA.isDeviceOffloading(Action::OFK_Host)) {
5447-
llvm::sys::path::replace_extension(F, "");
5448-
F += Action::GetOffloadingFileNamePrefix(JA.getOffloadingDeviceKind(),
5449-
Triple.normalize());
5450-
F += "-";
5451-
F += JA.getOffloadingArch();
5452-
}
5453-
}
5454-
5455-
// If we're having more than one "-arch", we should name the files
5456-
// differently so that every cc1 invocation writes to a different file.
5457-
// We're doing that by appending "-<arch>" with "<arch>" being the arch
5458-
// name from the triple.
5459-
if (hasMultipleArchs) {
5460-
// First, remember the extension.
5461-
SmallString<64> OldExtension = llvm::sys::path::extension(F);
5462-
// then, remove it.
5463-
llvm::sys::path::replace_extension(F, "");
5464-
// attach -<arch> to it.
5465-
F += "-";
5466-
F += Triple.getArchName();
5467-
// put back the extension.
5468-
llvm::sys::path::replace_extension(F, OldExtension);
5469-
}
5470-
5471-
std::string Extension = "opt.";
5472-
if (const Arg *A =
5473-
Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
5474-
Extension += A->getValue();
5475-
else
5476-
Extension += "yaml";
5477-
5478-
llvm::sys::path::replace_extension(F, Extension);
5479-
CmdArgs.push_back(Args.MakeArgString(F));
5480-
}
5481-
5482-
if (const Arg *A =
5483-
Args.getLastArg(options::OPT_foptimization_record_passes_EQ)) {
5484-
CmdArgs.push_back("-opt-record-passes");
5485-
CmdArgs.push_back(A->getValue());
5486-
}
5487-
5488-
if (const Arg *A =
5489-
Args.getLastArg(options::OPT_fsave_optimization_record_EQ)) {
5490-
CmdArgs.push_back("-opt-record-format");
5491-
CmdArgs.push_back(A->getValue());
5492-
}
5493-
}
5541+
if (shouldEmitRemarks(Args) && checkRemarksOptions(D, Args, Triple))
5542+
renderRemarksOptions(Args, CmdArgs, Triple, Input, JA);
54945543

54955544
bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
54965545
options::OPT_fno_rewrite_imports, false);

‎clang/test/Driver/darwin-opt-record.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// REQUIRES: system-darwin
22

33
// RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
4+
// RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -foptimization-record-file=tmp -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH-ERROR
45
//
56
// CHECK-MULTIPLE-ARCH: "-cc1"
67
// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
78
// CHECK-MULTIPLE-ARCH: "-cc1"
89
// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
10+
//
11+
// CHECK-MULTIPLE-ARCH-ERROR: cannot use '-foptimization-record-file' output with multiple -arch options

0 commit comments

Comments
 (0)