diff --git a/android/config.go b/android/config.go index f9cb84233..407aeb7ec 100644 --- a/android/config.go +++ b/android/config.go @@ -1499,18 +1499,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..00b22456f 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 { @@ -62,17 +50,32 @@ func (afdo *afdo) begin(ctx BaseModuleContext) { } // afdoEnabled returns true for binaries and shared libraries -// that set afdo prop to True and there is a profile available +// that set afdo prop to True. func (afdo *afdo) afdoEnabled() bool { return afdo != nil && afdo.Properties.Afdo } +func (afdo *afdo) isAfdoCompile(ctx ModuleContext) bool { + fdoProfilePath := getFdoProfilePathFromDep(ctx) + return !ctx.Host() && (afdo.Properties.Afdo || afdo.Properties.AfdoDep) && (fdoProfilePath != "") +} + +func getFdoProfilePathFromDep(ctx ModuleContext) string { + fdoProfileDeps := ctx.GetDirectDepsWithTag(FdoProfileTag) + if len(fdoProfileDeps) > 0 && fdoProfileDeps[0] != nil { + if info, ok := android.OtherModuleProvider(ctx, fdoProfileDeps[0], FdoProfileProvider); ok { + return info.Path.String() + } + } + return "" +} + func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { if ctx.Host() { return 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 @@ -90,15 +93,15 @@ 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 { + if fdoProfilePath := getFdoProfilePathFromDep(ctx); fdoProfilePath != "" { // The flags are prepended to allow overriding. - profileUseFlag := fmt.Sprintf(afdoFlagsFormat, *path) + 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, *path) + pathForSrc := android.PathForSource(ctx, fdoProfilePath) flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc) flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc) } @@ -106,130 +109,86 @@ func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { return flags } -func (afdo *afdo) addDep(ctx BaseModuleContext, actx android.BottomUpMutatorContext) { +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) + } +} + +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 + } +} + +// 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 + 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}..., - ) + 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 "" } -// 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 +func (a *afdoTransitionMutator) IncomingTransition(ctx android.IncomingTransitionContext, incomingVariation string) string { + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + return incomingVariation } - - 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()) - } - }) + return "" } -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 - }) - } -} - -// Create afdo variants for modules that need them -func afdoMutator(mctx android.BottomUpMutatorContext) { - if mctx.Host() { - 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 - } - variation := modules[i].(*Module) - variation.Properties.PreventInstall = true - variation.Properties.HideFromMake = true - variation.afdo.Properties.Afdo = true - variation.afdo.Properties.FdoProfilePath = variantNameToProfilePath[name] +func (a *afdoTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) { + if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { + if variation == "" { + // The empty variation is either a module that has enabled AFDO for itself, or the non-AFDO + // variant of a dependency. + if m.afdo.afdoEnabled() && !(m.static() && !m.staticBinary()) && !m.Host() { + m.afdo.addDep(ctx, ctx.ModuleName()) } + } else { + // The non-empty variation is the AFDO variant of a dependency of a module that enabled AFDO + // for itself. + m.Properties.PreventInstall = true + m.Properties.HideFromMake = true + m.afdo.Properties.AfdoDep = true + m.afdo.addDep(ctx, decodeTarget(variation)) } } } diff --git a/cc/cc.go b/cc/cc.go index 449d38f33..39024aa3c 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{}) @@ -527,7 +525,7 @@ type ModuleContextIntf interface { selectedStl() string baseModuleName() string getVndkExtendsModuleName() string - isAfdoCompile() bool + isAfdoCompile(ctx ModuleContext) bool isOrderfileCompile() bool isCfi() bool isFuzzer() bool @@ -1381,9 +1379,9 @@ func (c *Module) IsVndk() bool { return false } -func (c *Module) isAfdoCompile() bool { +func (c *Module) isAfdoCompile(ctx ModuleContext) bool { if afdo := c.afdo; afdo != nil { - return afdo.Properties.FdoProfilePath != nil + return afdo.isAfdoCompile(ctx) } return false } @@ -1706,8 +1704,8 @@ func (ctx *moduleContextImpl) isVndk() bool { return ctx.mod.IsVndk() } -func (ctx *moduleContextImpl) isAfdoCompile() bool { - return ctx.mod.isAfdoCompile() +func (ctx *moduleContextImpl) isAfdoCompile(mctx ModuleContext) bool { + return ctx.mod.isAfdoCompile(mctx) } func (ctx *moduleContextImpl) isOrderfileCompile() bool { @@ -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/cc/lto.go b/cc/lto.go index b2b45706d..05fa8eea5 100644 --- a/cc/lto.go +++ b/cc/lto.go @@ -100,7 +100,7 @@ func (lto *lto) begin(ctx BaseModuleContext) { lto.Properties.LtoEnabled = ltoEnabled } -func (lto *lto) flags(ctx BaseModuleContext, flags Flags) Flags { +func (lto *lto) flags(ctx ModuleContext, flags Flags) Flags { // TODO(b/131771163): CFI and Fuzzer controls LTO flags by themselves. // This has be checked late because these properties can be mutated. if ctx.isCfi() || ctx.isFuzzer() { @@ -139,7 +139,7 @@ func (lto *lto) flags(ctx BaseModuleContext, flags Flags) Flags { // Reduce the inlining threshold for a better balance of binary size and // performance. if !ctx.Darwin() { - if ctx.isAfdoCompile() { + if ctx.isAfdoCompile(ctx) { ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=40") } else { ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=5") 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}..., ) } }