From d38feb0d4aef1bc3c407900be77a887882765f85 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 23 Jan 2024 16:38:06 -0800 Subject: [PATCH] Convert AFDO mutators to TransitionMutator Convert afdoDepsMutator and afdoMutator to a TransitionMutator as a step towards variants-on-demand. This relands Ib05845455ccf43a07b3915a0d7b0a95896062f13 with a fix to maintain the current behavior of not using AFDO variants for dependencies of the linker static binary. Bug: 319288033 Bug: 324141705 Test: afdo_test.go Change-Id: I76e30021173fc5b7e9e1fa826039776eb3dc7b6e --- android/config.go | 8 +- cc/afdo.go | 201 +++++++++++++++++++++------------------------- cc/cc.go | 9 +-- rust/afdo.go | 4 +- 4 files changed, 98 insertions(+), 124 deletions(-) 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 6cc17467d..1e2290069 100644 --- a/cc/afdo.go +++ b/cc/afdo.go @@ -27,23 +27,12 @@ import ( // 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"` } type afdo struct { @@ -106,29 +95,6 @@ func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { 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}..., - ) - } - } -} - // 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) { @@ -140,95 +106,108 @@ func (c *Module) fdoProfileMutator(ctx android.BottomUpMutatorContext) { 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()) + if c.Host() { + return + } + + if c.static() && !c.staticBinary() { + return + } + + if c, ok := ctx.Module().(*Module); ok && c.Enabled() { + if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(ctx.ModuleName()); fdoProfileName != "" && err == nil { + deps := ctx.AddFarVariationDependencies( + []blueprint.Variation{ + {Mutator: "arch", Variation: ctx.Target().ArchVariation()}, + {Mutator: "os", Variation: "android"}, + }, + FdoProfileTag, + fdoProfileName) + if len(deps) > 0 && deps[0] != nil { + if info, ok := android.OtherModuleProvider(ctx, deps[0], 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 mctx.Host() { - return - } - - 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 mctx.Host() { +// 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 ctx.Host() { + return "" + } + + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + if !afdoPropagateViaDepTag(ctx.DepTag()) { + return "" + } + + if sourceVariation != "" { + return sourceVariation + } + + if !m.afdo.afdoEnabled() { + return "" + } + + // TODO(b/324141705): this is designed to prevent propagating AFDO from static libraries that have afdo: true set, but + // it should be m.static() && !m.staticBinary() so that static binaries use AFDO variants of dependencies. + if m.static() { + return "" + } + + 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 variation == "" { return } - if m, ok := mctx.Module().(*Module); ok && m.afdo != nil { - if !m.static() && m.afdo.Properties.Afdo { - mctx.SetDependencyVariation(encodeTarget(m.Name())) - 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 len(variationNames) > 1 { - modules := mctx.CreateVariations(variationNames...) - for i, name := range variationNames { - if name == "" { - continue + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + m.Properties.PreventInstall = true + m.Properties.HideFromMake = true + m.afdo.Properties.Afdo = true + if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(decodeTarget(variation)); fdoProfileName != "" && err == nil { + deps := ctx.AddFarVariationDependencies( + []blueprint.Variation{ + {Mutator: "arch", Variation: ctx.Target().ArchVariation()}, + {Mutator: "os", Variation: "android"}, + }, + FdoProfileTag, + fdoProfileName) + if len(deps) > 0 && deps[0] != nil { + if info, ok := android.OtherModuleProvider(ctx, deps[0], FdoProfileProvider); ok { + m.afdo.Properties.FdoProfilePath = proptools.StringPtr(info.Path.String()) } - variation := modules[i].(*Module) - variation.Properties.PreventInstall = true - variation.Properties.HideFromMake = true - variation.afdo.Properties.Afdo = true - variation.afdo.Properties.FdoProfilePath = variantNameToProfilePath[name] } } } diff --git a/cc/cc.go b/cc/cc.go index 449d38f33..3688e9573 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -55,7 +55,7 @@ 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.BottomUp("fdo_profile", fdoProfileMutator).Parallel() }) ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) { @@ -70,8 +70,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{}) @@ -2351,10 +2350,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/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}..., ) } }