LRA for avr: Maintain live range info for pseudos assigned to FP?
Hi,
One more execution failure for the avr target, this time from
gcc.c-torture/execute/bitfld-3.c.
Steps to reproduce
Enable LRA in avr.cc by removing TARGET_LRA_P hook, build with
$ make all-host && make install-host
and then
$ avr-gcc gcc/testsuite/gcc.c-torture/execute/bitfld-3.c -S -Os -mmcu=avr51
-fdump-rtl-all
When lra_update_fp2sp_elimination runs and pseudos assigned to the
FP have to be spilled to stack slots, they sometimes end up in a
slot that already has a reg with an overlapping live range. This is
because lra_reg_info[regno].live_ranges is NULL for such spilled
pseudos, and therefore when assign_stack_slot_num_and_sort_pseduos
checks if lra_intersected_live_ranges_p, it always returns false.
In the below reload dump, all the pseudos assigned to FP get
allocated to slot 0. The live ranges for some of them (r1241 for
e.g.) conflicts with r603 that was originally assigned to slot 0,
but they still end up in the same slot, causing the execution failure.
r472: [86..87]
r473: [0..85]
...
r603: [254..655]
...
r1241: [282..283]
r1242: [268..269]
r1243: [254..255]
r1244: [238..239]
r1245: [222..223]
r1314: [88..89]
Ranges after the compression:
r591: [0..1]
r603: [0..1]
r604: [0..1]
r605: [0..1]
r606: [0..1]
r607: [0..1]
r623: [0..1]
r624: [0..1]
r635: [0..1]
r636: [0..1]
r637: [0..1]
r638: [0..1]
r639: [0..1]
r668: [0..1]
r669: [0..1]
r670: [0..1]
r671: [0..1]
r672: [0..1]
Frame pointer can not be eliminated anymore
Spilling non-eliminable hard regs: 28 29
Spilling r472(28)
Spilling r473(28)
Spilling r589(29)
Spilling r590(28)
Spilling r704(29)
Spilling r1241(28)
Spilling r1242(28)
Spilling r1243(28)
Spilling r1244(28)
Spilling r1245(28)
Spilling r1314(28)
Slot 0 regnos (width = 0): 603 13141245124412431242
1241704 590 589 473 472
Live ranges for those pseudos is NULL because when
lra_create_live_ranges_1 ran with all_p = false, they were not in
memory (they were assigned to FP). Computing live range info for
pseudos assigned to FP fixes the problem for avr,
Is that the right fix for this problem? After applying the below
patch, the reload dump looks like this
Ranges after the compression:
r472: [2..3]
r473: [0..1]
r589: [16..17]
r590: [16..17]
r591: [16..17]
r603: [10..17]
r604: [10..17]
r605: [10..17]
r606: [10..17]
r607: [10..17]
r623: [6..17]
r624: [6..17]
r635: [8..17]
r636: [8..17]
r637: [8..17]
r638: [8..17]
r639: [8..17]
r668: [6..17]
r669: [6..17]
r670: [6..17]
r671: [6..17]
r672: [6..17]
r704: [0..1]
r1241: [14..15]
r1242: [12..13]
r1243: [10..11]
r1244: [8..9]
r1245: [6..7]
r1314: [4..5]
Frame pointer can not be eliminated anymore
Spilling non-eliminable hard regs: 28 29
Spilling r472(28)
Spilling r473(28)
Spilling r589(29)
Spilling r590(28)
Spilling r704(29)
Spilling r1241(28)
Spilling r1242(28)
Spilling r1243(28)
Spilling r1244(28)
Spilling r1245(28)
Spilling r1314(28)
Slot 0 regnos (width = 0): 603 131412451244473 472
Slot 1 regnos (width = 0): 604 704
...
Slot 17 regnos (width = 0):591 124312421241
Slot 18 regnos (width = 0):589
Slot 19 regnos (width = 0):590
Regards
Senthil
diff --git a/gcc/lra-lives.cc b/gcc/lra-lives.cc
index f60e564da82..e4289f13979 100644
--- a/gcc/lra-lives.cc
+++ b/gcc/lra-lives.cc
@@ -250,7 +250,17 @@ update_pseudo_point (int regno, int point, enum point_type
type)
if (HARD_REGISTER_NUM_P (regno))
return;
- if (complete_info_p || lra_get_regno_hard_regno (regno) < 0)
+ /* Pseudos assigned to the FP register could potentially get spilled
+ to stack slots when lra_update_fp2sp_elimination runs, so keep
+ their live range info up to date, even if they aren't in memory
+ right now. */
+ int hard_regno = lra_get_regno_hard_regno (regno);
+ HARD_REG_SET set;
+ CLEAR_HARD_REG_SET(set);
+ add_to_hard_reg_set (&set, Pmode, HARD_FRAME_POINTER_REGNUM);
+
+ if (complete_info_p || hard_regno < 0
+ || overlaps_hard_reg_set_p (set, PSEUDO_REGNO_MODE (regno), hard_regno))
{
if (type == DEF_POINT)
{
Re: [PATCH] analyzer: implement symbolic value support for CPython plugin's refcnt checker [PR107646]
On Mon, 2023-09-04 at 22:13 -0400, Eric Feng wrote:
> Hi Dave,
Hi Eric, thanks for the patch.
>
> Recently I've been working on symbolic value support for the reference
> count checker. I've attached a patch for it below; let me know it looks
> OK for trunk. Thanks!
>
> Best,
> Eric
>
> ---
>
> This patch enhances the reference count checker in the CPython plugin by
> adding support for symbolic values. Whereas previously we were only able
> to check the reference count of PyObject* objects created in the scope
> of the function; we are now able to emit diagnostics on reference count
> mismatch of objects that were, for example, passed in as a function
> parameter.
>
> rc6.c:6:10: warning: expected ‘obj’ to have reference count: N + ‘1’ but
> ob_refcnt field is N + ‘2’
> 6 | return obj;
> | ^~~
[...snip...]
> create mode 100644 gcc/testsuite/gcc.dg/plugin/cpython-plugin-test-refcnt.c
>
> diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> index bf1982e79c3..d7ecd7fce09 100644
> --- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> @@ -314,17 +314,20 @@ public:
>{
> diagnostic_metadata m;
> bool warned;
> -// just assuming constants for now
> -auto actual_refcnt
> - = m_actual_refcnt->dyn_cast_constant_svalue ()->get_constant ();
> -auto ob_refcnt = m_ob_refcnt->dyn_cast_constant_svalue ()->get_constant
> ();
> -warned = warning_meta (rich_loc, m, get_controlling_option (),
> -"expected %qE to have "
> -"reference count: %qE but ob_refcnt field is: %qE",
> -m_reg_tree, actual_refcnt, ob_refcnt);
> -
> -// location_t loc = rich_loc->get_loc ();
> -// foo (loc);
> +
> +const auto *actual_refcnt_constant
> + = m_actual_refcnt->dyn_cast_constant_svalue ();
> +const auto *ob_refcnt_constant = m_ob_refcnt->dyn_cast_constant_svalue
> ();
> +if (!actual_refcnt_constant || !ob_refcnt_constant)
> + return false;
> +
> +auto actual_refcnt = actual_refcnt_constant->get_constant ();
> +auto ob_refcnt = ob_refcnt_constant->get_constant ();
> +warned = warning_meta (
> + rich_loc, m, get_controlling_option (),
> + "expected %qE to have "
> + "reference count: N + %qE but ob_refcnt field is N + %qE",
> + m_reg_tree, actual_refcnt, ob_refcnt);
> return warned;
I know you're emulating the old behavior I implemented way back in
cpychecker, but I don't like that behavior :(
Specifically, although the patch improves the behavior for symbolic
values, it regresses the precision of wording for the concrete values
case. If we have e.g. a concrete ob_refcnt of 2, whereas we only have
1 pointer, then it's more readable to say:
warning: expected ‘obj’ to have reference count: ‘1’ but ob_refcnt
field is ‘2’
than:
warning: expected ‘obj’ to have reference count: N + ‘1’ but ob_refcnt field
is N + ‘2’
...and we shouldn't quote concrete numbers, the message should be:
warning: expected ‘obj’ to have reference count of 1 but ob_refcnt field is 2
or better:
warning: ‘*obj’ is pointed to by 1 pointer but 'ob_refcnt' field is 2
Can you move the unwrapping of the svalue from the tests below into the
emit vfunc? That way the m_actual_refcnt doesn't have to be a
constant_svalue; you could have logic in the emit vfunc to print
readable messages based on what kind of svalue it is.
Rather than 'N', it might be better to say 'initial'; how about:
warning: ‘*obj’ is pointed to by 0 additional pointers but 'ob_refcnt' field
has increased by 1
warning: ‘*obj’ is pointed to by 1 additional pointer but 'ob_refcnt' field
has increased by 2
warning: ‘*obj’ is pointed to by 1 additional pointer but 'ob_refcnt' field
is unchanged
warning: ‘*obj’ is pointed to by 2 additional pointers but 'ob_refcnt' field
has decreased by 1
warning: ‘*obj’ is pointed to by 1 fewer pointers but 'ob_refcnt' field is
unchanged
and similar?
Maybe have a flag that tracks whether we're talking about a concrete
value that's absolute versus a concrete value that's relative to the
initial value?
[...snip...]
> @@ -369,6 +368,19 @@ increment_region_refcnt (hash_map
> &map, const region *key)
>refcnt = existed ? refcnt + 1 : 1;
> }
>
> +static const region *
> +get_region_from_svalue (const svalue *sval, region_model_manager *mgr)
> +{
> + const auto *region_sval = sval->dyn_cast_region_svalue ();
> + if (region_sval)
> +return region_sval->get_pointee ();
> +
> + const auto *initial_sval = sval->dyn_cast_initial_svalue ();
> + if (initial_sval)
> +return mgr->get_symbolic_region (initial_sval);
> +
> + return nullptr;
> +}
This is dereferencing a pointer, right?
Can the caller use region_model::deref_rvalue instead?
[...snip...]
> +static void
> +unwrap_any_ob_refcnt_sval
gcc-11-20230907 is now available
Snapshot gcc-11-20230907 is now available on https://gcc.gnu.org/pub/gcc/snapshots/11-20230907/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 11 git branch with the following options: git://gcc.gnu.org/git/gcc.git branch releases/gcc-11 revision 857072e79ac364a41d3994fc86a02976f4a6aec3 You'll find: gcc-11-20230907.tar.xz Complete GCC SHA256=5ba50d62adf162476a6e05cac0bcbae635dd5980421994ff07d1e5b9afd4be44 SHA1=8959f780237d3454e51942830c8c1caa0259989b Diffs from 11-20230831 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-11 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Incremental LTO Project
Hi, I noticed that adding incremental LTO was a GSoC project that was not claimed this cycle ( https://summerofcode.withgoogle.com/programs/2023/organizations/gnu-compiler-collection-gcc). I was curious about working on this project, but wanted to check on the state of the project. Has it already been completed? Is someone actively working on it? If not, what would be the appropriate method to contact the mentor (Jan Hubička)? Thanks!
Re: Last call for bikeshedding on attribute sym/exalias/reverse_alias
On Fri, 08 Sep 2023 02:25:38 -0300 Alexandre Oliva via Gcc wrote: > Attribute sym, named after symver, is the one in the latest version of > the patch. mnemonic_alias, convenience_alias and asm_alias are other > possibilities that comes to mind. The 2020-August thread has many more. Sounds like a useful feature. Since it specifically deals with C++ name mangling, how about 'demangle'? C++ is an abomination and name mangling is a perfect example of the brain damage surrounding this abortion of a language, but I digress...
