RE: [PATCH] [MIPS] fix mips_prepend insn.

[Fu, Chao-Ying]

Liu [pro...@gmail.com] wrote:

> OK, I get.
> But, sorry, my mips64dspr2 patch has be done...
> Should I summit it?

  I just wonder what version of the MIPS64 DSP/DSPr2 spec that you implemented. 
 Do you have a target CPU that has these MIPS64 DSP/DSPr2 instructions?  My 
concern is that the latest spec removed a lot of new instructions.  Thanks!

Regards,
Chao-ying

Re: [PATCH] Fix RTL sharing bug in loop unswitching (PR rtl-optimization/52092)

[Zdenek Dvorak]

Hi,

> It seems loop-iv.c happily creates shared RTL in lots of places,
> loop-unroll.c solves that by unsharing everything it adds, this is
> an attempt to do the same in unswitching to unshare everything iv_analyze
> came up with.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I would suggest to move the unsharing to the point where it is clear that
the rtl will added, i.e.,

loop-unswitch.c:370
  nloop = unswitch_loop (loop, bbs[i], cond, cinsn);
->
  nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn);

Zdenek

Re: [PATCH] [MIPS] fix mips_prepend insn.

[Liu]

On Fri, Feb 3, 2012 at 3:59 PM, Fu, Chao-Ying  wrote:
> Liu [pro...@gmail.com] wrote:
>
>> OK, I get.
>> But, sorry, my mips64dspr2 patch has be done...
>> Should I summit it?
>
>  I just wonder what version of the MIPS64 DSP/DSPr2 spec that you 
> implemented.  Do you have a target CPU that has these MIPS64 DSP/DSPr2 
> instructions?  My concern is that the latest spec removed a lot of new 
> instructions.  Thanks!
>

MIPS® Architecture for Programmers
VolumeIV-e: The MIPS® DSP Application-
Specific Extension to the MIPS64®
Architecture
Document Number: MD00375
Revision 2.34
May 6, 2011

I have lots of MIPS64 CPU but no DSP:(

> Regards,
> Chao-ying

Re: [patch libjava]: PR 48512 Avoid crtmt.o in startfile-spec for w64 based mingw toolchains

[Andrew Haley]

On 02/02/2012 06:47 PM, Kai Tietz wrote:
> 2012-02-02  Kai Tietz  
> 
> PR libjava/48512
> * configure.ac (THREADSTARTFILESPEC): Don't add crtmet.o file for
> w64 windows targets.
> * configure: Regenerated.
> 
> Tested for i686-w64-mingw32, and i686-pc-mingw32.  Ok for apply?
> 

OK, thanks.

Andrew.

Re: [PATCH] fix PR51910, take 2

[Jakub Jelinek]

On Thu, Feb 02, 2012 at 09:10:50PM -0700, Sandra Loosemore wrote:
> --- gcc/testsuite/g++.dg/torture/pr51910.C(revision 0)
> +++ gcc/testsuite/g++.dg/torture/pr51910.C(revision 0)
> @@ -0,0 +1,19 @@
> +// PR c++/51910 
> +// Check that -frepo works in the presence of linker symbol demangling.
> +//
> +// { dg-options "-frepo -Wl,--demangle" }

I don't think you want to use this in the testsuite, that assumes the
linker handles it, which is not the case for non-GNU linkers.

Anyway, here is the tweaking all the symbols that demangle the same
equally patch (untested so far) as an alternative.  On the example it
unfortunately causes also the D2 dtor into the link which wasn't there
otherwise (and with -Wl,--no-demangle).  While D2 dtors are generally small,
perhaps it will sometimes force into the link also ctor or dtor variants
that couldn't be aliased together.

I wonder if it wouldn't be better to just use a linker plugin for -frepo
if the linker supports plugins.

2012-02-03  Jakub Jelinek  

PR c++/51910
* tlink.c (demangleq_equiv): New typedef.
(demangled): Add equiv field.
(demangle_new_symbols): Chain mangled symbols that demangle the
same into dem->equiv chain.
(handle_sym): New function.
(scan_linker_output): Use it.  Call handle_sym on all other symbols
that demangle the same.

--- gcc/tlink.c.jj  2012-01-30 00:08:28.0 +0100
+++ gcc/tlink.c 2012-02-03 10:13:08.799161429 +0100
@@ -67,10 +67,17 @@ typedef struct file_hash_entry
   int tweaking;
 } file;
 
+typedef struct demangled_equiv_entry
+{
+  const char *mangled;
+  struct demangled_equiv_entry *next;
+} demangled_equiv;
+
 typedef struct demangled_hash_entry
 {
   const char *key;
   const char *mangled;
+  demangled_equiv *equiv;
 } demangled;
 
 /* Hash and comparison functions for these hash tables.  */
@@ -598,10 +605,43 @@ demangle_new_symbols (void)
continue;
 
   dem = demangled_hash_lookup (p, true);
-  dem->mangled = sym->key;
+  if (dem->mangled)
+   {
+ demangled_equiv *equiv = XCNEW (demangled_equiv);
+ equiv->mangled = sym->key;
+ equiv->next = dem->equiv;
+ dem->equiv = equiv;
+   }
+  else
+dem->mangled = sym->key;
 }
 }
 
+/* Helper function of scan_linker_output.  Handle a single symbol SYM.
+   Returns 1 if an error happened, 0 otherwise.  */
+
+static bool
+handle_sym (FILE *stream, symbol *sym)
+{
+  if (sym && sym->tweaked)
+{
+  error ("'%s' was assigned to '%s', but was not defined "
+"during recompilation, or vice versa",
+sym->key, sym->file->key);
+  fclose (stream);
+  return true;
+}
+  if (sym && !sym->tweaking)
+{
+  if (tlink_verbose >= 2)
+   fprintf (stderr, _("collect: tweaking %s in %s\n"),
+sym->key, sym->file->key);
+  sym->tweaking = 1;
+  file_push (sym->file);
+}
+  return false;
+}
+
 /* Step through the output of the linker, in the file named FNAME, and
adjust the settings for each symbol encountered.  */
 
@@ -719,7 +759,19 @@ scan_linker_output (const char *fname)
  *q = 0;
  dem = demangled_hash_lookup (p, false);
  if (dem)
-   sym = symbol_hash_lookup (dem->mangled, false);
+   {
+ if (dem->equiv)
+   {
+ demangled_equiv *equiv;
+ for (equiv = dem->equiv; equiv; equiv = equiv->next)
+   {
+ sym = symbol_hash_lookup (equiv->mangled, false);
+ if (handle_sym (stream, sym))
+   return 0;
+   }
+   }
+ sym = symbol_hash_lookup (dem->mangled, false);
+   }
  else
{
  if (!strncmp (p, USER_LABEL_PREFIX,
@@ -730,22 +782,8 @@ scan_linker_output (const char *fname)
}
}
 
-  if (sym && sym->tweaked)
-   {
- error ("'%s' was assigned to '%s', but was not defined "
-"during recompilation, or vice versa",
-sym->key, sym->file->key);
- fclose (stream);
- return 0;
-   }
-  if (sym && !sym->tweaking)
-   {
- if (tlink_verbose >= 2)
-   fprintf (stderr, _("collect: tweaking %s in %s\n"),
-sym->key, sym->file->key);
- sym->tweaking = 1;
- file_push (sym->file);
-   }
+  if (handle_sym (stream, sym))
+   return 0;
 
   obstack_free (&temporary_obstack, temporary_firstobj);
 }

Jakub

Re: [PATCH] Fix RTL sharing bug in loop unswitching (PR rtl-optimization/52092)

[Richard Guenther]

On Thu, 2 Feb 2012, Jakub Jelinek wrote:

> Hi!
> 
> It seems loop-iv.c happily creates shared RTL in lots of places,
> loop-unroll.c solves that by unsharing everything it adds, this is
> an attempt to do the same in unswitching to unshare everything iv_analyze
> came up with.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2012-02-02  Jakub Jelinek  
> 
>   PR rtl-optimization/52092
>   * loop-unswitch.c (may_unswitch_on): Call copy_rtx_if_shared
>   on get_iv_value result.
> 
>   * gcc.c-torture/compile/pr52092.c: New test.
> 
> --- gcc/loop-unswitch.c.jj2010-06-28 15:36:30.0 +0200
> +++ gcc/loop-unswitch.c   2012-02-02 14:00:55.127749909 +0100
> @@ -204,6 +204,7 @@ may_unswitch_on (basic_block bb, struct
>   return NULL_RTX;
>  
>op[i] = get_iv_value (&iv, const0_rtx);
> +  op[i] = copy_rtx_if_shared (op[i]);
>  }
>  
>mode = GET_MODE (op[0]);
> --- gcc/testsuite/gcc.c-torture/compile/pr52092.c.jj  2012-02-02 
> 14:11:37.722031008 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr52092.c 2012-02-02 
> 14:11:14.0 +0100
> @@ -0,0 +1,25 @@
> +/* PR rtl-optimization/52092 */
> +
> +int a, b, c, d, e, f, g;
> +
> +void
> +foo (void)
> +{
> +  for (;;)
> +{
> +  int *h = 0;
> +  int i = 3;
> +  int **j = &h;
> +  if (e)
> + {
> +   c = d || a;
> +   g = c ? a : b;
> +   if ((char) (i * g))
> + {
> +   h = &f;
> +   *h = 0;
> + }
> +   **j = 0;
> + }
> +}
> +}
> 
>   Jakub
> 
> 

-- 
Richard Guenther 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] MIPS16 TLS support for GCC

[Chung-Lin Tang]

Hi Richard,

Sorry for the delay to respond, and thanks for revising and committing
the changes to trunk. The revised changes look much cleaner :)

About the other concerns:

> (2) is interesting if there is also a way to build those MIPS16 libraries
> out of the box.  I'd like such a mechanism to be added at the same time,
> so that the new support is easy to test.  This is still a 4.7 candidate
> if it can be done in time, although it's probably a little tight.

I assume you mean the libgomp patch? That's mostly a potential bug fix;
as for MIP16 multilib additions, so far I haven't added any to the
default mips*-linux* configs, as I guess this should be vendor-stuff
(though the current additions should clear away any obstacles)

> You've given the built-in function the generic name __builtin_thread_pointer.
> I agree that's a good idea, but I think we should also _treat_ it as a generic
> function, and make it available on all targets.  The actual implementation
> can be delegated to a target hook.  That makes (3) 4.8 material.

Actually, I named and added __builtin_thread_pointer() only because it
was needed for building glibc (to avoid using 32-bit asm in MIPS16
mode), and because some other targets also have it (I'm sure ARM also
has it, and at least one other)

> (Incidentally, I don't think it's correct to define the builtin if TLS
> isn't available, so if we did keep it as a MIPS-specific function,
> d->avail would need to be nonnull.  There would need to be some other
> way of indicating that MIPS16 was OK.)

The function is essentially a wrapper for mips_get_tp(), which I don't
see any guarding for the no TLS case? FWIW, TARGET_HAVE_TLS is just
defined as HAVE_AS_TLS for MIPS...

>> The __mips16_rdhwr() function is not intended for general use, just
>> tightly coupled compiler runtime support; therefore, it is only linked
>> statically from libgcc.a, not exported from shared libgcc.
> 
> Well, a fair bit of libgcc is tightly-coupled compiler runtime support.
> I don't really see any need to handle the new function differently from
> the other __mips16 wrappers.  It's not like we're gaining any benefit in
> the PIC call overhead: we can't turn JALRs into branches like we can for
> nearby non-MIPS16-to-non-MIPS16 calls, so PIC calls will still go via
> the GOT.  And we're not gaining any benefit in terms of ABI baggage either.
> Future libgcc(.a)s would need to continue providing the function as
> specified now, in order for future gccs to be able to link library
> archives built with 4.7.
> 
> By making the function hidden, we lose the important ability to replace
> all instances of it.  E.g. it isn't inconceivable that someone will find
> a more efficient way to implement the function in cases where the RDHWR
> is emulated (perhaps on a kernel other than Linux -- this support isn't
> specific to *-linux-gnu).  Being able to replace all instances of a
> function is useful for other things, such as profiling, debugging,
> or working around processor errata.

