Re: [PATCH] [asan] Fix for PR asan/56330

2013-02-16 Thread Jakub Jelinek
On Fri, Feb 15, 2013 at 06:01:05PM -0500, Jack Howarth wrote:
> On Fri, Feb 15, 2013 at 09:54:19PM +0100, Dodji Seketeli wrote:
> FAIL: c-c++-common/asan/no-redundant-instrumentation-7.c  -O0   
> scan-tree-dump-times asan0 "__builtin___asan_report_load" 6
> 
> at both -m32 and -m64. The no-redundant-instrumentation-7.s from the failing 
> -m64
> test case is attached, generated with...

I think
void
foo  (int *a, char *b, char *c)
{
  __builtin_memcmp (s.a, e, 100);
  __builtin_memcmp (s.a, e, 200);
}
is problematic, for -O1 both calls would be definitely removed, because they
are pure, and even at -O0 I guess they might be expanded into nothing.
Perhaps
int
foo ()
{
  int i = __builtin_memcmp (s.a, e, 100);
  i += __builtin_memcmp (s.a, e, 200);
  return i;
}
or similar would work better.  And for pr56330.c testcase, which is there to
verify that we don't ICE on it and I believe there was important that both
builtins are adjacent, perhaps it should be __builtin_memcpy instead.

Jakub


Re: [PATCH] [asan] Fix for PR asan/56330

2013-02-16 Thread Dodji Seketeli
Jakub Jelinek  writes:

> On Fri, Feb 15, 2013 at 06:01:05PM -0500, Jack Howarth wrote:
>> On Fri, Feb 15, 2013 at 09:54:19PM +0100, Dodji Seketeli wrote:
>> FAIL: c-c++-common/asan/no-redundant-instrumentation-7.c  -O0   
>> scan-tree-dump-times asan0 "__builtin___asan_report_load" 6
>> 
>> at both -m32 and -m64. The no-redundant-instrumentation-7.s from the failing 
>> -m64
>> test case is attached, generated with...
>
> I think
> void
> foo  (int *a, char *b, char *c)
> {
>   __builtin_memcmp (s.a, e, 100);
>   __builtin_memcmp (s.a, e, 200);
> }
> is problematic, for -O1 both calls would be definitely removed, because they
> are pure, and even at -O0 I guess they might be expanded into nothing.
> Perhaps
> int
> foo ()
> {
>   int i = __builtin_memcmp (s.a, e, 100);
>   i += __builtin_memcmp (s.a, e, 200);
>   return i;
> }
> or similar would work better.  And for pr56330.c testcase, which is there to
> verify that we don't ICE on it and I believe there was important that both
> builtins are adjacent, perhaps it should be __builtin_memcpy instead.

Right.  My first bootstrap actually caught this, I updated the patch
locally to just modify that test and sent out the wrong one.  Sorry for
this.  This is the patch that actually passed bootstrap.

gcc/
* asan.c (get_mem_refs_of_builtin_call): White space and style
cleanup.
(instrument_mem_region_access): Do not forget to always put
instrumentation of the of 'base' and 'base + len' in a "if (len !=
0) statement, even for cases where either 'base' or 'base + len'
are not instrumented -- because they have been previously
instrumented.  Simplify the logic by putting all the statements
instrument 'base + len' inside a sequence, and then insert that
sequence right before the current insertion point.  Then, to
instrument 'base + len', just get an iterator on that statement.
And do not forget to update the pointer to iterator the function
received as argument.

gcc/testsuite/

* c-c++-common/asan/no-redundant-instrumentation-4.c: New test file.
* c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise.
* c-c++-common/asan/pr56330.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-1.c (test1):
Ensure the size argument of __builtin_memcpy is a constant.
---
 gcc/ChangeLog  | 16 
 gcc/asan.c | 97 --
 gcc/testsuite/ChangeLog| 12 +++
 .../asan/no-redundant-instrumentation-1.c  |  2 +-
 .../asan/no-redundant-instrumentation-4.c  | 13 +++
 .../asan/no-redundant-instrumentation-5.c  | 13 +++
 .../asan/no-redundant-instrumentation-6.c  | 14 
 .../asan/no-redundant-instrumentation-7.c  | 23 +
 .../asan/no-redundant-instrumentation-8.c  | 14 
 gcc/testsuite/c-c++-common/asan/pr56330.c  | 23 +
 10 files changed, 182 insertions(+), 45 deletions(-)
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pr56330.c

diff --git a/gcc/asan.c b/gcc/asan.c
index a569479..67236a9 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -747,20 +747,17 @@ get_mem_refs_of_builtin_call (const gimple call,
 
   got_reference_p = true;
 }
-else
-  {
-   if (dest)
- {
-   dst->start = dest;
-   dst->access_size = access_size;
-   *dst_len = NULL_TREE;
-   *dst_is_store = is_store;
-   *dest_is_deref = true;
-   got_reference_p = true;
- }
-  }
+  else if (dest)
+{
+  dst->start = dest;
+  dst->access_size = access_size;
+  *dst_len = NULL_TREE;
+  *dst_is_store = is_store;
+  *dest_is_deref = true;
+  got_reference_p = true;
+}
 
-return got_reference_p;
+  return got_reference_p;
 }
 
 /* Return true iff a given gimple statement has been instrumented.
@@ -1535,8 +1532,15 @@ instrument_mem_region_access (tree base, tree len,
 
   /* If the beginning of the memory region has already been
  instrumented, do not instrument it.  */
-  if (has_mem_ref_been_instrumented (base, 1))
-goto after_first_instrumentation;
+  bool start_instrumented = has_mem_ref_been_instrumented (base, 1);
+
+  /* If the end of

Re: [PATCH] [asan] Fix for PR asan/56330

2013-02-16 Thread Jakub Jelinek
On Sat, Feb 16, 2013 at 09:44:51AM +0100, Dodji Seketeli wrote:
> Right.  My first bootstrap actually caught this, I updated the patch
> locally to just modify that test and sent out the wrong one.  Sorry for
> this.  This is the patch that actually passed bootstrap.

Ok.  Actually, please change gcc/testsuite/c-c++-common/asan/pr56330.c
to the same thing, I've just verified it ICEs the same way without the patch
even when it is
int
foo (void)
{
  int d = __builtin_memcmp (s.a, e, 100);
  d += __builtin_memcmp (s.a, e, 200);
  return d;
}

(while with
  __builtin_memcpy (s.a, e, 100);
  __builtin_memcpy (s.a, e, 200);
it doesn't ICE without the patch).
While pr56330.c test look to be redundant, it is at least tortured at all
compilation options, so it tests something that the other tests don't.

No need to rebootstrap/retest.

Jakub


Re: [PATCH] [asan] Fix for PR asan/56330

2013-02-16 Thread Dodji Seketeli
Jakub Jelinek  writes:

> On Sat, Feb 16, 2013 at 09:44:51AM +0100, Dodji Seketeli wrote:
>> Right.  My first bootstrap actually caught this, I updated the patch
>> locally to just modify that test and sent out the wrong one.  Sorry for
>> this.  This is the patch that actually passed bootstrap.
>
> Ok.  Actually, please change gcc/testsuite/c-c++-common/asan/pr56330.c
> to the same thing,

Oops, right.  Done below.

> I've just verified it ICEs the same way without the patch
> even when it is
> int
> foo (void)
> {
>   int d = __builtin_memcmp (s.a, e, 100);
>   d += __builtin_memcmp (s.a, e, 200);
>   return d;
> }

Yes, I have just verified this too, thanks.

>
> (while with
>   __builtin_memcpy (s.a, e, 100);
>   __builtin_memcpy (s.a, e, 200);
> it doesn't ICE without the patch).

Hmmh, interesting.

> While pr56330.c test look to be redundant, it is at least tortured at all
> compilation options, so it tests something that the other tests don't.

Yes, that is why I kept it in the set of tests.

> No need to rebootstrap/retest.

I re-ran the tests locally w/o bootstrap though.  I am going to
commit this now.  Thanks.

gcc/
* asan.c (get_mem_refs_of_builtin_call): White space and style
cleanup.
(instrument_mem_region_access): Do not forget to always put
instrumentation of the of 'base' and 'base + len' in a "if (len !=
0) statement, even for cases where either 'base' or 'base + len'
are not instrumented -- because they have been previously
instrumented.  Simplify the logic by putting all the statements
instrument 'base + len' inside a sequence, and then insert that
sequence right before the current insertion point.  Then, to
instrument 'base + len', just get an iterator on that statement.
And do not forget to update the pointer to iterator the function
received as argument.

gcc/testsuite/

* c-c++-common/asan/no-redundant-instrumentation-4.c: New test file.
* c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise.
* c-c++-common/asan/pr56330.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-1.c (test1):
Ensure the size argument of __builtin_memcpy is a constant.
---
 gcc/ChangeLog  | 16 
 gcc/asan.c | 97 --
 gcc/testsuite/ChangeLog| 12 +++
 .../asan/no-redundant-instrumentation-1.c  |  2 +-
 .../asan/no-redundant-instrumentation-4.c  | 13 +++
 .../asan/no-redundant-instrumentation-5.c  | 13 +++
 .../asan/no-redundant-instrumentation-6.c  | 14 
 .../asan/no-redundant-instrumentation-7.c  | 23 +
 .../asan/no-redundant-instrumentation-8.c  | 14 
 gcc/testsuite/c-c++-common/asan/pr56330.c  | 24 ++
 10 files changed, 183 insertions(+), 45 deletions(-)
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pr56330.c

diff --git a/gcc/asan.c b/gcc/asan.c
index a569479..67236a9 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -747,20 +747,17 @@ get_mem_refs_of_builtin_call (const gimple call,
 
   got_reference_p = true;
 }
-else
-  {
-   if (dest)
- {
-   dst->start = dest;
-   dst->access_size = access_size;
-   *dst_len = NULL_TREE;
-   *dst_is_store = is_store;
-   *dest_is_deref = true;
-   got_reference_p = true;
- }
-  }
+  else if (dest)
+{
+  dst->start = dest;
+  dst->access_size = access_size;
+  *dst_len = NULL_TREE;
+  *dst_is_store = is_store;
+  *dest_is_deref = true;
+  got_reference_p = true;
+}
 
-return got_reference_p;
+  return got_reference_p;
 }
 
 /* Return true iff a given gimple statement has been instrumented.
@@ -1535,8 +1532,15 @@ instrument_mem_region_access (tree base, tree len,
 
   /* If the beginning of the memory region has already been
  instrumented, do not instrument it.  */
-  if (has_mem_ref_been_instrumented (base, 1))
-goto after_first_instrumentation;
+  bool start_instrumented = has_mem_ref_been_instrumented (base, 1);
+
+  /* If the end of the memory region has already been instrumented, do
+ not instrument it. */
+  tree end = asan_mem_ref_get_end (base, len

[PATCH] Add new debug_bb_range debugging function

2013-02-16 Thread Dodji Seketeli
Hello,

In my first foray of debugging gimple stuff, I felt the need for a
function function to dump a range of basic block from number N to P to
stderr.  I find this a bit more handy than calling debug_bb_n on each
basic block instead.

I understand this is material for 4.9, so if you agree I'll stage it in
my queue of patches for when stage1 opens.

Is this OK, or is there a function that does this already (or something
else :))?

Thanks.

Tested on x86_64-unknown-linux-gnu against trunk.

gcc/

* basic-block.h (debug_bb_range): Declare ...
* cfg.c (debug_bb_range): ... new function.
---
 gcc/basic-block.h |  1 +
 gcc/cfg.c | 15 +++
 2 files changed, 16 insertions(+)

diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index 90eb57b..8407c4a 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -752,6 +752,7 @@ extern bool predictable_edge_p (edge);
 extern void init_flow (struct function *);
 extern void debug_bb (basic_block);
 extern basic_block debug_bb_n (int);
+extern basic_block debug_bb_range (int, int);
 extern void dump_flow_info (FILE *, int);
 extern void expunge_block (basic_block);
 extern void link_block (basic_block, basic_block);
diff --git a/gcc/cfg.c b/gcc/cfg.c
index 9e4380c..d34f27e 100644
--- a/gcc/cfg.c
+++ b/gcc/cfg.c
@@ -663,6 +663,21 @@ debug_bb_n (int n)
   return bb;
 }
 
