| Submitter | Jakub Jelinek |
|---|---|
| Date | Feb. 27, 2011, 6:16 p.m. |
| Message ID | <20110227181619.GE30899@tyan-ft48-01.lab.bos.redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/84699/ |
| State | New |
| Headers | show |
Comments
On Sun, Feb 27, 2011 at 7:16 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Sun, Feb 27, 2011 at 03:51:47PM +0100, Richard Guenther wrote: >> On Sun, Feb 27, 2011 at 1:13 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> > In some places CONST_DOUBLE bits are compared as the HWIs, thus it is >> > undesirable to have uninitialized bits among them. At least on LP64 hosts >> > there are 32 bits of padding between uexp and sign fields. >> > Most of the places in gcc that work with REAL_VALUE_TYPE seem to actually >> > either clear the whole REAL_VALUE_TYPE by memset, or start by copying >> > to result one of the arguments. The exception seems to be real_arithmetic >> > for PLUS/MINUS/MULT/RDIV, where the result might be the same as either >> > of the operands, or might be completely different. Either real_arithmetic >> > in those 4 cases could compare result ptr with both operands and if it is >> > not equal to either of them, clear it, or the following patch adjusts >> > the callers where they do it. >> >> Hm, it seems fishy to compare CONST_DOUBLEs as HWIs if they have >> padding. But well - I guess it was always that way. > > In loc_cmp case it wants stable comparison for sorting, and real comparison > isn't suitable for that, as it e.g. considers +0. and -0. to be equal. > But the code in real_value_from_int_cst is quite old: > 31038 kenner /* Clear all bits of the real value type so that we can later do > 31038 kenner bitwise comparisons to see if two values are the same. */ > 69587 ghazi memset (&d, 0, sizeof d); > so I think it loc_cmp isn't the only one which does something like that. > >> That said, I still would prefer the clearing to be done in >> real_arithmetic. Just patching the existing callers won't save us >> from introducing more cases like this. > > Here it is, bootstrapped/regtested on x86_64-linux and i686-linux, > verified with valgrind on the testcase: Ok. Thanks, Richard. > 2011-02-27 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/47903 > * real.c (real_arithmetic) <case PLUS_EXPR, MINUS_EXPR, > MULT_EXPR, RDIV_EXPR>: Clear padding bits in *r first if > r isn't op0 nor op1. > > --- gcc/real.c.jj 2010-12-02 13:15:24.000000000 +0100 > +++ gcc/real.c 2011-02-27 16:55:43.474134359 +0100 > @@ -1009,15 +1009,25 @@ real_arithmetic (REAL_VALUE_TYPE *r, int > switch (code) > { > case PLUS_EXPR: > + /* Clear any padding areas in *r if it isn't equal to one of the > + operands so that we can later do bitwise comparisons later on. */ > + if (r != op0 && r != op1) > + memset (r, '\0', sizeof (*r)); > return do_add (r, op0, op1, 0); > > case MINUS_EXPR: > + if (r != op0 && r != op1) > + memset (r, '\0', sizeof (*r)); > return do_add (r, op0, op1, 1); > > case MULT_EXPR: > + if (r != op0 && r != op1) > + memset (r, '\0', sizeof (*r)); > return do_multiply (r, op0, op1); > > case RDIV_EXPR: > + if (r != op0 && r != op1) > + memset (r, '\0', sizeof (*r)); > return do_divide (r, op0, op1); > > case MIN_EXPR: > > > Jakub >
Patch
--- gcc/real.c.jj 2010-12-02 13:15:24.000000000 +0100 +++ gcc/real.c 2011-02-27 16:55:43.474134359 +0100 @@ -1009,15 +1009,25 @@ real_arithmetic (REAL_VALUE_TYPE *r, int switch (code) { case PLUS_EXPR: + /* Clear any padding areas in *r if it isn't equal to one of the + operands so that we can later do bitwise comparisons later on. */ + if (r != op0 && r != op1) + memset (r, '\0', sizeof (*r)); return do_add (r, op0, op1, 0); case MINUS_EXPR: + if (r != op0 && r != op1) + memset (r, '\0', sizeof (*r)); return do_add (r, op0, op1, 1); case MULT_EXPR: + if (r != op0 && r != op1) + memset (r, '\0', sizeof (*r)); return do_multiply (r, op0, op1); case RDIV_EXPR: + if (r != op0 && r != op1) + memset (r, '\0', sizeof (*r)); return do_divide (r, op0, op1); case MIN_EXPR: