Re: [patch i386][google][4.6]Add new target builtins to check for corei7 and amdfam10 (issue5495075)

2011-12-17 Thread Xinliang David Li
ok for google branches.

David

On Fri, Dec 16, 2011 at 7:54 PM, Sriraman Tallam  wrote:
> Add new target builtins __builtin_cpu_is_intel_corei7 and
> __builtin_cpu_is_amdfam10.
>
>        * config/i386/i386-cpuinfo.c (__processor_model): Add new members
>        __cpu_is_intel_corei7 and __cpu_is_amdfam10.
>        (get_amd_cpu): Set __cpu_is_amdfam10.
>        (get_intel_cpu): Set __cpu_is_intel_corei7.
>
>
>        * gcc/config/i386/i386.c
>        (IX86_BUILTIN_CPU_IS_INTEL_COREI7): New enum value.
>        (IX86_BUILTIN_CPU_IS_AMDFAM10): New enum value.
>        (fold_builtin_cpu): Fold the new builtins.
>        (ix86_init_platform_type_builtins): Make new buitins
>        __builtin_cpu_is_intel_corei7 and
>        __builtin_cpu_is_amdfam10.
>        * testsuite/gcc.target/i386/builtin_target.c (fn1):
>        Call the new builtins.
>
> Index: libgcc/config/i386/i386-cpuinfo.c
> ===
> --- libgcc/config/i386/i386-cpuinfo.c   (revision 182428)
> +++ libgcc/config/i386/i386-cpuinfo.c   (working copy)
> @@ -55,9 +55,11 @@ struct __processor_model
>   /* CPU type. */
>   unsigned int __cpu_is_intel_atom : 1;
>   unsigned int __cpu_is_intel_core2 : 1;
> +  unsigned int __cpu_is_intel_corei7 : 1;
>   unsigned int __cpu_is_intel_corei7_nehalem : 1;
>   unsigned int __cpu_is_intel_corei7_westmere : 1;
>   unsigned int __cpu_is_intel_corei7_sandybridge : 1;
> +  unsigned int __cpu_is_amdfam10 : 1;
>   unsigned int __cpu_is_amdfam10_barcelona : 1;
>   unsigned int __cpu_is_amdfam10_shanghai : 1;
>   unsigned int __cpu_is_amdfam10_istanbul : 1;
> @@ -74,12 +76,15 @@ get_amd_cpu (unsigned int family, unsigned int mod
>       switch (model)
>        {
>        case 0x2:
> +         __cpu_model.__cpu_is_amdfam10 = 1;
>          __cpu_model.__cpu_is_amdfam10_barcelona = 1;
>          break;
>        case 0x4:
> +         __cpu_model.__cpu_is_amdfam10 = 1;
>          __cpu_model.__cpu_is_amdfam10_shanghai = 1;
>          break;
>        case 0x8:
> +         __cpu_model.__cpu_is_amdfam10 = 1;
>          __cpu_model.__cpu_is_amdfam10_istanbul = 1;
>          break;
>        default:
> @@ -117,16 +122,19 @@ get_intel_cpu (unsigned int family, unsigned int m
>            case 0x1f:
>            case 0x2e:
>              /* Nehalem.  */
> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>              __cpu_model.__cpu_is_intel_corei7_nehalem = 1;
>              break;
>            case 0x25:
>            case 0x2c:
>            case 0x2f:
>              /* Westmere.  */
> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>              __cpu_model.__cpu_is_intel_corei7_westmere = 1;
>              break;
>            case 0x2a:
>              /* Sandy Bridge.  */
> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>              __cpu_model.__cpu_is_intel_corei7_sandybridge = 1;
>              break;
>            case 0x17:
> Index: gcc/testsuite/gcc.target/i386/builtin_target.c
> ===
> --- gcc/testsuite/gcc.target/i386/builtin_target.c      (revision 182428)
> +++ gcc/testsuite/gcc.target/i386/builtin_target.c      (working copy)
> @@ -31,12 +31,16 @@ fn1 ()
>     return -1;
>   if (__builtin_cpu_is_intel_core2 () < 0)
>     return -1;
> +  if (__builtin_cpu_is_intel_corei7 () < 0)
> +    return -1;
>   if (__builtin_cpu_is_intel_corei7_nehalem () < 0)
>     return -1;
>   if (__builtin_cpu_is_intel_corei7_westmere () < 0)
>     return -1;
>   if (__builtin_cpu_is_intel_corei7_sandybridge () < 0)
>     return -1;
> +  if (__builtin_cpu_is_amdfam10 () < 0)
> +    return -1;
>   if (__builtin_cpu_is_amdfam10_barcelona () < 0)
>     return -1;
>   if (__builtin_cpu_is_amdfam10_shanghai () < 0)
> Index: gcc/config/i386/i386.c
> ===
> --- gcc/config/i386/i386.c      (revision 182428)
> +++ gcc/config/i386/i386.c      (working copy)
> @@ -24500,9 +24500,11 @@ enum ix86_builtins
>   IX86_BUILTIN_CPU_IS_INTEL,
>   IX86_BUILTIN_CPU_IS_INTEL_ATOM,
>   IX86_BUILTIN_CPU_IS_INTEL_CORE2,
> +  IX86_BUILTIN_CPU_IS_INTEL_COREI7,
>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_NEHALEM,
>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_WESTMERE,
>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_SANDYBRIDGE,
> +  IX86_BUILTIN_CPU_IS_AMDFAM10,
>   IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA,
>   IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI,
>   IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL,
> @@ -25981,9 +25983,11 @@ fold_builtin_cpu (enum ix86_builtins fn_code)
>     M_INTEL,
>     M_INTEL_ATOM,
>     M_INTEL_CORE2,
> +    M_INTEL_COREI7,
>     M_INTEL_COREI7_NEHALEM,
>     M_INTEL_COREI7_WESTMERE,
>     M_INTEL_COREI7_SANDYBRIDGE,
> +    M_AMDFAM10,
>     M_AMDFAM10_BARCELONA,
>     M_AMDFAM10_SHANGHAI,
>     M_AMDFAM10_ISTANBUL,
> @@ -26068,6 +26072,11 @@ fold_builtin_cpu (enum ix86_builtins fn_code)
>       field = get_field_from_struct (__processor_model_type, M_INTEL_CORE2);
>       which_st

Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-17 Thread Xinliang David Li
ok for google branches.

David

On Fri, Dec 16, 2011 at 2:05 PM,   wrote:
> I have uploaded a new patch set with all the mentioned changes made. If
> a function has the target attribute it will not be touched by the
> autoclone pass.
>
> Also fixed some test cases which were broken because the clone names
> used '_' instead of '.' for suffixing.
>
>
> On 2011/12/16 19:39:47, davidxl wrote:
>>
>> http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
>> File config/i386/i386.c (right):
>
>
>
> http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569
>>
>> config/i386/i386.c:26569: +mversion_for_core2 (tree
>
> *optimization_node,
>>
>> -> mversionable_for_core2_p ?
>
>
>> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
>> File mversn-dispatch.c (right):
>
>
>
> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931
>>
>> mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
>> Should you assert it instead? Should not clone ctor/dtors.
>
>
>
> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221
>>
>> mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR->preds, 0);
>> {} --> remove
>
>
>
> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389
>>
>> mversn-dispatch.c:2389:
>> How does it interact with manual multi-versioning from user? You
>
> probably don't
>>
>> want to clone functions that are marked with target attributes
>
> (explicitly --
>>
>> not implied from command line).
>
>
>
>
> http://codereview.appspot.com/5490054/


Re: PATCH: Add DEFAULT_LIBC=LIBC_BIONIC and ANDROID_DEFAULT=1 to *-android-*

2011-12-17 Thread Mike Stump
On Dec 16, 2011, at 3:27 PM, Joseph S. Myers wrote:
> I think using the "company" part of the target triplet to mean anything 
> should be avoided;

I disagree.  Think of the string as a key that subscripts a database which has 
precooked answers that are used instead of probing for the answers which are 
used to configure software.  In fact, any meaning whatsoever and the exact 
shape are, if you will completely arbitrary and don't really have any meaning 
in the grand scheme of things, save, just convention and an easier time on 
humans.  I think we have to approach each case one a case by case basis.

That said, in this case, I agree with the notion that the more natural form 
would be:

  arm-nokia-linux-android

where nokia is the hardware vendor the sells the device with the software on 
it.  android isn't a vendor, there is no such company that sells (vends) 
hardware.

> -linux-android (note the "-", not "-linuxandroid") matches the standard GNU 
> practice

Agreed.


Re: PATCH: Add DEFAULT_LIBC=LIBC_BIONIC and ANDROID_DEFAULT=1 to *-android-*

2011-12-17 Thread Mike Stump
On Dec 16, 2011, at 5:43 PM, H.J. Lu wrote:
> When I configure GCC with i686-linux-android target, don't I get
> linux as vendor and android OS?

No:

$ ./config.sub i686-linux-android
i686-pc-linux-android

pc is the vendor, though, I'd claim this is a misnomer and wrong.  A cell phone 
isn't PC compatible (usually).  unknown is the proper vendor, when, you don't 
want to guess or don't know or don't care.  The idea that linux only runs on a 
PC compatible computer, is, well, dated.  It used to be the case it only ran on 
PCs, but that is no longer true.

In matching, one would generally match i686-*-linux-android if one wanted to.  
In config.sub land, you'd usually generate i686-unknown-linux-android.

> Adding another arbitrary string as vendor looks very strange to me.

For typing in a triplet, one can use i686-google-linux-android, if one wanted, 
not that strange.  Really, the user should be able to put any string in place 
of google and it shouldn't matter (except when a target maintainer wants to key 
off the vendor field for whatever reason).


Re: [patch] Fix cygwin ada install [was Re: Yet another issue with gcc current trunk with ada on cygwin]

2011-12-17 Thread Christian Joensson
On 8 December 2011 14:02, Dave Korn wrote:
>
>    Hi again guys,
>
>  After the previous patch, there's still another bug remaining in the Ada
> makefile, relating to how it builds and installs the gnat/gnarl shared 
> libraries.
>
>  Windows doesn't have any concept of an rpath in executables, nor of
> LD_LIBRARY_PATH; all required DLLs must be found on the PATH when an exe is
> invoked.  The Ada shared libraries are currently installed into adaobj/ in the
> gcc private dir, which is not (and should not be) on users' PATHs, so the
> result is that executables compiled with the -shared binder option don't work.
>
>  The attached patch fixes Windows DLLs to be installed into $bindir, and
> while it does that it also generates import libraries, which live in the
> private adaobj/ directory and serve for linking executables to the DLLs (it's
> actually preferred to link against an import library than directly against the
> DLL itself).  Finally it adjusts the name of the DLLs on Cygwin to match the
> cyg*.dll naming scheme used there to avoid clashes with MinGW DLLs.
>
> libada/ChangeLog:
>
>        * Makefile.in (bindir): Import from autoconf and pass down to submake.
>
> gcc/ada/ChangeLog:
>
>        * gcc-interface/Makefile.in (WIN_SO_PREFIX [windows targets]): New
>        Windows-specific make variable.
>        (WIN_SO_INSTALL_DIR [windows targets]): Likewise.
>        (install-gnatlib): Respect the above during installation when set,
>        and also install any windows import library that has been built.
>        (gnatlib-shared-win32): Use WIN_SO_PREFIX to name output DLL and also
>        build a corresponding import library.
>
>  Built, tested, installed on i686-pc-cygwin and x86_64-unknown-linux-gnu, no
> regressions anywhere, verified that DLL install locations are corrected on
> windows and the .so install locations unchanged on Linux.  Ok?

I am happily using this patch and await this to be committed.

I can add that I also, manually, copy the
lto-plugin/.libs/cyglto_plugin-0.dll into $bindir too.

-- 
Cheers,

/ChJ


Re: [patch] Fix cygwin ada install [was Re: Yet another issue with gcc current trunk with ada on cygwin]

2011-12-17 Thread Eric Botcazou
> I am happily using this patch and await this to be committed.

Only after the import library issue is addressed.  What do the other libraries 
bundled with GCC do here?

-- 
Eric Botcazou


Re: [PATCH] Add Octeon2 indexed load instruction support (and also DSP64 LDX support)

2011-12-17 Thread Richard Sandiford
Andrew Pinski  writes:
> Index: testsuite/gcc.target/mips/octeon2-lx-1.c
> ===
> --- testsuite/gcc.target/mips/octeon2-lx-1.c  (revision 0)
> +++ testsuite/gcc.target/mips/octeon2-lx-1.c  (revision 0)
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=octeon2 -O -mgp64" } */
> +
> +#define TEST(N, R, T) \
> +  T f##N (T j, R *b, long long i) { return j + b[i]; } \
> +  T g##N (T j, unsigned R *b, long long i) { return j + b[i]; }
> +
> +TEST (1, char, int)
> +TEST (2, char, long long)
> +/* { dg-final { scan-assembler-times "\tlbx\t" 2 } } */
> +/* { dg-final { scan-assembler-times "\tlbux\t" 2 } } */
> +TEST (3, short, int)
> +TEST (4, short, long long)
> +/* { dg-final { scan-assembler-times "\tlhx\t" 2 } } */
> +/* { dg-final { scan-assembler-times "\tlhux\t" 2 } } */
> +TEST (5, int, long long)
> +/* { dg-final { scan-assembler-times "\tlwx\t" 1 } } */
> +/* { dg-final { scan-assembler-times "\tlwux\t" 1 } } */

There's obviously nothing wrong with testing long long indices,
but it doesn't seem very typical.  I'd prefer the attached, which
tests both "long long" and "int".  (I checked that it works for
-mabi=n32 and -mabi=64 on mips64-linux-gnu.)

> -(define_insn "mips_lwx_"
> -  [(set (match_operand:SI 0 "register_operand" "=d")
> - (mem:SI (plus:P (match_operand:P 1 "register_operand" "d")
> - (match_operand:P 2 "register_operand" "d"]
> -  "ISA_HAS_DSP"
> -  "lwx\t%0,%2(%1)"

(Reviewing the patch made me realise that this ought to be using IMOVE32,
just like lwxs does.  But that's something for another time.)

> +(define_expand "mips_ldx"
> +  [(match_operand:DI 0 "register_operand")
> +   (match_operand 1 "pmode_register_operand")
> +   (match_operand:SI 2 "register_operand")]
> +  "ISA_HAS_DSP && TARGET_64BIT"
> +{
> +  operands[2] = convert_to_mode (Pmode, operands[2], false);
> +  emit_insn (PMODE_INSN (gen_mips_ldx,
> +  (operands[0], operands[1], operands[2])));
> +  DONE;
> +})

Seems like this and lwx could be combined using :GPR.

> -;; This attribute gives the length suffix for a sign- or zero-extension
> -;; instruction.
> -(define_mode_attr size [(QI "b") (HI "h")])
> +;; This attribute gives the length suffix for a sign-, zero-extension
> +;; load and store instruction.
> +(define_mode_attr size [(QI "b") (HI "h") (SI "w") (DI "d")])
> +(define_mode_attr SIZE [(QI "B") (HI "H") (SI "W") (DI "D")])

;; This attribute gives the length suffix for a load or store instruction.
;; The same suffixes work for zero and sign extensions.

> Index: config/mips/mips.c
> ===
> --- config/mips/mips.c(revision 182342)
> +++ config/mips/mips.c(working copy)
> @@ -2159,6 +2159,29 @@ mips_lwxs_address_p (rtx addr)
>  }
>return false;
>  }
> +
> +/* Return true if ADDR matches the pattern for the L{B,H,W,D}{,U}X load 
> +   indexed address instruction.  Note that such addresses are
> +   not considered legitimate in the TARGET_LEGITIMATE_ADDRESS_P
> +   sense, because their use is so restricted.  */
> +
> +static bool
> +mips_loadindexed_address_p (rtx addr, enum machine_mode mode)

Nitlet, but I'd prefer mips_lx_address_p or mips_load_indexed_address_p.

> @@ -3552,6 +3575,11 @@ mips_rtx_costs (rtx x, int code, int out
> *total = COSTS_N_INSNS (2);
> return true;
>   }
> +  if (mips_loadindexed_address_p (addr, mode))
> + {
> +   *total = COSTS_N_INSNS (2);
> +   return true;
> + }

Please combine this with the lwxs condition.

> @@ -12959,6 +12988,9 @@ static const struct mips_builtin_descrip
>DIRECT_BUILTIN (mult, MIPS_DI_FTYPE_SI_SI, dsp_32),
>DIRECT_BUILTIN (multu, MIPS_DI_FTYPE_USI_USI, dsp_32),
>  
> +  /* Built-in functions for the DSP ASE (64-bit only).  */
> +  DIRECT_BUILTIN (ldx, MIPS_DI_FTYPE_POINTER_SI, dsp_64),
> +
>/* The following are for the MIPS DSP ASE REV 2 (32-bit only).  */
>DIRECT_BUILTIN (dpa_w_ph, MIPS_DI_FTYPE_DI_V2HI_V2HI, dspr2_32),
>DIRECT_BUILTIN (dps_w_ph, MIPS_DI_FTYPE_DI_V2HI_V2HI, dspr2_32),

You need to add this to the list in extend.texi too.

> +#define ISA_HAS_LDX  ((ISA_HAS_DSP || TARGET_OCTEON2) && 
> TARGET_64BIT)

Long line:

#define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2) \
 && TARGET_64BIT)

OK otherwise, thanks.

Richard


/* { dg-do compile } */
/* { dg-options "-march=octeon2 -O -mgp64" } */

#define TEST(N, R, T) \
  T fll##N (T j, R *b, long long i) { return j + b[i]; } \
  T gll##N (T j, unsigned R *b, long long i) { return j + b[i]; } \
  T fi##N (T j, R *b, int i) { return j + b[i]; } \
  T gi##N (T j, unsigned R *b, int i) { return j + b[i]; } \

TEST (1, char, int)
TEST (2, char, long long)
/* { dg-final { scan-assembler-times "\tlbx\t" 4 } } */
/* { dg-final { scan-assembler-times "\tlbux\t" 4 } } */
TEST (3, short, in

Ping: Fixing liveness information during prepare_shrink_wrap

2011-12-17 Thread Richard Sandiford
Ping for this prepare_shrink_wrap patch:

  http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00274.html

It fixes some wrong-code regressions on MIPS16.

Richard


Ping: Out-of-order update of new_spill_reg_store[]

2011-12-17 Thread Richard Sandiford
Ping for this reload patch:

http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00266.html

  or perhaps the original:

http://gcc.gnu.org/ml/gcc-patches/2011-10/msg00663.html

which fixes some wrong-code regressions on mips64-linux-gnu.

Richard


Invalid hard-reg decomposition in lower-subreg

2011-12-17 Thread Richard Sandiford
lower-subreg.c:can_decompose_p uses the following condition to test
whether a multiword hard register can be decomposed into words:

return (validate_subreg (word_mode, GET_MODE (x), x, UNITS_PER_WORD)
&& HARD_REGNO_MODE_OK (regno, word_mode));

This doesn't work reliably on MIPS, where a doubleword HI-LO value
cannot be split into independent HI and LO words; the LO word is OK,
but the HI word isn't.  This means that the test works correctly
on big-endian targets, where the invalid HI case comes first,
but not on little-endian ones, where the valid LO one does.

This endian difference is the cause of the mips-sde-elf build failure
that Maciej reported earlier in the week.  Tested on that target and
on x86_64-linux-gnu.  OK to install?

Richard


gcc/
* lower-subreg.c (can_decompose_p): Check every word of a hard
register.

Index: gcc/lower-subreg.c
===
--- gcc/lower-subreg.c  2011-12-14 20:44:42.0 +
+++ gcc/lower-subreg.c  2011-12-14 21:04:09.0 +
@@ -634,8 +634,15 @@ can_decompose_p (rtx x)
   unsigned int regno = REGNO (x);
 
   if (HARD_REGISTER_NUM_P (regno))
-   return (validate_subreg (word_mode, GET_MODE (x), x, UNITS_PER_WORD)
-   && HARD_REGNO_MODE_OK (regno, word_mode));
+   {
+ unsigned int byte, num_bytes;
+
+ num_bytes = GET_MODE_SIZE (GET_MODE (x));
+ for (byte = 0; byte < num_bytes; byte += UNITS_PER_WORD)
+   if (simplify_subreg_regno (regno, GET_MODE (x), byte, word_mode) < 
0)
+ return false;
+ return true;
+   }
   else
return !bitmap_bit_p (subreg_context, regno);
 }


[committed] Fix some MIPS entries in libgcc/config.host

2011-12-17 Thread Richard Sandiford
libgcc wasn't building the soft-float routines for mips-sde-elf,
because the config.host stanza removed t-mips from tmake_file.

Tested on mips-sde-elf and applied.   I should have noticed
this when the patches were posted for review, sorry.

Richard


libgcc/
* config.host (mips*-sde-elf*, mipsisa64sr71k-*-elf*): Add to
tmake_file rather replacing it.

Index: libgcc/config.host
===
--- libgcc/config.host  2011-12-14 22:34:31.0 +
+++ libgcc/config.host  2011-12-14 22:34:45.0 +
@@ -751,7 +751,7 @@ mips*-*-linux*) # Linux MIPS, 
either
 mips*-*-openbsd*)
;;
 mips*-sde-elf*)
-   tmake_file="mips/t-crtstuff mips/t-mips16"
+   tmake_file="$tmake_file mips/t-crtstuff mips/t-mips16"
case "${with_newlib}" in
  yes)
