Skip to content

Commit 0cb1dad

Browse files
review comment
Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
1 parent 20945b4 commit 0cb1dad

8 files changed

Lines changed: 76 additions & 37 deletions

File tree

src/cpp/encoder/aie2ps/aie2ps_encoder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ process(std::shared_ptr<preprocessed_output> input)
6060
auto& totalcoldata = tinput->get_coldata();
6161
auto& totalsyms = tinput->get_symbols();
6262
m_debug.set_annotations(tinput->get_annotations());
63+
m_debug.set_filename_table(tinput->get_filename_table());
6364
uint32_t optimizatiom_level = tinput->get_optimization_level();
6465
auto& ctrlpkt = tinput->get_ctrlpkt();
6566
auto& ctrlpkt_id_map = tinput->get_ctrlpkt_id_map();

src/cpp/encoder/aie2ps/report.h

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,27 @@ class Function {
6262

6363
// Takes a file index instead of the filename
6464
// string, avoiding a heap allocation per Function object.
65-
Function(uint32_t file_idx, const std::string& name, offset_type high_pc, offset_type low_pc, uint32_t col, pageid_type pagenum)
66-
: m_file_idx(file_idx), m_name(name), m_colnum(col), m_pagenum(pagenum), m_highpc(high_pc), m_lowpc(low_pc) {}
65+
Function(std::shared_ptr<const detail::filename_table> filename_table,
66+
uint32_t file_idx, const std::string& name, offset_type high_pc, offset_type low_pc,
67+
uint32_t col, pageid_type pagenum)
68+
: m_filename_table(std::move(filename_table)), m_file_idx(file_idx), m_name(name),
69+
m_colnum(col), m_pagenum(pagenum), m_highpc(high_pc), m_lowpc(low_pc) {}
6770

6871
void add_textline(std::shared_ptr<Line> line) { m_textlines.push_back(std::move(line)); }
6972
void add_dataline(std::shared_ptr<Line> line) { m_datalines.push_back(std::move(line)); }
7073

7174
const std::vector<std::shared_ptr<Line>>& get_textlines() const { return m_textlines; }
7275
const std::vector<std::shared_ptr<Line>>& get_datalines() const { return m_datalines; }
7376

74-
// Looks up the filename string on demand from the intern table.
75-
const std::string& get_filename() const { return detail::lookup_filename(m_file_idx); }
77+
const std::string& get_filename() const { return m_filename_table->lookup_filename(m_file_idx); }
7678
const std::string& get_name() const { return m_name; }
7779
uint32_t get_column() const { return m_colnum; }
7880
pageid_type get_pagenum() const { return m_pagenum; }
7981
offset_type get_highPc() const { return m_highpc; }
8082
offset_type get_lowPc() const { return m_lowpc; }
8183

8284
private:
85+
std::shared_ptr<const detail::filename_table> m_filename_table;
8386
uint32_t m_file_idx;
8487
std::string m_name;
8588
uint32_t m_colnum;
@@ -95,11 +98,15 @@ class Debug {
9598
m_annotation_list = std::move(annotations);
9699
}
97100

101+
void set_filename_table(std::shared_ptr<const detail::filename_table> table) {
102+
m_filename_table = std::move(table);
103+
}
104+
98105
std::string add_function(uint32_t file_idx, const std::string& name, offset_type high_pc, offset_type low_pc, uint32_t col, pageid_type pagenum) {
99106
// source/file/column/page do not overwrite previously recorded functions.
100107
std::string key = std::to_string(file_idx) + "_" + std::to_string(col) + "_"
101108
+ std::to_string(pagenum) + "_" + name;
102-
functions[key] = std::make_shared<Function>(file_idx, name, high_pc, low_pc, col, pagenum);
109+
functions[key] = std::make_shared<Function>(m_filename_table, file_idx, name, high_pc, low_pc, col, pagenum);
103110
insertion_order.push_back(key);
104111
return key;
105112
}
@@ -135,6 +142,7 @@ class Debug {
135142
}
136143

137144
private:
145+
std::shared_ptr<const detail::filename_table> m_filename_table;
138146
std::map<std::string, std::shared_ptr<Function>> functions;
139147
std::vector<std::string> insertion_order;
140148
std::vector<annotation_type> m_annotation_list;

src/cpp/preprocessor/aie2ps/aie2ps_preprocessed_output.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class aie2ps_preprocessed_output : public preprocessed_output
3232
std::shared_ptr<const aie_row_topology_info> m_aie_row_topology;
3333
std::map<std::string, std::vector<char>> m_ctrlpkt;
3434
std::map<uint32_t, std::string> m_ctrlpkt_id_map;
35+
std::shared_ptr<const detail::filename_table> m_filename_table;
3536
public:
3637

3738
explicit aie2ps_preprocessed_output(std::shared_ptr<const partition_info> partition,
@@ -107,6 +108,14 @@ class aie2ps_preprocessed_output : public preprocessed_output
107108
{
108109
m_ctrlpkt_id_map = ctrlpkt_id_map;
109110
}
111+
112+
void set_filename_table(std::shared_ptr<const detail::filename_table> table) {
113+
m_filename_table = std::move(table);
114+
}
115+
116+
const std::shared_ptr<const detail::filename_table>& get_filename_table() const {
117+
return m_filename_table;
118+
}
110119
};
111120

112121
template <typename T>

src/cpp/preprocessor/aie2ps/aie2ps_preprocessor.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ class aie2ps_preprocessor: public preprocessor
182182
toutput->set_ctrlpkt(controlpkts);
183183
toutput->set_ctrlpkt_id_map(ctrlpkt_id_map);
184184
toutput->set_annotations(parser->get_annotations());
185+
toutput->set_filename_table(
186+
std::make_shared<detail::filename_table>(parser->get_filename_table()));
185187

186188
for (auto col: collist)
187189
{
@@ -198,7 +200,7 @@ class aie2ps_preprocessor: public preprocessor
198200
std::vector<std::shared_ptr<asm_data>> data = coldata.get_label_asmdata(label);
199201
std::shared_ptr<assembler_state> state = create_assembler_state(m_isa, data, scratchpad, label_page_index, ctrlpkt_id_map, optimize, true);
200202
// create pages
201-
pager(PAGE_SIZE).pagify(*state, col, pages, relative_page_index);
203+
pager(PAGE_SIZE, parser->default_source_file_idx()).pagify(*state, col, pages, relative_page_index);
202204
label_page_index[get_pagelabel(label)] = relative_page_index;
203205
log_debug() << "num pages: " << pages.size() - relative_page_index << std::endl;
204206
relative_page_index = static_cast<uint32_t>(pages.size());

src/cpp/preprocessor/asm/asm_parser.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ parse_lines(const std::vector<char>& data, std::string& file)
354354
str += c;
355355
}
356356
}
357-
const uint32_t parse_file_idx = detail::intern_filename(file);
357+
const uint32_t parse_file_idx = intern_filename(file);
358358
// Scan str for newlines directly instead of copying it into an istringstream
359359
size_t pos = 0;
360360
const size_t str_len = str.size();
@@ -1397,7 +1397,7 @@ operate(std::shared_ptr<asm_parser> parserptr,
13971397
// dummy eof added if col change happens before eof
13981398
m_parserptr->insert_col_asmdata(std::make_shared<asm_data>(operation("eof", ""),
13991399
operation_type::op, code_section::unknown, 0,
1400-
(uint32_t)-1, 0, detail::default_source_file_idx()));
1400+
(uint32_t)-1, 0, m_parserptr->default_source_file_idx()));
14011401
m_parserptr->set_current_col(std::stoi(args_tail));
14021402
m_parserptr->set_data_state(false);
14031403
}
@@ -1511,7 +1511,7 @@ bool
15111511
include_directive::
15121512
read_include_file(std::string filename)
15131513
{
1514-
if (!m_parserptr->add_seen_file(filename))
1514+
if (m_parserptr->is_filename_seen(filename))
15151515
throw error(error::error_code::invalid_input,
15161516
"duplicate asm file name \"" + filename + "\"");
15171517
m_parserptr->set_data_state(false);

src/cpp/preprocessor/asm/asm_parser.h

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -226,36 +226,39 @@ class aie_row_topology_directive: public directive
226226
aie_row_topology_directive& operator=(aie_row_topology_directive&&) = default;
227227
};
228228

