Re: -Wfloat-equal and comparison to zero

2024-11-11 Thread Joseph Myers via Gcc
On Sat, 9 Nov 2024, Sad Clouds via Gcc wrote:

> Even though there is nothing unsafe here and comparison to floating
> point 0.0 value is well defined.

The point of the warning is that *if you are writing code that thinks of 
floating-point values as being approximations to real numbers* then such 
comparisons are suspect.  If you are writing code that thinks of 
floating-point values as exactly represented members of their type 
(subject to any excess range and precision), rather than as approximations 
(at least as regards any floating-point numbers for which you use equality 
comparisons), and follows algorithms appropriate for fully defined 
floating-point operations, then you should not use that warning option for 
such code.

I don't think this has anything to do with whether one operand of the 
comparison is a constant.  It's still the case when comparing with 0.0 
that it's OK if your algorithm is designed such that the other operand is 
exact, and questionable if it is an approximation.

-- 
Joseph S. Myers
josmy...@redhat.com



Re: VLA representation in GCC internals

2024-11-11 Thread Joseph Myers via Gcc
On Sat, 9 Nov 2024, Martin Uecker via Gcc wrote:

> BTW: My main practical issue with zero-sized arrays is that the
> UB sanitizers triggers for zero-sized variable arrays.

They are after all UB in standard C.  (Note for example that the UB 
sanitizers cover all cases of shifts that are undefined in the standard 
for the relevant language version, although we explicitly do not treat 
various signed shift cases as UB for optimization purposes - the only ones 
considered UB for optimization are where the shift count is negative or at 
least the width of the type, not the other cases involving negative first 
argument or shifting into or past the sign bit.  However, the floating 
cases -fsanitize=float-divide-by-zero and -fsanitize=float-cast-overflow, 
that are not UB in Annex F, are not included in -fsanitized=undefined.)

-- 
Joseph S. Myers
josmy...@redhat.com



FTP issue

2024-11-11 Thread 王皓冉 via Gcc
Dear GCC

I am running docker to build a GCC image for C++ compile. But the image
call the ftp://ftp.gnu.org/gnu/gcc, may I know the reason for this?


RE: [RFC] Enabling SVE with offloading to nvptx

2024-11-11 Thread Prathamesh Kulkarni via Gcc


> -Original Message-
> From: Jakub Jelinek 
> Sent: 04 November 2024 21:44
> To: Prathamesh Kulkarni 
> Cc: Richard Biener ; Richard Biener
> ; gcc@gcc.gnu.org; Thomas Schwinge
> 
> Subject: Re: [RFC] Enabling SVE with offloading to nvptx
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sat, Nov 02, 2024 at 03:53:34PM +, Prathamesh Kulkarni wrote:
> > The attached patch adds a new bitfield needs_max_vf_lowering to
> loop,
> > and sets that in expand_omp_simd for loops that need delayed
> lowering
> > of safelen and omp simd arrays.  The patch defines a new macro
> > OMP_COMMON_MAX_VF (arbitrarily set to 16), as a placeholder value
> for
> > max_vf (instead of INT_MAX), and is later replaced by appropriate
> > max_vf during omp_adjust_max_vf pass.  Does that look OK ?
> 
> No.
> The thing is, if user doesn't specify safelen, it defaults to infinity
> (which we represent as INT_MAX), if user specifies it, then that is
> the maximum for it (currently in OpenMP specification it is just an
> integral value, so can't be a poly int).
> And then the lowering uses the max_vf as another limit, what the hw
> can do at most and sizes the magic arrays with it.  So, one needs to
> use minimum of what user specified and what the hw can handle.
> So using 16 as some magic value is just wrong, safelen(16) can be
> specified in the source as well, or safelen(8), or safelen(32) or
> safelen(123).
> 
> Thus, the fact that the hw minimum hasn't been determined yet needs to
> be represented in some other flag, not in loop->safelen value, and
> before that is determined, loop->safelen should then represent what
> the user wrote (or was implied) and the later pass should use minimum
> from loop->safelen and the picked hw maximum.  Of course if the picked
> hw maximum is POLY_INT-ish, the big question is how to compare that
> against the user supplied integer value, either one can just handle
> the INT_MAX (aka
> infinity) special case, or say query the backend on what is the
> maximum value of the POLY_INT at runtime and only use the POLY_INT if
> it is always known to be smaller or equal to the user supplied
> safelen.
> 
> Another thing (already mentioned in the thread Andrew referenced) is
> that max_vf is used in two separate places.  One is just to size of
> the magic arrays and one of the operands of the minimum (the other is
> user specified safelen).  In this case, it is generally just fine to
> pick later value than strictly necessary (as long as it is never
> larger than user supplied safelen).
> The other case is simd modifier on schedule clause.  That value should
> better be the right one or slightly larger, but not too much.
> I think currently we just use the INTEGER_CST we pick as the maximum,
> if this sizing is deferred, maybe it needs to be another internal
> function that asks the value (though, it can refer to a loop vf in
> another function, which complicates stuff).
> 
> Regarding Richi's question, I'm afraid the OpenMP simd loop lowering
> can't be delayed until some later pass.
Hi Jakub,
Thanks for the suggestions! The attached patch makes the following changes:
(1) Delays setting of safelen for offloading by introducing a new bitfield 
needs_max_vf_lowering in loop, which is true with offloading enabled,
and safelen is then set to min(safelen, max_vf) for the target later in 
omp_device_lower pass.
Comparing user-specified safelen with poly_int max_vf may not be always 
possible at compile-time (say 32 and 16+16x),
and even if we determine runtime VL based on -mcpu flags, I guess relying on 
that won't be portable ?
The patch works around this by taking constant_lower_bound (max_vf), and 
comparing it with safelen instead, with the downside
that constant_lower_bound(max_vf) will not be the optimal max_vf for SVE target 
if it implements SIMD width > 128 bits.

(2) Since max_vf is used as length of omp simd array, it gets streamed out to 
device, and device compilation fails during streaming-in if max_vf
is poly_int (16+16x), and device's NUM_POLY_INT_COEFFS < 2 (which motivated my 
patch). The patch tries to address this by simply setting length to a
placeholder value (INT_MAX?) in lower_rec_simd_input_clauses if offloading is 
enabled, and will be later set to appropriate value in omp_device_lower pass.

