[PATCH] Fix get_bit_range and expand_assignment for negative bitpos (PR middle-end/58970)
Hi!
This is an updated version of the get_bit_range fix, this time it
ensures that bitpos is not negative already right after the
get_inner_reference call, because various parts of the expansion might be
confused by the negative values. As the second testcase shows, even with
non-negative bitpos we can still end up with bitoffset > *bitpos when *offset
is NULL, so get_bit_range handles that too.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (and
after a while 4.8)?
2013-11-05 Jakub Jelinek
PR middle-end/58970
* expr.c (get_bit_range): Handle *offset == NULL_TREE.
(expand_assignment): If *bitpos is negative, set *offset
and adjust *bitpos, so that it is not negative.
* gcc.c-torture/compile/pr58970-1.c: New test.
* gcc.c-torture/compile/pr58970-2.c: New test.
--- gcc/expr.c.jj 2013-11-04 11:27:35.769278058 +0100
+++ gcc/expr.c 2013-11-04 21:40:54.283437813 +0100
@@ -4574,19 +4574,19 @@ get_bit_range (unsigned HOST_WIDE_INT *b
- tree_low_cst (DECL_FIELD_BIT_OFFSET (repr), 1));
/* If the adjustment is larger than bitpos, we would have a negative bit
- position for the lower bound and this may wreak havoc later. This can
- occur only if we have a non-null offset, so adjust offset and bitpos
- to make the lower bound non-negative. */
+ position for the lower bound and this may wreak havoc later. Adjust
+ offset and bitpos to make the lower bound non-negative in that case. */
if (bitoffset > *bitpos)
{
HOST_WIDE_INT adjust = bitoffset - *bitpos;
-
gcc_assert ((adjust % BITS_PER_UNIT) == 0);
- gcc_assert (*offset != NULL_TREE);
*bitpos += adjust;
- *offset
- = size_binop (MINUS_EXPR, *offset, size_int (adjust / BITS_PER_UNIT));
+ if (*offset == NULL_TREE)
+ *offset = size_int (-adjust / BITS_PER_UNIT);
+ else
+ *offset
+ = size_binop (MINUS_EXPR, *offset, size_int (adjust / BITS_PER_UNIT));
*bitstart = 0;
}
else
@@ -4719,6 +4719,15 @@ expand_assignment (tree to, tree from, b
tem = get_inner_reference (to, &bitsize, &bitpos, &offset, &mode1,
&unsignedp, &volatilep, true);
+ /* Make sure bitpos is not negative, it can wreak havoc later. */
+ if (bitpos < 0)
+ {
+ gcc_assert (offset == NULL_TREE);
+ offset = size_int (bitpos >> (BITS_PER_UNIT == 8
+ ? 3 : exact_log2 (BITS_PER_UNIT)));
+ bitpos &= BITS_PER_UNIT - 1;
+ }
+
if (TREE_CODE (to) == COMPONENT_REF
&& DECL_BIT_FIELD_TYPE (TREE_OPERAND (to, 1)))
get_bit_range (&bitregion_start, &bitregion_end, to, &bitpos, &offset);
--- gcc/testsuite/gcc.c-torture/compile/pr58970-1.c.jj 2013-11-04
21:27:47.531590559 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr58970-1.c 2013-11-04
21:27:47.531590559 +0100
@@ -0,0 +1,11 @@
+/* PR middle-end/58970 */
+
+struct T { int b : 1; };
+struct S { struct T t[1]; };
+
+void
+foo (int x, struct S *s)
+{
+ if (x == -1)
+s->t[x].b = 0;
+}
--- gcc/testsuite/gcc.c-torture/compile/pr58970-2.c.jj 2013-08-25
18:20:55.717911035 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr58970-2.c 2013-11-04
21:45:41.997935365 +0100
@@ -0,0 +1,11 @@
+/* PR middle-end/58970 */
+
+struct T { char a : 8; char b : 1; };
+struct S { char x; struct T t[1]; };
+
+void
+foo (int x, struct S *s)
+{
+ if (x == -1)
+s->t[x].b = 0;
+}
Jakub
[driver] try to fix "driver specifies output file twice" bug
Hello,
This patch is trying to fix the bug (driver specifies output file
twice when compiling header with -S)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31316.
Changes:
* gcc.c (default_compilers): Update c-header specs.
By applying the patch, run command
$ gcc -S foo.h
will just output an assembly file foo.s.
$ gcc -fsyntax-only foo.h
no output file.
$ gcc foo.h -o foo.s
will output PCH file named foo.s.
$ gcc foo.h -save-temps
will output foo.i, foo.s, foo.h.gch.
But,
$ gcc foo.h -save-temps -o foo.s
will output foo.s, a mix of assembly and PCH. This should be fixed.
Any suggestions?
Thanks,
Mingjie
Index: gcc/gcc.c
===
--- gcc/gcc.c (revision 204285)
+++ gcc/gcc.c (working copy)
@@ -1048,14 +1048,15 @@ static const struct compiler default_com
%{!E:%{!M:%{!MM:\
%{save-temps*|traditional-cpp|no-integrated-cpp:%(trad_capable_cpp) \
%(cpp_options) -o %{save-temps*:%b.i} %{!save-temps*:%g.i} \n\
- cc1 -fpreprocessed %{save-temps*:%b.i} %{!save-temps*:%g.i} \
- %(cc1_options)\
-%{!fdump-ada-spec*:-o %g.s %{!o*:--output-pch=%i.gch}\
-%W{o*:--output-pch=%*}}%V}\
+ cc1 -fpreprocessed %{save-temps*:%b.i} %{!save-temps*:%g.i} \
+ %(cc1_options)\
+ %{!fsyntax-only:%{!S:%{!fdump-ada-spec*:-o %g.s \
+ %{!o*:--output-pch=%i.gch} %W{o*:--output-pch=%*%V}\
%{!save-temps*:%{!traditional-cpp:%{!no-integrated-cpp:\
cc1 %(cpp_unique_options) %(cc1_options)\
-%{!fdump-ada-spec*:-o %g.s %{!o*:--output-pch=%i.gch}\
-%W{o*:--output-pch=%*}}%V}}", 0, 0, 0},
+ %{!fsyntax-only:%{!S:%{!fdump-ada-spec*:-o %g.s \
+ %{!o*:--output-pch=%i.gch} %W{o*:--output-pch=%*%V}}}\
+}}}", 0, 0, 0},
{".i", "@cpp-output", 0, 0, 0},
{"@cpp-output",
"%{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %(cc1_options) %{!fsyntax-only:%(invoke_as)", 0, 0, 0},
Re: [PATCH i386 4/8] [AVX512] [1/n] Add substed patterns.
Hello, On 01 Nov 16:19, Kirill Yukhin wrote: > Coould you pls take a look? PING. -- Thanks, K
Re: [RFC] libgcov.c re-factoring and offline profile-tool
> I wonder if it makes sense to get rid of the conditional compile such as > > #ifdef L_gcov_x > .. > > #endif > > This has the advantage of producing slightly smaller instrumented > binary, but this benefit can also be achieved via -ffunction-sections > and let linker to garbage collect unused functions. > > (probably as a follow up if it makes sense). I believe we use this funny scheme primarily for targets that have no function section support (and also the hand made splitting can be more sane). For a low-level library like libgcov is that is supposed to have small footprint, I guess the current scheme is resonable thing to do Honza
Re: [RFC] libgcov.c re-factoring and offline profile-tool
On Tue, Nov 05, 2013 at 10:18:50AM +0100, Jan Hubicka wrote: > > I wonder if it makes sense to get rid of the conditional compile such as > > > > #ifdef L_gcov_x > > .. > > > > #endif > > > > This has the advantage of producing slightly smaller instrumented > > binary, but this benefit can also be achieved via -ffunction-sections > > and let linker to garbage collect unused functions. > > > > (probably as a follow up if it makes sense). > > I believe we use this funny scheme primarily for targets that have no function > section support (and also the hand made splitting can be more sane). > For a low-level library like libgcov is that is supposed to have small > footprint, > I guess the current scheme is resonable thing to do I think the #ifdef L* stuff is there mainly so that we have small granularity in the *.a libraries (one function per object file), so that one links only what is really required (at least, that is why libgcc.a does that). Of course, if there are many interdependencies between the functions and you always essentially link in everything, the usefulness of that is lower. Jakub
Rework doloop interface
Following on from:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00092.html
it looks like I'll have to "fix" the doloop_begin/end interface after all.
The current code goes on to do some rtx arithmetic on invalid CONST_INTs.
(FWIW, the arithmetic seems unnecessary and I think we could simply drop it,
but the weirdness of the interface would remain.)
So: doloop_end has 6 parameters, of which 4 are really just information
about the loop. These 4 parameters never end up in the instruction
stream on current targets and aren't IMO really rtxes. But because
expanders can only take rtx arguments, we still need to wrap them in
some kind of rtx container.
This causes problems because once you start representing something as
an rtx, you need to start thinking about modes. One of the parameters
is the maximum number of iterations, and it isn't obvious what its mode
should be; the mode of the loop counter is too small to hold an iteration
count of 1 << GET_MODE_PRECISION. Another of the parameters is the loop
level, which has no obvious mode at all. One is a boolean, which could
be represented as BImode, but then we enter STORE_FLAG_VALUE territory.
So this patch adds a new target hook, can_use_doloop_p, that takes
these parameters as normal C arguments. The default returns true,
but there's also a canned alternative for targets that can only handle
innermost loops.
This should also be slightly more efficient, since we can avoid creating
the loop counter register and label if the loop "obviously" isn't suitable.
It also avoids creating garbage rtl for the 4 not-really-rtx parameters.
Tested by building C and C++ for:
arc-elf
arm-linux-gnueabi
bfin-elf
c6x-elf
ia64-linux-gnu
mep-elf
powerpc-linux-gnu
s390-linux-gnu
sh-linux-gnu
spu-elf
tilegx-elf
tilepro-elf
v850-elf
and comparing the -O2 assembly output of gcc.c-torture, gcc.dg and g++.dg
from before and after the patch (as far as possible without target headers).
For this I moved the can_use_doloop_p test to just before the gen_doloop_end
so that register and label numbers stayed the same. There were two
differences:
- gcc.c-torture/compile/pr44030.c for arc-elf, where we now use the doloop
instruction and didn't previously. This is because arc-elf checked:
/* Setting up the loop with two sr isntructions costs 6 cycles. */
if (TARGET_ARC700 && !INTVAL (operands[5])
&& INTVAL (operands[1]) && INTVAL (operands[1]) <= (flag_pic ? 6 : 3))
FAIL;
where operands[1] is the constant number of iterations. Unlike the
maximum number of iterations (operands[2]), this value has the same
mode as the counter register and is properly sign-extended. So a large
positive iteration count appears negative and unintentionally triggered
the FAIL. So I think this counts as a bug fix.
- gcc.dg/graphite/id-9.c for bfin-elf. In this testcase the maximum number
of iterations is calculated as the double_int { high = 0, low = -1 },
which doesn't fit in a signed HWI, so the maximum number of iterations
was previously treated as 0. And the bfin code doesn't handle the
0/unknown case, which might be a bug:
/* Due to limitations in the hardware (an initial loop count of 0
does not loop 2^32 times) we must avoid to generate a hardware
loops when we cannot rule out this case. */
if (!flag_unsafe_loop_optimizations
&& (unsigned HOST_WIDE_INT) INTVAL (operands[2]) >= 0x)
FAIL;
So the old code allowed the doloop to be used here, which I don't
think was intentional. Now the bfin code sees the original double_int
and rejects it as too big.
That said, it might also be a bug that we have { 0, -1 } rather than
{ 0, 0x } or { 0, 0x1 } (or even { -1, -1 }) for an
SImode loop, but this has got to stop somewhere...
Also bootstrapped & regression-tested on powerpc64-linux-gnu. OK to install?
Thanks,
Richard
gcc/
* target.def (can_use_doloop_p): New hook.
* doc/tm.texi.in (TARGET_CAN_USE_DOLOOP_P): Add.
* doc/tm.texi: Regenerate.
* doc/md.texi (doloop_begin, doloop_end): Update documentation.
* hooks.h (hook_bool_dint_dint_uint_true): Declare.
* hooks.c (hook_bool_dint_dint_uint_true): New function.
* targhooks.h (can_use_doloop_if_innermost): Declare.
* targhooks.c (can_use_doloop_if_innermost): New function.
* target.h: Include double-int.h.
* loop-doloop.c (doloop_optimize): Call targetm.can_use_doloop_p.
Remove iteration count, maximum iteration count, loop depth and
enter-at-top inputs from doloop_begin and doloop_end.
* config/arc/arc.md (doloop_begin, doloop_end): Update for new
interface.
* config/arc/arc.c (arc_can_use_doloop_p): New function.
(TARGET_CAN_USE_DOLOOP_P): Define.
* config/arm/thumb2.md (doloop_end): Update
Re: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals
Bernd Edlinger writes:
[...]
>> if (!string_len)
>> {
>> string_len = 200;
>> - string = XNEWVEC (char, string_len);
>> + string = XCNEWVEC (char, string_len);
>> }
>> + else
>> + memset (string, 0, string_len);
>
> Is this memset still necessary?
Of course not ...
[...]
> If "ptr" is passed to get_line it will try to reallocate it,
> which must fail, right?
>
> Maybe, this line of code is unreachable?
>
> Who is responsible for reallocating "string" get_line or read_line?
Correct, these are real concerns.
I am wondering what I was thinking. Actually, I think read_line should
almost just call get_line now. Like what is done in the new version of
the patch below; basically if there is a line to return, read_line just
gets it (the static buffer containing the line) from get_line and
returns it, otherwise the static buffer containing the last read line is
left untouched and read_line returns a NULL constant.
I guess this resolves the valid concern that you raised below:
> If the previous invocation of read_line already had read some
> characters of the following line, how is that information
> recovered? How is it detected if another file is to be read this
> time?
Thank you very much for this thorough review.
Here is the updated patch that I am bootstrapping:
gcc/ChangeLog:
* input.h (location_get_source_line): Take an additional line_size
parameter.
* input.c (get_line): New static function definition.
(read_line): Take an additional line_length output parameter to be
set to the size of the line. Use the new get_line function do the
actual line reading.
(location_get_source_line): Take an additional output line_len
parameter. Update the use of read_line to pass it the line_len
parameter.
* diagnostic.c (adjust_line): Take an additional input parameter
for the length of the line, rather than calculating it with
strlen.
(diagnostic_show_locus): Adjust the use of
location_get_source_line and adjust_line with respect to their new
signature. While displaying a line now, do not stop at the first
null byte. Rather, display the zero byte as a space and keep
going until we reach the size of the line.
gcc/testsuite/ChangeLog:
* c-c++-common/cpp/warning-zero-in-literals-1.c: New test file.
---
gcc/diagnostic.c | 17 ++--
gcc/input.c| 111 -
gcc/input.h| 3 +-
.../c-c++-common/cpp/warning-zero-in-literals-1.c | Bin 0 -> 240 bytes
4 files changed, 97 insertions(+), 34 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/cpp/warning-zero-in-literals-1.c
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 36094a1..e0c5d9d 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -259,12 +259,13 @@ diagnostic_build_prefix (diagnostic_context *context,
MAX_WIDTH by some margin, then adjust the start of the line such
that the COLUMN is smaller than MAX_WIDTH minus the margin. The
margin is either 10 characters or the difference between the column
- and the length of the line, whatever is smaller. */
+ and the length of the line, whatever is smaller. The length of
+ LINE is given by LINE_WIDTH. */
static const char *
-adjust_line (const char *line, int max_width, int *column_p)
+adjust_line (const char *line, int line_width,
+int max_width, int *column_p)
{
int right_margin = 10;
- int line_width = strlen (line);
int column = *column_p;
right_margin = MIN (line_width - column, right_margin);
@@ -284,6 +285,7 @@ diagnostic_show_locus (diagnostic_context * context,
const diagnostic_info *diagnostic)
{
const char *line;
+ int line_width;
char *buffer;
expanded_location s;
int max_width;
@@ -297,22 +299,25 @@ diagnostic_show_locus (diagnostic_context * context,
context->last_location = diagnostic->location;
s = expand_location_to_spelling_point (diagnostic->location);
- line = location_get_source_line (s);
+ line = location_get_source_line (s, &line_width);
if (line == NULL)
return;
max_width = context->caret_max_width;
- line = adjust_line (line, max_width, &(s.column));
+ line = adjust_line (line, line_width, max_width, &(s.column));
pp_newline (context->printer);
saved_prefix = pp_get_prefix (context->printer);
pp_set_prefix (context->printer, NULL);
pp_space (context->printer);
- while (max_width > 0 && *line != '\0')
+ while (max_width > 0 && line_width > 0)
{
char c = *line == '\t' ? ' ' : *line;
+ if (c == '\0')
+ c = ' ';
pp_character (context->printer, c);
max_width--;
+ line_width--;
line++;
}
pp_newline (context->printer);
diff --git a/gcc/input.c b/gcc/input.c
index a141a92..9526d88 100644
--- a/gcc/input.c
+++
Re: Testsuite / Cilk Plus: Include library path in compile flags in gcc.dg/cilk-plus/cilk-plus.exp
hi Balaji sorry for a short reply, but I'm traveling this week, On 4 Nov 2013, at 23:02, Iyer, Balaji V wrote: > 2). The C tests are extremely time-consuming (m32/m64) with NO -jxx on the > c/l: > > x86_64-linux (32 cores 2.8G Xeon) : 36m/421m/6m > x86_64-darwin12 (8 cores 2.8G Xeon) : 45m/309m/2m. > > c.f. the g++ tests which take ~2m wall clock. > > BVI: yes, I know about this. It is because some of the Cilk tests are run for > higher iterations to make it force a steal. This is one of the main place > were runtime bugs could occur. My original solution was to put them all in > separate directories with separate scripts to run them, but someone in GCC > community did not like that idea. This way, when people compile with -jN, > they will be run in parallel. If that is a good idea, I can look into > reorganising them. total CPU time consumed is relevant - folks can already get parallel testing (different suites for different languages will run in parallel). However, GCC is tested on a wide variety of hardware, e.g. even I use from core duo - 32core x86_64. There are facilities to identify tests as "expensive" - and testing that might add hundreds of minutes to the cycle would come into that category (at least, IMO). There should be some way to "opt in" if the tests are specifically of interest. sorry again for brevity, Iain
Re: [PATCH] Introducing SAD (Sum of Absolute Differences) operation to GCC vectorizer.
On Mon, Nov 04, 2013 at 06:30:55PM +, Cong Hou wrote:
> On Mon, Nov 4, 2013 at 2:06 AM, James Greenhalgh
> wrote:
> > On Fri, Nov 01, 2013 at 04:48:53PM +, Cong Hou wrote:
> >> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> >> index 2a5a2e1..8f5d39a 100644
> >> --- a/gcc/doc/md.texi
> >> +++ b/gcc/doc/md.texi
> >> @@ -4705,6 +4705,16 @@ wider mode, is computed and added to operand 3.
> >> Operand 3 is of a mode equal or
> >> wider than the mode of the product. The result is placed in operand 0,
> >> which
> >> is of the same mode as operand 3.
> >>
> >> +@cindex @code{ssad@var{m}} instruction pattern
> >> +@item @samp{ssad@var{m}}
> >> +@cindex @code{usad@var{m}} instruction pattern
> >> +@item @samp{usad@var{m}}
> >> +Compute the sum of absolute differences of two signed/unsigned elements.
> >> +Operand 1 and operand 2 are of the same mode. Their absolute difference,
> >> which
> >> +is of a wider mode, is computed and added to operand 3. Operand 3 is of a
> >> mode
> >> +equal or wider than the mode of the absolute difference. The result is
> >> placed
> >> +in operand 0, which is of the same mode as operand 3.
> >> +
> >> @cindex @code{ssum_widen@var{m3}} instruction pattern
> >> @item @samp{ssum_widen@var{m3}}
> >> @cindex @code{usum_widen@var{m3}} instruction pattern
> >> diff --git a/gcc/expr.c b/gcc/expr.c
> >> index 4975a64..1db8a49 100644
> >
> > I'm not sure I follow, and if I do - I don't think it matches what
> > you have implemented for i386.
> >
> > From your text description I would guess the series of operations to be:
> >
> > v1 = widen (operands[1])
> > v2 = widen (operands[2])
> > v3 = abs (v1 - v2)
> > operands[0] = v3 + operands[3]
> >
> > But if I understand the behaviour of PSADBW correctly, what you have
> > actually implemented is:
> >
> > v1 = widen (operands[1])
> > v2 = widen (operands[2])
> > v3 = abs (v1 - v2)
> > v4 = reduce_plus (v3)
> > operands[0] = v4 + operands[3]
> >
> > To my mind, synthesizing the reduce_plus step will be wasteful for targets
> > who do not get this for free with their Absolute Difference step. Imagine a
> > simple loop where we have synthesized the reduce_plus, we compute partial
> > sums each loop iteration, though we would be better to leave the reduce_plus
> > step until after the loop. "REDUC_PLUS_EXPR" would be the appropriate
> > Tree code for this.
>
> What do you mean when you use "synthesizing" here? For each pattern,
> the only synthesized operation is the one being returned from the
> pattern recognizer. In this case, it is USAD_EXPR. The recognition of
> reduce sum is necessary as we need corresponding prolog and epilog for
> reductions, which is already done before pattern recognition. Note
> that reduction is not a pattern but is a type of vector definition. A
> vectorization pattern can still be a reduction operation as long as
> STMT_VINFO_RELATED_STMT of this pattern is a reduction operation. You
> can check the other two reduction patterns: widen_sum_pattern and
> dot_prod_pattern for reference.
My apologies for not being clear. What I mean is, for a target which does
not have a dedicated PSADBW instruction, the individual steps of
'usad' must be "synthesized" in such a way as to match the expected
behaviour of the tree code.
So, I must expand 'usadm' to a series of equivalent instructions
as USAD_EXPR expects.
If USAD_EXPR requires me to emit a reduction on each loop iteration,
I think that will be inefficient compared to performing the reduction
after the loop body.
To a first approximation on ARM, I would expect from your description
of 'usad' that generating,
VABAL ops[3], ops[1], ops[2]
(Vector widening Absolute Difference and Accumulate)
would fulfil the requirements.
But to match the behaviour you have implemented in the i386
backend I would be required to generate:
VABAL ops[3], ops[1], ops[2]
VPADD ops[3], ops[3], ops[3] (add one set of pairs)
VPADD ops[3], ops[3], ops[3] (and the other)
VANDops[0], ops[3], MASK (clear high lanes)
Which additionally performs the (redundant) vector reduction
and high lane zeroing step on each loop iteration.
My comment is that your documentation and implementation are
inconsistent so I am not sure which behaviour you intend for USAD_EXPR.
Additionally, I think it would be more generic to choose the first
behaviour, rather than requiring a wasteful decomposition to match
a very particular i386 opcode.
Thanks,
James
Re: Patch RFA: With -fnon-call-exceptions sync builtins may throw
Ian Lance Taylor writes:
> gcc/testsuite/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor
>
> * g++.dg/ext/sync-4.C: New test.
Tested on m68k-suse-linux and x86_64-suse-linux and installed as obvious.
Andreas.
* g++.dg/ext/sync-4.C: Require sync_long_long_runtime support.
diff --git a/gcc/testsuite/g++.dg/ext/sync-4.C
b/gcc/testsuite/g++.dg/ext/sync-4.C
index f99fd48..14ed273 100644
--- a/gcc/testsuite/g++.dg/ext/sync-4.C
+++ b/gcc/testsuite/g++.dg/ext/sync-4.C
@@ -1,4 +1,5 @@
/* { dg-do run { target hppa*-*-hpux* *-*-linux* *-*-gnu* powerpc*-*-darwin*
*-*-darwin[912]* } } */
+/* { dg-require-effective-target sync_long_long_runtime } */
/* { dg-options "-fexceptions -fnon-call-exceptions -O2" } */
/* Verify that the builtin functions are correctly marked as trapping
--
1.8.4.2
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: PATCH: middle-end/58981: movmem/setmem use mode wider than Pmode for size
"H.J. Lu" writes: > This is the updated patch with md.texi change. The testcase is the same. > Tested on x32. OK to install? Since I commented on the original, this looks good to me, but I can't approve it. Thanks, Richard
RE: [PATCH GCC]Simplify address expression in IVOPT
> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of bin.cheng
> Sent: Monday, November 04, 2013 4:35 PM
> To: 'Richard Biener'
> Cc: GCC Patches
> Subject: RE: [PATCH GCC]Simplify address expression in IVOPT
>
>
>
> > -Original Message-
> > From: Richard Biener [mailto:richard.guent...@gmail.com]
> > Sent: Wednesday, October 30, 2013 10:46 PM
> > To: Bin Cheng
> > Cc: GCC Patches
> > Subject: Re: [PATCH GCC]Simplify address expression in IVOPT
> >
> > On Tue, Oct 29, 2013 at 10:18 AM, bin.cheng wrote:
> >
> > Hmm. I think you want what get_inner_reference_aff does, using the
> > return value of get_inner_reference as starting point for
> determine_base_object.
> > And you don't want to restrict yourselves so much on what exprs to
> process,
> > but only exclude DECL_Ps.
> Did you mean I should pass all address expressions to
> get_inner_reference_aff except the one with declaration operand?
> I am a little confused here why DECL_Ps should be handled specially?
Seems
> it can be handled properly by the get_* function. Anything important I
> missed?
>
> > Just amend get_inner_reference_aff to return the tree base object.
> I found it's inappropriate to do this because: functions like
> get_inner_reference* only accept reference expressions, while base_object
> has to be computed for other kinds of expressions too. Take gimple
> statement "a_1 = *ptr_1" as an example, the base passed to alloc_iv is
ptr_1;
> the base_object computed by determine_base_object is also ptr_1, which
> we can't rely on get_inner_reference* .
>
> Also It seems the comment before determine_base_object might be
> inaccurate:
> " Returns a memory object to that EXPR points." which I think is " Returns
a
> pointer pointing to the main memory object to that EXPR points."
> Right?
> >
> > Note that this isn't really "simplifying" but rather lowering all
> addresses.
> >
> > The other changes in this patch are unrelated, right?
> Right, I should have named this message like "refine cost computation of
> expressions in IVOPT" just as the patch file.
Hi,
I updated the patch according to review comments. Now it lowers all address
expressions except ones with DECL_P to get_inner_reference_aff, it also
computes base_object which is used later to ease determine_base_object.
Bootstrap and test on x86/x86_64/arm on going, is it OK if tests pass?
Thanks.
bin
gcc/testsuite/ChangeLog
2013-11-05 Bin Cheng
* gcc.dg/tree-ssa/loop-2.c: Refine check condition.
* gcc.dg/tree-ssa/ivopt_infer_2.c: Ditto.
* gcc.dg/tree-ssa/ivopt_mult_3.c: Ditto.
2013-11-05 Bin Cheng
* tree-ssa-loop-ivopts.c (alloc_iv): Lower address expressions.
(get_shiftadd_cost): Check equality using operand_equal_p.
(force_expr_to_var_cost): Refactor the code. Handle type
conversion.
(split_address_cost): Call force_expr_to_var_cost.
* tree-affine.c (get_inner_reference_aff): Return base_addr.
* tree-affine.h (get_inner_reference_aff): Change prototype.Index: gcc/testsuite/gcc.dg/tree-ssa/loop-2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/loop-2.c (revision 204117)
+++ gcc/testsuite/gcc.dg/tree-ssa/loop-2.c (working copy)
@@ -27,7 +27,7 @@ void xxx(void)
/* { dg-final { scan-tree-dump-times " \\* \[^\\n\\r\]*=" 0 "optimized" } } */
/* { dg-final { scan-tree-dump-times "\[^\\n\\r\]*= \\* " 0 "optimized" } } */
-/* { dg-final { scan-tree-dump-times "MEM" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[base" 1 "optimized" } } */
/* 17 * iter should be strength reduced. */
Index: gcc/testsuite/gcc.dg/tree-ssa/ivopt_infer_2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ivopt_infer_2.c (revision 204117)
+++ gcc/testsuite/gcc.dg/tree-ssa/ivopt_infer_2.c (working copy)
@@ -7,7 +7,8 @@
extern char a[];
-/* Can not infer loop iteration from array -- exit test can not be replaced.
*/
+/* Can not infer loop iteration from array -- exit test can not be
+ replaced by the array address. */
void foo (unsigned int i_width, TYPE dst)
{
unsigned long long i = 0;
@@ -21,5 +22,5 @@ void foo (unsigned int i_width, TYPE dst)
}
}
-/* { dg-final { scan-tree-dump-times "Replacing" 0 "ivopts"} } */
+/* { dg-final { scan-tree-dump-times "\[^:\]*if \\(.*j_\[0-9\]+.*\\)" 1
"ivopts"} } */
/* { dg-final { cleanup-tree-dump "ivopts" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ivopt_mult_3.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ivopt_mult_3.c(revision 204117)
+++ gcc/testsuite/gcc.dg/tree-ssa/ivopt_mult_3.c(working copy)
@@ -18,5 +18,5 @@ long foo(long* p, long* p2, int N1, int N2)
return s;
}
-/* { dg-final { scan-tree-dump-times "Replacing" 1 "ivopts"} } */
+/* {
Re: PR 58958: wrong aliasing info
On Mon, 4 Nov 2013, Richard Biener wrote:
Well, you cannot use the size argument unchanged for the null return
case. You could fallback to get_base_address and -1 size in that case.
Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
(I think I'll disable cilk for my future bootstraps: it takes forever and
confuses contrib/compare_tests)
2013-11-05 Marc Glisse
PR tree-optimization/58958
gcc/
* tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
get_addr_base_and_unit_offset instead of get_ref_base_and_extent.
gcc/testsuite/
* gcc.dg/tree-ssa/pr58958.c: New file.
--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+double a[10];
+int f(int n){
+ a[3]=9;
+ __builtin_memset(&a[n],3,sizeof(double));
+ return a[3]==9;
+}
+
+/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
___
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: gcc/tree-ssa-alias.c
===
--- gcc/tree-ssa-alias.c(revision 204381)
+++ gcc/tree-ssa-alias.c(working copy)
@@ -559,41 +559,50 @@ ao_ref_alias_set (ao_ref *ref)
}
/* Init an alias-oracle reference representation from a gimple pointer
PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the
size is assumed to be unknown. The access is assumed to be only
to or after of the pointer target, not before it. */
void
ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
{
- HOST_WIDE_INT t1, t2, extra_offset = 0;
+ HOST_WIDE_INT t, extra_offset = 0;
ref->ref = NULL_TREE;
if (TREE_CODE (ptr) == SSA_NAME)
{
gimple stmt = SSA_NAME_DEF_STMT (ptr);
if (gimple_assign_single_p (stmt)
&& gimple_assign_rhs_code (stmt) == ADDR_EXPR)
ptr = gimple_assign_rhs1 (stmt);
else if (is_gimple_assign (stmt)
&& gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
&& host_integerp (gimple_assign_rhs2 (stmt), 0)
- && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
+ && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
{
ptr = gimple_assign_rhs1 (stmt);
- extra_offset = BITS_PER_UNIT * t1;
+ extra_offset = BITS_PER_UNIT * t;
}
}
if (TREE_CODE (ptr) == ADDR_EXPR)
-ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
-&ref->offset, &t1, &t2);
+{
+ ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
+ if (ref->base)
+ ref->offset = BITS_PER_UNIT * t;
+ else
+ {
+ size = NULL_TREE;
+ ref->offset = 0;
+ ref->base = get_base_address (TREE_OPERAND (ptr, 0));
+ }
+}
else
{
ref->base = build2 (MEM_REF, char_type_node,
ptr, null_pointer_node);
ref->offset = 0;
}
ref->offset += extra_offset;
if (size
&& host_integerp (size, 0)
&& TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
Re: [PATCH] Fix ipcp_transform_function (PR tree-optimization/58984)
On Mon, 4 Nov 2013, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes PR58984, where determine_known_aggregate_parts
> notices that o.f0 = 1 store (because of ESRA) and p.f0 = 1
> store (because the field isn't a bitfield) are known constants
> at offset 0 of the first parameter, but doesn't record the access
> of that size (8 bits) and happily replaces BIT_FIELD_REF
> in the called function with 1 (which is wrong, because the f1 bitfield
> also contains 1). As the size isn't recorded explicitly, this patch
> looks at TYPE_SIZE of the constants we've recorded, if the IL was correct
> then the store to it should be a useless type conversion and thus have the
> same number of bits.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (together with a short
> hack to record how many times this has changed compiler's behavior and the
> only case during bootstrap/regtest was on this testcase on x86_64-linux
> 64-bit).
>
> Ok for trunk/4.8?
Looks ok to me.
Thanks,
Richard.
> 2013-11-04 Jakub Jelinek
>
> PR tree-optimization/58984
> * ipa-prop.c (ipa_load_from_parm_agg_1): Add SIZE_P argument,
> set *SIZE_P if non-NULL on success.
> (ipa_load_from_parm_agg, ipa_analyze_indirect_call_uses): Adjust
> callers.
> (ipcp_transform_function): Likewise. Punt if size of access
> is different from TYPE_SIZE on v->value's type.
>
> * gcc.c-torture/execute/pr58984.c: New test.
>
> --- gcc/ipa-prop.c.jj 2013-11-01 12:41:34.0 +0100
> +++ gcc/ipa-prop.c2013-11-04 15:47:36.373955817 +0100
> @@ -852,7 +852,7 @@ static bool
> ipa_load_from_parm_agg_1 (vec descriptors,
> struct param_analysis_info *parms_ainfo, gimple stmt,
> tree op, int *index_p, HOST_WIDE_INT *offset_p,
> - bool *by_ref_p)
> + HOST_WIDE_INT *size_p, bool *by_ref_p)
> {
>int index;
>HOST_WIDE_INT size, max_size;
> @@ -870,6 +870,8 @@ ipa_load_from_parm_agg_1 (vec {
> *index_p = index;
> *by_ref_p = false;
> + if (size_p)
> + *size_p = size;
> return true;
> }
>return false;
> @@ -912,6 +914,8 @@ ipa_load_from_parm_agg_1 (vec {
>*index_p = index;
>*by_ref_p = true;
> + if (size_p)
> + *size_p = size;
>return true;
> }
>return false;
> @@ -926,7 +930,7 @@ ipa_load_from_parm_agg (struct ipa_node_
> bool *by_ref_p)
> {
>return ipa_load_from_parm_agg_1 (info->descriptors, NULL, stmt, op,
> index_p,
> -offset_p, by_ref_p);
> +offset_p, NULL, by_ref_p);
> }
>
> /* Given that an actual argument is an SSA_NAME (given in NAME) and is a
> result
> @@ -1826,7 +1830,7 @@ ipa_analyze_indirect_call_uses (struct c
>if (gimple_assign_single_p (def)
>&& ipa_load_from_parm_agg_1 (info->descriptors, parms_ainfo, def,
> gimple_assign_rhs1 (def), &index, &offset,
> -&by_ref))
> +NULL, &by_ref))
> {
>struct cgraph_edge *cs = ipa_note_param_call (node, index, call);
>cs->indirect_info->offset = offset;
> @@ -4567,7 +4571,7 @@ ipcp_transform_function (struct cgraph_n
> struct ipa_agg_replacement_value *v;
> gimple stmt = gsi_stmt (gsi);
> tree rhs, val, t;
> - HOST_WIDE_INT offset;
> + HOST_WIDE_INT offset, size;
> int index;
> bool by_ref, vce;
>
> @@ -4594,13 +4598,15 @@ ipcp_transform_function (struct cgraph_n
> continue;
>
> if (!ipa_load_from_parm_agg_1 (descriptors, parms_ainfo, stmt,
> -rhs, &index, &offset, &by_ref))
> +rhs, &index, &offset, &size, &by_ref))
> continue;
> for (v = aggval; v; v = v->next)
> if (v->index == index
> && v->offset == offset)
> break;
> - if (!v || v->by_ref != by_ref)
> + if (!v
> + || v->by_ref != by_ref
> + || tree_low_cst (TYPE_SIZE (TREE_TYPE (v->value)), 0) != size)
> continue;
>
> gcc_checking_assert (is_gimple_ip_invariant (v->value));
> --- gcc/testsuite/gcc.c-torture/execute/pr58984.c.jj 2013-11-04
> 15:49:48.657239937 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr58984.c 2013-11-04
> 16:00:44.876785998 +0100
> @@ -0,0 +1,57 @@
> +/* PR tree-optimization/58984 */
> +
> +struct S { int f0 : 8; int : 6; int f1 : 5; };
> +struct T { char f0; int : 6; int f1 : 5; };
> +
> +int a, *c = &a, e, n, b, m;
> +
> +static int
> +foo (struct S p)
> +{
> + const unsigned short *f[36];
> + for (; e < 2; e++)
> +{
> + const unsigned short **i = &f[0];
> + *c ^= 1;
> + if (p.f1)
> + {
> + *i = 0;
> + return b;
> + }
> +}
> + return 0;
> +}
> +
> +static int
> +bar (struct T p)
> +{
> + const
Re: Aliasing: look through pointer's def stmt
On Mon, 4 Nov 2013, Richard Biener wrote:
Marc Glisse wrote:
On Mon, 4 Nov 2013, Richard Biener wrote:
Well, host_integer_p (, 0) looks correct to me. But ...
Ok, I'll put it back.
Er, actually host_integerp(, 0) can't work, because arithmetic on pointers
is done with unsigned integers and for x86_64 it would return false for
(size_t)-4. So it would need to be host_integerp(, 1) instead. I still
test TREE_CODE == INTEGER_CST in the current patch, let me know if you
want host_integerp(, 1).
Index: gcc/tree-ssa-alias.h
===
--- gcc/tree-ssa-alias.h(revision 204267)
+++ gcc/tree-ssa-alias.h(working copy)
@@ -139,30 +139,30 @@ extern void pt_solution_set_var (struct
extern void dump_pta_stats (FILE *);
extern GTY(()) struct pt_solution ipa_escaped_pt;
/* Return true, if the two ranges [POS1, SIZE1] and [POS2, SIZE2]
overlap. SIZE1 and/or SIZE2 can be (unsigned)-1 in which case
the
range is open-ended. Otherwise return false. */
static inline bool
-ranges_overlap_p (unsigned HOST_WIDE_INT pos1,
- unsigned HOST_WIDE_INT size1,
- unsigned HOST_WIDE_INT pos2,
- unsigned HOST_WIDE_INT size2)
+ranges_overlap_p (HOST_WIDE_INT pos1,
+ HOST_WIDE_INT size1,
+ HOST_WIDE_INT pos2,
+ HOST_WIDE_INT size2)
I think size[12] should still be unsigned (we don't allow negative
sizes but only the special value -1U which we special-case only to
avoid pos + size to wrap)
But all we do with size[12] is compare it to -1 and perform arithmetic
with pos[12]. At least for the second one, we need to cast size to a
signed type so the comparisons make sense (unless we decide to use a
double_int there). So I thought I would do the cast in the argument.
Otherwise, I'll start the function with:
HOST_WIDE_INT ssize1 = (HOST_WIDE_INT)size1;
and replace size1 with ssize1 through the function.
Is the reason of keeping size[12] unsigned for documentation? Or am I
missing a reason why I may be breaking things?
It is mostly documentation indeed.
Ok, then here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu).
2013-11-05 Marc Glisse
gcc/
* tree-ssa-alias.h (ranges_overlap_p): Handle negative offsets.
* tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Likewise.
gcc/testsuite/
* gcc.dg/tree-ssa/alias-26.c: New file.
--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/alias-26.c(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/alias-26.c(working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+void f (const char *c, int *i)
+{
+ *i = 42;
+ __builtin_memcpy (i - 1, c, sizeof (int));
+ if (*i != 42) __builtin_abort();
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+
Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
___
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: gcc/tree-ssa-alias.c
===
--- gcc/tree-ssa-alias.c(revision 204381)
+++ gcc/tree-ssa-alias.c(working copy)
@@ -552,42 +552,42 @@ ao_ref_base_alias_set (ao_ref *ref)
alias_set_type
ao_ref_alias_set (ao_ref *ref)
{
if (ref->ref_alias_set != -1)
return ref->ref_alias_set;
ref->ref_alias_set = get_alias_set (ref->ref);
return ref->ref_alias_set;
}
/* Init an alias-oracle reference representation from a gimple pointer
- PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the
+ PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE then the
size is assumed to be unknown. The access is assumed to be only
to or after of the pointer target, not before it. */
void
ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
{
HOST_WIDE_INT t1, t2, extra_offset = 0;
ref->ref = NULL_TREE;
if (TREE_CODE (ptr) == SSA_NAME)
{
gimple stmt = SSA_NAME_DEF_STMT (ptr);
if (gimple_assign_single_p (stmt)
&& gimple_assign_rhs_code (stmt) == ADDR_EXPR)
ptr = gimple_assign_rhs1 (stmt);
else if (is_gimple_assign (stmt)
&& gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
- && host_integerp (gimple_assign_rhs2 (stmt), 0)
- && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
+ && TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST)
{
ptr = gimple_assign_rhs1 (stmt);
- extra_offset = BITS_PER_UNIT * t1;
+ extra_offset = BITS_PER_UNIT
+
Re: PR 58958: wrong aliasing info
On Tue, Nov 05, 2013 at 11:40:02AM +0100, Marc Glisse wrote: > >Well, you cannot use the size argument unchanged for the null > >return case. You could fallback to get_base_address and -1 size > >in that case. > > Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu. > (I think I'll disable cilk for my future bootstraps: it takes > forever and confuses contrib/compare_tests) Ah, I was wondering why my make check times (admittedly --enable-checking=yes,rtl but I've done that for years) went up drastically between Friday and Monday (from around 40 minutes to 70 minutes or more). That is clearly highly undesirable. Looking at gcc.dg/cilk-plus/cilk-plus.exp it seems every test is run 24 resp. 25 times, that is clearly way too much, sure, we have torture kind of tests, but that is typically 6 or 8 times at most, and it also depends on how expensive the tests are. As the /AN/ tests were already preexisting, I guess we are talking about the 9 new /CK/ dg-do run tests, so first of all, they must be very expensive themselves, times 25: dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus $ALWAYS_CFLAGS " " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O0 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O1 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O0 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O1 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O3 -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -ftree-vectorize -fcilkplus -g $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O0 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O1 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O3 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O0 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O1 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O3 -std=c99 $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -ftree-vectorize -std=c99 -g -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O0 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O2 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " It doesn't make sense to test -g at all levels, just test it for one or two, it doesn't make sense to test say no -O* vs. -O0, that is the same thing, or -O3 vs. -O3 -ftree-vectorize, that is the same thing, testing -std=c99 vs. default is reasonable for compile time tests if they particularly care for some reason, otherwise, just attach -std=c99 to one (doesn't matter which one) variant. So, please change this to at most 7 or so variants, and even then, if some of the test is really expensive, use it as dg-do compile for all variants and dg-do run just for one (say -O2 -ftree-vectorize or -O3 -g). Jakub
Re: libsanitizer merge from upstream r191666
Hello! 2013-11-04 Kostya Serebryany * All source files: Merge from upstream r191666. * merge.sh: Added lsan. * configure.ac (AC_CONFIG_FILES): Added lsan. * Makefile.am (SUBDIRS): Added lsan. * sanitizer_common/Makefile.am (sanitizer_common_files): Added new fles. * asan/Makefile.am (asan_files): Added new files. (libasan_la_LIBADD): Added a dependency on lsan. * lsan/Makefile.am: New file. * asan/Makefile.in: Regenerate. * lsan/Makefile.in: Regenerate. * Makefile.in: Regenerate. * configure: Regenerate. * sanitizer_common/Makefile.in: Regenerate. This patch breaks bootstrap on Centos 5.10 due to missing linux/perf_event.h: ... libtool: compile: /home/uros/gcc-build/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build/./gcc -nostdinc++ -L/home/uros/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:25:30: fatal error: linux/perf_event.h: No such file or directory #include ^ compilation terminated. gmake[4]: *** [sanitizer_platform_limits_linux.lo] Error 1 gmake[4]: *** Waiting for unfinished jobs ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_linux.cc: Assembler messages: ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_linux.cc:823: Error: .cfi_endproc without corresponding .cfi_startproc :22131: Error: open CFI at the end of file; missing .cfi_endproc directive gmake[4]: *** [sanitizer_linux.lo] Error 1 mv -f .deps/sanitizer_linux_libcdep.Tpo .deps/sanitizer_linux_libcdep.Plo mv -f .deps/sanitizer_allocator.Tpo .deps/sanitizer_allocator.Plo gmake[4]: Leaving directory `/home/uros/gcc-build/x86_64-unknown-linux-gnu/libsanitizer/sanitizer_common' gmake[3]: *** [all-recursive] Error 1 gmake[3]: Leaving directory `/home/uros/gcc-build/x86_64-unknown-linux-gnu/libsanitizer' gmake[2]: *** [all-stage1-target-libsanitizer] Error 2 gmake[2]: Leaving directory `/home/uros/gcc-build' gmake[1]: *** [stage1-bubble] Error 2 gmake[1]: Leaving directory `/home/uros/gcc-build' gmake: *** [all] Error 2 Uros.
RE: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals
Hi,
you're welcome.
Just one more thought on the design.
If you want to have at least a chance to survive something like:
dd if=/dev/zero of=test.c bs=10240 count=1000
gcc -Wall test.c
Then you should change the implementation of read_line to
_not_ returning something like 100GB of zeros.
IMHO it would be nice to limit lines returned to 10.000 bytes,
maybe add "..." or "" if the limit is reached.
Just skip over-sized bytes until the newline is consumed, to make
the line numbers consistent.
And maybe it would make the life of read_line's callers lots easier
if the zero-chars are silently replaced with spaces in the returned
line buffer.
That would allow to keep the current interface, and somehow
reduce the complexity of this patch.
What do you think?
Regards
Bernd.
On Tue, 5 Nov 2013 10:41:19, Dodji Seketeli wrote:
>
> Bernd Edlinger writes:
>
> [...]
>
>>> if (!string_len)
>>> {
>>> string_len = 200;
>>> - string = XNEWVEC (char, string_len);
>>> + string = XCNEWVEC (char, string_len);
>>> }
>>> + else
>>> + memset (string, 0, string_len);
>>
>> Is this memset still necessary?
>
> Of course not ...
>
> [...]
>
>> If "ptr" is passed to get_line it will try to reallocate it,
>> which must fail, right?
>>
>> Maybe, this line of code is unreachable?
>>
>> Who is responsible for reallocating "string" get_line or read_line?
>
> Correct, these are real concerns.
>
>
> I am wondering what I was thinking. Actually, I think read_line should
> almost just call get_line now. Like what is done in the new version of
> the patch below; basically if there is a line to return, read_line just
> gets it (the static buffer containing the line) from get_line and
> returns it, otherwise the static buffer containing the last read line is
> left untouched and read_line returns a NULL constant.
>
>
> I guess this resolves the valid concern that you raised below:
>
>> If the previous invocation of read_line already had read some
>> characters of the following line, how is that information
>> recovered? How is it detected if another file is to be read this
>> time?
>
> Thank you very much for this thorough review.
>
> Here is the updated patch that I am bootstrapping:
>
> gcc/ChangeLog:
>
> * input.h (location_get_source_line): Take an additional line_size
> parameter.
> * input.c (get_line): New static function definition.
> (read_line): Take an additional line_length output parameter to be
> set to the size of the line. Use the new get_line function do the
> actual line reading.
> (location_get_source_line): Take an additional output line_len
> parameter. Update the use of read_line to pass it the line_len
> parameter.
> * diagnostic.c (adjust_line): Take an additional input parameter
> for the length of the line, rather than calculating it with
> strlen.
> (diagnostic_show_locus): Adjust the use of
> location_get_source_line and adjust_line with respect to their new
> signature. While displaying a line now, do not stop at the first
> null byte. Rather, display the zero byte as a space and keep
> going until we reach the size of the line.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/cpp/warning-zero-in-literals-1.c: New test file.
> ---
> gcc/diagnostic.c | 17 ++--
> gcc/input.c | 111 -
> gcc/input.h | 3 +-
> .../c-c++-common/cpp/warning-zero-in-literals-1.c | Bin 0 -> 240 bytes
> 4 files changed, 97 insertions(+), 34 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/cpp/warning-zero-in-literals-1.c
>
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 36094a1..e0c5d9d 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -259,12 +259,13 @@ diagnostic_build_prefix (diagnostic_context *context,
> MAX_WIDTH by some margin, then adjust the start of the line such
> that the COLUMN is smaller than MAX_WIDTH minus the margin. The
> margin is either 10 characters or the difference between the column
> - and the length of the line, whatever is smaller. */
> + and the length of the line, whatever is smaller. The length of
> + LINE is given by LINE_WIDTH. */
> static const char *
> -adjust_line (const char *line, int max_width, int *column_p)
> +adjust_line (const char *line, int line_width,
> + int max_width, int *column_p)
> {
> int right_margin = 10;
> - int line_width = strlen (line);
> int column = *column_p;
>
> right_margin = MIN (line_width - column, right_margin);
> @@ -284,6 +285,7 @@ diagnostic_show_locus (diagnostic_context * context,
> const diagnostic_info *diagnostic)
> {
> const char *line;
> + int line_width;
> char *buffer;
> expanded_location s;
> int max_width;
> @@ -297,22 +299,25 @@ diagnostic_show_locus (diagnostic_context * context,
>
> context->last_location = diagnostic->location;
> s = expand_location_to_spelling_point (diagnostic->location);
> - line = location_get_source_line (s);
> + line = location_get_source_line (s, &line_width);
> if (line == NULL)
> return;
>
> max_width = context->caret_max_width;
> - line = adjust_line (line, max_width,
Re: [PATCH GCC]Simplify address expression in IVOPT
On 11/05/13 10:13, bin.cheng wrote:
Index: gcc/tree-affine.c
===
--- gcc/tree-affine.c (revision 204117)
+++ gcc/tree-affine.c (working copy)
@@ -874,10 +874,11 @@ debug_aff (aff_tree *val)
fprintf (stderr, "\n");
}
-/* Returns address of the reference REF in ADDR. The size of the accessed
- location is stored to SIZE. */
+/* Computes address of the reference REF in ADDR. The size of the accessed
+ location is stored to SIZE. Returns pointer to the ultimate containing
+ object to which REF refers. */
-void
+tree
get_inner_reference_aff (tree ref, aff_tree *addr, double_int *size)
{
HOST_WIDE_INT bitsize, bitpos;
@@ -904,6 +905,8 @@ get_inner_reference_aff (tree ref, aff_tree *addr,
aff_combination_add (addr,&tmp);
*size = double_int::from_shwi ((bitsize + BITS_PER_UNIT - 1) /
BITS_PER_UNIT);
+
+ return base_addr;
}
I think what Richard suggests is to return the base object rather the
address of the base object, i.e.
return base;
This is good in keeping the consistency in the return values between
get_inner_reference_aff and get_inner_reference.
Yufeng
Re: Testsuite / Cilk Plus: Include library path in compile flags in gcc.dg/cilk-plus/cilk-plus.exp
Hi!
On Mon, Nov 04, 2013 at 11:02:13PM +, Iyer, Balaji V wrote:
See my other mail on this topic, the number of different option combinations
is just excessive and must be reduced to a reasonable minimum.
> 2). The C tests are extremely time-consuming (m32/m64) with NO -jxx on the
> c/l:
>
> x86_64-linux (32 cores 2.8G Xeon) : 36m/421m/6m
> x86_64-darwin12 (8 cores 2.8G Xeon) : 45m/309m/2m.
>
> c.f. the g++ tests which take ~2m wall clock.
>
> BVI: yes, I know about this. It is because some of the Cilk tests are run for
> higher iterations to make it force a steal. This is one of the main place
> were runtime bugs could occur. My original solution was to put them all in
> separate directories with separate scripts to run them, but someone in GCC
> community did not like that idea. This way, when people compile with -jN,
> they will be run in parallel. If that is a good idea, I can look into
> reorganizing them.
>
> this might be nothing to do with cilk, but it certainly warrants
> investigation.
Tests that many seconds or more on fast machines, especially if you run them
25 times, are simply not appropriate for gcc testsuite, at least not by
default. We have run_expensive_tests (from GCC_RUN_EXPENSIVE_TESTS in
environment), which can be used say for:
/* { dg-additional-options "-DEXPENSIVE" { target run_expensive_tests } } */
or
// { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS=1" { target { !
run_expensive_tests } } }
or
/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O2" } } */
(just grep for it), but still it shouldn't be prohibitively slow.
Remember what is slow on fast machines might turn into days on really slow
machines. Say, if all you are looking for is look for library
synchronization issues, I guess all optimization levels still result in
similar pattern of library calls, so even for run_expensive_tests you could
use higher iteration count for a single optimization level (say -O2)
and for all others just use smaller iteration count.
Jakub
Re: libsanitizer merge from upstream r191666
It is a different failure. There are 3 problems: 1. Hard coded 64-bit registers for pointers in x86-64 asm statements. 2. Treat WORD_SIZE == 32 as ia32 for machine state on x86-64. 3. On x86-64, assume ia32 FS system calls when LP64 is false. H.J. On Mon, Nov 4, 2013 at 10:35 PM, Konstantin Serebryany wrote: > Is this the same failure or different? > > On Mon, Nov 4, 2013 at 9:49 PM, H.J. Lu wrote: >> It also breaks x32 build. >> >> >> On Mon, Nov 4, 2013 at 5:48 PM, Konstantin Serebryany >> wrote: >>> Hi Peter. >>> Does this also mean that asan in llvm trunk is broken for Power? >>> We'll need to fix it there too (or, in fact, first). >>> >>> --kcc >>> >>> On Mon, Nov 4, 2013 at 4:33 PM, Peter Bergner wrote: On Mon, 2013-11-04 at 06:47 -0800, Konstantin Serebryany wrote: > This patch has not been tested on Mac and we generally do not claim > that gcc-asan is supported on Mac. > clang-asan *is* supported on Mac and our bots are green (this patch is > a merge of the sources which are regularly tested on Mac, > but the build procedure is different). The merge from upstream r191666 seems to have broken my powerpc64-linux build. I'm seeing: In file included from /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:21:0: /usr/include/asm/stat.h:31:2: error: ‘ino_t’ does not name a type ino_t st_ino; ^ /usr/include/asm/stat.h:33:2: error: ‘nlink_t’ does not name a type nlink_t st_nlink; ^ /usr/include/asm/stat.h:34:2: error: ‘mode_t’ does not name a type mode_t st_mode; ^ /usr/include/asm/stat.h:39:2: error: ‘uid_t’ does not name a type uid_t st_uid; ^ /usr/include/asm/stat.h:40:2: error: ‘gid_t’ does not name a type gid_t st_gid; ^ /usr/include/asm/stat.h:42:2: error: ‘off_t’ does not name a type off_t st_size; ^ /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:29:73: error: invalid application of ‘sizeof’ to incomplete type ‘__sanitizer::__old_kernel_stat’ unsigned struct___old_kernel_stat_sz = sizeof(struct __old_kernel_stat); ^ The problem seems to be that the RHEL6 system I am on, the linux/types.h kernel header file doesn't define ino_t like it does on newer systems which leads to the undefined type errors. Digging through the other kernel header files, I'm not really seeing another header file I can include to get it either. :( Peter >> >> >> >> -- >> H.J. -- H.J.
Re: Testsuite / Cilk Plus: Include library path in compile flags in gcc.dg/cilk-plus/cilk-plus.exp
On Tue, Nov 05, 2013 at 12:21:04PM +0100, Jakub Jelinek wrote:
> Tests that many seconds or more on fast machines, especially if you run them
> 25 times, are simply not appropriate for gcc testsuite, at least not by
> default. We have run_expensive_tests (from GCC_RUN_EXPENSIVE_TESTS in
> environment), which can be used say for:
> /* { dg-additional-options "-DEXPENSIVE" { target run_expensive_tests } } */
> or
> // { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS=1" { target { !
> run_expensive_tests } } }
> or
> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O2" } } */
> (just grep for it), but still it shouldn't be prohibitively slow.
> Remember what is slow on fast machines might turn into days on really slow
> machines. Say, if all you are looking for is look for library
> synchronization issues, I guess all optimization levels still result in
> similar pattern of library calls, so even for run_expensive_tests you could
> use higher iteration count for a single optimization level (say -O2)
> and for all others just use smaller iteration count.
Oh, another thing, runtime tests should abort () or __builtin_abort () on
failure, rather then just exit with non-zero status.
Also, I wonder about the fib_* tests, fib_serial calls fib rather than
fib_serial, so effectively the only difference between calling fib_serial
and fib is just the outermost iteration. Also, are you really sure you have
to call fib with all values from 0 up to 40? Isn't it enough to just call
fib (40) once and compare that with precomputed fib (40) number?
Because, when a single test takes about 2 minutes to run, it is really too
much, and very much unnecessarily so.
Jakub
Re: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals
Bernd Edlinger writes: > If you want to have at least a chance to survive something like: > > > dd if=/dev/zero of=test.c bs=10240 count=1000 > > gcc -Wall test.c > > > Then you should change the implementation of read_line to > _not_ returning something like 100GB of zeros. I'd say that in that case, we'd rather just die in an OOM condition and be done with it. Otherwise, If fear that read_line might become too slow; you'd have to detect that the content is just zeros, for instance. > IMHO it would be nice to limit lines returned to 10.000 bytes, > maybe add "..." or "" if the limit is reached. In general, setting a limit for pathological cases like this is a good idea, I think. But that seems a bit ouf of the scope of this particular bug fix; we'd need to e.g, define a new command line argument to extend that limit if need be, for instance. If people really feel strongly about this I can propose a later patch to set a limit in get_line and define a command like argument that would override that parameter. > And maybe it would make the life of read_line's callers lots easier > if the zero-chars are silently replaced with spaces in the returned > line buffer. As speed seemed to be a concern (even if, in my opinion, we are dealing with diagnostics that are being emitted when the compilation has been halted anyway, so we shouldn't be too concerned, unless we are talking about pathological cases), I think that read_line should be fast by default. If a particular caller doesn't want to see the zeros (and thus is ready to pay the speed price) then it can replace the zeros with white space. Otherwise, let's have read_line be as fast as possible. Also keep in mind that in subsequent patches, read_line might be re-used by e.g, gcov in nominal contexts where we don't have zeros in the middle of the line. In that case, speed can be a concern. Thanks for the helpful thoughts. -- Dodji
Re: Remove SHIFT_COUNT_TRUNCATED from double-int.c
On Mon, Nov 4, 2013 at 9:36 PM, Richard Sandiford
wrote:
> ...as suggested by Richard. This means that out-of-range shift values
> produce the same results on all targets at the tree level. The rtl level
> isn't affected since it explicitly truncates the count.
>
> Tested on mips64-linux-gnu. OK to install?
Ok (assuming you didn't pick TARGET_LOONGSON_VECTORS).
Thanks,
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * double-int.c (lshift_double, rshift_double): Remove
> SHIFT_COUNT_TRUNCATED handling.
>
> Index: gcc/double-int.c
> ===
> --- gcc/double-int.c2013-09-28 09:41:49.490113799 +0100
> +++ gcc/double-int.c2013-11-04 15:49:56.963164424 +
> @@ -20,7 +20,7 @@ Free Software Foundation; either version
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
> -#include "tm.h"/* For SHIFT_COUNT_TRUNCATED. */
> +#include "tm.h"/* For BITS_PER_UNIT and
> *_BIG_ENDIAN. */
> #include "tree.h"
>
> static int add_double_with_sign (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
> @@ -237,9 +237,6 @@ rshift_double (unsigned HOST_WIDE_INT l1
> ? -((unsigned HOST_WIDE_INT) h1 >> (HOST_BITS_PER_WIDE_INT - 1))
> : 0);
>
> - if (SHIFT_COUNT_TRUNCATED)
> -count %= prec;
> -
>if (count >= HOST_BITS_PER_DOUBLE_INT)
> {
>/* Shifting by the host word size is undefined according to the
> @@ -295,9 +292,6 @@ lshift_double (unsigned HOST_WIDE_INT l1
> {
>unsigned HOST_WIDE_INT signmask;
>
> - if (SHIFT_COUNT_TRUNCATED)
> -count %= prec;
> -
>if (count >= HOST_BITS_PER_DOUBLE_INT)
> {
>/* Shifting by the host word size is undefined according to the
Re: [PATCH GCC]Simplify address expression in IVOPT
On Tue, Nov 5, 2013 at 7:19 PM, Yufeng Zhang wrote:
> On 11/05/13 10:13, bin.cheng wrote:
>>
>> Index: gcc/tree-affine.c
>> ===
>> --- gcc/tree-affine.c (revision 204117)
>> +++ gcc/tree-affine.c (working copy)
>> @@ -874,10 +874,11 @@ debug_aff (aff_tree *val)
>> fprintf (stderr, "\n");
>> }
>>
>> -/* Returns address of the reference REF in ADDR. The size of the
>> accessed
>> - location is stored to SIZE. */
>> +/* Computes address of the reference REF in ADDR. The size of the
>> accessed
>> + location is stored to SIZE. Returns pointer to the ultimate
>> containing
>> + object to which REF refers. */
>>
>> -void
>> +tree
>> get_inner_reference_aff (tree ref, aff_tree *addr, double_int *size)
>> {
>> HOST_WIDE_INT bitsize, bitpos;
>> @@ -904,6 +905,8 @@ get_inner_reference_aff (tree ref, aff_tree *addr,
>> aff_combination_add (addr,&tmp);
>>
>> *size = double_int::from_shwi ((bitsize + BITS_PER_UNIT - 1) /
>> BITS_PER_UNIT);
>> +
>> + return base_addr;
>> }
>>
>
> I think what Richard suggests is to return the base object rather the
> address of the base object, i.e.
I am not sure about that. We have to pass pointer_type expression to
function determine_base_object for address expressions, because there
is no way to tell pointer from object once we are in
determine_base_object.
Thanks.
bin
>
> return base;
>
> This is good in keeping the consistency in the return values between
> get_inner_reference_aff and get_inner_reference.
>
> Yufeng
>
--
Best Regards.
[PATCH, SH] Cleanup/simplify str builtins
Hello,
This patch is just for code simplification, Main changes are that I now
use adjust_address instead of adjust_automodify_address and the prob
variables are globalized to ease tuning and avoid duplication.
No regression for sh-none-elf. Prerequisite for upcoming builtin_strlen
patch.
Many thanks
Christian
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 204346)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2013-11-05 Christian Bruel
+
+ * gcc/config/sh/sh-mem.cc (sh_expand_cmpnstr, sh_expand_cmpstr):
+ Factorize probabilities, Use adjust_address instead of
+ adjust_automodify_address when possible. Enable for optimize.
+
2013-11-04 Richard Sandiford
* config/avr/avr-log.c (avr_double_int_pop_digit): Delete.
Index: gcc/config/sh/sh-mem.cc
===
--- gcc/config/sh/sh-mem.cc (revision 204346)
+++ gcc/config/sh/sh-mem.cc (working copy)
@@ -179,32 +179,31 @@ expand_block_move (rtx *operands)
return false;
}
+static int prob_unlikely = REG_BR_PROB_BASE / 10;
+static int prob_likely = REG_BR_PROB_BASE / 4;
+
/* Emit code to perform a strcmp.
OPERANDS[0] is the destination.
OPERANDS[1] is the first string.
OPERANDS[2] is the second string.
- OPERANDS[3] is the align. */
+ OPERANDS[3] is the known alignment. */
bool
sh_expand_cmpstr (rtx *operands)
{
- rtx s1 = copy_rtx (operands[1]);
- rtx s2 = copy_rtx (operands[2]);
- rtx s1_addr = copy_addr_to_reg (XEXP (s1, 0));
- rtx s2_addr = copy_addr_to_reg (XEXP (s2, 0));
+ rtx addr1 = operands[1];
+ rtx addr2 = operands[2];
+ rtx s1_addr = copy_addr_to_reg (XEXP (addr1, 0));
+ rtx s2_addr = copy_addr_to_reg (XEXP (addr2, 0));
rtx tmp0 = gen_reg_rtx (SImode);
rtx tmp1 = gen_reg_rtx (SImode);
rtx tmp2 = gen_reg_rtx (SImode);
rtx tmp3 = gen_reg_rtx (SImode);
+ rtx jump;
rtx L_return = gen_label_rtx ();
rtx L_loop_byte = gen_label_rtx ();
rtx L_end_loop_byte = gen_label_rtx ();
-
- rtx jump, addr1, addr2;
- int prob_unlikely = REG_BR_PROB_BASE / 10;
- int prob_likely = REG_BR_PROB_BASE / 4;
-
rtx L_loop_long = gen_label_rtx ();
rtx L_end_loop_long = gen_label_rtx ();
@@ -220,8 +219,8 @@ sh_expand_cmpstr (rtx *operands)
add_int_reg_note (jump, REG_BR_PROB, prob_likely);
}
- addr1 = adjust_automodify_address (s1, SImode, s1_addr, 0);
- addr2 = adjust_automodify_address (s2, SImode, s2_addr, 0);
+ addr1 = adjust_automodify_address (addr1, SImode, s1_addr, 0);
+ addr2 = adjust_automodify_address (addr2, SImode, s2_addr, 0);
/* tmp2 is aligned, OK to load. */
emit_move_insn (tmp3, addr2);
@@ -276,8 +275,8 @@ sh_expand_cmpstr (rtx *operands)
emit_move_insn (s2_addr, plus_constant (Pmode, s2_addr, -4));
/* start byte loop. */
- addr1 = adjust_automodify_address (s1, QImode, s1_addr, 0);
- addr2 = adjust_automodify_address (s2, QImode, s2_addr, 0);
+ addr1 = adjust_address (addr1, QImode, 0);
+ addr2 = adjust_address (addr2, QImode, 0);
emit_label (L_loop_byte);
@@ -317,27 +316,23 @@ sh_expand_cmpstr (rtx *operands)
OPERANDS[1] is the first string.
OPERANDS[2] is the second string.
OPERANDS[3] is the length.
- OPERANDS[4] is the align. */
+ OPERANDS[4] is the known alignment. */
bool
sh_expand_cmpnstr (rtx *operands)
{
- rtx s1 = copy_rtx (operands[1]);
- rtx s2 = copy_rtx (operands[2]);
-
- rtx s1_addr = copy_addr_to_reg (XEXP (s1, 0));
- rtx s2_addr = copy_addr_to_reg (XEXP (s2, 0));
+ rtx addr1 = operands[1];
+ rtx addr2 = operands[2];
+ rtx s1_addr = copy_addr_to_reg (XEXP (addr1, 0));
+ rtx s2_addr = copy_addr_to_reg (XEXP (addr2, 0));
rtx tmp0 = gen_reg_rtx (SImode);
rtx tmp1 = gen_reg_rtx (SImode);
rtx tmp2 = gen_reg_rtx (SImode);
+ rtx jump;
rtx L_return = gen_label_rtx ();
rtx L_loop_byte = gen_label_rtx ();
rtx L_end_loop_byte = gen_label_rtx ();
- rtx jump, addr1, addr2;
- int prob_unlikely = REG_BR_PROB_BASE / 10;
- int prob_likely = REG_BR_PROB_BASE / 4;
-
rtx len = force_reg (SImode, operands[3]);
int constp = (CONST_INT_P (operands[3]));
int bytes = (constp ? INTVAL (operands[3]) : 0);
@@ -366,10 +361,10 @@ sh_expand_cmpnstr (rtx *operands)
add_int_reg_note (jump, REG_BR_PROB, prob_likely);
}
- addr1 = adjust_automodify_address (s1, SImode, s1_addr, 0);
- addr2 = adjust_automodify_address (s2, SImode, s2_addr, 0);
+ addr1 = adjust_automodify_address (addr1, SImode, s1_addr, 0);
+ addr2 = adjust_automodify_address (addr2, SImode, s2_addr, 0);
- /* words count. */
+ /* word count. Do we have iterations ? */
emit_insn (gen_lshrsi3 (lenw, len, GEN_INT (2)));
/*start long loop. */
@@ -429,48 +424,48 @@ sh_expand_cmpnstr (rtx *operands)
emit_move_insn (s2_addr, plus_constant (Pmode, s2_addr, -4));
}
-addr1 = adjust_automodify_address (s1, QImode, s1_addr, 0);
-
Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)
On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm wrote:
> On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
>> On 11/01/2013 06:58 PM, David Malcolm wrote:
>> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
>> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
>> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
>> >static inline void
>> > ! gimple_call_set_lhs (gimple gs, tree lhs)
>> >{
>> > - GIMPLE_CHECK (gs, GIMPLE_CALL);
>> >> The checking you are removing here.
>> >>
>> >>> What checking? There ought to be no checking at all in this
>> >>> example... gimple_build_call_vec returns a gimple_call, and
>> >>> gimple_call_set_lhs() doesn't have to check anything because it
>> >>> only accepts gimple_call's.. so there is no checking other than the
>> >>> usual "does my parameter match" that the compiler has to do...
>> >> and want to replace it by checking of the types at compile time.
>> >> The problem is that it uglifies the source too much, and, when you
>> >> actually don't have a gimple_call but supposedly a base class of it,
>> >> I expect you'd do as_a which is not only further uglification, but has
>> >> runtime cost also for --enable-checking=release.
>> > I can have a look next week at every call to gimple_call_set_lhs in the
>> > tree, and see to what extent we know at compile-time that the initial
>> > arg is indeed a call (of the ones I quickly grepped just now, most are
>> > from gimple_build_call and friends, but one was from a gimple_copy).
>> >
>> > FWIW I did some performance testing of the is_a/as_a code in the earlier
>> > version of the patch, and it didn't have a noticable runtime cost
>> > compared to the GIMPLE_CHECK in the existing code:
>> > Size of compiler executable:
>> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
>> > Compile times:
>> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
>> I actually really dislike as_a<> and is_a<>, and think code needs to be
>> restructured rather than use them, other than possibly at the very
>> bottom level when we're allocating memory or something like that, or
>> some kind of emergency :-)... If we require frequent uses of those,
>> I'd be against it, I find them quite ugly.
>>
>> Like I said in the other reply, no rush, I don't think any of this
>> follow up is appropriate this late in stage 1. It would be more of an
>> "interest" examination right now.. at least in my opinion... I suspect
>> thinks like gimple_assign are more complex cases, but without looking
>> its hard to tell for sure.
>
> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> than a gimple, and excitingly, it was easiest to also convert
> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> gimple.
Does that work (using gimple_call * objects) for our garbage collector?
That is, does it know it is looking at a 'gimple'? It doesn't matter for this
case as all stmts are reachable from struct function as sequence of 'gimple',
but in general?
Richard.
> Am attaching a patch (on top of the patch series being discussed) which
> adds this compile-time typesafety; bootstrap is in-progress. IMHO very
> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
> no use of is_a).
>
> I'm also attaching a followup patch which eliminates gimple_call_set_lhs
> in favor of a method of gimple_statement_call.
>
Re: [RFA][PATCH] Isolate erroneous paths optimization
On Tue, Nov 5, 2013 at 2:57 AM, Jeff Law wrote:
> On 11/04/13 06:19, Richard Biener wrote:
>>
>> On Thu, Oct 31, 2013 at 7:11 AM, Jeff Law wrote:
>>>
>>>
>>> I've incorporated the various suggestions from Marc and Richi, except for
>>> Richi's to integrate this into jump threading.
>>>
>>> I've also made the following changes since the last version:
>>>
>>>1. Added more testcases.
>>>
>>>2. Use infer_nonnull_range, moving it from tree-vrp.c
>>>into gimple.c. Minor improvements to infer_nonnull_range
>>>to make it handle more cases we care about and avoid using
>>>unnecessary routines from tree-ssa.c (which can now be removed)
>>>
>>>3. Multiple undefined statements in a block are handled in the
>>>logical way.
>>>
>>> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. OK for
>>> the
>>> trunk?
>>
>>
>> Comments inline
>
> Thanks, always appreciated.
>
>
>>> index deeb3f2..6db9f56 100644
>>> --- a/gcc/common.opt
>>> +++ b/gcc/common.opt
>>> @@ -2104,6 +2104,12 @@ foptimize-strlen
>>> Common Report Var(flag_optimize_strlen) Optimization
>>> Enable string length optimizations on trees
>>>
>>> +fisolate-erroneous-paths
>>> +Common Report Var(flag_isolate_erroneous_paths) Init(1) Optimization
>>
>>
>> Drop Init(1) (see below)
>
> Probably a cut-n-paste. As I've mentioned in another thread, the option
> stuff is a huge black box that I haven't really looked at. I'm pretty sure I
> just took something and hacked it. Fixed.
>
>
>
>
>>> +
>>> + /* If we did not run to the end of DUPLICATE, then SI points to STMT
>>> and
>>> + SI2 points to the duplicate of STMT in DUPLICATE. */
>>> + if (!gsi_end_p (si2))
>>> +{
>>> + /* SI2 is the iterator in the duplicate block and it now points
>>> +to our victim statement. */
>>> + gimple_seq seq = NULL;
>>> + gimple stmt
>>> + = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0);
>>> + gimple_seq_add_stmt (&seq, stmt);
>>> + gsi_insert_before (&si2, seq, GSI_SAME_STMT);
>>> + /* Now delete all remaining statements in this block. */
>>> + for (; !gsi_end_p (si2);)
>>> + gsi_remove (&si2, true);
>>
>>
>> Please do
>>
>> stmt = gsi_stmt (si2);
>> unlink_stmt_vdef (stmt);
>> gsi_remove (&si2, true);
>> release_defs (stmt);
>>
>> to "completely" remove the stmts correctly (you've left SSA names
>> unreleased and virtual SSA form broken).
>
> I had to think about this for a while -- we have to be a bit careful here
> because of the limitations of the name manager. But I think this case is
> OK. Specifically any SSA_NAMEs we release here won't have any dangling
> references due to unreachable blocks. Thus we don't run afoul of the
> limitations of the name manager (yes, that problem is still on my todo list
> to fix up).
>
>
>
>>> + next_i = i + 1;
>>> + if (integer_zerop (op))
>>> + {
>>
>>
>> I always prefer
>>
>> if (!integer_zerop (op))
>> continue;
>>
>> to reduce indentation of following code (but that's personal
>> preference).
>
> No strong opinions here. Changed per your request.
>
>
>
>>> + FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
>>> + {
>>> + /* We only care about uses in BB which are
>>> assignements
>>> +with memory operands.
>>> +
>>> +We could see if any uses are as function
>>> arguments
>>> +when the callee has marked the argument as being
>>> +non-null. */
>>> + if (gimple_bb (use_stmt) != bb
>>> + || (!is_gimple_assign (use_stmt)
>>> + && !is_gimple_call (use_stmt)
>>> + && gimple_code (use_stmt) !=
>>> GIMPLE_RETURN))
>>
>>
>> any reason for this restrictions on use_stmt?
>
> Historical when this used to open-code the check for NULL pointer
> dereferences. infer_nonnull_range should do the right thing. Redundant
> checks bits removed, comment updated.
>
>
>
>
>
>
>
>>> +
>>> + /* Now look at the statements in the block and see if any of
>>> +them explicitly dereference a NULL pointer. Believe it or
>>> +not, this does happen from time to time. */
>>
>>
>> "happens because of constant propagation."
>
> "because of jump threading and constant propagation" I went through the
> trouble of tracking this down a little while ago to ensure this code was
> still useful and didn't update the comment. FWIW, the included tests show
> instances where we can get explicit dereferences of NULL due to jump
> threading + constant propagation.
>
>
>
>
>
>
>>
>>> + for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
>>> + {
>>> + gimple stmt = gsi_stmt (si);
>>> +
>>> +
>>
>>
>> extra vertical space
>
> Fixed.
>
>
>>
>>> + /* By passing null_point
[C++ Patch Ping] PR 58724
Hi, http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01166.html Thanks! Paolo.
Re: Rework doloop interface
On Tue, Nov 5, 2013 at 10:33 AM, Richard Sandiford
wrote:
> Following on from:
>
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00092.html
>
> it looks like I'll have to "fix" the doloop_begin/end interface after all.
> The current code goes on to do some rtx arithmetic on invalid CONST_INTs.
> (FWIW, the arithmetic seems unnecessary and I think we could simply drop it,
> but the weirdness of the interface would remain.)
>
> So: doloop_end has 6 parameters, of which 4 are really just information
> about the loop. These 4 parameters never end up in the instruction
> stream on current targets and aren't IMO really rtxes. But because
> expanders can only take rtx arguments, we still need to wrap them in
> some kind of rtx container.
>
> This causes problems because once you start representing something as
> an rtx, you need to start thinking about modes. One of the parameters
> is the maximum number of iterations, and it isn't obvious what its mode
> should be; the mode of the loop counter is too small to hold an iteration
> count of 1 << GET_MODE_PRECISION. Another of the parameters is the loop
> level, which has no obvious mode at all. One is a boolean, which could
> be represented as BImode, but then we enter STORE_FLAG_VALUE territory.
>
> So this patch adds a new target hook, can_use_doloop_p, that takes
> these parameters as normal C arguments. The default returns true,
> but there's also a canned alternative for targets that can only handle
> innermost loops.
>
> This should also be slightly more efficient, since we can avoid creating
> the loop counter register and label if the loop "obviously" isn't suitable.
> It also avoids creating garbage rtl for the 4 not-really-rtx parameters.
>
> Tested by building C and C++ for:
>
> arc-elf
> arm-linux-gnueabi
> bfin-elf
> c6x-elf
> ia64-linux-gnu
> mep-elf
> powerpc-linux-gnu
> s390-linux-gnu
> sh-linux-gnu
> spu-elf
> tilegx-elf
> tilepro-elf
> v850-elf
>
> and comparing the -O2 assembly output of gcc.c-torture, gcc.dg and g++.dg
> from before and after the patch (as far as possible without target headers).
> For this I moved the can_use_doloop_p test to just before the gen_doloop_end
> so that register and label numbers stayed the same. There were two
> differences:
>
> - gcc.c-torture/compile/pr44030.c for arc-elf, where we now use the doloop
> instruction and didn't previously. This is because arc-elf checked:
>
> /* Setting up the loop with two sr isntructions costs 6 cycles. */
> if (TARGET_ARC700 && !INTVAL (operands[5])
> && INTVAL (operands[1]) && INTVAL (operands[1]) <= (flag_pic ? 6 : 3))
> FAIL;
>
> where operands[1] is the constant number of iterations. Unlike the
> maximum number of iterations (operands[2]), this value has the same
> mode as the counter register and is properly sign-extended. So a large
> positive iteration count appears negative and unintentionally triggered
> the FAIL. So I think this counts as a bug fix.
>
> - gcc.dg/graphite/id-9.c for bfin-elf. In this testcase the maximum number
> of iterations is calculated as the double_int { high = 0, low = -1 },
> which doesn't fit in a signed HWI, so the maximum number of iterations
> was previously treated as 0. And the bfin code doesn't handle the
> 0/unknown case, which might be a bug:
>
> /* Due to limitations in the hardware (an initial loop count of 0
>does not loop 2^32 times) we must avoid to generate a hardware
>loops when we cannot rule out this case. */
> if (!flag_unsafe_loop_optimizations
> && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) >= 0x)
> FAIL;
>
> So the old code allowed the doloop to be used here, which I don't
> think was intentional. Now the bfin code sees the original double_int
> and rejects it as too big.
>
> That said, it might also be a bug that we have { 0, -1 } rather than
> { 0, 0x } or { 0, 0x1 } (or even { -1, -1 }) for an
> SImode loop, but this has got to stop somewhere...
>
> Also bootstrapped & regression-tested on powerpc64-linux-gnu. OK to install?
Ok. Please leave target maintainers a second to comment.
Thanks,
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * target.def (can_use_doloop_p): New hook.
> * doc/tm.texi.in (TARGET_CAN_USE_DOLOOP_P): Add.
> * doc/tm.texi: Regenerate.
> * doc/md.texi (doloop_begin, doloop_end): Update documentation.
> * hooks.h (hook_bool_dint_dint_uint_true): Declare.
> * hooks.c (hook_bool_dint_dint_uint_true): New function.
> * targhooks.h (can_use_doloop_if_innermost): Declare.
> * targhooks.c (can_use_doloop_if_innermost): New function.
> * target.h: Include double-int.h.
> * loop-doloop.c (doloop_optimize): Call targetm.can_use_doloop_p.
> Remove iteration count, maximum iteration count
[SH] Split addi,subdi,negdi before reload
Hello,
It seems that splitting multi-word insns before reload results in
slightly better code on average (according to CSiBE). The attached
patch implements that.
Tested on rev. 204263 with
make -k -j4 check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,-m4-single/
-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
and no new failures. However, as far as I recall there have been some
issues with this in the past, I just can't find/remember what it was
exactly. Kaz, could you please run it through your test setup, too and
let me know if it's OK for trunk?
Thanks,
Oleg
gcc/ChangeLog:
* config/sh/sh.md (adddi3): Remove empty constraints. Remove
can_create_pseudo_p and arith_reg_operand check.
(adddi3_compact, subdi3_compact, *negdi2): Remove constraints,
split before reload.
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 204263)
+++ gcc/config/sh/sh.md (working copy)
@@ -1747,15 +1747,13 @@
;; -
(define_expand "adddi3"
- [(set (match_operand:DI 0 "arith_reg_operand" "")
- (plus:DI (match_operand:DI 1 "arith_reg_operand" "")
- (match_operand:DI 2 "arith_operand" "")))]
+ [(set (match_operand:DI 0 "arith_reg_operand")
+ (plus:DI (match_operand:DI 1 "arith_reg_operand")
+ (match_operand:DI 2 "arith_operand")))]
""
{
if (TARGET_SH1)
{
- if (!can_create_pseudo_p () && ! arith_reg_operand (operands[2], DImode))
- FAIL;
operands[2] = force_reg (DImode, operands[2]);
emit_insn (gen_adddi3_compact (operands[0], operands[1], operands[2]));
DONE;
@@ -1794,22 +1792,22 @@
(set_attr "highpart" "ignore")])
(define_insn_and_split "adddi3_compact"
- [(set (match_operand:DI 0 "arith_reg_dest" "=&r")
- (plus:DI (match_operand:DI 1 "arith_reg_operand" "%0")
- (match_operand:DI 2 "arith_reg_operand" "r")))
+ [(set (match_operand:DI 0 "arith_reg_dest")
+ (plus:DI (match_operand:DI 1 "arith_reg_operand")
+ (match_operand:DI 2 "arith_reg_operand")))
(clobber (reg:SI T_REG))]
"TARGET_SH1"
"#"
- "&& reload_completed"
+ "&& can_create_pseudo_p ()"
[(const_int 0)]
{
- rtx high0 = gen_highpart (SImode, operands[0]);
- rtx high2 = gen_highpart (SImode, operands[2]);
- rtx low0 = gen_lowpart (SImode, operands[0]);
-
emit_insn (gen_clrt ());
- emit_insn (gen_addc (low0, low0, gen_lowpart (SImode, operands[2])));
- emit_insn (gen_addc (high0, high0, high2));
+ emit_insn (gen_addc (gen_lowpart (SImode, operands[0]),
+ gen_lowpart (SImode, operands[1]),
+ gen_lowpart (SImode, operands[2])));
+ emit_insn (gen_addc (gen_highpart (SImode, operands[0]),
+ gen_highpart (SImode, operands[1]),
+ gen_highpart (SImode, operands[2])));
DONE;
})
@@ -2108,22 +2106,22 @@
(set_attr "highpart" "ignore")])
(define_insn_and_split "subdi3_compact"
- [(set (match_operand:DI 0 "arith_reg_dest" "=&r")
- (minus:DI (match_operand:DI 1 "arith_reg_operand" "0")
- (match_operand:DI 2 "arith_reg_operand" "r")))
+ [(set (match_operand:DI 0 "arith_reg_dest")
+ (minus:DI (match_operand:DI 1 "arith_reg_operand")
+ (match_operand:DI 2 "arith_reg_operand")))
(clobber (reg:SI T_REG))]
"TARGET_SH1"
"#"
- "&& reload_completed"
+ "&& can_create_pseudo_p ()"
[(const_int 0)]
{
- rtx high0 = gen_highpart (SImode, operands[0]);
- rtx high2 = gen_highpart (SImode, operands[2]);
- rtx low0 = gen_lowpart (SImode, operands[0]);
-
emit_insn (gen_clrt ());
- emit_insn (gen_subc (low0, low0, gen_lowpart (SImode, operands[2])));
- emit_insn (gen_subc (high0, high0, high2));
+ emit_insn (gen_subc (gen_lowpart (SImode, operands[0]),
+ gen_lowpart (SImode, operands[1]),
+ gen_lowpart (SImode, operands[2])));
+ emit_insn (gen_subc (gen_highpart (SImode, operands[0]),
+ gen_highpart (SImode, operands[1]),
+ gen_highpart (SImode, operands[2])));
DONE;
})
@@ -5567,8 +5565,8 @@
"sub r63, %1, %0"
[(set_attr "type" "arith_media")])
-;; Don't expand immediately because otherwise neg:DI (abs:DI) will not be
-;; combined.
+;; Don't split into individual negc insns immediately so that neg:DI (abs:DI)
+;; can be combined.
(define_expand "negdi2"
[(parallel [(set (match_operand:DI 0 "arith_reg_dest")
(neg:DI (match_operand:DI 1 "arith_reg_operand")))
@@ -5576,12 +5574,12 @@
"TARGET_SH1")
(define_insn_and_split "*negdi2"
- [(set (match_operand:DI 0 "arith_reg_dest" "=&r")
- (neg:DI (match_operand:DI 1 "arith_reg_operand" "r")))
+ [(set (match_operand:DI 0 "arith_reg_dest")
+ (neg:DI (match_operand:DI 1 "arith_reg_operand")))
(clobber (reg:SI T_REG))]
"TARGET_SH1"
"#"
- "&& reload_completed"
+ "&& can_create_pseudo_p ()"
[(const_int 0)]
{
emit_insn (gen_clrt ());
Re: PATCH: middle-end/58981: movmem/setmem use mode wider than Pmode for size
On Tue, 5 Nov 2013, Richard Sandiford wrote: > "H.J. Lu" writes: > > This is the updated patch with md.texi change. The testcase is the same. > > Tested on x32. OK to install? > > Since I commented on the original, this looks good to me, but I can't > approve it. Approved then. Thanks, Richard.
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
2013/11/4 Richard Biener :
> Richard Biener wrote:
>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>> wrote:
>>> Hi,
>>>
>>> Here is a patch which hadles the problem with optimization of
>>BUILT_IN_CHKP_ARG_BND calls. Pointer Bounds Checker expects that
>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>bounds we want to get. The problem is in optimizations which may
>>replace arg with it's copy or a known value. This patch suppress such
>>modifications.
>>
>>This doesn't seem like a good fix. I suppose you require the same on
>>RTL, that is, have the incoming arg reg coalesced with the use?
>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>
> Btw, I would have chosen
>
> P_2 = __builtin_xyz (p_1, bound)
> Call (p_2)
>
> Thus make the builtins a transparent pass-through which effectively binds
> parameter to their bound, removing the need for the artificial arguments and
> making propagations a non.issue
>
> Also, how does the feature interact with other extensions such as nested
> functions or optimizations like inlining?
In RTL all incoming bounds are materialized into slot where bounds are
passed and arg_bnd call is expanded into this slot. Thus in RTL bound
arg looks more like a regular arg.
If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
because it does not used in abnormal phi, shouldn't it? Also it would
prevent all optimizations for these SSA_NAMEs right? Instrumentation
is performed on the earlier stage, right after we build SSA. I think
using abnormal phi flag and binding pointers with bounds via calls
would prevent some useful optimizations.
Many interprocedural optimizations require some support when work with
instrumented calls. Inlining support includes:
- replacement of arg_bnd calls with actual bounds passed to the inlined call
- replacement of retbnd call with bounds, returned by inlined function
Not all IPA passes are fully enabled right now. E.g. I restrict
bounded value propagation in ipa-prop and bounded args in functions
generated by ipa-split. Such features will be enabled later.
For nested functions I do not see much difference from checker point
of view. It just has an additional static chain param. Probably I
miss here something. I did just few tests with nested functions.
Ilya
>
> Richard.
>
>
>>Richard.
>>
>>> Thanks,
>>> Ilya
>>> --
>>>
>>> gcc/
>>>
>>> 2013-10-28 Ilya Enkovich
>>>
>>> * tree-into-ssa.c: Include "target.h"
>>> (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>> * tree-ssa-dom.c: Include "target.h"
>>> (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>
>>>
>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>> index 981e9f4..8d48f6d 100644
>>> --- a/gcc/tree-into-ssa.c
>>> +++ b/gcc/tree-into-ssa.c
>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see
>>> #include "params.h"
>>> #include "diagnostic-core.h"
>>> #include "tree-into-ssa.h"
>>> +#include "target.h"
>>>
>>>
>>> /* This file builds the SSA form for a function as described in:
>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>gimple_stmt_iterator gsi)
>>> }
>>>
>>>/* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>underlying
>>> - symbol is marked for renaming. */
>>> - if (rewrite_uses_p (stmt))
>>> + symbol is marked for renaming.
>>> + Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>> + renamed. */
>>> + if (rewrite_uses_p (stmt)
>>> + && !(flag_check_pointer_bounds
>>> + && (gimple_code (stmt) == GIMPLE_CALL)
>>> + && gimple_call_fndecl (stmt)
>>> + == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>> {
>>>if (is_gimple_debug (stmt))
>>> {
>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>> index 211bfcf..445278a 100644
>>> --- a/gcc/tree-ssa-dom.c
>>> +++ b/gcc/tree-ssa-dom.c
>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see
>>> #include "params.h"
>>> #include "tree-ssa-threadedge.h"
>>> #include "tree-ssa-dom.h"
>>> +#include "target.h"
>>>
>>> /* This file implements optimizations on the dominator tree. */
>>>
>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>use_operand_p op_p;
>>>ssa_op_iter iter;
>>>
>>> + /* Call used to obtain bounds of input arg by Pointer Bounds
>>Checker
>>> + should not be optimized. Argument of the call is a default
>>> + SSA_NAME of PARM_DECL. It should never be replaced by value.
>>*/
>>> + if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>GIMPLE_CALL)
>>> +{
>>> + tree fndecl = gimple_call_fndecl (stmt);
>>> + if (fndecl == targetm.builtin_chkp_function
>>(BUILT_IN_CHKP_ARG_BND))
>>> + return;
>>> +}
>>> +
>>>FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>> cprop_operand (stmt, op_p);
>>> }
>
>
[PATCH, SH] Implement builtin_strlen
Hello,
This patch inlines strlen when optimizing for speed.
A strlen body is now inlined as:
mov r4,r0
tst #3,r0
bf/s.L6
mov r4,r1
mov #0,r3
.L4:
mov.l @r1+,r2
cmp/str r3,r2
bf .L4
add #-4,r1
.L6:
mov.b @r1+,r2
tst r2,r2
bf/s.L6
sett
mov r1,r0
rts
subcr4,r0
A few percent performance improvement here and there for regexp based
benchmarks, but worth to highlight is a 70% speedup for eembc
networking/qos that now nicely combines sequences like !strncmp(*av,
"any", strlen(*av))
No regressions for sh-none-elf. OK for trunk ?
Many thanks
Christian
2013-11-05 Christian Bruel
* gcc/config/sh/sh-mem.cc (sh_expand_strlen): New function.
* gcc/config/sh/sh-protos.h (sh_expand_strlen): Declare.
* gcc/config/sh/sh.md (strlensi): New pattern.
(UNSPEC_BUILTIN_STRLEN): Define.
2013-11-05 Christian Bruel
* gcc.target/sh/strlen.c: New test.
diff --exclude='*~' --exclude=.svn -ruN gcc/config/sh/sh.md ../../gnu_trunk.test/gcc/gcc/config/sh/sh.md
--- gcc/config/sh/sh.md 2013-11-05 12:28:38.0 +0100
+++ ../../gnu_trunk.test/gcc/gcc/config/sh/sh.md 2013-11-05 11:16:00.0 +0100
@@ -161,6 +161,9 @@
;; (unspec [OFFSET ANCHOR] UNSPEC_PCREL_SYMOFF) == OFFSET - (ANCHOR - .).
(UNSPEC_PCREL_SYMOFF 46)
+ ;; Misc builtins
+ (UNSPEC_BUILTIN_STRLEN 47)
+
;; These are used with unspec_volatile.
(UNSPECV_BLOCKAGE 0)
(UNSPECV_ALIGN 1)
@@ -12081,6 +12084,20 @@
FAIL;
})
+(define_expand "strlensi"
+ [(set (match_operand:SI 0 "register_operand")
+ (unspec:SI [(match_operand:BLK 1 "memory_operand")
+ (match_operand:SI 2 "immediate_operand")
+ (match_operand:SI 3 "immediate_operand")]
+ UNSPEC_BUILTIN_STRLEN))]
+ "TARGET_SH1 && optimize"
+{
+ if (! optimize_insn_for_size_p () && sh_expand_strlen (operands))
+ DONE;
+ else
+ FAIL;
+})
+
;; -
;; Floating point instructions.
diff --exclude='*~' --exclude=.svn -ruN gcc/config/sh/sh-mem.cc ../../gnu_trunk.test/gcc/gcc/config/sh/sh-mem.cc
--- gcc/config/sh/sh-mem.cc 2013-11-05 12:30:33.0 +0100
+++ ../../gnu_trunk.test/gcc/gcc/config/sh/sh-mem.cc 2013-11-04 15:34:05.0 +0100
@@ -469,3 +469,83 @@
return true;
}
+
+/* Emit code to perform a strlen
+
+ OPERANDS[0] is the destination.
+ OPERANDS[1] is the string.
+ OPERANDS[2] is the char to search.
+ OPERANDS[3] is the alignment. */
+bool
+sh_expand_strlen (rtx *operands)
+{
+ rtx addr1 = operands[1];
+ rtx start_addr = copy_addr_to_reg (XEXP (addr1, 0));
+ rtx end_addr = gen_reg_rtx (Pmode);
+ rtx tmp0 = gen_reg_rtx (SImode);
+ rtx tmp1 = gen_reg_rtx (SImode);
+ rtx L_return = gen_label_rtx ();
+ rtx L_loop_byte = gen_label_rtx ();
+
+ rtx jump;
+ rtx L_loop_long = gen_label_rtx ();
+ rtx L_end_loop_long = gen_label_rtx ();
+
+ int align = INTVAL (operands[3]);
+
+ emit_move_insn (operands[0], GEN_INT (-1));
+
+ /* remember start of string. */
+ emit_move_insn (end_addr, start_addr);
+
+ if (align < 4)
+{
+ emit_insn (gen_tstsi_t (GEN_INT (3), start_addr));
+ jump = emit_jump_insn (gen_branch_false (L_loop_byte));
+ add_int_reg_note (jump, REG_BR_PROB, prob_likely);
+}
+
+ emit_move_insn (tmp0, operands[2]);
+
+ addr1 = adjust_automodify_address (addr1, SImode, start_addr, 0);
+
+ /*start long loop. */
+ emit_label (L_loop_long);
+
+ /* tmp1 is aligned, OK to load. */
+ emit_move_insn (tmp1, addr1);
+ emit_move_insn (start_addr, plus_constant (Pmode, start_addr, 4));
+
+ /* Is there a 0 byte ? */
+ emit_insn (gen_cmpstr_t (tmp0, tmp1));
+
+ jump = emit_jump_insn (gen_branch_false (L_loop_long));
+ add_int_reg_note (jump, REG_BR_PROB, prob_likely);
+ /* end loop. */
+
+ emit_label (L_end_loop_long);
+
+ emit_move_insn (start_addr, plus_constant (Pmode, start_addr, -4));
+
+ /* start byte loop. */
+ addr1 = adjust_address (addr1, QImode, 0);
+
+ emit_label (L_loop_byte);
+
+ emit_insn (gen_extendqisi2 (tmp1, addr1));
+ emit_move_insn (start_addr, plus_constant (Pmode, start_addr, 1));
+
+ emit_insn (gen_cmpeqsi_t (tmp1, const0_rtx));
+ jump = emit_jump_insn (gen_branch_false (L_loop_byte));
+ add_int_reg_note (jump, REG_BR_PROB, prob_likely);
+
+ /* end loop. */
+
+ emit_label (L_return);
+
+ emit_insn (gen_addsi3 (end_addr, end_addr, GEN_INT (1)));
+
+ emit_insn (gen_subsi3 (operands[0], start_addr, end_addr));
+
+ return true;
+}
diff --exclude='*~' --exclude=.svn -ruN gcc/config/sh/sh-protos.h ../../gnu_trunk.test/gcc/gcc/config/sh/sh-protos.h
--- gcc/config/sh/sh-protos.h 2013-11-05 12:47:44.0 +0100
+++ ../../gnu_trunk.test/gcc/gcc/config/sh/sh-protos.h 2013-11-05 10:14:48.0 +0100
@@ -118,6 +118,7 @@
extern void prepare_move_operands (rtx[], enum machine_mode mode);
extern bool sh_expand_cmpstr (rtx *);
ext
Re: Aliasing: look through pointer's def stmt
On Tue, Nov 5, 2013 at 11:50 AM, Marc Glisse wrote:
> On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> Marc Glisse wrote:
>>>
>>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>>
Well, host_integer_p (, 0) looks correct to me. But ...
>>>
>>>
>>> Ok, I'll put it back.
>
>
> Er, actually host_integerp(, 0) can't work, because arithmetic on pointers
> is done with unsigned integers and for x86_64 it would return false for
> (size_t)-4. So it would need to be host_integerp(, 1) instead. I still test
> TREE_CODE == INTEGER_CST in the current patch, let me know if you want
> host_integerp(, 1).
Hmm, indeed there isn't an exact match to what int_cst_value would
require. Just testing INTEGER_CST should work, we know the
constant is of sizetype which needs to fit a HWI (in this case meaning
TREE_INT_CST_HIGH is either zero or -1).
> Index: gcc/tree-ssa-alias.h
> ===
> --- gcc/tree-ssa-alias.h(revision 204267)
> +++ gcc/tree-ssa-alias.h(working copy)
> @@ -139,30 +139,30 @@ extern void pt_solution_set_var (struct
>
> extern void dump_pta_stats (FILE *);
>
> extern GTY(()) struct pt_solution ipa_escaped_pt;
>
> /* Return true, if the two ranges [POS1, SIZE1] and [POS2, SIZE2]
> overlap. SIZE1 and/or SIZE2 can be (unsigned)-1 in which case
>>>
>>> the
>
> range is open-ended. Otherwise return false. */
>
> static inline bool
> -ranges_overlap_p (unsigned HOST_WIDE_INT pos1,
> - unsigned HOST_WIDE_INT size1,
> - unsigned HOST_WIDE_INT pos2,
> - unsigned HOST_WIDE_INT size2)
> +ranges_overlap_p (HOST_WIDE_INT pos1,
> + HOST_WIDE_INT size1,
> + HOST_WIDE_INT pos2,
> + HOST_WIDE_INT size2)
I think size[12] should still be unsigned (we don't allow negative
sizes but only the special value -1U which we special-case only to
avoid pos + size to wrap)
>>>
>>>
>>> But all we do with size[12] is compare it to -1 and perform arithmetic
>>> with pos[12]. At least for the second one, we need to cast size to a
>>> signed type so the comparisons make sense (unless we decide to use a
>>> double_int there). So I thought I would do the cast in the argument.
>>> Otherwise, I'll start the function with:
>>> HOST_WIDE_INT ssize1 = (HOST_WIDE_INT)size1;
>>> and replace size1 with ssize1 through the function.
>>>
>>> Is the reason of keeping size[12] unsigned for documentation? Or am I
>>> missing a reason why I may be breaking things?
>>
>>
>> It is mostly documentation indeed.
>
>
> Ok, then here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu).
Ok.
Thanks,
Richard.
> 2013-11-05 Marc Glisse
>
>
> gcc/
> * tree-ssa-alias.h (ranges_overlap_p): Handle negative offsets.
> * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Likewise.
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/alias-26.c: New file.
>
> --
> Marc Glisse
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/alias-26.c(revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/alias-26.c(working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +void f (const char *c, int *i)
> +{
> + *i = 42;
> + __builtin_memcpy (i - 1, c, sizeof (int));
> + if (*i != 42) __builtin_abort();
> +}
> +
> +/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> +
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> ___
> Added: svn:keywords
> ## -0,0 +1 ##
> +Author Date Id Revision URL
> \ No newline at end of property
> Added: svn:eol-style
> ## -0,0 +1 ##
> +native
> \ No newline at end of property
> Index: gcc/tree-ssa-alias.c
> ===
> --- gcc/tree-ssa-alias.c(revision 204381)
> +++ gcc/tree-ssa-alias.c(working copy)
> @@ -552,42 +552,42 @@ ao_ref_base_alias_set (ao_ref *ref)
> alias_set_type
> ao_ref_alias_set (ao_ref *ref)
> {
>if (ref->ref_alias_set != -1)
> return ref->ref_alias_set;
>ref->ref_alias_set = get_alias_set (ref->ref);
>return ref->ref_alias_set;
> }
>
> /* Init an alias-oracle reference representation from a gimple pointer
> - PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the
> + PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE then the
> size is assumed to be unknown. The access is assumed to be only
> to or after of the pointer target, not before it. */
>
> void
> ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
> {
>HOST_WIDE_INT t1, t2, extra_offset = 0;
>ref->ref
Re: PR 58958: wrong aliasing info
On Tue, Nov 5, 2013 at 11:40 AM, Marc Glisse wrote:
> On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> Well, you cannot use the size argument unchanged for the null return case.
>> You could fallback to get_base_address and -1 size in that case.
>
>
> Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
> (I think I'll disable cilk for my future bootstraps: it takes forever and
> confuses contrib/compare_tests)
Yes, this is ok.
Thanks,
Richard.
> 2013-11-05 Marc Glisse
>
> PR tree-optimization/58958
> gcc/
> * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
> get_addr_base_and_unit_offset instead of get_ref_base_and_extent.
>
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/pr58958.c: New file.
>
> --
> Marc Glisse
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +double a[10];
> +int f(int n){
> + a[3]=9;
> + __builtin_memset(&a[n],3,sizeof(double));
> + return a[3]==9;
> +}
> +
> +/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
> ___
> Added: svn:keywords
> ## -0,0 +1 ##
> +Author Date Id Revision URL
> \ No newline at end of property
> Added: svn:eol-style
> ## -0,0 +1 ##
> +native
> \ No newline at end of property
> Index: gcc/tree-ssa-alias.c
> ===
> --- gcc/tree-ssa-alias.c(revision 204381)
> +++ gcc/tree-ssa-alias.c(working copy)
> @@ -559,41 +559,50 @@ ao_ref_alias_set (ao_ref *ref)
> }
>
> /* Init an alias-oracle reference representation from a gimple pointer
> PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the
> size is assumed to be unknown. The access is assumed to be only
> to or after of the pointer target, not before it. */
>
> void
> ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
> {
> - HOST_WIDE_INT t1, t2, extra_offset = 0;
> + HOST_WIDE_INT t, extra_offset = 0;
>ref->ref = NULL_TREE;
>if (TREE_CODE (ptr) == SSA_NAME)
> {
>gimple stmt = SSA_NAME_DEF_STMT (ptr);
>if (gimple_assign_single_p (stmt)
> && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
> ptr = gimple_assign_rhs1 (stmt);
>else if (is_gimple_assign (stmt)
>&& gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
>&& host_integerp (gimple_assign_rhs2 (stmt), 0)
> - && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
> + && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
> {
> ptr = gimple_assign_rhs1 (stmt);
> - extra_offset = BITS_PER_UNIT * t1;
> + extra_offset = BITS_PER_UNIT * t;
> }
> }
>
>if (TREE_CODE (ptr) == ADDR_EXPR)
> -ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
> -&ref->offset, &t1, &t2);
> +{
> + ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0),
> &t);
> + if (ref->base)
> + ref->offset = BITS_PER_UNIT * t;
> + else
> + {
> + size = NULL_TREE;
> + ref->offset = 0;
> + ref->base = get_base_address (TREE_OPERAND (ptr, 0));
> + }
> +}
>else
> {
>ref->base = build2 (MEM_REF, char_type_node,
> ptr, null_pointer_node);
>ref->offset = 0;
> }
>ref->offset += extra_offset;
>if (size
>&& host_integerp (size, 0)
>&& TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
>
[PATCH] Add POST_LINK_SPEC for adding possibility of additional steps after linking
Attached patch adds a possibility to add additional build steps after linking.
Without this patch only possibility is to redefine entire LINK_COMMAND_SPEC.
Currently only DJGPP seems to need it
2013-11-05Andris Pavenis
* gcc/gcc.c: Add macro POST_LINK SPEC for specifying additional steps
to invoke after linking
* gcc/doc/tm.texi.in: (POST_LINK_SPEC): new
* gcc/doc/tm.texi: regenerate
Bootstrapped and tested on Linux x86_64 (Fedora 19)
Andris
>From 3dd27dc54f3fd38d2b1e9b7fb53938be4df8da7c Mon Sep 17 00:00:00 2001
From: Andris Pavenis
Date: Tue, 5 Nov 2013 11:52:17 +0200
Subject: [PATCH] POST_LINK_SPEC: new define for additional steps after linking
This allows to avoid redefining LINK_COMMAND_SPEC when
additional steps are needed after linking
An example: invoking stubify after linking for DJGPP
---
gcc/doc/tm.texi| 6 ++
gcc/doc/tm.texi.in | 6 ++
gcc/gcc.c | 8 +++-
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 72daf09..692f675 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -402,6 +402,12 @@ the target makefile fragment or if none of the options listed in
@xref{Target Fragment}.
@end defmac
+@defmac POST_LINK_SPEC
+A C string constant giving complete commands needed to execute after linker.
+This macro allows to avoid redefining LINK_COMMAND_SPEC when additional
+steps are needed after invoking linker. The default value is empty string.
+@end defmac
+
@defmac RELATIVE_PREFIX_NOT_LINKDIR
Define this macro to tell @command{gcc} that it should only translate
a @option{-B} prefix into a @option{-L} linker option if the prefix
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 2828361..edc2b9d 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -400,6 +400,12 @@ the target makefile fragment or if none of the options listed in
@xref{Target Fragment}.
@end defmac
+@defmac POST_LINK_SPEC
+A C string constant giving complete commands needed to execute after linker.
+This macro allows to avoid redefining LINK_COMMAND_SPEC when additional
+steps are needed after invoking linker. The default value is empty string.
+@end defmac
+
@defmac RELATIVE_PREFIX_NOT_LINKDIR
Define this macro to tell @command{gcc} that it should only translate
a @option{-B} prefix into a @option{-L} linker option if the prefix
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 2298249..cc0ac29 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -745,6 +745,10 @@ proper position among the other output files. */
%{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start -u_vtable_map_vars_end}}"
#endif
+#ifndef POST_LINK_SPEC
+#define POST_LINK_SPEC ""
+#endif
+
/* -u* was put back because both BSD and SysV seem to support it. */
/* %{static:} simply prevents an error message if the target machine
doesn't handle -static. */
@@ -770,7 +774,7 @@ proper position among the other output files. */
%(mflib) " STACK_SPLIT_SPEC "\
%{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
%{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
-%{!nostdlib:%{!nostartfiles:%E}} %{T*} }}"
+%{!nostdlib:%{!nostartfiles:%E}} %{T*} \n%(post_link) }}"
#endif
#ifndef LINK_LIBGCC_SPEC
@@ -812,6 +816,7 @@ static const char *linker_name_spec = LINKER_NAME;
static const char *linker_plugin_file_spec = "";
static const char *lto_wrapper_spec = "";
static const char *lto_gcc_spec = "";
+static const char *post_link_spec = POST_LINK_SPEC;
static const char *link_command_spec = LINK_COMMAND_SPEC;
static const char *link_libgcc_spec = LINK_LIBGCC_SPEC;
static const char *startfile_prefix_spec = STARTFILE_PREFIX_SPEC;
@@ -1304,6 +1309,7 @@ static struct spec_list static_specs[] =
INIT_STATIC_SPEC ("linker_plugin_file", &linker_plugin_file_spec),
INIT_STATIC_SPEC ("lto_wrapper",
Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion
Sorry for my late reply on this. I just have one very small nit for one ChangeLog entry, expressed below. If nobody objects in the next 48 hours, I'd say this is OK to commit with the nit fixed. I am not seeing your name in the MAINTAINERS file. Have you filed copyright assignment to the FSF and do you have it all sorted? Joshua J Cogliati writes: > Since part of it was to go into c-family (as per Joseph S. Myers's > email) and parts are not in c-family, I split the changelog into three > parts. I also changed the formatting of the changelog and patch as > per Dodji's comments. The attached patch is the same as before, but > the diff for Wfloat-conversion.c is as a new file, not as a copy of > Wconversion-real.c. Thanks for reviewing it. If there is anything > else that needs changing, please tell me. > > > For gcc/c-family/ChangeLog: > > PR c/53001 > Splitting out a -Wfloat-conversion from -Wconversion for > conversions that lower floating point number precision > or conversion from floating point numbers to integers > * c-common.c (unsafe_conversion_p): Make this function > return an enumeration with more detail. This is nicely formatted now, thank you. > (conversion_warning): Use the new return type of > unsafe_conversion_p to separately warn either about conversions > that lower floating point number precision or about the other > kinds of conversions. > * c-common.h: Adding conversion_safety enumeration. > (unsafe_conversion_p): switching return type to > conversion_safety enumeration. Here, this should be: * c-common.h (enum conversion_safety): New enumeration. (unsafe_conversion_p): switching return type to conversion_safety enumeration. OK to commit with the above changes, if you have your copyright properly assigned to the FSF. Thank for working on this. It is appreciated. -- Dodji
Re: [PATCH GCC]Simplify address expression in IVOPT
On 11/05/13 11:45, Bin.Cheng wrote:
On Tue, Nov 5, 2013 at 7:19 PM, Yufeng Zhang wrote:
> On 11/05/13 10:13, bin.cheng wrote:
>>
>> Index: gcc/tree-affine.c
>> ===
>> --- gcc/tree-affine.c (revision 204117)
>> +++ gcc/tree-affine.c (working copy)
>> @@ -874,10 +874,11 @@ debug_aff (aff_tree *val)
>> fprintf (stderr, "\n");
>> }
>>
>> -/* Returns address of the reference REF in ADDR. The size of the
>> accessed
>> - location is stored to SIZE. */
>> +/* Computes address of the reference REF in ADDR. The size of the
>> accessed
>> + location is stored to SIZE. Returns pointer to the ultimate
>> containing
>> + object to which REF refers. */
>>
>> -void
>> +tree
>> get_inner_reference_aff (tree ref, aff_tree *addr, double_int *size)
>> {
>> HOST_WIDE_INT bitsize, bitpos;
>> @@ -904,6 +905,8 @@ get_inner_reference_aff (tree ref, aff_tree *addr,
>> aff_combination_add (addr,&tmp);
>>
>> *size = double_int::from_shwi ((bitsize + BITS_PER_UNIT - 1) /
>> BITS_PER_UNIT);
>> +
>> + return base_addr;
>> }
>>
>
> I think what Richard suggests is to return the base object rather the
> address of the base object, i.e.
I am not sure about that. We have to pass pointer_type expression to
function determine_base_object for address expressions, because there
is no way to tell pointer from object once we are in
determine_base_object.
I'm just concerned with the consistency in what is returned between
get_inner_reference and get_inner_reference_aff. If
determine_base_object expects reference only, you can probably work
around it with something like:
base_object = build_fold_addr_expr (base_object);
after the get_inner_reference_aff call.
Yufeng
Re: [PATCH GCC]Simplify address expression in IVOPT
On Tue, Nov 5, 2013 at 11:13 AM, bin.cheng wrote:
>
>
>> -Original Message-
>> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
>> ow...@gcc.gnu.org] On Behalf Of bin.cheng
>> Sent: Monday, November 04, 2013 4:35 PM
>> To: 'Richard Biener'
>> Cc: GCC Patches
>> Subject: RE: [PATCH GCC]Simplify address expression in IVOPT
>>
>>
>>
>> > -Original Message-
>> > From: Richard Biener [mailto:richard.guent...@gmail.com]
>> > Sent: Wednesday, October 30, 2013 10:46 PM
>> > To: Bin Cheng
>> > Cc: GCC Patches
>> > Subject: Re: [PATCH GCC]Simplify address expression in IVOPT
>> >
>> > On Tue, Oct 29, 2013 at 10:18 AM, bin.cheng wrote:
>> >
>> > Hmm. I think you want what get_inner_reference_aff does, using the
>> > return value of get_inner_reference as starting point for
>> determine_base_object.
>> > And you don't want to restrict yourselves so much on what exprs to
>> process,
>> > but only exclude DECL_Ps.
>> Did you mean I should pass all address expressions to
>> get_inner_reference_aff except the one with declaration operand?
>> I am a little confused here why DECL_Ps should be handled specially?
> Seems
>> it can be handled properly by the get_* function. Anything important I
>> missed?
>>
>> > Just amend get_inner_reference_aff to return the tree base object.
>> I found it's inappropriate to do this because: functions like
>> get_inner_reference* only accept reference expressions, while base_object
>> has to be computed for other kinds of expressions too. Take gimple
>> statement "a_1 = *ptr_1" as an example, the base passed to alloc_iv is
> ptr_1;
>> the base_object computed by determine_base_object is also ptr_1, which
>> we can't rely on get_inner_reference* .
>>
>> Also It seems the comment before determine_base_object might be
>> inaccurate:
>> " Returns a memory object to that EXPR points." which I think is " Returns
> a
>> pointer pointing to the main memory object to that EXPR points."
>> Right?
>> >
>> > Note that this isn't really "simplifying" but rather lowering all
>> addresses.
>> >
>> > The other changes in this patch are unrelated, right?
>> Right, I should have named this message like "refine cost computation of
>> expressions in IVOPT" just as the patch file.
>
> Hi,
> I updated the patch according to review comments. Now it lowers all address
> expressions except ones with DECL_P to get_inner_reference_aff, it also
> computes base_object which is used later to ease determine_base_object.
>
> Bootstrap and test on x86/x86_64/arm on going, is it OK if tests pass?
static struct iv *
alloc_iv (tree base, tree step)
{
+ tree expr = base;
+ tree base_object = base;
struct iv *iv = XCNEW (struct iv);
gcc_assert (step != NULL_TREE);
+ /* Lower all address expressions except ones with DECL_P as oeprand.
+ By doing this:
+ 1) More accurate cost can be computed for address expressions;
+ 2) Duplicate candidates won't be created for bases in different
+ forms, like &a[0] and &a. */
+ STRIP_NOPS (expr);
+ if (TREE_CODE (expr) == ADDR_EXPR && !DECL_P (TREE_OPERAND (expr, 0)))
+{
+ aff_tree comb;
+ double_int size;
+ base_object = get_inner_reference_aff (TREE_OPERAND (expr, 0),
+ &comb, &size);
+ gcc_assert (base_object != NULL_TREE);
+ base = fold_convert (TREE_TYPE (base), aff_combination_to_tree (&comb));
+}
+
iv->base = base;
- iv->base_object = determine_base_object (base);
+ iv->base_object = determine_base_object (base_object);
for less confusion do not introduce 'expr' but just base_object
(s/expr/base_object/). Also can you split out this change (and the
related tree-affine.c one) from the rest?
Ok with that change assuming it passes bootstrap & testing.
Thanks,
Richard.
> Thanks.
> bin
>
> gcc/testsuite/ChangeLog
> 2013-11-05 Bin Cheng
>
> * gcc.dg/tree-ssa/loop-2.c: Refine check condition.
> * gcc.dg/tree-ssa/ivopt_infer_2.c: Ditto.
> * gcc.dg/tree-ssa/ivopt_mult_3.c: Ditto.
>
> 2013-11-05 Bin Cheng
>
> * tree-ssa-loop-ivopts.c (alloc_iv): Lower address expressions.
> (get_shiftadd_cost): Check equality using operand_equal_p.
> (force_expr_to_var_cost): Refactor the code. Handle type
> conversion.
> (split_address_cost): Call force_expr_to_var_cost.
> * tree-affine.c (get_inner_reference_aff): Return base_addr.
> * tree-affine.h (get_inner_reference_aff): Change prototype.
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich wrote:
> 2013/11/4 Richard Biener :
>> Richard Biener wrote:
>>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>>> wrote:
Hi,
Here is a patch which hadles the problem with optimization of
>>>BUILT_IN_CHKP_ARG_BND calls. Pointer Bounds Checker expects that
>>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>>bounds we want to get. The problem is in optimizations which may
>>>replace arg with it's copy or a known value. This patch suppress such
>>>modifications.
>>>
>>>This doesn't seem like a good fix. I suppose you require the same on
>>>RTL, that is, have the incoming arg reg coalesced with the use?
>>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>
>> Btw, I would have chosen
>>
>> P_2 = __builtin_xyz (p_1, bound)
>> Call (p_2)
>>
>> Thus make the builtins a transparent pass-through which effectively binds
>> parameter to their bound, removing the need for the artificial arguments and
>> making propagations a non.issue
>>
>> Also, how does the feature interact with other extensions such as nested
>> functions or optimizations like inlining?
>
> In RTL all incoming bounds are materialized into slot where bounds are
> passed and arg_bnd call is expanded into this slot. Thus in RTL bound
> arg looks more like a regular arg.
I don't care so much for RTL, but this description hints at that the
suggestion above would work (and it would eliminate all my concerns about
the representation on the GIMPLE level - you'd not even need this
strange POINTER_BOUNDS_TYPE as far as I can see.
> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
> because it does not used in abnormal phi, shouldn't it? Also it would
> prevent all optimizations for these SSA_NAMEs right? Instrumentation
> is performed on the earlier stage, right after we build SSA. I think
> using abnormal phi flag and binding pointers with bounds via calls
> would prevent some useful optimizations.
Well, what are the constraints that you need to avoid propagation in
the first place?
> Many interprocedural optimizations require some support when work with
> instrumented calls. Inlining support includes:
> - replacement of arg_bnd calls with actual bounds passed to the inlined call
> - replacement of retbnd call with bounds, returned by inlined function
>
> Not all IPA passes are fully enabled right now. E.g. I restrict
> bounded value propagation in ipa-prop and bounded args in functions
> generated by ipa-split. Such features will be enabled later.
>
> For nested functions I do not see much difference from checker point
> of view. It just has an additional static chain param. Probably I
> miss here something. I did just few tests with nested functions.
I was thinking of
int foo (char *s)
{
int bar (void)
{
... use bound of 's' of the containing function ...
foo (q, and pass it along here for q)
}
}
that is references to the containing functions parameters and their bounds.
Richard.
> Ilya
>
>>
>> Richard.
>>
>>
>>>Richard.
>>>
Thanks,
Ilya
--
gcc/
2013-10-28 Ilya Enkovich
* tree-into-ssa.c: Include "target.h"
(rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
* tree-ssa-dom.c: Include "target.h"
(cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index 981e9f4..8d48f6d 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see
#include "params.h"
#include "diagnostic-core.h"
#include "tree-into-ssa.h"
+#include "target.h"
/* This file builds the SSA form for a function as described in:
@@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>>gimple_stmt_iterator gsi)
}
/* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>>underlying
- symbol is marked for renaming. */
- if (rewrite_uses_p (stmt))
+ symbol is marked for renaming.
+ Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
+ renamed. */
+ if (rewrite_uses_p (stmt)
+ && !(flag_check_pointer_bounds
+ && (gimple_code (stmt) == GIMPLE_CALL)
+ && gimple_call_fndecl (stmt)
+ == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
{
if (is_gimple_debug (stmt))
{
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 211bfcf..445278a 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see
#include "params.h"
#include "tree-ssa-threadedge.h"
#include "tree-ssa-dom.h"
+#include "target.h"
/* This file implements optimizations on the domin
Re: [PATCH GCC]Simplify address expression in IVOPT
On Tue, Nov 5, 2013 at 1:18 PM, Yufeng Zhang wrote:
> On 11/05/13 11:45, Bin.Cheng wrote:
>>
>> On Tue, Nov 5, 2013 at 7:19 PM, Yufeng Zhang wrote:
>>>
>>> > On 11/05/13 10:13, bin.cheng wrote:
>>
>> Index: gcc/tree-affine.c
>> ===
>> --- gcc/tree-affine.c (revision 204117)
>> +++ gcc/tree-affine.c (working copy)
>> @@ -874,10 +874,11 @@ debug_aff (aff_tree *val)
>> fprintf (stderr, "\n");
>> }
>>
>> -/* Returns address of the reference REF in ADDR. The size of the
>> accessed
>> - location is stored to SIZE. */
>> +/* Computes address of the reference REF in ADDR. The size of the
>> accessed
>> + location is stored to SIZE. Returns pointer to the ultimate
>> containing
>> + object to which REF refers. */
>>
>> -void
>> +tree
>> get_inner_reference_aff (tree ref, aff_tree *addr, double_int
>> *size)
>> {
>> HOST_WIDE_INT bitsize, bitpos;
>> @@ -904,6 +905,8 @@ get_inner_reference_aff (tree ref, aff_tree
>> *addr,
>> aff_combination_add (addr,&tmp);
>>
>> *size = double_int::from_shwi ((bitsize + BITS_PER_UNIT - 1) /
>> BITS_PER_UNIT);
>> +
>> + return base_addr;
>> }
>>
>>>
>>> >
>>> > I think what Richard suggests is to return the base object rather the
>>> > address of the base object, i.e.
>>
>> I am not sure about that. We have to pass pointer_type expression to
>> function determine_base_object for address expressions, because there
>> is no way to tell pointer from object once we are in
>> determine_base_object.
>
>
> I'm just concerned with the consistency in what is returned between
> get_inner_reference and get_inner_reference_aff. If determine_base_object
> expects reference only, you can probably work around it with something like:
>
> base_object = build_fold_addr_expr (base_object);
>
> after the get_inner_reference_aff call.
Note that get_inner_reference_aff is already different from get_inner_reference
in that it does not separate base object and offset but lumps them together
into an address represented as affine combination.
For this reason get_inner_reference_aff may not exactly be the best
name for the function ... it could be split to not do
aff_combination_add (addr, &tmp);
but instead return 'base'. The two existing users of the function could
be unified into aff_comb_cannot_overlap_p (giving that a better name
as well), even avoiding combining the base but instead handle
comparing the base itself.
Of course that's preexisting issues that I don't want to force Bin Cheng
to clean up and fix (given I've, too, just spent a minute thinking about it).
Thanks,
Richard.
> Yufeng
>
Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)
On Tue, 2013-11-05 at 12:47 +0100, Richard Biener wrote:
> On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm wrote:
> > On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
> >> On 11/01/2013 06:58 PM, David Malcolm wrote:
> >> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
> >> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
> >> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
> >> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
> >> >static inline void
> >> > ! gimple_call_set_lhs (gimple gs, tree lhs)
> >> >{
> >> > - GIMPLE_CHECK (gs, GIMPLE_CALL);
> >> >> The checking you are removing here.
> >> >>
> >> >>> What checking? There ought to be no checking at all in this
> >> >>> example... gimple_build_call_vec returns a gimple_call, and
> >> >>> gimple_call_set_lhs() doesn't have to check anything because it
> >> >>> only accepts gimple_call's.. so there is no checking other than the
> >> >>> usual "does my parameter match" that the compiler has to do...
> >> >> and want to replace it by checking of the types at compile time.
> >> >> The problem is that it uglifies the source too much, and, when you
> >> >> actually don't have a gimple_call but supposedly a base class of it,
> >> >> I expect you'd do as_a which is not only further uglification, but has
> >> >> runtime cost also for --enable-checking=release.
> >> > I can have a look next week at every call to gimple_call_set_lhs in the
> >> > tree, and see to what extent we know at compile-time that the initial
> >> > arg is indeed a call (of the ones I quickly grepped just now, most are
> >> > from gimple_build_call and friends, but one was from a gimple_copy).
> >> >
> >> > FWIW I did some performance testing of the is_a/as_a code in the earlier
> >> > version of the patch, and it didn't have a noticable runtime cost
> >> > compared to the GIMPLE_CHECK in the existing code:
> >> > Size of compiler executable:
> >> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
> >> > Compile times:
> >> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
> >> I actually really dislike as_a<> and is_a<>, and think code needs to be
> >> restructured rather than use them, other than possibly at the very
> >> bottom level when we're allocating memory or something like that, or
> >> some kind of emergency :-)... If we require frequent uses of those,
> >> I'd be against it, I find them quite ugly.
> >>
> >> Like I said in the other reply, no rush, I don't think any of this
> >> follow up is appropriate this late in stage 1. It would be more of an
> >> "interest" examination right now.. at least in my opinion... I suspect
> >> thinks like gimple_assign are more complex cases, but without looking
> >> its hard to tell for sure.
> >
> > I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> > than a gimple, and excitingly, it was easiest to also convert
> > cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> > gimple.
>
> Does that work (using gimple_call * objects) for our garbage collector?
> That is, does it know it is looking at a 'gimple'? It doesn't matter for this
> case as all stmts are reachable from struct function as sequence of 'gimple',
> but in general?
Yes (as of r204146, I believe).
For example, the patch converts cgraph_edge's call_stmt to be a
"gimple_call", rather than just a "gimple", but gengtype handles this by
emitting a call to the base class visitor for call_stmt i.e.
gimple_statement_base:
void
gt_ggc_mx_cgraph_edge (void *x_p)
{
struct cgraph_edge * x = (struct cgraph_edge *)x_p;
[...snip chain_next/prev handling...]
[...snip other fields...]
gt_ggc_m_21gimple_statement_base ((*x).call_stmt);
[..etc..]
}
Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)
On Tue, Nov 5, 2013 at 1:30 PM, David Malcolm wrote:
> On Tue, 2013-11-05 at 12:47 +0100, Richard Biener wrote:
>> On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm wrote:
>> > On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
>> >> On 11/01/2013 06:58 PM, David Malcolm wrote:
>> >> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
>> >> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
>> >> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
>> >> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
>> >> >static inline void
>> >> > ! gimple_call_set_lhs (gimple gs, tree lhs)
>> >> >{
>> >> > - GIMPLE_CHECK (gs, GIMPLE_CALL);
>> >> >> The checking you are removing here.
>> >> >>
>> >> >>> What checking? There ought to be no checking at all in this
>> >> >>> example... gimple_build_call_vec returns a gimple_call, and
>> >> >>> gimple_call_set_lhs() doesn't have to check anything because it
>> >> >>> only accepts gimple_call's.. so there is no checking other than the
>> >> >>> usual "does my parameter match" that the compiler has to do...
>> >> >> and want to replace it by checking of the types at compile time.
>> >> >> The problem is that it uglifies the source too much, and, when you
>> >> >> actually don't have a gimple_call but supposedly a base class of it,
>> >> >> I expect you'd do as_a which is not only further uglification, but has
>> >> >> runtime cost also for --enable-checking=release.
>> >> > I can have a look next week at every call to gimple_call_set_lhs in the
>> >> > tree, and see to what extent we know at compile-time that the initial
>> >> > arg is indeed a call (of the ones I quickly grepped just now, most are
>> >> > from gimple_build_call and friends, but one was from a gimple_copy).
>> >> >
>> >> > FWIW I did some performance testing of the is_a/as_a code in the earlier
>> >> > version of the patch, and it didn't have a noticable runtime cost
>> >> > compared to the GIMPLE_CHECK in the existing code:
>> >> > Size of compiler executable:
>> >> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
>> >> > Compile times:
>> >> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
>> >> I actually really dislike as_a<> and is_a<>, and think code needs to be
>> >> restructured rather than use them, other than possibly at the very
>> >> bottom level when we're allocating memory or something like that, or
>> >> some kind of emergency :-)... If we require frequent uses of those,
>> >> I'd be against it, I find them quite ugly.
>> >>
>> >> Like I said in the other reply, no rush, I don't think any of this
>> >> follow up is appropriate this late in stage 1. It would be more of an
>> >> "interest" examination right now.. at least in my opinion... I suspect
>> >> thinks like gimple_assign are more complex cases, but without looking
>> >> its hard to tell for sure.
>> >
>> > I tried converting gimple_call_set_lhs to accept a gimple_call, rather
>> > than a gimple, and excitingly, it was easiest to also convert
>> > cgraph_edge's call_stmt to also be a gimple_call, rather than just a
>> > gimple.
>>
>> Does that work (using gimple_call * objects) for our garbage collector?
>> That is, does it know it is looking at a 'gimple'? It doesn't matter for
>> this
>> case as all stmts are reachable from struct function as sequence of 'gimple',
>> but in general?
> Yes (as of r204146, I believe).
>
> For example, the patch converts cgraph_edge's call_stmt to be a
> "gimple_call", rather than just a "gimple", but gengtype handles this by
> emitting a call to the base class visitor for call_stmt i.e.
> gimple_statement_base:
>
> void
> gt_ggc_mx_cgraph_edge (void *x_p)
> {
> struct cgraph_edge * x = (struct cgraph_edge *)x_p;
> [...snip chain_next/prev handling...]
> [...snip other fields...]
> gt_ggc_m_21gimple_statement_base ((*x).call_stmt);
> [..etc..]
> }
Ah, nice to know.
Thanks,
Richard.
>
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich wrote:
>> 2013/11/4 Richard Biener :
>>> Richard Biener wrote:
On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
wrote:
> Hi,
>
> Here is a patch which hadles the problem with optimization of
BUILT_IN_CHKP_ARG_BND calls. Pointer Bounds Checker expects that
argument of this call is a default SSA_NAME of the PARM_DECL whose
bounds we want to get. The problem is in optimizations which may
replace arg with it's copy or a known value. This patch suppress such
modifications.
This doesn't seem like a good fix. I suppose you require the same on
RTL, that is, have the incoming arg reg coalesced with the use?
In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>>
>>> Btw, I would have chosen
>>>
>>> P_2 = __builtin_xyz (p_1, bound)
>>> Call (p_2)
>>>
>>> Thus make the builtins a transparent pass-through which effectively binds
>>> parameter to their bound, removing the need for the artificial arguments
>>> and making propagations a non.issue
>>>
>>> Also, how does the feature interact with other extensions such as nested
>>> functions or optimizations like inlining?
>>
>> In RTL all incoming bounds are materialized into slot where bounds are
>> passed and arg_bnd call is expanded into this slot. Thus in RTL bound
>> arg looks more like a regular arg.
>
> I don't care so much for RTL, but this description hints at that the
> suggestion above would work (and it would eliminate all my concerns about
> the representation on the GIMPLE level - you'd not even need this
> strange POINTER_BOUNDS_TYPE as far as I can see.
>
>> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
>> because it does not used in abnormal phi, shouldn't it? Also it would
>> prevent all optimizations for these SSA_NAMEs right? Instrumentation
>> is performed on the earlier stage, right after we build SSA. I think
>> using abnormal phi flag and binding pointers with bounds via calls
>> would prevent some useful optimizations.
>
> Well, what are the constraints that you need to avoid propagation in
> the first place?
For input parameter P I need to have
BOUNDS = __builtin_arg_bnd (P)
to somehow refer to bounds of P in GIMPLE. Optimizations may modify
__builtin_arg_bnd (P) replacing P with its copy or some value. It
makes call useless because removes information about parameter whose
bounds we refer to. I want such optimization to ignore
__builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
there as arg.
>
>> Many interprocedural optimizations require some support when work with
>> instrumented calls. Inlining support includes:
>> - replacement of arg_bnd calls with actual bounds passed to the inlined
>> call
>> - replacement of retbnd call with bounds, returned by inlined function
>>
>> Not all IPA passes are fully enabled right now. E.g. I restrict
>> bounded value propagation in ipa-prop and bounded args in functions
>> generated by ipa-split. Such features will be enabled later.
>>
>> For nested functions I do not see much difference from checker point
>> of view. It just has an additional static chain param. Probably I
>> miss here something. I did just few tests with nested functions.
>
> I was thinking of
>
> int foo (char *s)
> {
>int bar (void)
>{
> ... use bound of 's' of the containing function ...
> foo (q, and pass it along here for q)
>}
> }
>
> that is references to the containing functions parameters and their
All foo's locals referenced by nested bar here are placed into special
structure and all bounds should be stored in appropriated entries of
Bound Table. Nested function may refer to them via this Table.
Ilya
>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>
Richard.
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-28 Ilya Enkovich
>
> * tree-into-ssa.c: Include "target.h"
> (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
> * tree-ssa-dom.c: Include "target.h"
> (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>
>
> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
> index 981e9f4..8d48f6d 100644
> --- a/gcc/tree-into-ssa.c
> +++ b/gcc/tree-into-ssa.c
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see
> #include "params.h"
> #include "diagnostic-core.h"
> #include "tree-into-ssa.h"
> +#include "target.h"
>
>
> /* This file builds the SSA form for a function as described in:
> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
gimple_stmt_iterator gsi)
> }
>
>/* Rewrite USES included in OLD_SSA_NAMES and USES whose
underlying
> - symbol is marked for renaming. */
> - if (rewrite_uses_p (stmt))
> + symbol is marked for renaming.
> + Skip calls to BUILT_IN_CHK
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/2 Joseph S. Myers : > On Thu, 31 Oct 2013, Ilya Enkovich wrote: > >> This patch adds support Pointer Bounds Checker into c-family and LTO >> front-ends. The main purpose of changes in front-end is to register all >> statically initialized objects for checker. We also need to register >> such objects created by compiler. > > What happens with statically initialized TLS objects? It would seem that > you need something like how the C++ front end handles static constructors > for C++11 thread-local objects, but I don't see anything obvious for that > here. This patch takes care of pointers initialized by linker. TLS objects are dynamically allocated for each thread and should have some constructor to perform initialization. And if there is such constructors then it should be instrumented by Pointer Bounds Checker. Therefore it should not require additional changes in front-end, right? The only problem I now see for TLS objects is that in my constructors initialize bounds for an actual TLS object, not for object in .tdata. How can I refer to .tdata objects in GIMPLE to get correct bounds initialization for .tdata section? Thanks, Ilya > > -- > Joseph S. Myers > jos...@codesourcery.com
Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion, [gnu.org #746725]
Donald R Robertson III or the current copyright clerk, Can you check and make sure that my copyright assignment paperwork is finished? I was number: 746725 It should have been received September 20, 2013. Thank you. Joshua Cogliati On Tue, 11/5/13, Dodji Seketeli wrote: Subject: Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion To: "Joshua J Cogliati" Cc: "Joseph S. Myers" , gcc-patches@gcc.gnu.org, ja...@redhat.com, m...@gcc.gnu.org Date: Tuesday, November 5, 2013, 5:12 AM Sorry for my late reply on this. I just have one very small nit for one ChangeLog entry, expressed below. If nobody objects in the next 48 hours, I'd say this is OK to commit with the nit fixed. I am not seeing your name in the MAINTAINERS file. Have you filed copyright assignment to the FSF and do you have it all sorted? Joshua J Cogliati writes: > Since part of it was to go into c-family (as per Joseph S. Myers's > email) and parts are not in c-family, I split the changelog into three > parts. I also changed the formatting of the changelog and patch as > per Dodji's comments. The attached patch is the same as before, but > the diff for Wfloat-conversion.c is as a new file, not as a copy of > Wconversion-real.c. Thanks for reviewing it. If there is anything > else that needs changing, please tell me. > > > For gcc/c-family/ChangeLog: > > PR c/53001 > Splitting out a -Wfloat-conversion from -Wconversion for > conversions that lower floating point number precision > or conversion from floating point numbers to integers > * c-common.c (unsafe_conversion_p): Make this function > return an enumeration with more detail. This is nicely formatted now, thank you. > (conversion_warning): Use the new return type of > unsafe_conversion_p to separately warn either about conversions > that lower floating point number precision or about the other > kinds of conversions. > * c-common.h: Adding conversion_safety enumeration. > (unsafe_conversion_p): switching return type to > conversion_safety enumeration. Here, this should be: * c-common.h (enum conversion_safety): New enumeration. (unsafe_conversion_p): switching return type to conversion_safety enumeration. OK to commit with the above changes, if you have your copyright properly assigned to the FSF. Thank for working on this. It is appreciated. -- Dodji
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/4 Richard Biener : > On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich > wrote: >> Hi, >> >> This patch adds support Pointer Bounds Checker into c-family and LTO >> front-ends. The main purpose of changes in front-end is to register all >> statically initialized objects for checker. We also need to register such >> objects created by compiler. LTO is quite specific front-end. For LTO Pointer Bounds Checker support macro just means it allows instrumented code as input because all instrumentation is performed before code is streamed out for LTO. Ilya > > You define CHKP as supported in LTO. That means it has to be supported > for all languages and thus you should drop the langhook. > > Richard. > >> Thanks, >> Ilya >> -- >> >> gcc/ >> >> 2013-10-29 Ilya Enkovich >> >> * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >> * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >> * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >> * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >> * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >> * c/c-parser.c (c_parser_declaration_or_fndef): Register statically >> initialized decls in Pointer Bounds Checker. >> * cp/decl.c (cp_finish_decl): Likewise. >> * gimplify.c (gimplify_init_constructor): Likewise. >> >> >> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c >> index 614c46d..a32bc6b 100644 >> --- a/gcc/c/c-lang.c >> +++ b/gcc/c/c-lang.c >> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c; >> #define LANG_HOOKS_INIT c_objc_common_init >> #undef LANG_HOOKS_INIT_TS >> #define LANG_HOOKS_INIT_TS c_common_init_ts >> +#undef LANG_HOOKS_CHKP_SUPPORTED >> +#define LANG_HOOKS_CHKP_SUPPORTED true >> >> /* Each front end provides its own lang hook initializer. */ >> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER; >> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c >> index 9ccae3b..65d83c8 100644 >> --- a/gcc/c/c-parser.c >> +++ b/gcc/c/c-parser.c >> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser, bool >> fndef_ok, >> maybe_warn_string_init (TREE_TYPE (d), init); >> finish_decl (d, init_loc, init.value, >>init.original_type, asm_name); >> + >> + /* Register all decls with initializers in Pointer >> +Bounds Checker to generate required static bounds >> +initializers. */ >> + if (DECL_INITIAL (d) != error_mark_node) >> + chkp_register_var_initializer (d); >> } >> } >> else >> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c >> index a7fa8e4..6d138bd 100644 >> --- a/gcc/cp/cp-lang.c >> +++ b/gcc/cp/cp-lang.c >> @@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded >> (const_tree); >> #define LANG_HOOKS_EH_PERSONALITY cp_eh_personality >> #undef LANG_HOOKS_EH_RUNTIME_TYPE >> #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type >> +#undef LANG_HOOKS_CHKP_SUPPORTED >> +#define LANG_HOOKS_CHKP_SUPPORTED true >> >> /* Each front end provides its own lang hook initializer. */ >> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER; >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c >> index 1e92f2a..db40e75 100644 >> --- a/gcc/cp/decl.c >> +++ b/gcc/cp/decl.c >> @@ -6379,6 +6379,12 @@ cp_finish_decl (tree decl, tree init, bool >> init_const_expr_p, >> the class specifier. */ >> if (!DECL_EXTERNAL (decl)) >> var_definition_p = true; >> + >> + /* If var has initilizer then we need to register it in >> +Pointer Bounds Checker to generate static bounds initilizer >> +if required. */ >> + if (DECL_INITIAL (decl) && DECL_INITIAL (decl) != error_mark_node) >> + chkp_register_var_initializer (decl); >> } >>/* If the variable has an array type, lay out the type, even if >> there is no initializer. It is valid to index through the >> diff --git a/gcc/gimplify.c b/gcc/gimplify.c >> index 4f52c27..503450f 100644 >> --- a/gcc/gimplify.c >> +++ b/gcc/gimplify.c >> @@ -4111,6 +4111,11 @@ gimplify_init_constructor (tree *expr_p, gimple_seq >> *pre_p, gimple_seq *post_p, >> >> walk_tree (&ctor, force_labels_r, NULL, NULL); >> ctor = tree_output_constant_def (ctor); >> + >> + /* We need to register created constant object to >> + initialize bounds for pointers in it. */ >> + chkp_register_var_initializer (ctor); >> + >> if (!useless_type_conversion_p (type, TREE_TYPE (ctor))) >> ctor = build1 (VIEW_CONVERT_EXPR, type, ctor); >> TREE_OPERAND (*expr_p, 1) = ctor; >> diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c >> index 0fa0fc9..b6073d9 100644 >> --- a/gcc/lto/lto-lang.c >> +++ b/gcc/lto/lto-lang.c >> @@ -1278,6 +
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
On Tue, Nov 05, 2013 at 04:54:38PM +0400, Ilya Enkovich wrote: > 2013/11/2 Joseph S. Myers : > > On Thu, 31 Oct 2013, Ilya Enkovich wrote: > > > >> This patch adds support Pointer Bounds Checker into c-family and LTO > >> front-ends. The main purpose of changes in front-end is to register all > >> statically initialized objects for checker. We also need to register > >> such objects created by compiler. > > > > What happens with statically initialized TLS objects? It would seem that > > you need something like how the C++ front end handles static constructors > > for C++11 thread-local objects, but I don't see anything obvious for that > > here. > > This patch takes care of pointers initialized by linker. TLS objects > are dynamically allocated for each thread and should have some > constructor to perform initialization. And if there is such > constructors then it should be instrumented by Pointer Bounds Checker. > Therefore it should not require additional changes in front-end, > right? > > The only problem I now see for TLS objects is that in my constructors > initialize bounds for an actual TLS object, not for object in .tdata. > How can I refer to .tdata objects in GIMPLE to get correct bounds > initialization for .tdata section? You can't. .tdata section is only used as data to copy to the TLS objects when they are initialized (libc copies the whole .tdata section to the right spot in the TLS are of each thread, followed by clearing the area corresponding to .tbss section's size). TLS objects can have dynamic constructors (fairly recent change), but they don't have to, if they don't, they are just initialized by the memcpy from .tdata section. Jakub
Re: [PATCH i386 4/8] [AVX512] [1/n] Add substed patterns.
Hello,
Small correction.
On 01 Nov 16:19, Kirill Yukhin wrote:
> +(define_insn "avx512f_store_mask"
> + [(set (match_operand:VI48F_512 0 "memory_operand" "=m")
> + (vec_merge:VI48F_512
> + (match_operand:VI48F_512 1 "register_operand" "v")
> + (match_dup 0)
> + (match_operand: 2 "register_operand" "k")))]
> + "TARGET_AVX512F"
> +{
> + switch (mode)
Need to be MODE_. Same for load.
Is it ok with that change?
--
Thanks, K
[patch] move phi_ssa_name_p.
I noticed that gimple.h::phi_ssa_name_p is used in exactly one location,
tree-outof-ssa.c. Its used in an expression, and the name isn't really
very clear.
I renamed it and rolled the rest of the expression into the new
function, and made it static within tree-outof-ssa.c.
Bootstraps on x86_64-unknown-linux-gnu with no new regressions. OK?
Andrew
* tree-outof-ssa.c (queue_phi_copy_p): Combine phi_ssa_name_p from
gimple.h and the rest of the condition in eliminate_build.
(eliminate_build): Call new routine.
* gimple.h (phi_ssa_name_p): Delete.
Index: tree-outof-ssa.c
===
*** tree-outof-ssa.c (revision 204355)
--- tree-outof-ssa.c (working copy)
*** eliminate_name (elim_graph g, int T)
*** 548,553
--- 548,570
elim_graph_add_node (g, T);
}
+ /* Return true if this phi argument T should have a copy queued when using
+var_map MAP. PHI nodes should contain only ssa_names and invariants. A
+test for ssa_name is definitely simpler, but don't let invalid contents
+slip through in the meantime. */
+
+ static inline bool
+ queue_phi_copy_p (var_map map, tree t)
+ {
+ if (TREE_CODE (t) == SSA_NAME)
+ {
+ if (var_to_partition (map, t) == NO_PARTITION)
+ return true;
+ return false;
+ }
+ gcc_checking_assert (is_gimple_min_invariant (t));
+ return true;
+ }
/* Build elimination graph G for basic block BB on incoming PHI edge
G->e. */
*** eliminate_build (elim_graph g)
*** 577,585
/* If this argument is a constant, or a SSA_NAME which is being
left in SSA form, just queue a copy to be emitted on this
edge. */
! if (!phi_ssa_name_p (Ti)
! || (TREE_CODE (Ti) == SSA_NAME
! && var_to_partition (g->map, Ti) == NO_PARTITION))
{
/* Save constant copies until all other copies have been emitted
on this edge. */
--- 594,600
/* If this argument is a constant, or a SSA_NAME which is being
left in SSA form, just queue a copy to be emitted on this
edge. */
! if (queue_phi_copy_p (g->map, Ti))
{
/* Save constant copies until all other copies have been emitted
on this edge. */
Index: gimple.h
===
*** gimple.h (revision 204355)
--- gimple.h (working copy)
*** gimple_phi_set_arg (gimple gs, unsigned
*** 3633,3651
gs->gimple_phi.args[index] = *phiarg;
}
- /* PHI nodes should contain only ssa_names and invariants. A test
-for ssa_name is definitely simpler; don't let invalid contents
-slip in in the meantime. */
-
- static inline bool
- phi_ssa_name_p (const_tree t)
- {
- if (TREE_CODE (t) == SSA_NAME)
- return true;
- gcc_checking_assert (is_gimple_min_invariant (t));
- return false;
- }
-
/* Return the PHI nodes for basic block BB, or NULL if there are no
PHI nodes. */
--- 3633,3638
[PATCH] loop-{iv,unswitch}.c fixes (PR rtl-optimization/58997)
Hi!
Apparently, for iv->extend == IV_UNKNOWN_EXTEND if iv->mode !=
iv->extend_mode get_iv_value returns a value in iv->mode mode, while
otherwise in iv->extend_mode. This makes some sense, because with
IV_UNKNOWN_EXTEND the upper bits are unknown, so there is nothing reasonable
to return in the extended mode. But e.g. iv_subreg when it calls assumed
unconditionally the value would be in iv->extend_mode (the reason for the
ICE on the attached testcase). Furthermore, for iv_extend, IMHO if
iv->extend isn't IV_UNKNOWN_EXTEND, but is different from the new extend,
the middle bits might be incorrect. And, lastly, I think may_unswitch_on
assumes the returned values will be in the expected mode (iv->mode), but
they can be wider and that can result in invalid RTL (say comparing of
SImode with DImode rtl), or wrong code.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk and for 4.8 later?
2013-11-05 Jakub Jelinek
PR rtl-optimization/58997
* loop-iv.c (iv_subreg): For IV_UNKNOWN_EXTEND, expect
get_iv_value to be in iv->mode rather than iv->extend_mode.
(iv_extend): Likewise. Otherwise, if iv->extend != extend,
use lowpart_subreg on get_iv_value before calling simplify_gen_unary.
* loop-unswitch.c (may_unswitch_on): Make sure op[i] is in the right
mode.
* gcc.c-torture/compile/pr58997.c: New test.
--- gcc/loop-iv.c.jj2013-10-17 22:30:56.0 +0200
+++ gcc/loop-iv.c 2013-11-05 10:10:31.685719102 +0100
@@ -436,7 +436,9 @@ iv_subreg (struct rtx_iv *iv, enum machi
&& !iv->first_special)
{
rtx val = get_iv_value (iv, const0_rtx);
- val = lowpart_subreg (mode, val, iv->extend_mode);
+ val = lowpart_subreg (mode, val,
+ iv->extend == IV_UNKNOWN_EXTEND
+ ? iv->mode : iv->extend_mode);
iv->base = val;
iv->extend = IV_UNKNOWN_EXTEND;
@@ -476,8 +478,14 @@ iv_extend (struct rtx_iv *iv, enum iv_ex
&& !iv->first_special)
{
rtx val = get_iv_value (iv, const0_rtx);
+ if (iv->extend_mode != iv->mode
+ && iv->extend != IV_UNKNOWN_EXTEND
+ && iv->extend != extend)
+ val = lowpart_subreg (iv->mode, val, iv->extend_mode);
val = simplify_gen_unary (iv_extend_to_rtx_code (extend), mode,
- val, iv->extend_mode);
+ val,
+ iv->extend == extend
+ ? iv->extend_mode : iv->mode);
iv->base = val;
iv->extend = IV_UNKNOWN_EXTEND;
iv->mode = iv->extend_mode = mode;
--- gcc/loop-unswitch.c.jj 2013-10-17 22:30:41.780251692 +0200
+++ gcc/loop-unswitch.c 2013-11-05 10:28:57.172719881 +0100
@@ -191,6 +191,7 @@ may_unswitch_on (basic_block bb, struct
if (!test)
return NULL_RTX;
+ mode = VOIDmode;
for (i = 0; i < 2; i++)
{
op[i] = XEXP (test, i);
@@ -205,11 +206,15 @@ may_unswitch_on (basic_block bb, struct
return NULL_RTX;
op[i] = get_iv_value (&iv, const0_rtx);
+ if (iv.extend != IV_UNKNOWN_EXTEND
+ && iv.mode != iv.extend_mode)
+ op[i] = lowpart_subreg (iv.mode, op[i], iv.extend_mode);
+ if (mode == VOIDmode)
+ mode = iv.mode;
+ else
+ gcc_assert (mode == iv.mode);
}
- mode = GET_MODE (op[0]);
- if (mode == VOIDmode)
-mode = GET_MODE (op[1]);
if (GET_MODE_CLASS (mode) == MODE_CC)
{
if (at != BB_END (bb))
--- gcc/testsuite/gcc.c-torture/compile/pr58997.c.jj2013-11-05
10:26:30.802517924 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr58997.c 2013-11-05
10:26:15.0 +0100
@@ -0,0 +1,19 @@
+/* PR rtl-optimization/58997 */
+
+int a, b, c, e;
+short d;
+char h;
+
+void
+foo ()
+{
+ while (b)
+{
+ d = a ? c : 1 % a;
+ c = d;
+ h = d;
+ if (!h)
+ while (e)
+ ;
+}
+}
Jakub
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich wrote:
> 2013/11/5 Richard Biener :
>> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich wrote:
>>> 2013/11/4 Richard Biener :
Richard Biener wrote:
>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
> wrote:
>> Hi,
>>
>> Here is a patch which hadles the problem with optimization of
>BUILT_IN_CHKP_ARG_BND calls. Pointer Bounds Checker expects that
>argument of this call is a default SSA_NAME of the PARM_DECL whose
>bounds we want to get. The problem is in optimizations which may
>replace arg with it's copy or a known value. This patch suppress such
>modifications.
>
>This doesn't seem like a good fix. I suppose you require the same on
>RTL, that is, have the incoming arg reg coalesced with the use?
>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
Btw, I would have chosen
P_2 = __builtin_xyz (p_1, bound)
Call (p_2)
Thus make the builtins a transparent pass-through which effectively binds
parameter to their bound, removing the need for the artificial arguments
and making propagations a non.issue
Also, how does the feature interact with other extensions such as nested
functions or optimizations like inlining?
>>>
>>> In RTL all incoming bounds are materialized into slot where bounds are
>>> passed and arg_bnd call is expanded into this slot. Thus in RTL bound
>>> arg looks more like a regular arg.
>>
>> I don't care so much for RTL, but this description hints at that the
>> suggestion above would work (and it would eliminate all my concerns about
>> the representation on the GIMPLE level - you'd not even need this
>> strange POINTER_BOUNDS_TYPE as far as I can see.
>>
>>> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
>>> because it does not used in abnormal phi, shouldn't it? Also it would
>>> prevent all optimizations for these SSA_NAMEs right? Instrumentation
>>> is performed on the earlier stage, right after we build SSA. I think
>>> using abnormal phi flag and binding pointers with bounds via calls
>>> would prevent some useful optimizations.
>>
>> Well, what are the constraints that you need to avoid propagation in
>> the first place?
>
> For input parameter P I need to have
> BOUNDS = __builtin_arg_bnd (P)
> to somehow refer to bounds of P in GIMPLE. Optimizations may modify
> __builtin_arg_bnd (P) replacing P with its copy or some value. It
> makes call useless because removes information about parameter whose
> bounds we refer to. I want such optimization to ignore
> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
> there as arg.
How does a compilable testcase look like that shows how the default def
is used? And what transforms break that use? I really cannot see
how this would happen (and you didn't add a testcase).
Richard.
>>
>>> Many interprocedural optimizations require some support when work with
>>> instrumented calls. Inlining support includes:
>>> - replacement of arg_bnd calls with actual bounds passed to the inlined
>>> call
>>> - replacement of retbnd call with bounds, returned by inlined function
>>>
>>> Not all IPA passes are fully enabled right now. E.g. I restrict
>>> bounded value propagation in ipa-prop and bounded args in functions
>>> generated by ipa-split. Such features will be enabled later.
>>>
>>> For nested functions I do not see much difference from checker point
>>> of view. It just has an additional static chain param. Probably I
>>> miss here something. I did just few tests with nested functions.
>>
>> I was thinking of
>>
>> int foo (char *s)
>> {
>>int bar (void)
>>{
>> ... use bound of 's' of the containing function ...
>> foo (q, and pass it along here for q)
>>}
>> }
>>
>> that is references to the containing functions parameters and their
>
> All foo's locals referenced by nested bar here are placed into special
> structure and all bounds should be stored in appropriated entries of
> Bound Table. Nested function may refer to them via this Table.
>
> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>
Richard.
>Richard.
>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-28 Ilya Enkovich
>>
>> * tree-into-ssa.c: Include "target.h"
>> (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>> * tree-ssa-dom.c: Include "target.h"
>> (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>
>>
>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>> index 981e9f4..8d48f6d 100644
>> --- a/gcc/tree-into-ssa.c
>> +++ b/gcc/tree-into-ssa.c
>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "params.h"
>> #include "diagnostic-core.h"
>> #include "tree-into-ssa.h"
>> +#include "target.h"
>>
>>
>> /* This file builds t
Re: [PATCH, SH] Implement builtin_strlen
Christian Bruel wrote: > No regressions for sh-none-elf. OK for trunk ? OK. Regards, kaz
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/5 Jakub Jelinek : > On Tue, Nov 05, 2013 at 04:54:38PM +0400, Ilya Enkovich wrote: >> 2013/11/2 Joseph S. Myers : >> > On Thu, 31 Oct 2013, Ilya Enkovich wrote: >> > >> >> This patch adds support Pointer Bounds Checker into c-family and LTO >> >> front-ends. The main purpose of changes in front-end is to register all >> >> statically initialized objects for checker. We also need to register >> >> such objects created by compiler. >> > >> > What happens with statically initialized TLS objects? It would seem that >> > you need something like how the C++ front end handles static constructors >> > for C++11 thread-local objects, but I don't see anything obvious for that >> > here. >> >> This patch takes care of pointers initialized by linker. TLS objects >> are dynamically allocated for each thread and should have some >> constructor to perform initialization. And if there is such >> constructors then it should be instrumented by Pointer Bounds Checker. >> Therefore it should not require additional changes in front-end, >> right? >> >> The only problem I now see for TLS objects is that in my constructors >> initialize bounds for an actual TLS object, not for object in .tdata. >> How can I refer to .tdata objects in GIMPLE to get correct bounds >> initialization for .tdata section? > > You can't. .tdata section is only used as data to copy to the TLS objects > when they are initialized (libc copies the whole .tdata section to the right > spot in the TLS are of each thread, followed by clearing the area > corresponding to .tbss section's size). TLS objects can have dynamic > constructors (fairly recent change), but they don't have to, if they don't, > they are just initialized by the memcpy from .tdata section. It's sad I cannot just initialize bounds in Bounds Tables for .tdata, memcpy would do all the job for me copying them for each new TLS instance. The other way is to move all bounds initialization for TLS objects into separate constructor. How can I make such constructor? > > Jakub
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich wrote: > 2013/11/4 Richard Biener : >> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich >> wrote: >>> Hi, >>> >>> This patch adds support Pointer Bounds Checker into c-family and LTO >>> front-ends. The main purpose of changes in front-end is to register all >>> statically initialized objects for checker. We also need to register such >>> objects created by compiler. > > LTO is quite specific front-end. For LTO Pointer Bounds Checker > support macro just means it allows instrumented code as input because > all instrumentation is performed before code is streamed out for LTO. But your patch doesn't even make use of the langhook... Richard. > Ilya > >> >> You define CHKP as supported in LTO. That means it has to be supported >> for all languages and thus you should drop the langhook. >> >> Richard. >> >>> Thanks, >>> Ilya >>> -- >>> >>> gcc/ >>> >>> 2013-10-29 Ilya Enkovich >>> >>> * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >>> * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >>> * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >>> * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >>> * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New. >>> * c/c-parser.c (c_parser_declaration_or_fndef): Register statically >>> initialized decls in Pointer Bounds Checker. >>> * cp/decl.c (cp_finish_decl): Likewise. >>> * gimplify.c (gimplify_init_constructor): Likewise. >>> >>> >>> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c >>> index 614c46d..a32bc6b 100644 >>> --- a/gcc/c/c-lang.c >>> +++ b/gcc/c/c-lang.c >>> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c; >>> #define LANG_HOOKS_INIT c_objc_common_init >>> #undef LANG_HOOKS_INIT_TS >>> #define LANG_HOOKS_INIT_TS c_common_init_ts >>> +#undef LANG_HOOKS_CHKP_SUPPORTED >>> +#define LANG_HOOKS_CHKP_SUPPORTED true >>> >>> /* Each front end provides its own lang hook initializer. */ >>> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER; >>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c >>> index 9ccae3b..65d83c8 100644 >>> --- a/gcc/c/c-parser.c >>> +++ b/gcc/c/c-parser.c >>> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser, >>> bool fndef_ok, >>> maybe_warn_string_init (TREE_TYPE (d), init); >>> finish_decl (d, init_loc, init.value, >>>init.original_type, asm_name); >>> + >>> + /* Register all decls with initializers in Pointer >>> +Bounds Checker to generate required static bounds >>> +initializers. */ >>> + if (DECL_INITIAL (d) != error_mark_node) >>> + chkp_register_var_initializer (d); >>> } >>> } >>> else >>> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c >>> index a7fa8e4..6d138bd 100644 >>> --- a/gcc/cp/cp-lang.c >>> +++ b/gcc/cp/cp-lang.c >>> @@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded >>> (const_tree); >>> #define LANG_HOOKS_EH_PERSONALITY cp_eh_personality >>> #undef LANG_HOOKS_EH_RUNTIME_TYPE >>> #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type >>> +#undef LANG_HOOKS_CHKP_SUPPORTED >>> +#define LANG_HOOKS_CHKP_SUPPORTED true >>> >>> /* Each front end provides its own lang hook initializer. */ >>> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER; >>> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c >>> index 1e92f2a..db40e75 100644 >>> --- a/gcc/cp/decl.c >>> +++ b/gcc/cp/decl.c >>> @@ -6379,6 +6379,12 @@ cp_finish_decl (tree decl, tree init, bool >>> init_const_expr_p, >>> the class specifier. */ >>> if (!DECL_EXTERNAL (decl)) >>> var_definition_p = true; >>> + >>> + /* If var has initilizer then we need to register it in >>> +Pointer Bounds Checker to generate static bounds initilizer >>> +if required. */ >>> + if (DECL_INITIAL (decl) && DECL_INITIAL (decl) != error_mark_node) >>> + chkp_register_var_initializer (decl); >>> } >>>/* If the variable has an array type, lay out the type, even if >>> there is no initializer. It is valid to index through the >>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c >>> index 4f52c27..503450f 100644 >>> --- a/gcc/gimplify.c >>> +++ b/gcc/gimplify.c >>> @@ -4111,6 +4111,11 @@ gimplify_init_constructor (tree *expr_p, gimple_seq >>> *pre_p, gimple_seq *post_p, >>> >>> walk_tree (&ctor, force_labels_r, NULL, NULL); >>> ctor = tree_output_constant_def (ctor); >>> + >>> + /* We need to register created constant object to >>> + initialize bounds for pointers in it. */ >>> + chkp_register_var_initializer (ctor); >>> + >>> if (!useless_type_conversion_p (type, TREE_TYPE (ctor))) >>> ctor = build1 (
Re: [patch] move phi_ssa_name_p.
On Tue, Nov 5, 2013 at 2:06 PM, Andrew MacLeod wrote: > I noticed that gimple.h::phi_ssa_name_p is used in exactly one location, > tree-outof-ssa.c. Its used in an expression, and the name isn't really very > clear. > > I renamed it and rolled the rest of the expression into the new function, > and made it static within tree-outof-ssa.c. > > Bootstraps on x86_64-unknown-linux-gnu with no new regressions. OK? Ok. Thanks, Richard. > Andrew >
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
On Tue, Nov 05, 2013 at 05:13:12PM +0400, Ilya Enkovich wrote: > It's sad I cannot just initialize bounds in Bounds Tables for .tdata, > memcpy would do all the job for me copying them for each new TLS Why do you think so? glibc will surely use an internal version of memcpy rather than the global one, so unless glibc is MPX enabled, it would not do the job. > instance. The other way is to move all bounds initialization for TLS > objects into separate constructor. How can I make such constructor? Just create a constructor for those, similarly how the C++ FE for __thread vars with constructors (or OpenMP lowering for #pragma omp threadprivate vars with constructors) expands them. Jakub
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich wrote:
>> 2013/11/4 Richard Biener :
>>> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich
>>> wrote:
Hi,
This patch adds support Pointer Bounds Checker into c-family and LTO
front-ends. The main purpose of changes in front-end is to register all
statically initialized objects for checker. We also need to register such
objects created by compiler.
>>
>> LTO is quite specific front-end. For LTO Pointer Bounds Checker
>> support macro just means it allows instrumented code as input because
>> all instrumentation is performed before code is streamed out for LTO.
>
> But your patch doesn't even make use of the langhook...
Use of langhook is in previous patch
(http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
it's part:
diff --git a/gcc/toplev.c b/gcc/toplev.c
index db269b7..0eaf081 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1282,6 +1282,15 @@ process_options (void)
"and -ftree-loop-linear)");
#endif
+ if (flag_check_pointer_bounds)
+{
+ if (targetm.chkp_bound_mode () == VOIDmode)
+ error ("-fcheck-pointers is not supported for this target");
+
+ if (!lang_hooks.chkp_supported)
+ flag_check_pointer_bounds = 0;
+}
+
/* One region RA really helps to decrease the code size. */
if (flag_ira_region == IRA_REGION_AUTODETECT)
flag_ira_region
If we try to use -fcheck-pointers -flto for some unsupported language,
code will not be instrumented.
Ilya
>
> Richard.
>
>> Ilya
>>
>>>
>>> You define CHKP as supported in LTO. That means it has to be supported
>>> for all languages and thus you should drop the langhook.
>>>
>>> Richard.
>>>
Thanks,
Ilya
--
gcc/
2013-10-29 Ilya Enkovich
* c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* c/c-parser.c (c_parser_declaration_or_fndef): Register statically
initialized decls in Pointer Bounds Checker.
* cp/decl.c (cp_finish_decl): Likewise.
* gimplify.c (gimplify_init_constructor): Likewise.
diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
index 614c46d..a32bc6b 100644
--- a/gcc/c/c-lang.c
+++ b/gcc/c/c-lang.c
@@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
#define LANG_HOOKS_INIT c_objc_common_init
#undef LANG_HOOKS_INIT_TS
#define LANG_HOOKS_INIT_TS c_common_init_ts
+#undef LANG_HOOKS_CHKP_SUPPORTED
+#define LANG_HOOKS_CHKP_SUPPORTED true
/* Each front end provides its own lang hook initializer. */
struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 9ccae3b..65d83c8 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser,
bool fndef_ok,
maybe_warn_string_init (TREE_TYPE (d), init);
finish_decl (d, init_loc, init.value,
init.original_type, asm_name);
+
+ /* Register all decls with initializers in Pointer
+Bounds Checker to generate required static bounds
+initializers. */
+ if (DECL_INITIAL (d) != error_mark_node)
+ chkp_register_var_initializer (d);
}
}
else
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index a7fa8e4..6d138bd 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded
(const_tree);
#define LANG_HOOKS_EH_PERSONALITY cp_eh_personality
#undef LANG_HOOKS_EH_RUNTIME_TYPE
#define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
+#undef LANG_HOOKS_CHKP_SUPPORTED
+#define LANG_HOOKS_CHKP_SUPPORTED true
/* Each front end provides its own lang hook initializer. */
struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 1e92f2a..db40e75 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6379,6 +6379,12 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
the class specifier. */
if (!DECL_EXTERNAL (decl))
var_definition_p = true;
+
+ /* If var has initilizer then we need to register it in
+Pointer Bounds Checker to generate static bounds initilizer
+if required. */
+ if (DECL_INITIAL (decl) && D
[PATCH] Fix PR58941
The following fixes PR58941 - we fail to detect a trailing array
access in get_ref_base_and_extent because, while we remember
the type to check, we still apply an extra offset that confuses us.
Fixed by handling trailing arrays at places we view-convert.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
sofar.
Richard.
2013-11-05 Richard Biener
PR middle-end/58941
* tree-dfa.c (get_ref_base_and_extent): Merge common code
in MEM_REF and TARGET_MEM_REF handling. Make sure to
process trailing array detection before diving into the
view-converted object (and possibly apply some extra offset).
* gcc.dg/torture/pr58941.c: New testcase.
Index: gcc/tree-dfa.c
===
*** gcc/tree-dfa.c (revision 204346)
--- gcc/tree-dfa.c (working copy)
*** get_ref_base_and_extent (tree exp, HOST_
*** 389,395
double_int bit_offset = double_int_zero;
HOST_WIDE_INT hbit_offset;
bool seen_variable_array_ref = false;
- tree base_type;
/* First get the final access size from just the outermost expression. */
if (TREE_CODE (exp) == COMPONENT_REF)
--- 389,394
*** get_ref_base_and_extent (tree exp, HOST_
*** 420,427
and find the ultimate containing object. */
while (1)
{
- base_type = TREE_TYPE (exp);
-
switch (TREE_CODE (exp))
{
case BIT_FIELD_REF:
--- 419,424
*** get_ref_base_and_extent (tree exp, HOST_
*** 544,550
--- 541,578
case VIEW_CONVERT_EXPR:
break;
+ case TARGET_MEM_REF:
+ /* Via the variable index or index2 we can reach the
+whole object. Still hand back the decl here. */
+ if (TREE_CODE (TMR_BASE (exp)) == ADDR_EXPR
+ && (TMR_INDEX (exp) || TMR_INDEX2 (exp)))
+ {
+ exp = TREE_OPERAND (TMR_BASE (exp), 0);
+ bit_offset = double_int_zero;
+ maxsize = -1;
+ goto done;
+ }
+ /* Fallthru. */
case MEM_REF:
+ /* We need to deal with variable arrays ending structures such as
+struct { int length; int a[1]; } x; x.a[d]
+struct { struct { int a; int b; } a[1]; } x; x.a[d].a
+struct { struct { int a[1]; } a[1]; } x; x.a[0][d], x.a[d][0]
+struct { int len; union { int a[1]; struct X x; } u; } x; x.u.a[d]
+where we do not know maxsize for variable index accesses to
+the array. The simplest way to conservatively deal with this
+is to punt in the case that offset + maxsize reaches the
+base type boundary. This needs to include possible trailing
+padding that is there for alignment purposes. */
+ if (seen_variable_array_ref
+ && maxsize != -1
+ && (!bit_offset.fits_shwi ()
+ || !host_integerp (TYPE_SIZE (TREE_TYPE (exp)), 1)
+ || (bit_offset.to_shwi () + maxsize
+ == (signed) TREE_INT_CST_LOW
+ (TYPE_SIZE (TREE_TYPE (exp))
+ maxsize = -1;
+
/* Hand back the decl for MEM[&decl, off]. */
if (TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR)
{
*** get_ref_base_and_extent (tree exp, HOST_
*** 555,595
double_int off = mem_ref_offset (exp);
off = off.lshift (BITS_PER_UNIT == 8
? 3 : exact_log2 (BITS_PER_UNIT));
- off = off + bit_offset;
- if (off.fits_shwi ())
- {
- bit_offset = off;
- exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
- }
- }
- }
- goto done;
-
- case TARGET_MEM_REF:
- /* Hand back the decl for MEM[&decl, off]. */
- if (TREE_CODE (TMR_BASE (exp)) == ADDR_EXPR)
- {
- /* Via the variable index or index2 we can reach the
-whole object. */
- if (TMR_INDEX (exp) || TMR_INDEX2 (exp))
- {
- exp = TREE_OPERAND (TMR_BASE (exp), 0);
- bit_offset = double_int_zero;
- maxsize = -1;
- goto done;
- }
- if (integer_zerop (TMR_OFFSET (exp)))
- exp = TREE_OPERAND (TMR_BASE (exp), 0);
- else
- {
- double_int off = mem_ref_offset (exp);
- off = off.lshift (BITS_PER_UNIT == 8
- ? 3 : exact_log2 (BITS_PER_UNIT));
off += bit_offset;
if (off.fits_shwi ())
{
bit_offset = off;
! exp = TREE_OPERAND (TMR_BASE (exp), 0);
Re: libsanitizer merge from upstream r191666
Another problem is internal_syscall casts pointers to uint64. It doesn't work for x32 since 32-bit pointers are signed extended to int64. We can need to cast pointer to uptr first. On Tue, Nov 5, 2013 at 3:23 AM, H.J. Lu wrote: > It is a different failure. There are 3 problems: > > 1. Hard coded 64-bit registers for pointers in x86-64 asm statements. > 2. Treat WORD_SIZE == 32 as ia32 for machine state on x86-64. > 3. On x86-64, assume ia32 FS system calls when LP64 is false. > > H.J. > On Mon, Nov 4, 2013 at 10:35 PM, Konstantin Serebryany > wrote: >> Is this the same failure or different? >> >> On Mon, Nov 4, 2013 at 9:49 PM, H.J. Lu wrote: >>> It also breaks x32 build. >>> >>> >>> On Mon, Nov 4, 2013 at 5:48 PM, Konstantin Serebryany >>> wrote: Hi Peter. Does this also mean that asan in llvm trunk is broken for Power? We'll need to fix it there too (or, in fact, first). --kcc On Mon, Nov 4, 2013 at 4:33 PM, Peter Bergner wrote: > On Mon, 2013-11-04 at 06:47 -0800, Konstantin Serebryany wrote: >> This patch has not been tested on Mac and we generally do not claim >> that gcc-asan is supported on Mac. >> clang-asan *is* supported on Mac and our bots are green (this patch is >> a merge of the sources which are regularly tested on Mac, >> but the build procedure is different). > > The merge from upstream r191666 seems to have broken my > powerpc64-linux build. I'm seeing: > > In file included from > /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:21:0: > /usr/include/asm/stat.h:31:2: error: ‘ino_t’ does not name a type > ino_t st_ino; > ^ > /usr/include/asm/stat.h:33:2: error: ‘nlink_t’ does not name a type > nlink_t st_nlink; > ^ > /usr/include/asm/stat.h:34:2: error: ‘mode_t’ does not name a type > mode_t st_mode; > ^ > /usr/include/asm/stat.h:39:2: error: ‘uid_t’ does not name a type > uid_t st_uid; > ^ > /usr/include/asm/stat.h:40:2: error: ‘gid_t’ does not name a type > gid_t st_gid; > ^ > /usr/include/asm/stat.h:42:2: error: ‘off_t’ does not name a type > off_t st_size; > ^ > /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:29:73: > error: invalid application of ‘sizeof’ to incomplete type > ‘__sanitizer::__old_kernel_stat’ >unsigned struct___old_kernel_stat_sz = sizeof(struct > __old_kernel_stat); > ^ > The problem seems to be that the RHEL6 system I am on, the linux/types.h > kernel header file doesn't define ino_t like it does on newer systems > which leads to the undefined type errors. Digging through the other > kernel > header files, I'm not really seeing another header file I can include > to get it either. :( > > Peter > > >>> >>> >>> >>> -- >>> H.J. > > > > -- > H.J. -- H.J.
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/5 Jakub Jelinek : > On Tue, Nov 05, 2013 at 05:13:12PM +0400, Ilya Enkovich wrote: >> It's sad I cannot just initialize bounds in Bounds Tables for .tdata, >> memcpy would do all the job for me copying them for each new TLS > > Why do you think so? glibc will surely use an internal version of > memcpy rather than the global one, so unless glibc is MPX enabled, it would > not do the job. > >> instance. The other way is to move all bounds initialization for TLS >> objects into separate constructor. How can I make such constructor? > > Just create a constructor for those, similarly how the C++ FE for __thread > vars with constructors (or OpenMP lowering for #pragma omp threadprivate > vars with constructors) expands them. Will look into it. Thanks! Ilya > > Jakub
Re: libsanitizer merge from upstream r191666
On Tue, Nov 5, 2013 at 2:24 PM, H.J. Lu wrote:
> Another problem is internal_syscall casts pointers to
> uint64. It doesn't work for x32 since 32-bit pointers are
> signed extended to int64. We can need to cast pointer
> to uptr first.
Another issue is that it breaks with 2.6.32 kernel headers on x86_64:
219:24: warning: missing initializer for member
'kernel_timeval::tv_sec' [-Wmissing-field-initializers]
kernel_timeval tv = {};
and thus bootstrap breaks for me.
Richard.
> On Tue, Nov 5, 2013 at 3:23 AM, H.J. Lu wrote:
>> It is a different failure. There are 3 problems:
>>
>> 1. Hard coded 64-bit registers for pointers in x86-64 asm statements.
>> 2. Treat WORD_SIZE == 32 as ia32 for machine state on x86-64.
>> 3. On x86-64, assume ia32 FS system calls when LP64 is false.
>>
>> H.J.
>> On Mon, Nov 4, 2013 at 10:35 PM, Konstantin Serebryany
>> wrote:
>>> Is this the same failure or different?
>>>
>>> On Mon, Nov 4, 2013 at 9:49 PM, H.J. Lu wrote:
It also breaks x32 build.
On Mon, Nov 4, 2013 at 5:48 PM, Konstantin Serebryany
wrote:
> Hi Peter.
> Does this also mean that asan in llvm trunk is broken for Power?
> We'll need to fix it there too (or, in fact, first).
>
> --kcc
>
> On Mon, Nov 4, 2013 at 4:33 PM, Peter Bergner
> wrote:
>> On Mon, 2013-11-04 at 06:47 -0800, Konstantin Serebryany wrote:
>>> This patch has not been tested on Mac and we generally do not claim
>>> that gcc-asan is supported on Mac.
>>> clang-asan *is* supported on Mac and our bots are green (this patch is
>>> a merge of the sources which are regularly tested on Mac,
>>> but the build procedure is different).
>>
>> The merge from upstream r191666 seems to have broken my
>> powerpc64-linux build. I'm seeing:
>>
>> In file included from
>> /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:21:0:
>> /usr/include/asm/stat.h:31:2: error: ‘ino_t’ does not name a type
>> ino_t st_ino;
>> ^
>> /usr/include/asm/stat.h:33:2: error: ‘nlink_t’ does not name a type
>> nlink_t st_nlink;
>> ^
>> /usr/include/asm/stat.h:34:2: error: ‘mode_t’ does not name a type
>> mode_t st_mode;
>> ^
>> /usr/include/asm/stat.h:39:2: error: ‘uid_t’ does not name a type
>> uid_t st_uid;
>> ^
>> /usr/include/asm/stat.h:40:2: error: ‘gid_t’ does not name a type
>> gid_t st_gid;
>> ^
>> /usr/include/asm/stat.h:42:2: error: ‘off_t’ does not name a type
>> off_t st_size;
>> ^
>> /home/bergner/gcc/gcc-fsf-mainline-base/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:29:73:
>> error: invalid application of ‘sizeof’ to incomplete type
>> ‘__sanitizer::__old_kernel_stat’
>>unsigned struct___old_kernel_stat_sz = sizeof(struct
>> __old_kernel_stat);
>>
>> ^
>> The problem seems to be that the RHEL6 system I am on, the linux/types.h
>> kernel header file doesn't define ino_t like it does on newer systems
>> which leads to the undefined type errors. Digging through the other
>> kernel
>> header files, I'm not really seeing another header file I can include
>> to get it either. :(
>>
>> Peter
>>
>>
--
H.J.
>>
>>
>>
>> --
>> H.J.
>
>
>
> --
> H.J.
Re: Fix __GCC_IEC_559 determination for 32-bit x86
On Tue, 5 Nov 2013, Jakub Jelinek wrote: > Besides: > +FAIL: gcc.dg/iec-559-macros-9.c (test for excess errors) > > that this patch supposedly fixes on i686-linux I'm also seeing: > > +FAIL: gcc.dg/pch/save-temps-1.c -O0 -g -I. -Dwith_PCH (test for excess > errors) I see no obvious reason for PCH issues connected with my patch or this fix - I'd expect PCH to care only at most about whether the predefined macros are consistent between different compilations for which they should be consistent, not whether they are correct - but I suppose that if whether flag_excess_precision was initialized at the relevant time depended on compiler options, that could have resulted in an inconsistency affecting PCH. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich wrote:
> 2013/11/5 Richard Biener :
>> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich wrote:
>>> 2013/11/4 Richard Biener :
On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich
wrote:
> Hi,
>
> This patch adds support Pointer Bounds Checker into c-family and LTO
> front-ends. The main purpose of changes in front-end is to register all
> statically initialized objects for checker. We also need to register
> such objects created by compiler.
>>>
>>> LTO is quite specific front-end. For LTO Pointer Bounds Checker
>>> support macro just means it allows instrumented code as input because
>>> all instrumentation is performed before code is streamed out for LTO.
>>
>> But your patch doesn't even make use of the langhook...
>
> Use of langhook is in previous patch
> (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
> it's part:
>
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index db269b7..0eaf081 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1282,6 +1282,15 @@ process_options (void)
>"and -ftree-loop-linear)");
> #endif
>
> + if (flag_check_pointer_bounds)
> +{
> + if (targetm.chkp_bound_mode () == VOIDmode)
> + error ("-fcheck-pointers is not supported for this target");
> +
> + if (!lang_hooks.chkp_supported)
> + flag_check_pointer_bounds = 0;
> +}
> +
>/* One region RA really helps to decrease the code size. */
>if (flag_ira_region == IRA_REGION_AUTODETECT)
> flag_ira_region
>
> If we try to use -fcheck-pointers -flto for some unsupported language,
> code will not be instrumented.
What's the reason to have unsupported languages?
Richard.
> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>
You define CHKP as supported in LTO. That means it has to be supported
for all languages and thus you should drop the langhook.
Richard.
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-29 Ilya Enkovich
>
> * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
> * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
> * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
> * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
> * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
> * c/c-parser.c (c_parser_declaration_or_fndef): Register
> statically
> initialized decls in Pointer Bounds Checker.
> * cp/decl.c (cp_finish_decl): Likewise.
> * gimplify.c (gimplify_init_constructor): Likewise.
>
>
> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
> index 614c46d..a32bc6b 100644
> --- a/gcc/c/c-lang.c
> +++ b/gcc/c/c-lang.c
> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
> #define LANG_HOOKS_INIT c_objc_common_init
> #undef LANG_HOOKS_INIT_TS
> #define LANG_HOOKS_INIT_TS c_common_init_ts
> +#undef LANG_HOOKS_CHKP_SUPPORTED
> +#define LANG_HOOKS_CHKP_SUPPORTED true
>
> /* Each front end provides its own lang hook initializer. */
> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index 9ccae3b..65d83c8 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser,
> bool fndef_ok,
> maybe_warn_string_init (TREE_TYPE (d), init);
> finish_decl (d, init_loc, init.value,
>init.original_type, asm_name);
> +
> + /* Register all decls with initializers in Pointer
> +Bounds Checker to generate required static bounds
> +initializers. */
> + if (DECL_INITIAL (d) != error_mark_node)
> + chkp_register_var_initializer (d);
> }
> }
> else
> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
> index a7fa8e4..6d138bd 100644
> --- a/gcc/cp/cp-lang.c
> +++ b/gcc/cp/cp-lang.c
> @@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded
> (const_tree);
> #define LANG_HOOKS_EH_PERSONALITY cp_eh_personality
> #undef LANG_HOOKS_EH_RUNTIME_TYPE
> #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
> +#undef LANG_HOOKS_CHKP_SUPPORTED
> +#define LANG_HOOKS_CHKP_SUPPORTED true
>
> /* Each front end provides its own lang hook initializer. */
> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 1e92f2a..db40e75 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6379,6 +6379,12 @@ cp_finish_decl (tree decl, tree init, bool
> init_const_expr_p,
> the class specifier. */
> if (!DECL_EXTERNAL (decl))
Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification
2013/11/4 Richard Biener :
> On Thu, Oct 31, 2013 at 10:24 AM, Ilya Enkovich
> wrote:
>> Hi,
>>
>> Here is a patch to support of instrumented code in calls verifiers and calls
>> copy with skipped args.
>>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-29 Ilya Enkovich
>>
>> * cgraph.c (gimple_check_call_args): Handle bound args.
>> * gimple.c (gimple_call_copy_skip_args): Likewise.
>> (validate_call): Likewise.
>>
>>
>> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
>> index 52d9ab0..9d7ae85 100644
>> --- a/gcc/cgraph.c
>> +++ b/gcc/cgraph.c
>> @@ -3030,40 +3030,54 @@ gimple_check_call_args (gimple stmt, tree fndecl,
>> bool args_count_match)
>> {
>>for (i = 0, p = DECL_ARGUMENTS (fndecl);
>>i < nargs;
>> - i++, p = DECL_CHAIN (p))
>> + i++)
>> {
>> - tree arg;
>> + tree arg = gimple_call_arg (stmt, i);
>> +
>> + /* Skip bound args inserted by Pointer Bounds Checker. */
>> + if (POINTER_BOUNDS_P (arg))
>> + continue;
>> +
>> /* We cannot distinguish a varargs function from the case
>> of excess parameters, still deferring the inlining decision
>> to the callee is possible. */
>> if (!p)
>> break;
>> - arg = gimple_call_arg (stmt, i);
>> +
>> if (p == error_mark_node
>> || arg == error_mark_node
>> || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
>> && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>> return false;
>> +
>> + p = DECL_CHAIN (p);
>> }
>>if (args_count_match && p)
>> return false;
>> }
>>else if (parms)
>> {
>> - for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
>> + for (i = 0, p = parms; i < nargs; i++)
>> {
>> - tree arg;
>> + tree arg = gimple_call_arg (stmt, i);
>> +
>> + /* Skip bound args inserted by Pointer Bounds Checker. */
>> + if (POINTER_BOUNDS_P (arg))
>> + continue;
>> +
>> /* If this is a varargs function defer inlining decision
>> to callee. */
>> if (!p)
>> break;
>> - arg = gimple_call_arg (stmt, i);
>> +
>> if (TREE_VALUE (p) == error_mark_node
>> || arg == error_mark_node
>> || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
>> || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
>> && !fold_convertible_p (TREE_VALUE (p), arg)))
>> return false;
>> +
>> + p = TREE_CHAIN (p);
>> }
>> }
>>else
>> diff --git a/gcc/gimple.c b/gcc/gimple.c
>> index 20f6010..dc85bf8 100644
>> --- a/gcc/gimple.c
>> +++ b/gcc/gimple.c
>> @@ -3048,15 +3048,20 @@ canonicalize_cond_expr_cond (tree t)
>> gimple
>> gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
>> {
>> - int i;
>> + int i, bit;
>>int nargs = gimple_call_num_args (stmt);
>>vec vargs;
>>vargs.create (nargs);
>>gimple new_stmt;
>>
>> - for (i = 0; i < nargs; i++)
>> -if (!bitmap_bit_p (args_to_skip, i))
>> - vargs.quick_push (gimple_call_arg (stmt, i));
>> + for (i = 0, bit = 0; i < nargs; i++, bit++)
>> + if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
>> + {
>> + if (!bitmap_bit_p (args_to_skip, --bit))
>> + vargs.quick_push (gimple_call_arg (stmt, i));
>> + }
>> + else if (!bitmap_bit_p (args_to_skip, bit))
>> + vargs.quick_push (gimple_call_arg (stmt, i));
>
> The new code is completely confusing. You need to update
> comments with what bits args_to_skip refers to and what happens
> with pointer bounds. I suppose the bitmap also contains
> bound parameters as they are in calls (but not in fndecls -- I think
> this discrepancy still is a way to desaster).
args_to_skip here does not hold bits for bounds. It's content is equal
for both instrumented and original call. Will make it explicit in
comments.
>
>>if (gimple_call_internal_p (stmt))
>> new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn
>> (stmt),
>> @@ -3702,6 +3707,9 @@ validate_call (gimple stmt, tree fndecl)
>>if (!targs)
>> return true;
>>tree arg = gimple_call_arg (stmt, i);
>> + /* Skip bounds. */
>> + if (flag_check_pointer_bounds && POINTER_BOUNDS_P (arg))
>> + continue;
>
> Why check flag_check_pointer_bounds here but not elsewhere?
No reason actually. I'll remove flag check here.
>
>>if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
>> && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
>> ;
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich wrote:
>> 2013/11/5 Richard Biener :
>>> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich
>>> wrote:
2013/11/4 Richard Biener :
> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich
> wrote:
>> Hi,
>>
>> This patch adds support Pointer Bounds Checker into c-family and LTO
>> front-ends. The main purpose of changes in front-end is to register all
>> statically initialized objects for checker. We also need to register
>> such objects created by compiler.
LTO is quite specific front-end. For LTO Pointer Bounds Checker
support macro just means it allows instrumented code as input because
all instrumentation is performed before code is streamed out for LTO.
>>>
>>> But your patch doesn't even make use of the langhook...
>>
>> Use of langhook is in previous patch
>> (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
>> it's part:
>>
>> diff --git a/gcc/toplev.c b/gcc/toplev.c
>> index db269b7..0eaf081 100644
>> --- a/gcc/toplev.c
>> +++ b/gcc/toplev.c
>> @@ -1282,6 +1282,15 @@ process_options (void)
>>"and -ftree-loop-linear)");
>> #endif
>>
>> + if (flag_check_pointer_bounds)
>> +{
>> + if (targetm.chkp_bound_mode () == VOIDmode)
>> + error ("-fcheck-pointers is not supported for this target");
>> +
>> + if (!lang_hooks.chkp_supported)
>> + flag_check_pointer_bounds = 0;
>> +}
>> +
>>/* One region RA really helps to decrease the code size. */
>>if (flag_ira_region == IRA_REGION_AUTODETECT)
>> flag_ira_region
>>
>> If we try to use -fcheck-pointers -flto for some unsupported language,
>> code will not be instrumented.
>
> What's the reason to have unsupported languages?
For some languages (e.g. Java) Pointer Bounds Checker does not make
sense at all. Others may require additional support in front-end. The
primary target is C family where solved problem is more critical.
Ilya
>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
Ilya
>
> You define CHKP as supported in LTO. That means it has to be supported
> for all languages and thus you should drop the langhook.
>
> Richard.
>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-29 Ilya Enkovich
>>
>> * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>> * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>> * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>> * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>> * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>> * c/c-parser.c (c_parser_declaration_or_fndef): Register
>> statically
>> initialized decls in Pointer Bounds Checker.
>> * cp/decl.c (cp_finish_decl): Likewise.
>> * gimplify.c (gimplify_init_constructor): Likewise.
>>
>>
>> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
>> index 614c46d..a32bc6b 100644
>> --- a/gcc/c/c-lang.c
>> +++ b/gcc/c/c-lang.c
>> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
>> #define LANG_HOOKS_INIT c_objc_common_init
>> #undef LANG_HOOKS_INIT_TS
>> #define LANG_HOOKS_INIT_TS c_common_init_ts
>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>
>> /* Each front end provides its own lang hook initializer. */
>> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index 9ccae3b..65d83c8 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser,
>> bool fndef_ok,
>> maybe_warn_string_init (TREE_TYPE (d), init);
>> finish_decl (d, init_loc, init.value,
>>init.original_type, asm_name);
>> +
>> + /* Register all decls with initializers in Pointer
>> +Bounds Checker to generate required static bounds
>> +initializers. */
>> + if (DECL_INITIAL (d) != error_mark_node)
>> + chkp_register_var_initializer (d);
>> }
>> }
>> else
>> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
>> index a7fa8e4..6d138bd 100644
>> --- a/gcc/cp/cp-lang.c
>> +++ b/gcc/cp/cp-lang.c
>> @@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded
>> (const_tree);
>> #define LANG_HOOKS_EH_PERSONALITY cp_eh_personality
>> #undef LANG_HOOKS_EH_RUNTIME_TYPE
>> #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>
>> /* Each front end provides its own lang hook initializer. */
>> s
Re: [PATCH, MPX, 2/X] Pointers Checker [6/25] Instrumentation pass
On Tue, 5 Nov 2013, Ilya Enkovich wrote: > On 01 Nov 21:59, Joseph S. Myers wrote: > > On Thu, 31 Oct 2013, Ilya Enkovich wrote: > > > > > * tree-chkp.c: New. > > > > This file includes tm.h. Inclusion of tm.h in front-end and GIMPLE files > > is discouraged (well, we'd like to convert all target macros to hooks, but > > the limited subset used in front-end and GIMPLE files are lower-hanging > > fruit than eliminating tm.h from the RTL parts of the compiler). If you > > need it, please put a comment on the #include saying exactly what target > > macros you need, so people know which macros' conversion to hooks would > > eliminate the need for the tm.h inclusion. > > There was a work to remove all target macros from this file. No all > target dependencies should go through hooks. Actually I can remove tm.h > include from this file but it does not guarantee it is not used because > it is implicitly included via expr.h, right? So the next question is why this file includes expr.h. It's a tree-* file. The presumption is that it should not need anything to do with RTL and so shouldn't need to include expr.h, rtl.h or tm.h, directly or indirectly. If there are just a few bits of RTL-related code for this functionality, but most of it works on GIMPLE, I'd suggest separating the RTL pieces out into a separate file. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, MPX, 2/X] Pointers Checker [6/25] Instrumentation pass
2013/11/5 Joseph S. Myers : > On Tue, 5 Nov 2013, Ilya Enkovich wrote: > >> On 01 Nov 21:59, Joseph S. Myers wrote: >> > On Thu, 31 Oct 2013, Ilya Enkovich wrote: >> > >> > > * tree-chkp.c: New. >> > >> > This file includes tm.h. Inclusion of tm.h in front-end and GIMPLE files >> > is discouraged (well, we'd like to convert all target macros to hooks, but >> > the limited subset used in front-end and GIMPLE files are lower-hanging >> > fruit than eliminating tm.h from the RTL parts of the compiler). If you >> > need it, please put a comment on the #include saying exactly what target >> > macros you need, so people know which macros' conversion to hooks would >> > eliminate the need for the tm.h inclusion. >> >> There was a work to remove all target macros from this file. No all >> target dependencies should go through hooks. Actually I can remove tm.h >> include from this file but it does not guarantee it is not used because >> it is implicitly included via expr.h, right? > > So the next question is why this file includes expr.h. It's a tree-* > file. The presumption is that it should not need anything to do with RTL > and so shouldn't need to include expr.h, rtl.h or tm.h, directly or > indirectly. > > If there are just a few bits of RTL-related code for this functionality, > but most of it works on GIMPLE, I'd suggest separating the RTL pieces out > into a separate file. It exports some functions manipulating with RTL and used by expand and target. Iface macros DECL_BOUNDS_RTL and SET_DECL_BOUNDS_RTL also use rtx type. Should I make two separate .h and .c files or include RTL part into something existent? Ilya > > -- > Joseph S. Myers > jos...@codesourcery.com
[Patch, AArch64] Define vec_extract.
Hi,
The attached patch defines standard pattern name vec_extract for aarch64.
Tested on aarch64-none-elf and aarch64_be-none-elf. OK for trunk?
Thanks,
Tejas Belagod
ARM.
Changelog:
2013-11-05 Tejas Belagod
gcc/
* config/aarch64/aarch64-simd.md (vec_extract): New.diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index a747ee8..879a0fd 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -4578,3 +4578,19 @@
(set_attr "simd_mode" "")]
)
+;; Standard pattern name vec_extract.
+
+(define_insn "vec_extract"
+ [(set (match_operand: 0 "aarch64_simd_nonimmediate_operand" "=r, w,
Utv")
+ (vec_select:
+ (match_operand:VALL 1 "register_operand" "w, w, w")
+ (parallel [(match_operand:SI 2 "immediate_operand" "i,i,i")])))]
+ "TARGET_SIMD"
+ "@
+ umov\\t%0, %1.[%2]
+ dup\\t%0, %1.[%2]
+ st1\\t{%1.}[%2], %0"
+ [(set_attr "simd_type" "simd_movgp, simd_dup, simd_store1s")
+ (set_attr "type" "neon_to_gp, neon_dup, neon_store1_one_lane")
+ (set_attr "simd_mode" "")]
+)
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
On Tue, Nov 5, 2013 at 2:37 PM, Ilya Enkovich wrote:
> 2013/11/5 Richard Biener :
>> On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich wrote:
>>> 2013/11/5 Richard Biener :
On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich
wrote:
> 2013/11/4 Richard Biener :
>> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich
>> wrote:
>>> Hi,
>>>
>>> This patch adds support Pointer Bounds Checker into c-family and LTO
>>> front-ends. The main purpose of changes in front-end is to register
>>> all statically initialized objects for checker. We also need to
>>> register such objects created by compiler.
>
> LTO is quite specific front-end. For LTO Pointer Bounds Checker
> support macro just means it allows instrumented code as input because
> all instrumentation is performed before code is streamed out for LTO.
But your patch doesn't even make use of the langhook...
>>>
>>> Use of langhook is in previous patch
>>> (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
>>> it's part:
>>>
>>> diff --git a/gcc/toplev.c b/gcc/toplev.c
>>> index db269b7..0eaf081 100644
>>> --- a/gcc/toplev.c
>>> +++ b/gcc/toplev.c
>>> @@ -1282,6 +1282,15 @@ process_options (void)
>>>"and -ftree-loop-linear)");
>>> #endif
>>>
>>> + if (flag_check_pointer_bounds)
>>> +{
>>> + if (targetm.chkp_bound_mode () == VOIDmode)
>>> + error ("-fcheck-pointers is not supported for this target");
>>> +
>>> + if (!lang_hooks.chkp_supported)
>>> + flag_check_pointer_bounds = 0;
>>> +}
>>> +
>>>/* One region RA really helps to decrease the code size. */
>>>if (flag_ira_region == IRA_REGION_AUTODETECT)
>>> flag_ira_region
>>>
>>> If we try to use -fcheck-pointers -flto for some unsupported language,
>>> code will not be instrumented.
>>
>> What's the reason to have unsupported languages?
>
> For some languages (e.g. Java) Pointer Bounds Checker does not make
> sense at all. Others may require additional support in front-end. The
> primary target is C family where solved problem is more critical.
What does break if you "enable" it for Java or other "unsupported"
languages? That is, if LTO is able to handle a mixed Java and
C binary then why can Java alone not handle it?
Richard.
> Ilya
>>
>> Richard.
>>
>>> Ilya
>>>
Richard.
> Ilya
>
>>
>> You define CHKP as supported in LTO. That means it has to be supported
>> for all languages and thus you should drop the langhook.
>>
>> Richard.
>>
>>> Thanks,
>>> Ilya
>>> --
>>>
>>> gcc/
>>>
>>> 2013-10-29 Ilya Enkovich
>>>
>>> * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>> * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>> * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>> * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>> * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>> * c/c-parser.c (c_parser_declaration_or_fndef): Register
>>> statically
>>> initialized decls in Pointer Bounds Checker.
>>> * cp/decl.c (cp_finish_decl): Likewise.
>>> * gimplify.c (gimplify_init_constructor): Likewise.
>>>
>>>
>>> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
>>> index 614c46d..a32bc6b 100644
>>> --- a/gcc/c/c-lang.c
>>> +++ b/gcc/c/c-lang.c
>>> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
>>> #define LANG_HOOKS_INIT c_objc_common_init
>>> #undef LANG_HOOKS_INIT_TS
>>> #define LANG_HOOKS_INIT_TS c_common_init_ts
>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>
>>> /* Each front end provides its own lang hook initializer. */
>>> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>>> index 9ccae3b..65d83c8 100644
>>> --- a/gcc/c/c-parser.c
>>> +++ b/gcc/c/c-parser.c
>>> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser,
>>> bool fndef_ok,
>>> maybe_warn_string_init (TREE_TYPE (d), init);
>>> finish_decl (d, init_loc, init.value,
>>>init.original_type, asm_name);
>>> +
>>> + /* Register all decls with initializers in Pointer
>>> +Bounds Checker to generate required static bounds
>>> +initializers. */
>>> + if (DECL_INITIAL (d) != error_mark_node)
>>> + chkp_register_var_initializer (d);
>>> }
>>> }
>>> else
>>> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
>>> index a7fa8e4..6d138bd 100644
>>> --- a/gcc/cp/cp-lang.c
>>> +++ b/gcc/cp/cp-lang.c
>>> @@ -81,6 +81,8 @@ static tree get_template_argument_pac
Re: [C++ Patch] PR 58724
OK. Jason
[PATCH 1/4] Cast pointers to uptr for 64-bit syscalls
internal_syscall casts pointers to uint64. It doesn't work for x32 since
32-bit pointers are signed extended to int64. This patch casts pointers
to uptr first. Tested with x32, x86-64 and x86. Please install it.
Thanks.
H.J.
---
libsanitizer/ChangeLog.x32 | 20 +
libsanitizer/sanitizer_common/sanitizer_linux.cc | 38
2 files changed, 39 insertions(+), 19 deletions(-)
create mode 100644 libsanitizer/ChangeLog.x32
diff --git a/libsanitizer/ChangeLog.x32 b/libsanitizer/ChangeLog.x32
new file mode 100644
index 000..7b54005
--- /dev/null
+++ b/libsanitizer/ChangeLog.x32
@@ -0,0 +1,20 @@
+2013-11-05 H.J. Lu
+
+ * sanitizer_common/sanitizer_linux.cc (internal_mmap): Cast
+ pointers to uptr for 64-bit syscalls.
+ (internal_munmap): Likewise.
+ (internal_open): Likewise.
+ (internal_read): Likewise.
+ (internal_write): Likewise.
+ (internal_stat): Likewise.
+ (internal_lstat): Likewise.
+ (internal_fstat): Likewise.
+ (internal_readlink): Likewise.
+ (internal_unlink): Likewise.
+ (internal_execve): Likewise.
+ (NanoTime): Likewise.
+ (BlockingMutex::Lock): Likewise.
+ (BlockingMutex::Unlock): Likewise.
+ (internal_ptrace): Likewise.
+ (internal_getdents): Likewise.
+ (internal_sigaltstack): Likewise.
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux.cc
b/libsanitizer/sanitizer_common/sanitizer_linux.cc
index 666f15b..e48bee5 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_linux.cc
@@ -77,14 +77,14 @@ namespace __sanitizer {
uptr internal_mmap(void *addr, uptr length, int prot, int flags,
int fd, u64 offset) {
#if SANITIZER_LINUX_USES_64BIT_SYSCALLS
- return internal_syscall(__NR_mmap, addr, length, prot, flags, fd, offset);
+ return internal_syscall(__NR_mmap, (uptr)addr, length, prot, flags, fd,
offset);
#else
return internal_syscall(__NR_mmap2, addr, length, prot, flags, fd, offset);
#endif
}
uptr internal_munmap(void *addr, uptr length) {
- return internal_syscall(__NR_munmap, addr, length);
+ return internal_syscall(__NR_munmap, (uptr)addr, length);
}
uptr internal_close(fd_t fd) {
@@ -92,11 +92,11 @@ uptr internal_close(fd_t fd) {
}
uptr internal_open(const char *filename, int flags) {
- return internal_syscall(__NR_open, filename, flags);
+ return internal_syscall(__NR_open, (uptr)filename, flags);
}
uptr internal_open(const char *filename, int flags, u32 mode) {
- return internal_syscall(__NR_open, filename, flags, mode);
+ return internal_syscall(__NR_open, (uptr)filename, flags, mode);
}
uptr OpenFile(const char *filename, bool write) {
@@ -106,13 +106,13 @@ uptr OpenFile(const char *filename, bool write) {
uptr internal_read(fd_t fd, void *buf, uptr count) {
sptr res;
- HANDLE_EINTR(res, (sptr)internal_syscall(__NR_read, fd, buf, count));
+ HANDLE_EINTR(res, (sptr)internal_syscall(__NR_read, fd, (uptr)buf, count));
return res;
}
uptr internal_write(fd_t fd, const void *buf, uptr count) {
sptr res;
- HANDLE_EINTR(res, (sptr)internal_syscall(__NR_write, fd, buf, count));
+ HANDLE_EINTR(res, (sptr)internal_syscall(__NR_write, fd, (uptr)buf, count));
return res;
}
@@ -138,7 +138,7 @@ static void stat64_to_stat(struct stat64 *in, struct stat
*out) {
uptr internal_stat(const char *path, void *buf) {
#if SANITIZER_LINUX_USES_64BIT_SYSCALLS
- return internal_syscall(__NR_stat, path, buf);
+ return internal_syscall(__NR_stat, (uptr)path, (uptr)buf);
#else
struct stat64 buf64;
int res = internal_syscall(__NR_stat64, path, &buf64);
@@ -149,7 +149,7 @@ uptr internal_stat(const char *path, void *buf) {
uptr internal_lstat(const char *path, void *buf) {
#if SANITIZER_LINUX_USES_64BIT_SYSCALLS
- return internal_syscall(__NR_lstat, path, buf);
+ return internal_syscall(__NR_lstat, (uptr)path, (uptr)buf);
#else
struct stat64 buf64;
int res = internal_syscall(__NR_lstat64, path, &buf64);
@@ -160,7 +160,7 @@ uptr internal_lstat(const char *path, void *buf) {
uptr internal_fstat(fd_t fd, void *buf) {
#if SANITIZER_LINUX_USES_64BIT_SYSCALLS
- return internal_syscall(__NR_fstat, fd, buf);
+ return internal_syscall(__NR_fstat, fd, (uptr)buf);
#else
struct stat64 buf64;
int res = internal_syscall(__NR_fstat64, fd, &buf64);
@@ -181,11 +181,11 @@ uptr internal_dup2(int oldfd, int newfd) {
}
uptr internal_readlink(const char *path, char *buf, uptr bufsize) {
- return internal_syscall(__NR_readlink, path, buf, bufsize);
+ return internal_syscall(__NR_readlink, (uptr)path, (uptr)buf, bufsize);
}
uptr internal_unlink(const char *path) {
- return internal_syscall(__NR_unlink, path);
+ return internal_syscall(__NR_unlink, (uptr)path);
}
uptr internal_sched_yield() {
@@ -199,7 +199,7 @@ void internal__exit(int exitcode) {
uptr internal_execve(const
RE: PR 58958: wrong aliasing info
> -Original Message- > From: Jakub Jelinek [mailto:ja...@redhat.com] > Sent: Tuesday, November 5, 2013 6:02 AM > To: Marc Glisse; Iyer, Balaji V > Cc: Richard Biener; GCC Patches > Subject: Re: PR 58958: wrong aliasing info > > On Tue, Nov 05, 2013 at 11:40:02AM +0100, Marc Glisse wrote: > > >Well, you cannot use the size argument unchanged for the null return > > >case. You could fallback to get_base_address and -1 size in that > > >case. > > > > Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu. > > (I think I'll disable cilk for my future bootstraps: it takes forever > > and confuses contrib/compare_tests) > > Ah, I was wondering why my make check times (admittedly --enable- > checking=yes,rtl but I've done that for years) went up drastically between > Friday and Monday (from around 40 minutes to 70 minutes or more). > > That is clearly highly undesirable. Looking at > gcc.dg/cilk-plus/cilk-plus.exp it > seems every test is run 24 resp. 25 times, that is clearly way too much, sure, > we have torture kind of tests, but that is typically 6 or 8 times at most, > and it > also depends on how expensive the tests are. > As the /AN/ tests were already preexisting, I guess we are talking about the 9 > new /CK/ dg-do run tests, so first of all, they must be very expensive > themselves, times 25: > > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus $ALWAYS_CFLAGS " " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O0 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O1 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O3 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -g -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -g -O0 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -g -O1 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -g -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -g -O3 -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O3 -ftree-vectorize -fcilkplus -g $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -O0 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -O1 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -O3 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -g -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -g -O0 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -g -O1 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -g -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -fcilkplus -g -O3 -std=c99 $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O3 -ftree-vectorize -std=c99 -g -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O0 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O2 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " > dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] > " -O3 -flto -g -fcilkplus $ALWAYS_CFLAGS" " " > > It doesn't make sense to test -g at all levels, just test it for one or two, > it > doesn't make sense to test say no -O* vs. -O0, that is the same thing, or -O3 > vs. -O3 -ftree-vectorize, that is the same thing, testing -std=c99 vs. > default is > reasonable for compile time tests if they particularly care for some reason, > otherwise, just attach -std=c99 to one (doesn't matter which one) variant. > So, please change this to at most 7 or so variants, and even then, if
[PATCH 2/4] Fix internal_clone for x32
We need to allcate 2 64-bit integers to restore fn and arg with pop,
which only pop 64-bit integer into 64-bit register. Also pointers
are 32-bit for x32. This patch properly loads r8/r10 fpr syscall.
Please install it.
Thanks.
H.J.
---
libsanitizer/ChangeLog.x32 | 6 ++
libsanitizer/sanitizer_common/sanitizer_linux.cc | 16
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/libsanitizer/ChangeLog.x32 b/libsanitizer/ChangeLog.x32
index 7b54005..40b185d 100644
--- a/libsanitizer/ChangeLog.x32
+++ b/libsanitizer/ChangeLog.x32
@@ -1,5 +1,11 @@
2013-11-05 H.J. Lu
+ * sanitizer_common/sanitizer_linux.cc (internal_clone): Allocate
+ 2 64-bit integers to save and restore fn and arg. Properly load
+ newtls/child_tidptr into r8/r10.
+
+2013-11-05 H.J. Lu
+
* sanitizer_common/sanitizer_linux.cc (internal_mmap): Cast
pointers to uptr for 64-bit syscalls.
(internal_munmap): Likewise.
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux.cc
b/libsanitizer/sanitizer_common/sanitizer_linux.cc
index e48bee5..bb43437 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_linux.cc
@@ -772,9 +772,11 @@ uptr internal_clone(int (*fn)(void *), void *child_stack,
int flags, void *arg,
if (!fn || !child_stack)
return -EINVAL;
CHECK_EQ(0, (uptr)child_stack % 16);
- child_stack = (char *)child_stack - 2 * sizeof(void *);
- ((void **)child_stack)[0] = (void *)(uptr)fn;
- ((void **)child_stack)[1] = arg;
+ child_stack = (char *)child_stack - 2 * sizeof(unsigned long long);
+ ((unsigned long long *)child_stack)[0] = (uptr)fn;
+ ((unsigned long long *)child_stack)[1] = (uptr)arg;
+ void *r8 __asm__ ("r8") = newtls;
+ int *r10 __asm__ ("r10") = child_tidptr;
__asm__ __volatile__(
/* %rax = syscall(%rax = __NR_clone,
*%rdi = flags,
@@ -783,8 +785,6 @@ uptr internal_clone(int (*fn)(void *), void *child_stack,
int flags, void *arg,
*%r8 = new_tls,
*%r10 = child_tidptr)
*/
- "movq %6,%%r8\n"
- "movq %7,%%r10\n"
".cfi_endproc\n"
"syscall\n"
@@ -816,9 +816,9 @@ uptr internal_clone(int (*fn)(void *), void *child_stack,
int flags, void *arg,
"S"(child_stack),
"D"(flags),
"d"(parent_tidptr),
- "r"(newtls),
- "r"(child_tidptr)
- : "rsp", "memory", "r8", "r10", "r11", "rcx");
+ "r"(r8),
+ "r"(r10)
+ : "rsp", "memory", "r11", "rcx");
return res;
}
#endif // defined(__x86_64__)
--
1.8.3.1
[Patch, AArch64] Add w -> w constraint to vec_set.
Hi,
The attached patch adds a w -> w constraint to vec_set.
Tested on aarch64-none-elf and aarch64_be-none-elf. OK for trunk?
Thanks,
Tejas Belagod
ARM.
Changelog:
2013-11-05 Tejas Belagod
gcc/
* config/aarch64/aarch64-simd.md (vec_set): Add w -> w option to
the constraint.diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index a747ee8..df4ef95 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -695,16 +695,18 @@
)
(define_insn "aarch64_simd_vec_set"
- [(set (match_operand:VQ_S 0 "register_operand" "=w")
+ [(set (match_operand:VQ_S 0 "register_operand" "=w,w")
(vec_merge:VQ_S
(vec_duplicate:VQ_S
- (match_operand: 1 "register_operand" "r"))
- (match_operand:VQ_S 3 "register_operand" "0")
- (match_operand:SI 2 "immediate_operand" "i")))]
+ (match_operand: 1 "register_operand" "r,w"))
+ (match_operand:VQ_S 3 "register_operand" "0,0")
+ (match_operand:SI 2 "immediate_operand" "i,i")))]
"TARGET_SIMD"
- "ins\t%0.[%p2], %w1";
- [(set_attr "simd_type" "simd_insgp")
- (set_attr "type" "neon_from_gp")
+ "@
+ ins\t%0.[%p2], %w1
+ ins\\t%0.[%p2], %1.[0]"
+ [(set_attr "simd_type" "simd_insgp, simd_ins")
+ (set_attr "type" "neon_from_gp, neon_ins")
(set_attr "simd_mode" "")]
)
@@ -958,9 +960,9 @@
})
(define_expand "vec_set"
- [(match_operand:VQ_S 0 "register_operand" "+w")
- (match_operand: 1 "register_operand" "r")
- (match_operand:SI 2 "immediate_operand" "")]
+ [(match_operand:VQ_S 0 "register_operand")
+ (match_operand: 1 "register_operand")
+ (match_operand:SI 2 "immediate_operand")]
"TARGET_SIMD"
{
HOST_WIDE_INT elem = (HOST_WIDE_INT) 1 << INTVAL (operands[2]);
@@ -971,23 +973,25 @@
)
(define_insn "aarch64_simd_vec_setv2di"
- [(set (match_operand:V2DI 0 "register_operand" "=w")
+ [(set (match_operand:V2DI 0 "register_operand" "=w,w")
(vec_merge:V2DI
(vec_duplicate:V2DI
- (match_operand:DI 1 "register_operand" "r"))
- (match_operand:V2DI 3 "register_operand" "0")
- (match_operand:SI 2 "immediate_operand" "i")))]
+ (match_operand:DI 1 "register_operand" "r,w"))
+ (match_operand:V2DI 3 "register_operand" "0,0")
+ (match_operand:SI 2 "immediate_operand" "i,i")))]
"TARGET_SIMD"
- "ins\t%0.d[%p2], %1";
- [(set_attr "simd_type" "simd_insgp")
- (set_attr "type" "neon_from_gp")
+ "@
+ ins\t%0.d[%p2], %1
+ ins\\t%0.d[%p2], %1.d[0]"
+ [(set_attr "simd_type" "simd_insgp, simd_ins")
+ (set_attr "type" "neon_from_gp, neon_ins_q")
(set_attr "simd_mode" "V2DI")]
)
(define_expand "vec_setv2di"
- [(match_operand:V2DI 0 "register_operand" "+w")
- (match_operand:DI 1 "register_operand" "r")
- (match_operand:SI 2 "immediate_operand" "")]
+ [(match_operand:V2DI 0 "register_operand")
+ (match_operand:DI 1 "register_operand")
+ (match_operand:SI 2 "immediate_operand")]
"TARGET_SIMD"
{
HOST_WIDE_INT elem = (HOST_WIDE_INT) 1 << INTVAL (operands[2]);
[PATCH 3/4] Use 64-bit system types for x86-64
x32 uses the same file system calls as x86-64. This patch fixes it
for x32. Tested on x32, x86-64 and x86. Please install it.
Thanks.
H.J.
---
libsanitizer/ChangeLog.x32 | 14 ++
.../sanitizer_common/sanitizer_platform_limits_linux.cc| 2 +-
.../sanitizer_common/sanitizer_platform_limits_posix.h | 7 ---
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/libsanitizer/ChangeLog.x32 b/libsanitizer/ChangeLog.x32
index 40b185d..51d0fd4 100644
--- a/libsanitizer/ChangeLog.x32
+++ b/libsanitizer/ChangeLog.x32
@@ -1,5 +1,19 @@
2013-11-05 H.J. Lu
+ * sanitizer_common/sanitizer_platform_limits_linux.cc
+ (struct_kernel_stat64_sz): Initialize to 0 if __x86_64__ is
+ defined.
+ * sanitizer_common/sanitizer_platform_limits_posix.h
+ (__sanitizer_dirent): Use 64-bit d_ino/d_off if __x86_64__ is
+ defined.
+ (__sanitizer___kernel_uid_t): Typedef as unsigned if __x86_64__
+ is defined.
+ (__sanitizer___kernel_gid_t): Likewise.
+ (__sanitizer___kernel_off_t): Typedef as long long if __x86_64__
+ is defined.
+
+2013-11-05 H.J. Lu
+
* sanitizer_common/sanitizer_linux.cc (internal_clone): Allocate
2 64-bit integers to save and restore fn and arg. Properly load
newtls/child_tidptr into r8/r10.
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
index fbea596..9870243 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
@@ -31,7 +31,7 @@ namespace __sanitizer {
unsigned struct_io_event_sz = sizeof(struct io_event);
unsigned struct_iocb_sz = sizeof(struct iocb);
-#ifndef _LP64
+#if !defined(_LP64) && !defined(__x86_64__)
unsigned struct_kernel_stat64_sz = sizeof(struct stat64);
#else
unsigned struct_kernel_stat64_sz = 0;
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
index 007b4ec..67c459c 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -133,7 +133,7 @@ namespace __sanitizer {
unsigned short d_reclen;
// more fields that we don't care about
};
-#elif SANITIZER_ANDROID
+#elif SANITIZER_ANDROID || defined(__x86_64__)
struct __sanitizer_dirent {
unsigned long long d_ino;
unsigned long long d_off;
@@ -159,16 +159,17 @@ namespace __sanitizer {
#endif
#if SANITIZER_LINUX
-#ifdef _LP64
+#if defined(_LP64) || defined(__x86_64__)
typedef unsigned __sanitizer___kernel_uid_t;
typedef unsigned __sanitizer___kernel_gid_t;
+ typedef long long __sanitizer___kernel_off_t;
#else
typedef unsigned short __sanitizer___kernel_uid_t;
typedef unsigned short __sanitizer___kernel_gid_t;
+ typedef long __sanitizer___kernel_off_t;
#endif
typedef unsigned short __sanitizer___kernel_old_uid_t;
typedef unsigned short __sanitizer___kernel_old_gid_t;
- typedef long __sanitizer___kernel_off_t;
typedef long long __sanitizer___kernel_loff_t;
typedef struct {
unsigned long fds_bits[1024 / (8 * sizeof(long))];
--
1.8.3.1
Re: [PATCH] decide edge's hotness when there is profile info
Ping.
Thanks, Teresa
On Wed, Oct 30, 2013 at 1:15 PM, Teresa Johnson wrote:
> On Fri, Oct 18, 2013 at 2:17 PM, Jan Hubicka wrote:
>>> Here is the patch updated to use the new parameter from r203830.
>>> Passed bootstrap and regression tests.
>>>
>>> 2013-10-18 Jan Hubicka
>>> Teresa Johnson
>>>
>>> * predict.c (handle_missing_profiles): New function.
>>> (counts_to_freqs): Don't overwrite estimated frequencies
>>> when function has no profile counts.
>>> * predict.h (handle_missing_profiles): Declare.
>>> * tree-profile.c (tree_profiling): Invoke handle_missing_profiles.
>>>
>>> Index: predict.c
>>> ===
>>> --- predict.c (revision 203830)
>>> +++ predict.c (working copy)
>>> @@ -2758,6 +2758,40 @@ estimate_loops (void)
>>>BITMAP_FREE (tovisit);
>>> }
>>>
>>
>> You need block comment. It should explain the problem of COMDATs and how
>> they are handled.
>> It is not an obvious thing.
>
> Done.
>
>>
>>> +void
>>> +handle_missing_profiles (void)
>>> +{
>>> + struct cgraph_node *node;
>>> + int unlikely_count_fraction = PARAM_VALUE (UNLIKELY_BB_COUNT_FRACTION);
>> extra line
>>> + /* See if 0 count function has non-0 count callers. In this case we
>>> + lost some profile. Drop its function profile to PROFILE_GUESSED. */
>>> + FOR_EACH_DEFINED_FUNCTION (node)
>>> +{
>>> + struct cgraph_edge *e;
>>> + gcov_type call_count = 0;
>>> + struct function *fn = DECL_STRUCT_FUNCTION (node->symbol.decl);
>> Extra line
>>> + if (node->count)
>>> +continue;
>>> + for (e = node->callers; e; e = e->next_caller)
>>> +call_count += e->count;
>> What about checking if the sum is way off even for non-0 counts.
>> I.e. for case where function was inlined to some calls but not to others? In
>> that case we may want to scale up the counts (with some resonable care for
>> capping)
>
> In this patch I am not changing any counts, so I am leaving this one
> for follow-on work (even for the routines missing counts completely
> like these, we don't apply any counts, just mark them as guessed. I
> have a follow-on patch to send once this goes in that does apply
> counts to these 0-count routines only when we decide to inline as we
> discussed).
>
>>
>> Also I think the code really should propagate - i.e. have simple worklist and
>> look for functions that are COMDAT and are called by other COMDAT functions
>> where we decided to drop the profile. Having non-trivial chains of comdats
>> is
>> common thing.
>
> Done.
>
>>
>> What about outputting user visible warning/error on the incosnsistency when
>> no COMDAT function is involved same way as we do for BB profile?
>
> Done. This one caught the fact that we have this situation for "extern
> template" functions as well when I tested on cpu2006. I added in a
> check to ignore those when issuing the warning (they are not emitted
> thus don't get any profile counts).
>
> Updated patch below.
>
> Bootstrapped and tested on x86-64-unknown-linux-gnu. Also tested on
> profiledbootstrap and profile-use build of SPEC cpu2006. Ok for trunk?
>
> Thanks,
> Teresa
>
> 2013-10-30 Teresa Johnson
>
> * predict.c (drop_profile): New function.
> (handle_missing_profiles): Ditto.
> (counts_to_freqs): Don't overwrite estimated frequencies
> when function has no profile counts.
> * predict.h (handle_missing_profiles): Declare.
> * tree-profile.c (tree_profiling): Invoke handle_missing_profiles.
>
> Index: predict.c
> ===
> --- predict.c (revision 204213)
> +++ predict.c (working copy)
> @@ -2765,6 +2765,107 @@ estimate_loops (void)
>BITMAP_FREE (tovisit);
> }
>
> +/* Drop the profile for NODE to guessed, and update its frequency based on
> + whether it is expected to be HOT. */
> +
> +static void
> +drop_profile (struct cgraph_node *node, bool hot)
> +{
> + struct function *fn = DECL_STRUCT_FUNCTION (node->decl);
> +
> + if (dump_file)
> +fprintf (dump_file,
> + "Dropping 0 profile for %s/%i. %s based on calls.\n",
> + cgraph_node_name (node), node->order,
> + hot ? "Function is hot" : "Function is normal");
> + /* We only expect to miss profiles for functions that are reached
> + via non-zero call edges in cases where the function may have
> + been linked from another module or library (COMDATs and extern
> + templates). See the comments below for handle_missing_profiles. */
> + if (!DECL_COMDAT (node->decl) && !DECL_EXTERNAL (node->decl))
> +warning (0,
> + "Missing counts for called function %s/%i",
> + cgraph_node_name (node), node->order);
> +
> + profile_status_for_function (fn)
> + = (flag_guess_branch_prob ? PROFILE_GUESSED : PROFILE_ABSENT);
> + node->frequency
> + = hot ? NODE_FREQUENCY_HOT :
[PATCH 4/4] Check __x86_64__ for FPU state
X32 uses the same FPU state as x86-64. Please install it. Thanks.
H.J.
---
libsanitizer/ChangeLog.x32 | 6 ++
libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/libsanitizer/ChangeLog.x32 b/libsanitizer/ChangeLog.x32
index 51d0fd4..23b7fc9 100644
--- a/libsanitizer/ChangeLog.x32
+++ b/libsanitizer/ChangeLog.x32
@@ -1,5 +1,11 @@
2013-11-05 H.J. Lu
+ * sanitizer_common/sanitizer_platform_limits_posix.cc
+ (struct_user_fpxregs_struct_sz): Initialize to 0 if __x86_64__ is
+ defined.
+
+2013-11-05 H.J. Lu
+
* sanitizer_common/sanitizer_platform_limits_linux.cc
(struct_kernel_stat64_sz): Initialize to 0 if __x86_64__ is
defined.
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
index 971a193..b771583 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
@@ -191,7 +191,7 @@ namespace __sanitizer {
(defined(__i386) || defined (__x86_64)) // NOLINT
unsigned struct_user_regs_struct_sz = sizeof(struct user_regs_struct);
unsigned struct_user_fpregs_struct_sz = sizeof(struct user_fpregs_struct);
-#if __WORDSIZE == 64
+#ifdef __x86_64
unsigned struct_user_fpxregs_struct_sz = 0;
#else
unsigned struct_user_fpxregs_struct_sz = sizeof(struct user_fpxregs_struct);
--
1.8.3.1
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 2:37 PM, Ilya Enkovich wrote:
>> 2013/11/5 Richard Biener :
>>> On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich
>>> wrote:
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich
> wrote:
>> 2013/11/4 Richard Biener :
>>> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich
>>> wrote:
Hi,
This patch adds support Pointer Bounds Checker into c-family and LTO
front-ends. The main purpose of changes in front-end is to register
all statically initialized objects for checker. We also need to
register such objects created by compiler.
>>
>> LTO is quite specific front-end. For LTO Pointer Bounds Checker
>> support macro just means it allows instrumented code as input because
>> all instrumentation is performed before code is streamed out for LTO.
>
> But your patch doesn't even make use of the langhook...
Use of langhook is in previous patch
(http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
it's part:
diff --git a/gcc/toplev.c b/gcc/toplev.c
index db269b7..0eaf081 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1282,6 +1282,15 @@ process_options (void)
"and -ftree-loop-linear)");
#endif
+ if (flag_check_pointer_bounds)
+{
+ if (targetm.chkp_bound_mode () == VOIDmode)
+ error ("-fcheck-pointers is not supported for this target");
+
+ if (!lang_hooks.chkp_supported)
+ flag_check_pointer_bounds = 0;
+}
+
/* One region RA really helps to decrease the code size. */
if (flag_ira_region == IRA_REGION_AUTODETECT)
flag_ira_region
If we try to use -fcheck-pointers -flto for some unsupported language,
code will not be instrumented.
>>>
>>> What's the reason to have unsupported languages?
>>
>> For some languages (e.g. Java) Pointer Bounds Checker does not make
>> sense at all. Others may require additional support in front-end. The
>> primary target is C family where solved problem is more critical.
>
> What does break if you "enable" it for Java or other "unsupported"
> languages? That is, if LTO is able to handle a mixed Java and
> C binary then why can Java alone not handle it?
In such case checker will produce useless overhead in Java code.
Resulting code will probably report some bound violations because Java
FE may generate code which seems wrong for Pointer Bounds Checker.
Ilya
>
> Richard.
>
>> Ilya
>>>
>>> Richard.
>>>
Ilya
>
> Richard.
>
>> Ilya
>>
>>>
>>> You define CHKP as supported in LTO. That means it has to be supported
>>> for all languages and thus you should drop the langhook.
>>>
>>> Richard.
>>>
Thanks,
Ilya
--
gcc/
2013-10-29 Ilya Enkovich
* c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
* c/c-parser.c (c_parser_declaration_or_fndef): Register
statically
initialized decls in Pointer Bounds Checker.
* cp/decl.c (cp_finish_decl): Likewise.
* gimplify.c (gimplify_init_constructor): Likewise.
diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
index 614c46d..a32bc6b 100644
--- a/gcc/c/c-lang.c
+++ b/gcc/c/c-lang.c
@@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
#define LANG_HOOKS_INIT c_objc_common_init
#undef LANG_HOOKS_INIT_TS
#define LANG_HOOKS_INIT_TS c_common_init_ts
+#undef LANG_HOOKS_CHKP_SUPPORTED
+#define LANG_HOOKS_CHKP_SUPPORTED true
/* Each front end provides its own lang hook initializer. */
struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 9ccae3b..65d83c8 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser
*parser, bool fndef_ok,
maybe_warn_string_init (TREE_TYPE (d), init);
finish_decl (d, init_loc, init.value,
init.original_type, asm_name);
+
+ /* Register all decls with initializers in Pointer
+Bounds Checker to generate required static bounds
+initializers. */
+ if (DECL_INITIAL (d)
Re: [PATCH GCC]Simplify address expression in IVOPT
On Tue, Nov 5, 2013 at 8:29 PM, Richard Biener
wrote:
> On Tue, Nov 5, 2013 at 1:18 PM, Yufeng Zhang wrote:
>> On 11/05/13 11:45, Bin.Cheng wrote:
>>>
>>> On Tue, Nov 5, 2013 at 7:19 PM, Yufeng Zhang wrote:
> On 11/05/13 10:13, bin.cheng wrote:
>
> >>
> >> Index: gcc/tree-affine.c
> >> ===
> >> --- gcc/tree-affine.c (revision 204117)
> >> +++ gcc/tree-affine.c (working copy)
> >> @@ -874,10 +874,11 @@ debug_aff (aff_tree *val)
> >> fprintf (stderr, "\n");
> >> }
> >>
> >> -/* Returns address of the reference REF in ADDR. The size of the
> >> accessed
> >> - location is stored to SIZE. */
> >> +/* Computes address of the reference REF in ADDR. The size of the
> >> accessed
> >> + location is stored to SIZE. Returns pointer to the ultimate
> >> containing
> >> + object to which REF refers. */
> >>
> >> -void
> >> +tree
> >> get_inner_reference_aff (tree ref, aff_tree *addr, double_int
> >> *size)
> >> {
> >> HOST_WIDE_INT bitsize, bitpos;
> >> @@ -904,6 +905,8 @@ get_inner_reference_aff (tree ref, aff_tree
> >> *addr,
> >> aff_combination_add (addr,&tmp);
> >>
> >> *size = double_int::from_shwi ((bitsize + BITS_PER_UNIT - 1) /
> >> BITS_PER_UNIT);
> >> +
> >> + return base_addr;
> >> }
> >>
>
> I think what Richard suggests is to return the base object rather the
> address of the base object, i.e.
>>>
>>> I am not sure about that. We have to pass pointer_type expression to
>>> function determine_base_object for address expressions, because there
>>> is no way to tell pointer from object once we are in
>>> determine_base_object.
>>
>>
>> I'm just concerned with the consistency in what is returned between
>> get_inner_reference and get_inner_reference_aff. If determine_base_object
>> expects reference only, you can probably work around it with something like:
>>
>> base_object = build_fold_addr_expr (base_object);
>>
>> after the get_inner_reference_aff call.
>
> Note that get_inner_reference_aff is already different from
> get_inner_reference
> in that it does not separate base object and offset but lumps them together
> into an address represented as affine combination.
>
> For this reason get_inner_reference_aff may not exactly be the best
> name for the function ... it could be split to not do
>
> aff_combination_add (addr, &tmp);
>
> but instead return 'base'. The two existing users of the function could
> be unified into aff_comb_cannot_overlap_p (giving that a better name
> as well), even avoiding combining the base but instead handle
> comparing the base itself.
>
> Of course that's preexisting issues that I don't want to force Bin Cheng
> to clean up and fix (given I've, too, just spent a minute thinking about it).
>
I can investigate that later after finishing all my IVOPT patches.
Thanks,
bin
--
Best Regards.
Re: [wide-int] Restore VAX build
On Sun, 2013-11-03 08:29:12 -0500, Kenneth Zadeck
wrote:
> On Nov 3, 2013, at 5:24 AM, Richard Sandiford
> wrote:
> > Index: gcc/config/vax/vax.c
> > ===
> > --- gcc/config/vax/vax.c2013-11-03 10:24:01.013238719 +
> > +++ gcc/config/vax/vax.c2013-11-03 10:24:02.308247979 +
> > @@ -643,7 +643,7 @@ vax_float_literal (rtx c)
> > {
> > int x = 1 << i;
> > bool ok;
> > - REAL_VALUE_FROM_INT (s, x, 0, mode);
> > + REAL_VALUE_FROM_INT (s, x, mode);
> >
> > if (REAL_VALUES_EQUAL (r, s))
> >return true;
>
> Vax, seriously??? Are there any left?
There are! It's not all that dead.
MfG, JBG
--
Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481
Signature of: "Debugging is twice as hard as writing the code in the first
place.
the second : Therefore, if you write the code as cleverly as possible, you
are,
by definition, not smart enough to debug it." - Brian W.
Kernighan
signature.asc
Description: Digital signature
Re: [wide-int] Restore VAX build
On 11/05/2013 09:12 AM, Jan-Benedict Glaw wrote:
On Sun, 2013-11-03 08:29:12 -0500, Kenneth Zadeck
wrote:
On Nov 3, 2013, at 5:24 AM, Richard Sandiford
wrote:
Index: gcc/config/vax/vax.c
===
--- gcc/config/vax/vax.c2013-11-03 10:24:01.013238719 +
+++ gcc/config/vax/vax.c2013-11-03 10:24:02.308247979 +
@@ -643,7 +643,7 @@ vax_float_literal (rtx c)
{
int x = 1 << i;
bool ok;
- REAL_VALUE_FROM_INT (s, x, 0, mode);
+ REAL_VALUE_FROM_INT (s, x, mode);
if (REAL_VALUES_EQUAL (r, s))
return true;
Vax, seriously??? Are there any left?
There are! It's not all that dead.
MfG, JBG
when i was in grad school in the late 70's early 80's, our dept
submitted an nsf grant to buy 1mb for our 11-780. I believe 1mb cost
about 100K. That was a very long time ago. Unlike ibm with it's z
series (formally 360 or 370), dec has been out of business for a long time.
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
2013/11/5 Richard Biener :
> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich wrote:
>>
>> For input parameter P I need to have
>> BOUNDS = __builtin_arg_bnd (P)
>> to somehow refer to bounds of P in GIMPLE. Optimizations may modify
>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>> makes call useless because removes information about parameter whose
>> bounds we refer to. I want such optimization to ignore
>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>> there as arg.
>
> How does a compilable testcase look like that shows how the default def
> is used? And what transforms break that use? I really cannot see
> how this would happen (and you didn't add a testcase).
Here is a test source:
extern int bar1 (int *p);
extern int bar2 (int);
int foo (int *p)
{
if (!p)
return bar1 (p);
return bar2 (10);
}
After instrumentation GIMPLE looks like:
foo (int * p)
{
__bound_tmp.0;
int _1;
int _6;
int _8;
:
__bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
:
if (p_3(D) == 0B)
goto ;
else
goto ;
:
_6 = bar1 (p_3(D), __bound_tmp.0_9);
goto ;
:
_8 = bar2 (10);
:
# _1 = PHI <_6(3), _8(4)>
return _1;
}
Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
foo (int * p)
{
__bound_tmp.0;
int _1;
int _6;
int _8;
:
if (p_3(D) == 0B)
goto ;
else
goto ;
:
__bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
_6 = bar1 (0B, __bound_tmp.0_9); [tail call]
goto ;
:
_8 = bar2 (10); [tail call]
:
# _1 = PHI <_6(3), _8(4)>
return _1;
}
Now during expand or inline of foo I cannot determine the value for
__bound_tmp.0_9.
Ilya
>
> Richard.
>
[PATCH] Fix PR58492
This fixes PR58492 - when late IPA passes are run all IPA transforms
are applied and the last fixup-cfg is run. This breaks the IL
when for example late local-pure-const makes changes that are not
fixed up for in the callers.
Bootstrap and tests running on x86_64-unknown-linux-gnu.
Richard.
2013-11-05 Richard Biener
PR ipa/58492
* passes.def (all_passes): Start with pass_fixup_cfg again.
* gcc.dg/ipa/pr58492.c: New testcase.
Index: gcc/passes.def
===
--- gcc/passes.def (revision 204390)
+++ gcc/passes.def (working copy)
@@ -126,6 +126,7 @@ along with GCC; see the file COPYING3.
/* These passes are run after IPA passes on every function that is being
output to the assembler file. */
INSERT_PASSES_AFTER (all_passes)
+ NEXT_PASS (pass_fixup_cfg);
NEXT_PASS (pass_lower_eh_dispatch);
NEXT_PASS (pass_all_optimizations);
PUSH_INSERT_PASSES_WITHIN (pass_all_optimizations)
Index: gcc/testsuite/gcc.dg/ipa/pr58492.c
===
--- gcc/testsuite/gcc.dg/ipa/pr58492.c (revision 0)
+++ gcc/testsuite/gcc.dg/ipa/pr58492.c (working copy)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fipa-pta" } */
+
+void f(int p, short q)
+{
+ f(0, 0);
+}
Re: libsanitizer merge from upstream r191666
Hi Kostya,
As an aside, all of these asan tests have failed on
powerpc64le-unknown-linux-gnu since we began testing that target. I
have not yet had time to investigate why. Do you think there may be an
implicit or explicit assumption anywhere that PowerPC64 is big endian?
This is generally not safe to assume. Anyway, I hope it's something
easy like that, and thought you might have an idea.
Thanks,
Bill
On Mon, 2013-11-04 at 11:46 -0500, Jack Howarth wrote:
> On Mon, Nov 04, 2013 at 06:47:13AM -0800, Konstantin Serebryany wrote:
> > +glider, our Mac expert.
> >
> > Hi Jack,
> >
> > This patch has not been tested on Mac and we generally do not claim
> > that gcc-asan is supported on Mac.
> > clang-asan *is* supported on Mac and our bots are green (this patch is
> > a merge of the sources which are regularly tested on Mac,
> > but the build procedure is different).
> >
> > The failing assertion is weird, I haven't seen it since last year
> > (https://code.google.com/p/address-sanitizer/issues/detail?id=117),
> > but Alexander should know more.
> >
> > What is your version of Mac OS?
>
> I am testing on MacOS X 10.8.5 with FSF gcc trunk built as...
>
> % gcc-fsf-4.9 -v
> Using built-in specs.
> COLLECT_GCC=gcc-fsf-4.9
> COLLECT_LTO_WRAPPER=/sw/lib/gcc4.9/libexec/gcc/x86_64-apple-darwin12.5.0/4.9.0/lto-wrapper
> Target: x86_64-apple-darwin12.5.0
> Configured with: ../gcc-4.9-20131101/configure --prefix=/sw
> --prefix=/sw/lib/gcc4.9 --mandir=/sw/share/man --infodir=/sw/lib/gcc4.9/info
> --enable-languages=c,c++,fortran,lto,objc,obj-c++,java --with-gmp=/sw
> --with-libiconv-prefix=/sw --with-isl=/sw --with-cloog=/sw --with-mpc=/sw
> --with-system-zlib --enable-checking=yes --x-includes=/usr/X11R6/include
> --x-libraries=/usr/X11R6/lib --program-suffix=-fsf-4.9
> Thread model: posix
> gcc version 4.9.0 20131101 (experimental) (GCC)
>
> >
> > --kcc
> >
> > On Sat, Nov 2, 2013 at 10:25 AM, Jack Howarth
> > wrote:
> > > On Fri, Nov 01, 2013 at 04:13:05PM -0700, Konstantin Serebryany wrote:
> > >> Jukub,
> > >>
> > >> This works!
> > >> New patch attached.
> > >
> > > Konstantin,
> > >This patch, when applied on top of r204305, bootstraps fine on
> > > x86_64-apple-darwin12 but
> > > produces a lot of regressions with...
> > >
> > > make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m64}'"
> > >
> > > Native configuration is x86_64-apple-darwin12.5.0
> > >
> > > === g++ tests ===
> > >
> > >
> > > Running target unix/-m64
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O0 execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O1 execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O2 execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O3 -g execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -Os execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O2 -flto
> > > -flto-partition=none execution test
> > > FAIL: c-c++-common/asan/global-overflow-1.c -O2 -flto execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O0 execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O1 execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O2 execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O3 -g execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -Os execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O2 -flto -flto-partition=none
> > > execution test
> > > FAIL: c-c++-common/asan/heap-overflow-1.c -O2 -flto execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O0 execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O1 execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O2 execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O3 -fomit-frame-pointer execution
> > > test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O3 -g execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -Os execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O2 -flto -flto-partition=none
> > > execution test
> > > FAIL: c-c++-common/asan/memcmp-1.c -O2 -flto execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O0 execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O1 execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O2 execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O3 -g execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -Os execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O2 -flto -flto-partition=none
> > > execution test
> > > FAIL: c-c++-common/asan/null-deref-1.c -O2 -flto execution test
> > > FAIL: c-c++-common/asan/sanity-check-pure-c-1.c -O0 execu
Re: RFA: Change behaviour of %* in spec strings
On Thu, 17 Oct 2013, Nick Clifton wrote:
> 2013-10-17 Nick Clifton
>
> * gcc.c (do_spec_1): Do not insert a space after a %* substitution
> unless it is the last part of a spec substring.
> * doc/invoke.texi (Spec Files): Document space insertion
> behaviour of %*.
OK, with one fix:
> +If @code{%*} appears as the last part of a spec sequence then a space
> +will added after the end of the last substitution. If there is more
"will be added".
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH, MPX, 2/X] Pointers Checker [6/25] Instrumentation pass
On Tue, 5 Nov 2013, Ilya Enkovich wrote: > > So the next question is why this file includes expr.h. It's a tree-* > > file. The presumption is that it should not need anything to do with RTL > > and so shouldn't need to include expr.h, rtl.h or tm.h, directly or > > indirectly. > > > > If there are just a few bits of RTL-related code for this functionality, > > but most of it works on GIMPLE, I'd suggest separating the RTL pieces out > > into a separate file. > > It exports some functions manipulating with RTL and used by expand and > target. Iface macros DECL_BOUNDS_RTL and SET_DECL_BOUNDS_RTL also use > rtx type. Should I make two separate .h and .c files or include RTL > part into something existent? I'd suggest separate files. -- Joseph S. Myers jos...@codesourcery.com
[PATCH] Fix PR58955
This fixes the wrong direction of dependence edges in loop
distribution (this happens when you think that if you choose
the wrong direction a testcase will trip and thus you don't
think hard enough ...).
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2013-11-05 Richard Biener
PR tree-optimization/58955
* tree-loop-distribution.c (pg_add_dependence_edges): Fix
edge direction.
* gcc.dg/torture/pr58955-1.c: New testcase.
* gcc.dg/torture/pr58955-2.c: Likewise.
Index: gcc/tree-loop-distribution.c
===
*** gcc/tree-loop-distribution.c(revision 204388)
--- gcc/tree-loop-distribution.c(working copy)
*** pg_add_dependence_edges (struct graph *r
*** 1324,1330
for (int ii = 0; drs1.iterate (ii, &dr1); ++ii)
for (int jj = 0; drs2.iterate (jj, &dr2); ++jj)
{
! int this_dir = 1;
ddr_p ddr;
/* Re-shuffle data-refs to be in dominator order. */
if (rdg_vertex_for_stmt (rdg, DR_STMT (dr1))
--- 1324,1330
for (int ii = 0; drs1.iterate (ii, &dr1); ++ii)
for (int jj = 0; drs2.iterate (jj, &dr2); ++jj)
{
! int this_dir = -1;
ddr_p ddr;
/* Re-shuffle data-refs to be in dominator order. */
if (rdg_vertex_for_stmt (rdg, DR_STMT (dr1))
Index: gcc/testsuite/gcc.dg/torture/pr58955-1.c
===
*** gcc/testsuite/gcc.dg/torture/pr58955-1.c(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr58955-1.c(working copy)
***
*** 0
--- 1,20
+ /* { dg-do run } */
+
+ extern void abort (void);
+
+ int a, b, c, d[4] = { 0, 0, 0, 1 };
+
+ int
+ main ()
+ {
+ for (; a < 4; a++)
+ {
+ int e = d[a];
+ for (c = 1; c < 1; c++);
+ b = e;
+ d[a] = 0;
+ }
+ if (b != 1)
+ abort ();
+ return 0;
+ }
Index: gcc/testsuite/gcc.dg/torture/pr58955-2.c
===
*** gcc/testsuite/gcc.dg/torture/pr58955-2.c(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr58955-2.c(working copy)
***
*** 0
--- 1,18
+ /* { dg-do run } */
+
+ extern void abort (void);
+
+ int a, b[10];
+
+ int
+ main ()
+ {
+ for (; a < 2; a++)
+ {
+ b[a] = 1;
+ b[a + 1] = 0;
+ }
+ if (b[1] != 1)
+ abort ();
+ return 0;
+ }
patch implementing a new pass for register-pressure relief through live range shrinkage
I'd like to add a new experimental optimization to the trunk. This
optimization was discussed on RA BOF of this summer GNU Cauldron.
It is a register pressure relief through live-range shrinkage. It
is implemented on the scheduler base and uses register-pressure insn
scheduling infrastructure. By rearranging insns we shorten pseudo
live-ranges and increase a chance to them be assigned to a hard
register.
The code looks pretty simple but there are a lot of works behind
this patch. I've tried about ten different versions of this code
(different heuristics for two currently existing register-pressure
algorithms).
I think it is *upto target maintainers* to decide to use or not to
use this optimization for their targets. I'd recommend to use this at
least for x86/x86-64. I think any OOO processor with small or
moderate register file which does not use the 1st insn scheduling
might benefit from this too.
On SPEC2000 for x86/x86-64 (I use Haswell processor, -O3 with
general tuning), the optimization usage results in smaller code size
in average (for floating point and integer benchmarks in 32- and
64-bit mode). The improvement better visible for SPECFP2000 (although
I have the same improvement on x86-64 SPECInt2000 but it might be
attributed mostly mcf benchmark unstability). It is about 0.5% for
32-bit and 64-bit mode. It is understandable, as the optimization has
more opportunities to improve the code on longer BBs. Different from
other heuristic optimizations, I don't see any significant worse
performance. It gives practically the same or better performance (a
few benchmarks imporoved by 1% or more upto 3%).
The single but significant drawback is additional compilation time
(4%-6%) as the 1st insn scheduling pass is quite expensive. So I'd
recommend target maintainers to switch it on only for -Ofast. If
somebody finds that the optimization works on processors which uses
1st insn scheduling by default (in which I slightly doubt), we could
improve the compilation time by reusing data for this optimization and
the 1st insn scheduling.
Any comments, questions, thoughts are appreciated.
2013-11-05 Vladimir Makarov
* tree-pass.h (make_pass_live_range_shrinkage): New external.
* timevar.def (TV_LIVE_RANGE_SHRINKAGE): New.
* sched-rgn.c (gate_handle_live_range_shrinkage): New.
(rest_of_handle_live_range_shrinkage): Ditto
(class pass_live_range_shrinkage): Ditto.
(pass_data_live_range_shrinkage): Ditto.
(make_pass_live_range_shrinkage): Ditto.
* sched-int.h (sched_relief_p): New external.
* sched-deps.c (create_insn_reg_set): Make void return value.
* passes.def: Add pass_live_range_shrinkage.
* ira.c (update_equiv_regs): Don't move if
flag_live_range_shrinkage.
* haifa-sched.c (sched_relief_p): New.
(rank_for_schedule): Add code for pressure relief through live
range shrinkage.
(schedule_insn): Print more debug info.
(sched_init): Setup SCHED_PRESSURE_WEIGHTED for pressure relief
through live range shrinkage.
* doc/invoke.texi (-flive-range-shrinkage): New.
* common.opt (flive-range-shrinkage): New.
Index: common.opt
===
--- common.opt (revision 204380)
+++ common.opt (working copy)
@@ -1738,6 +1738,10 @@ fregmove
Common Ignore
Does nothing. Preserved for backward compatibility.
+flive-range-shrinkage
+Common Report Var(flag_live_range_shrinkage) Init(0) Optimization
+Relief of register pressure through live range shrinkage
+
frename-registers
Common Report Var(flag_rename_registers) Init(2) Optimization
Perform a register renaming optimization pass
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 204216)
+++ doc/invoke.texi (working copy)
@@ -378,7 +378,7 @@ Objective-C and Objective-C++ Dialects}.
-fira-region=@var{region} -fira-hoist-pressure @gol
-fira-loop-pressure -fno-ira-share-save-slots @gol
-fno-ira-share-spill-slots -fira-verbose=@var{n} @gol
--fivopts -fkeep-inline-functions -fkeep-static-consts @gol
+-fivopts -fkeep-inline-functions -fkeep-static-consts -flive-range-shrinkage @gol
-floop-block -floop-interchange -floop-strip-mine -floop-nest-optimize @gol
-floop-parallelize-all -flto -flto-compression-level @gol
-flto-partition=@var{alg} -flto-report -flto-report-wpa -fmerge-all-constants @gol
@@ -7257,6 +7257,12 @@ registers after writing to their lower 3
Enabled for x86 at levels @option{-O2}, @option{-O3}.
+@item -flive-range-shrinkage
+@opindex flive-range-shrinkage
+Attempt to decrease register pressure through register live range
+shrinkage. This is helpful for fast processors with small or moderate
+size register sets.
+
@item -fira-algorithm=@var{algorithm}
Use the specified coloring algorithm for the integrated register
allocator. The @var{algor
Re: [Patch] Implementation of n3793
On 3 November 2013 11:30, Paolo Carlini wrote:
> On 11/03/2013 12:19 PM, Jonathan Wakely wrote:
>>
>> Yes, Paolo pointed out these are failing on 32-bit targets, I've got a
>> patch coming. Luc is there any reason not to just replace all those large
>> constants with 0x1234ABCD, which fits in a long on 32-bit targets?
>
> By the way, that's what I did/hacked in my local tree ;)
I needed some other changes, otherwise comparing optional{} ==
0x1234ABCD doesn't compile, because 0x1234ABCD is an int and
optional doesn't support comparisons with anything except T.
> Jon, I have got a bunch of other minor tweaks, from proper formatting of
> conditional operator and some curly braces, to using __and_ and __not_ when
> possible, and other thingies, like no full stops at the end of asserts and
> throws. Passes testing. You may want to integrate it with your other changes
> in testing... Or I can wait and apply it myself.
Here's the combined patch, tested x86_64-linux, 64-bit and 32-bit,
committed to trunk.
2013-11-05 Jonathan Wakely
Paolo Carlini
* include/experimental/optional: Use __and_<> and __not_<> in
conditions. Style fixes.
(__constexpr_addressof, swap): Make inline.
* testsuite/experimental/optional/cons/copy.cc: Adjust constants for
32-bit targets.
* testsuite/experimental/optional/cons/move.cc: Likewise.
* testsuite/experimental/optional/cons/value.cc: Likewise.
* testsuite/experimental/optional/constexpr/cons/value.cc: Likewise.
diff --git a/libstdc++-v3/include/experimental/optional
b/libstdc++-v3/include/experimental/optional
index 5915892..5882ff5 100644
--- a/libstdc++-v3/include/experimental/optional
+++ b/libstdc++-v3/include/experimental/optional
@@ -129,7 +129,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
struct _Has_addressof_impl<_Tp,
decltype( std::declval().operator&(), void() )>
-: std::true_type { };
+ : std::true_type { };
/**
* @brief Trait that detects the presence of an overloaded unary operator&.
@@ -157,7 +157,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
template::value,
int>::type...>
-_Tp* __constexpr_addressof(_Tp& __t)
+inline _Tp* __constexpr_addressof(_Tp& __t)
{ return std::__addressof(__t); }
/**
@@ -235,32 +235,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (this->_M_engaged && __other._M_engaged)
this->_M_get() = __other._M_get();
else
-{
- if (__other._M_engaged)
-this->_M_construct(__other._M_get());
- else
-this->_M_reset();
-}
+ {
+ if (__other._M_engaged)
+ this->_M_construct(__other._M_get());
+ else
+ this->_M_reset();
+ }
return *this;
}
_Optional_base&
operator=(_Optional_base&& __other)
- noexcept(is_nothrow_move_constructible<_Tp>()
- && is_nothrow_move_assignable<_Tp>())
+ noexcept(__and_,
+ is_nothrow_move_assignable<_Tp>>())
{
-if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = std::move(__other._M_get());
-else
-{
- if (__other._M_engaged)
-this->_M_construct(std::move(__other._M_get()));
- else
-this->_M_reset();
-}
-
-return *this;
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = std::move(__other._M_get());
+ else
+ {
+ if (__other._M_engaged)
+ this->_M_construct(std::move(__other._M_get()));
+ else
+ this->_M_reset();
+ }
+ return *this;
}
// [X.Y.4.2] Destructor.
@@ -373,35 +372,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Optional_base&
operator=(const _Optional_base& __other)
{
-if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = __other._M_get();
-else
-{
- if (__other._M_engaged)
-this->_M_construct(__other._M_get());
- else
-this->_M_reset();
-}
-
-return *this;
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = __other._M_get();
+ else
+ {
+ if (__other._M_engaged)
+ this->_M_construct(__other._M_get());
+ else
+ this->_M_reset();
+ }
+ return *this;
}
_Optional_base&
operator=(_Optional_base&& __other)
- noexcept(is_nothrow_move_constructible<_Tp>()
- && is_nothrow_move_assignable<_Tp>())
+ noexcept(__and_,
+ is_nothrow_move_assignable<_Tp>>())
{
-if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = std::move(__other._M_get());
-else
-{
- if (__other._M_engaged)
-this->_M_
[C11-atomic] Miscellaneous fixes 8/n
I've applied this further patch to C11-atomic branch to ensure that
the c11-atomic-exec-5.c test is only run where floating-point
exceptions are actually supported, to avoid spurious failures for
soft-float configurations (or any systems that have pthreads but not
).
2013-11-05 Joseph Myers
* lib/target-supports.exp
(check_effective_target_fenv_exceptions): New function.
* gcc.dg/atomic/c11-atomic-exec-5.c: Use
dg-require-effective-target fenv_exceptions.
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 204378)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -5477,3 +5477,40 @@ proc check_effective_target_aarch64_large { } {
return 0
}
}
+
+# Return 1 if is available with all the standard IEEE
+# exceptions and floating-point exceptions are raised by arithmetic
+# operations. (If the target requires special options for "inexact"
+# exceptions, those need to be specified in the testcases.)
+
+proc check_effective_target_fenv_exceptions {} {
+return [check_runtime fenv_exceptions {
+ #include
+ #include
+ #ifndef FE_DIVBYZERO
+ # error Missing FE_DIVBYZERO
+ #endif
+ #ifndef FE_INEXACT
+ # error Missing FE_INEXACT
+ #endif
+ #ifndef FE_INVALID
+ # error Missing FE_INVALID
+ #endif
+ #ifndef FE_OVERFLOW
+ # error Missing FE_OVERFLOW
+ #endif
+ #ifndef FE_UNDERFLOW
+ # error Missing FE_UNDERFLOW
+ #endif
+ volatile float a = 0.0f, r;
+ int
+ main (void)
+ {
+ r = a / a;
+ if (fetestexcept (FE_INVALID))
+ exit (0);
+ else
+ abort ();
+ }
+} "-std=gnu99"]
+}
Index: gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
===
--- gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c (revision 204378)
+++ gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c (working copy)
@@ -4,6 +4,7 @@
get properly cleared). */
/* { dg-do run } */
/* { dg-options "-std=c11 -pedantic-errors -pthread -D_POSIX_C_SOURCE=200809L"
} */
+/* { dg-require-effective-target fenv_exceptions } */
/* { dg-require-effective-target pthread } */
#include
--
Joseph S. Myers
jos...@codesourcery.com
Re: [Patch] Implementation of n3793
Hi,
On 11/05/2013 04:51 PM, Jonathan Wakely wrote:
On 3 November 2013 11:30, Paolo Carlini wrote:
On 11/03/2013 12:19 PM, Jonathan Wakely wrote:
Yes, Paolo pointed out these are failing on 32-bit targets, I've got a
patch coming. Luc is there any reason not to just replace all those large
constants with 0x1234ABCD, which fits in a long on 32-bit targets?
By the way, that's what I did/hacked in my local tree ;)
I needed some other changes, otherwise comparing optional{} ==
0x1234ABCD doesn't compile, because 0x1234ABCD is an int and
optional doesn't support comparisons with anything except T.
Yeah, I also had L suffixes.
Jon, I have got a bunch of other minor tweaks, from proper formatting of
conditional operator and some curly braces, to using __and_ and __not_ when
possible, and other thingies, like no full stops at the end of asserts and
throws. Passes testing. You may want to integrate it with your other changes
in testing... Or I can wait and apply it myself.
Here's the combined patch, tested x86_64-linux, 64-bit and 32-bit,
Thanks!
Paolo.
Re: libsanitizer merge from upstream r191666
On Tue, 2013-11-05 at 08:19 +0100, Jakub Jelinek wrote:
> On Mon, Nov 04, 2013 at 05:48:31PM -0800, Konstantin Serebryany wrote:
> > Hi Peter.
> > Does this also mean that asan in llvm trunk is broken for Power?
> > We'll need to fix it there too (or, in fact, first).
>
> I bet on all targets, not just PPC. By including kernel headers directly,
> you are entering very hairy maze of kernel header issues. So, for testing
> that you probably need to grab /usr/include/{linux,asm*} trees from various
> popular Linux distros (and various versions thereof), and do compile tests
> against all of those.
>
> Note, not even glibc itself includes , so the chances of that
> header actually working for you are low. glibc instead just defines the
> structures itself for each of the architectures.
I have to agree, including kernel header files is always frowned upon
and very risky. Jakub, do you think we should be doing the same thing
here that glibc does, namely having libsanitizer defining its own
structures?
Peter
[PATCH] gengtype: support empty GTY-marked structs
I'm attaching a patch which allows gengtype to handle GTY-marked structs
that have no fields. These are useful for supporting the "have a
subclass for every gimple code" idea from:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00057.html
(and am bootstrapping a patch that does that, which needed this one for
gengtype to cope).
Successfully bootstrapped®tested on x86_64-unknown-linux-gnu.
OK for trunk?
commit 030c6bd6856ee175adf7cf60cb44b2f2091360d0
Author: David Malcolm
Date: Mon Nov 4 21:21:23 2013 -0500
gengtype: support empty structs
gcc/
* gengtype-parse.c (struct_field_seq): Support empty structs.
diff --git a/gcc/gengtype-parse.c b/gcc/gengtype-parse.c
index f480503..8328e3a 100644
--- a/gcc/gengtype-parse.c
+++ b/gcc/gengtype-parse.c
@@ -730,7 +730,7 @@ declarator (type_p ty, const char **namep, options_p *optsp,
(
type bitfield ';'
| type declarator bitfield? ( ',' declarator bitfield? )+ ';'
- )+
+ )*
Knows that such declarations must end with a close brace (or,
erroneously, at EOF).
@@ -744,7 +744,7 @@ struct_field_seq (void)
const char *name;
bool another;
- do
+ while (token () != '}' && token () != EOF_TOKEN)
{
ty = type (&opts, true);
@@ -787,7 +787,6 @@ struct_field_seq (void)
}
while (another);
}
- while (token () != '}' && token () != EOF_TOKEN);
return nreverse_pairs (f);
}
[PATCH, i386]: Enable soft-float multilibs for x86-32 RTEMS
Hello!
Attached patch enables soft-float multilibs for x86-32 RTEMS. The
patch activates SFmode and DFmode soft-float support routines. The
XFmode is mapped to DFmode due to lack of XFmode support in
soft-float. We already disable FPU return registers for -mno-80387, so
ABI is already changed for soft-float. The XFmode->DFmode mapping just
rides on this change.
gcc/
2013-11-05 Uros Bizjak
* config/i386/i386-c.c (ix86_target_macros): Define _SOFT_FLOAT
for !TARGET_80387.
* config/i386/rtemself.h (TARGET_OS_CPP_BUILTINS): Do not define
_SOFT_FLOAT here.
(LONG_DOUBLE_TYPE_SIZE): New define.
(LIBGCC2_LONG_DOUBLE_TYPE_SIZE): Ditto.
libgcc/
2013-11-05 Uros Bizjak
* config/i386/32/sfp-machine.h (_FP_MUL_MEAT_S): Define.
(_FP_MUL_MEAT_D): Ditto.
(_FP_DIV_MEAT_S): Ditto.
(_FP_DIV_MEAT_D): Ditto.
* config.host (i[34567]86-*-rtems*): Remove i386/t-softfp, add
t-softfp-sfdf and t-softfp to tmake_file.
Patch was tested by Ralf on RTEMS (currently, -march=pentiumpro
multilib ICEs during build due to PR 58853 [1]).
It should be relatively straightforward to implement x86-32 Android
soft-fp multilib following the above RTEMS example. Let's leave this
implementation to an interested reader ;)
Patch was committed to mainline SVN.
[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58853
Uros.
Index: gcc/config/i386/i386-c.c
===
--- gcc/config/i386/i386-c.c(revision 204324)
+++ gcc/config/i386/i386-c.c(working copy)
@@ -467,6 +467,9 @@
builtin_define_std ("i386");
}
+ if (!TARGET_80387)
+cpp_define (parse_in, "_SOFT_FLOAT");
+
if (TARGET_LONG_DOUBLE_64)
cpp_define (parse_in, "__LONG_DOUBLE_64__");
Index: gcc/config/i386/rtemself.h
===
--- gcc/config/i386/rtemself.h (revision 204324)
+++ gcc/config/i386/rtemself.h (working copy)
@@ -26,7 +26,15 @@
builtin_define ("__rtems__"); \
builtin_define ("__USE_INIT_FINI__"); \
builtin_assert ("system=rtems");\
- if (!TARGET_80387) \
- builtin_define ("_SOFT_FLOAT"); \
} \
while (0)
+
+#undef LONG_DOUBLE_TYPE_SIZE
+#define LONG_DOUBLE_TYPE_SIZE (TARGET_80387 ? 80 : 64)
+
+#undef LIBGCC2_LONG_DOUBLE_TYPE_SIZE
+#ifdef _SOFT_FLOAT
+#define LIBGCC2_LONG_DOUBLE_TYPE_SIZE 64
+#else
+#define LIBGCC2_LONG_DOUBLE_TYPE_SIZE 80
+#endif
Index: gcc/config/i386/t-rtems
===
--- gcc/config/i386/t-rtems (revision 204324)
+++ gcc/config/i386/t-rtems (working copy)
@@ -17,9 +17,9 @@
#
Re: [PATCH, i386]: Enable soft-float multilibs for x86-32 RTEMS
On 11/5/2013 10:26 AM, Uros Bizjak wrote: > On Tue, Nov 5, 2013 at 5:23 PM, Uros Bizjak wrote: > >> Attached patch enables soft-float multilibs for x86-32 RTEMS. The >> patch activates SFmode and DFmode soft-float support routines. The >> XFmode is mapped to DFmode due to lack of XFmode support in >> soft-float. We already disable FPU return registers for -mno-80387, so >> ABI is already changed for soft-float. The XFmode->DFmode mapping just >> rides on this change. > > Please note that the change to gcc/config/i386/t-rtems, as included in > the patch, was only for reference and was not committed to SVN. Unless someone objects, Ralf tested them. Feel free to commit them. Thanks for fixing this. I assumed x86-32 soft-float was dead and buried. FWIW this configuration is easily testable on RTEMS or bare metal configurations using qemu. > Uros. > -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherr...@oarcorp.comOn-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available(256) 722-9985
Re: [PATCH, i386]: Enable soft-float multilibs for x86-32 RTEMS
On Tue, Nov 5, 2013 at 5:23 PM, Uros Bizjak wrote: > Attached patch enables soft-float multilibs for x86-32 RTEMS. The > patch activates SFmode and DFmode soft-float support routines. The > XFmode is mapped to DFmode due to lack of XFmode support in > soft-float. We already disable FPU return registers for -mno-80387, so > ABI is already changed for soft-float. The XFmode->DFmode mapping just > rides on this change. Please note that the change to gcc/config/i386/t-rtems, as included in the patch, was only for reference and was not committed to SVN. Uros.
Re: [RFC] libgcov.c re-factoring and offline profile-tool
Those functions are not referencing each other, but they get used in different logical groups, so at least control granularity can be improved. For instance, 1) all gcov user interfaces such as gcov_dump, gcov_flush, gcov_reset are either not used at all, or all of them are used -- there is no need to split this group. After Rong's refactoring (move them into a separate file), the guarding macro can be removed for them 2) _gcov_merge_add is used by 4 different profilers, so it is almost always linked in It is unclear how other value profilers are used on other targets. For x86, they are on by default, so the profiler bodies and merge functions are also always linked in. David On Tue, Nov 5, 2013 at 1:23 AM, Jakub Jelinek wrote: > On Tue, Nov 05, 2013 at 10:18:50AM +0100, Jan Hubicka wrote: >> > I wonder if it makes sense to get rid of the conditional compile such as >> > >> > #ifdef L_gcov_x >> > .. >> > >> > #endif >> > >> > This has the advantage of producing slightly smaller instrumented >> > binary, but this benefit can also be achieved via -ffunction-sections >> > and let linker to garbage collect unused functions. >> > >> > (probably as a follow up if it makes sense). >> >> I believe we use this funny scheme primarily for targets that have no >> function >> section support (and also the hand made splitting can be more sane). >> For a low-level library like libgcov is that is supposed to have small >> footprint, >> I guess the current scheme is resonable thing to do > > I think the #ifdef L* stuff is there mainly so that we have small > granularity in the *.a libraries (one function per object file), so that one > links only what is really required (at least, that is why libgcc.a does > that). Of course, if there are many interdependencies between the functions > and you always essentially link in everything, the usefulness of that is > lower. > > Jakub
Re: [PATCH] gengtype: support empty GTY-marked structs
On 11/05/13 09:20, David Malcolm wrote: I'm attaching a patch which allows gengtype to handle GTY-marked structs that have no fields. These are useful for supporting the "have a subclass for every gimple code" idea from: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00057.html (and am bootstrapping a patch that does that, which needed this one for gengtype to cope). Successfully bootstrapped®tested on x86_64-unknown-linux-gnu. OK for trunk? OK. jeff
