Re: [PATCH] loongarch: use -mno-check-zero-division as the default for optimized code
On Sat, 2022-07-02 at 16:35 +0800, Lulu Cheng wrote: > 在 2022/7/2 下午4:24, Xi Ruoyao 写道: > > > > I'll commit the patch with the hook removed after another regtest on > > loongarch64-linux-gnu. I just rebuilt the entire system on my > > 3A5000, > > so I need some time to set it up. Expectation time to commit is > > today > > evening or tomorrow morning. > > > > BTW I've included this patch building my system, no bad things has > > happened so far. > Ok,Thanks!:-) Pushed as r13-1410. How do you think about the suggestion from Richard about a backport into gcc-12 branch? Normally we don't backport behavior changes, but making 12.1 the only exception seems compelling.
Re: [PATCH] analyzer: Use fixed-width types in allocation size tests
On Sun, 2022-07-03 at 01:20 +0200, Tim Lange wrote:
> Hi,
>
> thanks for the mail! Embarrassingly, I did not account for the
> different
> sizes types may have on different systems. I've switched all
> testcases
> to the fixed-width types of stdint.h.
>
> Does this patch need an approval?
Arguably falls under the "obvious" rule.
That said, the patch looks good to me, though given that the sizes are
now fixed the \\d+ regexps could be made more precise, if you like; I
think it's good either way.
Dave
>
> - Tim
>
> The patch changes the types inside the tests for the allocation size
> checker to fixed-width types of stdint.h to account for different
> architectures with different type widths.
>
> 2022-07-03 Tim Lange
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/analyzer/allocation-size-1.c: Use fixed-length
> types.
> * gcc.dg/analyzer/allocation-size-2.c: Likewise.
> * gcc.dg/analyzer/allocation-size-3.c: Likewise.
> * gcc.dg/analyzer/allocation-size-4.c: Likewise.
> * gcc.dg/analyzer/allocation-size-5.c: Likewise.
>
> ---
> .../gcc.dg/analyzer/allocation-size-1.c | 53 +++---
> .../gcc.dg/analyzer/allocation-size-2.c | 73 ++---
> --
> .../gcc.dg/analyzer/allocation-size-3.c | 19 ++---
> .../gcc.dg/analyzer/allocation-size-4.c | 13 ++--
> .../gcc.dg/analyzer/allocation-size-5.c | 23 +++---
> 5 files changed, 93 insertions(+), 88 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
> b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
> index 4fc2bf75d6c..e6d021a128f 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
> @@ -1,79 +1,80 @@
> #include
> #include
> +#include
>
> /* Tests with constant buffer sizes. */
>
> void test_1 (void)
> {
> - short *ptr = malloc (21 * sizeof (short));
> + int16_t *ptr = malloc (21 * sizeof (int16_t));
> free (ptr);
> }
>
> void test_2 (void)
> {
> - int *ptr = malloc (21 * sizeof (short)); /* { dg-line malloc2 } */
> + int32_t *ptr = malloc (21 * sizeof (int16_t)); /* { dg-line
> malloc2 } */
> free (ptr);
>
> /* { dg-warning "allocated buffer size is not a multiple of the
> pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 }
> */
> /* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc2 } */
> - /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'"
> "note" { target *-*-* } malloc2 } */
> + /* { dg-message "'int32_t \\*' (\\\{aka 'int \\*'\\\})? here;
> 'sizeof \\(int32_t (\\\{aka int\\\})\\)' is '\\d+'" "note" { target
> *-*-* } malloc2 } */
> }
>
> void test_3 (void)
> {
> - void *ptr = malloc (21 * sizeof (short));
> - short *sptr = (short *)ptr;
> + void *ptr = malloc (21 * sizeof (int16_t));
> + int16_t *sptr = (int16_t *)ptr;
> free (sptr);
> }
>
> void test_4 (void)
> {
> - void *ptr = malloc (21 * sizeof (short)); /* { dg-message "\\d+
> bytes" } */
> - int *iptr = (int *)ptr; /* { dg-line assign4 } */
> + void *ptr = malloc (21 * sizeof (int16_t)); /* { dg-message "\\d+
> bytes" } */
> + int32_t *iptr = (int32_t *)ptr; /* { dg-line assign4 } */
> free (iptr);
>
> /* { dg-warning "allocated buffer size is not a multiple of the
> pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign4 }
> */
> - /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'"
> "note" { target *-*-* } assign4 } */
> + /* { dg-message "'int32_t \\*' (\\\{aka 'int \\*'\\\})? here;
> 'sizeof \\(int32_t (\\\{aka int\\\})\\)' is '\\d+'" "note" { target
> *-*-* } assign4 } */
> }
>
> void test_5 (void)
> {
> - int user_input;
> + int32_t user_input;
> scanf("%i", _input);
> - int n;
> + int32_t n;
> if (user_input == 0)
> - n = 21 * sizeof (short);
> + n = 21 * sizeof (int16_t);
> else
> - n = 42 * sizeof (short);
> + n = 42 * sizeof (int16_t);
> void *ptr = malloc (n);
> - short *sptr = (short *)ptr;
> + int16_t *sptr = (int16_t *)ptr;
> free (sptr);
> }
>
> void test_6 (void)
> {
> - int user_input;
> + int32_t user_input;
> scanf("%i", _input);
> - int n;
> + int32_t n;
> if (user_input == 0)
> - n = 21 * sizeof (short);
> + n = 21 * sizeof (int16_t);
> else
> - n = 42 * sizeof (short);
> + n = 42 * sizeof (int16_t);
> void *ptr = malloc (n); /* { dg-message "" "note" } */
> /* ^^^ on widening_svalues no expr is
> returned
> by get_representative_tree at the
> moment. */
> - int *iptr = (int *)ptr; /* { dg-line assign6 } */
> + int32_t *iptr = (int32_t *)ptr; /* { dg-line assign6 } */
> free (iptr);
>
> /* { dg-warning "allocated buffer size is not a multiple of the
> pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign6 }
> */
> - /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'"
> "note" { target *-*-* }
[PATCH] analyzer: Use fixed-width types in allocation size tests
Hi,
thanks for the mail! Embarrassingly, I did not account for the different
sizes types may have on different systems. I've switched all testcases
to the fixed-width types of stdint.h.
Does this patch need an approval?
- Tim
The patch changes the types inside the tests for the allocation size
checker to fixed-width types of stdint.h to account for different
architectures with different type widths.
2022-07-03 Tim Lange
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/allocation-size-1.c: Use fixed-length types.
* gcc.dg/analyzer/allocation-size-2.c: Likewise.
* gcc.dg/analyzer/allocation-size-3.c: Likewise.
* gcc.dg/analyzer/allocation-size-4.c: Likewise.
* gcc.dg/analyzer/allocation-size-5.c: Likewise.
---
.../gcc.dg/analyzer/allocation-size-1.c | 53 +++---
.../gcc.dg/analyzer/allocation-size-2.c | 73 ++-
.../gcc.dg/analyzer/allocation-size-3.c | 19 ++---
.../gcc.dg/analyzer/allocation-size-4.c | 13 ++--
.../gcc.dg/analyzer/allocation-size-5.c | 23 +++---
5 files changed, 93 insertions(+), 88 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
index 4fc2bf75d6c..e6d021a128f 100644
--- a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
@@ -1,79 +1,80 @@
#include
#include
+#include
/* Tests with constant buffer sizes. */
void test_1 (void)
{
- short *ptr = malloc (21 * sizeof (short));
+ int16_t *ptr = malloc (21 * sizeof (int16_t));
free (ptr);
}
void test_2 (void)
{
- int *ptr = malloc (21 * sizeof (short)); /* { dg-line malloc2 } */
+ int32_t *ptr = malloc (21 * sizeof (int16_t)); /* { dg-line malloc2 } */
free (ptr);
/* { dg-warning "allocated buffer size is not a multiple of the pointee's
size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 } */
/* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc2 } */
- /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" {
target *-*-* } malloc2 } */
+ /* { dg-message "'int32_t \\*' (\\\{aka 'int \\*'\\\})? here; 'sizeof
\\(int32_t (\\\{aka int\\\})\\)' is '\\d+'" "note" { target *-*-* } malloc2 } */
}
void test_3 (void)
{
- void *ptr = malloc (21 * sizeof (short));
- short *sptr = (short *)ptr;
+ void *ptr = malloc (21 * sizeof (int16_t));
+ int16_t *sptr = (int16_t *)ptr;
free (sptr);
}
void test_4 (void)
{
- void *ptr = malloc (21 * sizeof (short)); /* { dg-message "\\d+ bytes" } */
- int *iptr = (int *)ptr; /* { dg-line assign4 } */
+ void *ptr = malloc (21 * sizeof (int16_t)); /* { dg-message "\\d+ bytes" } */
+ int32_t *iptr = (int32_t *)ptr; /* { dg-line assign4 } */
free (iptr);
/* { dg-warning "allocated buffer size is not a multiple of the pointee's
size \\\[CWE-131\\\]" "warning" { target *-*-* } assign4 } */
- /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" {
target *-*-* } assign4 } */
+ /* { dg-message "'int32_t \\*' (\\\{aka 'int \\*'\\\})? here; 'sizeof
\\(int32_t (\\\{aka int\\\})\\)' is '\\d+'" "note" { target *-*-* } assign4 } */
}
void test_5 (void)
{
- int user_input;
+ int32_t user_input;
scanf("%i", _input);
- int n;
+ int32_t n;
if (user_input == 0)
-n = 21 * sizeof (short);
+n = 21 * sizeof (int16_t);
else
-n = 42 * sizeof (short);
+n = 42 * sizeof (int16_t);
void *ptr = malloc (n);
- short *sptr = (short *)ptr;
+ int16_t *sptr = (int16_t *)ptr;
free (sptr);
}
void test_6 (void)
{
- int user_input;
+ int32_t user_input;
scanf("%i", _input);
- int n;
+ int32_t n;
if (user_input == 0)
-n = 21 * sizeof (short);
+n = 21 * sizeof (int16_t);
else
-n = 42 * sizeof (short);
+n = 42 * sizeof (int16_t);
void *ptr = malloc (n); /* { dg-message "" "note" } */
/* ^^^ on widening_svalues no expr is returned
by get_representative_tree at the moment. */
- int *iptr = (int *)ptr; /* { dg-line assign6 } */
+ int32_t *iptr = (int32_t *)ptr; /* { dg-line assign6 } */
free (iptr);
/* { dg-warning "allocated buffer size is not a multiple of the pointee's
size \\\[CWE-131\\\]" "warning" { target *-*-* } assign6 } */
- /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" {
target *-*-* } assign6 } */
+ /* { dg-message "'int32_t \\*' (\\\{aka 'int \\*'\\\})? here; 'sizeof
\\(int32_t (\\\{aka int\\\})\\)' is '\\d+'" "note" { target *-*-* } assign6 } */
}
void test_7 (void)
{
- int user_input;
+ int32_t user_input;
scanf("%i", _input);
- int n;
+ int32_t n;
if (user_input == 0)
n = 1;
else if (user_input == 2)
@@ -82,18 +83,18 @@ void test_7 (void)
n = 7;
/* n is an unknown_svalue at this point. */
void *ptr = malloc (n);
- int *iptr = (int *)ptr;
+ int32_t *iptr = (int32_t *)ptr;
free (iptr);
}
[r13-1405 Regression] FAIL: gcc.dg/analyzer/allocation-size-4.c warning at line 31 (test for warnings, line 28) on Linux/x86_64
On Linux/x86_64,
e6c3bb379f515b27268d08e62b4b3e5d7200b437 is the first bad commit
commit e6c3bb379f515b27268d08e62b4b3e5d7200b437
Author: Tim Lange
Date: Fri Jul 1 00:02:17 2022 +0200
analyzer: add allocation size checker [PR105900]
caused
FAIL: gcc.dg/analyzer/allocation-size-4.c note at line 32 (test for warnings,
line 28)
FAIL: gcc.dg/analyzer/allocation-size-4.c note at line 33 (test for warnings,
line 28)
FAIL: gcc.dg/analyzer/allocation-size-4.c warning at line 31 (test for
warnings, line 28)
with GCC configured with
../../gcc/configure
--prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r13-1405/usr
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl
--enable-libmpx x86_64-linux --disable-bootstrap
To reproduce:
$ cd {build_dir}/gcc && make check
RUNTESTFLAGS="analyzer.exp=gcc.dg/analyzer/allocation-size-4.c
--target_board='unix{-m32}'"
$ cd {build_dir}/gcc && make check
RUNTESTFLAGS="analyzer.exp=gcc.dg/analyzer/allocation-size-4.c
--target_board='unix{-m32\ -march=cascadelake}'"
(Please do not reply to this email, for question about this report, contact me
at skpgkp2 at gmail dot com)
[PATCH] MAINTAINERS: Add myself to write after approval and DCO
Hi everyone, I've added myself to write after approval and DCO section. - Tim 2022-07-02 Tim Lange ChangeLog: * MAINTAINERS: Add myself. --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3c448ba9eb6..17bebefa2db 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -495,6 +495,7 @@ Razya Ladelsky Thierry Lafage Rask Ingemann Lambertsen Jerome Lambourg +Tim Lange Asher Langton Chris Lattner Terry Laurenzo @@ -716,6 +717,7 @@ information. Matthias Kretz +Tim Lange Jeff Law Jeff Law Gaius Mulley -- 2.36.1
Re: [PATCH take 2] middle-end: Support ABIs that pass FP values as wider integers.
On 6/26/2022 6:55 AM, Roger Sayle wrote: Hi Jeff, Sorry for the long delay getting back to this, but after deeper investigation, it turns out that your tingling spider senses that the original patch wasn't updating everywhere that was required were spot on. Although my nvptx testing showed no problems with -O2, compiling the same tests with -O0 found several additional assertion ICEs (exactly where you'd predicted they should/would be). Here's a revised patch that updates five locations (up from the previous two). Finding any remaining locations (if any) might be easier once folks are able to test things on their targets. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check with no new failures, and on nvptx-none, where it is the middle-end portion of a pair of patches to allow the default ISA to be advanced. Ok for mainline? 2022-06-26 Roger Sayle gcc/ChangeLog PR target/104489 * calls.cc (precompute_register_parameters): Allow promotion of floating point values to be passed in wider integer modes. (expand_call): Allow floating point results to be returned in wider integer modes. * cfgexpand.cc (expand_value_return): Allow backends to promote a scalar floating point return value to a wider integer mode. * expr.cc (expand_expr_real_1) : Likewise, allow backends to promote scalar FP PARM_DECLs to wider integer modes. * function.cc (assign_parm_setup_stack): Allow floating point values to be passed on the stack as wider integer modes. OK. Though please consider factoring the code which reinterprets the original into a new mode. It looks to me like we've got 5 copies of it now :-) Jeff
Re: [RFC] fortran: restore array indexing for all descriptor arrays [PR102043]
Le 02/07/2022 à 13:18, Thomas Koenig a écrit : > > One thought: If we have to bite the bullet and break the ABI, why not go > all the way and use the C descriptors in ISO_Fortran_binding.h as > gfortran's native format? > As far as I know, CFI descriptors are mutually exclusive with non-negative array indexes. CFI descriptors' base_addr field points to the first element in descriptor indexing order, so they can be indexed negatively and we can’t safely use array indexing with them. With an additional offset field it would be possible though (with a different semantics for the base_addr field). Or we just keep the different semantics of the base_addr field and assume that there is an implicit offset with value sum(extent * max(-sm, 0)). Anyway, this is the long-delayed array descriptor reform, right? Any one remembers how far we are from it?
Re: [RFC] fortran: restore array indexing for all descriptor arrays [PR102043]
Hi Mikael, Feel free to comment, do we need this? Thanks for taking this on. One thought: If we have to bite the bullet and break the ABI, why not go all the way and use the C descriptors in ISO_Fortran_binding.h as gfortran's native format? Best regards Thomas
Re: [PATCH] loongarch: use -mno-check-zero-division as the default for optimized code
在 2022/7/2 下午4:24, Xi Ruoyao 写道:
On Sat, 2022-07-02 at 15:39 +0800, Lulu Cheng wrote:
diff --git a/gcc/common/config/loongarch/loongarch-common.cc
b/gcc/common/config/loongarch/loongarch-common.cc
index b6cbd84b873..f8b4660fabf 100644
--- a/gcc/common/config/loongarch/loongarch-common.cc
+++ b/gcc/common/config/loongarch/loongarch-common.cc
@@ -38,7 +38,4 @@ static const struct default_options
loongarch_option_optimization_table[] =
{ OPT_LEVELS_NONE, 0, NULL, 0 }
};
-#undef TARGET_DEFAULT_TARGET_FLAGS
-#define TARGET_DEFAULT_TARGET_FLAGS MASK_CHECK_ZERO_DIV
-
I think this modifications are needed, and there is no problem with the
rest.
Yes, this hook is unneeded now. Though the removal is not strictly
needed, it's good to remove irrelevant code (CWE-1164 says we shouldn't
keep any irrelevant code).
I'll commit the patch with the hook removed after another regtest on
loongarch64-linux-gnu. I just rebuilt the entire system on my 3A5000,
so I need some time to set it up. Expectation time to commit is today
evening or tomorrow morning.
BTW I've included this patch building my system, no bad things has
happened so far.
Ok,Thanks!:-)
Re: [PATCH] loongarch: use -mno-check-zero-division as the default for optimized code
On Sat, 2022-07-02 at 15:39 +0800, Lulu Cheng wrote:
> diff --git a/gcc/common/config/loongarch/loongarch-common.cc
> b/gcc/common/config/loongarch/loongarch-common.cc
> index b6cbd84b873..f8b4660fabf 100644
> --- a/gcc/common/config/loongarch/loongarch-common.cc
> +++ b/gcc/common/config/loongarch/loongarch-common.cc
> @@ -38,7 +38,4 @@ static const struct default_options
> loongarch_option_optimization_table[] =
> { OPT_LEVELS_NONE, 0, NULL, 0 }
> };
>
> -#undef TARGET_DEFAULT_TARGET_FLAGS
> -#define TARGET_DEFAULT_TARGET_FLAGS MASK_CHECK_ZERO_DIV
> -
>
> I think this modifications are needed, and there is no problem with the
> rest.
Yes, this hook is unneeded now. Though the removal is not strictly
needed, it's good to remove irrelevant code (CWE-1164 says we shouldn't
keep any irrelevant code).
I'll commit the patch with the hook removed after another regtest on
loongarch64-linux-gnu. I just rebuilt the entire system on my 3A5000,
so I need some time to set it up. Expectation time to commit is today
evening or tomorrow morning.
BTW I've included this patch building my system, no bad things has
happened so far.
Re: [PATCH] loongarch: use -mno-check-zero-division as the default for optimized code
在 2022/6/30 下午2:55, Richard Biener 写道:
On Thu, Jun 30, 2022 at 5:00 AM Xi Ruoyao via Gcc-patches
wrote:
Hi,
We've made a consensus [1] that not to enable trapping for division by
zero by default for LLVM, and we think GCC should behave similarly.
The main rationales:
1. Division by zero is undefined behavior, so in theory any portable
program shall not depend on it.
2. There are already many targets where both the hardware and GCC port
do nothing to trap on division by zero. A list taken from
gcc.c-torture/execute/20101011-1.c: PowerPC, RISC-V, ARM64, MSP430, and
many others. So in practice any portable program cannot depend on this
trap.
3. As an ICPC assistant coach, I'm well aware that the main disadvantage
not to trap on division by zero is "it breaks expectations of newbies".
So, we keep -mcheck-zero-division defaulted for -O0 and -Og. For other
optimization levels, it's well known that UBs are already breaking
newbies' expectations [2].
4. GCC is going to optimize more heavily exploiting integer division by
zero [3]. So let's stop encouraging people to rely on any integer
division by zero behavior from now.
[1]: https://reviews.llvm.org/D128572/new/#3612039
[2]: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
[3]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595099.html
Patch content following. Bootstrapped and regtested on loongarch64-
linux-gnu. Ok for trunk?
It might be worth backporting this behavioral change to the GCC 12 branch
as well (so 12.1 is the only release with different default behavior) and
documenting the change in changes.html
diff --git a/gcc/common/config/loongarch/loongarch-common.cc
b/gcc/common/config/loongarch/loongarch-common.cc
index b6cbd84b873..f8b4660fabf 100644
--- a/gcc/common/config/loongarch/loongarch-common.cc
+++ b/gcc/common/config/loongarch/loongarch-common.cc
@@ -38,7 +38,4 @@ static const struct default_options
loongarch_option_optimization_table[] =
{ OPT_LEVELS_NONE, 0, NULL, 0 }
};
-#undef TARGET_DEFAULT_TARGET_FLAGS
-#define TARGET_DEFAULT_TARGET_FLAGS MASK_CHECK_ZERO_DIV
-
I think this modifications are needed, and there is no problem with the
rest.
Thanks!
Lulu Cheng
