| Message ID | 9143b5b4-e410-1c21-91e9-aa2a91b5ceeb@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 08/24/2017 09:03 AM, Martin Sebor wrote: > On 08/24/2017 08:03 AM, Richard Biener wrote: >> On Wed, Aug 23, 2017 at 9:42 PM, Martin Sebor <msebor@gmail.com> wrote: >>> Bug 81908 is about a -Wstringop-overflow warning for a Fortran >>> test triggered by a recent VRP improvement. A simple test case >>> that approximates the warning is: >>> >>> void f (char *d, const char *s, size_t n) >>> { >>> if (n > 0 && n <= SIZE_MAX / 2) >>> n = 0; >>> >>> memcpy (d, s, n); // n in ~[1, SIZE_MAX / 2] >>> } >>> >>> Since the only valid value of n is zero the call to memcpy can >>> be considered a no-op (a value of n > SIZE_MAX is in excess of >>> the maximum size of the largest object and would surely make >>> the call crash). >>> >>> The important difference between the test case and Bug 81908 >>> is that in the latter, the code is emitted by GCC itself from >>> what appears to be correct source (looks like it's the result >>> of the loop distribution pass). I believe the warning for >>> the test case above and for other human-written code like it >>> is helpful, but warning for code emitted by GCC, even if it's >>> dead or unreachable, is obviously not (at least not to users). >>> >>> The attached patch enhances the gimple_fold_builtin_memory_op >>> function to eliminate this patholohgical case by making use >>> of range information to fold into no-ops calls to memcpy whose >>> size argument is in a range where the only valid value is zero. >>> This gets rid of the warning and improves the emitted code. >>> >>> Tested on x86_64-linux. >> >> @@ -646,6 +677,12 @@ gimple_fold_builtin_memory_op >> (gimple_stmt_iterator *gsi, >> tree destvar, srcvar; >> location_t loc = gimple_location (stmt); >> >> + if (tree valid_len = only_valid_value (len)) >> + { >> + /* LEN is in range whose only valid value is zero. */ >> + len = valid_len; >> + } >> + >> /* If the LEN parameter is zero, return DEST. */ >> if (integer_zerop (len)) >> >> just enhance this check to >> >> if (integer_zerop (len) >> || size_must_be_zero_p (len)) >> >> ? 'only_valid_value' looks too generic for this. > > Sure. > > FWIW, the reason I did it this was because my original patch > returned error_mark_node for entirely invalid ranges and had > the caller replace the call with a trap. I decided not to > include that part in this fix to keep it contained. Seems reasonable. Though I would suggest going forward with trap replacement for clearly invalid ranges as a follow-up. Once you do trap replacement, the input operands all become dead as does all the code after the trap and the outgoing edges in the CFG. That often exposes a fair amount of cleanup. Furthermore on targets that have conditional traps, the controlling condition often turns into a conditional trap. In all, you get a lot of nice cascading effects when you turn something that is clearly bogus into a trap. gimple-ssa-isolate-erroneous-paths probably has the infrastructure you need, including a code you can crib to detect PHI arguments which would cause bogus behavior and allow you to isolate that specific path. > >> >> + if (!wi::fits_uhwi_p (min) || !wi::fits_uhwi_p (max)) >> + return NULL_TREE; >> + >> >> why? > > Only because I never remember what APIs are safe to use with > what input. > >> + if (wi::eq_p (min, wone) >> + && wi::geu_p (max + 1, ssize_max)) >> >> if (wi::eq_p (min, 1) >> && wi::gtu_p (max, wi::max_value (prec, SIGNED))) >> >> your ssize_max isn't signed size max, and max + 1 might overflow to zero. > > You're right that the addition to max would be better done > as subtraction from the result of (1 << N). Thank you. > > If (max + 1) overflowed then (max == TYPE_MAX) would have > to hold which I thought could never be true for an anti > range. (The patch includes tests for this case.) Was I > wrong? Couldn't we have an anti range like ~[TYPE_MAX,TYPE_MAX]? Or am I misunderstanding something. > > Attached is an updated version with the suggested changes > plus an additional test to verify the absence of warnings. The patch is OK. I'll note this is another use of anti-ranges. I'd really like to see Aldy's work on the new range API move forward and get rid of anti-ranges. THanks, jeff
On 08/24/2017 03:52 PM, Jeff Law wrote: > On 08/24/2017 09:03 AM, Martin Sebor wrote: >> On 08/24/2017 08:03 AM, Richard Biener wrote: >>> On Wed, Aug 23, 2017 at 9:42 PM, Martin Sebor <msebor@gmail.com> wrote: >>>> Bug 81908 is about a -Wstringop-overflow warning for a Fortran >>>> test triggered by a recent VRP improvement. A simple test case >>>> that approximates the warning is: >>>> >>>> void f (char *d, const char *s, size_t n) >>>> { >>>> if (n > 0 && n <= SIZE_MAX / 2) >>>> n = 0; >>>> >>>> memcpy (d, s, n); // n in ~[1, SIZE_MAX / 2] >>>> } >>>> >>>> Since the only valid value of n is zero the call to memcpy can >>>> be considered a no-op (a value of n > SIZE_MAX is in excess of >>>> the maximum size of the largest object and would surely make >>>> the call crash). >>>> >>>> The important difference between the test case and Bug 81908 >>>> is that in the latter, the code is emitted by GCC itself from >>>> what appears to be correct source (looks like it's the result >>>> of the loop distribution pass). I believe the warning for >>>> the test case above and for other human-written code like it >>>> is helpful, but warning for code emitted by GCC, even if it's >>>> dead or unreachable, is obviously not (at least not to users). >>>> >>>> The attached patch enhances the gimple_fold_builtin_memory_op >>>> function to eliminate this patholohgical case by making use >>>> of range information to fold into no-ops calls to memcpy whose >>>> size argument is in a range where the only valid value is zero. >>>> This gets rid of the warning and improves the emitted code. >>>> >>>> Tested on x86_64-linux. >>> >>> @@ -646,6 +677,12 @@ gimple_fold_builtin_memory_op >>> (gimple_stmt_iterator *gsi, >>> tree destvar, srcvar; >>> location_t loc = gimple_location (stmt); >>> >>> + if (tree valid_len = only_valid_value (len)) >>> + { >>> + /* LEN is in range whose only valid value is zero. */ >>> + len = valid_len; >>> + } >>> + >>> /* If the LEN parameter is zero, return DEST. */ >>> if (integer_zerop (len)) >>> >>> just enhance this check to >>> >>> if (integer_zerop (len) >>> || size_must_be_zero_p (len)) >>> >>> ? 'only_valid_value' looks too generic for this. >> >> Sure. >> >> FWIW, the reason I did it this was because my original patch >> returned error_mark_node for entirely invalid ranges and had >> the caller replace the call with a trap. I decided not to >> include that part in this fix to keep it contained. > Seems reasonable. Though I would suggest going forward with trap > replacement for clearly invalid ranges as a follow-up. Once you do trap > replacement, the input operands all become dead as does all the code > after the trap and the outgoing edges in the CFG. > > That often exposes a fair amount of cleanup. Furthermore on targets > that have conditional traps, the controlling condition often turns into > a conditional trap. In all, you get a lot of nice cascading effects > when you turn something that is clearly bogus into a trap. > > gimple-ssa-isolate-erroneous-paths probably has the infrastructure you > need, including a code you can crib to detect PHI arguments which would > cause bogus behavior and allow you to isolate that specific path. Thanks for the pointer. I'll look into it. >>> + if (!wi::fits_uhwi_p (min) || !wi::fits_uhwi_p (max)) >>> + return NULL_TREE; >>> + >>> >>> why? >> >> Only because I never remember what APIs are safe to use with >> what input. >> >>> + if (wi::eq_p (min, wone) >>> + && wi::geu_p (max + 1, ssize_max)) >>> >>> if (wi::eq_p (min, 1) >>> && wi::gtu_p (max, wi::max_value (prec, SIGNED))) >>> >>> your ssize_max isn't signed size max, and max + 1 might overflow to zero. >> >> You're right that the addition to max would be better done >> as subtraction from the result of (1 << N). Thank you. >> >> If (max + 1) overflowed then (max == TYPE_MAX) would have >> to hold which I thought could never be true for an anti >> range. (The patch includes tests for this case.) Was I >> wrong? > Couldn't we have an anti range like ~[TYPE_MAX,TYPE_MAX]? Or am I > misunderstanding something. I've never seen such an anti-range. In my tests, a range one of whose bounds is equal to one of the type's limits) ends up canonicalized into an ordinary range. If I'm reading the code right it's done in set_and_canonicalize_value_range. >> Attached is an updated version with the suggested changes >> plus an additional test to verify the absence of warnings. > The patch is OK. Thanks. Patch committed in r251347. > > I'll note this is another use of anti-ranges. I'd really like to see > Aldy's work on the new range API move forward and get rid of anti-ranges. You and me both :) Martin
On August 24, 2017 11:52:41 PM GMT+02:00, Jeff Law <law@redhat.com> wrote: >On 08/24/2017 09:03 AM, Martin Sebor wrote: >> On 08/24/2017 08:03 AM, Richard Biener wrote: >>> On Wed, Aug 23, 2017 at 9:42 PM, Martin Sebor <msebor@gmail.com> >wrote: >>>> Bug 81908 is about a -Wstringop-overflow warning for a Fortran >>>> test triggered by a recent VRP improvement. A simple test case >>>> that approximates the warning is: >>>> >>>> void f (char *d, const char *s, size_t n) >>>> { >>>> if (n > 0 && n <= SIZE_MAX / 2) >>>> n = 0; >>>> >>>> memcpy (d, s, n); // n in ~[1, SIZE_MAX / 2] >>>> } >>>> >>>> Since the only valid value of n is zero the call to memcpy can >>>> be considered a no-op (a value of n > SIZE_MAX is in excess of >>>> the maximum size of the largest object and would surely make >>>> the call crash). >>>> >>>> The important difference between the test case and Bug 81908 >>>> is that in the latter, the code is emitted by GCC itself from >>>> what appears to be correct source (looks like it's the result >>>> of the loop distribution pass). I believe the warning for >>>> the test case above and for other human-written code like it >>>> is helpful, but warning for code emitted by GCC, even if it's >>>> dead or unreachable, is obviously not (at least not to users). >>>> >>>> The attached patch enhances the gimple_fold_builtin_memory_op >>>> function to eliminate this patholohgical case by making use >>>> of range information to fold into no-ops calls to memcpy whose >>>> size argument is in a range where the only valid value is zero. >>>> This gets rid of the warning and improves the emitted code. >>>> >>>> Tested on x86_64-linux. >>> >>> @@ -646,6 +677,12 @@ gimple_fold_builtin_memory_op >>> (gimple_stmt_iterator *gsi, >>> tree destvar, srcvar; >>> location_t loc = gimple_location (stmt); >>> >>> + if (tree valid_len = only_valid_value (len)) >>> + { >>> + /* LEN is in range whose only valid value is zero. */ >>> + len = valid_len; >>> + } >>> + >>> /* If the LEN parameter is zero, return DEST. */ >>> if (integer_zerop (len)) >>> >>> just enhance this check to >>> >>> if (integer_zerop (len) >>> || size_must_be_zero_p (len)) >>> >>> ? 'only_valid_value' looks too generic for this. >> >> Sure. >> >> FWIW, the reason I did it this was because my original patch >> returned error_mark_node for entirely invalid ranges and had >> the caller replace the call with a trap. I decided not to >> include that part in this fix to keep it contained. >Seems reasonable. Though I would suggest going forward with trap >replacement for clearly invalid ranges as a follow-up. Once you do >trap >replacement, the input operands all become dead as does all the code >after the trap and the outgoing edges in the CFG. > >That often exposes a fair amount of cleanup. Furthermore on targets >that have conditional traps, the controlling condition often turns into >a conditional trap. In all, you get a lot of nice cascading effects >when you turn something that is clearly bogus into a trap. > >gimple-ssa-isolate-erroneous-paths probably has the infrastructure you >need, including a code you can crib to detect PHI arguments which would >cause bogus behavior and allow you to isolate that specific path. > > > >> >>> >>> + if (!wi::fits_uhwi_p (min) || !wi::fits_uhwi_p (max)) >>> + return NULL_TREE; >>> + >>> >>> why? >> >> Only because I never remember what APIs are safe to use with >> what input. >> >>> + if (wi::eq_p (min, wone) >>> + && wi::geu_p (max + 1, ssize_max)) >>> >>> if (wi::eq_p (min, 1) >>> && wi::gtu_p (max, wi::max_value (prec, SIGNED))) >>> >>> your ssize_max isn't signed size max, and max + 1 might overflow to >zero. >> >> You're right that the addition to max would be better done >> as subtraction from the result of (1 << N). Thank you. >> >> If (max + 1) overflowed then (max == TYPE_MAX) would have >> to hold which I thought could never be true for an anti >> range. (The patch includes tests for this case.) Was I >> wrong? >Couldn't we have an anti range like ~[TYPE_MAX,TYPE_MAX]? Or am I >misunderstanding something. > >> >> Attached is an updated version with the suggested changes >> plus an additional test to verify the absence of warnings. >The patch is OK. > >I'll note this is another use of anti-ranges. I'd really like to see >Aldy's work on the new range API move forward and get rid of >anti-ranges. Note that anti-ranges are not complicated to handle. In fact the current API could be trivially extended to return two ranges for them. The complication will be in the callers which will have to handle the unioning. I'd like to get rid of anti-ranges in VRP and mainly for the reason that handling unioning of N ranges will increase precision. I do not see too many uses of the increased precision outside of VRP which is where Aldhys work is solely residing. Richard. >THanks, >jeff
Martin Sebor <msebor@gmail.com> writes: > On 08/24/2017 08:03 AM, Richard Biener wrote: >> On Wed, Aug 23, 2017 at 9:42 PM, Martin Sebor <msebor@gmail.com> wrote: >>> Bug 81908 is about a -Wstringop-overflow warning for a Fortran >>> test triggered by a recent VRP improvement. A simple test case >>> that approximates the warning is: >>> >>> void f (char *d, const char *s, size_t n) >>> { >>> if (n > 0 && n <= SIZE_MAX / 2) >>> n = 0; >>> >>> memcpy (d, s, n); // n in ~[1, SIZE_MAX / 2] >>> } >>> >>> Since the only valid value of n is zero the call to memcpy can >>> be considered a no-op (a value of n > SIZE_MAX is in excess of >>> the maximum size of the largest object and would surely make >>> the call crash). >>> >>> The important difference between the test case and Bug 81908 >>> is that in the latter, the code is emitted by GCC itself from >>> what appears to be correct source (looks like it's the result >>> of the loop distribution pass). I believe the warning for >>> the test case above and for other human-written code like it >>> is helpful, but warning for code emitted by GCC, even if it's >>> dead or unreachable, is obviously not (at least not to users). >>> >>> The attached patch enhances the gimple_fold_builtin_memory_op >>> function to eliminate this patholohgical case by making use >>> of range information to fold into no-ops calls to memcpy whose >>> size argument is in a range where the only valid value is zero. >>> This gets rid of the warning and improves the emitted code. >>> >>> Tested on x86_64-linux. >> >> @@ -646,6 +677,12 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi, >> tree destvar, srcvar; >> location_t loc = gimple_location (stmt); >> >> + if (tree valid_len = only_valid_value (len)) >> + { >> + /* LEN is in range whose only valid value is zero. */ >> + len = valid_len; >> + } >> + >> /* If the LEN parameter is zero, return DEST. */ >> if (integer_zerop (len)) >> >> just enhance this check to >> >> if (integer_zerop (len) >> || size_must_be_zero_p (len)) >> >> ? 'only_valid_value' looks too generic for this. > > Sure. > > FWIW, the reason I did it this was because my original patch > returned error_mark_node for entirely invalid ranges and had > the caller replace the call with a trap. I decided not to > include that part in this fix to keep it contained. > >> >> + if (!wi::fits_uhwi_p (min) || !wi::fits_uhwi_p (max)) >> + return NULL_TREE; >> + >> >> why? > > Only because I never remember what APIs are safe to use with > what input. > >> + if (wi::eq_p (min, wone) >> + && wi::geu_p (max + 1, ssize_max)) >> >> if (wi::eq_p (min, 1) >> && wi::gtu_p (max, wi::max_value (prec, SIGNED))) >> >> your ssize_max isn't signed size max, and max + 1 might overflow to zero. > > You're right that the addition to max would be better done > as subtraction from the result of (1 << N). Thank you. > > If (max + 1) overflowed then (max == TYPE_MAX) would have > to hold which I thought could never be true for an anti > range. (The patch includes tests for this case.) Was I > wrong? > > Attached is an updated version with the suggested changes > plus an additional test to verify the absence of warnings. > > Martin > > PR middle-end/81908 - FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g -m32 > > gcc/ChangeLog: > > PR middle-end/81908 > * gimple-fold.c (size_must_be_zero_p): New function. > (gimple_fold_builtin_memory_op): Call it. > > gcc/testsuite/ChangeLog: > > PR middle-end/81908 > * gcc.dg/tree-ssa/builtins-folding-gimple-2.c: New test. > * gcc.dg/tree-ssa/builtins-folding-gimple-3.c: New test. > * gcc.dg/tree-ssa/pr81908.c: New test. > > diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c > index 251446c..c4184fa 100644 > --- a/gcc/gimple-fold.c > +++ b/gcc/gimple-fold.c > @@ -628,6 +628,36 @@ var_decl_component_p (tree var) > return SSA_VAR_P (inner); > } > > +/* If the SIZE argument representing the size of an object is in a range > + of values of which exactly one is valid (and that is zero), return > + true, otherwise false. */ > + > +static bool > +size_must_be_zero_p (tree size) > +{ > + if (integer_zerop (size)) > + return true; > + > + if (TREE_CODE (size) != SSA_NAME) > + return false; > + > + wide_int min, max; > + enum value_range_type rtype = get_range_info (size, &min, &max); > + if (rtype != VR_ANTI_RANGE) > + return false; > + > + tree type = TREE_TYPE (size); > + int prec = TYPE_PRECISION (type); > + > + wide_int wone = wi::one (prec); > + > + /* Compute the value of SSIZE_MAX, the largest positive value that > + can be stored in ssize_t, the signed counterpart of size_t . */ > + wide_int ssize_max = wi::lshift (wi::one (prec), prec - 1) - 1; > + > + return wi::eq_p (min, wone) && wi::geu_p (max, ssize_max); Just a stylistic thing, but since the only use of "wone" is in the eq_p, it'd be simpler just to use "1". Also, the maximum value is better calculated as "wi::max_value (prec, SIGNED)". So: /* Compute the value of SSIZE_MAX, the largest positive value that can be stored in ssize_t, the signed counterpart of size_t . */ wide_int ssize_max = wi::max_value (prec, SIGNED); return wi::eq_p (min, 1) && wi::geu_p (max, ssize_max); Thanks, Richard
> Just a stylistic thing, but since the only use of "wone" is in > the eq_p, it'd be simpler just to use "1". Also, the maximum > value is better calculated as "wi::max_value (prec, SIGNED)". So: > > /* Compute the value of SSIZE_MAX, the largest positive value that > can be stored in ssize_t, the signed counterpart of size_t . */ > wide_int ssize_max = wi::max_value (prec, SIGNED); > > return wi::eq_p (min, 1) && wi::geu_p (max, ssize_max); Thanks, I didn't know about the implicit conversion or the max_value API. I'll remember to use them the next time. FWIW, going slightly off topic, and while I'm sure it's a matter of getting used to the design, I can't say I find the wide_int classes exactly intuitive. It seems that the most natural way to write the return statement in C++ is: return min == 1 && max >= ssize_max; The first subexpression is accepted but the second one doesn't even compile. If it did, it would treat the operands as signed which isn't what I need here. One still has to resort to the clunky predicate for it: return min == 1 && wi::geu_p (max, ssize_max); It also doesn't help that the names of the predicates don't follow the same convention as those that operate on trees (e.g., eq_p vs tree_int_cst_equal). Unless there's some inherent limitation that makes it impossible to use the class the way I would like it might be worth investing some time into making it more user-friendly. Martin
Martin Sebor <msebor@gmail.com> writes: >> Just a stylistic thing, but since the only use of "wone" is in >> the eq_p, it'd be simpler just to use "1". Also, the maximum >> value is better calculated as "wi::max_value (prec, SIGNED)". So: >> >> /* Compute the value of SSIZE_MAX, the largest positive value that >> can be stored in ssize_t, the signed counterpart of size_t . */ >> wide_int ssize_max = wi::max_value (prec, SIGNED); >> >> return wi::eq_p (min, 1) && wi::geu_p (max, ssize_max); > > Thanks, I didn't know about the implicit conversion or the max_value > API. I'll remember to use them the next time. > > FWIW, going slightly off topic, and while I'm sure it's a matter > of getting used to the design, I can't say I find the wide_int > classes exactly intuitive. It seems that the most natural way > to write the return statement in C++ is: > > return min == 1 && max >= ssize_max; > > The first subexpression is accepted but the second one doesn't even > compile. If it did, it would treat the operands as signed which > isn't what I need here. One still has to resort to the clunky > predicate for it: > > return min == 1 && wi::geu_p (max, ssize_max); There are two main reasons for the wi:: stuff: 1. When we were adding wide-int initially, one of the requirements was that we could operate directly on rtx and tree representations of integers, without first converting them to wide_int etc. Since trees and rtxes are pointers, it wouldn't make sense to rely on operator overloading there. 2. wide_int itself has no inherent sign. (So I disagree with "If it did, it would treat the operands as signed which isn't what I need here." The reason for not having the overload is exactly that there is no obvious choice between signed and unsigned here.) And yes, Richard has been giving me grief about the lack of a wide_int-with-sign :-) > It also doesn't help that the names of the predicates don't follow > the same convention as those that operate on trees (e.g., eq_p vs > tree_int_cst_equal). > > Unless there's some inherent limitation that makes it impossible > to use the class the way I would like it might be worth investing > some time into making it more user-friendly. I think we already provide the overloads we can (i.e. those where the sign is known or isn't relevant). Let me know if we've missed some though. Thanks, Richard
PR middle-end/81908 - FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g -m32 gcc/ChangeLog: PR middle-end/81908 * gimple-fold.c (size_must_be_zero_p): New function. (gimple_fold_builtin_memory_op): Call it. gcc/testsuite/ChangeLog: PR middle-end/81908 * gcc.dg/tree-ssa/builtins-folding-gimple-2.c: New test. * gcc.dg/tree-ssa/builtins-folding-gimple-3.c: New test. * gcc.dg/tree-ssa/pr81908.c: New test. diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 251446c..c4184fa 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -628,6 +628,36 @@ var_decl_component_p (tree var) return SSA_VAR_P (inner); } +/* If the SIZE argument representing the size of an object is in a range + of values of which exactly one is valid (and that is zero), return + true, otherwise false. */ + +static bool +size_must_be_zero_p (tree size) +{ + if (integer_zerop (size)) + return true; + + if (TREE_CODE (size) != SSA_NAME) + return false; + + wide_int min, max; + enum value_range_type rtype = get_range_info (size, &min, &max); + if (rtype != VR_ANTI_RANGE) + return false; + + tree type = TREE_TYPE (size); + int prec = TYPE_PRECISION (type); + + wide_int wone = wi::one (prec); + + /* Compute the value of SSIZE_MAX, the largest positive value that + can be stored in ssize_t, the signed counterpart of size_t . */ + wide_int ssize_max = wi::lshift (wi::one (prec), prec - 1) - 1; + + return wi::eq_p (min, wone) && wi::geu_p (max, ssize_max); +} + /* Fold function call to builtin mem{{,p}cpy,move}. Return false if no simplification can be made. If ENDP is 0, return DEST (like memcpy). @@ -646,8 +676,9 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi, tree destvar, srcvar; location_t loc = gimple_location (stmt); - /* If the LEN parameter is zero, return DEST. */ - if (integer_zerop (len)) + /* If the LEN parameter is a constant zero or in range where + the only valid value is zero, return DEST. */ + if (size_must_be_zero_p (len)) { gimple *repl; if (gimple_call_lhs (stmt)) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-2.c new file mode 100644 index 0000000..5bdb342 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-2.c @@ -0,0 +1,44 @@ +/* PR 81908 - FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g -m32 + Test to verify that calls to memcpy et al. where the size is in a range + with just one valid value -- zero -- are eliminated. + { dg-do compile } + { dg-options "-O2 -Wall -fdump-tree-optimized" } */ + +#define INT_MAX __INT_MAX__ +#define SHRT_MAX __SHRT_MAX__ +#define SIZE_MAX __SIZE_MAX__ +#define SSIZE_MAX (SIZE_MAX / 2) + +typedef __PTRDIFF_TYPE__ ssize_t; +typedef __SIZE_TYPE__ size_t; + +#define UNIQUE_FUNCNAME(func, line) test_ ## func ## _ ## line +#define FUNCNAME(func, line) UNIQUE_FUNCNAME (func, line) + +#define AR(func, type, min, max, val) \ + void __attribute__ ((noclone, noinline)) \ + FUNCNAME (func, __LINE__) (char *d, const char *s, type n) \ + { \ + if ((type)min <= n && n <= (type)max) \ + n = val; \ + __builtin_ ## func (d, s, n); \ + } typedef void DummyType + +AR (memcpy, short, 1, SHRT_MAX, 0); +AR (memcpy, int, 1, INT_MAX, 0); +AR (memcpy, size_t, 1, SSIZE_MAX, 0); +AR (memcpy, ssize_t, 1, SSIZE_MAX, 0); + +AR (memmove, short, 1, SHRT_MAX, 0); +AR (memmove, int, 1, INT_MAX, 0); +AR (memmove, ssize_t, 1, SSIZE_MAX, 0); +AR (memmove, ssize_t, 1, SSIZE_MAX, 0); + +AR (mempcpy, short, 1, SHRT_MAX, 0); +AR (mempcpy, int, 1, INT_MAX, 0); +AR (mempcpy, size_t, 1, SSIZE_MAX, 0); +AR (mempcpy, ssize_t, 1, SSIZE_MAX, 0); + +/* { dg-final { scan-tree-dump-not "builtin_memcpy" "optimized" } } + { dg-final { scan-tree-dump-not "builtin_memmove" "optimized" } } + { dg-final { scan-tree-dump-not "builtin_mempcpy" "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-3.c new file mode 100644 index 0000000..716be5b8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-3.c @@ -0,0 +1,43 @@ +/* PR 81908 - FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g -m32 + Test to verify that calls to memcpy et al. where the size is in a range + with more than one valid value are not eliminated (this test complements + builtins-folding-gimple-2.c). + { dg-do compile } + { dg-options "-O2 -Wall -fdump-tree-optimized" } */ + +#define SHRT_MAX __SHRT_MAX__ +#define SHRT_MIN (-SHRT_MAX - 1) +#define INT_MAX __INT_MAX__ +#define INT_MIN (-INT_MAX - 1) + +#define UNIQUE_FUNCNAME(func, line) test_ ## func ## _ ## line +#define FUNCNAME(func, line) UNIQUE_FUNCNAME (func, line) + +#define AR(func, type, min, max, val) \ + void __attribute__ ((noclone, noinline)) \ + FUNCNAME (func, __LINE__) (char *d, const char *s, type n) \ + { \ + if ((type)min <= n && n <= (type)max) \ + n = val; \ + __builtin_ ## func (d, s, n); \ + } typedef void DummyType + +AR (memcpy, short, SHRT_MIN, 0, 1); +AR (memcpy, short, SHRT_MIN, 1, 2); +AR (memcpy, short, 2, SHRT_MAX, 1); + +AR (memcpy, int, INT_MIN, 0, 1); +AR (memcpy, int, INT_MIN, 1, 2); +AR (memcpy, int, INT_MIN, 2, 3); +AR (memcpy, int, 2, INT_MAX, 1); +AR (memcpy, int, 2, INT_MAX, 1); + +AR (memmove, short, 2, SHRT_MAX, 1); +AR (memmove, int, 2, INT_MAX, 1); + +AR (mempcpy, short, 2, SHRT_MAX, 1); +AR (mempcpy, int, 2, INT_MAX, 1); + +/* { dg-final { scan-tree-dump-times "builtin_memcpy" 8 "optimized" } } + { dg-final { scan-tree-dump-times "builtin_memmove" 2 "optimized" } } + { dg-final { scan-tree-dump-times "builtin_mempcpy" 2 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr81908.c b/gcc/testsuite/gcc.dg/tree-ssa/pr81908.c new file mode 100644 index 0000000..b1e316a --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr81908.c @@ -0,0 +1,46 @@ +/* PR 81908 - FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g -m32 + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +#define SIZE_MAX __SIZE_MAX__ +typedef __SIZE_TYPE__ size_t; + +void f0 (char *d, const char *s, size_t n) +{ + if (n > 0 && n <= SIZE_MAX / 2 - 1) + n = 0; + + __builtin_memcpy (d, s, n); +} + +void f1 (char *d, const char *s, size_t n) +{ + if (n > 0 && n <= SIZE_MAX / 2) + n = 0; + + __builtin_memcpy (d, s, n); /* { dg-bogus "\\\[-Wstringop-overflow=]" } */ +} + +void f2 (char *d, const char *s, size_t n) +{ + if (n > 0 && n <= SIZE_MAX / 2 + 1) + n = 0; + + __builtin_memcpy (d, s, n); +} + +void f3 (char *d, const char *s, size_t n) +{ + if (n > 0 && n <= SIZE_MAX - 1) + n = 0; + + __builtin_memcpy (d, s, n); /* { dg-bogus "\\\[-Wstringop-overflow=]" } */ +} + +void f4 (char *d, const char *s, size_t n) +{ + if (n > 0 && n <= SIZE_MAX) + n = 0; + + __builtin_memcpy (d, s, n); +}