Merge "Enable -funique-internal-linkage-names when afdo=true" into udc-dev

2 files changed, 126 insertions, 30 deletions

diff --git a/cc/afdo.go b/cc/afdo.go
index be4f50adb..49f69873c 100644
--- a/cc/afdo.go
+++ b/cc/afdo.go
@@ -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,42 +143,41 @@ 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 {
-			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
-					}
+		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
 				}
-
-				if dep, ok := dep.(*Module); ok {
-					dep.afdo.Properties.AfdoRDeps = append(
-						dep.afdo.Properties.AfdoRDeps,
-						afdoRdep{
-							VariationName: proptools.StringPtr(encodeTarget(m.Name())),
-							ProfilePath:   path,
-						},
-					)
+			} else {
+				if tag != objDepTag && tag != reuseObjTag {
+					return false
 				}
+			}
 
-				return true
-			})
-		}
+			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 && 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]
 			}
 		}
diff --git a/cc/afdo_test.go b/cc/afdo_test.go
index 1c20bfc8c..b250ad1a1 100644
--- a/cc/afdo_test.go
+++ b/cc/afdo_test.go
@@ -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")
+	}
+}