diff --git a/android/config.go b/android/config.go index d94a86f71..652de5862 100644 --- a/android/config.go +++ b/android/config.go @@ -1466,18 +1466,18 @@ func (c *deviceConfig) PgoAdditionalProfileDirs() []string { } // AfdoProfile returns fully qualified path associated to the given module name -func (c *deviceConfig) AfdoProfile(name string) (*string, error) { +func (c *deviceConfig) AfdoProfile(name string) (string, error) { for _, afdoProfile := range c.config.productVariables.AfdoProfiles { split := strings.Split(afdoProfile, ":") if len(split) != 3 { - return nil, fmt.Errorf("AFDO_PROFILES has invalid value: %s. "+ + return "", fmt.Errorf("AFDO_PROFILES has invalid value: %s. "+ "The expected format is :", afdoProfile) } if split[0] == name { - return proptools.StringPtr(strings.Join([]string{split[1], split[2]}, ":")), nil + return strings.Join([]string{split[1], split[2]}, ":"), nil } } - return nil, nil + return "", nil } func (c *deviceConfig) VendorSepolicyDirs() []string { diff --git a/cc/afdo.go b/cc/afdo.go index 79fbae157..0de1d4ba6 100644 --- a/cc/afdo.go +++ b/cc/afdo.go @@ -21,29 +21,17 @@ import ( "android/soong/android" "github.com/google/blueprint" - "github.com/google/blueprint/proptools" ) // This flag needs to be in both CFlags and LdFlags to ensure correct symbol ordering const afdoFlagsFormat = "-fprofile-sample-use=%s -fprofile-sample-accurate" -func recordMissingAfdoProfileFile(ctx android.BaseModuleContext, missing string) { - getNamedMapForConfig(ctx.Config(), modulesMissingProfileFileKey).Store(missing, true) -} - -type afdoRdep struct { - VariationName *string - ProfilePath *string -} - type AfdoProperties struct { // Afdo allows developers self-service enroll for // automatic feedback-directed optimization using profile data. Afdo bool - FdoProfilePath *string `blueprint:"mutated"` - - AfdoRDeps []afdoRdep `blueprint:"mutated"` + AfdoDep bool `blueprint:"mutated"` } type afdo struct { @@ -68,7 +56,7 @@ func (afdo *afdo) afdoEnabled() bool { } func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { - if afdo.Properties.Afdo { + if afdo.Properties.Afdo || afdo.Properties.AfdoDep { // We use `-funique-internal-linkage-names` to associate profiles to the right internal // functions. This option should be used before generating a profile. Because a profile // generated for a binary without unique names doesn't work well building a binary with @@ -86,138 +74,91 @@ func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { // TODO(b/266595187): Remove the following feature once it is enabled in LLVM by default. flags.Local.CFlags = append([]string{"-mllvm", "-improved-fs-discriminator=true"}, flags.Local.CFlags...) } - if path := afdo.Properties.FdoProfilePath; path != nil { - // The flags are prepended to allow overriding. - profileUseFlag := fmt.Sprintf(afdoFlagsFormat, *path) - flags.Local.CFlags = append([]string{profileUseFlag}, flags.Local.CFlags...) - flags.Local.LdFlags = append([]string{profileUseFlag, "-Wl,-mllvm,-no-warn-sample-unused=true"}, flags.Local.LdFlags...) + fdoProfileDeps := ctx.GetDirectDepsWithTag(FdoProfileTag) + if len(fdoProfileDeps) > 0 && fdoProfileDeps[0] != nil { + if info, ok := android.OtherModuleProvider(ctx, fdoProfileDeps[0], FdoProfileProvider); ok { + fdoProfilePath := info.Path.String() - // Update CFlagsDeps and LdFlagsDeps so the module is rebuilt - // if profileFile gets updated - pathForSrc := android.PathForSource(ctx, *path) - flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc) - flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc) + // The flags are prepended to allow overriding. + profileUseFlag := fmt.Sprintf(afdoFlagsFormat, fdoProfilePath) + flags.Local.CFlags = append([]string{profileUseFlag}, flags.Local.CFlags...) + flags.Local.LdFlags = append([]string{profileUseFlag, "-Wl,-mllvm,-no-warn-sample-unused=true"}, flags.Local.LdFlags...) + + // Update CFlagsDeps and LdFlagsDeps so the module is rebuilt + // if profileFile gets updated + pathForSrc := android.PathForSource(ctx, fdoProfilePath) + flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc) + flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc) + } } return flags } -func (afdo *afdo) addDep(ctx BaseModuleContext, actx android.BottomUpMutatorContext) { - if ctx.Host() { - return - } - - if ctx.static() && !ctx.staticBinary() { - return - } - - if c, ok := ctx.Module().(*Module); ok && c.Enabled() { - if fdoProfileName, err := actx.DeviceConfig().AfdoProfile(actx.ModuleName()); fdoProfileName != nil && err == nil { - actx.AddFarVariationDependencies( - []blueprint.Variation{ - {Mutator: "arch", Variation: actx.Target().ArchVariation()}, - {Mutator: "os", Variation: "android"}, - }, - FdoProfileTag, - []string{*fdoProfileName}..., - ) - } +func (a *afdo) addDep(ctx android.BottomUpMutatorContext, fdoProfileTarget string) { + if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(fdoProfileTarget); fdoProfileName != "" && err == nil { + ctx.AddFarVariationDependencies( + []blueprint.Variation{ + {Mutator: "arch", Variation: ctx.Target().ArchVariation()}, + {Mutator: "os", Variation: "android"}, + }, + FdoProfileTag, + fdoProfileName) } } -// FdoProfileMutator reads the FdoProfileProvider from a direct dep with FdoProfileTag -// assigns FdoProfileInfo.Path to the FdoProfilePath mutated property -func (c *Module) fdoProfileMutator(ctx android.BottomUpMutatorContext) { - if !c.Enabled() { - return - } - - if !c.afdo.afdoEnabled() { - return - } - - ctx.VisitDirectDepsWithTag(FdoProfileTag, func(m android.Module) { - if info, ok := android.OtherModuleProvider(ctx, m, FdoProfileProvider); ok { - c.afdo.Properties.FdoProfilePath = proptools.StringPtr(info.Path.String()) - } - }) -} - -var _ FdoProfileMutatorInterface = (*Module)(nil) - -// Propagate afdo requirements down from binaries and shared libraries -func afdoDepsMutator(mctx android.TopDownMutatorContext) { - if m, ok := mctx.Module().(*Module); ok && m.afdo.afdoEnabled() { - path := m.afdo.Properties.FdoProfilePath - mctx.WalkDeps(func(dep android.Module, parent android.Module) bool { - tag := mctx.OtherModuleDependencyTag(dep) - libTag, isLibTag := tag.(libraryDependencyTag) - - // Do not recurse down non-static dependencies - if isLibTag { - if !libTag.static() { - return false - } - } else { - if tag != objDepTag && tag != reuseObjTag { - return false - } - } - - if dep, ok := dep.(*Module); ok { - dep.afdo.Properties.AfdoRDeps = append( - dep.afdo.Properties.AfdoRDeps, - afdoRdep{ - VariationName: proptools.StringPtr(encodeTarget(m.Name())), - ProfilePath: path, - }, - ) - } - - return true - }) +func afdoPropagateViaDepTag(tag blueprint.DependencyTag) bool { + libTag, isLibTag := tag.(libraryDependencyTag) + // Do not recurse down non-static dependencies + if isLibTag { + return libTag.static() + } else { + return tag == objDepTag || tag == reuseObjTag || tag == staticVariantTag } } -// Create afdo variants for modules that need them -func afdoMutator(mctx android.BottomUpMutatorContext) { - if m, ok := mctx.Module().(*Module); ok && m.afdo != nil { - if !m.static() && m.afdo.Properties.Afdo { - mctx.SetDependencyVariation(encodeTarget(m.Name())) - return +// afdoTransitionMutator creates afdo variants of cc modules. +type afdoTransitionMutator struct{} + +func (a *afdoTransitionMutator) Split(ctx android.BaseModuleContext) []string { + return []string{""} +} + +func (a *afdoTransitionMutator) OutgoingTransition(ctx android.OutgoingTransitionContext, sourceVariation string) string { + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + if !afdoPropagateViaDepTag(ctx.DepTag()) { + return "" } - variationNames := []string{""} - - variantNameToProfilePath := make(map[string]*string) - - for _, afdoRDep := range m.afdo.Properties.AfdoRDeps { - variantName := *afdoRDep.VariationName - // An rdep can be set twice in AfdoRDeps because there can be - // more than one path from an afdo-enabled module to - // a static dep such as - // afdo_enabled_foo -> static_bar ----> static_baz - // \ ^ - // ----------------------| - // We only need to create one variant per unique rdep - if _, exists := variantNameToProfilePath[variantName]; !exists { - variationNames = append(variationNames, variantName) - variantNameToProfilePath[variantName] = afdoRDep.ProfilePath - } + if sourceVariation != "" { + return sourceVariation } - if len(variationNames) > 1 { - modules := mctx.CreateVariations(variationNames...) - for i, name := range variationNames { - if name == "" { - continue - } - variation := modules[i].(*Module) - variation.Properties.PreventInstall = true - variation.Properties.HideFromMake = true - variation.afdo.Properties.Afdo = true - variation.afdo.Properties.FdoProfilePath = variantNameToProfilePath[name] + if m.afdo.afdoEnabled() && !(m.static() && !m.staticBinary()) && !m.Host() { + return encodeTarget(ctx.Module().Name()) + } + } + return "" +} + +func (a *afdoTransitionMutator) IncomingTransition(ctx android.IncomingTransitionContext, incomingVariation string) string { + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + return incomingVariation + } + return "" +} + +func (a *afdoTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) { + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + if variation == "" { + if m.afdo.afdoEnabled() && !(m.static() && !m.staticBinary()) && !m.Host() { + m.afdo.addDep(ctx, ctx.ModuleName()) } + } else { + m.Properties.PreventInstall = true + m.Properties.HideFromMake = true + m.afdo.Properties.AfdoDep = true + m.afdo.addDep(ctx, decodeTarget(variation)) } } } diff --git a/cc/afdo_test.go b/cc/afdo_test.go index b250ad1a1..584af0d6f 100644 --- a/cc/afdo_test.go +++ b/cc/afdo_test.go @@ -174,11 +174,11 @@ func TestAfdoEnabledOnStaticDepNoAfdo(t *testing.T) { libBar := result.ModuleForTests("libBar", "android_arm64_armv8-a_static").Module() if !hasDirectDep(result, libTest, libFoo.Module()) { - t.Errorf("libTest missing dependency on afdo variant of libFoo") + t.Errorf("libTest missing dependency on non-afdo variant of libFoo") } if !hasDirectDep(result, libFoo.Module(), libBar) { - t.Errorf("libFoo missing dependency on afdo variant of libBar") + t.Errorf("libFoo missing dependency on non-afdo variant of libBar") } fooVariants := result.ModuleVariantsForTests("foo") diff --git a/cc/cc.go b/cc/cc.go index 449d38f33..11924b14f 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -55,7 +55,6 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) { ctx.BottomUp("test_per_src", TestPerSrcMutator).Parallel() ctx.BottomUp("version", versionMutator).Parallel() ctx.BottomUp("begin", BeginMutator).Parallel() - ctx.BottomUp("fdo_profile", fdoProfileMutator) }) ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) { @@ -70,8 +69,7 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) { ctx.Transition("coverage", &coverageTransitionMutator{}) - ctx.TopDown("afdo_deps", afdoDepsMutator) - ctx.BottomUp("afdo", afdoMutator).Parallel() + ctx.Transition("afdo", &afdoTransitionMutator{}) ctx.Transition("orderfile", &orderfileTransitionMutator{}) @@ -1383,7 +1381,7 @@ func (c *Module) IsVndk() bool { func (c *Module) isAfdoCompile() bool { if afdo := c.afdo; afdo != nil { - return afdo.Properties.FdoProfilePath != nil + return afdo.Properties.Afdo || afdo.Properties.AfdoDep } return false } @@ -2351,10 +2349,6 @@ func (c *Module) beginMutator(actx android.BottomUpMutatorContext) { } ctx.ctx = ctx - if !actx.Host() || !ctx.static() || ctx.staticBinary() { - c.afdo.addDep(ctx, actx) - } - c.begin(ctx) } diff --git a/cc/fdo_profile.go b/cc/fdo_profile.go index 0893da5e0..1a3395773 100644 --- a/cc/fdo_profile.go +++ b/cc/fdo_profile.go @@ -43,23 +43,10 @@ type FdoProfileInfo struct { } // FdoProfileProvider is used to provide path to an fdo profile -var FdoProfileProvider = blueprint.NewMutatorProvider[FdoProfileInfo]("fdo_profile") - -// FdoProfileMutatorInterface is the interface implemented by fdo_profile module type -// module types that can depend on an fdo_profile module -type FdoProfileMutatorInterface interface { - // FdoProfileMutator eithers set or get FdoProfileProvider - fdoProfileMutator(ctx android.BottomUpMutatorContext) -} - -var _ FdoProfileMutatorInterface = (*fdoProfile)(nil) +var FdoProfileProvider = blueprint.NewProvider[FdoProfileInfo]() // GenerateAndroidBuildActions of fdo_profile does not have any build actions -func (fp *fdoProfile) GenerateAndroidBuildActions(ctx android.ModuleContext) {} - -// FdoProfileMutator sets FdoProfileProvider to fdo_profile module -// or sets afdo.Properties.FdoProfilePath to path in FdoProfileProvider of the depended fdo_profile -func (fp *fdoProfile) fdoProfileMutator(ctx android.BottomUpMutatorContext) { +func (fp *fdoProfile) GenerateAndroidBuildActions(ctx android.ModuleContext) { if fp.properties.Profile != nil { path := android.PathForModuleSrc(ctx, *fp.properties.Profile) android.SetProvider(ctx, FdoProfileProvider, FdoProfileInfo{ @@ -68,14 +55,6 @@ func (fp *fdoProfile) fdoProfileMutator(ctx android.BottomUpMutatorContext) { } } -// fdoProfileMutator calls the generic fdoProfileMutator function of fdoProfileMutator -// which is implemented by cc and cc.FdoProfile -func fdoProfileMutator(ctx android.BottomUpMutatorContext) { - if f, ok := ctx.Module().(FdoProfileMutatorInterface); ok { - f.fdoProfileMutator(ctx) - } -} - func FdoProfileFactory() android.Module { m := &fdoProfile{} m.AddProperties(&m.properties) diff --git a/rust/afdo.go b/rust/afdo.go index 323ee36a5..6116c5e21 100644 --- a/rust/afdo.go +++ b/rust/afdo.go @@ -44,14 +44,14 @@ func (afdo *afdo) addDep(ctx BaseModuleContext, actx android.BottomUpMutatorCont if err != nil { ctx.ModuleErrorf("%s", err.Error()) } - if fdoProfileName != nil { + if fdoProfileName != "" { actx.AddFarVariationDependencies( []blueprint.Variation{ {Mutator: "arch", Variation: actx.Target().ArchVariation()}, {Mutator: "os", Variation: "android"}, }, cc.FdoProfileTag, - []string{*fdoProfileName}..., + []string{fdoProfileName}..., ) } }