* [PING][PATCH] Add instruction level discriminator support.
@ 2022-06-10 19:03 Eugene Rozenfeld
2022-06-27 19:45 ` Eugene Rozenfeld
0 siblings, 1 reply; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-06-10 19:03 UTC (permalink / raw)
To: gcc-patches, Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
Hello,
I'd like to ping this patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
Thanks,
Eugene
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
Sent: Thursday, June 02, 2022 12:22 AM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support in AutoFDO.
This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 31949 bytes --]
From 7d43f2b3b69420f1b57618dc8ccd225c266423a3 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_locus): New function to get the discriminator
from a location.
* input.h: Declarations of new functions and definition of
LOCATION_DISCRIMINATOR.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 +++++++--------
gcc/gimple-pretty-print.cc | 2 --
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 ++++++++++++++++++--
gcc/input.h | 7 ++++
gcc/lto-streamer-in.cc | 21 ++++++++++--
gcc/lto-streamer-out.cc | 7 ++++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 +++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 +++-
gcc/tree-cfg.cc | 39 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 ++++
gcc/tree.cc | 10 ++++--
libcpp/include/line-map.h | 10 ++++--
libcpp/lex.cc | 7 ++--
libcpp/line-map.cc | 38 +++++++++++++++++----
21 files changed, 185 insertions(+), 52 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index e435891fac6..7a0c5f77ef9 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1dc73724d1..5ed6b7b0f94 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15587,6 +15587,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15671,8 +15673,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index a9868861bd2..0d8c21b0681 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1697,8 +1689,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2243,7 +2234,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2982,7 +2972,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_locus (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3006,6 +2996,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3015,6 +3012,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3044,7 +3042,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index ebd87b20a0a..5e7039dbf66 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2899,8 +2899,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 2acbfdea4f8..ef4bacf0da6 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1045,7 +1045,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1055,7 +1056,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1729,6 +1730,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_locus (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..564ebd893e9 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_locus (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
@@ -174,6 +178,9 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
+ : 0)
#define RESERVED_LOCATION_P(LOC) \
(LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index fe5a4e7fe1d..271b8006ac0 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..d2b6c69315f 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = LOCATION_DISCRIMINATOR (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 488016bb42a..b0528235ef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 8de1b144a42..3b4063f18fd 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1162,7 +1162,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1177,6 +1203,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1190,12 +1217,12 @@ assign_discriminators (void)
|| (last && same_line_p (locus, &locus_e,
gimple_location (last))))
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (((first && has_discriminator (gimple_location (first)))
+ || (last && has_discriminator (gimple_location (last))))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 6acd394a079..ed9c368742e 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1416,6 +1416,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_locus (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1426,6 +1427,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 2bfb67489c6..a35a1e824d0 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14124,7 +14124,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14142,11 +14143,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_locus (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 80335721e03..f2d9e3cfbb7 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -755,6 +755,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1032,12 +1033,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1056,9 +1059,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index f891d3e17df..7bbd8c0f8e1 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1828,7 +1829,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4025,7 +4026,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PING][PATCH] Add instruction level discriminator support.
2022-06-10 19:03 [PING][PATCH] Add instruction level discriminator support Eugene Rozenfeld
@ 2022-06-27 19:45 ` Eugene Rozenfeld
2022-08-04 0:25 ` Eugene Rozenfeld
0 siblings, 1 reply; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-06-27 19:45 UTC (permalink / raw)
To: gcc-patches, Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]
Another ping for https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html .
I got a review from Andi (https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596549.html) but I also need a review from someone who can approve the changes.
Thanks,
Eugene
-----Original Message-----
From: Eugene Rozenfeld
Sent: Friday, June 10, 2022 12:03 PM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [PING][PATCH] Add instruction level discriminator support.
Hello,
I'd like to ping this patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
Thanks,
Eugene
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
Sent: Thursday, June 02, 2022 12:22 AM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support in AutoFDO.
This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 31949 bytes --]
From 7d43f2b3b69420f1b57618dc8ccd225c266423a3 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_locus): New function to get the discriminator
from a location.
* input.h: Declarations of new functions and definition of
LOCATION_DISCRIMINATOR.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 +++++++--------
gcc/gimple-pretty-print.cc | 2 --
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 ++++++++++++++++++--
gcc/input.h | 7 ++++
gcc/lto-streamer-in.cc | 21 ++++++++++--
gcc/lto-streamer-out.cc | 7 ++++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 +++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 +++-
gcc/tree-cfg.cc | 39 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 ++++
gcc/tree.cc | 10 ++++--
libcpp/include/line-map.h | 10 ++++--
libcpp/lex.cc | 7 ++--
libcpp/line-map.cc | 38 +++++++++++++++++----
21 files changed, 185 insertions(+), 52 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index e435891fac6..7a0c5f77ef9 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1dc73724d1..5ed6b7b0f94 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15587,6 +15587,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15671,8 +15673,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index a9868861bd2..0d8c21b0681 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1697,8 +1689,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2243,7 +2234,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2982,7 +2972,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_locus (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3006,6 +2996,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3015,6 +3012,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3044,7 +3042,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index ebd87b20a0a..5e7039dbf66 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2899,8 +2899,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 2acbfdea4f8..ef4bacf0da6 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1045,7 +1045,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1055,7 +1056,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1729,6 +1730,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_locus (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..564ebd893e9 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_locus (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
@@ -174,6 +178,9 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
+ : 0)
#define RESERVED_LOCATION_P(LOC) \
(LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index fe5a4e7fe1d..271b8006ac0 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..d2b6c69315f 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = LOCATION_DISCRIMINATOR (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 488016bb42a..b0528235ef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 8de1b144a42..3b4063f18fd 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1162,7 +1162,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1177,6 +1203,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1190,12 +1217,12 @@ assign_discriminators (void)
|| (last && same_line_p (locus, &locus_e,
gimple_location (last))))
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (((first && has_discriminator (gimple_location (first)))
+ || (last && has_discriminator (gimple_location (last))))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 6acd394a079..ed9c368742e 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1416,6 +1416,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_locus (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1426,6 +1427,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 2bfb67489c6..a35a1e824d0 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14124,7 +14124,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14142,11 +14143,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_locus (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 80335721e03..f2d9e3cfbb7 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -755,6 +755,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1032,12 +1033,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1056,9 +1059,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index f891d3e17df..7bbd8c0f8e1 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1828,7 +1829,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4025,7 +4026,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PING][PATCH] Add instruction level discriminator support.
2022-06-27 19:45 ` Eugene Rozenfeld
@ 2022-08-04 0:25 ` Eugene Rozenfeld
2022-08-19 1:55 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-08-04 0:25 UTC (permalink / raw)
To: gcc-patches; +Cc: Andi Kleen, Jan Hubicka, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
One more ping for this patch https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
CC Jason since this changes discriminators emitted in dwarf.
Thanks,
Eugene
-----Original Message-----
From: Eugene Rozenfeld
Sent: Monday, June 27, 2022 12:45 PM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: RE: [PING][PATCH] Add instruction level discriminator support.
Another ping for https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html .
I got a review from Andi (https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596549.html) but I also need a review from someone who can approve the changes.
Thanks,
Eugene
-----Original Message-----
From: Eugene Rozenfeld
Sent: Friday, June 10, 2022 12:03 PM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [PING][PATCH] Add instruction level discriminator support.
Hello,
I'd like to ping this patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
Thanks,
Eugene
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
Sent: Thursday, June 02, 2022 12:22 AM
To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support in AutoFDO.
This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 31949 bytes --]
From 7d43f2b3b69420f1b57618dc8ccd225c266423a3 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_locus): New function to get the discriminator
from a location.
* input.h: Declarations of new functions and definition of
LOCATION_DISCRIMINATOR.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 +++++++--------
gcc/gimple-pretty-print.cc | 2 --
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 ++++++++++++++++++--
gcc/input.h | 7 ++++
gcc/lto-streamer-in.cc | 21 ++++++++++--
gcc/lto-streamer-out.cc | 7 ++++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 +++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 +++-
gcc/tree-cfg.cc | 39 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 ++++
gcc/tree.cc | 10 ++++--
libcpp/include/line-map.h | 10 ++++--
libcpp/lex.cc | 7 ++--
libcpp/line-map.cc | 38 +++++++++++++++++----
21 files changed, 185 insertions(+), 52 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index e435891fac6..7a0c5f77ef9 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1dc73724d1..5ed6b7b0f94 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15587,6 +15587,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15671,8 +15673,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index a9868861bd2..0d8c21b0681 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1697,8 +1689,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2243,7 +2234,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2982,7 +2972,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_locus (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3006,6 +2996,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3015,6 +3012,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3044,7 +3042,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index ebd87b20a0a..5e7039dbf66 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2899,8 +2899,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 2acbfdea4f8..ef4bacf0da6 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1045,7 +1045,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1055,7 +1056,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1729,6 +1730,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_locus (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..564ebd893e9 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_locus (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
@@ -174,6 +178,9 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
+ : 0)
#define RESERVED_LOCATION_P(LOC) \
(LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index fe5a4e7fe1d..271b8006ac0 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..d2b6c69315f 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = LOCATION_DISCRIMINATOR (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 488016bb42a..b0528235ef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 8de1b144a42..3b4063f18fd 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1162,7 +1162,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1177,6 +1203,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1190,12 +1217,12 @@ assign_discriminators (void)
|| (last && same_line_p (locus, &locus_e,
gimple_location (last))))
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (((first && has_discriminator (gimple_location (first)))
+ || (last && has_discriminator (gimple_location (last))))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 6acd394a079..ed9c368742e 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1416,6 +1416,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_locus (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1426,6 +1427,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 2bfb67489c6..a35a1e824d0 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14124,7 +14124,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14142,11 +14143,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_locus (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 80335721e03..f2d9e3cfbb7 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -755,6 +755,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1032,12 +1033,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1056,9 +1059,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index f891d3e17df..7bbd8c0f8e1 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1828,7 +1829,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4025,7 +4026,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PING][PATCH] Add instruction level discriminator support.
2022-08-04 0:25 ` Eugene Rozenfeld
@ 2022-08-19 1:55 ` Jason Merrill
2022-09-01 20:48 ` [EXTERNAL] " Eugene Rozenfeld
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2022-08-19 1:55 UTC (permalink / raw)
To: Eugene Rozenfeld, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
On 8/3/22 17:25, Eugene Rozenfeld wrote:
> One more ping for this patch https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
>
> CC Jason since this changes discriminators emitted in dwarf.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Monday, June 27, 2022 12:45 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>
> Another ping for https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html .
>
> I got a review from Andi (https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596549.html) but I also need a review from someone who can approve the changes.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Friday, June 10, 2022 12:03 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: [PING][PATCH] Add instruction level discriminator support.
>
> Hello,
>
> I'd like to ping this patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596065.html
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, June 02, 2022 12:22 AM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>
> This is the first in a series of patches to enable discriminator support in AutoFDO.
>
> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>
> The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>
> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>
> Tested on x86_64-pc-linux-gnu.
> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
> || (last && same_line_p (locus, &locus_e,
> gimple_location (last))))
> {
> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
> - bb->discriminator
> - = next_discriminator_for_locus (locus_e.line);
> + if (((first && has_discriminator (gimple_location (first)))
> + || (last && has_discriminator (gimple_location (last))))
I think you want to check has_discriminator only for the one of first or
last that we find to have the same line as locus above?
Incidentally, I wonder why we ignore column number here, but that's not
an issue for this patch.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-08-19 1:55 ` Jason Merrill
@ 2022-09-01 20:48 ` Eugene Rozenfeld
2022-09-01 22:22 ` Eugene Rozenfeld
0 siblings, 1 reply; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-09-01 20:48 UTC (permalink / raw)
To: Jason Merrill, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 5708 bytes --]
Jason,
Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line.
I updated the patch (attached).
Thanks,
Eugene
-----Original Message-----
From: Jason Merrill <jason@redhat.com>
Sent: Thursday, August 18, 2022 6:55 PM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
On 8/3/22 17:25, Eugene Rozenfeld wrote:
> One more ping for this patch
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>
> CC Jason since this changes discriminators emitted in dwarf.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Monday, June 27, 2022 12:45 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>
> Another ping for https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8185dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dTDYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0 .
>
> I got a review from Andi (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8185dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=se6x1LD0GQyFz%2B28gdVqsye3Aw8kPoMRhVQO1BSPg6I%3D&reserved=0) but I also need a review from someone who can approve the changes.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Friday, June 10, 2022 12:03 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: [PING][PATCH] Add instruction level discriminator support.
>
> Hello,
>
> I'd like to ping this patch:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches
> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
> Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, June 02, 2022 12:22 AM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>
> This is the first in a series of patches to enable discriminator support in AutoFDO.
>
> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>
> The idea of this patch is based on commit
> 4c311d95cf6d9519c3c20f641cc77af7df491fdf
> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>
> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>
> Tested on x86_64-pc-linux-gnu.
> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
> || (last && same_line_p (locus, &locus_e,
> gimple_location (last))))
> {
> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
> - bb->discriminator
> - = next_discriminator_for_locus (locus_e.line);
> + if (((first && has_discriminator (gimple_location (first)))
> + || (last && has_discriminator (gimple_location (last))))
I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above?
Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch.
Jason
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 32307 bytes --]
From 4da1e259acba8ec0f8adbf8ad167f75a4a0ec5b4 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_locus): New function to get the discriminator
from a location.
* input.h: Declarations of new functions and definition of
LOCATION_DISCRIMINATOR.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 +++++------
gcc/gimple-pretty-print.cc | 2 -
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 ++++++++++++++-
gcc/input.h | 7 +++
gcc/lto-streamer-in.cc | 21 ++++++++-
gcc/lto-streamer-out.cc | 7 +++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 ++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 ++-
gcc/tree-cfg.cc | 52 +++++++++++++++++-----
gcc/tree-pretty-print.cc | 6 +++
gcc/tree.cc | 10 +++--
libcpp/include/line-map.h | 10 +++--
libcpp/lex.cc | 7 +--
libcpp/line-map.cc | 38 +++++++++++++---
21 files changed, 193 insertions(+), 57 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index c6ac9532cb8..29ded570734 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..7264bcec40b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15764,6 +15764,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15889,8 +15891,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index 0352786e49b..4023116d98e 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1696,8 +1688,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2242,7 +2233,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2981,7 +2971,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_locus (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3005,6 +2995,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3014,6 +3011,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3043,7 +3041,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index f18baec438a..a87e2ae68b3 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2875,8 +2875,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 060ca160126..16d35ac3def 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1082,7 +1082,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1092,7 +1093,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1766,6 +1767,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_locus (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..564ebd893e9 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_locus (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
@@ -174,6 +178,9 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
+ : 0)
#define RESERVED_LOCATION_P(LOC) \
(LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index a7dad70363f..bccd5d04541 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..d2b6c69315f 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = LOCATION_DISCRIMINATOR (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 645c009a340..7a8c4709257 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 91ec33c80a4..179de2fdbd7 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1165,7 +1165,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1180,6 +1206,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1188,18 +1215,21 @@ assign_discriminators (void)
{
gimple *first = first_non_label_stmt (e->dest);
gimple *last = last_stmt (e->dest);
- if ((first && same_line_p (locus, &locus_e,
+
+ gimple *stmt_on_same_line = NULL;
+ if (first && same_line_p (locus, &locus_e,
gimple_location (first)))
- || (last && same_line_p (locus, &locus_e,
- gimple_location (last))))
- {
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ stmt_on_same_line = first;
+ else if (last && same_line_p (locus, &locus_e,
+ gimple_location (last)))
+ stmt_on_same_line = last;
+
+ if (stmt_on_same_line
+ && has_discriminator (gimple_location (stmt_on_same_line))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
- }
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 47371d8bcbe..1747732364f 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1437,6 +1437,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_locus (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1447,6 +1448,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 007c9325b17..dc9f5819941 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14177,7 +14177,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14195,11 +14196,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_locus (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9bdd5b9d30c..50207cacc12 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -757,6 +757,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1034,12 +1035,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1058,9 +1061,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index 528d598aeb9..73f150b0ea3 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1895,7 +1896,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4099,7 +4100,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-09-01 20:48 ` [EXTERNAL] " Eugene Rozenfeld
@ 2022-09-01 22:22 ` Eugene Rozenfeld
2022-09-08 17:25 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-09-01 22:22 UTC (permalink / raw)
To: Eugene Rozenfeld, Jason Merrill, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 6215 bytes --]
Jason,
I made another small change in addressing your feedback (attached).
Thanks,
Eugene
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
Sent: Thursday, September 01, 2022 1:49 PM
To: Jason Merrill <jason@redhat.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
Jason,
Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line.
I updated the patch (attached).
Thanks,
Eugene
-----Original Message-----
From: Jason Merrill <jason@redhat.com>
Sent: Thursday, August 18, 2022 6:55 PM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
On 8/3/22 17:25, Eugene Rozenfeld wrote:
> One more ping for this patch
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>
> CC Jason since this changes discriminators emitted in dwarf.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Monday, June 27, 2022 12:45 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>
> Another ping for https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b0kTdzWRyiwdtcEFasyNlKv1vj%2FqwnipN3776C7xWcg%3D&reserved=0 .
>
> I got a review from Andi (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qxjBUCcGiKXtR4%2BOJq%2FFQN11C2M6BBurTguOBOjWJDw%3D&reserved=0) but I also need a review from someone who can approve the changes.
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Eugene Rozenfeld
> Sent: Friday, June 10, 2022 12:03 PM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: [PING][PATCH] Add instruction level discriminator support.
>
> Hello,
>
> I'd like to ping this patch:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches
> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
> Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, June 02, 2022 12:22 AM
> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
> Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>
> This is the first in a series of patches to enable discriminator support in AutoFDO.
>
> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>
> The idea of this patch is based on commit
> 4c311d95cf6d9519c3c20f641cc77af7df491fdf
> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>
> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>
> Tested on x86_64-pc-linux-gnu.
> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
> || (last && same_line_p (locus, &locus_e,
> gimple_location (last))))
> {
> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
> - bb->discriminator
> - = next_discriminator_for_locus (locus_e.line);
> + if (((first && has_discriminator (gimple_location (first)))
> + || (last && has_discriminator (gimple_location (last))))
I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above?
Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch.
Jason
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 32304 bytes --]
From 8d7cbcf9471ac76f300535f5b6262db38198a790 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_locus): New function to get the discriminator
from a location.
* input.h: Declarations of new functions and definition of
LOCATION_DISCRIMINATOR.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 ++++++-----
gcc/gimple-pretty-print.cc | 2 -
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 +++++++++++++++-
gcc/input.h | 7 +++
gcc/lto-streamer-in.cc | 21 ++++++++-
gcc/lto-streamer-out.cc | 7 +++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 ++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 ++-
gcc/tree-cfg.cc | 50 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 +++
gcc/tree.cc | 10 +++--
libcpp/include/line-map.h | 10 +++--
libcpp/lex.cc | 7 +--
libcpp/line-map.cc | 38 +++++++++++++---
21 files changed, 193 insertions(+), 55 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index c6ac9532cb8..29ded570734 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..7264bcec40b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15764,6 +15764,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15889,8 +15891,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index 0352786e49b..4023116d98e 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1696,8 +1688,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2242,7 +2233,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2981,7 +2971,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_locus (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3005,6 +2995,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3014,6 +3011,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3043,7 +3041,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index f18baec438a..a87e2ae68b3 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2875,8 +2875,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 060ca160126..16d35ac3def 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1082,7 +1082,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1092,7 +1093,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1766,6 +1767,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_locus (location_t locus)
+{
+ return LOCATION_DISCRIMINATOR(locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..564ebd893e9 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_locus (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
@@ -174,6 +178,9 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
+ : 0)
#define RESERVED_LOCATION_P(LOC) \
(LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index a7dad70363f..bccd5d04541 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..d2b6c69315f 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = LOCATION_DISCRIMINATOR (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 645c009a340..7a8c4709257 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 91ec33c80a4..6388879132d 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1165,7 +1165,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1180,6 +1206,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1188,17 +1215,22 @@ assign_discriminators (void)
{
gimple *first = first_non_label_stmt (e->dest);
gimple *last = last_stmt (e->dest);
- if ((first && same_line_p (locus, &locus_e,
+
+ gimple *stmt_on_same_line = NULL;
+ if (first && same_line_p (locus, &locus_e,
gimple_location (first)))
- || (last && same_line_p (locus, &locus_e,
- gimple_location (last))))
+ stmt_on_same_line = first;
+ else if (last && same_line_p (locus, &locus_e,
+ gimple_location (last)))
+ stmt_on_same_line = last;
+
+ if (stmt_on_same_line)
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (has_discriminator (gimple_location (stmt_on_same_line))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 47371d8bcbe..1747732364f 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1437,6 +1437,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_locus (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1447,6 +1448,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 007c9325b17..dc9f5819941 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14177,7 +14177,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14195,11 +14196,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_locus (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9bdd5b9d30c..50207cacc12 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -757,6 +757,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1034,12 +1035,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1058,9 +1061,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index 528d598aeb9..73f150b0ea3 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1895,7 +1896,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4099,7 +4100,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-09-01 22:22 ` Eugene Rozenfeld
@ 2022-09-08 17:25 ` Jason Merrill
2022-09-09 0:45 ` Eugene Rozenfeld
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2022-09-08 17:25 UTC (permalink / raw)
To: Eugene Rozenfeld, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
On 9/1/22 18:22, Eugene Rozenfeld wrote:
> Jason,
>
> I made another small change in addressing your feedback (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, September 01, 2022 1:49 PM
> To: Jason Merrill <jason@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> Jason,
>
> Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line.
> I updated the patch (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Jason Merrill <jason@redhat.com>
> Sent: Thursday, August 18, 2022 6:55 PM
> To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> On 8/3/22 17:25, Eugene Rozenfeld wrote:
>> One more ping for this patch
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> CC Jason since this changes discriminators emitted in dwarf.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Monday, June 27, 2022 12:45 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>>
>> Another ping for https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b0kTdzWRyiwdtcEFasyNlKv1vj%2FqwnipN3776C7xWcg%3D&reserved=0 .
>>
>> I got a review from Andi (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qxjBUCcGiKXtR4%2BOJq%2FFQN11C2M6BBurTguOBOjWJDw%3D&reserved=0) but I also need a review from someone who can approve the changes.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Friday, June 10, 2022 12:03 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [PING][PATCH] Add instruction level discriminator support.
>>
>> Hello,
>>
>> I'd like to ping this patch:
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Gcc-patches
>> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
>> Eugene Rozenfeld via Gcc-patches
>> Sent: Thursday, June 02, 2022 12:22 AM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>>
>> This is the first in a series of patches to enable discriminator support in AutoFDO.
>>
>> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>>
>> The idea of this patch is based on commit
>> 4c311d95cf6d9519c3c20f641cc77af7df491fdf
>> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>>
>> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>>
>> Tested on x86_64-pc-linux-gnu.
>
>> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
>> || (last && same_line_p (locus, &locus_e,
>> gimple_location (last))))
>> {
>> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
>> - bb->discriminator
>> - = next_discriminator_for_locus (locus_e.line);
>> + if (((first && has_discriminator (gimple_location (first)))
>> + || (last && has_discriminator (gimple_location (last))))
>
> I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above?
>
> Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch.
>
> Jason
This seems like excessive redundancy, especially with the slightly
different names with slightly different semantics:
> +++ b/gcc/input.h
> +#define LOCATION_DISCRIMINATOR(LOC) \
> + ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
> + : 0)
> +extern int get_discriminator_from_locus (location_t);
> +++ b/libcpp/include/line-map.h
> +extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
Maybe gcc/input.h could overload get_discriminator_from_loc to take a
single argument and forward to the libcpp version with 'line_table', and
then remove both the _locus version and the macro?
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-09-08 17:25 ` Jason Merrill
@ 2022-09-09 0:45 ` Eugene Rozenfeld
2022-09-19 22:13 ` Eugene Rozenfeld
2022-09-26 21:22 ` Jason Merrill
0 siblings, 2 replies; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-09-09 0:45 UTC (permalink / raw)
To: Jason Merrill, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 7580 bytes --]
Jason,
Thank for your suggestion. The patch is updated (attached).
Thanks,
Eugene
-----Original Message-----
From: Jason Merrill <jason@redhat.com>
Sent: Thursday, September 08, 2022 10:26 AM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
On 9/1/22 18:22, Eugene Rozenfeld wrote:
> Jason,
>
> I made another small change in addressing your feedback (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches
> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
> Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, September 01, 2022 1:49 PM
> To: Jason Merrill <jason@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> Jason,
>
> Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line.
> I updated the patch (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Jason Merrill <jason@redhat.com>
> Sent: Thursday, August 18, 2022 6:55 PM
> To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>;
> gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> On 8/3/22 17:25, Eugene Rozenfeld wrote:
>> One more ping for this patch
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> CC Jason since this changes discriminators emitted in dwarf.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Monday, June 27, 2022 12:45 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>>
>> Another ping for https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Kj3YWJrDqjX%2B0Ml3At5P8NRWc1Xs6mbI%2F1vCk9%2BLaQs%3D&reserved=0 .
>>
>> I got a review from Andi (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NBzFtD0mH7EYKxsVWfqZgSpQmUG18SIt01XRYUlwwW4%3D&reserved=0) but I also need a review from someone who can approve the changes.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Friday, June 10, 2022 12:03 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [PING][PATCH] Add instruction level discriminator support.
>>
>> Hello,
>>
>> I'd like to ping this patch:
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Gcc-patches
>> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
>> Eugene Rozenfeld via Gcc-patches
>> Sent: Thursday, June 02, 2022 12:22 AM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>>
>> This is the first in a series of patches to enable discriminator support in AutoFDO.
>>
>> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>>
>> The idea of this patch is based on commit
>> 4c311d95cf6d9519c3c20f641cc77af7df491fdf
>> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>>
>> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>>
>> Tested on x86_64-pc-linux-gnu.
>
>> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
>> || (last && same_line_p (locus, &locus_e,
>> gimple_location (last))))
>> {
>> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
>> - bb->discriminator
>> - = next_discriminator_for_locus (locus_e.line);
>> + if (((first && has_discriminator (gimple_location (first)))
>> + || (last && has_discriminator (gimple_location (last))))
>
> I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above?
>
> Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch.
>
> Jason
This seems like excessive redundancy, especially with the slightly different names with slightly different semantics:
> +++ b/gcc/input.h
> +#define LOCATION_DISCRIMINATOR(LOC) \
> + ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
> + : 0)
> +extern int get_discriminator_from_locus (location_t);
> +++ b/libcpp/include/line-map.h
> +extern unsigned get_discriminator_from_loc (line_maps *set,
> +location_t loc);
Maybe gcc/input.h could overload get_discriminator_from_loc to take a single argument and forward to the libcpp version with 'line_table', and then remove both the _locus version and the macro?
Jason
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 31876 bytes --]
From b69df1b0994f79a933f48032241b56a094f560ff Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
* input.h: Declarations of new functions.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 ++++++-----
gcc/gimple-pretty-print.cc | 2 -
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 +++++++++++++++-
gcc/input.h | 4 ++
gcc/lto-streamer-in.cc | 21 ++++++++-
gcc/lto-streamer-out.cc | 7 +++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 ++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 ++-
gcc/tree-cfg.cc | 50 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 +++
gcc/tree.cc | 10 +++--
libcpp/include/line-map.h | 10 +++--
libcpp/lex.cc | 7 +--
libcpp/line-map.cc | 38 +++++++++++++---
21 files changed, 190 insertions(+), 55 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index c6ac9532cb8..29ded570734 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..7264bcec40b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15764,6 +15764,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15889,8 +15891,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index 0352786e49b..554dece53ba 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1696,8 +1688,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2242,7 +2233,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2981,7 +2971,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_loc (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3005,6 +2995,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3014,6 +3011,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3043,7 +3041,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index f18baec438a..a87e2ae68b3 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2875,8 +2875,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 060ca160126..a28abfac5ac 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1082,7 +1082,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1092,7 +1093,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1766,6 +1767,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return get_discriminator_from_loc (locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_loc (location_t locus)
+{
+ return get_discriminator_from_loc (line_table, locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..11c571d076f 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_loc (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index a7dad70363f..bccd5d04541 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..dac8329769d 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = get_discriminator_from_loc (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 645c009a340..7a8c4709257 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 91ec33c80a4..6388879132d 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1165,7 +1165,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1180,6 +1206,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1188,17 +1215,22 @@ assign_discriminators (void)
{
gimple *first = first_non_label_stmt (e->dest);
gimple *last = last_stmt (e->dest);
- if ((first && same_line_p (locus, &locus_e,
+
+ gimple *stmt_on_same_line = NULL;
+ if (first && same_line_p (locus, &locus_e,
gimple_location (first)))
- || (last && same_line_p (locus, &locus_e,
- gimple_location (last))))
+ stmt_on_same_line = first;
+ else if (last && same_line_p (locus, &locus_e,
+ gimple_location (last)))
+ stmt_on_same_line = last;
+
+ if (stmt_on_same_line)
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (has_discriminator (gimple_location (stmt_on_same_line))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 47371d8bcbe..1caa453ec77 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1437,6 +1437,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_loc (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1447,6 +1448,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 007c9325b17..8ce2236cc33 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14177,7 +14177,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14195,11 +14196,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_loc (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9bdd5b9d30c..50207cacc12 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -757,6 +757,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1034,12 +1035,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1058,9 +1061,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index 528d598aeb9..73f150b0ea3 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1895,7 +1896,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4099,7 +4100,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-09-09 0:45 ` Eugene Rozenfeld
@ 2022-09-19 22:13 ` Eugene Rozenfeld
2022-09-26 21:22 ` Jason Merrill
1 sibling, 0 replies; 10+ messages in thread
From: Eugene Rozenfeld @ 2022-09-19 22:13 UTC (permalink / raw)
To: Jason Merrill, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 7974 bytes --]
Hi Jason,
Do you have any more feedback for this patch?
Thanks,
Eugene
-----Original Message-----
From: Eugene Rozenfeld
Sent: Thursday, September 08, 2022 5:46 PM
To: Jason Merrill <jason@redhat.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
Jason,
Thank for your suggestion. The patch is updated (attached).
Thanks,
Eugene
-----Original Message-----
From: Jason Merrill <jason@redhat.com>
Sent: Thursday, September 08, 2022 10:26 AM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
On 9/1/22 18:22, Eugene Rozenfeld wrote:
> Jason,
>
> I made another small change in addressing your feedback (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Gcc-patches
> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
> Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, September 01, 2022 1:49 PM
> To: Jason Merrill <jason@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> Jason,
>
> Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line.
> I updated the patch (attached).
>
> Thanks,
>
> Eugene
>
> -----Original Message-----
> From: Jason Merrill <jason@redhat.com>
> Sent: Thursday, August 18, 2022 6:55 PM
> To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>;
> gcc-patches@gcc.gnu.org
> Cc: Andi Kleen <ak@linux.intel.com>; Jan Hubicka <hubicka@ucw.cz>
> Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
>
> On 8/3/22 17:25, Eugene Rozenfeld wrote:
>> One more ping for this patch
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> CC Jason since this changes discriminators emitted in dwarf.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Monday, June 27, 2022 12:45 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>>
>> Another ping for https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Kj3YWJrDqjX%2B0Ml3At5P8NRWc1Xs6mbI%2F1vCk9%2BLaQs%3D&reserved=0 .
>>
>> I got a review from Andi (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NBzFtD0mH7EYKxsVWfqZgSpQmUG18SIt01XRYUlwwW4%3D&reserved=0) but I also need a review from someone who can approve the changes.
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Eugene Rozenfeld
>> Sent: Friday, June 10, 2022 12:03 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [PING][PATCH] Add instruction level discriminator support.
>>
>> Hello,
>>
>> I'd like to ping this patch:
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0
>>
>> Thanks,
>>
>> Eugene
>>
>> -----Original Message-----
>> From: Gcc-patches
>> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
>> Eugene Rozenfeld via Gcc-patches
>> Sent: Thursday, June 02, 2022 12:22 AM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen <ak@linux.intel.com>; Jan
>> Hubicka <hubicka@ucw.cz>
>> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>>
>> This is the first in a series of patches to enable discriminator support in AutoFDO.
>>
>> This patch switches to tracking discriminators per statement/instruction instead of per basic block. Tracking per basic block was problematic since not all statements in a basic block needed a discriminator and, also, later optimizations could move statements between basic blocks making correlation during AutoFDO compilation unreliable. Tracking per statement also allows us to assign different discriminators to multiple function calls in the same basic block. A subsequent patch will add that support.
>>
>> The idea of this patch is based on commit
>> 4c311d95cf6d9519c3c20f641cc77af7df491fdf
>> by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different approach. In Dehao's work special (normally unused) location ids and side tables were used to keep track of locations with discriminators. Things have changed since then and I don't think we have unused location ids anymore. Instead, I made discriminators a part of ad-hoc locations.
>>
>> The difference from Dehao's work also includes support for discriminator reading/writing in lto streaming and in modules.
>>
>> Tested on x86_64-pc-linux-gnu.
>
>> @@ -1190,12 +1217,12 @@ assign_discriminators (void)
>> || (last && same_line_p (locus, &locus_e,
>> gimple_location (last))))
>> {
>> - if (e->dest->discriminator != 0 && bb->discriminator == 0)
>> - bb->discriminator
>> - = next_discriminator_for_locus (locus_e.line);
>> + if (((first && has_discriminator (gimple_location (first)))
>> + || (last && has_discriminator (gimple_location (last))))
>
> I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above?
>
> Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch.
>
> Jason
This seems like excessive redundancy, especially with the slightly different names with slightly different semantics:
> +++ b/gcc/input.h
> +#define LOCATION_DISCRIMINATOR(LOC) \
> + ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC)) \
> + : 0)
> +extern int get_discriminator_from_locus (location_t);
> +++ b/libcpp/include/line-map.h
> +extern unsigned get_discriminator_from_loc (line_maps *set,
> +location_t loc);
Maybe gcc/input.h could overload get_discriminator_from_loc to take a single argument and forward to the libcpp version with 'line_table', and then remove both the _locus version and the macro?
Jason
[-- Attachment #2: 0001-Add-instruction-level-discriminator-support.patch --]
[-- Type: application/octet-stream, Size: 31876 bytes --]
From b69df1b0994f79a933f48032241b56a094f560ff Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Thu, 21 Apr 2022 15:42:15 -0700
Subject: [PATCH] Add instruction level discriminator support.
This is the first in a series of patches to enable discriminator support
in AutoFDO.
This patch switches to tracking discriminators per statement/instruction
instead of per basic block. Tracking per basic block was problematic since
not all statements in a basic block needed a discriminator and, also, later
optimizations could move statements between basic blocks making correlation
during AutoFDO compilation unreliable. Tracking per statement also allows
us to assign different discriminators to multiple function calls in the same
basic block. A subsequent patch will add that support.
The idea of this patch is based on commit 4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different
approach. In Dehao's work special (normally unused) location ids and side tables
were used to keep track of locations with discriminators. Things have changed
since then and I don't think we have unused location ids anymore. Instead,
I made discriminators a part of ad-hoc locations.
The difference from Dehao's work also includes support for discriminator
reading/writing in lto streaming and in modules.
Tested on x86_64-pc-linux-gnu.
gcc/ChangeLog:
* basic-block.h: Remove discriminator from basic blocks.
* cfghooks.cc (split_block_1): Remove discriminator from basic blocks.
* cp/module.cc (write_location): Write discriminator.
(read_location): Read discriminator.
* final.cc (final_start_function_1): Switch from per-bb to per statement
discriminator.
(final_scan_insn_1): Don't keep track of basic block discriminators.
(compute_discriminator): Switch from basic block discriminators to
instruction discriminators.
(insn_discriminator): New function to return instruction discriminator.
(notice_source_line): Use insn_discriminator.
* gimple-pretty-print.cc (dump_gimple_bb_header): Remove dumping of
basic block discriminators.
* gimple-streamer-in.cc (input_bb): Remove reading of basic block
discriminators.
* gimple-streamer-out.cc (output_bb): Remove writing of basic block
discriminators.
* input.cc (make_location): Pass 0 discriminator to COMBINE_LOCATION_DATA.
(location_with_discriminator): New function to combine locus with
a discriminator.
(has_discriminator): New function to check if a location has a discriminator.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
* input.h: Declarations of new functions.
* lto-streamer-in.cc (cmp_loc): Use discriminators in location comparison.
(apply_location_cache): Keep track of current discriminator.
(input_location_and_block): Read discriminator from stream.
* lto-streamer-out.cc (clear_line_info): Set current discriminator to
UINT_MAX.
(lto_output_location_1): Write discriminator to stream.
* lto-streamer.h: Add discriminator to cached_location.
Add current_discr to lto_location_cache.
Add current_discr to output_block.
* print-rtl.cc (print_rtx_operand_code_i): Print discriminator.
* rtl.h: Add extern declaration of insn_discriminator.
* tree-cfg.cc (assign_discriminator): New function to assign a unique
discriminator value to all statements in a basic block that have the given
line number.
(assign_discriminators): Assign discriminators to statement locations.
* tree-pretty-print.cc (dump_location): Dump discriminators.
* tree.cc (set_block): Preserve discriminator when setting block.
(set_source_range): Preserve discriminator when setting source range.
libcpp/ChangeLog:
* include/line-map.h: Add discriminator to location_adhoc_data.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): Add external declaration.
(get_discriminator_from_loc): Add external declaration.
(COMBINE_LOCATION_DATA): Add discriminator parameter.
* lex.cc (get_location_for_byte_range_in_cur_line) Pass 0 discriminator
in a call to COMBINE_LOCATION_DATA.
(warn_about_normalization): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
(_cpp_lex_direct): Pass 0 discriminator in a call to
COMBINE_LOCATION_DATA.
* line-map.cc (location_adhoc_data_hash): Use discriminator compute
location_adhoc_data hash.
(location_adhoc_data_eq): Use discriminator when comparing
location_adhoc_data.
(can_be_stored_compactly_p): Check discriminator to determine
compact storage.
(get_combined_adhoc_loc): Add discriminator parameter.
(get_discriminator_from_adhoc_loc): New function to get the discriminator
from an ad-hoc location.
(get_discriminator_from_loc): New function to get the discriminator
from a location.
gcc/testsuite/ChangeLog:
* c-c++-common/ubsan/pr85213.c: Pass -gno-statement-frontiers.
---
gcc/basic-block.h | 5 ---
gcc/cfghooks.cc | 1 -
gcc/cp/module.cc | 5 ++-
gcc/final.cc | 26 ++++++-----
gcc/gimple-pretty-print.cc | 2 -
gcc/gimple-streamer-in.cc | 1 -
gcc/gimple-streamer-out.cc | 1 -
gcc/input.cc | 36 +++++++++++++++-
gcc/input.h | 4 ++
gcc/lto-streamer-in.cc | 21 ++++++++-
gcc/lto-streamer-out.cc | 7 +++
gcc/lto-streamer.h | 3 ++
gcc/print-rtl.cc | 4 ++
gcc/rtl.h | 1 +
gcc/testsuite/c-c++-common/ubsan/pr85213.c | 7 ++-
gcc/tree-cfg.cc | 50 ++++++++++++++++++----
gcc/tree-pretty-print.cc | 6 +++
gcc/tree.cc | 10 +++--
libcpp/include/line-map.h | 10 +++--
libcpp/lex.cc | 7 +--
libcpp/line-map.cc | 38 +++++++++++++---
21 files changed, 190 insertions(+), 55 deletions(-)
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index c9d1fc91bbb..1eae03d1aca 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -148,11 +148,6 @@ struct GTY((chain_next ("%h.next_bb"), chain_prev ("%h.prev_bb"))) basic_block_d
/* Expected number of executions: calculated in profile.cc. */
profile_count count;
-
- /* The discriminator for this block. The discriminator distinguishes
- among several basic blocks that share a common locus, allowing for
- more accurate sample-based profiling. */
- int discriminator;
};
/* This ensures that struct gimple_bb_info is smaller than
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index c6ac9532cb8..29ded570734 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -541,7 +541,6 @@ split_block_1 (basic_block bb, void *i)
return NULL;
new_bb->count = bb->count;
- new_bb->discriminator = bb->discriminator;
if (dom_info_available_p (CDI_DOMINATORS))
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..7264bcec40b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -15764,6 +15764,8 @@ module_state::write_location (bytes_out &sec, location_t loc)
range.m_start = UNKNOWN_LOCATION;
write_location (sec, range.m_start);
write_location (sec, range.m_finish);
+ unsigned discriminator = get_discriminator_from_adhoc_loc (line_table, loc);
+ sec.u (discriminator);
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
@@ -15889,8 +15891,9 @@ module_state::read_location (bytes_in &sec) const
if (range.m_start == UNKNOWN_LOCATION)
range.m_start = locus;
range.m_finish = read_location (sec);
+ unsigned discriminator = sec.u ();
if (locus != loc && range.m_start != loc && range.m_finish != loc)
- locus = get_combined_adhoc_loc (line_table, locus, range, NULL);
+ locus = get_combined_adhoc_loc (line_table, locus, range, NULL, discriminator);
}
break;
diff --git a/gcc/final.cc b/gcc/final.cc
index 0352786e49b..554dece53ba 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -126,18 +126,10 @@ static int last_columnnum;
/* Discriminator written to assembly. */
static int last_discriminator;
-/* Discriminator to be written to assembly for current instruction.
+/* Compute discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
-static int discriminator;
static inline int compute_discriminator (location_t loc);
-/* Discriminator identifying current basic block among others sharing
- the same locus. */
-static int bb_discriminator;
-
-/* Basic block discriminator for previous instruction. */
-static int last_bb_discriminator;
-
/* Highest line number in current block. */
static int high_block_linenum;
@@ -1696,8 +1688,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
last_filename = LOCATION_FILE (prologue_location);
last_linenum = LOCATION_LINE (prologue_location);
last_columnnum = LOCATION_COLUMN (prologue_location);
- last_discriminator = discriminator = 0;
- last_bb_discriminator = bb_discriminator = 0;
+ last_discriminator = 0;
force_source_line = false;
high_block_linenum = high_function_linenum = last_linenum;
@@ -2242,7 +2233,6 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (targetm.asm_out.unwind_emit)
targetm.asm_out.unwind_emit (asm_out_file, insn);
- bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
break;
case NOTE_INSN_EH_REGION_BEG:
@@ -2981,7 +2971,7 @@ compute_discriminator (location_t loc)
int discriminator;
if (!decl_to_instance_map)
- discriminator = bb_discriminator;
+ discriminator = get_discriminator_from_loc (loc);
else
{
tree block = LOCATION_BLOCK (loc);
@@ -3005,6 +2995,13 @@ compute_discriminator (location_t loc)
return discriminator;
}
+/* Return discriminator of the statement that produced this insn. */
+int
+insn_discriminator (const rtx_insn *insn)
+{
+ return compute_discriminator (INSN_LOCATION (insn));
+}
+
/* Return whether a source line note needs to be emitted before INSN.
Sets IS_STMT to TRUE if the line should be marked as a possible
breakpoint location. */
@@ -3014,6 +3011,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
{
const char *filename;
int linenum, columnnum;
+ int discriminator;
if (NOTE_MARKER_P (insn))
{
@@ -3043,7 +3041,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- discriminator = compute_discriminator (INSN_LOCATION (insn));
+ discriminator = insn_discriminator (insn);
}
else
{
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index f18baec438a..a87e2ae68b3 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -2875,8 +2875,6 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
indent, "", get_lineno (gsi_stmt (gsi)));
break;
}
- if (bb->discriminator)
- fprintf (outf, ", discriminator %i", bb->discriminator);
fputc ('\n', outf);
}
}
diff --git a/gcc/gimple-streamer-in.cc b/gcc/gimple-streamer-in.cc
index e7f3256302f..ea8891e8e92 100644
--- a/gcc/gimple-streamer-in.cc
+++ b/gcc/gimple-streamer-in.cc
@@ -267,7 +267,6 @@ input_bb (class lto_input_block *ib, enum LTO_tags tag,
bb->count
= bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
bb->flags = streamer_read_hwi (ib);
- bb->discriminator = streamer_read_hwi (ib);
/* LTO_bb1 has statements. LTO_bb0 does not. */
if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.cc b/gcc/gimple-streamer-out.cc
index 33365251295..45832547bf5 100644
--- a/gcc/gimple-streamer-out.cc
+++ b/gcc/gimple-streamer-out.cc
@@ -208,7 +208,6 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
streamer_write_uhwi (ob, bb->index);
bb->count.stream_out (ob);
streamer_write_hwi (ob, bb->flags);
- streamer_write_hwi (ob, bb->discriminator);
if (!gsi_end_p (bsi) || phi_nodes (bb))
{
diff --git a/gcc/input.cc b/gcc/input.cc
index 060ca160126..a28abfac5ac 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -1082,7 +1082,8 @@ make_location (location_t caret, location_t start, location_t finish)
location_t combined_loc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1092,7 +1093,7 @@ location_t
make_location (location_t caret, source_range src_range)
{
location_t pure_loc = get_pure_location (caret);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL, 0);
}
/* An expanded_location stores the column in byte units. This function
@@ -1766,6 +1767,37 @@ get_location_within_string (cpp_reader *pfile,
return NULL;
}
+/* Associate the DISCRIMINATOR with LOCUS, and return a new locus. */
+
+location_t
+location_with_discriminator (location_t locus, int discriminator)
+{
+ tree block = LOCATION_BLOCK (locus);
+ source_range src_range = get_range_from_loc (line_table, locus);
+ locus = get_pure_location (locus);
+
+ if (locus == UNKNOWN_LOCATION)
+ return locus;
+
+ return COMBINE_LOCATION_DATA (line_table, locus, src_range, block, discriminator);
+}
+
+/* Return TRUE if LOCUS represents a location with a discriminator. */
+
+bool
+has_discriminator (location_t locus)
+{
+ return get_discriminator_from_loc (locus) != 0;
+}
+
+/* Return the discriminator for LOCUS. */
+
+int
+get_discriminator_from_loc (location_t locus)
+{
+ return get_discriminator_from_loc (line_table, locus);
+}
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/input.h b/gcc/input.h
index f1ae3aec95c..11c571d076f 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -165,6 +165,10 @@ extern location_t expansion_point_location (location_t);
extern location_t input_location;
+extern location_t location_with_discriminator (location_t, int);
+extern bool has_discriminator (location_t);
+extern int get_discriminator_from_loc (location_t);
+
#define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
#define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
#define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index a7dad70363f..bccd5d04541 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -409,6 +409,8 @@ lto_location_cache::cmp_loc (const void *pa, const void *pb)
return a->line - b->line;
if (a->col != b->col)
return a->col - b->col;
+ if (a->discr != b->discr)
+ return a->discr - b->discr;
if ((a->block == NULL_TREE) != (b->block == NULL_TREE))
return a->block ? 1 : -1;
if (a->block)
@@ -460,6 +462,8 @@ lto_location_cache::apply_location_cache ()
current_loc = linemap_position_for_column (line_table, loc.col);
if (loc.block)
current_loc = set_block (current_loc, loc.block);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
else if (current_block != loc.block)
{
@@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
current_loc = set_block (current_loc, loc.block);
else
current_loc = LOCATION_LOCUS (current_loc);
+ if (loc.discr)
+ current_loc = location_with_discriminator (current_loc, loc.discr);
+ }
+ else if (current_discr != loc.discr)
+ {
+ current_loc = location_with_discriminator (current_loc, loc.discr);
}
*loc.loc = current_loc;
current_line = loc.line;
prev_file = current_file = loc.file;
current_col = loc.col;
current_block = loc.block;
+ current_discr = loc.discr;
}
loc_cache.truncate (0);
accepted_length = 0;
@@ -512,6 +523,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
static int stream_col;
static bool stream_sysp;
static tree stream_block;
+ static unsigned stream_discr;
static const char *stream_relative_path_prefix;
gcc_assert (current_cache == this);
@@ -538,6 +550,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
*loc = RESERVED_LOCATION_COUNT;
bool line_change = bp_unpack_value (bp, 1);
bool column_change = bp_unpack_value (bp, 1);
+ bool discr_change = bp_unpack_value (bp, 1);
if (file_change)
{
@@ -563,6 +576,9 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ if (discr_change)
+ stream_discr = bp_unpack_var_len_unsigned (bp);
+
tree block = NULL_TREE;
if (ib)
{
@@ -578,7 +594,8 @@ lto_location_cache::input_location_and_block (location_t *loc,
if (current_file == stream_file
&& current_line == stream_line
&& current_col == stream_col
- && current_sysp == stream_sysp)
+ && current_sysp == stream_sysp
+ && current_discr == stream_discr)
{
if (current_block == block)
*loc = current_loc;
@@ -590,7 +607,7 @@ lto_location_cache::input_location_and_block (location_t *loc,
}
struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ = {stream_file, loc, stream_line, stream_col, stream_sysp, block, stream_discr};
loc_cache.safe_push (entry);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 471f35c315f..dac8329769d 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -67,6 +67,7 @@ clear_line_info (struct output_block *ob)
so that the first location with block in a function etc.
always streams a change_block bit and the first block. */
ob->current_block = void_node;
+ ob->current_discr = UINT_MAX;
}
@@ -194,6 +195,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (loc >= RESERVED_LOCATION_COUNT)
{
expanded_location xloc = expand_location (loc);
+ unsigned discr = get_discriminator_from_loc (orig_loc);
if (ob->reset_locus)
{
@@ -216,6 +218,7 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
bp_pack_value (bp, ob->current_line != xloc.line, 1);
bp_pack_value (bp, ob->current_col != xloc.column, 1);
+ bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_file != xloc.file)
{
@@ -242,6 +245,10 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp,
if (ob->current_col != xloc.column)
bp_pack_var_len_unsigned (bp, xloc.column);
ob->current_col = xloc.column;
+
+ if (ob->current_discr != discr)
+ bp_pack_var_len_unsigned (bp, discr);
+ ob->current_discr = discr;
}
else
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 597e9e405ec..2e3abd97959 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -311,6 +311,7 @@ private:
int line, col;
bool sysp;
tree block;
+ unsigned discr;
};
/* The location cache. */
@@ -333,6 +334,7 @@ private:
bool current_sysp;
location_t current_loc;
tree current_block;
+ unsigned current_discr;
};
/* Structure used as buffer for reading an LTO file. */
@@ -723,6 +725,7 @@ struct output_block
bool reset_locus;
bool emit_pwd;
tree current_block;
+ unsigned current_discr;
/* Cache of nodes written in this section. */
struct streamer_tree_cache_d *writer_cache;
diff --git a/gcc/print-rtl.cc b/gcc/print-rtl.cc
index 60c845485bc..e115f987173 100644
--- a/gcc/print-rtl.cc
+++ b/gcc/print-rtl.cc
@@ -453,6 +453,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
expanded_location xloc = insn_location (in_insn);
fprintf (m_outfile, " \"%s\":%i:%i", xloc.file, xloc.line,
xloc.column);
+ int discriminator = insn_discriminator (in_insn);
+ if (discriminator)
+ fprintf (m_outfile, " discrim %d", discriminator);
+
}
#endif
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 645c009a340..7a8c4709257 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3369,6 +3369,7 @@ extern int insn_line (const rtx_insn *);
extern const char * insn_file (const rtx_insn *);
extern tree insn_scope (const rtx_insn *);
extern expanded_location insn_location (const rtx_insn *);
+extern int insn_discriminator (const rtx_insn *);
extern location_t prologue_location, epilogue_location;
/* In jump.cc */
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr85213.c b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
index 8a6be81d20f..e903e976f2c 100644
--- a/gcc/testsuite/c-c++-common/ubsan/pr85213.c
+++ b/gcc/testsuite/c-c++-common/ubsan/pr85213.c
@@ -1,6 +1,11 @@
/* PR sanitizer/85213 */
/* { dg-do compile } */
-/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+/* Pass -gno-statement-frontiers to work around
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100733 :
+ without it the IR coming from the front end may be different with and without
+ debug information turned on. That may cause e.g., different discriminator values
+ and -fcompare-debug failures. */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug -gno-statement-frontiers" } */
int
foo (int x)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 91ec33c80a4..6388879132d 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -1165,7 +1165,33 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2)
&& filename_cmp (from->file, to.file) == 0);
}
-/* Assign discriminators to each basic block. */
+/* Assign a unique discriminator value to all statements in block bb that
+ have the same line number as locus. */
+
+static void
+assign_discriminator (location_t locus, basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ int discriminator;
+
+ if (locus == UNKNOWN_LOCATION)
+ return;
+
+ expanded_location locus_e = expand_location (locus);
+
+ discriminator = next_discriminator_for_locus (locus_e.line);
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ location_t stmt_locus = gimple_location (stmt);
+ if (same_line_p (locus, &locus_e, stmt_locus))
+ gimple_set_location (stmt,
+ location_with_discriminator (stmt_locus, discriminator));
+ }
+}
+
+/* Assign discriminators to statement locations. */
static void
assign_discriminators (void)
@@ -1180,6 +1206,7 @@ assign_discriminators (void)
location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
if (locus == UNKNOWN_LOCATION)
+
continue;
expanded_location locus_e = expand_location (locus);
@@ -1188,17 +1215,22 @@ assign_discriminators (void)
{
gimple *first = first_non_label_stmt (e->dest);
gimple *last = last_stmt (e->dest);
- if ((first && same_line_p (locus, &locus_e,
+
+ gimple *stmt_on_same_line = NULL;
+ if (first && same_line_p (locus, &locus_e,
gimple_location (first)))
- || (last && same_line_p (locus, &locus_e,
- gimple_location (last))))
+ stmt_on_same_line = first;
+ else if (last && same_line_p (locus, &locus_e,
+ gimple_location (last)))
+ stmt_on_same_line = last;
+
+ if (stmt_on_same_line)
{
- if (e->dest->discriminator != 0 && bb->discriminator == 0)
- bb->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ if (has_discriminator (gimple_location (stmt_on_same_line))
+ && !has_discriminator (locus))
+ assign_discriminator (locus, bb);
else
- e->dest->discriminator
- = next_discriminator_for_locus (locus_e.line);
+ assign_discriminator (locus, e->dest);
}
}
}
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 47371d8bcbe..1caa453ec77 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -1437,6 +1437,7 @@ void
dump_location (pretty_printer *pp, location_t loc)
{
expanded_location xloc = expand_location (loc);
+ int discriminator = get_discriminator_from_loc (loc);
pp_left_bracket (pp);
if (xloc.file)
@@ -1447,6 +1448,11 @@ dump_location (pretty_printer *pp, location_t loc)
pp_decimal_int (pp, xloc.line);
pp_colon (pp);
pp_decimal_int (pp, xloc.column);
+ if (discriminator)
+ {
+ pp_string (pp, " discrim ");
+ pp_decimal_int (pp, discriminator);
+ }
pp_string (pp, "] ");
}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 007c9325b17..8ce2236cc33 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14177,7 +14177,8 @@ set_block (location_t loc, tree block)
{
location_t pure_loc = get_pure_location (loc);
source_range src_range = get_range_from_loc (line_table, loc);
- return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block);
+ unsigned discriminator = get_discriminator_from_loc (line_table, loc);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, block, discriminator);
}
location_t
@@ -14195,11 +14196,14 @@ set_source_range (tree expr, source_range src_range)
if (!EXPR_P (expr))
return UNKNOWN_LOCATION;
- location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
+ location_t expr_location = EXPR_LOCATION (expr);
+ location_t pure_loc = get_pure_location (expr_location);
+ unsigned discriminator = get_discriminator_from_loc (expr_location);
location_t adhoc = COMBINE_LOCATION_DATA (line_table,
pure_loc,
src_range,
- NULL);
+ NULL,
+ discriminator);
SET_EXPR_LOCATION (expr, adhoc);
return adhoc;
}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9bdd5b9d30c..50207cacc12 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -757,6 +757,7 @@ struct GTY(()) location_adhoc_data {
location_t locus;
source_range src_range;
void * GTY((skip)) data;
+ unsigned discriminator;
};
struct htab;
@@ -1034,12 +1035,14 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
}
extern location_t get_combined_adhoc_loc (line_maps *, location_t,
- source_range, void *);
+ source_range, void *, unsigned);
extern void *get_data_from_adhoc_loc (const line_maps *, location_t);
+extern unsigned get_discriminator_from_adhoc_loc (const line_maps *, location_t);
extern location_t get_location_from_adhoc_loc (const line_maps *,
location_t);
extern source_range get_range_from_loc (line_maps *set, location_t loc);
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
@@ -1058,9 +1061,10 @@ inline location_t
COMBINE_LOCATION_DATA (class line_maps *set,
location_t loc,
source_range src_range,
- void *block)
+ void *block,
+ unsigned discriminator)
{
- return get_combined_adhoc_loc (set, loc, src_range, block);
+ return get_combined_adhoc_loc (set, loc, src_range, block, discriminator);
}
extern void rebuild_location_adhoc_htab (class line_maps *);
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index 528d598aeb9..73f150b0ea3 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1359,7 +1359,8 @@ get_location_for_byte_range_in_cur_line (cpp_reader *pfile,
location_t combined_loc = COMBINE_LOCATION_DATA (pfile->line_table,
start_loc,
src_range,
- NULL);
+ NULL,
+ 0);
return combined_loc;
}
@@ -1895,7 +1896,7 @@ warn_about_normalization (cpp_reader *pfile,
CPP_BUF_COLUMN (pfile->buffer,
pfile->buffer->cur));
loc = COMBINE_LOCATION_DATA (pfile->line_table,
- loc, tok_range, NULL);
+ loc, tok_range, NULL, 0);
}
encoding_rich_location rich_loc (pfile, loc);
@@ -4099,7 +4100,7 @@ _cpp_lex_direct (cpp_reader *pfile)
result->src_loc = COMBINE_LOCATION_DATA (pfile->line_table,
result->src_loc,
- tok_range, NULL);
+ tok_range, NULL, 0);
}
return result;
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..e17d109d9f1 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -67,7 +67,8 @@ location_adhoc_data_hash (const void *l)
return ((hashval_t) lb->locus
+ (hashval_t) lb->src_range.m_start
+ (hashval_t) lb->src_range.m_finish
- + (size_t) lb->data);
+ + (size_t) lb->data
+ + lb->discriminator);
}
/* Compare function for location_adhoc_data hashtable. */
@@ -82,7 +83,8 @@ location_adhoc_data_eq (const void *l1, const void *l2)
return (lb1->locus == lb2->locus
&& lb1->src_range.m_start == lb2->src_range.m_start
&& lb1->src_range.m_finish == lb2->src_range.m_finish
- && lb1->data == lb2->data);
+ && lb1->data == lb2->data
+ && lb1->discriminator == lb2->discriminator);
}
/* Update the hashtable when location_adhoc_data is reallocated. */
@@ -116,13 +118,17 @@ static bool
can_be_stored_compactly_p (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
/* If there's an ad-hoc pointer, we can't store it directly in the
location_t, we need the lookaside. */
if (data)
return false;
+ if (discriminator != 0)
+ return false;
+
/* We only store ranges that begin at the locus and that are sufficiently
"sane". */
if (src_range.m_start != locus)
@@ -157,7 +163,8 @@ location_t
get_combined_adhoc_loc (line_maps *set,
location_t locus,
source_range src_range,
- void *data)
+ void *data,
+ unsigned discriminator)
{
struct location_adhoc_data lb;
struct location_adhoc_data **slot;
@@ -175,7 +182,7 @@ get_combined_adhoc_loc (line_maps *set,
|| pure_location_p (set, locus));
/* Consider short-range optimization. */
- if (can_be_stored_compactly_p (set, locus, src_range, data))
+ if (can_be_stored_compactly_p (set, locus, src_range, data, discriminator))
{
/* The low bits ought to be clear. */
linemap_assert (pure_location_p (set, locus));
@@ -195,15 +202,16 @@ get_combined_adhoc_loc (line_maps *set,
when locus == start == finish (and data is NULL). */
if (locus == src_range.m_start
&& locus == src_range.m_finish
- && !data)
+ && !data && discriminator == 0)
return locus;
- if (!data)
+ if (!data && discriminator == 0)
set->num_unoptimized_ranges++;
lb.locus = locus;
lb.src_range = src_range;
lb.data = data;
+ lb.discriminator = discriminator;
slot = (struct location_adhoc_data **)
htab_find_slot (set->location_adhoc_data_map.htab, &lb, INSERT);
if (*slot == NULL)
@@ -248,6 +256,13 @@ get_data_from_adhoc_loc (const class line_maps *set, location_t loc)
return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].data;
}
+unsigned
+get_discriminator_from_adhoc_loc (const class line_maps *set, location_t loc)
+{
+ linemap_assert (IS_ADHOC_LOC (loc));
+ return set->location_adhoc_data_map.data[loc & MAX_LOCATION_T].discriminator;
+}
+
/* Return the location for the adhoc loc. */
location_t
@@ -293,6 +308,15 @@ get_range_from_loc (line_maps *set,
return source_range::from_location (loc);
}
+unsigned
+get_discriminator_from_loc (line_maps *set,
+ location_t loc)
+{
+ if (IS_ADHOC_LOC (loc))
+ return get_discriminator_from_adhoc_loc (set, loc);
+ return 0;
+}
+
/* Get whether location LOC is a "pure" location, or
whether it is an ad-hoc location, or embeds range information. */
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.
2022-09-09 0:45 ` Eugene Rozenfeld
2022-09-19 22:13 ` Eugene Rozenfeld
@ 2022-09-26 21:22 ` Jason Merrill
1 sibling, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2022-09-26 21:22 UTC (permalink / raw)
To: Eugene Rozenfeld, gcc-patches; +Cc: Andi Kleen, Jan Hubicka
On 9/8/22 20:45, Eugene Rozenfeld wrote:
> Jason,
>
> Thank for your suggestion. The patch is updated (attached).
> @@ -467,12 +471,19 @@ lto_location_cache::apply_location_cache ()
> current_loc = set_block (current_loc, loc.block);
> else
> current_loc = LOCATION_LOCUS (current_loc);
> + if (loc.discr)
> + current_loc = location_with_discriminator (current_loc, loc.discr);
> + }
> + else if (current_discr != loc.discr)
> + {
> + current_loc = location_with_discriminator (current_loc, loc.discr);
> }
We usually don't use { } around a single line.
> @@ -1180,6 +1206,7 @@ assign_discriminators (void)
> location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
>
> if (locus == UNKNOWN_LOCATION)
> +
> continue;
Stray newline.
OK with those tweaks.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-26 21:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 19:03 [PING][PATCH] Add instruction level discriminator support Eugene Rozenfeld
2022-06-27 19:45 ` Eugene Rozenfeld
2022-08-04 0:25 ` Eugene Rozenfeld
2022-08-19 1:55 ` Jason Merrill
2022-09-01 20:48 ` [EXTERNAL] " Eugene Rozenfeld
2022-09-01 22:22 ` Eugene Rozenfeld
2022-09-08 17:25 ` Jason Merrill
2022-09-09 0:45 ` Eugene Rozenfeld
2022-09-19 22:13 ` Eugene Rozenfeld
2022-09-26 21:22 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).