+/* Dumps cfg related information about basic blocks, from number 'S'
+   to number E, to stderr.  */
+
+DEBUG_FUNCTION basic_block
+debug_bb_range (int s, int e)
+{
+  basic_block bb =  NULL;
+  for (int i = s; i <= e; ++i)
+{
+  bb = BASIC_BLOCK (i);
+  debug_bb (bb);
+}
+  return bb;
+}
+
 /* Dumps cfg related information about basic block BB to OUTF.
If HEADER is true, dump things that appear before the instructions
contained in BB.  If FOOTER is true, dump things that appear after.
-- 
Dodji


Re: [PATCH] Add new debug_bb_range debugging function

2013-02-16 Thread Jakub Jelinek
On Sat, Feb 16, 2013 at 10:40:43AM +0100, Dodji Seketeli wrote:
> --- a/gcc/cfg.c
> +++ b/gcc/cfg.c
> @@ -663,6 +663,21 @@ debug_bb_n (int n)
>return bb;
>  }
>  
> +/* Dumps cfg related information about basic blocks, from number 'S'
> +   to number E, to stderr.  */
> +
> +DEBUG_FUNCTION basic_block
> +debug_bb_range (int s, int e)
> +{
> +  basic_block bb =  NULL;
> +  for (int i = s; i <= e; ++i)
> +{
> +  bb = BASIC_BLOCK (i);
> +  debug_bb (bb);

At some points after cfg changes, but before cfg cleanup there might be
gaps, so I think you want if (bb) debug_bb (bb);, otherwise it could crash
in that case.

Jakub


Re: [PATCH] Add new debug_bb_range debugging function

2013-02-16 Thread Dodji Seketeli
Jakub Jelinek  writes:

> On Sat, Feb 16, 2013 at 10:40:43AM +0100, Dodji Seketeli wrote:
>> --- a/gcc/cfg.c
>> +++ b/gcc/cfg.c
>> @@ -663,6 +663,21 @@ debug_bb_n (int n)
>>return bb;
>>  }
>>  
>> +/* Dumps cfg related information about basic blocks, from number 'S'
>> +   to number E, to stderr.  */
>> +
>> +DEBUG_FUNCTION basic_block
>> +debug_bb_range (int s, int e)
>> +{
>> +  basic_block bb =  NULL;
>> +  for (int i = s; i <= e; ++i)
>> +{
>> +  bb = BASIC_BLOCK (i);
>> +  debug_bb (bb);
>
> At some points after cfg changes, but before cfg cleanup there might be
> gaps, so I think you want if (bb) debug_bb (bb);, otherwise it could crash
> in that case.

Right.  I have updated the patch below for that and also to ensure that
we don't try to access a basic block with number >= last_basic_block.

Thanks.

gcc/

* basic-block.h (debug_bb_range): Declare ...
* cfg.c (debug_bb_range): ... new function.
---
 gcc/basic-block.h |  1 +
 gcc/cfg.c | 16 
 2 files changed, 17 insertions(+)

diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index 90eb57b..8407c4a 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -752,6 +752,7 @@ extern bool predictable_edge_p (edge);
 extern void init_flow (struct function *);
 extern void debug_bb (basic_block);
 extern basic_block debug_bb_n (int);
+extern basic_block debug_bb_range (int, int);
 extern void dump_flow_info (FILE *, int);
 extern void expunge_block (basic_block);
 extern void link_block (basic_block, basic_block);
diff --git a/gcc/cfg.c b/gcc/cfg.c
index 9e4380c..d72e075 100644
--- a/gcc/cfg.c
+++ b/gcc/cfg.c
@@ -663,6 +663,22 @@ debug_bb_n (int n)
   return bb;
 }
 
+/* Dumps cfg related information about basic blocks, from number 'S'
+   to number E, to stderr.  */
+
+DEBUG_FUNCTION basic_block
+debug_bb_range (int s, int e)
+{
+  basic_block bb =  NULL;
+  for (int i = s; i <= e && i < last_basic_block; ++i)
+{
+  bb = BASIC_BLOCK (i);
+  if (bb)
+   debug_bb (bb);
+}
+  return bb;
+}
+
 /* Dumps cfg related information about basic block BB to OUTF.
If HEADER is true, dump things that appear before the instructions
contained in BB.  If FOOTER is true, dump things that appear after.
-- 
1.8.1



-- 
Dodji


Re: PR target/52555: attribute optimize is overriding command line options

2013-02-16 Thread Richard Sandiford
Aldy Hernandez  writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index b203cdd..5e98485 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p)
>if (mips16_p)
>  {
>if (!mips16_globals)
> - mips16_globals = save_target_globals ();
> + {
> +   if (optimization_current_node != optimization_default_node)
> + {
> +   tree opts = optimization_current_node;
> +   /* Temporarily switch to the default optimization node,
> +  so that *this_target_optabs is set to the default,
> +  not reflecting whatever the first mips16 function
> +  uses for the optimize attribute.  */
> +   optimization_current_node = optimization_default_node;
> +   cl_optimization_restore
> + (&global_options,
> +  TREE_OPTIMIZATION (optimization_default_node));
> +   mips16_globals = save_target_globals ();
> +   optimization_current_node = opts;
> +   cl_optimization_restore (&global_options,
> +TREE_OPTIMIZATION (opts));
> + }
> +   else
> + mips16_globals = save_target_globals ();  
> + }