We touched this internally as well, but interestingly came to the
opposite conclusion :)  We're not sure the __mips16_rdhwr() is
ultimately the best choice of API, plus the non-standardness of the
calling convention, hence simply decided to hide it from libgcc_s.so, in
case we need to change the MIPS16 TLS interface.

On the issue of hiding stuff in mips16.S, it may be suitable to raise
this issue here: there is a problem when MIPS16 hard-float calls go
through PLTs, because the hard-float stub functions in mips16.S use
v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
by BFD. (I think I described the scenario mostly correct, CCing Maciej
here who worked on this issue)  I think the solution was to mark a large
portion of the mips16.S functions as all hidden.

>> 5) The libgomp MIPS futex.h header has been adjusted; sys_futex0() has
>> been modified to be static, non-inlined, nomips16 under MIPS16 mode (for
>> sake of using 'syscall'). The inline assembly has also been fixed, as
>> Maciej noticed a possible violation of the MIPS syscall restart
>> convention; the 'li $2, #syscall_number' must be right before the
>> syscall insn.
> 
> This change is OK as part of (2).

Okay thanks, I commit this part soon.

Thanks,
Chung-Lin

Patch ping

[Jakub Jelinek]

Hi!

I'd like to ping 3 patches:

- http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01335.html
  PR debug/51950
  -gdwarf-4 -fdebug-types-section cloning bug

- http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00034.html
  PR middle-end/52074
  fix EXPAND_NORMAL expansion with -fsection-anchors

- http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01492.html
  update libstdc++ baseline_symbols.txt for several targets

Jakub

Re: [trans-mem, PATCH] do not dereference node if null in expand_call_tm (PR middle-end/52047)

[Richard Guenther]

On Thu, Feb 2, 2012 at 8:00 PM, Patrick Marlier
 wrote:
> On 02/02/2012 04:22 AM, Richard Guenther wrote:
>>
>> On Wed, Feb 1, 2012 at 10:19 PM, Patrick Marlier
>>   wrote:
>>>
>>> On 02/01/2012 03:59 AM, Richard Guenther wrote:


 The patch looks ok, but I'm not sure why you do not get a cgraph node
 here - cgraph doesn't really care for builtins as far as I can see.
  Honza?
>>>
>>>
>>> I cannot help on this...
>>>
>>>
 Don't you maybe want to handle builtins in a special way here?
>>>
>>>
>>> Indeed, I think my patch is wrong. __builtin_prefetch should have the
>>> transaction_pure attribute. I don't know how usually it should be done
>>> but
>>> what about adding a gcc_assert before to dereference node (potentially
>>> NULL)?
>>>
>>> How the attached patch looks like now?
>>> (Tested on i686)
>>>
>>>
 At least those that are const/pure?
>>>
>>> About const/pure, we cannot consider those functions as transaction_pure
>>> because they can read global and shared variable.
>>
>>
>> Well, const functions cannot access global memory, they can only inspect
>> their arguments.
>
>
> Actually, in this example, GCC does not complain at all about those const
> functions which read global memory... Should it? or the user is allowed to
> indicate const even if it is not?

Yes, it's the users fault if he does something wrong (after all GCC may
not even see the definition).  GCC will simply assume it doesn't touch
global memory and thus optimize

a = 1;
b = f (&a);
a = 2;
if (b != f(&a))

to if (0).

> static int a = 0;
>
> int f (int* a) __attribute__((const));
> int f (int* a)
> {
>  return *a+1;
> }
>
> int g () __attribute__((const));
> int g ()
> {
>  return a+1;
> }
>
> int main()
> {
>  int tmp = 0;
>  __transaction_atomic {
>    tmp += f(&a);
>    tmp += g();
>  }
>
>  return tmp;
> }
>
> In this test, the transaction is completely removed since no transactional
> operation is detected.
>
>
>> Of course __builtin_prefetch seems to be special in some way.  Note that
>> users can explicitely call it, so setting the attribute from the
>> prefetching
>> pass isn't the correct thing to do.
>
> If the user calls it explicitly, at least the compiler doesn't ICE.
> Ex: error: unsafe function call ‘__builtin_prefetch’ within atomic
> transaction.
> So what's your proposal? to add directly in builtins.def the
> transaction_pure attribute to required functions?

No, have a transaction_pure_function_p predicate which checks for
the attribute and also has an explicit list of builtin functions that
are considered transaction pure.

>> Note that __builtin_prefetch has the 'no vops' attribute - I think you
>> should
>> simply consider all 'no vops' builtins as transaction pure instead or
>> explicitely
>> consider a set of builtins as transaction pure (that's more scalable than
>> sticking the attribute onto random builtins - see how we handle builtins
>> in
>> the alias machinery, we avoid sticking the fnspec attribute onto each
>> builtin but simply have special handling for them).
>
>
> 'no vops' attribute description is quite vague. "may have arbitrary side
> effects" scares me ;)
> I will have a look at this alias machinery (any files particularly? tree.c
> seems to have a lot of builtins things in it).

We don't seem to have many uses of "no vops" left, so I suppose we
should eventually remove it in favor of something more precise ;)

> In this patch, I do manage 'no vops' the same way as const is.
> Attached the new version where we add ECF_TM_PURE to NOVOPS functions.
> Tested on x86_64-unknown-linux-gnu.

Looks good to me.

Thanks,
Richard.

> Thanks!
> Patrick.
>
> 2012-02-02  Patrick Marlier  
>
>
>        PR middle-end/52047
>        * trans-mem.c (expand_call_tm): Add an assertion.
>        * calls.c (flags_from_decl_or_type): Add ECF_TM_PURE to 'no vops'
>        functions.
>
>
>>
>> Thanks,
>> Richard.
>>
>>> BTW, I will post a PR (and probably a patch) about this.
>>>
>>> Thanks for your comment!
>>>
>>> Patrick.
>>>
>>>        PR middle-end/52047
>>>        * trans-mem.c (expand_call_tm): Add an assertion.
>>>        * tree-ssa-loop-prefetch.c (issue_prefetch_ref): Add
>>> transaction_pure
>>>        attribute to __builtin_prefetch.
>>>        (tree_ssa_prefetch_arrays): Likewise.
>
>

Re: Patch ping

[Paolo Carlini]

On 02/03/2012 11:13 AM, Jakub Jelinek wrote:

- http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01492.html
   update libstdc++ baseline_symbols.txt for several targets

This is Ok (sorry for not telling you explicitly earlier)

Thanks,
Paolo.

[PATCH] Fix SRA access replacement renaming

[Richard Guenther]

If we have something like

BIT_FIELD_REF  = D.1722_3;

where a.D.1707 is of integer type then SRA will happily create
an SSA name replacement for a.D1707 resulting in invalid GIMPLE
IL with partial updates to SSA names (and crash later in the
verifiers).

The following patch cures this - I have not been able to come up
with a testcase with an unpatched trunk though.

Bootstrap & regtest pending on x86_64-unknown-linux-gnu.

Richard.

2012-02-03  Richard Guenther  

* tree-sra.c (create_access_replacement): Only rename the
replacement if we can rewrite it into SSA form.  Properly
mark register typed replacements that we cannot rewrite
with TREE_ADDRESSABLE.

Index: gcc/tree-sra.c
===
--- gcc/tree-sra.c  (revision 183867)
+++ gcc/tree-sra.c  (working copy)
@@ -1908,13 +1908,19 @@ create_access_replacement (struct access
 
   repl = create_tmp_var (access->type, "SR");
   add_referenced_var (repl);
-  if (rename)
+  if (!access->grp_partial_lhs
+  && rename)
 mark_sym_for_renaming (repl);
 
-  if (!access->grp_partial_lhs
-  && (TREE_CODE (access->type) == COMPLEX_TYPE
- || TREE_CODE (access->type) == VECTOR_TYPE))
-DECL_GIMPLE_REG_P (repl) = 1;
+  if (TREE_CODE (access->type) == COMPLEX_TYPE
+  || TREE_CODE (access->type) == VECTOR_TYPE)
+{
+  if (!access->grp_partial_lhs)
+   DECL_GIMPLE_REG_P (repl) = 1;
+}
+  else if (access->grp_partial_lhs
+  && is_gimple_reg_type (access->type))
+TREE_ADDRESSABLE (repl) = 1;
 
   DECL_SOURCE_LOCATION (repl) = DECL_SOURCE_LOCATION (access->base);
   DECL_ARTIFICIAL (repl) = 1;

Re: Memory corruption due to word sharing

[Richard Guenther]

(Somehow my reply was private to Aldy ... forwarding to gcc-patches
now, given that it contains a patch and we changed topics)

On Fri, 3 Feb 2012, Richard Guenther wrote:

> On Thu, 2 Feb 2012, Aldy Hernandez wrote:
> 
> > Linus Torvalds  writes:
> > 
> > > Seriously - is there any real argument *against* just using the base
> > > type as a hint for access size?
> > 
> > If I'm on the hook for attempting to fix this again, I'd also like to
> > know if there are any arguments against using the base type.
> 
> Well, if you consider
> 
> struct {
>   int i : 1;
>   char c;
> };
> 
> then you'll realize that 'i' has SImode (and int type) but the
> underlying bitfield has only 1 byte size (thus, QImode) and
> 'c' starts at offset 1.
> 
> So no, you cannot use the base type either.
> 
> I've playing with the following patch yesterday, which computes
> an "underlying object" for all bitfields and forcefully lowers
> all bitfield component-refs to use that underlying object
> (just to check correctness, it doesn't generate nice code as
> BIT_FIELD_REF on memory is effectively resulting in the same
> code as if using the bitfield FIELD_DECLs directly - we'd
> need to explicitely split things into separate stmts with RMW
> cycles).
> 
> You should be able to re-use the underlying object compute though
> (and we can make it more intelligent even) during expansion
> for the C++ memory model (and in fact underlying object compute
> might just do sth different dependent on the memory model in
> effect).
> 
> Disclaimer: untested.

The following works (roughly, still mostly untested).  SRA needs
a fix (included) and the gimplify.c hunk really only shows what
we are supposed to be able to do (access the representative).
As-is SRA could now do a nice job on bitfields, but that needs
some changes - or we lower all bitfield ops in some extra pass
(if not then expand would need to be changed to look at the
representatives instead).

Still the idea is to compute all these things up-front during
type layout instead of re-discovering them at each bitfield
access we expand in get_bit_range.  And we can use that information
consistently across passes.

We should of course try harder to avoid adding a new field to
struct tree_field_decl - DECL_INITIAL came to my mind, but
the C frontend happens to use that for bitfields ... while
it probably could as well use lang_type.enum_{min,max}?

Comments?

Thanks,
Richard.

2012-02-03  Richard Guenther  

* tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
(struct tree_field_decl): New field bit_field_representative.
* stor-layout.c (start_bitfield_representative): New function.
(finish_bitfield_representative): Likewise.
(finish_bitfield_layout): Likewise.
(finish_record_layout): Call finish_bitfield_layout.

* gimplify.c (gimplify_expr): Translate bitfield accesses
to BIT_FIELD_REFs of the representative.

* tree-sra.c (create_access_replacement): Only rename the
replacement if we can rewrite it into SSA form.  Properly
mark register typed replacements that we cannot rewrite
with TREE_ADDRESSABLE.

Index: gcc/stor-layout.c
===
*** gcc/stor-layout.c.orig  2012-02-03 10:42:49.0 +0100
--- gcc/stor-layout.c   2012-02-03 12:37:38.0 +0100
*** finalize_type_size (tree type)
*** 1722,1727 
--- 1722,1889 
  }
  }
  
