Re: [PATCH] c-family, c++, v2: Fix up handling of types which may have padding in __atomic_{compare_}exchange

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Feb 20, 2024 at 11:11:22AM +0100, Richard Biener wrote:
> > --- gcc/c-family/c-common.cc.jj	2024-02-17 16:40:42.831571693 +0100
> > +++ gcc/c-family/c-common.cc	2024-02-20 10:58:56.599865656 +0100
> > @@ -7793,9 +7793,14 @@ resolve_overloaded_atomic_exchange (loca
> >    /* Convert object pointer to required type.  */
> >    p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0);
> >    (*params)[0] = p0; 
> > -  /* Convert new value to required type, and dereference it.  */
> > -  p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR);
> > -  p1 = build1 (VIEW_CONVERT_EXPR, I_type, p1);
> > +  /* Convert new value to required type, and dereference it.
> > +     If *p1 type can have padding or may involve floating point which
> > +     could e.g. be promoted to wider precision and demoted afterwards,
> > +     state of padding bits might not be preserved.  */
> > +  build_indirect_ref (loc, p1, RO_UNARY_STAR);
> > +  p1 = build2_loc (loc, MEM_REF, I_type,
> > +		   build1 (VIEW_CONVERT_EXPR, I_type_ptr, p1),
> 
> Why the V_C_E to I_type_ptr?  The type of p1 doesn't
> really matter (unless it could be a non-pointer).

Just to help the FE when trying to constexpr evaluate it, because
for constexpr evaluation it evaluates MEM_REF the same as INDIRECT_REF
(and punts on non-0 second argument).  The actual call is non-constexpr,
just wanted to avoid ICEs or something similar, constexpr evaluation can
try to process the arguments (and succeed or fail, doesn't matter,
but not ICE) and then the call will not be constant expression and so
everything won't be.

> Also note that I_type needs to be properly address-space qualified
> in case the access should be to an address-space.  Formerly with
> the INDIRECT_REF that would likely be automagic.

I don't think using __atomic_*exchange on non-default as is valid,
if one doesn't have the exact __atomic_*exchange_N, it is handled
as a library call which is passed pointers and that definitely will
not deal with non-default address spaces.
Furthermore, the type should be the same in all arguments, and
the first argument is just converted to I_type_ptr and dealt with later, so
I don't think it ever worked even for the supported sizes.
int                                                                                                                                                                                  
foo (__seg_gs int *a, __seg_gs int *b, __seg_gs int *c)                                                                                                                                  
{                                                                                                                                                                                     
  return __atomic_compare_exchange (a, b, c, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);                                                                                                     
}                                                                                                                                                                                     
results in
	movl	(%rsi), %eax
	movl	%gs:(%rdx), %edx
	lock cmpxchgl	%edx, (%rdi)
	sete	%dl
	je	.L2
	movl	%eax, (%rsi)
.L2:
i.e. pretty much random what is loaded from a different AS and what is not.

	Jakub