This is an archive of the discontinued LLVM Phabricator instance.

Differential D157157

[clang] Add clang support for Machine Function Splitting on AArch64
ClosedPublic

Authored by dhoekwater on Aug 4 2023, 4:24 PM.

Details

Reviewers
shenhan
xur
snehasish
wenlei
mingmingl
Commits
rG776e3b052738: [clang] Add clang support for Machine Function Splitting on AArch64
Summary

On x86 targets, -fsplit-machine-functions enables splitting of machine
functions using profile information. This patch rolls out the flag for
AArch64 targets.

Depends on D158647

Diff Detail

Event Timeline

dhoekwater created this revision.Aug 4 2023, 4:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 4:24 PM
Herald added subscribers: pengfei, kristof.beyls. · View Herald Transcript
dhoekwater requested review of this revision.Aug 4 2023, 4:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 4:24 PM
Herald added subscribers: cfe-commits, MaskRay. · View Herald Transcript
Harbormaster completed remote builds in B250463: Diff 547393.Aug 4 2023, 4:25 PM
dhoekwater updated this revision to Diff 547884.Aug 7 2023, 11:38 AM
Harbormaster completed remote builds in B250864: Diff 547884.Aug 7 2023, 11:39 AM
dhoekwater added a subscriber: tmsriram.Aug 8 2023, 4:08 PM
snehasish mentioned this in D157750: Properly handle -fsplit-machine-functions for fatbinary compilation.Aug 11 2023, 1:30 PM
mingmingl accepted this revision.Aug 18 2023, 1:06 PM
mingmingl added inline comments.
clang/test/Driver/fsplit-machine-functions.c
2

nit: would -target aarch64 be sufficient here? if yes it's more general.

This revision is now accepted and ready to land.Aug 18 2023, 1:06 PM
MaskRay added inline comments.Aug 18 2023, 1:08 PM
clang/lib/Driver/ToolChains/Clang.cpp
5879–5880

AArch64.

Arm can mean AArch32 (often written as "ARM").

clang/test/Driver/fsplit-machine-functions.c
2

Use --target= for new tests. --target=aarch64 is sufficient and preferred for generic ELF features.

2

Just reuse CHECK-OPT

dhoekwater updated this revision to Diff 554070.Aug 28 2023, 3:15 PM
dhoekwater marked 4 inline comments as done.
dhoekwater edited the summary of this revision. (Show Details)
dhoekwater added a parent revision: D158647: [CodeGen][AArch64] Don't split inline asm goto blocks or their targets.

Make a comment more accurate and simplify test command functions

Harbormaster completed remote builds in B255320: Diff 554070.Aug 28 2023, 3:16 PM
dhoekwater added inline comments.Aug 28 2023, 3:45 PM
clang/lib/Driver/ToolChains/Clang.cpp
5879–5880

Nice catch, fixed.

clang/test/Driver/fsplit-machine-functions.c
2

Thanks for the feedback! Fixed.

2

Fixed.

dhoekwater updated this revision to Diff 554082.Aug 28 2023, 3:50 PM

Rebase onto main

Harbormaster completed remote builds in B255330: Diff 554082.Aug 28 2023, 3:50 PM
dhoekwater updated this revision to Diff 554094.Aug 28 2023, 4:17 PM

Hopefully get rid of a dead dependency

Harbormaster completed remote builds in B255340: Diff 554094.Aug 28 2023, 4:18 PM
dhoekwater updated this revision to Diff 554098.Aug 28 2023, 4:29 PM
dhoekwater edited the summary of this revision. (Show Details)

Remove a dependency to see if that fixes things.

Harbormaster completed remote builds in B255342: Diff 554098.Aug 28 2023, 4:29 PM
dhoekwater updated this revision to Diff 554099.Aug 28 2023, 4:30 PM
dhoekwater edited the summary of this revision. (Show Details)
dhoekwater added a parent revision: D157063: [CodeGen] Make the MachineFunctionSplitter pass testable via MIR.

Add the old dependency to hopefully get the change to propagate when I remove it

dhoekwater updated this revision to Diff 554100.Aug 28 2023, 4:31 PM
dhoekwater edited the summary of this revision. (Show Details)

Remove the dependency again

