Remove fdoProfileMutator

Add the dependency on the fdo_profile module in afdoTransitionMutator
and read the provider in GenerateAndroidBuildActions.

This relands Ied8fd7b52d5694a3691652318e87b8fe14dda126 with a fix
to use the correct LTO ldflag when an afdo variant doesn't have a
profile.

Bug: 319288033
Bug: 324141705
Test: afdo_test.go
Change-Id: I024ca316cfb565b5fb0251793f05a54ce16cc1cb
This commit is contained in:
Colin Cross
2024-01-24 14:44:47 -08:00
parent d38feb0d4a
commit 3513fb17cd
4 changed files with 51 additions and 93 deletions

View File

@@ -21,7 +21,6 @@ import (
"android/soong/android" "android/soong/android"
"github.com/google/blueprint" "github.com/google/blueprint"
"github.com/google/blueprint/proptools"
) )
// This flag needs to be in both CFlags and LdFlags to ensure correct symbol ordering // This flag needs to be in both CFlags and LdFlags to ensure correct symbol ordering
@@ -32,7 +31,7 @@ type AfdoProperties struct {
// automatic feedback-directed optimization using profile data. // automatic feedback-directed optimization using profile data.
Afdo bool Afdo bool
FdoProfilePath *string `blueprint:"mutated"` AfdoDep bool `blueprint:"mutated"`
} }
type afdo struct { type afdo struct {
@@ -51,17 +50,32 @@ func (afdo *afdo) begin(ctx BaseModuleContext) {
} }
// afdoEnabled returns true for binaries and shared libraries // 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 { func (afdo *afdo) afdoEnabled() bool {
return afdo != nil && afdo.Properties.Afdo 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 { func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags {
if ctx.Host() { if ctx.Host() {
return flags 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 // 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 // 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 // generated for a binary without unique names doesn't work well building a binary with
@@ -79,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. // 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...) 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. // 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.CFlags = append([]string{profileUseFlag}, flags.Local.CFlags...)
flags.Local.LdFlags = append([]string{profileUseFlag, "-Wl,-mllvm,-no-warn-sample-unused=true"}, flags.Local.LdFlags...) 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 // Update CFlagsDeps and LdFlagsDeps so the module is rebuilt
// if profileFile gets updated // if profileFile gets updated
pathForSrc := android.PathForSource(ctx, *path) pathForSrc := android.PathForSource(ctx, fdoProfilePath)
flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc) flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc)
flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc) flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc)
} }
@@ -95,45 +109,18 @@ func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags {
return flags return flags
} }
// FdoProfileMutator reads the FdoProfileProvider from a direct dep with FdoProfileTag func (a *afdo) addDep(ctx android.BottomUpMutatorContext, fdoProfileTarget string) {
// assigns FdoProfileInfo.Path to the FdoProfilePath mutated property if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(fdoProfileTarget); fdoProfileName != "" && err == nil {
func (c *Module) fdoProfileMutator(ctx android.BottomUpMutatorContext) { ctx.AddFarVariationDependencies(
if !c.Enabled() {
return
}
if !c.afdo.afdoEnabled() {
return
}
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{ []blueprint.Variation{
{Mutator: "arch", Variation: ctx.Target().ArchVariation()}, {Mutator: "arch", Variation: ctx.Target().ArchVariation()},
{Mutator: "os", Variation: "android"}, {Mutator: "os", Variation: "android"},
}, },
FdoProfileTag, FdoProfileTag,
fdoProfileName) 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)
func afdoPropagateViaDepTag(tag blueprint.DependencyTag) bool { func afdoPropagateViaDepTag(tag blueprint.DependencyTag) bool {
libTag, isLibTag := tag.(libraryDependencyTag) libTag, isLibTag := tag.(libraryDependencyTag)
// Do not recurse down non-static dependencies // Do not recurse down non-static dependencies
@@ -188,27 +175,20 @@ func (a *afdoTransitionMutator) IncomingTransition(ctx android.IncomingTransitio
} }
func (a *afdoTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) { func (a *afdoTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) {
if variation == "" {
return
}
if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { 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.PreventInstall = true
m.Properties.HideFromMake = true m.Properties.HideFromMake = true
m.afdo.Properties.Afdo = true m.afdo.Properties.AfdoDep = true
if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(decodeTarget(variation)); fdoProfileName != "" && err == nil { m.afdo.addDep(ctx, decodeTarget(variation))
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())
}
}
} }
} }
} }