229-
// Filename intern table: maps each unique source-file name to a compact 32-bit index.
230-
// thread_local: It means each thread gets its own separate copy of the table,
231-
// which keeps concurrent parse jobs on different threads isolated.
232-
// The table is never cleared; indices remain valid as long as the thread is alive,
233-
// which always outlives any asm_data objects created on that thread.
234-
// inline: This global variable can be defined in multiple translation units without
235-
// violating the One Definition Rule (ODR).
229+
// Per-parser filename intern table: maps each unique source-file name to a compact
230+
// 32-bit index. One table per asm_parser so config.json instances do not share indices.
236231
namespace detail {
237-
inline thread_local std::vector<std::string> g_filename_table;
238-
inline thread_local std::unordered_map<std::string, uint32_t> g_filename_index;
232+
class filename_table {
233+
std::vector<std::string> m_table;
234+
std::unordered_map<std::string, uint32_t> m_index;
235+
uint32_t m_default_idx = static_cast<uint32_t>(-1);
239236

240-
inline uint32_t intern_filename(const std::string& fname) {
241-
auto it = g_filename_index.find(fname);
242-
if (it != g_filename_index.end())
237+
public:
238+
uint32_t intern_filename(const std::string& fname) {
239+
auto it = m_index.find(fname);
240+
if (it != m_index.end())
243241
return it->second;
244-
const auto idx = static_cast<uint32_t>(g_filename_table.size());
245-
g_filename_table.push_back(fname);
246-
g_filename_index.emplace(g_filename_table.back(), idx);
242+
const auto idx = static_cast<uint32_t>(m_table.size());
243+
m_table.push_back(fname);
244+
m_index.emplace(m_table.back(), idx);
247245
return idx;
248246
}
249247

250-
inline const std::string& lookup_filename(uint32_t idx) {
251-
return g_filename_table[idx];
248+
const std::string& lookup_filename(uint32_t idx) const {
249+
return m_table[idx];
252250
}
253251

254-
// Cached index for synthetic ops that are not tied to a real source path.
255-
inline uint32_t default_source_file_idx() {
256-
thread_local const uint32_t idx = intern_filename(std::string("default"));
257-
return idx;
252+
bool is_filename_seen(const std::string& fname) const {
253+
return m_index.find(fname) != m_index.end();
254+
}
255+
256+
uint32_t default_source_file_idx() {
257+
if (m_default_idx == static_cast<uint32_t>(-1))
258+
m_default_idx = intern_filename(std::string("default"));
259+
return m_default_idx;
258260
}
261+
};
259262
} // namespace detail
260263

261264
class asm_data
@@ -267,7 +270,7 @@ class asm_data
267270
pageid_type m_pagenum;
268271
uint32_t m_linenumber;
269272
// m_line removed: assembly text is reconstructed on demand via get_line().
270-
// m_file replaced with a 32-bit index into a thread_local intern table.
273+
// m_file replaced with a 32-bit index into the owning parser's filename_table.
271274
uint32_t m_file_idx;
272275
int m_annotation_index = -1;
273276

@@ -294,7 +297,9 @@ class asm_data
294297
HEADER_ACCESS_GET_SET(offset_type, size);
295298
HEADER_ACCESS_GET_SET(pageid_type, pagenum);
296299
HEADER_ACCESS_GET_SET(uint32_t, linenumber);
297-
const std::string& get_file() const { return detail::lookup_filename(m_file_idx); }
300+
const std::string& get_file(const detail::filename_table& table) const {
301+
return table.lookup_filename(m_file_idx);
302+
}
298303
uint32_t get_file_idx() const { return m_file_idx; }
299304
// Qualify the operation's own name as a label-map key (e.g. "0:start_job").
300305
std::string get_qualify_op_name() const {
@@ -512,7 +517,7 @@ class asm_parser: public std::enable_shared_from_this<asm_parser>
512517
std::map<int, std::vector<std::string>> m_preempt_hintmaps; // group -> vector of hintmap_labels (multiple PREEMPT opcodes per group)
513518
std::map<std::string, std::pair<std::string, std::string>> m_hintmap_labels; // hintmap_label -> (save_label, restore_label)
514519
std::set<int> m_preempt_without_hintmap; // groups that have PREEMPT opcodes without hintmaps
515-
std::set<std::string> m_seen_files; // tracks included file names to detect duplicates
520+
detail::filename_table m_filename_table;
516521

517522
// One unique scratchpad region: all hintmap labels that share the same scratchbase+size
518523
struct hintmap_group_entry {
@@ -768,8 +773,6 @@ class asm_parser: public std::enable_shared_from_this<asm_parser>
768773
std::string get_current_label() const { return m_current_label; }
769774
const file_artifact* get_artifacts() const { return m_artifacts;}
770775

771-
// Register filename as seen; returns true on first visit, false if already seen (duplicate).
772-
bool add_seen_file(const std::string& filename) { return m_seen_files.insert(filename).second; }
773776

774777
std::string top_label() const
775778
{
@@ -823,6 +826,20 @@ class asm_parser: public std::enable_shared_from_this<asm_parser>
823826

824827
void set_aie_row_topology_is_set(bool val) { m_aie_row_topology->set_is_set(val); }
825828

829+
const detail::filename_table& get_filename_table() const { return m_filename_table; }
830+
831+
uint32_t intern_filename(const std::string& fname) {
832+
return m_filename_table.intern_filename(fname);
833+
}
834+
835+
bool is_filename_seen(const std::string& fname) const {
836+
return m_filename_table.is_filename_seen(fname);
837+
}
838+
839+
uint32_t default_source_file_idx() {
840+
return m_filename_table.default_source_file_idx();
841+
}
842+
826843
void parse_lines();
827844

828845
void parse_lines(const std::vector<char>& data, std::string& file);

src/cpp/preprocessor/asm/pager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ assignpagenumber(assembler_state& state, uint32_t colnum,
318318
if (aligner)
319319
lpage.m_text.emplace_back(std::make_shared<asm_data>(operation(".align", "16"),
320320
operation_type::op, code_section::text, 0,
321-
(uint32_t)-1, 0, detail::default_source_file_idx()));
321+
(uint32_t)-1, 0, m_default_file_idx));
322322

323323
uint32_t cur_page_len = PAGE_HEADER_SIZE + tsize + EOF_SIZE + aligner + dsize;
324324
cur_page_len = (((cur_page_len + 3) >> 2) << 2); // round off to next multiple of 4

src/cpp/preprocessor/asm/pager.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace aiebu {
1414
class pager
1515
{
1616
uint32_t m_page_size;
17+
uint32_t m_default_file_idx;
1718
constexpr static offset_type PAGE_HEADER_SIZE = 16;
1819
constexpr static offset_type EOF_SIZE = 4;
1920
constexpr static offset_type DATA_SECTION_ALIGNMENT = 16;
@@ -62,7 +63,8 @@ class pager
6263
}
6364

6465
public:
65-
pager(uint32_t page_size): m_page_size(page_size) {}
66+
pager(uint32_t page_size, uint32_t default_file_idx)
67+
: m_page_size(page_size), m_default_file_idx(default_file_idx) {}
6668
uint32_t pagify(assembler_state& state, uint32_t col, std::vector<page>& pages, uint32_t relative_page_index);
6769
};
6870

0 commit comments

Comments
 (0)