# newlib / libgloss.
@@ -771,7 +771,7 @@ mipsisa64r2-*-elf* | mipsisa64r2el-*-elf
extra_parts="$extra_parts crti.o crtn.o"
;;
 mipsisa64sr71k-*-elf*)
-   tmake_file="mips/t-elf mips/t-crtstuff t-fdpbit"
+   tmake_file="$tmake_file mips/t-elf mips/t-crtstuff t-fdpbit"
extra_parts="$extra_parts crti.o crtn.o"
 ;;
 mipsisa64sb1-*-elf* | mipsisa64sb1el-*-elf*)


Re: Ignore thunks in ThreadSanitizer pass (issue 5492055)

2011-12-17 Thread Diego Novillo
On Sat, Dec 17, 2011 at 06:53,   wrote:

> Yes, for that particular function on Linux/amd64 I see
> artificial+ignored and is_thunk==0.
>
> So is it LGTY?

Yes.


Diego.


Re: [build] Fix bootstrap/51072: libitm not disabled without c++

2011-12-17 Thread Eric Botcazou
> Putting into gcc/cp/config-lang.in is a layering violation, it's true.  But
> until there's another instance that needs handling, it seems premature to
> build infrastructure to handle this.  And it's only one line after all...

I don't know exactly why, but this breaks bootstrap on some machines:

