diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 161a7ffcf..b321b38cb 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -63,6 +63,7 @@ bootstrap_go_package { "cc_yasm_conversion_test.go", "conversion_test.go", "droiddoc_exported_dir_conversion_test.go", + "fdo_profile_conversion_test.go", "filegroup_conversion_test.go", "genrule_conversion_test.go", "gensrcs_conversion_test.go", diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index b667fe9dc..3957ff767 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -3640,8 +3640,8 @@ cc_library { { description: "cc_library with afdo enabled and existing profile", filesystem: map[string]string{ - "vendor/google_data/pgo_profile/sampling/BUILD": "", - "vendor/google_data/pgo_profile/sampling/foo.afdo": "", + "vendor/google_data/pgo_profile/sampling/Android.bp": "", + "vendor/google_data/pgo_profile/sampling/foo.afdo": "", }, expectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{}), @@ -3653,8 +3653,8 @@ cc_library { { description: "cc_library with afdo enabled and existing profile in AOSP", filesystem: map[string]string{ - "toolchain/pgo-profiles/sampling/BUILD": "", - "toolchain/pgo-profiles/sampling/foo.afdo": "", + "toolchain/pgo-profiles/sampling/Android.bp": "", + "toolchain/pgo-profiles/sampling/foo.afdo": "", }, expectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{}), @@ -3666,8 +3666,8 @@ cc_library { { description: "cc_library with afdo enabled but profile filename doesn't match with module name", filesystem: map[string]string{ - "toolchain/pgo-profiles/sampling/BUILD": "", - "toolchain/pgo-profiles/sampling/bar.afdo": "", + "toolchain/pgo-profiles/sampling/Android.bp": "", + "toolchain/pgo-profiles/sampling/bar.afdo": "", }, expectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{}), diff --git a/bp2build/fdo_profile_conversion_test.go b/bp2build/fdo_profile_conversion_test.go new file mode 100644 index 000000000..4d04283ca --- /dev/null +++ b/bp2build/fdo_profile_conversion_test.go @@ -0,0 +1,85 @@ +// Copyright 2023 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 runFdoProfileTestCase(t *testing.T, tc Bp2buildTestCase) { + t.Helper() + (&tc).ModuleTypeUnderTest = "fdo_profile" + (&tc).ModuleTypeUnderTestFactory = cc.FdoProfileFactory + RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, tc) +} + +func TestFdoProfile(t *testing.T) { + testcases := []struct { + name string + bp string + expectedBazelAttrs AttrNameToString + }{ + { + name: "fdo_profile with arch-specific profiles", + bp: ` +fdo_profile { + name: "foo", + arch: { + arm: { + profile: "foo_arm.afdo", + }, + arm64: { + profile: "foo_arm64.afdo", + } + } +}`, + expectedBazelAttrs: AttrNameToString{ + "profile": `select({ + "//build/bazel/platforms/arch:arm": "foo_arm.afdo", + "//build/bazel/platforms/arch:arm64": "foo_arm64.afdo", + "//conditions:default": None, + })`, + }, + }, + { + name: "fdo_profile with arch-agnostic profile", + bp: ` +fdo_profile { + name: "foo", + profile: "foo.afdo", +}`, + expectedBazelAttrs: AttrNameToString{ + "profile": `"foo.afdo"`, + }, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + expectedBazelTargets := []string{ + // TODO(b/276287371): Add device-only restriction back to fdo_profile targets + MakeBazelTargetNoRestrictions("fdo_profile", "foo", test.expectedBazelAttrs), + } + runFdoProfileTestCase(t, Bp2buildTestCase{ + Description: test.name, + Blueprint: test.bp, + ExpectedBazelTargets: expectedBazelTargets, + }) + }) + } +} diff --git a/cc/bp2build.go b/cc/bp2build.go index 039a3cf74..ce8c96d0c 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1000,6 +1000,8 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) if module.afdo != nil && module.afdo.Properties.Afdo { fdoProfileDep := bp2buildFdoProfile(ctx, module) if fdoProfileDep != nil { + // TODO(b/276287371): Only set fdo_profile for android platform + // https://cs.android.com/android/platform/superproject/main/+/main:build/soong/cc/afdo.go;l=105;drc=2dbe160d1af445de32725098570ec594e3944fc5 (&compilerAttrs).fdoProfile.SetValue(*fdoProfileDep) } } @@ -1109,22 +1111,15 @@ func bp2buildFdoProfile( ctx android.Bp2buildMutatorContext, m *Module, ) *bazel.Label { + // TODO(b/267229066): Convert to afdo boolean attribute and let Bazel handles finding + // fdo_profile target from AfdoProfiles product var for _, project := range globalAfdoProfileProjects { - // Ensure handcrafted BUILD file exists in the project - BUILDPath := android.ExistentPathForSource(ctx, project, "BUILD") - if BUILDPath.Valid() { - // We handcraft a BUILD file with fdo_profile targets that use the existing profiles in the project - // This implementation is assuming that every afdo profile in globalAfdoProfileProjects already has - // an associated fdo_profile target declared in the same package. + // Ensure it's a Soong package + bpPath := android.ExistentPathForSource(ctx, project, "Android.bp") + if bpPath.Valid() { // TODO(b/260714900): Handle arch-specific afdo profiles (e.g. `-arm<64>.afdo`) path := android.ExistentPathForSource(ctx, project, m.Name()+".afdo") if path.Valid() { - // FIXME: Some profiles only exist internally and are not released to AOSP. - // When generated BUILD files are checked in, we'll run into merge conflict. - // The cc_library_shared target in AOSP won't have reference to an fdo_profile target because - // the profile doesn't exist. Internally, the same cc_library_shared target will - // have reference to the fdo_profile. - // For more context, see b/258682955#comment2 fdoProfileLabel := "//" + strings.TrimSuffix(project, "/") + ":" + m.Name() return &bazel.Label{ Label: fdoProfileLabel, diff --git a/cc/fdo_profile.go b/cc/fdo_profile.go index 7fbe71940..d61af7e44 100644 --- a/cc/fdo_profile.go +++ b/cc/fdo_profile.go @@ -16,8 +16,10 @@ package cc import ( "android/soong/android" + "android/soong/bazel" "github.com/google/blueprint" + "github.com/google/blueprint/proptools" ) func init() { @@ -25,11 +27,12 @@ func init() { } func RegisterFdoProfileBuildComponents(ctx android.RegistrationContext) { - ctx.RegisterModuleType("fdo_profile", fdoProfileFactory) + ctx.RegisterModuleType("fdo_profile", FdoProfileFactory) } type fdoProfile struct { android.ModuleBase + android.BazelModuleBase properties fdoProfileProperties } @@ -38,6 +41,49 @@ type fdoProfileProperties struct { Profile *string `android:"arch_variant"` } +type bazelFdoProfileAttributes struct { + Profile bazel.StringAttribute +} + +func (fp *fdoProfile) ConvertWithBp2build(ctx android.TopDownMutatorContext) { + var profileAttr bazel.StringAttribute + + archVariantProps := fp.GetArchVariantProperties(ctx, &fdoProfileProperties{}) + for axis, configToProps := range archVariantProps { + for config, _props := range configToProps { + if archProps, ok := _props.(*fdoProfileProperties); ok { + if axis.String() == "arch" || axis.String() == "no_config" { + if archProps.Profile != nil { + profileAttr.SetSelectValue(axis, config, archProps.Profile) + } + } + } + } + } + + // Ideally, cc_library_shared's fdo_profile attr can be a select statement so that we + // don't lift the restriction here. However, in cc_library_shared macro, fdo_profile + // is used as a string, we need to temporarily lift the host restriction until we can + // pass use fdo_profile attr with select statement + // https://cs.android.com/android/platform/superproject/+/master:build/bazel/rules/cc/cc_library_shared.bzl;l=127;drc=cc01bdfd39857eddbab04ef69ab6db22dcb1858a + // TODO(b/276287371): Drop the restriction override after fdo_profile path is handled properly + var noRestriction bazel.BoolAttribute + noRestriction.SetSelectValue(bazel.NoConfigAxis, "", proptools.BoolPtr(true)) + + ctx.CreateBazelTargetModuleWithRestrictions( + bazel.BazelTargetModuleProperties{ + Rule_class: "fdo_profile", + }, + android.CommonAttributes{ + Name: fp.Name(), + }, + &bazelFdoProfileAttributes{ + Profile: profileAttr, + }, + noRestriction, + ) +} + // FdoProfileInfo is provided by FdoProfileProvider type FdoProfileInfo struct { Path android.Path @@ -77,9 +123,10 @@ func fdoProfileMutator(ctx android.BottomUpMutatorContext) { } } -func fdoProfileFactory() android.Module { +func FdoProfileFactory() android.Module { m := &fdoProfile{} m.AddProperties(&m.properties) android.InitAndroidMultiTargetsArchModule(m, android.DeviceSupported, android.MultilibBoth) + android.InitBazelModule(m) return m } diff --git a/cc/testing.go b/cc/testing.go index 24d6b0f5f..07bee01be 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -671,7 +671,7 @@ var PrepareForTestWithHostMusl = android.GroupFixturePreparers( // PrepareForTestWithFdoProfile registers module types to test with fdo_profile var PrepareForTestWithFdoProfile = android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) { ctx.RegisterModuleType("soong_namespace", android.NamespaceFactory) - ctx.RegisterModuleType("fdo_profile", fdoProfileFactory) + ctx.RegisterModuleType("fdo_profile", FdoProfileFactory) }) // TestConfig is the legacy way of creating a test Config for testing cc modules.