From 7071a05c9320b1d4a5f2340630fd3c6f15c7593c Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 29 Jul 2022 15:58:33 -0700 Subject: [PATCH] bp2build support for .asm files Bug: 233948256 Test: ./build/bazel/ci/bp2build.sh Change-Id: I387c2aeb36df004f0e1838a08a4f28b38503d6ed --- android/allowlists/allowlists.go | 13 +- bp2build/cc_yasm_conversion_test.go | 179 ++++++++++++++++++++++++++++ cc/bp2build.go | 63 +++++++++- 3 files changed, 249 insertions(+), 6 deletions(-) create mode 100644 bp2build/cc_yasm_conversion_test.go diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index 5b440b950..79d4910cb 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -132,6 +132,7 @@ var ( "external/libevent": Bp2BuildDefaultTrueRecursively, "external/libgav1": Bp2BuildDefaultTrueRecursively, "external/libhevc": Bp2BuildDefaultTrueRecursively, + "external/libjpeg-turbo": Bp2BuildDefaultTrueRecursively, "external/libmpeg2": Bp2BuildDefaultTrueRecursively, "external/libpng": Bp2BuildDefaultTrueRecursively, "external/lz4/lib": Bp2BuildDefaultTrue, @@ -263,6 +264,7 @@ var ( "prebuilts/gcc":/* recursive = */ true, "prebuilts/build-tools":/* recursive = */ true, "prebuilts/jdk/jdk11":/* recursive = */ false, + "prebuilts/misc":/* recursive = */ false, // not recursive because we need bp2build converted build files in prebuilts/misc/common/asm "prebuilts/sdk":/* recursive = */ false, "prebuilts/sdk/current/extras/app-toolkit":/* recursive = */ false, "prebuilts/sdk/current/support":/* recursive = */ false, @@ -378,15 +380,16 @@ var ( "gen-kotlin-build-file.py", // TODO(b/198619163) module has same name as source "libgtest_ndk_c++", "libgtest_main_ndk_c++", // TODO(b/201816222): Requires sdk_version support. "linkerconfig", "mdnsd", // TODO(b/202876379): has arch-variant static_executable - "linker", // TODO(b/228316882): cc_binary uses link_crt - "libdebuggerd", // TODO(b/228314770): support product variable-specific header_libs - "versioner", // TODO(b/228313961): depends on prebuilt shared library libclang-cpp_host as a shared library, which does not supply expected providers for a shared library - "libspeexresampler", // TODO(b/231995978): Filter out unknown cflags - "libjpeg", "libvpx", // TODO(b/233948256): Convert .asm files + "linker", // TODO(b/228316882): cc_binary uses link_crt + "libdebuggerd", // TODO(b/228314770): support product variable-specific header_libs + "versioner", // TODO(b/228313961): depends on prebuilt shared library libclang-cpp_host as a shared library, which does not supply expected providers for a shared library + "libspeexresampler", // TODO(b/231995978): Filter out unknown cflags + "libvpx", // TODO(b/240756936): Arm neon variant not supported "art_libartbase_headers", // TODO(b/236268577): Header libraries do not support export_shared_libs_headers "apexer_test", // Requires aapt2 "apexer_test_host_tools", "host_apex_verifier", + "tjbench", // TODO(b/240563612): Stem property // java bugs "libbase_ndk", // TODO(b/186826477): fails to link libctscamera2_jni for device (required for CtsCameraTestCases) diff --git a/bp2build/cc_yasm_conversion_test.go b/bp2build/cc_yasm_conversion_test.go new file mode 100644 index 000000000..6114a4933 --- /dev/null +++ b/bp2build/cc_yasm_conversion_test.go @@ -0,0 +1,179 @@ +// Copyright 2022 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package bp2build + +import ( + "testing" + + "android/soong/android" + "android/soong/cc" +) + +func runYasmTestCase(t *testing.T, tc bp2buildTestCase) { + t.Helper() + runBp2BuildTestCase(t, registerYasmModuleTypes, tc) +} + +func registerYasmModuleTypes(ctx android.RegistrationContext) { + cc.RegisterCCBuildComponents(ctx) + ctx.RegisterModuleType("filegroup", android.FileGroupFactory) + ctx.RegisterModuleType("cc_library_static", cc.LibraryStaticFactory) + ctx.RegisterModuleType("cc_prebuilt_library_static", cc.PrebuiltStaticLibraryFactory) + ctx.RegisterModuleType("cc_library_headers", cc.LibraryHeaderFactory) +} + +func TestYasmSimple(t *testing.T) { + runYasmTestCase(t, bp2buildTestCase{ + description: "Simple yasm test", + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + filesystem: map[string]string{ + "main.cpp": "", + "myfile.asm": "", + }, + blueprint: ` +cc_library { + name: "foo", + srcs: ["main.cpp", "myfile.asm"], +}`, + expectedBazelTargets: append([]string{ + makeBazelTarget("yasm", "foo_yasm", map[string]string{ + "include_dirs": `["."]`, + "srcs": `["myfile.asm"]`, + }), + }, makeCcLibraryTargets("foo", map[string]string{ + "local_includes": `["."]`, + "srcs": `[ + "main.cpp", + ":foo_yasm", + ]`, + })...), + }) +} + +func TestYasmWithIncludeDirs(t *testing.T) { + runYasmTestCase(t, bp2buildTestCase{ + description: "Simple yasm test", + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + filesystem: map[string]string{ + "main.cpp": "", + "myfile.asm": "", + "include1/foo/myinclude.inc": "", + "include2/foo/myinclude2.inc": "", + }, + blueprint: ` +cc_library { + name: "foo", + local_include_dirs: ["include1/foo"], + export_include_dirs: ["include2/foo"], + srcs: ["main.cpp", "myfile.asm"], +}`, + expectedBazelTargets: append([]string{ + makeBazelTarget("yasm", "foo_yasm", map[string]string{ + "include_dirs": `[ + "include1/foo", + ".", + "include2/foo", + ]`, + "srcs": `["myfile.asm"]`, + }), + }, makeCcLibraryTargets("foo", map[string]string{ + "local_includes": `[ + "include1/foo", + ".", + ]`, + "export_includes": `["include2/foo"]`, + "srcs": `[ + "main.cpp", + ":foo_yasm", + ]`, + })...), + }) +} + +func TestYasmConditionalBasedOnArch(t *testing.T) { + runYasmTestCase(t, bp2buildTestCase{ + description: "Simple yasm test", + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + filesystem: map[string]string{ + "main.cpp": "", + "myfile.asm": "", + }, + blueprint: ` +cc_library { + name: "foo", + srcs: ["main.cpp"], + arch: { + x86: { + srcs: ["myfile.asm"], + }, + }, +}`, + expectedBazelTargets: append([]string{ + makeBazelTarget("yasm", "foo_yasm", map[string]string{ + "include_dirs": `["."]`, + "srcs": `select({ + "//build/bazel/platforms/arch:x86": ["myfile.asm"], + "//conditions:default": [], + })`, + }), + }, makeCcLibraryTargets("foo", map[string]string{ + "local_includes": `["."]`, + "srcs": `["main.cpp"] + select({ + "//build/bazel/platforms/arch:x86": [":foo_yasm"], + "//conditions:default": [], + })`, + })...), + }) +} + +func TestYasmPartiallyConditional(t *testing.T) { + runYasmTestCase(t, bp2buildTestCase{ + description: "Simple yasm test", + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + filesystem: map[string]string{ + "main.cpp": "", + "myfile.asm": "", + "mysecondfile.asm": "", + }, + blueprint: ` +cc_library { + name: "foo", + srcs: ["main.cpp", "myfile.asm"], + arch: { + x86: { + srcs: ["mysecondfile.asm"], + }, + }, +}`, + expectedBazelTargets: append([]string{ + makeBazelTarget("yasm", "foo_yasm", map[string]string{ + "include_dirs": `["."]`, + "srcs": `["myfile.asm"] + select({ + "//build/bazel/platforms/arch:x86": ["mysecondfile.asm"], + "//conditions:default": [], + })`, + }), + }, makeCcLibraryTargets("foo", map[string]string{ + "local_includes": `["."]`, + "srcs": `[ + "main.cpp", + ":foo_yasm", + ]`, + })...), + }) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index 6cd67330f..61a55ee69 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -30,6 +30,7 @@ import ( const ( cSrcPartition = "c" asSrcPartition = "as" + asmSrcPartition = "asm" lSrcPartition = "l" llSrcPartition = "ll" cppSrcPartition = "cpp" @@ -81,6 +82,7 @@ func groupSrcsByExtension(ctx android.BazelConversionPathContext, srcs bazel.Lab protoSrcPartition: android.ProtoSrcLabelPartition, cSrcPartition: bazel.LabelPartition{Extensions: []string{".c"}, LabelMapper: addSuffixForFilegroup("_c_srcs")}, asSrcPartition: bazel.LabelPartition{Extensions: []string{".s", ".S"}, LabelMapper: addSuffixForFilegroup("_as_srcs")}, + asmSrcPartition: bazel.LabelPartition{Extensions: []string{".asm"}}, // TODO(http://b/231968910): If there is ever a filegroup target that // contains .l or .ll files we will need to find a way to add a // LabelMapper for these that identifies these filegroups and @@ -289,6 +291,7 @@ type compilerAttributes struct { // Assembly options and sources asFlags bazel.StringListAttribute asSrcs bazel.LabelListAttribute + asmSrcs bazel.LabelListAttribute // C options and sources conlyFlags bazel.StringListAttribute cSrcs bazel.LabelListAttribute @@ -447,6 +450,7 @@ func (ca *compilerAttributes) finalize(ctx android.BazelConversionPathContext, i ca.srcs = partitionedSrcs[cppSrcPartition] ca.cSrcs = partitionedSrcs[cSrcPartition] ca.asSrcs = partitionedSrcs[asSrcPartition] + ca.asmSrcs = partitionedSrcs[asmSrcPartition] ca.lSrcs = partitionedSrcs[lSrcPartition] ca.llSrcs = partitionedSrcs[llSrcPartition] @@ -527,6 +531,61 @@ func includesFromLabelList(labelList bazel.LabelList) (relative, absolute []stri return relative, absolute } +type YasmAttributes struct { + Srcs bazel.LabelListAttribute + Flags bazel.StringListAttribute + Include_dirs bazel.StringListAttribute +} + +func bp2BuildYasm(ctx android.Bp2buildMutatorContext, m *Module, ca compilerAttributes) *bazel.LabelAttribute { + if ca.asmSrcs.IsEmpty() { + return nil + } + + // Yasm needs the include directories from both local_includes and + // export_include_dirs. We don't care about actually exporting them from the + // yasm rule though, because they will also be present on the cc_ rule that + // wraps this yasm rule. + includes := ca.localIncludes.Clone() + bp2BuildPropParseHelper(ctx, m, &FlagExporterProperties{}, func(axis bazel.ConfigurationAxis, config string, props interface{}) { + if flagExporterProperties, ok := props.(*FlagExporterProperties); ok { + if len(flagExporterProperties.Export_include_dirs) > 0 { + x := bazel.StringListAttribute{} + x.SetSelectValue(axis, config, flagExporterProperties.Export_include_dirs) + includes.Append(x) + } + } + }) + + ctx.CreateBazelTargetModule( + bazel.BazelTargetModuleProperties{ + Rule_class: "yasm", + Bzl_load_location: "//build/bazel/rules/cc:yasm.bzl", + }, + android.CommonAttributes{Name: m.Name() + "_yasm"}, + &YasmAttributes{ + Srcs: ca.asmSrcs, + Flags: ca.asFlags, + Include_dirs: *includes, + }) + + // We only want to add a dependency on the _yasm target if there are asm + // sources in the current configuration. If there are unconfigured asm + // sources, always add the dependency. Otherwise, add the dependency only + // on the configuration axes and values that had asm sources. + if len(ca.asmSrcs.Value.Includes) > 0 { + return bazel.MakeLabelAttribute(":" + m.Name() + "_yasm") + } + + ret := &bazel.LabelAttribute{} + for _, axis := range ca.asmSrcs.SortedConfigurationAxes() { + for config := range ca.asmSrcs.ConfigurableValues[axis] { + ret.SetSelectValue(axis, config, bazel.Label{Label: ":" + m.Name() + "_yasm"}) + } + } + return ret +} + // bp2BuildParseBaseProps returns all compiler, linker, library attributes of a cc module.. func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) baseAttributes { archVariantCompilerProps := module.GetArchVariantProperties(ctx, &BaseCompilerProperties{}) @@ -612,6 +671,8 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) (&compilerAttrs).finalize(ctx, implementationHdrs) (&linkerAttrs).finalize(ctx) + (&compilerAttrs.srcs).Add(bp2BuildYasm(ctx, module, compilerAttrs)) + protoDep := bp2buildProto(ctx, module, compilerAttrs.protoSrcs) // bp2buildProto will only set wholeStaticLib or implementationWholeStaticLib, but we don't know