Merge changes from topic "revert-2925209-KVPWEBRQHT" into main

* changes:
  Revert "Convert AFDO mutators to TransitionMutator"
  Revert "Remove fdoProfileMutator"
This commit is contained in:
Ke-Yu Lu 2024-02-06 05:46:07 +00:00 committed by Gerrit Code Review
commit 196d9493c4
6 changed files with 167 additions and 81 deletions

View file

@ -1493,18 +1493,18 @@ func (c *deviceConfig) PgoAdditionalProfileDirs() []string {
} }
// AfdoProfile returns fully qualified path associated to the given module name // 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 { for _, afdoProfile := range c.config.productVariables.AfdoProfiles {
split := strings.Split(afdoProfile, ":") split := strings.Split(afdoProfile, ":")
if len(split) != 3 { if len(split) != 3 {
return "", fmt.Errorf("AFDO_PROFILES has invalid value: %s. "+ return nil, fmt.Errorf("AFDO_PROFILES has invalid value: %s. "+
"The expected format is <module>:<fully-qualified-path-to-fdo_profile>", afdoProfile) "The expected format is <module>:<fully-qualified-path-to-fdo_profile>", afdoProfile)
} }
if split[0] == name { if split[0] == name {
return strings.Join([]string{split[1], split[2]}, ":"), nil return proptools.StringPtr(strings.Join([]string{split[1], split[2]}, ":")), nil
} }
} }
return "", nil return nil, nil
} }
func (c *deviceConfig) VendorSepolicyDirs() []string { func (c *deviceConfig) VendorSepolicyDirs() []string {

View file

@ -21,17 +21,29 @@ 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
const afdoFlagsFormat = "-fprofile-sample-use=%s -fprofile-sample-accurate" 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 { type AfdoProperties struct {
// Afdo allows developers self-service enroll for // Afdo allows developers self-service enroll for
// automatic feedback-directed optimization using profile data. // automatic feedback-directed optimization using profile data.
Afdo bool Afdo bool
AfdoDep bool `blueprint:"mutated"` FdoProfilePath *string `blueprint:"mutated"`
AfdoRDeps []afdoRdep `blueprint:"mutated"`
} }
type afdo struct { type afdo struct {
@ -56,7 +68,7 @@ func (afdo *afdo) afdoEnabled() bool {
} }
func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags { func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags {
if afdo.Properties.Afdo || afdo.Properties.AfdoDep { if afdo.Properties.Afdo {
// 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
@ -74,91 +86,138 @@ 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...)
} }
fdoProfileDeps := ctx.GetDirectDepsWithTag(FdoProfileTag) if path := afdo.Properties.FdoProfilePath; path != nil {
if len(fdoProfileDeps) > 0 && fdoProfileDeps[0] != nil { // The flags are prepended to allow overriding.
if info, ok := android.OtherModuleProvider(ctx, fdoProfileDeps[0], FdoProfileProvider); ok { profileUseFlag := fmt.Sprintf(afdoFlagsFormat, *path)
fdoProfilePath := info.Path.String() 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...)
// The flags are prepended to allow overriding. // Update CFlagsDeps and LdFlagsDeps so the module is rebuilt
profileUseFlag := fmt.Sprintf(afdoFlagsFormat, fdoProfilePath) // if profileFile gets updated
flags.Local.CFlags = append([]string{profileUseFlag}, flags.Local.CFlags...) pathForSrc := android.PathForSource(ctx, *path)
flags.Local.LdFlags = append([]string{profileUseFlag, "-Wl,-mllvm,-no-warn-sample-unused=true"}, flags.Local.LdFlags...) flags.CFlagsDeps = append(flags.CFlagsDeps, pathForSrc)
flags.LdFlagsDeps = append(flags.LdFlagsDeps, pathForSrc)
// 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 return flags
} }
func (a *afdo) addDep(ctx android.BottomUpMutatorContext, fdoProfileTarget string) { func (afdo *afdo) addDep(ctx BaseModuleContext, actx android.BottomUpMutatorContext) {
if fdoProfileName, err := ctx.DeviceConfig().AfdoProfile(fdoProfileTarget); fdoProfileName != "" && err == nil { if ctx.Host() {
ctx.AddFarVariationDependencies( return
[]blueprint.Variation{
{Mutator: "arch", Variation: ctx.Target().ArchVariation()},
{Mutator: "os", Variation: "android"},
},
FdoProfileTag,
fdoProfileName)
} }
}
func afdoPropagateViaDepTag(tag blueprint.DependencyTag) bool { if ctx.static() && !ctx.staticBinary() {
libTag, isLibTag := tag.(libraryDependencyTag) return
// 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. if c, ok := ctx.Module().(*Module); ok && c.Enabled() {
type afdoTransitionMutator struct{} if fdoProfileName, err := actx.DeviceConfig().AfdoProfile(actx.ModuleName()); fdoProfileName != nil && err == nil {
actx.AddFarVariationDependencies(
func (a *afdoTransitionMutator) Split(ctx android.BaseModuleContext) []string { []blueprint.Variation{
return []string{""} {Mutator: "arch", Variation: actx.Target().ArchVariation()},
} {Mutator: "os", Variation: "android"},
},
func (a *afdoTransitionMutator) OutgoingTransition(ctx android.OutgoingTransitionContext, sourceVariation string) string { FdoProfileTag,
if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { []string{*fdoProfileName}...,
if !afdoPropagateViaDepTag(ctx.DepTag()) { )
return ""
}
if sourceVariation != "" {
return sourceVariation
}
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 { // FdoProfileMutator reads the FdoProfileProvider from a direct dep with FdoProfileTag
if m, ok := ctx.Module().(*Module); ok && m.afdo != nil { // assigns FdoProfileInfo.Path to the FdoProfilePath mutated property
return incomingVariation func (c *Module) fdoProfileMutator(ctx android.BottomUpMutatorContext) {
if !c.Enabled() {
return
} }
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())
}
})
} }
func (a *afdoTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) { var _ FdoProfileMutatorInterface = (*Module)(nil)
if m, ok := ctx.Module().(*Module); ok && m.afdo != nil {
if variation == "" { // Propagate afdo requirements down from binaries and shared libraries
if m.afdo.afdoEnabled() && !(m.static() && !m.staticBinary()) && !m.Host() { func afdoDepsMutator(mctx android.TopDownMutatorContext) {
m.afdo.addDep(ctx, ctx.ModuleName()) 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 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]
} }
} else {
m.Properties.PreventInstall = true
m.Properties.HideFromMake = true
m.afdo.Properties.AfdoDep = true
m.afdo.addDep(ctx, decodeTarget(variation))
} }
} }
} }

View file

@ -174,11 +174,11 @@ func TestAfdoEnabledOnStaticDepNoAfdo(t *testing.T) {
libBar := result.ModuleForTests("libBar", "android_arm64_armv8-a_static").Module() libBar := result.ModuleForTests("libBar", "android_arm64_armv8-a_static").Module()
if !hasDirectDep(result, libTest, libFoo.Module()) { if !hasDirectDep(result, libTest, libFoo.Module()) {
t.Errorf("libTest missing dependency on non-afdo variant of libFoo") t.Errorf("libTest missing dependency on afdo variant of libFoo")
} }
if !hasDirectDep(result, libFoo.Module(), libBar) { if !hasDirectDep(result, libFoo.Module(), libBar) {
t.Errorf("libFoo missing dependency on non-afdo variant of libBar") t.Errorf("libFoo missing dependency on afdo variant of libBar")
} }
fooVariants := result.ModuleVariantsForTests("foo") fooVariants := result.ModuleVariantsForTests("foo")

View file

@ -55,6 +55,7 @@ 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)
}) })
ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) { ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) {
@ -69,7 +70,8 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) {
ctx.Transition("coverage", &coverageTransitionMutator{}) ctx.Transition("coverage", &coverageTransitionMutator{})
ctx.Transition("afdo", &afdoTransitionMutator{}) ctx.TopDown("afdo_deps", afdoDepsMutator)
ctx.BottomUp("afdo", afdoMutator).Parallel()
ctx.Transition("orderfile", &orderfileTransitionMutator{}) ctx.Transition("orderfile", &orderfileTransitionMutator{})
@ -1381,7 +1383,7 @@ func (c *Module) IsVndk() bool {
func (c *Module) isAfdoCompile() bool { func (c *Module) isAfdoCompile() bool {
if afdo := c.afdo; afdo != nil { if afdo := c.afdo; afdo != nil {
return afdo.Properties.Afdo || afdo.Properties.AfdoDep return afdo.Properties.FdoProfilePath != nil
} }
return false return false
} }
@ -2349,6 +2351,10 @@ func (c *Module) beginMutator(actx android.BottomUpMutatorContext) {
} }
ctx.ctx = ctx ctx.ctx = ctx
if !actx.Host() || !ctx.static() || ctx.staticBinary() {
c.afdo.addDep(ctx, actx)
}
c.begin(ctx) c.begin(ctx)
} }

View file

@ -43,10 +43,23 @@ 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.NewProvider[FdoProfileInfo]() 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)
// 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{
@ -55,6 +68,14 @@ func (fp *fdoProfile) GenerateAndroidBuildActions(ctx android.ModuleContext) {
} }
} }
// 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

@ -44,14 +44,14 @@ func (afdo *afdo) addDep(ctx BaseModuleContext, actx android.BottomUpMutatorCont
if err != nil { if err != nil {
ctx.ModuleErrorf("%s", err.Error()) ctx.ModuleErrorf("%s", err.Error())
} }
if fdoProfileName != "" { if fdoProfileName != nil {
actx.AddFarVariationDependencies( actx.AddFarVariationDependencies(
[]blueprint.Variation{ []blueprint.Variation{
{Mutator: "arch", Variation: actx.Target().ArchVariation()}, {Mutator: "arch", Variation: actx.Target().ArchVariation()},
{Mutator: "os", Variation: "android"}, {Mutator: "os", Variation: "android"},
}, },
cc.FdoProfileTag, cc.FdoProfileTag,
[]string{fdoProfileName}..., []string{*fdoProfileName}...,
) )
} }
} }