View File

@@ -55,7 +55,6 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) {
ctx.BottomUp("test_per_src", TestPerSrcMutator).Parallel() ctx.BottomUp("test_per_src", TestPerSrcMutator).Parallel()
ctx.BottomUp("version", versionMutator).Parallel() ctx.BottomUp("version", versionMutator).Parallel()
ctx.BottomUp("begin", BeginMutator).Parallel() ctx.BottomUp("begin", BeginMutator).Parallel()
ctx.BottomUp("fdo_profile", fdoProfileMutator).Parallel()
}) })
ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) { ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) {
@@ -526,7 +525,7 @@ type ModuleContextIntf interface {
selectedStl() string selectedStl() string
baseModuleName() string baseModuleName() string
getVndkExtendsModuleName() string getVndkExtendsModuleName() string
isAfdoCompile() bool isAfdoCompile(ctx ModuleContext) bool
isOrderfileCompile() bool isOrderfileCompile() bool
isCfi() bool isCfi() bool
isFuzzer() bool isFuzzer() bool
@@ -1380,9 +1379,9 @@ func (c *Module) IsVndk() bool {
return false return false
} }
func (c *Module) isAfdoCompile() bool { func (c *Module) isAfdoCompile(ctx ModuleContext) bool {
if afdo := c.afdo; afdo != nil { if afdo := c.afdo; afdo != nil {
return afdo.Properties.FdoProfilePath != nil return afdo.isAfdoCompile(ctx)
} }
return false return false
} }
@@ -1705,8 +1704,8 @@ func (ctx *moduleContextImpl) isVndk() bool {
return ctx.mod.IsVndk() return ctx.mod.IsVndk()
} }
func (ctx *moduleContextImpl) isAfdoCompile() bool { func (ctx *moduleContextImpl) isAfdoCompile(mctx ModuleContext) bool {
return ctx.mod.isAfdoCompile() return ctx.mod.isAfdoCompile(mctx)
} }
func (ctx *moduleContextImpl) isOrderfileCompile() bool { func (ctx *moduleContextImpl) isOrderfileCompile() bool {

View File

@@ -43,23 +43,10 @@ type FdoProfileInfo struct {
} }
// FdoProfileProvider is used to provide path to an fdo profile // FdoProfileProvider is used to provide path to an fdo profile
var FdoProfileProvider = blueprint.NewMutatorProvider[FdoProfileInfo]("fdo_profile") var FdoProfileProvider = blueprint.NewProvider[FdoProfileInfo]()
// 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)
// GenerateAndroidBuildActions of fdo_profile does not have any build actions // GenerateAndroidBuildActions of fdo_profile does not have any build actions
func (fp *fdoProfile) GenerateAndroidBuildActions(ctx android.ModuleContext) {} 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) {
if fp.properties.Profile != nil { if fp.properties.Profile != nil {
path := android.PathForModuleSrc(ctx, *fp.properties.Profile) path := android.PathForModuleSrc(ctx, *fp.properties.Profile)
android.SetProvider(ctx, FdoProfileProvider, FdoProfileInfo{ 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 { func FdoProfileFactory() android.Module {
m := &fdoProfile{} m := &fdoProfile{}
m.AddProperties(&m.properties) m.AddProperties(&m.properties)

View File

@@ -100,7 +100,7 @@ func (lto *lto) begin(ctx BaseModuleContext) {
lto.Properties.LtoEnabled = ltoEnabled 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. // TODO(b/131771163): CFI and Fuzzer controls LTO flags by themselves.
// This has be checked late because these properties can be mutated. // This has be checked late because these properties can be mutated.
if ctx.isCfi() || ctx.isFuzzer() { 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 // Reduce the inlining threshold for a better balance of binary size and
// performance. // performance.
if !ctx.Darwin() { if !ctx.Darwin() {
if ctx.isAfdoCompile() { if ctx.isAfdoCompile(ctx) {
ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=40") ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=40")
} else { } else {
ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=5") ltoLdFlags = append(ltoLdFlags, "-Wl,-plugin-opt,-import-instr-limit=5")