From db7a9ea7f6b2af3c4c1f43ad4aa50cd4eca02921 Mon Sep 17 00:00:00 2001 From: Pedro Liberal Fernandez Date: Wed, 11 Jan 2017 16:10:09 +0000 Subject: Rollback of commit 1da8ac3a8bb0780251976e0dbcfebd3a7ba6a066. -- PiperOrigin-RevId: 144207746 MOS_MIGRATED_REVID=144207746 --- .../build/lib/rules/cpp/CppLinkActionBuilder.java | 102 +++++++-------------- .../build/lib/rules/cpp/CppLinkActionConfigs.java | 31 ------- .../build/lib/rules/cpp/LinkCommandLine.java | 30 +++--- 3 files changed, 45 insertions(+), 118 deletions(-) (limited to 'src/main/java/com') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index e42b6a4d84..f82c35b162 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -80,20 +80,6 @@ public class CppLinkActionBuilder { /** A build variable for flags providing files to link as inputs in the linker invocation */ public static final String LIBRARIES_TO_LINK_VARIABLE = "libraries_to_link"; - /** - * A build variable for thinlto params file produced by thinlto-indexing action and consumed by - * normal linking actions. - */ - public static final String THINLTO_PARAMS_FILE_VARIABLE = "thinlto_params_file"; - - public static final String THINLTO_PREFIX_REPLACE_VARIABLE = "thinlto_prefix_replace"; - - /** - * A build variable for linker param file created by Bazel to overcome the command line length - * limit. - */ - public static final String LINKER_PARAM_FILE_VARIABLE = "linker_param_file"; - /** A build variable for the execpath of the output of the linker. */ public static final String OUTPUT_EXECPATH_VARIABLE = "output_execpath"; @@ -525,7 +511,7 @@ public class CppLinkActionBuilder { } PathFragment linkerParamsFileRootPath = null; - @Nullable Artifact thinltoParamsFile = null; + @Nullable Artifact linkerParamsFile = null; if (allLTOArtifacts != null) { // Create artifact for the file that the LTO indexing step will emit // object file names into for any that were included in the link as @@ -536,7 +522,7 @@ public class CppLinkActionBuilder { // of the native object files not the input bitcode files. linkerParamsFileRootPath = ParameterFile.derivePath(output.getRootRelativePath(), "lto-final"); - thinltoParamsFile = + linkerParamsFile = linkArtifactFactory.create(ruleContext, configuration, linkerParamsFileRootPath); } @@ -546,8 +532,8 @@ public class CppLinkActionBuilder { for (LTOBackendArtifacts ltoA : allLTOArtifacts) { ltoA.addIndexingOutputs(builder); } - if (thinltoParamsFile != null) { - builder.add(thinltoParamsFile); + if (linkerParamsFile != null) { + builder.add(linkerParamsFile); } actionOutputs = builder.build(); } else { @@ -562,16 +548,6 @@ public class CppLinkActionBuilder { ImmutableList runtimeLinkerInputs = ImmutableList.copyOf(LinkerInputs.simpleLinkerInputs(runtimeInputs, runtimeType)); - - PathFragment paramRootPath = - ParameterFile.derivePath(output.getRootRelativePath(), (isLTOIndexing) ? "lto-index" : "2"); - - @Nullable - final Artifact paramFile = - canSplitCommandLine() - ? linkArtifactFactory.create(ruleContext, configuration, paramRootPath) - : null; - // Add build variables necessary to template link args into the crosstool. Variables.Builder buildVariablesBuilder = new Variables.Builder(); CppLinkVariablesExtension variablesExtension = @@ -583,8 +559,7 @@ public class CppLinkActionBuilder { linkerInputs, runtimeLinkerInputs, null, - paramFile, - thinltoParamsFile, + linkerParamsFile, ltoOutputRootPrefix, null, null) @@ -595,8 +570,7 @@ public class CppLinkActionBuilder { linkerInputs, runtimeLinkerInputs, output, - paramFile, - thinltoParamsFile, + linkerParamsFile, PathFragment.EMPTY_FRAGMENT, getInterfaceSoBuilder(), interfaceOutput); @@ -606,6 +580,15 @@ public class CppLinkActionBuilder { } Variables buildVariables = buildVariablesBuilder.build(); + PathFragment paramRootPath = + ParameterFile.derivePath(output.getRootRelativePath(), (isLTOIndexing) ? "lto-index" : "2"); + + @Nullable + final Artifact paramFile = + canSplitCommandLine() + ? linkArtifactFactory.create(ruleContext, configuration, paramRootPath) + : null; + Preconditions.checkArgument( linkType != LinkTargetType.INTERFACE_DYNAMIC_LIBRARY, "you can't link an interface dynamic library directly"); @@ -721,8 +704,8 @@ public class CppLinkActionBuilder { .add(dependencyInputsBuilder.build()) .add(ImmutableIterable.from(expandedInputs)); - if (thinltoParamsFile != null && !isLTOIndexing) { - inputsBuilder.add(ImmutableList.of(thinltoParamsFile)); + if (linkerParamsFile != null && !isLTOIndexing) { + inputsBuilder.add(ImmutableList.of(linkerParamsFile)); } if (linkCommandLine.getParamFile() != null) { inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile())); @@ -1234,6 +1217,7 @@ public class CppLinkActionBuilder { public ImmutableSet getLibrarySearchDirectories() { return librarySearchDirectories; } + } private class CppLinkVariablesExtension implements VariablesExtension { @@ -1246,8 +1230,7 @@ public class CppLinkActionBuilder { private final Artifact outputArtifact; private final Artifact interfaceLibraryBuilder; private final Artifact interfaceLibraryOutput; - private final Artifact paramFile; - private final Artifact thinltoParamsFile; + private final Artifact linkerParamsFile; private final PathFragment ltoOutputRootPrefix; private final LinkArgCollector linkArgCollector = new LinkArgCollector(); @@ -1259,8 +1242,7 @@ public class CppLinkActionBuilder { Iterable linkerInputs, ImmutableList runtimeLinkerInputs, Artifact output, - Artifact paramFile, - Artifact thinltoParamsFile, + Artifact linkerParamsFile, PathFragment ltoOutputRootPrefix, Artifact interfaceLibraryBuilder, Artifact interfaceLibraryOutput) { @@ -1272,8 +1254,7 @@ public class CppLinkActionBuilder { this.outputArtifact = output; this.interfaceLibraryBuilder = interfaceLibraryBuilder; this.interfaceLibraryOutput = interfaceLibraryOutput; - this.paramFile = paramFile; - this.thinltoParamsFile = thinltoParamsFile; + this.linkerParamsFile = linkerParamsFile; this.ltoOutputRootPrefix = ltoOutputRootPrefix; addInputFileLinkOptions(linkArgCollector); @@ -1323,10 +1304,6 @@ public class CppLinkActionBuilder { buildVariables.addCustomBuiltVariable( LIBRARIES_TO_LINK_VARIABLE, linkArgCollector.getLibrariesToLink()); - if (paramFile != null) { - buildVariables.addStringVariable(LINKER_PARAM_FILE_VARIABLE, paramFile.getExecPathString()); - } - buildVariables.addStringSequenceVariable( LIBRARY_SEARCH_DIRECTORIES_VARIABLE, linkArgCollector.getLibrarySearchDirectories()); @@ -1341,30 +1318,18 @@ public class CppLinkActionBuilder { OUTPUT_EXECPATH_VARIABLE, outputArtifact.getExecPathString()); } - if (isLTOIndexing()) { - if (thinltoParamsFile != null) { - // This is a lto-indexing action and we want it to populate params file. - buildVariables.addStringVariable( - THINLTO_PARAMS_FILE_VARIABLE, thinltoParamsFile.getExecPathString()); - // TODO(b/33846234): Remove once all the relevant crosstools don't depend on the variable. + if (!ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) { + if (linkerParamsFile != null) { buildVariables.addStringVariable( - "thinlto_optional_params_file", "=" + thinltoParamsFile.getExecPathString()); + "thinlto_optional_params_file", "=" + linkerParamsFile.getExecPathString()); } else { - buildVariables.addStringVariable(THINLTO_PARAMS_FILE_VARIABLE, ""); - // TODO(b/33846234): Remove once all the relevant crosstools don't depend on the variable. buildVariables.addStringVariable("thinlto_optional_params_file", ""); } buildVariables.addStringVariable( - THINLTO_PREFIX_REPLACE_VARIABLE, + "thinlto_prefix_replace", configuration.getBinDirectory().getExecPathString() + ";" + configuration.getBinDirectory().getExecPath().getRelative(ltoOutputRootPrefix)); - } else { - if (thinltoParamsFile != null) { - // This is a normal link action and we need to use params file created by lto-indexing. - buildVariables.addStringVariable( - THINLTO_PARAMS_FILE_VARIABLE, thinltoParamsFile.getExecPathString()); - } } boolean shouldGenerateInterfaceLibrary = outputArtifact != null @@ -1389,10 +1354,6 @@ public class CppLinkActionBuilder { CppHelper.getFdoSupport(ruleContext).getLinkOptions(featureConfiguration, buildVariables); } - private boolean isLTOIndexing() { - return !ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT); - } - private boolean isSharedNativeLibrary() { return isNativeDeps && cppConfiguration.shareNativeDeps(); } @@ -1551,6 +1512,11 @@ public class CppLinkActionBuilder { addStaticInputLinkOptions(input, librariesToLink, true, ltoMap); } } + if (linkerParamsFile != null && ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) { + librariesToLink.addValue( + new LibraryToLinkValue( + "-Wl,@" + linkerParamsFile.getExecPathString(), needWholeArchive)); + } // rpath ordering matters for performance; first add the one where most libraries are found. if (includeSolibDir && rpathRoot != null) { @@ -1638,13 +1604,13 @@ public class CppLinkActionBuilder { @Nullable Map ltoMap) { Preconditions.checkState(!(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY)); // If we had any LTO artifacts, ltoMap whould be non-null. In that case, - // we should have created a thinltoParamsFile which the LTO indexing + // we should have created a linkerParamsFile which the LTO indexing // step will populate with the exec paths that correspond to the LTO // artifacts that the linker decided to include based on symbol resolution. // Those files will be included directly in the link (and not wrapped // in --start-lib/--end-lib) to ensure consistency between the two link // steps. - Preconditions.checkState(ltoMap == null || thinltoParamsFile != null); + Preconditions.checkState(ltoMap == null || linkerParamsFile != null); // start-lib/end-lib library: adds its input object files. if (Link.useStartEndLib(input, cppConfiguration.archiveType())) { @@ -1654,7 +1620,7 @@ public class CppLinkActionBuilder { for (Artifact member : archiveMembers) { if (ltoMap != null && ltoMap.remove(member) != null) { // The LTO artifacts that should be included in the final link - // are listed in the thinltoParamsFile. When ltoMap is non-null + // are listed in the linkerParamsFile. When ltoMap is non-null // the backend artifact may be missing due to libraries that list .o // files explicitly, or generate .o files from assembler. continue; @@ -1679,7 +1645,7 @@ public class CppLinkActionBuilder { if (ltoMap != null && ltoMap.remove(inputArtifact) != null) { // The LTO artifacts that should be included in the final link - // are listed in the thinltoParamsFile. + // are listed in the linkerParamsFile. return; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java index f94435494c..8b8ae161b9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java @@ -68,7 +68,6 @@ public class CppLinkActionConfigs { " implies: 'libraries_to_link'", " implies: 'force_pic_flags'", " implies: 'legacy_link_flags'", - " implies: 'linker_param_file'", "}", "action_config {", " config_name: 'c++-link-dynamic-library'", @@ -87,7 +86,6 @@ public class CppLinkActionConfigs { " implies: 'input_param_flags'", " implies: 'libraries_to_link'", " implies: 'legacy_link_flags'", - " implies: 'linker_param_file'", "}", "action_config {", " config_name: 'c++-link-static-library'", @@ -99,7 +97,6 @@ public class CppLinkActionConfigs { " implies: 'library_search_directories'", " implies: 'input_param_flags'", " implies: 'libraries_to_link'", - " implies: 'linker_param_file'", "}", "action_config {", " config_name: 'c++-link-alwayslink-static-library'", @@ -111,7 +108,6 @@ public class CppLinkActionConfigs { " implies: 'library_search_directories'", " implies: 'input_param_flags'", " implies: 'libraries_to_link'", - " implies: 'linker_param_file'", "}", "action_config {", " config_name: 'c++-link-pic-static-library'", @@ -123,7 +119,6 @@ public class CppLinkActionConfigs { " implies: 'library_search_directories'", " implies: 'input_param_flags'", " implies: 'libraries_to_link'", - " implies: 'linker_param_file'", "}", "action_config {", " config_name: 'c++-link-alwayslink-pic-static-library'", @@ -135,7 +130,6 @@ public class CppLinkActionConfigs { " implies: 'library_search_directories'", " implies: 'input_param_flags'", " implies: 'libraries_to_link'", - " implies: 'linker_param_file'", "}", "feature {", " name: 'build_interface_libraries'", @@ -328,10 +322,6 @@ public class CppLinkActionConfigs { " flag: '-Wl,--end-lib'", " }", " }", - " flag_group {", - " expand_if_true: 'thinlto_params_file'", - " flag: '-Wl,@%{thinlto_params_file}'", - " }", " }", "}", "feature {", @@ -355,27 +345,6 @@ public class CppLinkActionConfigs { " flag: '%{legacy_link_flags}'", " }", " }", - "}", - "feature {", - " name: 'linker_param_file'", - " flag_set {", - " expand_if_all_available: 'linker_param_file'", - " action: 'c++-link-executable'", - " action: 'c++-link-dynamic-library'", - " flag_group {", - " flag: '-Wl,@%{linker_param_file}'", - " }", - " }", - " flag_set {", - " expand_if_all_available: 'linker_param_file'", - " action: 'c++-link-static-library'", - " action: 'c++-link-alwayslink-static-library'", - " action: 'c++-link-pic-static-library'", - " action: 'c++-link-alwayslink-pic-static-library'", - " flag_group {", - " flag: '@%{linker_param_file}'", - " }", - " }", "}")); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 69d8bb9773..b253826683 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -218,16 +218,19 @@ public final class LinkCommandLine extends CommandLine { * Splits the link command-line into a part to be written to a parameter file, and the remaining * actual command line to be executed (which references the parameter file). Should only be used * if getParamFile() is not null. + * + * @throws IllegalStateException if the command-line cannot be split */ @VisibleForTesting final Pair, List> splitCommandline() { List args = getRawLinkArgv(); if (linkTargetType.staticness() == Staticness.STATIC) { // Ar link commands can also generate huge command lines. - List paramFileArgs = new ArrayList<>(); + List paramFileArgs = args.subList(1, args.size()); List commandlineArgs = new ArrayList<>(); - extractArgumentsForStaticLinkParamsFile(args, commandlineArgs, paramFileArgs); + commandlineArgs.add(args.get(0)); + commandlineArgs.add("@" + paramFile.getExecPath().getPathString()); return Pair.of(commandlineArgs, paramFileArgs); } else { // Gcc link commands tend to generate humongous commandlines for some targets, which may @@ -235,14 +238,17 @@ public final class LinkCommandLine extends CommandLine { // a parameter file and pass any linker options through it. List paramFileArgs = new ArrayList<>(); List commandlineArgs = new ArrayList<>(); - extractArgumentsForDynamicLinkParamsFile(args, commandlineArgs, paramFileArgs); + extractArgumentsForParamFile(args, commandlineArgs, paramFileArgs); + commandlineArgs.add("-Wl,@" + paramFile.getExecPath().getPathString()); return Pair.of(commandlineArgs, paramFileArgs); } } /** * Returns just the .params file portion of the command-line as a {@link CommandLine}. + * + * @throws IllegalStateException if the command-line cannot be split */ CommandLine paramCmdLine() { Preconditions.checkNotNull(paramFile); @@ -255,22 +261,8 @@ public final class LinkCommandLine extends CommandLine { } - public static void extractArgumentsForStaticLinkParamsFile( - List args, List commandlineArgs, List paramFileArgs) { - commandlineArgs.add(args.get(0)); // ar command, must not be moved! - int argsSize = args.size(); - for (int i = 1; i < argsSize; i++) { - String arg = args.get(i); - if (arg.startsWith("@")) { - commandlineArgs.add(arg); // params file, keep it in the command line - } else { - paramFileArgs.add(arg); // the rest goes to the params file - } - } - } - - public static void extractArgumentsForDynamicLinkParamsFile( - List args, List commandlineArgs, List paramFileArgs) { + public static void extractArgumentsForParamFile(List args, List commandlineArgs, + List paramFileArgs) { // Note, that it is not important that all linker arguments are extracted so that // they can be moved into a parameter file, but the vast majority should. commandlineArgs.add(args.get(0)); // gcc command, must not be moved! -- cgit v1.2.3