(3) Andrew's patches seems to already fix the case for adjusting chunk_size for 
schedule clause with simd modifier by introducing a new internal
function .GOMP_MAX_VF, which is then replaced by target's max_vf. To keep it 
consistent with safelen, the patch here uses constant_lower_bound (max_vf) too.

Patch passes libgomp testing for AArch64/nvptx offloading (with and without 
GPU).
Does it look OK ?

Thanks,
Prathamesh
> 
> Jakub

Delay lowering safelen if offloading is enabled.

gcc/ChangeLog:

* cfgloop.h (loop): New bitfield needs_max_vf_lowering.
* omp-expand.cc (expand_omp_simd): Set loop->needs_max_vf_lowering to 
result
of i

Re: Handling C2Y zero-length operations on null pointers

2024-11-11 Thread Martin Uecker via Gcc
Am Montag, dem 07.10.2024 um 15:14 + schrieb Qing Zhao:
> 
> > On Oct 7, 2024, at 10:13, Jakub Jelinek via Gcc  wrote:
> > 
> > On Fri, Oct 04, 2024 at 12:42:24AM +0200, Florian Weimer wrote:
> > > * Joseph Myers:
> > > 
> > > > The real question is how to achieve optimal warnings in the absence of 
> > > > the 
> > > > attribute.  Should we have a variant of the nonnull attribute that 
> > > > warns 
> > > > for NULL arguments but without optimizing based on them?
> > > 
> > > I think attribute access already covers part of it:
> > > 
> > > #include 
> > > void read_array (void *, size_t) __attribute__ ((access (read_only, 1, 
> > > 2)));
> > > void
> > > f (void)
> > > {
> > >  read_array (NULL, 0); // No warning.
> > >  read_array (NULL, 1); // Warning.
> > > }
> > > 
> > > It does not work for functions like strndup that support both string
> > > arguments (of any length) and array arguments of a specified size.
> > > The read_only variant requires an initialized array of the specified
> > > length.
> > 
> > access attribute can't deal with various other things.
> > 
> > Consider the qsort case.  My understanding was that the paper is making
> > typedef int (*cmpfn) (const void *, const void *);
> > qsort (NULL, 0, 1, (cmpfn) NULL);
> > valid (but is
> > qsort (NULL, 1, 0, (cmpfn) NULL);
> > still invalid?).
> > How do you express that with access attribute, which can only have 1 size
> > argument?  The accessed memory for the read/write pointee of the first
> > argument has nmemb * size parameter bytes size.
> 
> For the other attribute “alloc_size”, we have two forms, 
> A. alloc_size (position)
> and
> B. alloc_size (position-1, position-2)
> 
> The 2nd form is used to represent nmemb * size. 
> 
> Is it possible that we extend the attribute “access” similarly? 
> 
> Then we can use the attribute “access” consistently for this purpose?

We also miss sanitizer support.