I think this should go in target-globals.c, maybe as
save_target_globals_default_opts.

> diff --git a/gcc/function.c b/gcc/function.c
> index 4ce2259..c5eea2e 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4400,6 +4400,31 @@ invoke_set_current_function_hook (tree fndecl)
>   }
>  
>targetm.set_current_function (fndecl);
> +
> +  if (opts == optimization_default_node)
> + this_fn_optabs = this_target_optabs;
> +  else
> + {
> +   struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
> +   if (fn->optabs == NULL)
> + {
> +   if (!SWITCHABLE_TARGET)
> + fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +   else
> + {
> +   if (this_target_optabs == &default_target_optabs)
> + fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +   else
> + {
> +   fn->optabs = (unsigned char *)
> + ggc_alloc_atomic (sizeof (struct target_optabs));
> +   init_all_optabs ((struct target_optabs *) fn->optabs);
> + }
> + }

Following on from Jakub's:

if (!SWITCHABLE_TARGET
|| this_target_optabs == &default_target_optabs)
  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

I'd prefer just:

if (this_target_optabs == &default_target_optabs)
  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

Looks good to me otherwise, thanks.

Sorry that SWITCHABLE_TARGETS has been so much hassle.  TBH, like Jakub
says in the PR, I was hoping things like the optimize attribute could
use the target_globals stuff too.  Today we just care about optabs,
but e.g. arm.c has:

  if (TARGET_THUMB1 && optimize_size)
{
  /* When optimizing for size on Thumb-1, it's better not
to use the HI regs, because of the overhead of
stacking them.  */
  for (regno = FIRST_HI_REGNUM;
   regno <= LAST_HI_REGNUM; ++regno)
fixed_regs[regno] = call_used_regs[regno] = 1;
}

which affects other cached global state like IRA tables.

The rtx_costs also often depend on optimize_size, and are cached in
various places like expmed.c.  E.g. for:

int
foo (int x, int y)
{
  return x * 10;
}

the -O2 version on MIPS32 is:

sll $2,$4,1
sll $4,$4,3
j   $31
addu$2,$2,$4

and the -Os version is:

li  $2,10
j   $31
mul $2,$4,$2

But even if an optimize attribute is added:

int __attribute__((optimize(2)))
foo (int x, int y)
{
  return x * 10;
}

what you get depends on whether -Os or -O2 was passed on the command line.
The attribute doesn't affect things either way.  The same thing happens
on x86_64.

So in the end I think we'll end up trying solve the same problem that
the SWITCHABLE_TARGETS stuff was trying to solve, but this time for
__attribute__((optimize)).

Richard


Re: PATCH: Correctly configure all big-endian ARM archs, not just arm*-*-linux-*.

2013-02-16 Thread Bernhard Reutner-Fischer

On 15 February 2013 19:28:29 Richard Earnshaw  wrote:

On 15/02/13 18:20, Seth LaForge wrote:
> Currently, for arm-* archs, TARGET_BIG_ENDIAN_DEFAULT is only set in
> the case branch for arm*-*-linux-*, not for other arm architectures.
> We're compiling for the TI TMS570, which is a big-endian processor,
> with target set to armeb-unknown-eabi.  The following patch moves the
> big-endian check out of the big architecture case, so that
> TARGET_BIG_ENDIAN_DEFAULT is consistently set for any armeb-*-*
> target.
>
> We've been using this with good results for over a year at Google on
> TMS570 processors.
>
> This fixes bug 52187 - armeb-unknown-eabi not recognized as big-endian.
>
> Seth LaForge
>
>
> gcc/
>  * config.gcc: Add TARGET_BIG_ENDIAN_DEFAULT=1 for all arm*b archs.
>
> diff -u -r gcc-4.8-20130210/gcc/config.gcc 
gcc-4.8-20130210.new/gcc/config.gcc

> --- gcc-4.8-20130210/gcc/config.gcc2013-02-08 08:02:47.0 -0800
> +++ gcc-4.8-20130210.new/gcc/config.gcc2013-02-14 16:37:14.282107219 -0800
> @@ -809,6 +809,13 @@
> ;;
>   esac
>
> +# Handle big-endian ARM architectures.
> +case ${target} in
> +arm*b-*-*)
> +  tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1"
> +  ;;

Not ok.  This would mismatch on

arm-blob-linux-gnueabi

and cause it to be treated as big-endian.


Sounds like a DUP of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16350
Is the missing hunk in by now (cannot look myself right now)?
Thanks,


R.





Sent with AquaMail for Android
http://www.aqua-mail.com




Re: GCC 4.8.0 Status Report (2013-02-14)

2013-02-16 Thread Gerald Pfeifer
I updated the main page accordingly.

Gerald

Index: index.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v
retrieving revision 1.869
diff -u -3 -p -r1.869 index.html
--- index.html  23 Jan 2013 20:02:00 -  1.869
+++ index.html  16 Feb 2013 15:42:38 -
@@ -194,7 +194,7 @@ Any additions?  Don't be shy, send them 
 
   Status:
   
-  http://gcc.gnu.org/ml/gcc/2013-01/msg00071.html";>2013-01-08
+  http://gcc.gnu.org/ml/gcc/2013-02/msg00177.html";>2013-02-14
   
   (regression fixes and docs only).
   


Re: [Patch, microblaze]: Add support for nested functions

2013-02-16 Thread Michael Eager

On 02/10/2013 10:37 PM, David Holsgrove wrote:

Add MicroBlaze support for nested functions.

Changelog

2013-02-11  Edgar E. Iglesias  

  * config/microblaze/microblaze.c
(microblaze_asm_trampoline_template): Replace
with a microblaze version.
(microblaze_trampoline_init): Adapt for microblaze.
  * config/microblaze/microblaze.h
(TRAMPOLINE_SIZE): Adapt for microblaze.


Committed revision 196103.

--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


Re: [Patch, microblaze]: Add TARGET_SUPPORTS_PIC check

2013-02-16 Thread Michael Eager

On 02/10/2013 10:38 PM, David Holsgrove wrote:

Add TARGET_SUPPORTS_PIC flag and check that the flag_pic = 2

Changelog

2013-02-11  Edgar E. Iglesias  

  * config/microblaze/linux.h (TARGET_SUPPORTS_PIC): Define as 1.
  * config/microblaze/microblaze.h (TARGET_SUPPORTS_PIC): Define as 1.
  * config/microblaze/microblaze.c (microblaze_option_override):
 Bail out early for PIC modes when target does not support PIC.

gcc/testsuite/Changelog

2013-02-11  Edgar E. Iglesias  

  * gcc.dg/20020312-2.c: Define MicroBlaze PIC register


Committed revision 196104.

--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


Re: [PATCH][www] Add loop case to non-bugs in bugs.html

2013-02-16 Thread Gerald Pfeifer
On Thu, 7 Feb 2013, Richard Biener wrote:
> As promised, a bugs.html entry.

Looks good to me.

Now, at the top of the page we have

  Before reporting that GCC compiles your code incorrectly, compile it
  with gcc -Wall -Wextra and see whether this shows anything
  wrong with your code.  Similarly, if compiling with
  -fno-strict-aliasing -fwrapv makes a difference, your code
  probably is not correct.

Should I add  -fno-aggressive-loop-optimizations  to the list in the
second sentence?

Gerald


Re: [PATCH][RFC] Add -fno-aggressive-loop-optimizations

2013-02-16 Thread Gerald Pfeifer
On Thu, 31 Jan 2013, Richard Biener wrote:
> +GCC now uses more a aggressive analysis to derive an upper bound for

I think "a " should be omitted here.

> +the number of iterations of loops using constraints imposed by language
> +standards.  This may cause non-conforming programs to no longer work as
> +expected, such as SPEC CPU 2006 464.h264ref and 416.gamess.  A new
> +option, -fno-aggressive-loop-optimizations, was added
> +to disable this aggressive analysis.

Personally I would reduce aggressiveness and omit "aggressive" in
the last line above. :-)

