This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix wrong-code when folding X - (X / Y) * Y (PR tree-optimization/67953)
- From: Marc Glisse <marc dot glisse at inria dot fr>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
- Date: Thu, 15 Oct 2015 12:13:48 +0200 (CEST)
- Subject: Re: [PATCH] Fix wrong-code when folding X - (X / Y) * Y (PR tree-optimization/67953)
- Authentication-results: sourceware.org; auth=none
- References: <20151014182731 dot GJ13672 at redhat dot com> <alpine dot DEB dot 2 dot 20 dot 1510142047390 dot 2081 at laptop-mg dot saclay dot inria dot fr> <20151015093724 dot GK13672 at redhat dot com>
On Thu, 15 Oct 2015, Marek Polacek wrote:
On Wed, Oct 14, 2015 at 08:54:11PM +0200, Marc Glisse wrote:
On Wed, 14 Oct 2015, Marek Polacek wrote:
Evidently, the X - (X / Y) * Y -> X % Y pattern can't change the
signedness of X from signed to unsigned, otherwise we'd generate
wrong code. (But unsigned -> signed should be fine.)
Does anyone see a better fix than this?
I was wondering about producing:
(convert (trunc_mod @0 @1))
That's exactly what we had before r225195.
aka https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02239.html
Maybe that patch was wrong? There are other ways to work around the fact
that operand_equal_p ignores the type of constants. One would be to use
the pattern:
(minus (convert1? @2) (convert2? (mult (trunc_div @0 @1) @1)))
and manually test operand_equal_p (@0, @2) (for constants we could even
catch one extra rare case by testing that @0 converted to type is the same
as @2), this way @0 has the right type. Another way would be to have
trunc_div@2, with type2=TREE_TYPE(@2), and get
(convert (trunc_mod (convert:type2 @0) @1))
Richard, when we have several occurences of @0 in a pattern, would it be
hard to add some annotation to tell the machinery which one to capture
(since they are not equivalent in the case of constants)? Or maybe by
default pick the first one that is not directly inside "convert?"?
Aren't there also problems if the conversion changes the precision? I can
imagine your patch ending in x % 0, with @1 non-zero but a narrowing cast.
Hmm, I haven't found such a case yet. If you find something, pass
it along ;).
It is hard to trigger because we are missing :c on the mult. If you add
it, I expect the following to be an issue:
#include <stdio.h>
int f(long a, long b){
int x = (int)a;
long z = a / b * b;
int t = (int)z;
return x-t;
}
int main(){
long a = (1L<<33)+2;
long b = 1L<<32;
printf("%d\n", f(a,b));
return 0;
}
--
Marc Glisse