How about letting "access" only be about access range
and instead have separate attribute that can be used to
express more complicated preconditions?

void* foo(void *p, size_t mmemb, size_t size)
[[precondition((p == NULL) == (mmemb * size == 0)]];

(not saying this is the right condition for any function
in the standard library)

Martin

> 
> Qing
> 
> > And using access attribute for function pointers doesn't work, there is
> > no data to be read/written there, just code.
> > 
> > Guess some of the nonnull cases could be replaced by access attribute
> > if we clarify the documentation that if SIZE_INDEX is specified and that
> > argument is non-zero then the pointer has to be non-NULL, and teach
> > sanitizers etc. to sanitize those.
> > 
> > For the rest, perhaps we need some nonnull_if_nonzero argument
> > which requires that the parameter identified by the first attribute
> > argument must be pointer which is non-NULL if the parameter identified
> > by the second attribute argument is non-zero.
> > And get clarified the qsort/bsearch cases whether it is about just
> > nmemb == 0 or nmemb * size == 0.
> > 
> > Jakub
> > 
> 



Re: Handling C2Y zero-length operations on null pointers

2024-11-11 Thread Martin Uecker via Gcc
Am Dienstag, dem 12.11.2024 um 07:51 +0100 schrieb Martin Uecker:
> Am Montag, dem 07.10.2024 um 15:14 + schrieb Qing Zhao:
> > 
> > > On Oct 7, 2024, at 10:13, Jakub Jelinek via Gcc  wrote:
> > > 
> > > On Fri, Oct 04, 2024 at 12:42:24AM +0200, Florian Weimer wrote:
> > > > * Joseph Myers:
> > > > 
> > > > > The real question is how to achieve optimal warnings in the absence 
> > > > > of the 
> > > > > attribute.  Should we have a variant of the nonnull attribute that 
> > > > > warns 
> > > > > for NULL arguments but without optimizing based on them?
> > > > 
> > > > I think attribute access already covers part of it:
> > > > 
> > > > #include 
> > > > void read_array (void *, size_t) __attribute__ ((access (read_only, 1, 
> > > > 2)));
> > > > void
> > > > f (void)
> > > > {
> > > >  read_array (NULL, 0); // No warning.
> > > >  read_array (NULL, 1); // Warning.
> > > > }
> > > > 
> > > > It does not work for functions like strndup that support both string
> > > > arguments (of any length) and array arguments of a specified size.
> > > > The read_only variant requires an initialized array of the specified
> > > > length.
> > > 
> > > access attribute can't deal with various other things.
> > > 
> > > Consider the qsort case.  My understanding was that the paper is making
> > > typedef int (*cmpfn) (const void *, const void *);
> > > qsort (NULL, 0, 1, (cmpfn) NULL);
> > > valid (but is
> > > qsort (NULL, 1, 0, (cmpfn) NULL);
> > > still invalid?).
> > > How do you express that with access attribute, which can only have 1 size
> > > argument?  The accessed memory for the read/write pointee of the first
> > > argument has nmemb * size parameter bytes size.
> > 
> > For the other attribute “alloc_size”, we have two forms, 
> > A. alloc_size (position)
> > and
> > B. alloc_size (position-1, position-2)
> > 
> > The 2nd form is used to represent nmemb * size. 
> > 
> > Is it possible that we extend the attribute “access” similarly? 
> > 
> > Then we can use the attribute “access” consistently for this purpose?
> 
> We also miss sanitizer support.
> 
> How about letting "access" only be about access range
> and instead have separate attribute that can be used to
> express more complicated preconditions?
> 
> void* foo(void *p, size_t mmemb, size_t size)
>   [[precondition((p == NULL) == (mmemb * size == 0)]];
> 
> (not saying this is the right condition for any function
> in the standard library)

And the condition should avoid wraparound.

Martin

> 
> Martin
> 
> > 
> > Qing
> > 
> > > And using access attribute for function pointers doesn't work, there is
> > > no data to be read/written there, just code.
> > > 
> > > Guess some of the nonnull cases could be replaced by access attribute
> > > if we clarify the documentation that if SIZE_INDEX is specified and that
> > > argument is non-zero then the pointer has to be non-NULL, and teach
> > > sanitizers etc. to sanitize those.
> > > 
> > > For the rest, perhaps we need some nonnull_if_nonzero argument
> > > which requires that the parameter identified by the first attribute
> > > argument must be pointer which is non-NULL if the parameter identified
> > > by the second attribute argument is non-zero.
> > > And get clarified the qsort/bsearch cases whether it is about just
> > > nmemb == 0 or nmemb * size == 0.
> > > 
> > > Jakub
> > > 
> > 
>