Gerald


[PATCH] Fix PR50293 - LTO plugin with space in path

2013-02-16 Thread Joey Ye
Mainline and 4.7 failed to use LTO when toolchain is installed to a path
with space in it. Resulting in error message like:

/ld.exe: c:/program: error loading plugin
collect2.exe: error: ld returned 1 exit status

Root cause is when GCC driver process specs, it doesn't handle plugin file
name properly. Here is a patch fixing it.

Bootstraped and make check on x86 and aarch32, no regression.

OK to trunk and 4.7?

ChangeLog:

PR lto/50293
* gcc.c (convert_white_space): New function.
(main): Handles white space in function name.

Index: gcc/gcc.c
===
--- gcc/gcc.c   (revision 195189)
+++ gcc/gcc.c   (working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char
**);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char * convert_white_space(char * orig);


 /* The Specs Language

@@ -6595,6 +6596,7 @@
X_OK, false);
   if (lto_wrapper_file)
 {
+  lto_wrapper_file = convert_white_space(lto_wrapper_file);
   lto_wrapper_spec = lto_wrapper_file;
   obstack_init (&collect_obstack);
   obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
  + strlen (fuse_linker_plugin), 0))
 #endif
{
- linker_plugin_file_spec = find_a_file (&exec_prefixes,
+ char * temp_spec = find_a_file (&exec_prefixes,
 LTOPLUGINSONAME, R_OK,
 false);
- if (!linker_plugin_file_spec)
+ if (!temp_spec)
fatal_error ("-fuse-linker-plugin, but %s not found",
 LTOPLUGINSONAME);
+ linker_plugin_file_spec = convert_white_space(temp_spec);
}
 #endif
  lto_gcc_spec = argv[0];
@@ -8506,3 +8509,30 @@
   free (name);
   return result;
 }
+
+/* Insert back slash before spaces in a string, to avoid path
+   that has space in it broken into multiple arguments.  */
+
+static char * convert_white_space(char * orig)
+{
+  int len, number_of_space = 0;
+  char *p = orig;
+  if (orig == NULL) return NULL;
+
+  for (len=0; p[len]; len++)
+if (p[len] == ' ' || p[len] == '\t') number_of_space ++;
+
+  if (number_of_space)
+{
+  char * new_spec = (char *)xmalloc(len + number_of_space + 1);
+  int j,k;
+  for (j=0, k=0; j<=len; j++, k++)
+   {
+ if (p[j] == ' ' || p[j] == '\t') new_spec[k++]='\\';
+ new_spec[k] = p[j];
+   }
+  free(CONST_CAST(char *, orig));
+  return new_spec;
+  }
+  else return orig;
+}







