Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 01c4456c authored by Yabin Cui's avatar Yabin Cui
Browse files

Enable -funique-internal-linkage-names when afdo=true

We used to enable unique names only when a profile is available.
But A profile generated for a binary without unique names doesn't
work well when building a binary with unique names. To avoid a
chicken-and-egg problem, this CL enables the flag when afdo=true,
whether a profile exists or not.

Bug: 241523910
Test: build
Change-Id: I74d834510d81d2db76e34d0488c74a60e1fcecd5
parent 0bf4ea53
Loading
Loading
Loading
Loading
+44 −30
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ var (

var afdoProfileProjectsConfigKey = android.NewOnceKey("AfdoProfileProjects")

const afdoCFlagsFormat = "-funique-internal-linkage-names -fprofile-sample-accurate -fprofile-sample-use=%s"
const afdoCFlagsFormat = "-fprofile-sample-accurate -fprofile-sample-use=%s"

func recordMissingAfdoProfileFile(ctx android.BaseModuleContext, missing string) {
	getNamedMapForConfig(ctx.Config(), modulesMissingProfileFileKey).Store(missing, true)
@@ -66,10 +66,24 @@ func (afdo *afdo) props() []interface{} {
// afdoEnabled returns true for binaries and shared libraries
// that set afdo prop to True and there is a profile available
func (afdo *afdo) afdoEnabled() bool {
	return afdo != nil && afdo.Properties.Afdo && afdo.Properties.FdoProfilePath != nil
	return afdo != nil && afdo.Properties.Afdo
}

func (afdo *afdo) flags(ctx ModuleContext, flags Flags) Flags {
	if afdo.Properties.Afdo {
		// 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
		// unique names (they have different internal function names).
		// To avoid a chicken-and-egg problem, we enable `-funique-internal-linkage-names` when
		// `afdo=true`, whether a profile exists or not.
		// The profile can take effect in three steps:
		// 1. Add `afdo: true` in Android.bp, and build the binary.
		// 2. Collect an AutoFDO profile for the binary.
		// 3. Make the profile searchable by the build system. So it's used the next time the binary
		//	  is built.
		flags.Local.CFlags = append([]string{"-funique-internal-linkage-names"}, flags.Local.CFlags...)
	}
	if path := afdo.Properties.FdoProfilePath; path != nil {
		// The flags are prepended to allow overriding.
		profileUseFlag := fmt.Sprintf(afdoCFlagsFormat, *path)
@@ -129,7 +143,7 @@ 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() {
		if path := m.afdo.Properties.FdoProfilePath; path != nil {
		path := m.afdo.Properties.FdoProfilePath
		mctx.WalkDeps(func(dep android.Module, parent android.Module) bool {
			tag := mctx.OtherModuleDependencyTag(dep)
			libTag, isLibTag := tag.(libraryDependencyTag)
@@ -159,12 +173,11 @@ func afdoDepsMutator(mctx android.TopDownMutatorContext) {
		})
	}
}
}

// 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 && m.afdo.Properties.FdoProfilePath != nil {
		if !m.static() && m.afdo.Properties.Afdo {
			mctx.SetDependencyVariation(encodeTarget(m.Name()))
			return
		}
@@ -182,7 +195,7 @@ func afdoMutator(mctx android.BottomUpMutatorContext) {
			//                   \                      ^
			//                    ----------------------|
			// We only need to create one variant per unique rdep
			if variantNameToProfilePath[variantName] == nil {
			if _, exists := variantNameToProfilePath[variantName]; !exists {
				variationNames = append(variationNames, variantName)
				variantNameToProfilePath[variantName] = afdoRDep.ProfilePath
			}
@@ -197,6 +210,7 @@ func afdoMutator(mctx android.BottomUpMutatorContext) {
				variation := modules[i].(*Module)
				variation.Properties.PreventInstall = true
				variation.Properties.HideFromMake = true
				variation.afdo.Properties.Afdo = true
				variation.afdo.Properties.FdoProfilePath = variantNameToProfilePath[name]
			}
		}
+82 −0
Original line number Diff line number Diff line
@@ -379,3 +379,85 @@ func TestMultipleAfdoRDeps(t *testing.T) {
		t.Errorf("libFoo missing dependency on non-afdo variant of libBar")
	}
}

func TestAfdoDepsWithoutProfile(t *testing.T) {
	t.Parallel()
	bp := `
	cc_library_shared {
		name: "libTest",
		srcs: ["test.c"],
		static_libs: ["libFoo"],
		afdo: true,
	}

	cc_library_static {
		name: "libFoo",
		srcs: ["foo.c"],
		static_libs: ["libBar"],
	}

	cc_library_static {
		name: "libBar",
		srcs: ["bar.c"],
	}
	`

	result := android.GroupFixturePreparers(
		PrepareForTestWithFdoProfile,
		prepareForCcTest,
	).RunTestWithBp(t, bp)

	// Even without a profile path, the afdo enabled libraries should be built with
	// -funique-internal-linkage-names.
	expectedCFlag := "-funique-internal-linkage-names"

	libTest := result.ModuleForTests("libTest", "android_arm64_armv8-a_shared")
	libFooAfdoVariant := result.ModuleForTests("libFoo", "android_arm64_armv8-a_static_afdo-libTest")
	libBarAfdoVariant := result.ModuleForTests("libBar", "android_arm64_armv8-a_static_afdo-libTest")

	// Check cFlags of afdo-enabled module and the afdo-variant of its static deps
	cFlags := libTest.Rule("cc").Args["cFlags"]
	if !strings.Contains(cFlags, expectedCFlag) {
		t.Errorf("Expected 'libTest' to enable afdo, but did not find %q in cflags %q", expectedCFlag, cFlags)
	}

	cFlags = libFooAfdoVariant.Rule("cc").Args["cFlags"]
	if !strings.Contains(cFlags, expectedCFlag) {
		t.Errorf("Expected 'libFooAfdoVariant' to enable afdo, but did not find %q in cflags %q", expectedCFlag, cFlags)
	}

	cFlags = libBarAfdoVariant.Rule("cc").Args["cFlags"]
	if !strings.Contains(cFlags, expectedCFlag) {
		t.Errorf("Expected 'libBarAfdoVariant' to enable afdo, but did not find %q in cflags %q", expectedCFlag, cFlags)
	}
	// Check dependency edge from afdo-enabled module to static deps
	if !hasDirectDep(result, libTest.Module(), libFooAfdoVariant.Module()) {
		t.Errorf("libTest missing dependency on afdo variant of libFoo")
	}

	if !hasDirectDep(result, libFooAfdoVariant.Module(), libBarAfdoVariant.Module()) {
		t.Errorf("libTest missing dependency on afdo variant of libBar")
	}

	// Verify non-afdo variant exists and doesn't contain afdo
	libFoo := result.ModuleForTests("libFoo", "android_arm64_armv8-a_static")
	libBar := result.ModuleForTests("libBar", "android_arm64_armv8-a_static")

	cFlags = libFoo.Rule("cc").Args["cFlags"]
	if strings.Contains(cFlags, expectedCFlag) {
		t.Errorf("Expected 'libFoo' to not enable afdo, but found %q in cflags %q", expectedCFlag, cFlags)
	}
	cFlags = libBar.Rule("cc").Args["cFlags"]
	if strings.Contains(cFlags, expectedCFlag) {
		t.Errorf("Expected 'libBar' to not enable afdo, but found %q in cflags %q", expectedCFlag, cFlags)
	}

	// Check dependency edges of static deps
	if hasDirectDep(result, libTest.Module(), libFoo.Module()) {
		t.Errorf("libTest should not depend on non-afdo variant of libFoo")
	}

	if !hasDirectDep(result, libFoo.Module(), libBar.Module()) {
		t.Errorf("libFoo missing dependency on non-afdo variant of libBar")
	}
}