| Message ID | 40a675db-261e-2fbf-9760-673bf0e990d8@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Okt 19 2016, Aldy Hernandez <aldyh@redhat.com> wrote: > commit 32b629dcab7b78e8181146338c4b077f1d55a0bf > Author: Aldy Hernandez <aldyh@redhat.com> > Date: Wed Oct 19 11:24:44 2016 -0400 > > * gcc.dg/Walloca-1.c: Adjust test for !lp64 targets. > * gcc.dg/Walloca-2.c: Same. This fixes both tests on m68k. Andreas.
> This fixes both tests on m68k.
Likewise on Visium.
On 10/19/2016 09:32 AM, Aldy Hernandez wrote: > On 10/19/2016 09:16 AM, Eric Botcazou wrote: >>> m68k-suse-linux >> >> visium-elf too. >> > > The attached patch fixes the failures on m68k-suse-linux, visium-elf, > and arm-eabi. > > There were a few problems. > > One problem is that on lp64 targets (where sizeof(size_t) != > sizeof(int)), the warning is slightly different-- and rightly so. I > have updated the test to handle both warnings on the respective targets. > > The other problem is that the following snippet is incorrectly warning > on 32-bit targets: > > if (n > 0 && n < 2000) > p = __builtin_alloca (n); > > Looking at the gimple it seems like another case of VRP failing to give > any range information whatsoever. I have xfailed it as another case > where Andrew's upcoming work should theoretically fix this. The test is > fine on 64-bit targets. > > Can y'all double check it on your respective targets as I only have a > crude cross build? OK for the trunk whenever you're ready. jeff
On 19 October 2016 at 17:55, Jeff Law <law@redhat.com> wrote: > On 10/19/2016 09:32 AM, Aldy Hernandez wrote: >> >> On 10/19/2016 09:16 AM, Eric Botcazou wrote: >>>> >>>> m68k-suse-linux >>> >>> >>> visium-elf too. >>> >> >> The attached patch fixes the failures on m68k-suse-linux, visium-elf, >> and arm-eabi. >> >> There were a few problems. >> >> One problem is that on lp64 targets (where sizeof(size_t) != >> sizeof(int)), the warning is slightly different-- and rightly so. I >> have updated the test to handle both warnings on the respective targets. >> >> The other problem is that the following snippet is incorrectly warning >> on 32-bit targets: >> >> if (n > 0 && n < 2000) >> p = __builtin_alloca (n); >> >> Looking at the gimple it seems like another case of VRP failing to give >> any range information whatsoever. I have xfailed it as another case >> where Andrew's upcoming work should theoretically fix this. The test is >> fine on 64-bit targets. >> >> Can y'all double check it on your respective targets as I only have a >> crude cross build? > > OK for the trunk whenever you're ready. > > jeff You are too fast for me :-) I do not have the build trees for all the configurations ready for manual testing... So, I just merged the initial patch and the 2nd one, and started a validation job to make sure the 2nd patch fixes all the regressions observed earlier. It will take a few hours. Christophe
>> OK for the trunk whenever you're ready. >> >> jeff > > You are too fast for me :-) > I do not have the build trees for all the configurations ready for > manual testing... > So, I just merged the initial patch and the 2nd one, and started > a validation job to make sure the 2nd patch fixes all the regressions > observed earlier. > It will take a few hours. There shouldn't be any problems that didn't surface in my cross build, since they are not execution tests, but compile tests. I have committed the patch. If there any specific failures specific to your target, let me know and I'll address those separately. Thanks. Aldy
On 19 October 2016 at 20:43, Aldy Hernandez <aldyh@redhat.com> wrote: > >>> OK for the trunk whenever you're ready. >>> >>> jeff >> >> >> You are too fast for me :-) >> I do not have the build trees for all the configurations ready for >> manual testing... >> So, I just merged the initial patch and the 2nd one, and started >> a validation job to make sure the 2nd patch fixes all the regressions >> observed earlier. >> It will take a few hours. > > > There shouldn't be any problems that didn't surface in my cross build, since > they are not execution tests, but compile tests. > > I have committed the patch. If there any specific failures specific to your > target, let me know and I'll address those separately. > Validation results of the 2 patches combined are OK. Thanks > Thanks. > Aldy
diff --git a/gcc/testsuite/gcc.dg/Walloca-1.c b/gcc/testsuite/gcc.dg/Walloca-1.c index 34a20c3..32e4ab8 100644 --- a/gcc/testsuite/gcc.dg/Walloca-1.c +++ b/gcc/testsuite/gcc.dg/Walloca-1.c @@ -23,7 +23,8 @@ void foo1 (size_t len, size_t len2, size_t len3) char *s = alloca (123); useit (s); // OK, constant argument to alloca - s = alloca (num); // { dg-warning "large due to conversion" } + s = alloca (num); // { dg-warning "large due to conversion" "" { target lp64 } } + // { dg-warning "unbounded use of 'alloca'" "" { target { ! lp64 } } 26 } useit (s); s = alloca(90000); /* { dg-warning "is too large" } */ diff --git a/gcc/testsuite/gcc.dg/Walloca-2.c b/gcc/testsuite/gcc.dg/Walloca-2.c index 284b34e..4695fda 100644 --- a/gcc/testsuite/gcc.dg/Walloca-2.c +++ b/gcc/testsuite/gcc.dg/Walloca-2.c @@ -8,7 +8,11 @@ g1 (int n) { void *p; if (n > 0 && n < 2000) - p = __builtin_alloca (n); + // FIXME: This is a bogus warning, and is currently happening on + // 32-bit targets because VRP is not giving us any range info for + // the argument to __builtin_alloca. This should be fixed by the + // upcoming range work. + p = __builtin_alloca (n); // { dg-bogus "unbounded use of 'alloca'" "" { xfail { ! lp64 } } } else p = __builtin_malloc (n); f (p); @@ -31,8 +35,9 @@ g3 (int n) void *p; if (n > 0 && n < 3000) { - p = __builtin_alloca (n); // { dg-warning "'alloca' may be too large" } - // { dg-message "note:.*argument may be as large as 2999" "note" { target *-*-* } 34 } + p = __builtin_alloca (n); // { dg-warning "'alloca' may be too large" "" { target lp64} } + // { dg-message "note:.*argument may be as large as 2999" "note" { target lp64 } 38 } + // { dg-warning "unbounded use of 'alloca'" "" { target { ! lp64 } } 38 } } else p = __builtin_malloc (n);