Skip to content

Commit

Permalink
Merged main:bbae59ae71c7 into amd-gfx:af6b7b051c01
Browse files Browse the repository at this point in the history
Local branch amd-gfx af6b7b0 Merged main:2f1399c73f52 into amd-gfx:88b84f4b16eb
Remote branch main bbae59a [clang-format] Finalize children after formatting them (llvm#73753)
  • Loading branch information
SC llvm team authored and SC llvm team committed Nov 29, 2023
2 parents af6b7b0 + bbae59a commit a5dbf94
Show file tree
Hide file tree
Showing 66 changed files with 2,232 additions and 880 deletions.
3 changes: 3 additions & 0 deletions bolt/include/bolt/Core/BinaryContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,9 @@ class BinaryContext {
/// Indicates if the binary contains split functions.
bool HasSplitFunctions{false};

/// Indicates if the function ordering of the binary is finalized.
bool HasFinalizedFunctionOrder{false};

/// Is the binary always loaded at a fixed address. Shared objects and
/// position-independent executables (PIEs) are examples of binaries that
/// will have HasFixedLoadAddress set to false.
Expand Down
5 changes: 4 additions & 1 deletion bolt/include/bolt/Passes/SplitFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ enum SplitFunctionsStrategy : char {
/// Split each function into a hot and cold fragment using profiling
/// information.
Profile2 = 0,
/// Split each function into a hot, warm, and cold fragment using
/// profiling information.
CDSplit,
/// Split each function into a hot and cold fragment at a randomly chosen
/// split point (ignoring any available profiling information).
Random2,
Expand All @@ -40,7 +43,7 @@ class SplitStrategy {

virtual ~SplitStrategy() = default;
virtual bool canSplit(const BinaryFunction &BF) = 0;
virtual bool keepEmpty() = 0;
virtual bool compactFragments() = 0;
virtual void fragment(const BlockIt Start, const BlockIt End) = 0;
};

Expand Down
2 changes: 2 additions & 0 deletions bolt/lib/Passes/ReorderFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ void ReorderFunctions::runOnFunctions(BinaryContext &BC) {

reorder(std::move(Clusters), BFs);

BC.HasFinalizedFunctionOrder = true;

std::unique_ptr<std::ofstream> FuncsFile;
if (!opts::GenerateFunctionOrderFile.empty()) {
FuncsFile = std::make_unique<std::ofstream>(opts::GenerateFunctionOrderFile,
Expand Down
85 changes: 77 additions & 8 deletions bolt/lib/Passes/SplitFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ static cl::opt<SplitFunctionsStrategy> SplitStrategy(
cl::values(clEnumValN(SplitFunctionsStrategy::Profile2, "profile2",
"split each function into a hot and cold fragment "
"using profiling information")),
cl::values(clEnumValN(SplitFunctionsStrategy::CDSplit, "cdsplit",
"split each function into a hot, warm, and cold "
"fragment using profiling information")),
cl::values(clEnumValN(
SplitFunctionsStrategy::Random2, "random2",
"split each function into a hot and cold fragment at a randomly chosen "
Expand Down Expand Up @@ -126,7 +129,7 @@ struct SplitProfile2 final : public SplitStrategy {
return BF.hasValidProfile() && hasFullProfile(BF) && !allBlocksCold(BF);
}

bool keepEmpty() override { return false; }
bool compactFragments() override { return true; }

void fragment(const BlockIt Start, const BlockIt End) override {
for (BinaryBasicBlock *const BB : llvm::make_range(Start, End)) {
Expand All @@ -136,14 +139,59 @@ struct SplitProfile2 final : public SplitStrategy {
}
};

struct SplitCacheDirected final : public SplitStrategy {
using BasicBlockOrder = BinaryFunction::BasicBlockOrderType;

bool canSplit(const BinaryFunction &BF) override {
return BF.hasValidProfile() && hasFullProfile(BF) && !allBlocksCold(BF);
}

// When some functions are hot-warm split and others are hot-warm-cold split,
// we do not want to change the fragment numbers of the blocks in the hot-warm
// split functions.
bool compactFragments() override { return false; }

void fragment(const BlockIt Start, const BlockIt End) override {
BasicBlockOrder BlockOrder(Start, End);
BinaryFunction &BF = *BlockOrder.front()->getFunction();

size_t BestSplitIndex = findSplitIndex(BF, BlockOrder);

// Assign fragments based on the computed best split index.
// All basic blocks with index up to the best split index become hot.
// All remaining blocks are warm / cold depending on if count is
// greater than 0 or not.
FragmentNum Main(0);
FragmentNum Cold(1);
FragmentNum Warm(2);
for (size_t Index = 0; Index < BlockOrder.size(); Index++) {
BinaryBasicBlock *BB = BlockOrder[Index];
if (Index <= BestSplitIndex)
BB->setFragmentNum(Main);
else
BB->setFragmentNum(BB->getKnownExecutionCount() > 0 ? Warm : Cold);
}
}

private:
/// Find the best index for splitting. The returned value is the index of the
/// last hot basic block. Hence, "no splitting" is equivalent to returning the
/// value which is one less than the size of the function.
size_t findSplitIndex(const BinaryFunction &BF,
const BasicBlockOrder &BlockOrder) {
// Placeholder: hot-warm split after entry block.
return 0;
}
};

struct SplitRandom2 final : public SplitStrategy {
std::minstd_rand0 Gen;

SplitRandom2() : Gen(opts::RandomSeed.getValue()) {}

bool canSplit(const BinaryFunction &BF) override { return true; }

bool keepEmpty() override { return false; }
bool compactFragments() override { return true; }

void fragment(const BlockIt Start, const BlockIt End) override {
using DiffT = typename std::iterator_traits<BlockIt>::difference_type;
Expand All @@ -170,7 +218,7 @@ struct SplitRandomN final : public SplitStrategy {

bool canSplit(const BinaryFunction &BF) override { return true; }

bool keepEmpty() override { return false; }
bool compactFragments() override { return true; }

void fragment(const BlockIt Start, const BlockIt End) override {
using DiffT = typename std::iterator_traits<BlockIt>::difference_type;
Expand Down Expand Up @@ -217,10 +265,10 @@ struct SplitRandomN final : public SplitStrategy {
struct SplitAll final : public SplitStrategy {
bool canSplit(const BinaryFunction &BF) override { return true; }

bool keepEmpty() override {
bool compactFragments() override {
// Keeping empty fragments allows us to test, that empty fragments do not
// generate symbols.
return true;
return false;
}

void fragment(const BlockIt Start, const BlockIt End) override {
Expand All @@ -246,10 +294,26 @@ void SplitFunctions::runOnFunctions(BinaryContext &BC) {
if (!opts::SplitFunctions)
return;

// If split strategy is not CDSplit, then a second run of the pass is not
// needed after function reordering.
if (BC.HasFinalizedFunctionOrder &&
opts::SplitStrategy != SplitFunctionsStrategy::CDSplit)
return;

std::unique_ptr<SplitStrategy> Strategy;
bool ForceSequential = false;

switch (opts::SplitStrategy) {
case SplitFunctionsStrategy::CDSplit:
// CDSplit runs two splitting passes: hot-cold splitting (SplitPrfoile2)
// before function reordering and hot-warm-cold splitting
// (SplitCacheDirected) after function reordering.
if (BC.HasFinalizedFunctionOrder)
Strategy = std::make_unique<SplitCacheDirected>();
else
Strategy = std::make_unique<SplitProfile2>();
opts::AggressiveSplitting = true;
break;
case SplitFunctionsStrategy::Profile2:
Strategy = std::make_unique<SplitProfile2>();
break;
Expand Down Expand Up @@ -382,7 +446,7 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
CurrentFragment = BB->getFragmentNum();
}

if (!S.keepEmpty()) {
if (S.compactFragments()) {
FragmentNum CurrentFragment = FragmentNum::main();
FragmentNum NewFragment = FragmentNum::main();
for (BinaryBasicBlock *const BB : NewLayout) {
Expand All @@ -394,7 +458,7 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
}
}

BF.getLayout().update(NewLayout);
const bool LayoutUpdated = BF.getLayout().update(NewLayout);

// For shared objects, invoke instructions and corresponding landing pads
// have to be placed in the same fragment. When we split them, create
Expand All @@ -404,7 +468,7 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
Trampolines = createEHTrampolines(BF);

// Check the new size to see if it's worth splitting the function.
if (BC.isX86() && BF.isSplit()) {
if (BC.isX86() && LayoutUpdated) {
std::tie(HotSize, ColdSize) = BC.calculateEmittedSize(BF);
LLVM_DEBUG(dbgs() << "Estimated size for function " << BF
<< " post-split is <0x" << Twine::utohexstr(HotSize)
Expand All @@ -431,6 +495,11 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
SplitBytesCold += ColdSize;
}
}

// Fix branches if the splitting decision of the pass after function
// reordering is different from that of the pass before function reordering.
if (LayoutUpdated && BC.HasFinalizedFunctionOrder)
BF.fixBranches();
}

SplitFunctions::TrampolineSetType
Expand Down
7 changes: 7 additions & 0 deletions bolt/lib/Rewrite/BinaryPassManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,13 @@ void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
Manager.registerPass(
std::make_unique<ReorderFunctions>(PrintReorderedFunctions));

// This is the second run of the SplitFunctions pass required by certain
// splitting strategies (e.g. cdsplit). Running the SplitFunctions pass again
// after ReorderFunctions allows the finalized function order to be utilized
// to make more sophisticated splitting decisions, like hot-warm-cold
// splitting.
Manager.registerPass(std::make_unique<SplitFunctions>(PrintSplit));

// Print final dyno stats right while CFG and instruction analysis are intact.
Manager.registerPass(
std::make_unique<DynoStatsPrintPass>(
Expand Down
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ C++2c Feature Support
This is applied to both C++ standard attributes, and other attributes supported by Clang.
This completes the implementation of `P2361R6 Unevaluated Strings <https://wg21.link/P2361R6>`_

- Implemented `P2864R2 Remove Deprecated Arithmetic Conversion on Enumerations From C++26 <https://wg21.link/P2864R2>`_.


Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 9 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -7220,6 +7220,11 @@ def warn_arith_conv_enum_float_cxx20 : Warning<
"%plural{2:with|4:from|:and}0 "
"%select{enumeration|floating-point}1 type %3 is deprecated">,
InGroup<DeprecatedEnumFloatConversion>;
def err_arith_conv_enum_float_cxx26 : Error<
"invalid %sub{select_arith_conv_kind}0 "
"%select{floating-point|enumeration}1 type %2 "
"%plural{2:with|4:from|:and}0 "
"%select{enumeration|floating-point}1 type %3">;
def warn_arith_conv_mixed_enum_types : Warning<
"%sub{select_arith_conv_kind}0 "
"different enumeration types%diff{ ($ and $)|}1,2">,
Expand All @@ -7228,6 +7233,10 @@ def warn_arith_conv_mixed_enum_types_cxx20 : Warning<
"%sub{select_arith_conv_kind}0 "
"different enumeration types%diff{ ($ and $)|}1,2 is deprecated">,
InGroup<DeprecatedEnumEnumConversion>;
def err_conv_mixed_enum_types_cxx26 : Error<
"invalid %sub{select_arith_conv_kind}0 "
"different enumeration types%diff{ ($ and $)|}1,2">;

def warn_arith_conv_mixed_anon_enum_types : Warning<
warn_arith_conv_mixed_enum_types.Summary>,
InGroup<AnonEnumEnumConversion>, DefaultIgnore;
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -3034,7 +3034,9 @@ struct FormatStyle {
bool isJson() const { return Language == LK_Json; }
bool isJavaScript() const { return Language == LK_JavaScript; }
bool isVerilog() const { return Language == LK_Verilog; }
bool isProto() const { return Language == LK_Proto; }
bool isProto() const {
return Language == LK_Proto || Language == LK_TextProto;
}

/// Language, this format style is targeted at.
/// \version 3.5
Expand Down
12 changes: 3 additions & 9 deletions clang/lib/Format/ContinuationIndenter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1229,9 +1229,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
return CurrentState.Indent;
}
if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
(Current.is(tok::greater) &&
(Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto))) &&
(Current.is(tok::greater) && Style.isProto())) &&
State.Stack.size() > 1) {
if (Current.closesBlockOrBlockTypeList(Style))
return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
Expand Down Expand Up @@ -1278,9 +1276,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
if (Current.is(tok::identifier) && Current.Next &&
(!Style.isVerilog() || Current.Next->is(tok::colon)) &&
(Current.Next->is(TT_DictLiteral) ||
((Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) &&
Current.Next->isOneOf(tok::less, tok::l_brace)))) {
(Style.isProto() && Current.Next->isOneOf(tok::less, tok::l_brace)))) {
return CurrentState.Indent;
}
if (NextNonComment->is(TT_ObjCStringLiteral) &&
Expand Down Expand Up @@ -1798,9 +1794,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
AvoidBinPacking = EndsInComma || Current.is(TT_DictLiteral) ||
Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto ||
!Style.BinPackArguments ||
Style.isProto() || !Style.BinPackArguments ||
(NextNonComment && NextNonComment->isOneOf(
TT_DesignatedInitializerPeriod,
TT_DesignatedInitializerLSquare));
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Format/FormatToken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ bool FormatToken::opensBlockOrBlockTypeList(const FormatStyle &Style) const {
(is(tok::l_brace) &&
(getBlockKind() == BK_Block || is(TT_DictLiteral) ||
(!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
(is(tok::less) && (Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto));
(is(tok::less) && Style.isProto());
}

TokenRole::~TokenRole() {}
Expand Down
5 changes: 1 addition & 4 deletions clang/lib/Format/FormatTokenLexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,11 +1313,8 @@ void FormatTokenLexer::readRawToken(FormatToken &Tok) {
}
}

if ((Style.isJavaScript() || Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) &&
Tok.is(tok::char_constant)) {
if ((Style.isJavaScript() || Style.isProto()) && Tok.is(tok::char_constant))
Tok.Tok.setKind(tok::string_literal);
}

if (Tok.is(tok::comment) && isClangFormatOn(Tok.TokenText))
FormattingDisabled = false;
Expand Down
Loading

0 comments on commit a5dbf94

Please sign in to comment.