make[3]: Entering directory 
`/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
g++  -I../../src/libcpp -I. -I../../src/libcpp/../include 
-I../../src/libcpp/include  -O2 -g -W -Wall -Wno-narrowing -Wwrite-strings 
-Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
-fno-rtti -I../../src/libcpp -I. -I../../src/libcpp/../include 
-I../../src/libcpp/include  -c -o 
charset.o -MT 
charset.o -MMD -MP -MF .deps/charset.Tpo ../../src/libcpp/charset.c
cc1plus: error: unrecognized command line option "-Wno-narrowing"
make[3]: *** [charset.o] Error 1
make[3]: Leaving directory 
`/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
make[2]: *** [all-stage2-libcpp] Error 2
make[2]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'
make[1]: *** [stage2-bubble] Error 2
make[1]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'

I have seen that on 3 different machines (i586, i686, x86-64).

-- 
Eric Botcazou


Re: [build] Fix bootstrap/51072: libitm not disabled without c++

2011-12-17 Thread H.J. Lu
On Sat, Dec 17, 2011 at 6:19 AM, Eric Botcazou  wrote:
>> Putting into gcc/cp/config-lang.in is a layering violation, it's true.  But
>> until there's another instance that needs handling, it seems premature to
>> build infrastructure to handle this.  And it's only one line after all...
>
> I don't know exactly why, but this breaks bootstrap on some machines:
>
> make[3]: Entering directory
> `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
> g++  -I../../src/libcpp -I. -I../../src/libcpp/../include 
> -I../../src/libcpp/include  -O2 -g -W -Wall -Wno-narrowing -Wwrite-strings 
> -Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
> -fno-rtti -I../../src/libcpp -I. -I../../src/libcpp/../include 
> -I../../src/libcpp/include  -c -o
> charset.o -MT
> charset.o -MMD -MP -MF .deps/charset.Tpo ../../src/libcpp/charset.c
> cc1plus: error: unrecognized command line option "-Wno-narrowing"
> make[3]: *** [charset.o] Error 1
> make[3]: Leaving directory
> `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
> make[2]: *** [all-stage2-libcpp] Error 2
> make[2]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'
> make[1]: *** [stage2-bubble] Error 2
> make[1]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'
>
> I have seen that on 3 different machines (i586, i686, x86-64).
>

I opened:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51599

-- 
H.J.


Re: [build] Fix bootstrap/51072: libitm not disabled without c++

