From c843b99b7151275486580ec3f71ae7863a02cb66 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 2 Aug 2022 18:06:50 -0700 Subject: [PATCH] Support arch features in bp2build Bug: 189972518 Test: New soong test Change-Id: I05d77c8f63ffe6697d8e0300226864658055e116 --- android/allowlists/allowlists.go | 2 +- android/arch.go | 39 +++++++- bazel/configurability.go | 75 +++++++++++++-- bazel/properties.go | 2 +- bp2build/java_library_conversion_test.go | 115 +++++++++++++++++++++++ 5 files changed, 220 insertions(+), 13 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index bcf335f36..d2abe0ff8 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -134,6 +134,7 @@ var ( "external/libjpeg-turbo": Bp2BuildDefaultTrueRecursively, "external/libmpeg2": Bp2BuildDefaultTrueRecursively, "external/libpng": Bp2BuildDefaultTrueRecursively, + "external/libvpx": Bp2BuildDefaultTrueRecursively, "external/libyuv": Bp2BuildDefaultTrueRecursively, "external/lz4/lib": Bp2BuildDefaultTrue, "external/lzma/C": Bp2BuildDefaultTrueRecursively, @@ -471,7 +472,6 @@ var ( "linker", // TODO(b/228316882): cc_binary uses link_crt "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 - "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", diff --git a/android/arch.go b/android/arch.go index 1952b1716..9bc9d8924 100644 --- a/android/arch.go +++ b/android/arch.go @@ -19,6 +19,7 @@ import ( "fmt" "reflect" "runtime" + "sort" "strings" "android/soong/bazel" @@ -2084,13 +2085,22 @@ func (m *ModuleBase) GetArchVariantProperties(ctx ArchVariantContext, propertySe // For each arch type (x86, arm64, etc.) for _, arch := range ArchTypeList() { // Arch properties are sometimes sharded (see createArchPropTypeDesc() ). - // Iterate over ever shard and extract a struct with the same type as the + // Iterate over every shard and extract a struct with the same type as the // input one that contains the data specific to that arch. propertyStructs := make([]reflect.Value, 0) + archFeaturePropertyStructs := make(map[string][]reflect.Value, 0) for _, archProperty := range archProperties { archTypeStruct, ok := getArchTypeStruct(ctx, archProperty, arch) if ok { propertyStructs = append(propertyStructs, archTypeStruct) + + // For each feature this arch supports (arm: neon, x86: ssse3, sse4, ...) + for _, feature := range archFeatures[arch] { + prefix := "arch." + arch.Name + "." + feature + if featureProperties, ok := getChildPropertyStruct(ctx, archTypeStruct, feature, prefix); ok { + archFeaturePropertyStructs[feature] = append(archFeaturePropertyStructs[feature], featureProperties) + } + } } multilibStruct, ok := getMultilibStruct(ctx, archProperty, arch) if ok { @@ -2098,10 +2108,31 @@ func (m *ModuleBase) GetArchVariantProperties(ctx ArchVariantContext, propertySe } } - // Create a new instance of the requested property set - value := reflect.New(reflect.ValueOf(propertySet).Elem().Type()).Interface() + archToProp[arch.Name] = mergeStructs(ctx, propertyStructs, propertySet) - archToProp[arch.Name] = mergeStructs(ctx, propertyStructs, value) + // In soong, if multiple features match the current configuration, they're + // all used. In bazel, we have to have unambiguous select() statements, so + // we can't have two features that are both active in the same select(). + // One alternative is to split out each feature into a separate select(), + // but then it's difficult to support exclude_srcs, which may need to + // exclude things from the regular arch select() statement if a certain + // feature is active. Instead, keep the features in the same select + // statement as the arches, but emit the power set of all possible + // combinations of features, so that bazel can match the most precise one. + allFeatures := make([]string, 0, len(archFeaturePropertyStructs)) + for feature := range archFeaturePropertyStructs { + allFeatures = append(allFeatures, feature) + } + for _, features := range bazel.PowerSetWithoutEmptySet(allFeatures) { + sort.Strings(features) + propsForCurrentFeatureSet := make([]reflect.Value, 0) + propsForCurrentFeatureSet = append(propsForCurrentFeatureSet, propertyStructs...) + for _, feature := range features { + propsForCurrentFeatureSet = append(propsForCurrentFeatureSet, archFeaturePropertyStructs[feature]...) + } + archToProp[arch.Name+"-"+strings.Join(features, "-")] = + mergeStructs(ctx, propsForCurrentFeatureSet, propertySet) + } } axisToProps[bazel.ArchConfigurationAxis] = archToProp diff --git a/bazel/configurability.go b/bazel/configurability.go index d9b0a12b4..e1cdd4acf 100644 --- a/bazel/configurability.go +++ b/bazel/configurability.go @@ -16,6 +16,8 @@ package bazel import ( "fmt" + "math" + "sort" "strings" ) @@ -69,6 +71,71 @@ const ( AndroidAndNonApex = "android-non_apex" ) +func PowerSetWithoutEmptySet[T any](items []T) [][]T { + resultSize := int(math.Pow(2, float64(len(items)))) + powerSet := make([][]T, 0, resultSize-1) + for i := 1; i < resultSize; i++ { + combination := make([]T, 0) + for j := 0; j < len(items); j++ { + if (i>>j)%2 == 1 { + combination = append(combination, items[j]) + } + } + powerSet = append(powerSet, combination) + } + return powerSet +} + +func createPlatformArchMap() map[string]string { + // Copy of archFeatures from android/arch_list.go because the bazel + // package can't access the android package + archFeatures := map[string][]string{ + "arm": { + "neon", + }, + "arm64": { + "dotprod", + }, + "x86": { + "ssse3", + "sse4", + "sse4_1", + "sse4_2", + "aes_ni", + "avx", + "avx2", + "avx512", + "popcnt", + "movbe", + }, + "x86_64": { + "ssse3", + "sse4", + "sse4_1", + "sse4_2", + "aes_ni", + "avx", + "avx2", + "avx512", + "popcnt", + }, + } + result := make(map[string]string) + for arch, allFeatures := range archFeatures { + result[arch] = "//build/bazel/platforms/arch:" + arch + // Sometimes we want to select on multiple features being active, so + // add the power set of all possible features to the map. More details + // in android.ModuleBase.GetArchVariantProperties + for _, features := range PowerSetWithoutEmptySet(allFeatures) { + sort.Strings(features) + archFeaturesName := arch + "-" + strings.Join(features, "-") + result[archFeaturesName] = "//build/bazel/platforms/arch/variants:" + archFeaturesName + } + } + result[ConditionsDefaultConfigKey] = ConditionsDefaultSelectKey + return result +} + var ( // These are the list of OSes and architectures with a Bazel config_setting // and constraint value equivalent. These exist in arch.go, but the android @@ -77,13 +144,7 @@ var ( // A map of architectures to the Bazel label of the constraint_value // for the @platforms//cpu:cpu constraint_setting - platformArchMap = map[string]string{ - archArm: "//build/bazel/platforms/arch:arm", - archArm64: "//build/bazel/platforms/arch:arm64", - archX86: "//build/bazel/platforms/arch:x86", - archX86_64: "//build/bazel/platforms/arch:x86_64", - ConditionsDefaultConfigKey: ConditionsDefaultSelectKey, // The default condition of as arch select map. - } + platformArchMap = createPlatformArchMap() // A map of target operating systems to the Bazel label of the // constraint_value for the @platforms//os:os constraint_setting diff --git a/bazel/properties.go b/bazel/properties.go index 13e36b582..d82fa6417 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -725,7 +725,7 @@ func (lla *LabelListAttribute) SelectValue(axis ConfigurationAxis, config string case noConfig: return lla.Value case arch, os, osArch, productVariables, osAndInApex: - return (lla.ConfigurableValues[axis][config]) + return lla.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) } diff --git a/bp2build/java_library_conversion_test.go b/bp2build/java_library_conversion_test.go index 4a4da18d0..74e2dbd09 100644 --- a/bp2build/java_library_conversion_test.go +++ b/bp2build/java_library_conversion_test.go @@ -534,3 +534,118 @@ java_library { ctx.RegisterModuleType("filegroup", android.FileGroupFactory) }) } + +func TestConvertArmNeonVariant(t *testing.T) { + t.Helper() + RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, Bp2buildTestCase{ + Description: "Android Library - simple arch feature", + ModuleTypeUnderTest: "android_library", + ModuleTypeUnderTestFactory: java.AndroidLibraryFactory, + Blueprint: simpleModuleDoNotConvertBp2build("android_library", "static_lib_dep") + ` +android_library { + name: "TestLib", + manifest: "manifest/AndroidManifest.xml", + srcs: ["lib.java"], + arch: { + arm: { + neon: { + srcs: ["arm_neon.java"], + }, + }, + }, +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget( + "android_library", + "TestLib", + AttrNameToString{ + "srcs": `["lib.java"] + select({ + "//build/bazel/platforms/arch/variants:arm-neon": ["arm_neon.java"], + "//conditions:default": [], + })`, + "manifest": `"manifest/AndroidManifest.xml"`, + "resource_files": `[]`, + }), + }}) +} + +func TestConvertMultipleArchFeatures(t *testing.T) { + t.Helper() + RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, Bp2buildTestCase{ + Description: "Android Library - multiple arch features", + ModuleTypeUnderTest: "android_library", + ModuleTypeUnderTestFactory: java.AndroidLibraryFactory, + Blueprint: simpleModuleDoNotConvertBp2build("android_library", "static_lib_dep") + ` +android_library { + name: "TestLib", + manifest: "manifest/AndroidManifest.xml", + srcs: ["lib.java"], + arch: { + x86: { + ssse3: { + srcs: ["ssse3.java"], + }, + sse4_1: { + srcs: ["sse4_1.java"], + }, + }, + }, +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget( + "android_library", + "TestLib", + AttrNameToString{ + "srcs": `["lib.java"] + select({ + "//build/bazel/platforms/arch/variants:x86-sse4_1": ["sse4_1.java"], + "//build/bazel/platforms/arch/variants:x86-sse4_1-ssse3": [ + "sse4_1.java", + "ssse3.java", + ], + "//build/bazel/platforms/arch/variants:x86-ssse3": ["ssse3.java"], + "//conditions:default": [], + })`, + "manifest": `"manifest/AndroidManifest.xml"`, + "resource_files": `[]`, + }), + }}) +} + +func TestConvertExcludeSrcsArchFeature(t *testing.T) { + t.Helper() + RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, Bp2buildTestCase{ + Description: "Android Library - exclude_srcs with arch feature", + ModuleTypeUnderTest: "android_library", + ModuleTypeUnderTestFactory: java.AndroidLibraryFactory, + Blueprint: simpleModuleDoNotConvertBp2build("android_library", "static_lib_dep") + ` +android_library { + name: "TestLib", + manifest: "manifest/AndroidManifest.xml", + srcs: ["lib.java"], + arch: { + arm: { + srcs: ["arm_non_neon.java"], + neon: { + exclude_srcs: ["arm_non_neon.java"], + }, + }, + }, +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget( + "android_library", + "TestLib", + AttrNameToString{ + "srcs": `["lib.java"] + select({ + "//build/bazel/platforms/arch/variants:arm-neon": [], + "//build/bazel/platforms/arch:arm": ["arm_non_neon.java"], + "//conditions:default": [], + })`, + "manifest": `"manifest/AndroidManifest.xml"`, + "resource_files": `[]`, + }), + }}) +}