Re: [PING^5] PR 54805: __gthread_tsd* in vxlib-tls.c

2013-02-16 Thread Maxim Kuvyrkov
On 14/02/2013, at 10:18 AM, rbmj wrote:

> On 18-Jan-13 20:35, Maxim Kuvyrkov wrote:
>> On 19/01/2013, at 9:18 AM, rbmj wrote:
>> 
>  -150,7 +158,7 @@ static __gthread_once_t tls_init_guard =
> need to read tls_keys.dtor[key] atomically.  */
> 
>  static void
> -tls_delete_hook (void *tcb ATTRIBUTE_UNUSED)
> +tls_delete_hook (void *tcb)
 
 Don't remove ATTRIBUTE_UNUSED.  TCB was and will remain unused #ifdef 
 __RTP__.
 
>>> 
>>> And #ifndef __RTP__ ?
>> 
>> No, simply leave that line as is.  ATTRIBUTE_UNUSED tells the compiler that 
>> a variable can be unused, but not necessarily is unused.  It's fine to have 
>> this attribute set on variables that are used under certain preprocessor 
>> configurations.
>> 
> 
> Seems like I kept this email in drafts and never sent it out...
> 
> Sorry about that.
> 
> Here's the updated, (trivial) patch.

Thanks.  I'll apply this once 4.8 branches and trunk is back into development 
mode.

--
Maxim Kuvyrkov