2011-12-17 Thread Richard Henderson
On 12/17/2011 06:19 AM, Eric Botcazou wrote:
>> Putting into gcc/cp/config-lang.in is a layering violation, it's true.  But
>> until there's another instance that needs handling, it seems premature to
>> build infrastructure to handle this.  And it's only one line after all...
> 
> I don't know exactly why, but this breaks bootstrap on some machines:
> 
> make[3]: Entering directory 
> `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
> g++  -I../../src/libcpp -I. -I../../src/libcpp/../include 
> -I../../src/libcpp/include  -O2 -g -W -Wall -Wno-narrowing -Wwrite-strings 
> -Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
> -fno-rtti -I../../src/libcpp -I. -I../../src/libcpp/../include 
> -I../../src/libcpp/include  -c -o 
> charset.o -MT 
> charset.o -MMD -MP -MF .deps/charset.Tpo ../../src/libcpp/charset.c
> cc1plus: error: unrecognized command line option "-Wno-narrowing"
> make[3]: *** [charset.o] Error 1
> make[3]: Leaving directory 
> `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj/libcpp'
> make[2]: *** [all-stage2-libcpp] Error 2
> make[2]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'
> make[1]: *** [stage2-bubble] Error 2
> make[1]: Leaving directory `/red.a/gnatmail-x/build-red/x86_64-linux/gnat/obj'
> 
> I have seen that on 3 different machines (i586, i686, x86-64).
> 

*shrug* It doesn't happen here.  And it certainly doesn't have anything
to do with that patch.

r~


Re: [patch committed SH] Add atomic patterns

2011-12-17 Thread Oleg Endo
On Mon, 2011-12-05 at 08:07 +0900, Kaz Kojima wrote:

> +  return \"\\
> +mova\\t1f, r0\\n\\
> +\\t\\t%2, %4\\n\\
> +\\tmov\\tr15, r1\\n\\
> +\\tmov\\t#(0f-1f), r15\\n\\
> +0:\\tmov.\\t@%1, %0\\n\\
> +\\tcmp/eq\\t%0, %4\\n\\
> +\\tbf\\t1f\\n\\
> +\\tmov.\\t%3, @%1\\n\\
> +\\t.align\\t2\\n\\
> +1:\\tmov\tr1, r15\";
> +}"
> +  [(set_attr "length" "20")])
> +

The function (C++11)...

#include 

int test (std::atomic& val, int x)
{
  return val += x;
}

results in...

mova1f, r0  ! 7 atomic_add_fetchsi_soft [length = 16]
mov r15, r1
mov #(0f-1f), r15
0:  mov.l   @r4, r2
add r5, r2
mov.l   r2, @r4
.align  2
1:  mov r1, r15
rts ! 27*return_i   [length = 2]
mov r2,r0   ! 12movsi_ie/2  [length = 2]


If I remember correctly, the ISR code in a gUSA implementation does
something like this:

if (interrupted_r15 < 0
&& interrupted_pc < atomic_exitpoint)
  interrupted_pc = interrupted_r0 + interrupted_r15;

This works only if the exit point is right after the write-back insn.
However the above code might end up as:

mova1f, r0
mov r15, r1
mov #(0f-1f), r15
0:  mov.l   @r4, r2
add r5, r2
mov.l   r2, @r4 <-- write-back
nop <-- nop inserted to satisfy .align 2
1:  mov r1, r15 <-- exit point
rts
mov r2,r0

... which means that if the atomic seauence is interrupted at the nop
insn, it would get rewound because it hasn't reached the exit-point yet.
This would make the atomic sequence execute twice, which is not correct.

The .align 2 should be placed somewhere before the write-back insn, or
am I missing something here?


Cheers,
Oleg



Re: [build] Fix bootstrap/51072: libitm not disabled without c++

2011-12-17 Thread Andreas Schwab
Richard Henderson  writes:

> There's no "good" place for this.  The description in Makefile.def that 
> libitm uses c++ is only used insofar as the dependencies for libitm -- it's 
> built after libstdc++ as the language support library.  If we put this into 
> the toplevel configure.ac directly, we have to write a bunch of shell code 
> which seems less than helpful.
>
> Putting into gcc/cp/config-lang.in is a layering violation, it's true.  But 
> until there's another instance that needs handling, it seems premature to 
> build infrastructure to handle this.  And it's only one line after all...

That doesn't work.  It breaks bootstrap on targets that are unsupported
by libitm because the stage2 compiler is built with the host g++ instead
of the stage1 g++, eventually breaking down due to -Werror.

You get the same error when configuring with --disable-target-libitm.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


C++ PATCH for c++/51587 (ICE with struct/enum conflict)

2011-12-17 Thread Jason Merrill
This testcase was aborting when we tried to check ENUM_UNDERLYING_TYPE 
on a non-enum.  Fixed by checking the tree code first.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit b01c647f6754097ac85e4ca1d7c80c1da8edc660
Author: Jason Merrill 
Date:   Sat Dec 17 09:30:43 2011 -0500

	PR c++/51587
	* decl.c (start_enum): Avoid using ENUM_UNDERLYING_TYPE on a
	non-enum.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index fedc52c..0a43fb8 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -12069,6 +12069,7 @@ start_enum (tree name, tree enumtype, tree underlying_type,
   /* Do not push the decl more than once, unless we need to
 	 compare underlying types at instantiation time */
   if (!enumtype
+	  || TREE_CODE (enumtype) != ENUMERAL_TYPE
 	  || (underlying_type
 	  && dependent_type_p (underlying_type))
 	  || (ENUM_UNDERLYING_TYPE (enumtype)
diff --git a/gcc/testsuite/g++.dg/parse/enum6.C b/gcc/testsuite/g++.dg/parse/enum6.C
new file mode 100644
index 000..e753f51
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/enum6.C
@@ -0,0 +1,8 @@
+// PR c++/51587
+
+namespace N
+{
+  struct X;			// { dg-message "previous declaration" }
+}
+
+enum N::X {};			// { dg-error "conflicting declaration" }


[Google Debugfission] Add pubnames and pubtypes to google/main (issue5489074)

2011-12-17 Thread Sterling Augustine
Enclosed for review and inclusion on the google/main and google/4_6 trees is the
first patch for the debugfission project.  It includes producing a complete and
correct set of .debug_pubnames and .debug_pubtypes, as well as switching the
default to generate .debug_pubnames and .debug_pubtypes.

It further (and the most worth of review), changes certain decl printers (which
are used to generate debug info) to be more consistent with the naming
conventions used by demanglers.

Tested:
Via complete bootstap and running the test suite. No new failures found.

ChangeLog:
2011-12-17   Sterling Augustine  

* gcc/dwarf2out.c (is_cu_die, is_namespace_die, is_class_die): New
functions.
(add_pubname): Call is_namespace_die, is_cu_die, and is_class_die in
conditional.
(add_enumerator_pubname): New function.
(add_pubtype): Call is_namespace_die. Rework name calculation.  Call
type_tag, lang_hooks.dwarf_name and add_enumerator_pubname.
(output_pubnames): Output debug_pubnames_section_label or
debug_pubtypes_section_label.
(base_type_die): Call add_pubtype.
(gen_namespace_die): Call add_pubname_string and lang_hooks.dwarf_name.
(dwarf2out_init): Generate debug_pubnames_section_label and
debug_pubtypes_section_label.
(dwarf2out_finish): Call add_AT_lineptr if pubnames or pubtypes is
non-empty.  When dealing with pubnames, change assertion to conditional.
Call pubtypes_section_empty.  Likewise when dealing with pubtypes.
Move code checking for empty section to...
(pubtypes_section_empty): ...here. New function.
* gcc/target.def: Switch default generate pubnames and types to true.

cp-family/ChangeLog:
2011-12-17   Sterling Augustine  

* gcc/c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Move
conditional from beginning to end.

cp/ChangeLog
2011-12-17   Sterling Augustine  

* gcc/cp/error.c (dump_decl): Reformat return value to
"(anonymous namespace)".
(lang_decl_name): Return "(anonymous namespace)" when appropriate.

include/ChangeLog
* include/dwarf2.h (enum dwarf_form): Add forms DW_FORM_GNU_ref_index,
DW_FORM_GNU_addr_index and DW_FORM_GNU_str_index.
(enum dwarf_attribute): Add attributes: Add DW_AT_GNU_dwo_name,
DW_AT_GNU_dwo_id, DW_AT_GNU_ref_base, DW_AT_GNU_addr_base,
DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes.
diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index 621b7f6..55ed41e 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -446,8 +446,6 @@ pp_c_specifier_qualifier_list (c_pretty_printer *pp, tree t)
 {
   const enum tree_code code = TREE_CODE (t);
 
-  if (TREE_CODE (t) != POINTER_TYPE)
-pp_c_type_qualifier_list (pp, t);
   switch (code)
 {
 case REFERENCE_TYPE:
@@ -494,6 +492,8 @@ pp_c_specifier_qualifier_list (c_pretty_printer *pp, tree t)
   pp_simple_type_specifier (pp, t);
   break;
 }
+  if (TREE_CODE (t) != POINTER_TYPE)
+pp_c_type_qualifier_list (pp, t);
 }
 
 /* parameter-type-list:
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 52c84cb..194fc76 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -988,7 +988,7 @@ dump_decl (tree t, int flags)
dump_scope (CP_DECL_CONTEXT (t), flags);
  flags &= ~TFF_UNQUALIFIED_NAME;
  if (DECL_NAME (t) == NULL_TREE)
-   pp_cxx_ws_string (cxx_pp, M_("{anonymous}"));
+   pp_cxx_ws_string (cxx_pp, M_("(anonymous namespace)"));
  else
pp_cxx_tree_identifier (cxx_pp, DECL_NAME (t));
}
@@ -2536,6 +2536,8 @@ lang_decl_name (tree decl, int v, bool translate)
 
   if (TREE_CODE (decl) == FUNCTION_DECL)
 dump_function_name (decl, TFF_PLAIN_IDENTIFIER);
+  else if (DECL_NAME (decl) == NULL && TREE_CODE (decl) == NAMESPACE_DECL)
+pp_string (cxx_pp, M_("(anonymous namespace)"));
   else
 dump_decl (DECL_NAME (decl), TFF_PLAIN_IDENTIFIER);
 
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 844bba3..6512292 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3343,6 +3343,7 @@ static void output_comp_unit (dw_die_ref, int);
 static void output_comdat_type_unit (comdat_type_node *);
 static const char *dwarf2_name (tree, int);
 static void add_pubname (tree, dw_die_ref);
+static void add_enumerator_pubname (const char *, const char *, dw_die_ref);
 static void add_pubname_string (const char *, dw_die_ref);
 static void add_pubtype (tree, dw_die_ref);
 static void output_pubnames (VEC (pubname_entry,gc) *);
@@ -3546,6 +3547,12 @@ static void gen_scheduled_generic_parms_dies (void);
 #ifndef COLD_TEXT_SECTION_LABEL
 #define COLD_TEXT_SECTION_LABEL "Ltext_cold"
 #endif
+#ifndef DEBUG_PUBNAMES_SECTION_LABEL
+#define DEBUG_PUBNAMES_SECTION_LABEL   "Ldebug_pubnames"
+#endif
+#ifndef DEBUG_PUBTYPES_SECTION_LABEL
+#define DEBUG_PUBTYPES_SECTION_LABEL   "Ldebug_pub

Re: [google] Add support for delete operator that takes the size of the object as a parameter

2011-12-17 Thread Easwaran Raman
Based on Paolo's comments I am dropping the changes to
baseline_symbols.txt. As far as minor version, I am bumping it up to
18. Assuming that this patch will be able to go in gcc 4.8 (with minor
version 18), I want to keep it at the same version in google/main and
google/gcc-4_6.

Bootstraps and no test regression. OK for google/main and
google/gcc-4_6 branches?

-
011-12-17  Easwaran Raman  

* common.opt (fsized-delete): New option.

cp/ChangeLog.google-main:

2011-12-17  Easwaran Raman  

* decl.c (cxx_init_decl_processing): Specify a function that
  takes a void* and size_t for DELETE_EXPR.
* call.c (build_op_delete_call): If fsized-delete is used, use
  the variant that takes size_t as the second parameter except
  when deleteting a pointer of type void *.

testsuite/ChangeLog.google-main:

2011-12-17  Easwaran Raman  

* g++.dg/other/sized-delete-1.C: New test.

libstdc++-v3/ChangeLog.google-main:

2011-12-17  Easwaran Raman  

* libsupc++/del_op.cc (delete): Define a new variant
  void operator delete(void*, std::size_t).
* libsupc++/new (delete): Declare
  void operator delete(void*, std::size_t) throw();
* testsuite/util/testsuite_abi.cc (check_version): Add new
  known version GLIBCXX_3.4.18.
* config/abi/pre/gnu.ver: Add new symbol _ZdlPv[jmy] at version
  GLIBCXX_3.4.18.

On Wed, Dec 14, 2011 at 3:31 AM, Paolo Carlini  wrote:
> Hi,
>
>> On Mon, Dec 12, 2011 at 12:41 PM, Paolo Carlini
>>   wrote:
>>>
>>> On 12/12/2011 09:37 PM, Easwaran Raman wrote:

 Thanks for the comments Paolo. I have attached the new patch. I have
 bumped the version to 3.4.18
>>>
>>> You shouldn't: 4.7 is not out yet, thus no reason to increase the minor
>>> version beyond the current 17.
>>
>> Ok, I then don't understand your comment
>>  "Note that backporting the patch as-is to 4_6-branch would be very
>> wrong in terms of ABI (in mainline we already have a 3.4.17)".
>> My original patch added the new symbol in version 3.4.17. Since we
>> don't want to add the symbol to 3.4.16 (if we have a situation where
>> the new runtime is not available when running a program compiled with
>> -fsized-delete) and you   said I shouldn't be using 3.4.17, I assumed
>> I had to bump up the version.
>
> Note this is going to be an academic discussion, because it's too late for
> 4.7 anyway, and I'm not even sure it's conforming to add overloads like this
> for operators.
>
> Anyway.
>
> For 4.6 branch at this point the situation would be very difficult. We could
> have a small time window before 4.7 is out to move **all** its new symbols
> vs 4.6, currently in 3.4.17,  to a new 3.4.18 and then we could bump 4.6 to
> 3.4.17. You see, after a minor is delivered you cannot add anything to it,
> and also you have the general requirement that the minor version is the
> minor ersion, thus irrespective whether we are talking about gcc4.6 or
> gcc4.7, a specific minor version number defines which symbols are exported.
> I hope now the issues are more clear.You must be always very careful.
>
  and used _ZdlPv[jmy] in gnu.ver.  I have
 also added the symbol to baseline_symbols.txt of other targets.
>>>
>>> You should not, just read again what I wrote. And you don't have to
>>> believe
>>> me: just browse the libstdc++ ChangeLogs and see if somebody ever does
>>> that
>>> when the linker map is touched.
>>
>> Sorry, I again misunderstood this as well (and still don't have a good
>> picture). Is the part which adds _ZdlPv[jmy] in gnu.ver ok?
>
> Looks fine yes. But I didn't really check the letters. General rule of thumb
> when fiddling with linker scripts: double check what you are doing with a
> multilib system, like x86_64, at least you can catch errors when you are
> mistakenly not accounting for the 32-bit version of the symbols. In the case
> at issue, size_t can boil down be unsigned int, unsigned long, unsigned long
> long, make sure the pattern includes all three.
>
>> I added
>> that by mimicking the symbol  _Znw[jmy] found in the same file. From
>> the log, it looks like the baseline_symbols.txt seems to be generated,
>> but I am not sure how that is to be done. For example, r145437 says a
>> bunch of these baseline_symbols.txt are regenerated, but I don't see
>> any other change from which this might be generated.
>
> General rule of thumb: normally, if you aren't a release manager, or a
> maintainer, **never** regenerate the baseline_symbols files. Just add
> symbols to the linker script, that is normally fine, doesn't break anything
> per se, adding is fine (subtracting/changing is not, of course).
>
> Paolo.
Index: libstdc++-v3/libsupc++/del_op.cc
===
--- libstdc++-v3/libsupc++/del_op.cc	(revision 182251)
+++ libstdc++-v3/libsupc++/del_op.cc	(working copy)
@@ -46,3 +46,11 @@ operator delete(void* ptr) throw ()
   if (ptr)
 

Re: [patch committed SH] Add atomic patterns

2011-12-17 Thread Oleg Endo
The attached patch should fix the align 2 issues mentioned before and
also fixes the ior fetchop_name.  It should be "or" instead of "ior".

I'm not sure whether it requires re-testing.
Just in case now running:

make -k -check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml/-msoft-atomic,
-m2/-mb/-msoft-atomic,
-m2a-single/-mb/-msoft-atomic,
-m4-single/-ml/-msoft-atomic,
-m4-single/-mb/-msoft-atomic,
-m4a-single/-ml,
-m4a-single/-mb/-msoft-atomic}"


Cheers,
Oleg


ChangeLog:

2011-12-17  Oleg Endo  

* config/sh/sync.md (fetchop_name): Change "ior" to "or".
(atomic_compare_and_swap_soft,
atomic_fetch__soft,
atomic_fetch_nand_soft,
atomic__fetch_soft,
atomic_nand_fetch_soft): Move align 2 above atomic
sequence entrance.
Index: gcc/config/sh/sync.md
===
--- gcc/config/sh/sync.md	(revision 182438)
+++ gcc/config/sh/sync.md	(working copy)
@@ -35,7 +35,7 @@
 
 (define_code_iterator FETCHOP [plus minus ior xor and])
 (define_code_attr fetchop_name
-  [(plus "add") (minus "sub") (ior "ior") (xor "xor") (and "and")])
+  [(plus "add") (minus "sub") (ior "or") (xor "xor") (and "and")])
 (define_code_attr fetchop_insn
   [(plus "add") (minus "sub") (ior "or") (xor "xor") (and "and")])
 
@@ -90,13 +90,13 @@
   return \"\\
 mova\\t1f, r0\\n\\
 \\t\\t%2, %4\\n\\
+\\t.align\\t2\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tcmp/eq\\t%0, %4\\n\\
 \\tbf\\t1f\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "20")])
@@ -142,13 +142,13 @@
 {
   return \"\\
 mova\\t1f, r0\\n\\
+\\t.align\\t2\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tmov\\t%0, %3\\n\\
 \\t\\t%2, %3\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "18")])
@@ -195,13 +195,13 @@
   return \"\\
 mova\\t1f, r0\\n\\
 \\tmov\\tr15, r1\\n\\
+\\t.align\\t2\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tmov\\t%2, %3\\n\\
 \\tand\\t%0, %3\\n\\
 \\tnot\\t%3, %3\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "20")])
@@ -249,11 +249,11 @@
   return \"\\
 mova\\t1f, r0\\n\\
 \\tmov\\tr15, r1\\n\\
+\\t.align\\t2\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\t\\t%2, %0\\n\\
 \\tmov.\\t%0, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "16")])
@@ -300,13 +300,13 @@
 {
   return \"\\
 mova\\t1f, r0\\n\\
+\\t.align\\t2\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tand\\t%2, %0\\n\\
 \\tnot\\t%0, %0\\n\\
 \\tmov.\\t%0, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "18")])


Re: [google] Add support for delete operator that takes the size of the object as a parameter

2011-12-17 Thread Xinliang David Li
ok.

David

On Sat, Dec 17, 2011 at 11:44 AM, Easwaran Raman  wrote:
> Based on Paolo's comments I am dropping the changes to
> baseline_symbols.txt. As far as minor version, I am bumping it up to
> 18. Assuming that this patch will be able to go in gcc 4.8 (with minor
> version 18), I want to keep it at the same version in google/main and
> google/gcc-4_6.
>
> Bootstraps and no test regression. OK for google/main and
> google/gcc-4_6 branches?
>
> -
> 011-12-17  Easwaran Raman  
>
>        * common.opt (fsized-delete): New option.
>
> cp/ChangeLog.google-main:
>
> 2011-12-17  Easwaran Raman  
>
>        * decl.c (cxx_init_decl_processing): Specify a function that
>          takes a void* and size_t for DELETE_EXPR.
>        * call.c (build_op_delete_call): If fsized-delete is used, use
>          the variant that takes size_t as the second parameter except
>          when deleteting a pointer of type void *.
>
> testsuite/ChangeLog.google-main:
>
> 2011-12-17  Easwaran Raman  
>
>        * g++.dg/other/sized-delete-1.C: New test.
>
> libstdc++-v3/ChangeLog.google-main:
>
> 2011-12-17  Easwaran Raman  
>
>        * libsupc++/del_op.cc (delete): Define a new variant
>          void operator delete(void*, std::size_t).
>        * libsupc++/new (delete): Declare
>          void operator delete(void*, std::size_t) throw();
>        * testsuite/util/testsuite_abi.cc (check_version): Add new
>          known version GLIBCXX_3.4.18.
>        * config/abi/pre/gnu.ver: Add new symbol _ZdlPv[jmy] at version
>          GLIBCXX_3.4.18.
>
> On Wed, Dec 14, 2011 at 3:31 AM, Paolo Carlini  
> wrote:
>> Hi,
>>
>>> On Mon, Dec 12, 2011 at 12:41 PM, Paolo Carlini
>>>   wrote:

 On 12/12/2011 09:37 PM, Easwaran Raman wrote:
>
> Thanks for the comments Paolo. I have attached the new patch. I have
> bumped the version to 3.4.18

 You shouldn't: 4.7 is not out yet, thus no reason to increase the minor
 version beyond the current 17.
>>>
>>> Ok, I then don't understand your comment
>>>  "Note that backporting the patch as-is to 4_6-branch would be very
>>> wrong in terms of ABI (in mainline we already have a 3.4.17)".
>>> My original patch added the new symbol in version 3.4.17. Since we
>>> don't want to add the symbol to 3.4.16 (if we have a situation where
>>> the new runtime is not available when running a program compiled with
>>> -fsized-delete) and you   said I shouldn't be using 3.4.17, I assumed
>>> I had to bump up the version.
>>
>> Note this is going to be an academic discussion, because it's too late for
>> 4.7 anyway, and I'm not even sure it's conforming to add overloads like this
>> for operators.
>>
>> Anyway.
>>
>> For 4.6 branch at this point the situation would be very difficult. We could
>> have a small time window before 4.7 is out to move **all** its new symbols
>> vs 4.6, currently in 3.4.17,  to a new 3.4.18 and then we could bump 4.6 to
>> 3.4.17. You see, after a minor is delivered you cannot add anything to it,
>> and also you have the general requirement that the minor version is the
>> minor ersion, thus irrespective whether we are talking about gcc4.6 or
>> gcc4.7, a specific minor version number defines which symbols are exported.
>> I hope now the issues are more clear.You must be always very careful.
>>
>  and used _ZdlPv[jmy] in gnu.ver.  I have
> also added the symbol to baseline_symbols.txt of other targets.

 You should not, just read again what I wrote. And you don't have to
 believe
 me: just browse the libstdc++ ChangeLogs and see if somebody ever does
 that
 when the linker map is touched.
>>>
>>> Sorry, I again misunderstood this as well (and still don't have a good
>>> picture). Is the part which adds _ZdlPv[jmy] in gnu.ver ok?
>>
>> Looks fine yes. But I didn't really check the letters. General rule of thumb
>> when fiddling with linker scripts: double check what you are doing with a
>> multilib system, like x86_64, at least you can catch errors when you are
>> mistakenly not accounting for the 32-bit version of the symbols. In the case
>> at issue, size_t can boil down be unsigned int, unsigned long, unsigned long
>> long, make sure the pattern includes all three.
>>
>>> I added
>>> that by mimicking the symbol  _Znw[jmy] found in the same file. From
>>> the log, it looks like the baseline_symbols.txt seems to be generated,
>>> but I am not sure how that is to be done. For example, r145437 says a
>>> bunch of these baseline_symbols.txt are regenerated, but I don't see
>>> any other change from which this might be generated.
>>
>> General rule of thumb: normally, if you aren't a release manager, or a
>> maintainer, **never** regenerate the baseline_symbols files. Just add
>> symbols to the linker script, that is normally fine, doesn't break anything
>> per se, adding is fine (subtracting/changing is not, of course).
>>
>> Paolo.


Re: [patch committed SH] Add atomic patterns

2011-12-17 Thread Richard Henderson
On 12/17/2011 12:13 PM, Oleg Endo wrote:
> @@ -90,13 +90,13 @@
>return \"\\
>  mova\\t1f, r0\\n\\
>  \\t\\t%2, %4\\n\\
> +\\t.align\\t2\\n\\

The other thing that should be remembered is that inside a { }
block you don't need to double-quote all the \'s.


r~


Re: C++ PATCH for c++/51587 (ICE with struct/enum conflict)

2011-12-17 Thread Fabien Chêne
2011/12/17 Jason Merrill :
> This testcase was aborting when we tried to check ENUM_UNDERLYING_TYPE on a
> non-enum.  Fixed by checking the tree code first.

Ahem ... thank you !

-- 
Fabien


Re: [patch i386][google][4.6]Add new target builtins to check for corei7 and amdfam10 (issue5495075)

2011-12-17 Thread Sriraman Tallam
Checked in.

Thanks,
-Sri.

On Sat, Dec 17, 2011 at 12:21 AM, Xinliang David Li  wrote:
> ok for google branches.
>
> David
>
> On Fri, Dec 16, 2011 at 7:54 PM, Sriraman Tallam  wrote:
>> Add new target builtins __builtin_cpu_is_intel_corei7 and
>> __builtin_cpu_is_amdfam10.
>>
>>        * config/i386/i386-cpuinfo.c (__processor_model): Add new members
>>        __cpu_is_intel_corei7 and __cpu_is_amdfam10.
>>        (get_amd_cpu): Set __cpu_is_amdfam10.
>>        (get_intel_cpu): Set __cpu_is_intel_corei7.
>>
>>
>>        * gcc/config/i386/i386.c
>>        (IX86_BUILTIN_CPU_IS_INTEL_COREI7): New enum value.
>>        (IX86_BUILTIN_CPU_IS_AMDFAM10): New enum value.
>>        (fold_builtin_cpu): Fold the new builtins.
>>        (ix86_init_platform_type_builtins): Make new buitins
>>        __builtin_cpu_is_intel_corei7 and
>>        __builtin_cpu_is_amdfam10.
>>        * testsuite/gcc.target/i386/builtin_target.c (fn1):
>>        Call the new builtins.
>>
>> Index: libgcc/config/i386/i386-cpuinfo.c
>> ===
>> --- libgcc/config/i386/i386-cpuinfo.c   (revision 182428)
>> +++ libgcc/config/i386/i386-cpuinfo.c   (working copy)
>> @@ -55,9 +55,11 @@ struct __processor_model
>>   /* CPU type. */
>>   unsigned int __cpu_is_intel_atom : 1;
>>   unsigned int __cpu_is_intel_core2 : 1;
>> +  unsigned int __cpu_is_intel_corei7 : 1;
>>   unsigned int __cpu_is_intel_corei7_nehalem : 1;
>>   unsigned int __cpu_is_intel_corei7_westmere : 1;
>>   unsigned int __cpu_is_intel_corei7_sandybridge : 1;
>> +  unsigned int __cpu_is_amdfam10 : 1;
>>   unsigned int __cpu_is_amdfam10_barcelona : 1;
>>   unsigned int __cpu_is_amdfam10_shanghai : 1;
>>   unsigned int __cpu_is_amdfam10_istanbul : 1;
>> @@ -74,12 +76,15 @@ get_amd_cpu (unsigned int family, unsigned int mod
>>       switch (model)
>>        {
>>        case 0x2:
>> +         __cpu_model.__cpu_is_amdfam10 = 1;
>>          __cpu_model.__cpu_is_amdfam10_barcelona = 1;
>>          break;
>>        case 0x4:
>> +         __cpu_model.__cpu_is_amdfam10 = 1;
>>          __cpu_model.__cpu_is_amdfam10_shanghai = 1;
>>          break;
>>        case 0x8:
>> +         __cpu_model.__cpu_is_amdfam10 = 1;
>>          __cpu_model.__cpu_is_amdfam10_istanbul = 1;
>>          break;
>>        default:
>> @@ -117,16 +122,19 @@ get_intel_cpu (unsigned int family, unsigned int m
>>            case 0x1f:
>>            case 0x2e:
>>              /* Nehalem.  */
>> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>>              __cpu_model.__cpu_is_intel_corei7_nehalem = 1;
>>              break;
>>            case 0x25:
>>            case 0x2c:
>>            case 0x2f:
>>              /* Westmere.  */
>> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>>              __cpu_model.__cpu_is_intel_corei7_westmere = 1;
>>              break;
>>            case 0x2a:
>>              /* Sandy Bridge.  */
>> +             __cpu_model.__cpu_is_intel_corei7 = 1;
>>              __cpu_model.__cpu_is_intel_corei7_sandybridge = 1;
>>              break;
>>            case 0x17:
>> Index: gcc/testsuite/gcc.target/i386/builtin_target.c
>> ===
>> --- gcc/testsuite/gcc.target/i386/builtin_target.c      (revision 182428)
>> +++ gcc/testsuite/gcc.target/i386/builtin_target.c      (working copy)
>> @@ -31,12 +31,16 @@ fn1 ()
>>     return -1;
>>   if (__builtin_cpu_is_intel_core2 () < 0)
>>     return -1;
>> +  if (__builtin_cpu_is_intel_corei7 () < 0)
>> +    return -1;
>>   if (__builtin_cpu_is_intel_corei7_nehalem () < 0)
>>     return -1;
>>   if (__builtin_cpu_is_intel_corei7_westmere () < 0)
>>     return -1;
>>   if (__builtin_cpu_is_intel_corei7_sandybridge () < 0)
>>     return -1;
>> +  if (__builtin_cpu_is_amdfam10 () < 0)
>> +    return -1;
>>   if (__builtin_cpu_is_amdfam10_barcelona () < 0)
>>     return -1;
>>   if (__builtin_cpu_is_amdfam10_shanghai () < 0)
>> Index: gcc/config/i386/i386.c
>> ===
>> --- gcc/config/i386/i386.c      (revision 182428)
>> +++ gcc/config/i386/i386.c      (working copy)
>> @@ -24500,9 +24500,11 @@ enum ix86_builtins
>>   IX86_BUILTIN_CPU_IS_INTEL,
>>   IX86_BUILTIN_CPU_IS_INTEL_ATOM,
>>   IX86_BUILTIN_CPU_IS_INTEL_CORE2,
>> +  IX86_BUILTIN_CPU_IS_INTEL_COREI7,
>>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_NEHALEM,
>>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_WESTMERE,
>>   IX86_BUILTIN_CPU_IS_INTEL_COREI7_SANDYBRIDGE,
>> +  IX86_BUILTIN_CPU_IS_AMDFAM10,
>>   IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA,
>>   IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI,
>>   IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL,
>> @@ -25981,9 +25983,11 @@ fold_builtin_cpu (enum ix86_builtins fn_code)
>>     M_INTEL,
>>     M_INTEL_ATOM,
>>     M_INTEL_CORE2,
>> +    M_INTEL_COREI7,
>>     M_INTEL_COREI7_NEHALEM,
>>     M_INTEL_COREI7_WESTMERE,
>>     M_INTEL_COREI7_SANDYBRIDGE,
>> +    M_AMDFAM10,
>>     M_AMDFAM10_BARCELONA,
>>     M

Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-17 Thread Sriraman Tallam
Committed to google 4_6 branch.

Thanks,
-Sri.

On Sat, Dec 17, 2011 at 12:25 AM, Xinliang David Li  wrote:
> ok for google branches.
>
> David
>
> On Fri, Dec 16, 2011 at 2:05 PM,   wrote:
>> I have uploaded a new patch set with all the mentioned changes made. If
>> a function has the target attribute it will not be touched by the
>> autoclone pass.
>>
>> Also fixed some test cases which were broken because the clone names
>> used '_' instead of '.' for suffixing.
>>
>>
>> On 2011/12/16 19:39:47, davidxl wrote:
>>>
>>> http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
>>> File config/i386/i386.c (right):
>>
>>
>>
>> http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569
>>>
>>> config/i386/i386.c:26569: +mversion_for_core2 (tree
>>
>> *optimization_node,
>>>
>>> -> mversionable_for_core2_p ?
>>
>>
>>> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
>>> File mversn-dispatch.c (right):
>>
>>
>>
>> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931
>>>
>>> mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
>>> Should you assert it instead? Should not clone ctor/dtors.
>>
>>
>>
>> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221
>>>
>>> mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR->preds, 0);
>>> {} --> remove
>>
>>
>>
>> http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389
>>>
>>> mversn-dispatch.c:2389:
>>> How does it interact with manual multi-versioning from user? You
>>
>> probably don't
>>>
>>> want to clone functions that are marked with target attributes
>>
>> (explicitly --
>>>
>>> not implied from command line).
>>
>>
>>
>>
>> http://codereview.appspot.com/5490054/


Re: [patch committed SH] Add atomic patterns

2011-12-17 Thread Kaz Kojima
Oleg Endo  wrote:
> The attached patch should fix the align 2 issues mentioned before and
> also fixes the ior fetchop_name.  It should be "or" instead of "ior".

You are right about that nop shouldn't be inserted after
write-back.  Thanks for pointing out my thinko.
Your patch doesn't work because SH soft atomic sequences have
another constraint that label 1 should have been 4-byte aligned.
And fetchop_name for the logical or operation uses ior instead
of or.  See genoptint.c, for example.  So fixed thusly, though
I'd like to commit it with fixing double-quote issues rth pointed
out.

Regards,
kaz
--
* config/sh/sync.md (atomic_compare_and_swap_soft):
Don't insert nop after the write-back instruction.
(atomic_fetch__soft): Likewise.
(atomic_fetch_nand_soft): Likewise.
(atomic__fetch_soft): Likewise.
(atomic_nand_fetch_soft): Likewise.

--- ORIG/trunk/gcc/config/sh/sync.md2011-12-05 10:04:44.0 +0900
+++ trunk/gcc/config/sh/sync.md 2011-12-18 07:21:56.0 +0900
@@ -88,7 +88,8 @@
   "*
 {
   return \"\\
-mova\\t1f, r0\\n\\
+.align\\t2\\n\\
+\\tmova\\t1f, r0\\n\\
 \\t\\t%2, %4\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
@@ -96,7 +97,6 @@ mova\\t1f, r0\\n\\
 \\tcmp/eq\\t%0, %4\\n\\
 \\tbf\\t1f\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "20")])
@@ -141,17 +141,18 @@ mova\\t1f, r0\\n\\
   "*
 {
   return \"\\
-mova\\t1f, r0\\n\\
+.align\\t2\\n\\
+\\tmova\\t1f, r0\\n\\
+\\tnop\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tmov\\t%0, %3\\n\\
 \\t\\t%2, %3\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
-  [(set_attr "length" "18")])
+  [(set_attr "length" "20")])
 
 (define_expand "atomic_fetch_nand"
   [(set (match_operand:I124 0 "register_operand" "")
@@ -193,7 +194,8 @@ mova\\t1f, r0\\n\\
   "*
 {
   return \"\\
-mova\\t1f, r0\\n\\
+.align\\t2\\n\\
+\\tmova\\t1f, r0\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
@@ -201,7 +203,6 @@ mova\\t1f, r0\\n\\
 \\tand\\t%0, %3\\n\\
 \\tnot\\t%3, %3\\n\\
 \\tmov.\\t%3, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "20")])
@@ -247,13 +248,13 @@ mova\\t1f, r0\\n\\
   "*
 {
   return \"\\
-mova\\t1f, r0\\n\\
+.align\\t2\\n\\
+\\tmova\\t1f, r0\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\t\\t%2, %0\\n\\
 \\tmov.\\t%0, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
   [(set_attr "length" "16")])
@@ -299,14 +300,15 @@ mova\\t1f, r0\\n\\
   "*
 {
   return \"\\
-mova\\t1f, r0\\n\\
+.align\\t2\\n\\
+\\tmova\\t1f, r0\\n\\
+\\tnop\\n\\
 \\tmov\\tr15, r1\\n\\
 \\tmov\\t#(0f-1f), r15\\n\\
 0:\\tmov.\\t@%1, %0\\n\\
 \\tand\\t%2, %0\\n\\
 \\tnot\\t%0, %0\\n\\
 \\tmov.\\t%0, @%1\\n\\
-\\t.align\\t2\\n\\
 1:\\tmov\tr1, r15\";
 }"
-  [(set_attr "length" "18")])
+  [(set_attr "length" "20")])


C++ PATCH for c++/51588 (ICE with invalid pointer-to-member)

2011-12-17 Thread Jason Merrill
In C++11 an enum can be used in a nested-name-specifier, but it still 
isn't valid for a pointer-to-member.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit 80c46fcfb6ea29a6566b285cf94e519ec414d2e9
Author: Jason Merrill 
Date:   Sat Dec 17 15:39:56 2011 -0500

	PR c++/51588
	* parser.c (cp_parser_ptr_operator): Reject pointer to member of enum.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ac7427e..51d04d4 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -16448,6 +16448,9 @@ cp_parser_ptr_operator (cp_parser* parser,
 
 	  if (TREE_CODE (parser->scope) == NAMESPACE_DECL)
 	error_at (token->location, "%qD is a namespace", parser->scope);
+	  else if (TREE_CODE (parser->scope) == ENUMERAL_TYPE)
+	error_at (token->location, "cannot form pointer to member of "
+		  "non-class %q#T", parser->scope);
 	  else
 	{
 	  /* The type of which the member is a member is given by the
diff --git a/gcc/testsuite/g++.dg/parse/enum7.C b/gcc/testsuite/g++.dg/parse/enum7.C
new file mode 100644
index 000..d9e3a89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/enum7.C
@@ -0,0 +1,9 @@
+// PR c++/51588
+
+enum A {};
+
+struct B : A {			// { dg-error "" }
+  int i;
+};
+
+int A::* p = &B::i;		// { dg-error "" }


[v3] fix typos in docs

2011-12-17 Thread Jonathan Wakely
* doc/xml/manual/iterators.xml: Replace "sect1" with "section".
* doc/xml/manual/algorithms.xml: Likewise.
* doc/html/manual/iterators.html: Likewise.
* doc/html/manual/algorithms.html: Likewise.

Fixes some errors probably introduced by s/section/sect1/

Rather than regenerate all the HTML pages I just fixed them manually.

Committed to trunk.
Index: doc/xml/manual/iterators.xml
===
--- doc/xml/manual/iterators.xml(revision 182452)
+++ doc/xml/manual/iterators.xml(working copy)
@@ -41,7 +41,7 @@ classes.
  that pointers are
   iterators, and that pointers can be used
  whenever an iterator would be.  All those functions in the
- Algorithms sect1 of the Standard will work just as well on plain
+ Algorithms section of the Standard will work just as well on plain
  arrays and their pointers.


Index: doc/xml/manual/algorithms.xml
===
--- doc/xml/manual/algorithms.xml   (revision 182452)
+++ doc/xml/manual/algorithms.xml   (working copy)
@@ -22,7 +22,7 @@
 
 
 
-  The neatest accomplishment of the algorithms sect1 is that all the
+  The neatest accomplishment of the algorithms section is that all the
   work is done via iterators, not containers directly.  This means two
   important things:
 
@@ -53,14 +53,14 @@
   N as a size in the examples is to keep things
   easy to read but probably won't be valid code.  You can use wrappers
   such as those described in
-  the containers sect1 to keep
+  the containers section to keep
   real code readable.
 
 
   The single thing that trips people up the most is the definition
   of range used with iterators; the famous
   "past-the-end" rule that everybody loves to hate.  The
-  iterators sect1 of this
+  iterators section of this
 document has a complete explanation of this simple rule that seems
 to cause so much confusion.  Once you
 get range into your head (it's not that hard,
Index: doc/html/manual/iterators.html
===
--- doc/html/manual/iterators.html  (revision 182452)
+++ doc/html/manual/iterators.html  (working copy)
@@ -23,7 +23,7 @@ classes.
  that pointers are
   iterators, and that pointers can 
be used
  whenever an iterator would be.  All those functions in the
- Algorithms sect1 of the Standard will work just as well on plain
+ Algorithms section of the Standard will work just as well on plain
  arrays and their pointers.

  That doesn't mean that when you pass in a pointer, it gets
Index: doc/html/manual/algorithms.html
===
--- doc/html/manual/algorithms.html (revision 182452)
+++ doc/html/manual/algorithms.html (working copy)
@@ -9,7 +9,7 @@
   Algorithms
   
 Table of 
ContentsMutatingswapSpecializations
-  The neatest accomplishment of the algorithms sect1 is that all the
+  The neatest accomplishment of the algorithms section is that all the
   work is done via iterators, not containers directly.  This means two
   important things:
 
@@ -31,13 +31,13 @@
   N as a size in the examples is to 
keep things
   easy to read but probably won't be valid code.  You can use wrappers
   such as those described in
-  the containers sect1 to keep
+  the containers section to keep
   real code readable.
 
   The single thing that trips people up the most is the definition
   of range used with iterators; the 
famous
   "past-the-end" rule that everybody loves to hate.  The
-  iterators sect1 of this
+  iterators section of this
 document has a complete explanation of this simple rule that seems
 to cause so much confusion.  Once you
 get range into your head (it's not 
that hard,


[PATCH SMS 2/2, RFC] Register pressure estimation for the partial schedule (re-submission)

2011-12-17 Thread Revital Eres
Hello,

The attached patch is a resubmission following comments made by Ayal
and Richard.

Tested and bootstrap with the other patches in the series on
ppc64-redhat-linux, enabling SMS on loops with SC 1.

Comments are welcome.

Thanks,
Revital


   2011-12-18  Richard Sandiford  
Revital Eres  

* loop-invariant.c (get_regno_pressure_class): Move function to...
* ira.c: Here.
* common.opt (fmodulo-sched-reg-pressure, -fmodulo-sched-verbose):
New flags.
* doc/invoke.texi (fmodulo-sched-reg-pressure,
-fmodulo-sched-verbose): Document the flags.
* ira.h (get_regno_pressure_class,
reset_pseudo_classes_defined_p): Declare.
* ira-costs.c (reset_pseudo_classes_defined_p): New function.
* Makefile.in (modulo-sched.o): Include ira.h and modulo-sched.h.
(modulo-sched-pressure.o): New.
* modulo-sched.c (ira.h, modulo-sched.h): New includes.
(partial_schedule_ptr, ps_insn_ptr, struct ps_insn,
struct ps_reg_move_info, struct partial_schedule): Move to
modulo-sched.h.
(ps_rtl_insn, ps_reg_move): Remove static.
(apply_reg_moves): Remove static and call df_insn_rescan only
if PS is final.
(undo_reg_moves): New function.
(sms_schedule): Call register pressure estimation.
* modulo-sched.h: New file.
* modulo-sched-pressure.c: New file.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 182357)
+++ doc/invoke.texi (working copy)
@@ -373,6 +373,7 @@ Objective-C and Objective-C++ Dialects}.
 -floop-parallelize-all -flto -flto-compression-level @gol
 -flto-partition=@var{alg} -flto-report -fmerge-all-constants @gol
 -fmerge-constants -fmodulo-sched -fmodulo-sched-allow-regmoves @gol
+-fmodulo-sched-reg-pressure -fmodulo-sched-verbose=@var{n} @gol
 -fmove-loop-invariants fmudflap -fmudflapir -fmudflapth -fno-branch-count-reg 
@gol
 -fno-default-inline @gol
 -fno-defer-pop -fno-function-cse -fno-guess-branch-probability @gol
@@ -6474,6 +6475,16 @@ deleted which will trigger the generatio
 life-range analysis.  This option is effective only with
 @option{-fmodulo-sched} enabled.
 
+@item -fmodulo-sched-reg-pressure
+@opindex fmodulo-sched-reg-pressure
+Do not apply @option{-fmodulo-sched} to loops if the result would lead
+to register spilling within the loop.
+This option is effective only with @option{-fmodulo-sched} enabled.
+
+@item -fmodulo-sched-verbose=@var{n}
+@opindex fmodulo-sched-verbose
+Set up how verbose dump file for the SMS will be.  
+
 @item -fno-branch-count-reg
 @opindex fno-branch-count-reg
 Do not use ``decrement and branch'' instructions on a count register,
Index: modulo-sched.h
===
--- modulo-sched.h  (revision 0)
+++ modulo-sched.h  (revision 0)
@@ -0,0 +1,120 @@
+/* Swing Modulo Scheduling implementation.
+   Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 
+   Free Software Foundation, Inc.
+   Contributed by Revital Eres  
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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 GCC; see the file COPYING3.  If not see
+.  */
+
+#ifndef GCC_SMS_H
+#define GCC_SMS_H
+
+#include "ddg.h"
+
+extern HARD_REG_SET eliminable_regset;
+
+typedef struct partial_schedule *partial_schedule_ptr;
+
+typedef struct ps_insn *ps_insn_ptr;
+
+/* A single instruction in the partial schedule.  */
+struct ps_insn
+{
+  /* Identifies the instruction to be scheduled.  Values smaller than
+ the ddg's num_nodes refer directly to ddg nodes.  A value of
+ X - num_nodes refers to register move X.  */
+  int id;
+
+  /* The (absolute) cycle in which the PS instruction is scheduled.
+ Same as SCHED_TIME (node).  */
+  int cycle;
+
+  /* The next/prev PS_INSN in the same row.  */
+  ps_insn_ptr next_in_row,
+ prev_in_row;
+
+};
+
+/* Information about a register move that has been added to a partial
+   schedule.  */
+struct ps_reg_move_info
+{
+  /* The source of the move is defined by the ps_insn with id DEF.
+ The destination is used by the ps_insns with the ids in USES.  */
+  int def;
+  sbitmap uses;
+
+  /* The original form of USES' instructions used OLD_REG, but they
+ should now use NEW_REG.  */
+  rtx old_reg;
+  rtx new_reg;
+
+  /* The number of consecutive stages that the move occupies.  */
+  int num_consecutive_

Re: [PATCH] PowerPC section type conflict

2011-12-17 Thread Chung-Lin Tang
On 2011/12/17 06:21 AM, Richard Henderson wrote:
> On 12/16/2011 03:16 AM, Chung-Lin Tang wrote:
>> Hi, under powerpc targets, using -mrelocatable under some cases triggers
>> an error during final assembly generation: rs6000_assemble_integer()
>> calls varasm.c:unlikely_text_section_p(), and the call chain eventually
>> gets to where the section htab is queried for ".text.unlikely", and
>> fails because of mismatched section flags: "error: foo causes a section
>> type conflict"
>>
>> This seems to happen after rev.167085, 4.6 branch is also affected.
>>
>> The flag mismatch seems quite straightforward: current_function_decl is
>> passed in as the decl here, and as it's final assembly now, it is
>> NULL_TREE. varasm.c:default_section_type_flags() adds SECTION_WRITE to
>> its return value, and things get borked.
> 
> I don't understand what was put into .text.unlikely that was not a
> function in the first place?  Did we try to put data there somewhere?

I don't think it's that kind of problem; the powerpc backend uses
unlikely_text_section_p(), which compares the passed in argument section
and the value of function_section_1(current_function_decl,true).

Since current_function_decl is NULL at assembly phase, it retrieves
".text.unlikely" to test for equality. It's the retrieving/lookup that
fails here, because the default looked-up section flags set when decl ==
NULL does not really seem to make sense (adds SECTION_WRITE).

Chung-Lin


Re: [PATCH SMS 1/2, RFC] Support traversing PS in reverse order

2011-12-17 Thread Revital Eres
Hello,

>> This patch support the estimation of register pressure in SMS.
>> Although GCC is in stage 3 I would appreciate comments on it.
>> Thanks to Richard and Ayal for discussing the implementation and their 
>> insights.
>>
>> This part of the patch enables iterating on the partial schedule in the
>> reverse order (from the last instruction the the first).
>>
>> Tested and bootstrap with the other patches in the series on
>> ppc64-redhat-linux,
>> enabling SMS on loops with SC 1.
>>
>> Comments are welcome.
>>
>
> This looks fine. Please rename rows_reverse to rows_last as discussed,
> and simplify the bit that tracks last_in_row in ps_insn_find_column().
> Thanks,
> Ayal.
>

Thanks, attached is the new version of the patch following your comments.

Tested and bootstrap with the other patches in the series on
ppc64-redhat-linux, enabling SMS on loops with SC 1.

Revital

* modulo-sched.c (rows_last): New field in struct partial_schedule.
(create_partial_schedule, free_partial_schedule,
ps_insert_empty_row, ps_insn_advance_column, remove_node_from_ps,
reset_partial_schedule, rotate_partial_schedule,
verify_partial_schedule): Update the new field.
(ps_insn_find_column): Likewise and remove last_in_row.
Index: modulo-sched.c
===
--- modulo-sched.c  (revision 181763)
+++ modulo-sched.c  (working copy)
@@ -177,6 +177,10 @@ struct partial_schedule
   /* rows[i] points to linked list of insns scheduled in row i (0<=inum_nodes.  */
   VEC (ps_reg_move_info, heap) *reg_moves;
@@ -2272,6 +2276,7 @@ ps_insert_empty_row (partial_schedule_pt
 {
   ps_insn_ptr crr_insn;
   ps_insn_ptr *rows_new;
+  ps_insn_ptr *rows_last_new;
   int ii = ps->ii;
   int new_ii = ii + 1;
   int row;
@@ -2290,10 +2295,12 @@ ps_insert_empty_row (partial_schedule_pt
   rotate_partial_schedule (ps, PS_MIN_CYCLE (ps));
 
   rows_new = (ps_insn_ptr *) xcalloc (new_ii, sizeof (ps_insn_ptr));
+  rows_last_new = (ps_insn_ptr *) xcalloc (new_ii, sizeof (ps_insn_ptr));
   rows_length_new = (int *) xcalloc (new_ii, sizeof (int));
   for (row = 0; row < split_row; row++)
 {
   rows_new[row] = ps->rows[row];
+  rows_last_new[row] = ps->rows_last[row];
   rows_length_new[row] = ps->rows_length[row];
   ps->rows[row] = NULL;
   for (crr_insn = rows_new[row];
@@ -2315,6 +2322,7 @@ ps_insert_empty_row (partial_schedule_pt
   for (row = split_row; row < ii; row++)
 {
   rows_new[row + 1] = ps->rows[row];
+  rows_last_new[row + 1] = ps->rows_last[row];
   rows_length_new[row + 1] = ps->rows_length[row];
   ps->rows[row] = NULL;
   for (crr_insn = rows_new[row + 1];
@@ -2337,6 +2345,8 @@ ps_insert_empty_row (partial_schedule_pt
 + (SMODULO (ps->max_cycle, ii) >= split_row ? 1 : 0);
   free (ps->rows);
   ps->rows = rows_new;
+  free (ps->rows_last);
+  ps->rows_last = rows_last_new;
   free (ps->rows_length);
   ps->rows_length = rows_length_new;
   ps->ii = new_ii;
@@ -2428,6 +2438,9 @@ verify_partial_schedule (partial_schedul
 popcount (sched_nodes) == number of insns in ps.  */
  gcc_assert (SCHED_TIME (u) >= ps->min_cycle);
  gcc_assert (SCHED_TIME (u) <= ps->max_cycle);
+ if (ps->rows_length[row] == length)
+   gcc_assert (ps->rows_last[row] == crr_insn);
+
}
   
   gcc_assert (ps->rows_length[row] == length);
@@ -2837,6 +2850,7 @@ create_partial_schedule (int ii, ddg_ptr
 {
   partial_schedule_ptr ps = XNEW (struct partial_schedule);
   ps->rows = (ps_insn_ptr *) xcalloc (ii, sizeof (ps_insn_ptr));
+  ps->rows_last = (ps_insn_ptr *) xcalloc (ii, sizeof (ps_insn_ptr));
   ps->rows_length = (int *) xcalloc (ii, sizeof (int));
   ps->reg_moves = NULL;
   ps->ii = ii;
@@ -2885,6 +2899,7 @@ free_partial_schedule (partial_schedule_
 
   free_ps_insns (ps);
   free (ps->rows);
+  free (ps->rows_last);
   free (ps->rows_length);
   free (ps);
 }
@@ -2903,6 +2918,9 @@ reset_partial_schedule (partial_schedule
   ps->rows = (ps_insn_ptr *) xrealloc (ps->rows, new_ii
 * sizeof (ps_insn_ptr));
   memset (ps->rows, 0, new_ii * sizeof (ps_insn_ptr));
+  ps->rows_last = (ps_insn_ptr *) xrealloc (ps->rows_last, new_ii
+  * sizeof (ps_insn_ptr));
+  memset (ps->rows_last, 0, new_ii * sizeof (ps_insn_ptr));
   ps->rows_length = (int *) xrealloc (ps->rows_length, new_ii * sizeof (int));
   memset (ps->rows_length, 0, new_ii * sizeof (int));
   ps->ii = new_ii;
@@ -2960,6 +2978,10 @@ remove_node_from_ps (partial_schedule_pt
   gcc_assert (ps && ps_i);
   
   row = SMODULO (ps_i->cycle, ps->ii);
+
+  if (! ps_i->next_in_row)
+ps->rows_last[row] = ps_i->prev_in_row;
+  
   if (! ps_i->prev_in_row)
 {
   gcc_assert (ps_i == ps->rows[row]);
@@ -2992,7 +3014,6 @@ ps_insn_find_column (partial_schedule_pt
   ps_insn_ptr next_ps_i;
   ps_insn_ptr

Re: [Google Debugfission] Add pubnames and pubtypes to google/main (issue5489074)

2011-12-17 Thread Cary Coutant
> 2011-12-17   Sterling Augustine  
>
>        * gcc/dwarf2out.c (is_cu_die, is_namespace_die, is_class_die): New
>        functions.
>        (add_pubname): Call is_namespace_die, is_cu_die, and is_class_die in
>        conditional.
>        (add_enumerator_pubname): New function.
>        (add_pubtype): Call is_namespace_die. Rework name calculation.  Call
>        type_tag, lang_hooks.dwarf_name and add_enumerator_pubname.
>        (output_pubnames): Output debug_pubnames_section_label or
>        debug_pubtypes_section_label.
>        (base_type_die): Call add_pubtype.
>        (gen_namespace_die): Call add_pubname_string and lang_hooks.dwarf_name.
>        (dwarf2out_init): Generate debug_pubnames_section_label and
>        debug_pubtypes_section_label.
>        (dwarf2out_finish): Call add_AT_lineptr if pubnames or pubtypes is
>        non-empty.  When dealing with pubnames, change assertion to 
> conditional.
>        Call pubtypes_section_empty.  Likewise when dealing with pubtypes.
>        Move code checking for empty section to...
>        (pubtypes_section_empty): ...here. New function.
>        * gcc/target.def: Switch default generate pubnames and types to true.
>
> cp-family/ChangeLog:
> 2011-12-17   Sterling Augustine  
>
>        * gcc/c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Move
>        conditional from beginning to end.
>
> cp/ChangeLog
> 2011-12-17   Sterling Augustine  
>
>        * gcc/cp/error.c (dump_decl): Reformat return value to
>        "(anonymous namespace)".
>        (lang_decl_name): Return "(anonymous namespace)" when appropriate.
>
> include/ChangeLog
>        * include/dwarf2.h (enum dwarf_form): Add forms DW_FORM_GNU_ref_index,
>        DW_FORM_GNU_addr_index and DW_FORM_GNU_str_index.
>        (enum dwarf_attribute): Add attributes: Add DW_AT_GNU_dwo_name,
>        DW_AT_GNU_dwo_id, DW_AT_GNU_ref_base, DW_AT_GNU_addr_base,
>        DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes.

This is OK for google/main and google/gcc-4_6.

-cary