On 11/7/25 11:15, Andrew MacLeod wrote:
On 11/7/25 00:46, Andrew Pinski wrote:
On Thu, Nov 6, 2025 at 6:22 PM Andrew MacLeod <[email protected]>
wrote:
I can add a check in the inferred range manager for atomic loads to
resolve this PR.
The IL sequence tends to look like:
_1 = &this_2(D)->b;
__atomic_load_8 (_1, 0);
Just want to make sure I get this right since memory operations are not
my strong suit.
The first argument to the atomic load is non-null (so _1), as well as
the base of the RHS of the address expression that defines _1 are
non-zero?. (this_2)
The attached patch scavenges a little code from
fold_using_range::range_of_address that I think works... but perhaps
there is a more efficient way to do this? Is this likely to work OK
and
be safe? or are there additional checks I need to be doing?
And I suppose there is an entire range of atomic operations this
applies
to? certainly atomic_store should qualify...
Bootstraps on x86_64-pc-linux-gnu with no regressions, but I'm not sure
that is a really extensive test for this..
Isn't the other route on fixing this is by adding the nonnull
attribute to these builtins? I tried looking to see if anyone had
mentioned why these builtins didn't have the nonnull attribute on them
but I didn't find anything.
The nonnull attribute support for builtins has been there since 2002
and atomic builtins were added afterwards (towards 2011).
DEF_ATTR_TREE_LIST (ATTR_NONNULL_1_NOTHROW_LEAF_LIST,
ATTR_NONNULL, \
ATTR_LIST_1, ATTR_NOTHROW_LEAF_LIST)
DEF_ATTR_TREE_LIST (ATTR_NONNULL_1_LEAF_LIST , ATTR_NONNULL, \
ATTR_LIST_1, ATTR_LEAF_LIST )
#define ATTR_NONNULL_1_NOTHROWCALL_LEAF_LIST
(flag_non_call_exceptions ? \
ATTR_NONNULL_1_LEAF_LIST : ATTR_NONNULL_1_NOTHROW_LEAF_LIST)
And then use ATTR_NONNULL_1_NOTHROWCALL_LEAF_LIST instead of
ATTR_NOTHROWCALL_LEAF_LIST in DEF_SYNC_BUILTIN for the builtins in
sync-builtins.def that are non null for the 1st argument.
I can only think that nobody has looked into the code generation
enough here to make a big difference. This would also handle isolate
paths too without any extra coding so I suspect special casing these
builtins everywhere is not a route which we want just for nonnullness.
Thanks,
Andrew
_1 = &this_2(D)->b;
__atomic_load_8 (_1, 0);
That would tag _1 as non_null, but that won't tag 'this_2' as
non-zero. I think the goal is to treat the use of _1 in the
__atomic_load as if it was dereferenced...
ie
struct d {
long a;
long b;
};
void call ();
void call2 ();
void fake (long *) __attribute__((nonnull));
void q (d *j)
{
long *p = &j->b;
fake (p);
if (j)
call ();
}
Replicates a similar thing. we know p is non-null, but never figure
out that j is. I guess we do not know that p is dereferenced in the
call to fake().
If however we add a derefernce:
void t (d *j)
{
long *p = &j->b;
fake (p);
if (*p)
call2 ()
if (j)
call ();
}
we *DO* know that j is non-null due to the
_1 = MEM[(long int *)j_4(D) + 8B];
That is introduced by the de-reference, and the 'if (j)' is removed.
So I think the goal is to treat the argument to the atomic calls as if
they are memory de-references, not just non-null? I don't suppose we
have anything like that already available :-P
Which means I probably need to do the same thing that the
non_null_loadstore () callback does at the very least.
Does any of this make sense?
Andrew
That got me to thinking... Should this maybe be an enhancement to
walk_stmt_load_store_ops() ? Where if the stmt is an atomic load (or
whatever operation) and the argument being operated on is an ssa_name,
that we do equivalent processing of the defining statement? It is
technically a load or store operation that isn't being processed. Do we
do anything for other builtins? like builtin_strcpy.? it also does
impliciit loads and stores like this
p_5 = &j_4(D)->b;
atomic_load_8 (p_5, 0);
_1 = MEM[(long int *)j_4(D) + 8B];
The _1 is the result of '*p'.. we want the equivalent effect for the
atomic load... In walk_load_store_ops () can we detect an atomic load,
expand the defining statement of p_5, so &j_4(D)-B into a MEM tree and
the walk that? or is that overthinking what needs to be done or wrong
path completely?
Andrew