Harbormaster completed remote builds in B255344: Diff 554099.Aug 28 2023, 4:31 PM
Harbormaster completed remote builds in B255345: Diff 554100.
dhoekwater removed a parent revision: D157063: [CodeGen] Make the MachineFunctionSplitter pass testable via MIR.Aug 28 2023, 4:57 PM
Closed by commit rG776e3b052738: [clang] Add clang support for Machine Function Splitting on AArch64 (authored by dhoekwater). · Explain WhySep 6 2023, 2:02 PM
This revision was automatically updated to reflect the committed changes.
dhoekwater added a commit: rG776e3b052738: [clang] Add clang support for Machine Function Splitting on AArch64.

Revision Contents

PathSize
clang/
lib/
Driver/
ToolChains/
4 lines
test/
Driver/
1 line

Diff 556082

clang/lib/Driver/ToolChains/Clang.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 5,870 Lines • ▼ Show 20 Lines#endif
Args.addOptInFlag(CmdArgs, options::OPT_funique_internal_linkage_names, Args.addOptInFlag(CmdArgs, options::OPT_funique_internal_linkage_names,
options::OPT_fno_unique_internal_linkage_names); options::OPT_fno_unique_internal_linkage_names);
Args.addOptInFlag(CmdArgs, options::OPT_funique_basic_block_section_names, Args.addOptInFlag(CmdArgs, options::OPT_funique_basic_block_section_names,
options::OPT_fno_unique_basic_block_section_names); options::OPT_fno_unique_basic_block_section_names);
Args.addOptInFlag(CmdArgs, options::OPT_fconvergent_functions, Args.addOptInFlag(CmdArgs, options::OPT_fconvergent_functions,
options::OPT_fno_convergent_functions); options::OPT_fno_convergent_functions);
if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions, if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) { options::OPT_fno_split_machine_functions)) {
if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) { if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
MaskRayUnsubmitted
Done
ReplyInline Actions

AArch64.

Arm can mean AArch32 (often written as "ARM").

MaskRay: AArch64. Arm can mean AArch32 (often written as "ARM").
dhoekwaterAuthorUnsubmitted
Done
ReplyInline Actions

Nice catch, fixed.

dhoekwater: Nice catch, fixed.
// This codegen pass is only available on x86-elf targets. // This codegen pass is only available on x86 and AArch64 ELF targets.
if (Triple.isX86() && Triple.isOSBinFormatELF()) if ((Triple.isX86() || Triple.isAArch64()) && Triple.isOSBinFormatELF())
A->render(Args, CmdArgs); A->render(Args, CmdArgs);
else else
D.Diag(diag::err_drv_unsupported_opt_for_target) D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << TripleStr; << A->getAsString(Args) << TripleStr;
} }
} }
Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions, Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
▲ Show 20 Lines • Show All 2,832 Lines • Show Last 20 Lines

clang/test/Driver/fsplit-machine-functions.c

// RUN: %clang -### --target=x86_64 -fprofile-use=default.profdata -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefixes=CHECK,LTO-NEG // RUN: %clang -### --target=x86_64 -fprofile-use=default.profdata -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefixes=CHECK,LTO-NEG
// RUN: %clang -### --target=aarch64 -fprofile-use=default.profdata -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefixes=CHECK,LTO-NEG
mingminglUnsubmitted
Done
ReplyInline Actions

nit: would -target aarch64 be sufficient here? if yes it's more general.

mingmingl: nit: would `-target aarch64` be sufficient here? if yes it's more general.
MaskRayUnsubmitted
Done
ReplyInline Actions

Use --target= for new tests. --target=aarch64 is sufficient and preferred for generic ELF features.

MaskRay: Use `--target=` for new tests. `--target=aarch64` is sufficient and preferred for generic ELF…
dhoekwaterAuthorUnsubmitted
Done
ReplyInline Actions

Thanks for the feedback! Fixed.

dhoekwater: Thanks for the feedback! Fixed.
MaskRayUnsubmitted
Done
ReplyInline Actions

Just reuse CHECK-OPT

MaskRay: Just reuse CHECK-OPT
dhoekwaterAuthorUnsubmitted
Done
ReplyInline Actions

Fixed.

dhoekwater: Fixed.
// RUN: %clang -### --target=x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=NEG %s // RUN: %clang -### --target=x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=NEG %s
// CHECK: "-fsplit-machine-functions" // CHECK: "-fsplit-machine-functions"
// CHECK-SAME: "-fprofile-instrument-use-path=default.profdata" // CHECK-SAME: "-fprofile-instrument-use-path=default.profdata"
// NEG-NOT: "-fsplit-machine-functions" // NEG-NOT: "-fsplit-machine-functions"
// RUN: %clang -### --target=x86_64-linux -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=LTO // RUN: %clang -### --target=x86_64-linux -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=LTO
Show All 9 Lines