Skip to content

Commit 0a55e61

Browse files
alexmalyshevfacebook-github-bot
alexmalyshev
authored and
facebook-github-bot
committed
Refactor HIRPrinter
Summary: Make setting the line prefix and controlling full printing of snapshots into explicit fields to be set. Currently the ctor and `Print(instr)` don't agree, `Print(cfg)` will do what's expected but `Print(instr)` silently overrides `full_snapshots_` to true. Remove unused `DebugPrint()`. Reviewed By: yoney Differential Revision: D78095240 fbshipit-source-id: 1f92ffcf3146ed2f2958da2b5d2ba894649d8789
1 parent 1c66b98 commit 0a55e61

9 files changed

Lines changed: 82 additions & 77 deletions

File tree

‎Jit/codegen/annotations.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ std::string Annotations::disassembleSection(
7373
auto new_hir = new_annot->instr;
7474
auto prev_hir = prev_annot ? prev_annot->instr : nullptr;
7575
if (new_hir != nullptr && new_hir != prev_hir) {
76-
annot_str = hir::HIRPrinter().ToString(*new_hir);
76+
annot_str =
77+
hir::HIRPrinter().setFullSnapshots(true).ToString(*new_hir);
7778
} else if (!new_annot->str.empty()) {
7879
annot_str = new_annot->str;
7980
}

‎Jit/hir/printer.cpp‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,7 @@ void HIRPrinter::Print(std::ostream& os, const Function& func) {
4444
}
4545

4646
void HIRPrinter::Print(std::ostream& os, const CFG& cfg) {
47-
return Print(os, cfg, cfg.entry_block);
48-
}
49-
50-
void HIRPrinter::Print(std::ostream& os, const CFG& cfg, BasicBlock* start) {
47+
auto start = cfg.entry_block;
5148
std::vector<BasicBlock*> blocks = cfg.GetRPOTraversal(start);
5249
auto last_block = blocks.back();
5350
for (auto block : blocks) {
@@ -78,7 +75,7 @@ void HIRPrinter::Print(std::ostream& os, const BasicBlock& block) {
7875
os << " {\n";
7976
Indent();
8077
for (auto& instr : block) {
81-
Print(os, instr, full_snapshots_);
78+
Print(os, instr);
8279
os << std::endl;
8380
}
8481
Dedent();
@@ -746,10 +743,7 @@ static std::string format_immediates(const Instr& instr) {
746743
JIT_ABORT("Invalid opcode {}", static_cast<int>(instr.opcode()));
747744
}
748745

749-
void HIRPrinter::Print(
750-
std::ostream& os,
751-
const Instr& instr,
752-
bool full_snapshots) {
746+
void HIRPrinter::Print(std::ostream& os, const Instr& instr) {
753747
Indented(os);
754748
if (Register* dst = instr.output()) {
755749
os << dst->name();
@@ -773,7 +767,7 @@ void HIRPrinter::Print(
773767
}
774768
}
775769

776-
if (instr.IsSnapshot() && !full_snapshots) {
770+
if (instr.IsSnapshot() && !full_snapshots_) {
777771
return;
778772
}
779773
auto fs = get_frame_state(instr);
@@ -868,20 +862,39 @@ void HIRPrinter::Print(std::ostream& os, const FrameState& state) {
868862
}
869863
}
870864

871-
void DebugPrint(const Function& func) {
872-
HIRPrinter(true).Print(std::cout, func);
865+
HIRPrinter& HIRPrinter::setFullSnapshots(bool full) {
866+
full_snapshots_ = full;
867+
return *this;
868+
}
869+
870+
HIRPrinter& HIRPrinter::setLinePrefix(std::string_view prefix) {
871+
line_prefix_ = std::string{prefix};
872+
return *this;
873+
}
874+
875+
std::ostream& operator<<(std::ostream& os, const Function& func) {
876+
HIRPrinter{}.Print(os, func);
877+
return os;
873878
}
874879

875-
void DebugPrint(const CFG& cfg) {
876-
HIRPrinter(true).Print(std::cout, cfg);
880+
std::ostream& operator<<(std::ostream& os, const CFG& cfg) {
881+
HIRPrinter{}.Print(os, cfg);
882+
return os;
877883
}
878884

879-
void DebugPrint(const BasicBlock& block) {
880-
HIRPrinter(true).Print(std::cout, block);
885+
std::ostream& operator<<(std::ostream& os, const BasicBlock& block) {
886+
HIRPrinter{}.Print(os, block);
887+
return os;
881888
}
882889

883-
void DebugPrint(const Instr& instr) {
884-
HIRPrinter(true).Print(std::cout, instr);
890+
std::ostream& operator<<(std::ostream& os, const Instr& instr) {
891+
HIRPrinter{}.Print(os, instr);
892+
return os;
893+
}
894+
895+
std::ostream& operator<<(std::ostream& os, const FrameState& state) {
896+
HIRPrinter{}.Print(os, state);
897+
return os;
885898
}
886899

887900
} // namespace jit::hir

‎Jit/hir/printer.h‎

Lines changed: 16 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <iostream>
99
#include <sstream>
1010
#include <string>
11+
#include <string_view>
1112

1213
namespace jit::hir {
1314

@@ -23,17 +24,15 @@ class HIRPrinter {
2324
// BasicBlock, CFG, or Function, will be printed only as the opcode name,
2425
// with no FrameState. When printing individual instructions, each caller can
2526
// specify whether or not the full instruction should be printed.
26-
explicit HIRPrinter(
27-
bool full_snapshots = false,
28-
const std::string& line_prefix = "")
29-
: full_snapshots_(full_snapshots), line_prefix_(line_prefix) {}
27+
HIRPrinter() = default;
3028

3129
void Print(std::ostream& os, const Function& func);
32-
void Print(std::ostream& os, const CFG& cfg);
3330
void Print(std::ostream& os, const BasicBlock& block);
34-
void Print(std::ostream& os, const Instr& instr, bool full_snapshots = true);
31+
void Print(std::ostream& os, const Instr& instr);
32+
33+
void Print(std::ostream& os, const CFG& cfg);
34+
3535
void Print(std::ostream& os, const FrameState& state);
36-
void Print(std::ostream& os, const CFG& cfg, BasicBlock* start);
3736

3837
template <class T>
3938
std::string ToString(const T& obj) {
@@ -47,45 +46,24 @@ class HIRPrinter {
4746
Print(std::cout, obj);
4847
}
4948

49+
HIRPrinter& setFullSnapshots(bool full);
50+
HIRPrinter& setLinePrefix(std::string_view prefix);
51+
5052
private:
5153
void Indent();
5254
void Dedent();
5355
std::ostream& Indented(std::ostream& os);
5456

55-
int indent_level_{0};
56-
bool full_snapshots_;
5757
std::string line_prefix_;
58+
int indent_level_{0};
59+
bool full_snapshots_{false};
5860
};
5961

60-
inline std::ostream& operator<<(std::ostream& os, const Function& func) {
61-
HIRPrinter().Print(os, func);
62-
return os;
63-
}
64-
65-
inline std::ostream& operator<<(std::ostream& os, const CFG& cfg) {
66-
HIRPrinter().Print(os, cfg);
67-
return os;
68-
}
69-
70-
inline std::ostream& operator<<(std::ostream& os, const BasicBlock& block) {
71-
HIRPrinter().Print(os, block);
72-
return os;
73-
}
74-
75-
inline std::ostream& operator<<(std::ostream& os, const Instr& instr) {
76-
HIRPrinter().Print(os, instr);
77-
return os;
78-
}
79-
80-
inline std::ostream& operator<<(std::ostream& os, const FrameState& state) {
81-
HIRPrinter().Print(os, state);
82-
return os;
83-
}
84-
85-
void DebugPrint(const Function& func);
86-
void DebugPrint(const CFG& cfg);
87-
void DebugPrint(const BasicBlock& block);
88-
void DebugPrint(const Instr& instr);
62+
std::ostream& operator<<(std::ostream& os, const Function& func);
63+
std::ostream& operator<<(std::ostream& os, const CFG& cfg);
64+
std::ostream& operator<<(std::ostream& os, const BasicBlock& block);
65+
std::ostream& operator<<(std::ostream& os, const Instr& instr);
66+
std::ostream& operator<<(std::ostream& os, const FrameState& state);
8967

9068
} // namespace jit::hir
9169

‎Jit/hir/refcount_insertion.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,7 @@ void RefcountInsertion::Run(Function& func) {
12621262
TRACE(
12631263
"Starting refcount insertion for '{}':\n{}",
12641264
func.fullname,
1265-
HIRPrinter(true).ToString(func));
1265+
HIRPrinter{}.setFullSnapshots(true).ToString(func));
12661266
Env env{func};
12671267

12681268
auto rpo_blocks = func.cfg.GetRPOTraversal();

‎Jit/lir/printer.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313

1414
namespace jit::lir {
1515

16+
Printer::Printer() {
17+
hir_printer_.setFullSnapshots(true);
18+
hir_printer_.setLinePrefix("# ");
19+
}
20+
1621
void Printer::print(std::ostream& out, const Function& func) {
1722
out << "Function:" << std::endl;
1823
for (auto& block : func.basicblocks()) {

‎Jit/lir/printer.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ namespace jit::lir {
1515

1616
class Printer {
1717
public:
18+
Printer();
19+
1820
void print(std::ostream& out, const Function& func);
1921
void print(std::ostream& out, const BasicBlock& block);
2022
void print(std::ostream& out, const Instruction& instr);
@@ -32,7 +34,7 @@ class Printer {
3234
return getFunction(*opnd.instr());
3335
}
3436

35-
hir::HIRPrinter hir_printer_{false, "# "};
37+
hir::HIRPrinter hir_printer_;
3638
};
3739

3840
inline std::ostream& operator<<(std::ostream& out, const Function& func) {

‎RuntimeTests/hir_frame_state_test.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ TEST_F(FrameStateCreationTest, InitialInstrOffset) {
1818
EXPECT_EQ(frame.cur_instr_offs.value(), -sizeof(_Py_CODEUNIT));
1919
}
2020

21-
#define EXPECT_HIR_EQ(irfunc, expected) \
22-
{ \
23-
ASSERT_TRUE(irfunc != nullptr); \
24-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected); \
21+
#define EXPECT_HIR_EQ(irfunc, expected) \
22+
{ \
23+
ASSERT_TRUE(irfunc != nullptr); \
24+
EXPECT_EQ( \
25+
HIRPrinter{}.setFullSnapshots(true).ToString(*(irfunc)), expected); \
2526
}
2627

2728
TEST_F(FrameStateCreationTest, LoadGlobal) {

‎RuntimeTests/hir_guard_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ static void testFillGuards(const char* hir_source, const char* expected) {
1818
ASSERT_TRUE(checkFunc(*func, std::cout));
1919
reflowTypes(*func);
2020
RefcountInsertion().Run(*func);
21-
ASSERT_EQ(HIRPrinter(true).ToString(*func), expected);
21+
ASSERT_EQ(HIRPrinter{}.setFullSnapshots(true).ToString(*func), expected);
2222
}
2323

2424
TEST_F(GuardTest, BindFrameStateFromBlock) {

‎RuntimeTests/hir_test.cpp‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Copyright (c) Meta Platforms, Inc. and affiliates.
2+
23
#include <Python.h>
34

45
#include <gtest/gtest.h>
@@ -21,6 +22,10 @@
2122
using namespace jit;
2223
using namespace jit::hir;
2324

25+
HIRPrinter fullPrinter() {
26+
return HIRPrinter{}.setFullSnapshots(true);
27+
}
28+
2429
TEST(BasicBlockTest, CanAppendInstrs) {
2530
Environment env;
2631
BasicBlock block;
@@ -630,7 +635,7 @@ TEST_F(HIRBuildTest, GetLength) {
630635
}
631636
)";
632637
#endif
633-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
638+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
634639
}
635640

636641
TEST_F(HIRBuildTest, LoadAssertionError) {
@@ -692,7 +697,7 @@ TEST_F(HIRBuildTest, LoadAssertionError) {
692697
}
693698
)";
694699
#endif
695-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
700+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
696701
}
697702

698703
TEST_F(HIRBuildTest, SetUpdate) {
@@ -835,7 +840,7 @@ TEST_F(HIRBuildTest, SetUpdate) {
835840
}
836841
)";
837842
#endif
838-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
843+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
839844
}
840845

841846
class EdgeCaseTest : public RuntimeTest {};
@@ -921,7 +926,7 @@ TEST_F(EdgeCaseTest, IgnoreUnreachableLoops) {
921926
}
922927
)";
923928
#endif
924-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
929+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
925930
}
926931

927932
TEST_F(EdgeCaseTest, JumpBackwardNoInterrupt) {
@@ -1006,7 +1011,7 @@ TEST_F(EdgeCaseTest, JumpBackwardNoInterrupt) {
10061011
}
10071012
)";
10081013
#endif
1009-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1014+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
10101015
}
10111016

10121017
class CppInlinerTest : public RuntimeTest {};
@@ -1171,7 +1176,7 @@ TEST_F(HIRCloneTest, CanCloneDeoptBase) {
11711176
}
11721177
}
11731178
)";
1174-
ASSERT_EQ(HIRPrinter(true).ToString(*irfunc), expected);
1179+
ASSERT_EQ(fullPrinter().ToString(*irfunc), expected);
11751180
BasicBlock* bb0 = irfunc->cfg.entry_block;
11761181
Instr& load_global = *(++(bb0->rbegin()));
11771182
ASSERT_TRUE(load_global.IsLoadGlobal());
@@ -1295,7 +1300,7 @@ TEST_F(HIRBuildTest, ROT_N) {
12951300
}
12961301
)";
12971302

1298-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1303+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
12991304
}
13001305
#endif
13011306

@@ -1383,7 +1388,7 @@ TEST_F(HIRBuildTest, MatchMapping) {
13831388
}
13841389
)";
13851390
#endif
1386-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1391+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
13871392
}
13881393

13891394
TEST_F(HIRBuildTest, MatchSequence) {
@@ -1470,7 +1475,7 @@ TEST_F(HIRBuildTest, MatchSequence) {
14701475
}
14711476
)";
14721477
#endif
1473-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1478+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
14741479
}
14751480

14761481
TEST_F(HIRBuildTest, MatchKeys) {
@@ -1580,7 +1585,7 @@ TEST_F(HIRBuildTest, MatchKeys) {
15801585
}
15811586
)";
15821587
#endif
1583-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1588+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
15841589
}
15851590

15861591
TEST_F(HIRBuildTest, ListExtend) {
@@ -1650,7 +1655,7 @@ TEST_F(HIRBuildTest, ListExtend) {
16501655
}
16511656
)";
16521657
#endif
1653-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1658+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
16541659
}
16551660

16561661
TEST_F(HIRBuildTest, ListToTuple) {
@@ -1717,7 +1722,7 @@ TEST_F(HIRBuildTest, ListToTuple) {
17171722
}
17181723
)";
17191724
#endif
1720-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1725+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
17211726
}
17221727

17231728
TEST_F(HIRBuildTest, LoadFastAndClear) {
@@ -1749,6 +1754,6 @@ TEST_F(HIRBuildTest, LoadFastAndClear) {
17491754
}
17501755
)";
17511756

1752-
EXPECT_EQ(HIRPrinter(true).ToString(*(irfunc)), expected);
1757+
EXPECT_EQ(fullPrinter().ToString(*(irfunc)), expected);
17531758
#endif
17541759
}

0 commit comments

Comments
 (0)