+ /* Return a new underlying object for a bitfield started with FIELD.  */
+ 
+ static tree
+ start_bitfield_representative (tree field)
+ {
+   tree repr = make_node (FIELD_DECL);
+   DECL_FIELD_OFFSET (repr) = DECL_FIELD_OFFSET (field);
+   DECL_FIELD_BIT_OFFSET (repr) = DECL_FIELD_BIT_OFFSET (field);
+   SET_DECL_OFFSET_ALIGN (repr, DECL_OFFSET_ALIGN (field));
+   DECL_SIZE (repr) = DECL_SIZE (field);
+   DECL_PACKED (repr) = DECL_PACKED (field);
+   DECL_CONTEXT (repr) = DECL_CONTEXT (field);
+   return repr;
+ }
+ 
+ /* Finish up a bitfield group that was started by creating the underlying
+object REPR with the last fied in the bitfield group FIELD.  */
+ 
+ static void
+ finish_bitfield_representative (tree repr, tree field)
+ {
+   unsigned HOST_WIDE_INT bitsize, maxbitsize, modesize;
+   enum machine_mode mode;
+   tree nextf, size;
+ 
+   size = size_diffop (DECL_FIELD_OFFSET (field),
+ DECL_FIELD_OFFSET (repr));
+   gcc_assert (host_integerp (size, 1));
+   bitsize = (tree_low_cst (size, 1) * BITS_PER_UNIT
++ tree_low_cst (DECL_FIELD_BIT_OFFSET (field), 1)
+- tree_low_cst (DECL_FIELD_BIT_OFFSET (repr), 1)
++ tree_low_cst (DECL_SIZE (field), 1));
+ 
+   /* Now nothing tells us how to pad out bitsize ...  */
+   nextf = DECL_CHAIN (field);
+   while (nextf && TREE_CODE (nextf) != FIELD_DECL)
+ nextf = DECL_CHAIN (nextf);
+   if (nextf)
+ {
+   tree maxsize = size_diffop (DECL_FIELD_OFFSET (nextf),
+ 

Re: [PATCH] MIPS16 TLS support for GCC

[Maciej W. Rozycki]

On Fri, 3 Feb 2012, Chung-Lin Tang wrote:

> On the issue of hiding stuff in mips16.S, it may be suitable to raise
> this issue here: there is a problem when MIPS16 hard-float calls go
> through PLTs, because the hard-float stub functions in mips16.S use
> v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
> by BFD. (I think I described the scenario mostly correct, CCing Maciej
> here who worked on this issue)  I think the solution was to mark a large
> portion of the mips16.S functions as all hidden.

 Not quite a large portion, just the function return stubs (which I reckon 
there are exactly four) that process values in $v0/$v1 as if they were 
arguments.  This is of course a non-standard calling convention that 
breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
Not even mentioning that three of these four stubs are not bigger than a 
PLT entry they would need, and the fourth is two or so instructions 
larger, so making them dynamic hardly makes point (the cost of the 
indirect call from PLT also matters).

 Given that this code (to support MIPS16 shared libraries) was obviously 
never fully functional before this effort, I think it is about the right 
time to remove some hacks I saw in glibc's dynamic linker that are meant 
to avoid clobbering $v0/$v1 before someone tries to rely on them for real.  

 And there's no reasonable way to support MIPS16 PLT entries without 
clobbering $v0/$v1 (to preempt your question -- MIPS16 code can of course 
work just fine with standard MIPS PLT entries (no pure-MIPS16 processors 
are possible), but on some processors the cost of flipping the ISA bit is 
prohibitive as it requires draining the pipeline to reconfigure the 
instruction decoder and as such should best be avoided in pure-MIPS16 
dynamic binaries).

  Maciej

Re: [PATCH] Fix RTL sharing bug in loop unswitching (PR rtl-optimization/52092)

[Jakub Jelinek]

On Fri, Feb 03, 2012 at 09:17:23AM +0100, Zdenek Dvorak wrote:
> > It seems loop-iv.c happily creates shared RTL in lots of places,
> > loop-unroll.c solves that by unsharing everything it adds, this is
> > an attempt to do the same in unswitching to unshare everything iv_analyze
> > came up with.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> I would suggest to move the unsharing to the point where it is clear that
> the rtl will added, i.e.,
> 
> loop-unswitch.c:370
>   nloop = unswitch_loop (loop, bbs[i], cond, cinsn);
> ->
>   nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn);

You're right, and that also cures the thing that inner unswitch_single_loop
might use again a condition based on the same rtl (or reverse_condition
thereof).  Bootstrapped/regtested again on x86_64-linux and i686-linux,
approved by Richard on IRC, committed to trunk.  Thanks.

2012-02-03  Jakub Jelinek  
Zdenek Dvorak  

PR rtl-optimization/52092
* loop-unswitch.c (unswitch_single_loop): Call copy_rtx_if_shared
on get_iv_value result.

* gcc.c-torture/compile/pr52092.c: New test.

--- gcc/loop-unswitch.c.jj  2010-06-28 15:36:30.0 +0200
+++ gcc/loop-unswitch.c 2012-02-03 09:24:24.385450397 +0100
@@ -367,7 +367,7 @@ unswitch_single_loop (struct loop *loop,
 fprintf (dump_file, ";; Unswitching loop\n");
 
   /* Unswitch the loop on this condition.  */
-  nloop = unswitch_loop (loop, bbs[i], cond, cinsn);
+  nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn);
   gcc_assert (nloop);
 
   /* Invoke itself on modified loops.  */
--- gcc/testsuite/gcc.c-torture/compile/pr52092.c.jj2012-02-02 
14:11:37.722031008 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr52092.c   2012-02-02 
14:11:14.0 +0100
@@ -0,0 +1,25 @@
+/* PR rtl-optimization/52092 */
+
+int a, b, c, d, e, f, g;
+
+void
+foo (void)
+{
+  for (;;)
+{
+  int *h = 0;
+  int i = 3;
+  int **j = &h;
+  if (e)
+   {
+ c = d || a;
+ g = c ? a : b;
+ if ((char) (i * g))
+   {
+ h = &f;
+ *h = 0;
+   }
+ **j = 0;
+   }
+}
+}

Jakub

Re: [PATCH] fix PR51910, take 2

[Jakub Jelinek]

On Fri, Feb 03, 2012 at 10:27:17AM +0100, Jakub Jelinek wrote:
> Anyway, here is the tweaking all the symbols that demangle the same
> equally patch (untested so far) as an alternative.  On the example it

Now bootstrapped/regtested on x86_64-linux and i686-linux.

Jakub

Re: Memory corruption due to word sharing

[Richard Guenther]

On Fri, 3 Feb 2012, Richard Guenther wrote:

> On Fri, 3 Feb 2012, Richard Guenther wrote:
> 
> > On Thu, 2 Feb 2012, Aldy Hernandez wrote:
> > 
> > > Linus Torvalds  writes:
> > > 
> > > > Seriously - is there any real argument *against* just using the base
> > > > type as a hint for access size?
> > > 
> > > If I'm on the hook for attempting to fix this again, I'd also like to
> > > know if there are any arguments against using the base type.
> > 
> > Well, if you consider
> > 
> > struct {
> >   int i : 1;
> >   char c;
> > };
> > 
> > then you'll realize that 'i' has SImode (and int type) but the
> > underlying bitfield has only 1 byte size (thus, QImode) and
> > 'c' starts at offset 1.
> > 
> > So no, you cannot use the base type either.
> > 
> > I've playing with the following patch yesterday, which computes
> > an "underlying object" for all bitfields and forcefully lowers
> > all bitfield component-refs to use that underlying object
> > (just to check correctness, it doesn't generate nice code as
> > BIT_FIELD_REF on memory is effectively resulting in the same
> > code as if using the bitfield FIELD_DECLs directly - we'd
> > need to explicitely split things into separate stmts with RMW
> > cycles).
> > 
> > You should be able to re-use the underlying object compute though
> > (and we can make it more intelligent even) during expansion
> > for the C++ memory model (and in fact underlying object compute
> > might just do sth different dependent on the memory model in
> > effect).
> > 
> > Disclaimer: untested.
> 
> The following works (roughly, still mostly untested).  SRA needs
> a fix (included) and the gimplify.c hunk really only shows what
> we are supposed to be able to do (access the representative).
> As-is SRA could now do a nice job on bitfields, but that needs
> some changes - or we lower all bitfield ops in some extra pass
> (if not then expand would need to be changed to look at the
> representatives instead).
> 
> Still the idea is to compute all these things up-front during
> type layout instead of re-discovering them at each bitfield
> access we expand in get_bit_range.  And we can use that information
> consistently across passes.
> 
> We should of course try harder to avoid adding a new field to
> struct tree_field_decl - DECL_INITIAL came to my mind, but
> the C frontend happens to use that for bitfields ... while
> it probably could as well use lang_type.enum_{min,max}?
> 
> Comments?

Funnily C++ uses tail-padding of base types to pack bitfields
and thus I run into

  gcc_assert (maxbitsize % BITS_PER_UNIT == 0);

Testcase is for example g++.dg/abi/bitfield5.C, bit layout annotated:

struct A {
  virtual void f();
  int f1 : 1; <--- bit 64
};

struct B : public A {
  int f2 : 1;  // { dg-warning "ABI" }<--- bit 65
  int : 0;
  int f3 : 4;
  int f4 : 3;
};

maybe it was a bug (above happens with -fabi-version=1 only),
but certainly an ABI may specify that we should do that packing.

What does the C++ memory model say here?  (incidentially that's
one case I was worried about when reviewing your patches,
just I didn't think of _bitfield_ tail-packing ... ;)).

I suppose I could just force the bitfield region to start
at a byte boundary.

Richard.

[PATCH] Fix PR50969

[William J. Schmidt]

This fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50969 by slightly
raising the cost of vector permutes on powerpc64 VSX targets (and
ensuring those costs are correctly used).  This reverses the performance
loss for 168.wupwise, and gives a slight boost to 433.milc as well.

In the long run, we will want to model VSX permutes differently, since
the real issue is that only one floating-point pipe can hold a permute
at a time.  Thus the present patch can be overly conservative when
permutes are rare compared with other vector instructions.

Bootstrapped and regtested on powerpc64-linux-gnu with no failures.  OK
for trunk?

Thanks,
Bill


2012-02-03  Bill Schmidt  

PR tree-optimization/50969
* tree-vect-stmts.c (vect_model_store_cost): Correct statement cost to
use vec_perm rather than vector_stmt.
(vect_model_load_cost): Likewise.
* config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Revise
cost of vec_perm for TARGET_VSX.


Index: gcc/tree-vect-stmts.c
===
--- gcc/tree-vect-stmts.c   (revision 183871)
+++ gcc/tree-vect-stmts.c   (working copy)
@@ -882,7 +882,7 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
 {
   /* Uses a high and low interleave operation for each needed permute.  */
   inside_cost = ncopies * exact_log2(group_size) * group_size
-* vect_get_stmt_cost (vector_stmt);
+* vect_get_stmt_cost (vec_perm);
 
   if (vect_print_dump_info (REPORT_COST))
 fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d .",
@@ -988,7 +988,7 @@ vect_model_load_cost (stmt_vec_info stmt_info, int
 {
   /* Uses an even and odd extract operations for each needed permute.  */
   inside_cost = ncopies * exact_log2(group_size) * group_size
-   * vect_get_stmt_cost (vector_stmt);
+   * vect_get_stmt_cost (vec_perm);
 
   if (vect_print_dump_info (REPORT_COST))
 fprintf (vect_dump, "vect_model_load_cost: strided group_size = %d .",
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 183871)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -3540,9 +3540,13 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
   case vec_to_scalar:
   case scalar_to_vec:
   case cond_branch_not_taken:
-  case vec_perm:
 return 1;
 
+  case vec_perm:
+   if (!TARGET_VSX)
+ return 1;
+   return 2;
+
   case cond_branch_taken:
 return 3;
 

Re: [PATCH] Fix PR50969

[Richard Guenther]

On Fri, Feb 3, 2012 at 2:24 PM, William J. Schmidt
 wrote:
> This fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50969 by slightly
> raising the cost of vector permutes on powerpc64 VSX targets (and
> ensuring those costs are correctly used).  This reverses the performance
> loss for 168.wupwise, and gives a slight boost to 433.milc as well.
>
> In the long run, we will want to model VSX permutes differently, since
> the real issue is that only one floating-point pipe can hold a permute
> at a time.  Thus the present patch can be overly conservative when
> permutes are rare compared with other vector instructions.
>
> Bootstrapped and regtested on powerpc64-linux-gnu with no failures.  OK
> for trunk?

Note this makes permutes artificially cheap for AMD K8, K10 and
Bulldozer.  Can you change config/i386/i386.c:ix86_builtin_vectorization_cost
to return ix86_cost->vec_stmt_cost instead of one for vec_perm?
The cost is otherwise only queried by SLP vectorization it seems.

Otherwise this looks ok.  Please give other maintainers a chance to
chime in (other cost hooks might need similar adjustments).

Thanks,
Richard.

> Thanks,
> Bill
>
>
> 2012-02-03  Bill Schmidt  
>
>        PR tree-optimization/50969
>        * tree-vect-stmts.c (vect_model_store_cost): Correct statement cost to
>        use vec_perm rather than vector_stmt.
>        (vect_model_load_cost): Likewise.
>        * config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Revise
>        cost of vec_perm for TARGET_VSX.
>
>
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c       (revision 183871)
> +++ gcc/tree-vect-stmts.c       (working copy)
> @@ -882,7 +882,7 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
>     {
>       /* Uses a high and low interleave operation for each needed permute.  */
>       inside_cost = ncopies * exact_log2(group_size) * group_size
> -        * vect_get_stmt_cost (vector_stmt);
> +        * vect_get_stmt_cost (vec_perm);
>
>       if (vect_print_dump_info (REPORT_COST))
>         fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d 
> .",
> @@ -988,7 +988,7 @@ vect_model_load_cost (stmt_vec_info stmt_info, int
>     {
>       /* Uses an even and odd extract operations for each needed permute.  */
>       inside_cost = ncopies * exact_log2(group_size) * group_size
> -       * vect_get_stmt_cost (vector_stmt);
> +       * vect_get_stmt_cost (vec_perm);
>
>       if (vect_print_dump_info (REPORT_COST))
>         fprintf (vect_dump, "vect_model_load_cost: strided group_size = %d .",
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c  (revision 183871)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -3540,9 +3540,13 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
>       case vec_to_scalar:
>       case scalar_to_vec:
>       case cond_branch_not_taken:
> -      case vec_perm:
>         return 1;
>
> +      case vec_perm:
> +       if (!TARGET_VSX)
> +         return 1;
> +       return 2;
> +
>       case cond_branch_taken:
>         return 3;
>
>
>

Re: [PATCH] MIPS16 TLS support for GCC

[Richard Sandiford]

Chung-Lin Tang  writes:
>> (2) is interesting if there is also a way to build those MIPS16 libraries
>> out of the box.  I'd like such a mechanism to be added at the same time,
>> so that the new support is easy to test.  This is still a 4.7 candidate
>> if it can be done in time, although it's probably a little tight.
>
> I assume you mean the libgomp patch? That's mostly a potential bug fix;

That, plus the CRT_CALL_STATIC_FUNCTION and crtfastmath.c changes.

> as for MIP16 multilib additions, so far I haven't added any to the
> default mips*-linux* configs, as I guess this should be vendor-stuff
> (though the current additions should clear away any obstacles)

Vendor stuff would be fine.  But to be clear, (2) isn't OK without
some way of getting MIPS16 multilibs out of the box, since it would
otherwise be dead code.

>> You've given the built-in function the generic name __builtin_thread_pointer.
>> I agree that's a good idea, but I think we should also _treat_ it as a 
>> generic
>> function, and make it available on all targets.  The actual implementation
>> can be delegated to a target hook.  That makes (3) 4.8 material.
>
> Actually, I named and added __builtin_thread_pointer() only because it
> was needed for building glibc (to avoid using 32-bit asm in MIPS16
> mode), and because some other targets also have it (I'm sure ARM also
> has it, and at least one other)

All the more reason to make it target-independent. :-)

>> (Incidentally, I don't think it's correct to define the builtin if TLS
>> isn't available, so if we did keep it as a MIPS-specific function,
>> d->avail would need to be nonnull.  There would need to be some other
>> way of indicating that MIPS16 was OK.)
>
> The function is essentially a wrapper for mips_get_tp(), which I don't
> see any guarding for the no TLS case? FWIW, TARGET_HAVE_TLS is just
> defined as HAVE_AS_TLS for MIPS...

This stuff is inherently guarded by TARGET_HAVE_TLS.

>>> 5) The libgomp MIPS futex.h header has been adjusted; sys_futex0() has
>>> been modified to be static, non-inlined, nomips16 under MIPS16 mode (for
>>> sake of using 'syscall'). The inline assembly has also been fixed, as
>>> Maciej noticed a possible violation of the MIPS syscall restart
>>> convention; the 'li $2, #syscall_number' must be right before the
>>> syscall insn.
>> 
>> This change is OK as part of (2).
>
> Okay thanks, I commit this part soon.

Please don't!  I was unnecessarily confusing here, sorry.  I was trying
to say earlier that (2) isn't OK as-is because people have no way of
testing it without changing the sources locally.  We need to add some
way of configuring MIPS16 multilibs at the same time.  The libgomp
change is OK as part of that patch, but not on its own.

Thanks,
Richard

Re: [PATCH] MIPS16 TLS support for GCC

[Richard Sandiford]

"Maciej W. Rozycki"  writes:
> On Fri, 3 Feb 2012, Chung-Lin Tang wrote:
>> On the issue of hiding stuff in mips16.S, it may be suitable to raise
>> this issue here: there is a problem when MIPS16 hard-float calls go
>> through PLTs, because the hard-float stub functions in mips16.S use
>> v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
>> by BFD. (I think I described the scenario mostly correct, CCing Maciej
>> here who worked on this issue)  I think the solution was to mark a large
>> portion of the mips16.S functions as all hidden.
>
>  Not quite a large portion, just the function return stubs (which I reckon 
> there are exactly four) that process values in $v0/$v1 as if they were 
> arguments.  This is of course a non-standard calling convention that 
> breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
> Not even mentioning that three of these four stubs are not bigger than a 
> PLT entry they would need, and the fourth is two or so instructions 
> larger, so making them dynamic hardly makes point (the cost of the 
> indirect call from PLT also matters).

But that isn't the only reason to use shared libraries.

>  Given that this code (to support MIPS16 shared libraries) was obviously 
> never fully functional before this effort, I think it is about the right 
> time to remove some hacks I saw in glibc's dynamic linker that are meant 
> to avoid clobbering $v0/$v1 before someone tries to rely on them for real.

FSF BFD doesn't generate MIPS16 PLTs though.  Is this a CS-local change?
If so, the criticism seems a little unfair.  The FSF code is self-
consistent: PLTs can be used for these stubs because the resolver
function preserves the necessary registers.  Traditional lazy binding
stubs are avoided by not using %call16.

I don't think changing the resolver function so that it preserves
fewer registers would be a good idea.  As well as the MIPS16 thing,
$3 is needed for _mcount.

You're right of course that PLTs can't be used for __mips16_rdhwr.
I'd forgotten about that, sorry.  Will fix this weekend.

If there's still some concern that __mips16_rdhwr might not have
the right ABI, then maybe we should simply emit a link-once function
in each object that needs it.  We could then switch to another function
(and another API) without having to keep the old one in libgcc.a for
compatibility.  It would also avoid the -shared-libgcc thing.

Admittedly that's just an off-the-top-of-my-head idea. :-)
What do you think?

Richard

Re: [PATCH] Fix PR50969

[William J. Schmidt]

On Fri, 2012-02-03 at 14:41 +0100, Richard Guenther wrote:
> On Fri, Feb 3, 2012 at 2:24 PM, William J. Schmidt
>  wrote:
> > This fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50969 by slightly
> > raising the cost of vector permutes on powerpc64 VSX targets (and
> > ensuring those costs are correctly used).  This reverses the performance
> > loss for 168.wupwise, and gives a slight boost to 433.milc as well.
> >
> > In the long run, we will want to model VSX permutes differently, since
> > the real issue is that only one floating-point pipe can hold a permute
> > at a time.  Thus the present patch can be overly conservative when
> > permutes are rare compared with other vector instructions.
> >
> > Bootstrapped and regtested on powerpc64-linux-gnu with no failures.  OK
> > for trunk?
> 
> Note this makes permutes artificially cheap for AMD K8, K10 and
> Bulldozer.  Can you change config/i386/i386.c:ix86_builtin_vectorization_cost
> to return ix86_cost->vec_stmt_cost instead of one for vec_perm?
> The cost is otherwise only queried by SLP vectorization it seems.

Sure, will do.

> 
> Otherwise this looks ok.  Please give other maintainers a chance to
> chime in (other cost hooks might need similar adjustments).

I'll give this until at least late Monday before committing.  Thanks for
the quick response!

Bill
 
> 
> Thanks,
> Richard.
> 
> > Thanks,
> > Bill
> >
> >
> > 2012-02-03  Bill Schmidt  
> >
> >PR tree-optimization/50969
> >* tree-vect-stmts.c (vect_model_store_cost): Correct statement cost 
> > to
> >use vec_perm rather than vector_stmt.
> >(vect_model_load_cost): Likewise.
> >* config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Revise
> >cost of vec_perm for TARGET_VSX.
> >
> >
> > Index: gcc/tree-vect-stmts.c
> > ===
> > --- gcc/tree-vect-stmts.c   (revision 183871)
> > +++ gcc/tree-vect-stmts.c   (working copy)
> > @@ -882,7 +882,7 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
> > {
> >   /* Uses a high and low interleave operation for each needed permute.  
> > */
> >   inside_cost = ncopies * exact_log2(group_size) * group_size
> > -* vect_get_stmt_cost (vector_stmt);
> > +* vect_get_stmt_cost (vec_perm);
> >
> >   if (vect_print_dump_info (REPORT_COST))
> > fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d 
> > .",
> > @@ -988,7 +988,7 @@ vect_model_load_cost (stmt_vec_info stmt_info, int
> > {
> >   /* Uses an even and odd extract operations for each needed permute.  
> > */
> >   inside_cost = ncopies * exact_log2(group_size) * group_size
> > -   * vect_get_stmt_cost (vector_stmt);
> > +   * vect_get_stmt_cost (vec_perm);
> >
> >   if (vect_print_dump_info (REPORT_COST))
> > fprintf (vect_dump, "vect_model_load_cost: strided group_size = %d 
> > .",
> > Index: gcc/config/rs6000/rs6000.c
> > ===
> > --- gcc/config/rs6000/rs6000.c  (revision 183871)
> > +++ gcc/config/rs6000/rs6000.c  (working copy)
> > @@ -3540,9 +3540,13 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
> >   case vec_to_scalar:
> >   case scalar_to_vec:
> >   case cond_branch_not_taken:
> > -  case vec_perm:
> > return 1;
> >
> > +  case vec_perm:
> > +   if (!TARGET_VSX)
> > + return 1;
> > +   return 2;
> > +
> >   case cond_branch_taken:
> > return 3;
> >
> >
> >
> 

MAINTAINERS (Write After Approval): Add myself.

[Kumar, Venkataramanan]

Hi all, 

Just adding my name under Write After Approval list. 

Please see the patch below.


Index: ChangeLog
===
--- ChangeLog   (revision 183872)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2012-02-03  Venkataramanan Kumar  
+
+   * MAINTAINERS (Write After Approval): Add myself.
+
 2012-02-02  Jayant R Sonar 

* MAINTAINERS (Write After Approval): Add myself.

Index: MAINTAINERS
===
--- MAINTAINERS (revision 183872)
+++ MAINTAINERS (working copy)
@@ -538,6 +538,7 @@
 Jon Zieglerj...@apple.com
 Roman Zippel   zip...@linux-m68k.org
 Josef Zlomek   josef.zlo...@email.cz
+Venkataramanan Kumar   venkataramanan.ku...@amd.com

 Bug database only accounts


Regards,
Venkat.

Re: MAINTAINERS (Write After Approval): Add myself.

[Patrick Marlier]

On 02/03/2012 11:44 AM, Kumar, Venkataramanan wrote:

Index: MAINTAINERS
===
--- MAINTAINERS (revision 183872)
+++ MAINTAINERS (working copy)
@@ -538,6 +538,7 @@
  Jon zieglerj...@apple.com
  Roman zippelzip...@linux-m68k.org
  Josef zlomekjosef.zlo...@email.cz
+Venkataramanan kumarvenkataramanan.ku...@amd.com


As far I can see, the list is sorted by name. Your name should be 
inserted in the appropriate place.

--
Patrick.

RE: MAINTAINERS (Write After Approval): Add myself.

[Kumar, Venkataramanan]

Hi Patrick,

Thank you for pointing that. I will fix it.

Regards,
Venkat.


> -Original Message-
> From: Patrick Marlier [mailto:patrick.marl...@gmail.com]
> Sent: Friday, February 03, 2012 10:20 PM
> To: Kumar, Venkataramanan
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: MAINTAINERS (Write After Approval): Add myself.
> 
> On 02/03/2012 11:44 AM, Kumar, Venkataramanan wrote:
> > Index: MAINTAINERS
> > ===
> > --- MAINTAINERS (revision 183872)
> > +++ MAINTAINERS (working copy)
> > @@ -538,6 +538,7 @@
> >   Jon zieglerj...@apple.com
> >   Roman zippelzip...@linux-m68k.org
> >   Josef zlomekjosef.zlo...@email.cz
> > +Venkataramanan kumarvenkataramanan.ku...@amd.com
> 
> As far I can see, the list is sorted by name. Your name should be
> inserted in the appropriate place.
> --
> Patrick.
> 

MAINTAINERS (Write After Approval): Move myself to maintain alphabetical order.

[Kumar, Venkataramanan]

Hi all,

I have inserted my name at correct order in the MAINTAINERS list.

Ok to commit ? please see the patch below.

Regards,
Venkat.


Index: ChangeLog
===
--- ChangeLog   (revision 183873)
+++ ChangeLog   (working copy)
@@ -1,5 +1,9 @@
 2012-02-03  Venkataramanan Kumar  

+   * MAINTAINERS (Write After Approval): Move myself to maintain 
alphabetical order.
+
+2012-02-03  Venkataramanan Kumar  
+
* MAINTAINERS (Write After Approval): Add myself.

 2012-02-02  Jayant R Sonar 
Index: MAINTAINERS
===
--- MAINTAINERS (revision 183873)
+++ MAINTAINERS (working copy)
@@ -413,6 +413,7 @@
 Michael Koch   konque...@gmx.de
 Matt Kraai kr...@ftbfs.org
 Jan Kratochvil jan.kratoch...@redhat.com
+Venkataramanan Kumar   venkataramanan.ku...@amd.com
 Maxim Kuvyrkov ma...@codesourcery.com
 Doug Kwan  dougk...@google.com
 Scott Robert Ladd  scott.l...@coyotegulch.com
@@ -538,7 +539,6 @@
 Jon Zieglerj...@apple.com
 Roman Zippel   zip...@linux-m68k.org
 Josef Zlomek   josef.zlo...@email.cz
-Venkataramanan Kumar   venkataramanan.ku...@amd.com

 Bug database only accounts

Re: [PATCH] MIPS16 TLS support for GCC

[Maciej W. Rozycki]

On Fri, 3 Feb 2012, Richard Sandiford wrote:

> >  Not quite a large portion, just the function return stubs (which I reckon 
> > there are exactly four) that process values in $v0/$v1 as if they were 
> > arguments.  This is of course a non-standard calling convention that 
> > breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
> > Not even mentioning that three of these four stubs are not bigger than a 
> > PLT entry they would need, and the fourth is two or so instructions 
> > larger, so making them dynamic hardly makes point (the cost of the 
> > indirect call from PLT also matters).
> 
> But that isn't the only reason to use shared libraries.

 That is true in the general case, but these MIPS16 stubs have essentially 
been cast in stone and there's little room if any for change here.  I 
can't imagine a case where static linking of these four stubs would cause 
trouble, but certainly I am open to any examples.

> >  Given that this code (to support MIPS16 shared libraries) was obviously 
> > never fully functional before this effort, I think it is about the right 
> > time to remove some hacks I saw in glibc's dynamic linker that are meant 
> > to avoid clobbering $v0/$v1 before someone tries to rely on them for real.
> 
> FSF BFD doesn't generate MIPS16 PLTs though.  Is this a CS-local change?

 A patch is in the queue, together with microMIPS PLT entries (that you 
have expected, haven't you?).  I can't give you a specific schedule, but I 
expect to propose it as soon as I've got the current batch of GDB changes 
followed by the outstanding binutils clean-ups I got your feedback on that 
I haven't finished processing yet, off my plate.

> If so, the criticism seems a little unfair.  The FSF code is self-
> consistent: PLTs can be used for these stubs because the resolver
> function preserves the necessary registers.  Traditional lazy binding
> stubs are avoided by not using %call16.

 Please don't take it as criticism but as a starting point for discussion. 
And certainly I did not intend to sound negative, I'm sorry if I did.  
I'm just raising points I noted as MIPS16 PLT entries were being 
developed.  I do think though that it's a bit unfortunate that such a 
creeping ABI change has been made here.  I think in case of doubt it's 
always safe to stick to the ABI.

 Lazy binding stubs are less of concern for some reason that is not 
entirely clear to me and we have no MIPS16 implementation in the queue 
(but we do have microMIPS stubs, as you might have expected -- to 
facilitate pure-microMIPS processors that may eventually appear).

> I don't think changing the resolver function so that it preserves
> fewer registers would be a good idea.  As well as the MIPS16 thing,
> $3 is needed for _mcount.

 I don't think _mcount has ever worked for dynamic libraries, has it? -- 
please correct me if I am wrong, but I believe `gprof' relies on memory 
segments to be contiguous which is certainly not the case for a dynamic 
executable where you have a separate set of mappings for the executable 
proper and then for each shared library loaded.

 I didn't know it required $3 for anything either -- I've thought it was 
$1 only.  How has it worked for standard MIPS code then?

 We have only about now got MIPS16 shared libraries to work -- are you 
sure removing code to save/restore $2/$3 in the dynamic linker is going to 
hit anyone?  While a small piece, this is still some wasted memory and 
execution time for every executable on every system and whether MIPS16 
code is involved or not (even on systems that do not support the MIPS16 
ASE at all), just to cope with an ABI anomaly of literally four functions 
only needed for some MIPS16 code (and which had not originally been 
expected to be ever used for dynamic linking -- until recently nobody even 
considered the use of MIPS16 code on a shared library system).  And I 
think you can still get into trouble if you use the wrong ld.so with your 
MIPS16 executable -- or has symbol versioning been used to ensure that 
ld.so bails out in this case?

> You're right of course that PLTs can't be used for __mips16_rdhwr.

 Unless you want to waste text (and possibly stack) space and time to save 
and restore call-clobbered registers in the caller that is. ;)

> I'd forgotten about that, sorry.  Will fix this weekend.
> 
> If there's still some concern that __mips16_rdhwr might not have
> the right ABI, then maybe we should simply emit a link-once function
> in each object that needs it.  We could then switch to another function
> (and another API) without having to keep the old one in libgcc.a for
> compatibility.  It would also avoid the -shared-libgcc thing.
> 
> Admittedly that's just an off-the-top-of-my-head idea. :-)
> What do you think?

 Actually I had that idea of a link-once function too, but it turned out 
quite complicated to do without rewriting some generic parts of GCC as it 
is currently not prepared to emit link-once functions

[PATCH] Fix -fdump-rtl-sms (PR rtl-optimization/52095)

[Jakub Jelinek]

Hi!

On some targets e.g. sms-7.c test fails, because fprintf is called
with %s format and NULL argument, GLIBC prints for that e.g.
SMS loop num: 1, file: (null), line: 0
but it isn't portable.  print-rtl.c guards the locator printing with
/*  Pretty-print insn locators.  Ignore scoping as it is mostly
redundant with line number information and do not print anything
when there is no location information available.  */
if (INSN_LOCATOR (in_rtx) && insn_file (in_rtx))
  fprintf(outfile, " %s:%i", insn_file (in_rtx), insn_line 
(in_rtx));
which fixes this, but there are 7 different spots that would need adjusting
in modulo-sched.c, so I've added a helper function for that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-02-03  Jakub Jelinek  

PR rtl-optimization/52095
* modulo-sched.c (dump_insn_locator): New function.
(loop_canon_p, sms_schedule): Use it.

--- gcc/modulo-sched.c.jj   2011-12-14 08:11:03.0 +0100
+++ gcc/modulo-sched.c  2012-02-03 13:45:49.137997767 +0100
@@ -1246,6 +1246,19 @@ loop_single_full_bb_p (struct loop *loop
   return true;
 }
 
+/* Dump file:line from INSN's location info to dump_file.  */
+
+static void
+dump_insn_locator (rtx insn)
+{
+  if (dump_file && INSN_LOCATOR (insn))
+{
+  const char *file = insn_file (insn);
+  if (file)
+   fprintf (dump_file, " %s:%i", file, insn_line (insn));
+}
+}
+
 /* A simple loop from SMS point of view; it is a loop that is composed of
either a single basic block or two BBs - a header and a latch.  */
 #define SIMPLE_SMS_LOOP_P(loop) ((loop->num_nodes < 3 )\
@@ -1271,9 +1284,9 @@ loop_canon_p (struct loop *loop)
{
  rtx insn = BB_END (loop->header);
 
- fprintf (dump_file, "SMS loop many exits ");
- fprintf (dump_file, " %s %d (file, line)\n",
-  insn_file (insn), insn_line (insn));
+ fprintf (dump_file, "SMS loop many exits");
+ dump_insn_locator (insn);
+ fprintf (dump_file, "\n");
}
   return false;
 }
@@ -1284,9 +1297,9 @@ loop_canon_p (struct loop *loop)
{
  rtx insn = BB_END (loop->header);
 
- fprintf (dump_file, "SMS loop many BBs. ");
- fprintf (dump_file, " %s %d (file, line)\n",
-  insn_file (insn), insn_line (insn));
+ fprintf (dump_file, "SMS loop many BBs.");
+ dump_insn_locator (insn);
+ fprintf (dump_file, "\n");
}
   return false;
 }
@@ -1407,13 +1420,13 @@ sms_schedule (void)
 }
 
   if (dump_file)
-  {
- rtx insn = BB_END (loop->header);
-
- fprintf (dump_file, "SMS loop num: %d, file: %s, line: %d\n",
-  loop->num, insn_file (insn), insn_line (insn));
+   {
+ rtx insn = BB_END (loop->header);
 
-  }
+ fprintf (dump_file, "SMS loop num: %d", loop->num);
+ dump_insn_locator (insn);
+ fprintf (dump_file, "\n");
+   }
 
   if (! loop_canon_p (loop))
 continue;
@@ -1440,9 +1453,8 @@ sms_schedule (void)
{
  if (dump_file)
{
- fprintf (dump_file, " %s %d (file, line)\n",
-  insn_file (tail), insn_line (tail));
- fprintf (dump_file, "SMS single-bb-loop\n");
+ dump_insn_locator (tail);
+ fprintf (dump_file, "\nSMS single-bb-loop\n");
  if (profile_info && flag_branch_probabilities)
{
  fprintf (dump_file, "SMS loop-count ");
@@ -1543,14 +1555,15 @@ sms_schedule (void)
 continue;
 
   if (dump_file)
-  {
- rtx insn = BB_END (loop->header);
+   {
+ rtx insn = BB_END (loop->header);
 
- fprintf (dump_file, "SMS loop num: %d, file: %s, line: %d\n",
-  loop->num, insn_file (insn), insn_line (insn));
+ fprintf (dump_file, "SMS loop num: %d", loop->num);
+ dump_insn_locator (insn);
+ fprintf (dump_file, "\n");
 
- print_ddg (dump_file, g);
-  }
+ print_ddg (dump_file, g);
+   }
 
   get_ebb_head_tail (loop->header, loop->header, &head, &tail);
 
@@ -1561,9 +1574,8 @@ sms_schedule (void)
 
   if (dump_file)
{
- fprintf (dump_file, " %s %d (file, line)\n",
-  insn_file (tail), insn_line (tail));
- fprintf (dump_file, "SMS single-bb-loop\n");
+ dump_insn_locator (tail);
+ fprintf (dump_file, "\nSMS single-bb-loop\n");
  if (profile_info && flag_branch_probabilities)
{
  fprintf (dump_file, "SMS loop-count ");
@@ -1705,9 +1717,9 @@ sms_schedule (void)
 
   if (dump_file)
 {
- fprintf (dump_file,
-  "%s:%d SMS succeeded %d %d (with ii, sc)\n",
-  insn_file (tail), insn_line (

Re: MAINTAINERS (Write After Approval): Move myself to maintain alphabetical order.

[Diego Novillo]

On 12-02-03 09:33 , Kumar, Venkataramanan wrote:

Hi all,

I have inserted my name at correct order in the MAINTAINERS list.

Ok to commit ? please see the patch below.


Yes.  These obviously correct changes do not need further approval. 
Simply post the patch to the list and commit it.



Diego.

Re: [wwwdocs] note in svnwrite.html to follow convention in MAINTAINERS file

[Diego Novillo]

On 12-02-03 11:36 , Quentin Neill wrote:

On Fri, Feb 3, 2012 at 12:58 PM, Diego Novillo mailto:dnovi...@google.com>> wrote:

On 12-02-03 09:33 , Kumar, Venkataramanan wrote:

Hi all,

I have inserted my name at correct order in the MAINTAINERS list.

Ok to commit ? please see the patch below.


Yes.  These obviously correct changes do not need further approval.
Simply post the patch to the list and commit it.

Diego.


Given this thread
http://gcc.gnu.org/ml/gcc-patches/2012-02/threads.html#00152
does the patch inlined below make sense?

If so okay to commit?


This is fine.  Thanks.


Diego.

[Patch,AVR]: Clean up hard-coded SFR addresses

[Georg-Johann Lay]

This patch removes the define_constants from avr.md:
SREG_ADDR, SP_ADDR, RAMPZ_ADDR.

The constants were not used in md directly and didn't take care of afr_offset
between RAM and I/O address.

The replacement is a new structure avr_addr that holds RAM addresses of
respective SFRs and takes into account avr_current_arch->sfr_offset.

sfr_offset is the same for all architectures, but that may change in the future.

Tested without regression.

Ok for trunk?

Johann

* config/avr/avr.md (SREG_ADDR): Remove constant definition.
(SP_ADDR): Ditto.
(RAMPZ_ADDR): Ditto.
* config/avr/avr.c (avr_addr_t): New typedef.
(avr_addr): New struct to hold RAM address of SP, RAMPZ, SREG.
(avr_init_expanders): Initialize it.
(expand_prologue): Use avr_addr instead of RAMPZ_ADDR, SP_ADDR,
SREG_ADDR.
(expand_epilogue): Ditto.
(avr_print_operand): Ditto.
(avr_file_start): Ditto.
(avr_emit_movmemhi): Ditto.
Index: config/avr/avr.md
===
--- config/avr/avr.md	(revision 183874)
+++ config/avr/avr.md	(working copy)
@@ -57,12 +57,6 @@ (define_constants
(LPM_REGNO	0)	; implicit target register of LPM
(TMP_REGNO	0)	; temporary register r0
(ZERO_REGNO	1)	; zero register r1
-
-   ;; RAM addresses of some SFRs common to all Devices.
-
-   (SREG_ADDR   0x5F)   ; Status Register
-   (SP_ADDR 0x5D)   ; Stack Pointer
-   (RAMPZ_ADDR  0x5B)   ; Address' high part when loading via ELPM
])
 
 (define_c_enum "unspec"
Index: config/avr/avr.c
===
--- config/avr/avr.c	(revision 183874)
+++ config/avr/avr.c	(working copy)
@@ -104,6 +104,23 @@ static const char* const progmem_section
 ".progmem5.data"
   };
 
+/* Holding RAM addresses of some SFRs used by the compiler and that
+   are unique over all devices in an architecture like 'avr4'.  */
+  
+typedef struct
+{
+  /* SREG: The pocessor status */
+  int sreg;
+
+  /* RAMPZ: The high byte of 24-bit address used with ELPM */ 
+  int rampz;
+
+  /* SP: The stack pointer and its low and high byte */
+  int sp, sp_l, sp_h;
+} avr_addr_t;
+
+static avr_addr_t avr_addr;
+
 
 /* Prototypes for local helper functions.  */
 
@@ -394,6 +411,19 @@ avr_option_override (void)
   avr_current_device = &avr_mcu_types[avr_mcu_index];
   avr_current_arch = &avr_arch_types[avr_current_device->arch];
   avr_extra_arch_macro = avr_current_device->macro;
+  
+  /* RAM addresses of some SFRs common to all Devices in respective Arch. */
+
+  /* SREG: Status Register containing flags like I (global IRQ) */
+  avr_addr.sreg = 0x3F + avr_current_arch->sfr_offset;
+
+  /* RAMPZ: Address' high part when loading via ELPM */
+  avr_addr.rampz = 0x3B + avr_current_arch->sfr_offset;
+
+  /* SP: Stack Pointer (SP_H:SP_L) */
+  avr_addr.sp = 0x3D + avr_current_arch->sfr_offset;
+  avr_addr.sp_l = avr_addr.sp;
+  avr_addr.sp_h = avr_addr.sp + 1;
 
   init_machine_status = avr_init_machine_status;
 
@@ -433,7 +463,7 @@ avr_init_expanders (void)
 
   lpm_addr_reg_rtx = gen_rtx_REG (HImode, REG_Z);
 
-  rampz_rtx = gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR));
+  rampz_rtx = gen_rtx_MEM (QImode, GEN_INT (avr_addr.rampz));
 
   xstring_empty = gen_rtx_CONST_STRING (VOIDmode, "");
   xstring_e = gen_rtx_CONST_STRING (VOIDmode, "e");
@@ -1133,7 +1163,8 @@ expand_prologue (void)
 
   /* Push SREG.  */
   /* ??? There's no dwarf2 column reserved for SREG.  */
-  emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)));
+  emit_move_insn (tmp_reg_rtx,
+  gen_rtx_MEM (QImode, GEN_INT (avr_addr.sreg)));
   emit_push_byte (TMP_REGNO, false);
 
   /* Push RAMPZ.  */
@@ -1386,7 +1417,7 @@ expand_epilogue (bool sibcall_p)
   /* Restore SREG using tmp reg as scratch.  */
   
   emit_pop_byte (TMP_REGNO);
-  emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)), 
+  emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (avr_addr.sreg)), 
   tmp_reg_rtx);
 
   /* Restore tmp REG.  */
@@ -1869,17 +1900,14 @@ avr_print_operand (FILE *file, rtx x, in
   else if (low_io_address_operand (x, VOIDmode)
|| high_io_address_operand (x, VOIDmode))
 {
-  switch (ival)
+  if (ival == avr_addr.rampz)   fprintf (file, "__RAMPZ__");
+  else if (ival == avr_addr.sreg)   fprintf (file, "__SREG__");
+  else if (ival == avr_addr.sp_l)   fprintf (file, "__SP_L__");
+  else if (ival == avr_addr.sp_h)   fprintf (file, "__SP_H__");
+  else
 {
-case RAMPZ_ADDR: fprintf (file, "__RAMPZ__"); break;
-case SREG_ADDR: fprintf (file, "__SREG__"); break;
-case SP_ADDR:   fprintf (file, "__SP_L__"); break;
-case SP_ADDR+1: fprintf (file, "__SP_H__"); break;
-  
-default:
   

[v3] libstdc++/51811

[Benjamin Kosnik]

Fix-ups for atomic<_tp*> operators. Found some open issues with
atomic that merit further study.

tested x86/linux

-benjamin2012-02-02  Benjamin Kosnik  

	PR libstdc++/52068
	* src/c++11/Makefile.am (toolexeclib_LTLIBRARIES,
	libc__11_la_SOURCES): Remove.
	* src/c++11/Makefile.in: Regenerate.
	* src/c++98/Makefile.am (toolexeclib_LTLIBRARIES,
	libc__98_la_SOURCES): Remove.
	* src/c++98/Makefile.in: Regenerate.

diff --git a/libstdc++-v3/src/c++11/Makefile.am b/libstdc++-v3/src/c++11/Makefile.am
index cc454bb..395af5c 100644
--- a/libstdc++-v3/src/c++11/Makefile.am
+++ b/libstdc++-v3/src/c++11/Makefile.am
@@ -25,7 +25,6 @@
 include $(top_srcdir)/fragment.am
 
 # Convenience library for C++11 runtime.
-toolexeclib_LTLIBRARIES = libc++11.la
 noinst_LTLIBRARIES = libc++11convenience.la
 
 headers =
@@ -63,7 +62,6 @@ endif
 vpath % $(top_srcdir)/src/c++11
 vpath % $(top_srcdir)
 
-libc__11_la_SOURCES = $(sources) $(inst_sources)
 libc__11convenience_la_SOURCES = $(sources)  $(inst_sources)
 
 # AM_CXXFLAGS needs to be in each subdirectory so that it can be
diff --git a/libstdc++-v3/src/c++98/Makefile.am b/libstdc++-v3/src/c++98/Makefile.am
index d5d39d1..e960d94 100644
--- a/libstdc++-v3/src/c++98/Makefile.am
+++ b/libstdc++-v3/src/c++98/Makefile.am
@@ -25,7 +25,6 @@
 include $(top_srcdir)/fragment.am
 
 # Convenience library for C++98 runtime.
-toolexeclib_LTLIBRARIES = libc++98.la
 noinst_LTLIBRARIES = libc++98convenience.la
 
 headers =
@@ -156,7 +155,6 @@ sources = \
 vpath % $(top_srcdir)/src/c++98
 vpath % $(top_srcdir)
 
-libc__98_la_SOURCES = $(sources)
 libc__98convenience_la_SOURCES = $(sources)
 
 # Use special rules for the deprecated source files so that they find

Re: [v3] libstdc++/51811

[Jakub Jelinek]

On Fri, Feb 03, 2012 at 11:52:44AM -0800, Benjamin Kosnik wrote:
> 
> Fix-ups for atomic<_tp*> operators. Found some open issues with
> atomic that merit further study.

Different patch attached?

Jakub

Re: [v3] libstdc++/51811

[Benjamin Kosnik]

> Different patch attached?

Indeed, sorry. Here's the right one.

-benjamin2012-02-03  Benjamin Kosnik  

	PR libstdc++/51811
	* include/bits/atomic_base.h (atomic<_Tp*>): Fix offsets.
	* testsuite/29_atomics/atomic/operators/51811.cc: New.
	* testsuite/29_atomics/atomic/operators/pointer_partial_void.cc: New.

diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h
index aa43bcc..9d5f4eb 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -1,6 +1,6 @@
 // -*- C++ -*- header.
 
-// Copyright (C) 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright (C) 2008, 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
 // software; you can redistribute it and/or modify it under the
@@ -621,6 +621,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   __pointer_type 	_M_p;
 
+  // Factored out to facilitate explicit specialization.
+  constexpr ptrdiff_t
+  _M_type_size(ptrdiff_t __d) { return __d * sizeof(_PTp); }
+
+  constexpr ptrdiff_t
+  _M_type_size(ptrdiff_t __d) volatile { return __d * sizeof(_PTp); }
+
 public:
   __atomic_base() noexcept = default;
   ~__atomic_base() noexcept = default;
@@ -669,43 +676,51 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   __pointer_type
   operator++() noexcept
-  { return __atomic_add_fetch(&_M_p, 1, memory_order_seq_cst); }
+  { return __atomic_add_fetch(&_M_p, _M_type_size(1),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator++() volatile noexcept
-  { return __atomic_add_fetch(&_M_p, 1, memory_order_seq_cst); }
+  { return __atomic_add_fetch(&_M_p, _M_type_size(1),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator--() noexcept
-  { return __atomic_sub_fetch(&_M_p, 1, memory_order_seq_cst); }
+  { return __atomic_sub_fetch(&_M_p, _M_type_size(1),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator--() volatile noexcept
-  { return __atomic_sub_fetch(&_M_p, 1, memory_order_seq_cst); }
+  { return __atomic_sub_fetch(&_M_p, _M_type_size(1),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator+=(ptrdiff_t __d) noexcept
-  { return __atomic_add_fetch(&_M_p, __d, memory_order_seq_cst); }
+  { return __atomic_add_fetch(&_M_p, _M_type_size(__d),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator+=(ptrdiff_t __d) volatile noexcept
-  { return __atomic_add_fetch(&_M_p, __d, memory_order_seq_cst); }
+  { return __atomic_add_fetch(&_M_p, _M_type_size(__d),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator-=(ptrdiff_t __d) noexcept
-  { return __atomic_sub_fetch(&_M_p, __d, memory_order_seq_cst); }
+  { return __atomic_sub_fetch(&_M_p, _M_type_size(__d),
+  memory_order_seq_cst); }
 
   __pointer_type
   operator-=(ptrdiff_t __d) volatile noexcept
-  { return __atomic_sub_fetch(&_M_p, __d, memory_order_seq_cst); }
+  { return __atomic_sub_fetch(&_M_p, _M_type_size(__d),
+  memory_order_seq_cst); }
 
   bool
   is_lock_free() const noexcept
-  { return __atomic_is_lock_free (sizeof (_M_p), &_M_p); }
+  { return __atomic_is_lock_free(_M_type_size(1), &_M_p); }
 
   bool
   is_lock_free() const volatile noexcept
-  { return __atomic_is_lock_free (sizeof (_M_p), &_M_p); }
+  { return __atomic_is_lock_free(_M_type_size(1), &_M_p); }
 
   void
   store(__pointer_type __p,
@@ -789,22 +804,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __pointer_type
   fetch_add(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) noexcept
-  { return __atomic_fetch_add(&_M_p, __d, __m); }
+  { return __atomic_fetch_add(&_M_p, _M_type_size(__d), __m); }
 
   __pointer_type
   fetch_add(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return __atomic_fetch_add(&_M_p, __d, __m); }
+  { return __atomic_fetch_add(&_M_p, _M_type_size(__d), __m); }
 
   __pointer_type
   fetch_sub(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) noexcept
-  { return __atomic_fetch_sub(&_M_p, __d, __m); }
+  { return __atomic_fetch_sub(&_M_p, _M_type_size(__d), __m); }
 
   __pointer_type
   fetch_sub(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return __atomic_fetch_sub(&_M_p, __d, __m); }
+  { return __atomic_fetch_sub(&_M_p, _M_type_size(__d), __m); }
 };
 
   // @} group atomics
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/operators/51811.cc b/libstdc++-v3/testsuite/29_atomics/atomic/operators/51811.cc
new file mode 100644
index 000..7c234f2
--- /dev/null
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/operators/51811.cc
@@ -0,0 +1,90 @@
+// { dg-options "-std=gnu++0x" }
+
+// Copyright (C) 2012 Free Softwa

[v3] libstdc++/49445

[Benjamin Kosnik]

I'm just going to check in this PR's derived test case, since it seems
like something we should be tracking.

tested x86/linux

-benjamindiff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 656cc96..187ee49 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,10 @@
 2012-02-03  Benjamin Kosnik  
 
+	PR libstdc++/49445
+	* testsuite/29_atomics/atomic/cons/49445.cc: Add.
+
+2012-02-03  Benjamin Kosnik  
+
 	PR libstdc++/51811
 	* include/bits/atomic_base.h (atomic<_Tp*>): Fix offsets.
 	* testsuite/29_atomics/atomic/operators/51811.cc: New.
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/cons/49445.cc b/libstdc++-v3/testsuite/29_atomics/atomic/cons/49445.cc
new file mode 100644
index 000..569df4a
--- /dev/null
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/cons/49445.cc
@@ -0,0 +1,42 @@
+// { dg-options "-std=gnu++0x" }
+
+// Copyright (C) 2012 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+
+enum class tacos : int
+{
+  cancun = 4,
+  el_loco = 5,
+  sabor = 6,
+  papalote = 9,
+  licious = 44,
+  jarritos = 55
+};
+
+// should minimally compile and link
+int main()
+{
+  std::atomic af(0.0f);
+  float non_af = af;
+  
+  std::atomic ae(tacos::sabor);
+  tacos non_ae = ae;
+
+  return 0;
+}

RE: [Patch,AVR]: Clean up hard-coded SFR addresses

[Weddington, Eric]

> -Original Message-
> From: Georg-Johann Lay [mailto:a...@gjlay.de]
> Sent: Friday, February 03, 2012 12:47 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Denis Chertykov; Weddington, Eric
> Subject: [Patch,AVR]: Clean up hard-coded SFR addresses
> 
> This patch removes the define_constants from avr.md:
> SREG_ADDR, SP_ADDR, RAMPZ_ADDR.
> 
> The constants were not used in md directly and didn't take care of
afr_offset
> between RAM and I/O address.
> 
> The replacement is a new structure avr_addr that holds RAM addresses
of
> respective SFRs and takes into account avr_current_arch->sfr_offset.
> 
> sfr_offset is the same for all architectures, but that may change in
the
> future.
> 
> Tested without regression.
> 
> Ok for trunk?

In the struct avr_addr_t is there any reason why you didn't want to have
the low and high bytes of the stack pointer in a union with the full
stack pointer?

Eric

[google] Fixing function patching tests (issue5626049)

[Harshit Chopra]

Fixes the following tests by restricting them to 64-bit target environment.

Tested: Using 'make -k check-gcc RUNTESTFLAGS="i386.exp=patch* 
--target_board=unix\{-m32,,-m64\}"' and crosstool-validate.py script.

Patch to be applied to google/main branch.

2012-02-03   Harshit Chopra  

  Restricting the tests to run for 64-bit target since function patching
  is only supported for 64-bit targets.

* gcc/testsuite/gcc.target/i386/patch-functions-1.c:
* gcc/testsuite/gcc.target/i386/patch-functions-2.c:
* gcc/testsuite/gcc.target/i386/patch-functions-3.c:
* gcc/testsuite/gcc.target/i386/patch-functions-4.c:
* gcc/testsuite/gcc.target/i386/patch-functions-5.c:
* gcc/testsuite/gcc.target/i386/patch-functions-6.c:
* gcc/testsuite/gcc.target/i386/patch-functions-7.c:

diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
index 1d88d45..308e8c3 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
@@ -1,5 +1,6 @@
 /* Verify -mpatch-functions-for-instrumentation works.  */
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation" } */
 
 /* Check nop-bytes at beginning.  */
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
index ed6e7ce..6baad32 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation 
-mno-patch-functions-main-always" } */
 
 /* Function is small to be instrumented with default values. Check there
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
index d7449c5..49b57a8 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation --param 
function-patch-min-instructions=0" } */
 
 /* Function should have nop-bytes with -mpatch-function-min-instructions=0.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
index f554d92..5fcbd0f 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation 
-mpatch-functions-ignore-loops -mno-patch-functions-main-always" } */
 
 /* Function is too small to be patched when ignoring the loop.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
index bd1dafb..1f9cccf 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation 
-mpatch-functions-ignore-loops --param function-patch-min-instructions=0" } */
 
 /* Function should be patched with nop bytes with given options.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
index 46b715d..5bf844f 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation" } */
 
 /* 'main' function should always be patched, irrespective of how small it is.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
index adeda59..e2c50a0 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-mpatch-functions-for-instrumentation 
-mno-patch-functions-main-always" } */
 
 /* 'main' shouldn't be patched with the option 
-mno-patch-functions-main-always.

--
This patch is available for review at http://codereview.appspot.com/5626049

[Patch] PR 52118 (Description of Wunused-local-typedefs)

[Paolo Carlini]

Hi,

when Dodji added the new warning something went wrong with the 
description in c.opt, which seems truncated. I'm proposing the below, 
very close to the text in invoke.texi besides the plural form, which 
seems more consistent. Is it Ok with you?


Thanks,
Paolo.

//
2012-02-04  Paolo Carlini  

PR c/52118
* c.opt ([Wunused-local-typedefs]): Fix description.

Index: c.opt
===
--- c.opt   (revision 183883)
+++ c.opt   (working copy)
@@ -1,6 +1,6 @@
 ; Options for the C, ObjC, C++ and ObjC++ front ends.
 ; Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
-; 2011 Free Software Foundation, Inc.
+; 2011, 2012 Free Software Foundation, Inc.
 ;
 ; This file is part of GCC.
 ;
@@ -663,7 +663,7 @@ Warn about unsuffixed float constants
 
 Wunused-local-typedefs
 C ObjC C++ ObjC++ Var(warn_unused_local_typedefs) Warning
-Warn about
+Warn when typedefs locally defined in a function are not used
 
 Wunused-macros
 C ObjC C++ ObjC++ Warning

Re: [google] Fixing function patching tests (issue5626049)

[Xinliang David Li]

ok for google branches.

David

On Fri, Feb 3, 2012 at 3:20 PM, Harshit Chopra  wrote:
> Fixes the following tests by restricting them to 64-bit target environment.
>
> Tested: Using 'make -k check-gcc RUNTESTFLAGS="i386.exp=patch* 
> --target_board=unix\{-m32,,-m64\}"' and crosstool-validate.py script.
>
> Patch to be applied to google/main branch.
>
> 2012-02-03   Harshit Chopra  
>
>  Restricting the tests to run for 64-bit target since function patching
>  is only supported for 64-bit targets.
>
>        * gcc/testsuite/gcc.target/i386/patch-functions-1.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-2.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-3.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-4.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-5.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-6.c:
>        * gcc/testsuite/gcc.target/i386/patch-functions-7.c:
>
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
> index 1d88d45..308e8c3 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
> @@ -1,5 +1,6 @@
>  /* Verify -mpatch-functions-for-instrumentation works.  */
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation" } */
>
>  /* Check nop-bytes at beginning.  */
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
> index ed6e7ce..6baad32 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation 
> -mno-patch-functions-main-always" } */
>
>  /* Function is small to be instrumented with default values. Check there
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
> index d7449c5..49b57a8 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation --param 
> function-patch-min-instructions=0" } */
>
>  /* Function should have nop-bytes with -mpatch-function-min-instructions=0.
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
> index f554d92..5fcbd0f 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation 
> -mpatch-functions-ignore-loops -mno-patch-functions-main-always" } */
>
>  /* Function is too small to be patched when ignoring the loop.
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
> index bd1dafb..1f9cccf 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation 
> -mpatch-functions-ignore-loops --param function-patch-min-instructions=0" } */
>
>  /* Function should be patched with nop bytes with given options.
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
> index 46b715d..5bf844f 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation" } */
>
>  /* 'main' function should always be patched, irrespective of how small it is.
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c 
> b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
> index adeda59..e2c50a0 100644
> --- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run} */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-options "-mpatch-functions-for-instrumentation 
> -mno-patch-functions-main-always" } */
>
>  /* 'main' shouldn't be patched with the option 
> -mno-patch-functions-main-always.
>
> --
> This patch is available for review at http://codereview.appspot.com/5626049

Re: [google] Fixing function patching tests (issue5626049)

[Harshit Chopra]

Thanks for the review. Committed to google-main.

--
Harshit



On Fri, Feb 3, 2012 at 4:33 PM, Xinliang David Li  wrote:
> ok for google branches.
>
> David
>
> On Fri, Feb 3, 2012 at 3:20 PM, Harshit Chopra  wrote:
>> Fixes the following tests by restricting them to 64-bit target environment.
>>
>> Tested: Using 'make -k check-gcc RUNTESTFLAGS="i386.exp=patch* 
>> --target_board=unix\{-m32,,-m64\}"' and crosstool-validate.py script.
>>
>> Patch to be applied to google/main branch.
>>
>> 2012-02-03   Harshit Chopra  
>>
>>  Restricting the tests to run for 64-bit target since function patching
>>  is only supported for 64-bit targets.
>>
>>        * gcc/testsuite/gcc.target/i386/patch-functions-1.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-2.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-3.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-4.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-5.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-6.c:
>>        * gcc/testsuite/gcc.target/i386/patch-functions-7.c:
>>
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
>> index 1d88d45..308e8c3 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
>> @@ -1,5 +1,6 @@
>>  /* Verify -mpatch-functions-for-instrumentation works.  */
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation" } */
>>
>>  /* Check nop-bytes at beginning.  */
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
>> index ed6e7ce..6baad32 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation 
>> -mno-patch-functions-main-always" } */
>>
>>  /* Function is small to be instrumented with default values. Check there
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
>> index d7449c5..49b57a8 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation --param 
>> function-patch-min-instructions=0" } */
>>
>>  /* Function should have nop-bytes with -mpatch-function-min-instructions=0.
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
>> index f554d92..5fcbd0f 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation 
>> -mpatch-functions-ignore-loops -mno-patch-functions-main-always" } */
>>
>>  /* Function is too small to be patched when ignoring the loop.
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
>> index bd1dafb..1f9cccf 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation 
>> -mpatch-functions-ignore-loops --param function-patch-min-instructions=0" } 
>> */
>>
>>  /* Function should be patched with nop bytes with given options.
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
>> index 46b715d..5bf844f 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation" } */
>>
>>  /* 'main' function should always be patched, irrespective of how small it 
>> is.
>> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c 
>> b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
>> index adeda59..e2c50a0 100644
>> --- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c
>> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
>> @@ -1,4 +1,5 @@
>> -/* { dg-do run} */
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target lp64 } */
>>  /* { dg-options "-mpatch-functions-for-instrumentation 
>> -mno-patch-functions

Re: [Patch,AVR]: Clean up hard-coded SFR addresses

[Georg-Johann Lay]

This patch removes the define_constants from avr.md:
SREG_ADDR, SP_ADDR, RAMPZ_ADDR.

The constants were not used in md directly and didn't take care of
afr_offset between RAM and I/O address.

The replacement is a new structure avr_addr that holds RAM addresses
of respective SFRs and takes into account avr_current_arch->sfr_offset.
sfr_offset is the same for all architectures, but that may change in
the future.

Tested without regression.

Ok for trunk?


In the struct avr_addr_t is there any reason why you didn't want to have
the low and high bytes of the stack pointer in a union with the full
stack pointer?


What would be the purpose of such a union?
Removing the .sp = .sp_l sugar? If so, then removing one of them 
altogether is the clearest approach, imo.


Johann

Go patch committed: Fix type hash codes and PtrTo

[Ian Lance Taylor]

This patch does two related things.  First, it fixes the hash code in
the type descript for a named type so that it matches the hash code the
compiler will use internally.  Second, it fixes the hash code in the new
type structure created by reflect.PtrTo to match the hash code which the
compiler will generate for that type.  Also it changes PtrTo to
canonicalize the type.  A change to the reflect test is included; I've
already committed that change in the upstream sources.  Bootstrapped and
ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

diff -r a935765ae010 go/types.cc
--- a/go/types.cc	Thu Feb 02 17:05:05 2012 -0800
+++ b/go/types.cc	Fri Feb 03 17:18:52 2012 -0800
@@ -1658,7 +1658,12 @@
 
   ++p;
   go_assert(p->is_field_name("hash"));
-  mpz_set_ui(iv, this->hash_for_method(gogo));
+  unsigned int h;
+  if (name != NULL)
+h = name->hash_for_method(gogo);
+  else
+h = this->hash_for_method(gogo);
+  mpz_set_ui(iv, h);
   vals->push_back(Expression::make_integer(&iv, p->type(), bloc));
 
   ++p;
diff -r a935765ae010 libgo/go/reflect/all_test.go
--- a/libgo/go/reflect/all_test.go	Thu Feb 02 17:05:05 2012 -0800
+++ b/libgo/go/reflect/all_test.go	Fri Feb 03 17:18:52 2012 -0800
@@ -1528,6 +1528,18 @@
 	if p.X != 4 {
 		t.Errorf("Addr.Elem.Set valued to set value in top value")
 	}
+
+	// Verify that taking the address of a type gives us a pointer
+	// which we can convert back using the usual interface
+	// notation.
+	var s struct {
+		B *bool
+	}
+	ps := ValueOf(&s).Elem().Field(0).Addr().Interface()
+	*(ps.(**bool)) = new(bool)
+	if s.B == nil {
+		t.Errorf("Addr.Interface direct assignment failed")
+	}
 }
 
 /* gccgo does do allocations here.
diff -r a935765ae010 libgo/go/reflect/type.go
--- a/libgo/go/reflect/type.go	Thu Feb 02 17:05:05 2012 -0800
+++ b/libgo/go/reflect/type.go	Fri Feb 03 17:18:52 2012 -0800
@@ -999,6 +999,17 @@
 		return &p.commonType
 	}
 
+	s := "*" + *ct.string
+
+	canonicalTypeLock.RLock()
+	r, ok := canonicalType[s]
+	canonicalTypeLock.RUnlock()
+	if ok {
+		ptrMap.m[ct] = (*ptrType)(unsafe.Pointer(r.(*commonType)))
+		ptrMap.Unlock()
+		return r.(*commonType)
+	}
+
 	rp := new(runtime.PtrType)
 
 	// initialize p using *byte's ptrType as a prototype.
@@ -1008,7 +1019,6 @@
 	bp := (*ptrType)(unsafe.Pointer(unsafe.Typeof((*byte)(nil)).(*runtime.PtrType)))
 	*p = *bp
 
-	s := "*" + *ct.string
 	p.string = &s
 
 	// For the type structures linked into the binary, the
@@ -1016,12 +1026,16 @@
 	// Create a good hash for the new string by using
 	// the FNV-1 hash's mixing function to combine the
 	// old hash and the new "*".
-	p.hash = ct.hash*16777619 ^ '*'
+	// p.hash = ct.hash*16777619 ^ '*'
+	// This is the gccgo version.
+	p.hash = (ct.hash << 4) + 9
 
 	p.uncommonType = nil
 	p.ptrToThis = nil
 	p.elem = (*runtime.Type)(unsafe.Pointer(ct))
 
+	p = canonicalize(p).(*ptrType)
+
 	ptrMap.m[ct] = p
 	ptrMap.Unlock()
 	return &p.commonType

[PATCH]Fix PR51867, in which gcc generates inconsistent code for same functions

[Bin Cheng]

Hi,

Here is the patch fixing pr51867 by removing the redundant check on
DECL_ASSEMBLER_NAME_SET_P.
I also changed '-O0' to '-O1' in signbit-2.c and added a new test.
The new test case won't bite if target cpu does not support hardware sqrtf
instruction.

Tested on arm-eabi and x86, Is it OK?

Thanks.

gcc/ChangeLog:
2012-02-04  Bin Cheng  

PR target/51867
* builtins.c (expand_builtin): Don't check
DECL_ASSEMBLER_NAME_SET_P.

gcc/testsuite/ChangeLog:
2012-02-04  Bin Cheng  

PR target/51867
* testsuite/c-c++-common/dfp/signbit-2.c: Change '-O0' to '-O1'.
* testsuite/gcc.dg/pr51867.c: New test.

Index: gcc/builtins.c
===
--- gcc/builtins.c  (revision 183205)
+++ gcc/builtins.c  (working copy)
@@ -5745,10 +5745,12 @@
 return targetm.expand_builtin (exp, target, subtarget, mode, ignore);
 
   /* When not optimizing, generate calls to library functions for a certain
- set of builtins.  */
+ set of builtins.
+
+ See PR51867.
+ Don't check "DECL_ASSEMBLER_NAME_SET_P (fndecl)" here any more.  */
   if (!optimize
   && !called_as_built_in (fndecl)
-  && DECL_ASSEMBLER_NAME_SET_P (fndecl)
   && fcode != BUILT_IN_ALLOCA
   && fcode != BUILT_IN_ALLOCA_WITH_ALIGN
   && fcode != BUILT_IN_FREE)
Index: gcc/testsuite/gcc.dg/pr51867.c
===
--- gcc/testsuite/gcc.dg/pr51867.c  (revision 0)
+++ gcc/testsuite/gcc.dg/pr51867.c  (revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -fno-math-errno -fdump-rtl-expand" } */
+#include 
+
+float a(float x)
+{
+return sqrtf(x);
+}
+float b(float x)
+{
+return sqrtf(x);
+}
+/* Here the calls to sqrtf should be expanded into CALL_INSNs, rather than
+   fpu sqrtf rtl patterns.  */
+/* { dg-final { scan-rtl-dump-times "call_insn" 2 "expand" } } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */
Index: gcc/testsuite/c-c++-common/dfp/signbit-2.c
===
--- gcc/testsuite/c-c++-common/dfp/signbit-2.c  (revision 183205)
+++ gcc/testsuite/c-c++-common/dfp/signbit-2.c  (working copy)
@@ -1,7 +1,10 @@
-/* { dg-options "-O0" } */
+/* { dg-options "-O1" } */
 
 /* Check that the compiler uses builtins for signbit; if not the link
will fail because library functions are in libm.  */
+/* See PR51867.
+   Since GCC uses library call when optimizing for "-O0", this test
+   case requires at least "-O1" level optimization now.  */
 
 #include "dfp-dbg.h"
 

Merge trunk to gccgo branch

[Ian Lance Taylor]

I have once again merged trunk to gccgo branch, this time merging
revision 183892.

Ian

[RS6000] Fix PR52107, TFmode constant load.

[Alan Modra]

http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01835.html changed the
code I'm tweaking here to use DFmode subregs when loading a TFmode
constant into regs for e500.  This just extends that change to all
rs6000 targets, the simplest fix I found for PR52107, a problem I
discovered when looking at powerpc64-linux libgcc.

If we leave these constant loads as DImode, here's what happens at
various stages of rtl optimisation:

.expand
(insn 7 6 8 (set (reg:DI 35 3)
(mem/u/c:DI (unspec:DI [
(symbol_ref/u:DI ("*.LC1") [flags 0x2])
(reg:DI 2 2)
] UNSPEC_TOCREL) [5 S8 A8])) /src/tmp/floatconst.c:10 -1
 (nil))

.cse1
(insn 7 6 8 2 (set (reg:DI 35 3)
(const_int 4318952042648305664 [0x3bf0])) 
/src/tmp/floatconst.c:10 401 {*movdi_internal64}
 (nil))

.split1
(insn 21 3 22 2 (set (reg:DI 35 3)
(mem/u/c:DI (unspec:DI [
(symbol_ref/u:DI ("*.LC3") [flags 0x2])
(reg:DI 2 2)
] UNSPEC_TOCREL) [5 S8 A8])) /src/tmp/floatconst.c:10 -1
 (expr_list:REG_EQUAL (const_int 1005584384 [0x3bf0])
(nil)))

(insn 22 21 8 2 (set (reg:DI 35 3)
(ashift:DI (reg:DI 35 3)
(const_int 32 [0x20]))) /src/tmp/floatconst.c:10 -1
 (expr_list:REG_EQUAL (const_int 4318952042648305664 [0x3bf0])
(nil)))

It would also be possible to fix this in the rs6000.md movdi splitter
dealing with large constants, at least for this testcase when we know
we are dealing with a hard float reg.  However, I think it's better
not to generate DImode fp values in the first place.  Bootstrapped and
regression tested powerpc64-linux.  OK to apply everywhere?

Note that the comment I remove is no longer true.

PR target/52107
* config/rs6000/rs6000.c (rs6000_emit_move): Don't create DImode
subregs of TFmode.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 183781)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -7006,17 +7006,14 @@ rs6000_emit_move (rtx dest, rtx source, 
   if (!TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128
   && mode == TFmode && GET_CODE (operands[1]) == CONST_DOUBLE)
 {
-  /* DImode is used, not DFmode, because simplify_gen_subreg doesn't
-know how to get a DFmode SUBREG of a TFmode.  */
-  enum machine_mode imode = (TARGET_E500_DOUBLE ? DFmode : DImode);
-  rs6000_emit_move (simplify_gen_subreg (imode, operands[0], mode, 0),
-   simplify_gen_subreg (imode, operands[1], mode, 0),
-   imode);
-  rs6000_emit_move (simplify_gen_subreg (imode, operands[0], mode,
-GET_MODE_SIZE (imode)),
-   simplify_gen_subreg (imode, operands[1], mode,
-GET_MODE_SIZE (imode)),
-   imode);
+  rs6000_emit_move (simplify_gen_subreg (DFmode, operands[0], mode, 0),
+   simplify_gen_subreg (DFmode, operands[1], mode, 0),
+   DFmode);
+  rs6000_emit_move (simplify_gen_subreg (DFmode, operands[0], mode,
+GET_MODE_SIZE (DFmode)),
+   simplify_gen_subreg (DFmode, operands[1], mode,
+GET_MODE_SIZE (DFmode)),
+   DFmode);
   return;
 }
 

-- 
Alan Modra
Australia Development Lab, IBM