Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

Hi

Jason Merrill  ha scritto:

>Probably the same reason that subtle changes in the testcase changed
>whether the bug appeared on x86_64-linux.  I guess we should figure
>that
>out instead of just saying "hunh, that's odd."

For sure. Let's see if I can in a reasonable amount of time on x86_64-linux. At 
some point when I saw that comment in the code I thought the reason was already 
obvious to somebody, not to me but to somebody.

Paolo

Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)

[Paolo Carlini]

Hi,

>Yes, that is intended.  Changing that could mean that the meaning of
>code depends on what max depth the user selected.

Indeed. Yesterday I wondered what would happen if the front-end had a way to 
detect, in some very specific and special cases only of course, really infinite 
recursions, in the sense that no increase in the depth would "cure" them. Would 
it be ok in that case to sfinae away?

Paolo

Re: [Patch] Whole regex refactoring and current status

[Tim Shen]

On Fri, Aug 9, 2013 at 12:51 AM, Tim Shen  wrote:
> So here's the change. It's under testing now, but could took several
> hours. If someone has a faster machine, please tell the result :)

Unfortuantely using `unsigned int => unsigned short` cannot work.
Instead, I create a enum and do some operator overloadings.

This patch passed bootstrap and all tests under the configuration under x86_64:

--with-arch=corei7 --with-cpu=corei7 --prefix=/usr/local
--enable-clocale=gnu --with-system-zlib --enable-shared
--with-demangler-in-ld --with-fpmath=sse --enable-languages=c++


-- 
Tim Shen


a.patch
Description: Binary data

Re: [FIXED] Generic lambda symbol table bug

[Jason Merrill]

On 08/08/2013 06:28 PM, Adam Butcher wrote:

So all seems to be okay with both versions.  Any ideas why?


Hmm, it sounds like processing_template_decl is being set after all, 
even without your change.


Jason

Re: [C++ Patch / RFC] PR 46206

[David Edelsohn]

On Thu, Aug 8, 2013 at 8:24 PM, Jason Merrill  wrote:
> On 08/08/2013 02:31 PM, Paolo Carlini wrote:
>>
>> On 08/08/2013 08:18 PM, David Edelsohn wrote:
>>>
>>> Why does the patch and fix have any architecture or OS-dependency?
>
>
> Probably the same reason that subtle changes in the testcase changed whether
> the bug appeared on x86_64-linux.  I guess we should figure that out instead
> of just saying "hunh, that's odd."

There's an AIX system in the GCC Compile Farm and I also can provide
additional dump file output if you tell me what would be helpful.

Thanks, David

Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv

[Mike Stump]

On Aug 8, 2013, at 3:34 PM, Caroline Tice  wrote:
> On Thu, Aug 8, 2013 at 3:26 PM, Mike Stump  wrote:
>> On Aug 8, 2013, at 3:09 PM, Caroline Tice  wrote:
>>> This patch changes where the logging file mechanism in libvtv tries to
>>> write its log files.  Instead of trying to use /tmp, it now first
>>> looks for an environment variable "VTV_LOGS_DIR".  If it can't find
>>> that it looks for the environment variable "HOME".
>> 
>> Hum…  I kinda don't think HOME should be used.
> 
> Why not?

Cause it violates a rather nice standard that spans decades for no compelling 
reason.  http://en.wikipedia.org/wiki/Home_directory  In the longer run, if 
linux/posix standardizes per-program logs for the user, then it can be squirted 
out there.  On darwin for example, it would be ~/Library/Logs/.  These 
sorts of locations can feature things like, automated reporting, cleaning, 
compressing, rotation and so on…  $HOME does not.  Also, the user might have 
his files, with those names, and you would not want to just stomp on them.

Re: [vtv] fix default configure

[Benjamin De Kosnik]

More patches to fix disable issus on non-linux.

> Here's a patch for the build failure on darwin.

Will check in when testing completes.

tested x86/linux
tested x86_64/darwin12

-benjamin2013-08-08  Benjamin Kosnik  
	Michael Meissner 

	* configure.tgt : Simplify, just use VTV_SUPPORTED.


diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt
index a84ed27..801d2f0 100644
--- a/libvtv/configure.tgt
+++ b/libvtv/configure.tgt
@@ -19,6 +19,7 @@
 # lets us skip running autoconf when modifying target specific information.
 
 # Filter out unsupported systems.
+VTV_SUPPORTED=no
 case "${target}" in
   x86_64-*-linux* | i?86-*-linux*)
 	VTV_SUPPORTED=yes
@@ -30,9 +31,7 @@ case "${target}" in
   arm*-*-linux*)
 	;;
   x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
-	VTV_SUPPORTED=no
 	;;
   *)
-	UNSUPPORTED=1
 	;;
 esac
2013-08-02  Benjamin Kosnik  

	* configure.ac: Adjust to check VTV_SUPPORTED.
	* configure: Regenerated.

diff --git a/configure.ac b/configure.ac
index bcbc95c..6f3d801 100644
--- a/configure.ac
+++ b/configure.ac
@@ -561,7 +561,7 @@ if test -d ${srcdir}/libvtv; then
 	AC_MSG_CHECKING([for libvtv support])
 	if (srcdir=${srcdir}/libvtv; \
 		. ${srcdir}/configure.tgt; \
-		test -n "$UNSUPPORTED")
+		test "$VTV_SUPPORTED" != "yes")
 	then
 	AC_MSG_RESULT([no])
 	noconfigdirs="$noconfigdirs target-libvtv"

Re: [C++ Patch / RFC] PR 46206

[Jason Merrill]

On 08/08/2013 02:31 PM, Paolo Carlini wrote:

On 08/08/2013 08:18 PM, David Edelsohn wrote:

Why does the patch and fix have any architecture or OS-dependency?


Probably the same reason that subtle changes in the testcase changed 
whether the bug appeared on x86_64-linux.  I guess we should figure that 
out instead of just saying "hunh, that's odd."


Jason

Re: [PATCH] Fix expansion issues on type changing MEM_REFs on LHS (PR middle-end/48335)

[Mike Stump]

On Mar 30, 2011, at 9:05 AM, Jakub Jelinek  wrote:
> +   else if (bitpos >= mode_bitsize / 2)
> + result = store_field (XEXP (to_rtx, 1), bitsize,
> +   bitpos - mode_bitsize / 2, mode1, from,
> +   TREE_TYPE (tem), get_alias_set (to),
> +   nontemporal);

> -   gcc_assert (bitpos == 0 || bitpos == GET_MODE_BITSIZE (mode1));
> -   result = store_expr (from, XEXP (to_rtx, bitpos != 0), false,
> -nontemporal);
> +   rtx temp = assign_stack_temp (GET_MODE (to_rtx),
> + GET_MODE_SIZE (GET_MODE (to_rtx)),
> + 0);
> +   write_complex_part (temp, XEXP (to_rtx, 0), false);
> +   write_complex_part (temp, XEXP (to_rtx, 1), true);
> +   result = store_field (temp, bitsize, bitpos, mode1, from,
> + TREE_TYPE (tem), get_alias_set (to),
> + nontemporal);
> +   emit_move_insn (XEXP (to_rtx, 0), read_complex_part (temp, 
> false));
> +   emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp, true));

I think this is bad.  I think this patch fixes it.  The problem is that we 
can't write outside the bounds of the object.  So, instead we punt out of the 
register case, and instead spill it to memory that _is_ big enough, and then 
spill it back to registers.  Of course, I'd rather emit a diagnostic when extra 
is non-zero…  but not sure people yet buy into that around here.

Thoughts?

diff --git a/gcc/expr.c b/gcc/expr.c
index 923f59b..f5744b0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -4815,7 +4815,8 @@ expand_assignment (tree to, tree from, bool nontemporal)
  bitregion_start, bitregion_end,
  mode1, from,
  get_alias_set (to), nontemporal);
- else if (bitpos >= mode_bitsize / 2)
+ else if (bitpos >= mode_bitsize / 2
+  && bitpos+bitsize <= mode_bitsize)
result = store_field (XEXP (to_rtx, 1), bitsize,
  bitpos - mode_bitsize / 2,
  bitregion_start, bitregion_end,
@@ -4834,8 +4835,12 @@ expand_assignment (tree to, tree from, bool nontemporal)
}
  else
{
+ HOST_WIDE_INT extra = 0;
+ if (bitpos+bitsize > mode_bitsize)
+   extra = bitpos+bitsize - mode_bitsize;
  rtx temp = assign_stack_temp (GET_MODE (to_rtx),
-   GET_MODE_SIZE (GET_MODE (to_rtx)));
+   GET_MODE_SIZE (GET_MODE (to_rtx))
+   + extra);
  write_complex_part (temp, XEXP (to_rtx, 0), false);
  write_complex_part (temp, XEXP (to_rtx, 1), true);
  result = store_field (temp, bitsize, bitpos,

Re: [PATCH 3/5] Atomic type qualifier - front end changes

[Joseph S. Myers]

Observations on this patch:

* build_qualified_type sets the qualifiers to exactly the set specified.  
Thus, it looks like your handle_atomic_attribute will remove existing 
qualifiers when adding "atomic".

* c-aux-info.c is meant to be generating actual valid C declarations, I 
think, meaning _Atomic rather than "atomic".

* The code you have checking for _Atomic with function declarators appears 
to be in the wrong place.  What you're testing is the combination of 
(_Atomic appears in declaration specifiers, as given by atomicp) and (some 
declarator in the nested sequence of declarators is a function 
declarator).  But there are valid cases for this - for example a function 
returning a pointer to atomic.  And there are invalid cases involving an 
atomic-qualified function type that you don't catch there - for example, 
if _Atomic is applied to a typedef for a function type rather than 
together with the declarator.

The relevant issue is whether the *function type* itself gets qualified.  
There are already pedwarns-if-pedantic for this, given that there's 
undefined behavior in ISO C for qualifiers on function types in general, 
but making this case a hard error seems reasonable enough.  To do that, 
you need to adjust the cases

case cdk_pointer:
  {
/* Merge any constancy or volatility into the target type
   for the pointer.  */

if (pedantic && TREE_CODE (type) == FUNCTION_TYPE
&& type_quals)
  pedwarn (loc, OPT_Wpedantic,
   "ISO C forbids qualified function types");

and similar for typedefs, type names, parameters and actual function 
declarations, to give such errors.  And similarly, for all the various 
cases of things that can be declared, whenever qualifiers get applied in 
grokdeclarator you need to ensure an error is given if the type is an 
array type, since that is also a constraint violation in ISO C.

* The pedwarn for _Atomic outside C11 mode should I think only be a 
pedwarn-if-pedantic, so pass OPT_Wpedantic instead of 0.  And the text 
should match other such pedwarns (i.e. "ISO C90 does not support _Atomic" 
or "ISO C99 does not support _Atomic", depending on the standard version 
selected).

* The parser comments with syntax should refer to _Atomic not "atomic".

* You change convert_for_assignment regarding discarding qualifiers.  
Doing so is correct as far as it goes - not because of the reason in your 
comment, but because in C11 terms _Atomic isn't a type qualifier.  But I 
don't think the change is enough.  Because it's inside a conditional 
checking for allowed cases: either side being a pointer to a void type, or 
the target types being compatible.  Now the rule in 6.5.16.1#1 refers to 
"qualified or unqualified version of void", which in ISO C terms does not 
include _Atomic void, so those checks need to disallow _Atomic void.  And 
similarly, it's not enough for comp_target_types to match the unqualified 
types when it also needs to match whether _Atomic is specified (for that, 
comp_target_types should probably be changed - conditional expressions 
have the same issue).

The above relate specifically to what's in the patch.  The listed issues 
should of course have testcases added.  I still need to review it on the 
basis of reviews of C11 for references to qualifiers or _Atomic, to see 
what might be missing from the patch, and on the basis of reviews of the C 
front end for handling of qualifiers, to see what places might need to 
handle _Atomic specially; I'll do those reviews separately.  As previously 
noted, one thing missing is the _Atomic ( type-name ) syntax.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

Which version gets used depends on their ordering in the link line.
When -fvtable-verify=std or -fvtable-verify=preinit is used, the gcc
driver inserts -lvtv very early into the link line (earlier than
-lstdc++), so this happens "automatically".

-- Caroline
cmt...@google.com


On Thu, Aug 8, 2013 at 4:33 PM, Jason Merrill  wrote:
> On 08/08/2013 06:34 PM, Caroline Tice wrote:
>>
>> Actually if you ever want to use the feature with your compiler you
>> should build your compiler with --enable-vtable-verify.  This will, as
>> you noted,  insert calls in libstdc++ to build the verification data
>> structures and to verify the virtual calls in libstdc++ itself.
>> However libstdc++ itself contains 'stub' (do-nothing) versions of the
>> functions that build the data structures and perform the verification.
>
>
> How do you ensure that the libvtv versions get used rather than the stubs in
> libstdc++?
>
> Jason
>

Re: [PATCH] Fix expansion issues on type changing MEM_REFs on LHS (PR middle-end/48335)

[Mike Stump]

On Mar 30, 2011, at 9:05 AM, Jakub Jelinek  wrote:
> MEM_REFs which can represent type punning on lhs don't force
> non-gimple types to be addressable.  This causes various problems
> in the expander, which wasn't prepared to handle that.
> 
> This patch tries to fix what I've found and adds a bunch of
> testcases.  The original report was with just -O2 on some large testcase
> from Eigen, most of the testcases have -fno-tree-sra just because
> I've given up on delta when it stopped reducing at 32KB.
> 
> The first problem (the one from Eigen) is _mm_store_pd into
> a std::complex var, which is a single field and thus
> has DCmode TYPE_MODE.  As starting with 4.6 that var is not
> TREE_ADDRESSABLE, its DECL_RTL is a CONCAT, and for assignments
> to concat expand_assignment was expecting either that
> from has COMPLEX_TYPE (and matching mode to the store), or
> that it is a real or imaginary subpart store, thus when
> trying to store a V2DF mode value it expected it to be
> real part store (bitpos == 0) and tried to set a DFmode pseudo
> from V2DFmode rtx.
> Further testing revealed that it is possible to hit many other
> cases with CONCAT destination, it can be a store of just a few bits,
> or can overlap parts of both real and imaginary, or be partially
> out of bounds.
> The patch handles the case from Eigen - bitpos == 0 bitsize ==
> GET_MODE_BITSIZE (GET_MODE (to_rtx)) non-COMPLEX_TYPE by setting
> each half separately, if it is a store which is not touching
> one of the parts by just adjusting bitpos for the imaginary
> case and storing just to one of the parts (this is
> the bitpos + bitsize < half_bitsize resp. bitpos >= half_bitsize
> case) and finally adds a generic slow one for the very unusual
> cases with partial overlap of both.
> 
> After testing it with the testcases I wrote, I found a bunch of
> other ICEs though, and reproduced them even without CONCAT
> on the LHS (the testcases below which don't contain any _Complex
> keyword).

> --- gcc/testsuite/gcc.dg/pr48335-2.c.jj   2011-03-30 10:57:29.0 
> +0200
> +++ gcc/testsuite/gcc.dg/pr48335-2.c  2011-03-29 18:27:53.0 +0200
> @@ -0,0 +1,58 @@
> +/* PR middle-end/48335 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-sra" } */
> +
> +typedef long long T __attribute__((may_alias, aligned (1)));
> +typedef short U __attribute__((may_alias, aligned (1)));
> +
> +struct S
> +{
> +  _Complex float d __attribute__((aligned (8)));
> +};
> +
> +void bar (struct S);
> +
> +void
> +f1 (T x)
> +{
> +  struct S s;
> +  *(T *) ((char *) &s.d + 1) = x;
> +  __real__ s.d *= 7.0;
> +  bar (s);
> +}
> +
> +void
> +f2 (int x)
> +{
> +  struct S s = { .d = 0.0f };
> +  ((U *)((char *) &s.d + 1))[0] = x;
> +  __real__ s.d *= 7.0;
> +  bar (s);
> +}
> +
> +void
> +f3 (int x)
> +{
> +  struct S s = { .d = 0.0f };
> +  ((U *)((char *) &s.d + 1))[1] = x;
> +  __real__ s.d *= 7.0;
> +  bar (s);
> +}
> +
> +void
> +f4 (int x)
> +{
> +  struct S s = { .d = 0.0f };
> +  ((U *)((char *) &s.d + 1))[2] = x;
> +  __real__ s.d *= 7.0;
> +  bar (s);
> +}
> +
> +void
> +f5 (int x)
> +{
> +  struct S s = { .d = 0.0f };
> +  ((U *)((char *) &s.d + 1))[3] = x;
> +  __real__ s.d *= 7.0;
> +  bar (s);
> +}

So, this test case writes outside the bounds of s.  I think it is a nice test 
case for a missed diagnostic of some form, but for code that is supposed to 
compile, kinda hate it.

Thoughts?

Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)

[Jason Merrill]

On 08/08/2013 03:54 PM, Paolo Carlini wrote:

the really interesting one is decltype28.C, which we don't reject
anymore, we simply accept it. What is happening is that the overload
which leads to excessive template instantiation depth is SFINAE-ed away
and the other one "wins"! Thus, this is the core of my message: it seems
that we behave wrt this issue - SFINAE vs template instantiation depth -
in a different way vs current clang++ and icc: we produce hard error
messages in SFINAE contexts. Is that intended?


Yes, that is intended.  Changing that could mean that the meaning of 
code depends on what max depth the user selected.


Jason

Re: [PATCH/Merge Request] Vtable Verification feature.

[Jason Merrill]

On 08/08/2013 06:34 PM, Caroline Tice wrote:

Actually if you ever want to use the feature with your compiler you
should build your compiler with --enable-vtable-verify.  This will, as
you noted,  insert calls in libstdc++ to build the verification data
structures and to verify the virtual calls in libstdc++ itself.
However libstdc++ itself contains 'stub' (do-nothing) versions of the
functions that build the data structures and perform the verification.


How do you ensure that the libvtv versions get used rather than the 
stubs in libstdc++?


Jason

Re: [PATCH] Fix PR48493

[Mike Stump]

In the below, the test case tries to write to the stack outside the bounds of 
the s variable?  I can't imagine any good coming from this, and indeed, would 
be nice for the compiler to complain about such code.  If S had a few more 
bytes at the end, at least the code would not be wildly bad.

Thoughts?

On May 31, 2012, at 3:59 AM, Richard Guenther  wrote:
> This fixes PR48493 by backporting a one-liner - we should not go
> the movmisalign path for destinations that are not memory.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu and on
> mips by Andrew, installed.
> 
> Richard.
> 
> 2012-05-31  Richard Guenther  
> 
>   PR middle-end/48493
>   * expr.c (expand_assignment): Do not use movmisalign on
>   non-memory.
> 
>   * gcc.dg/torture/pr48493.c: New testcase.

> Index: gcc/testsuite/gcc.dg/torture/pr48493.c
> ===
> *** gcc/testsuite/gcc.dg/torture/pr48493.c(revision 0)
> --- gcc/testsuite/gcc.dg/torture/pr48493.c(revision 0)
> ***
> *** 0 
> --- 1,18 
> + /* { dg-do compile } */
> + 
> + typedef long long T __attribute__((may_alias, aligned (1)));
> + 
> + struct S
> + {
> +   _Complex float d __attribute__((aligned (8)));
> + };
> + 
> + void bar (struct S);
> + 
> + void
> + f1 (T x)
> + {
> +   struct S s;
> +   *(T *) ((char *) &s.d + 1) = x;
> +   bar (s);
> + }

Re: [PATCH] Sanitize block partitioning under -freorder-blocks-and-partition

[Teresa Johnson]

On Thu, Aug 8, 2013 at 3:23 PM, Jan Hubicka  wrote:
> Hi,
> Martin Liska was kind enough to generate disk seeking graph of gimp statrup 
> with his function reordering.
> His code simply measures time of firest execution of a function and orders 
> functions in the given order.
> The functions stay in the subsections (unlikely/startup/exit/hot/normal) that 
> are then glued together
> in this order.
>
> I am attaching disk seeking with and without -freorder-blocks-and-partition 
> (with your patch).
>
> In 2.pdf you can see two increasing sequences in the text segment.  If I am 
> not mistaken the bottom
> one comes for hot and the top one for normal section.  The big unused part on 
> bottom is unlikely
> section since most of gimp is not trained.

2.pdf is reordered with Martin's technique?

>
> Now 1.pdf is with -freorder-blocks-and-partition and your patch.  You can see 
> there is third sequence
> near bottom of the text seciton. that is beggining of unlikely section, so it 
> tracks jumps where we
> fall into cold section of function.

1.pdf is generated using the usual FDO +
-freorder-blocks-and-partition (i.e. not using Martin's technique)?

>
> It still seems rather bad (i.e. good part of unlikely section is actually 
> used).  I think the dominator
> based approach is not going to work too reliably (I can "fix" my testcase to 
> contain multiple nested
> conditionals and then the heuristic about predecestors won't help).

Yes, this doesn't look good. Did you use the latest version of my
patch that doesn't walk the dominators?

Do you know how many training runs are done for this benchmark? I
think a lot of the issues that you pointed out with the hot loop
preceded by non-looping conditional code as in your earlier example,
or multiple nested conditionals, comes from the fact that the cold
cutoff is not 0, but some number less than the number of training
runs. Perhaps the cutoff for splitting should be 0. Then the main
issue that needs to be corrected is profile insanities, not code that
is executed once (since that would not be marked cold).

The only other issue that I can think of here is that the training
data was not representative and didn't execute these blocks.

>
> What about simply walking the CFG from entry through all edges with non-0 
> counts and making all reachable
> blocks hot + forcingly make any hot blocks not reachable this way reachable?

Is this different than what I currently have + changing the cold
cutoff to 0? In that case any blocks reachable through non-0 edges
should be non-0 and marked hot, and the current patch forces the most
frequent paths to all hot blocks to be hot.

Thanks,
Teresa

> I think we are really looking primarily for dead parts of the functions 
> (sanity checks/error handling)
> that should not be visited by train run.  We can then see how to make the 
> heuristic more aggressive?
>
> Honza



-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [PATCH/Merge Request] Vtable Verification feature.

[Joseph S. Myers]

On Thu, 8 Aug 2013, Caroline Tice wrote:

> Actually if you ever want to use the feature with your compiler you
> should build your compiler with --enable-vtable-verify.  This will, as
> you noted,  insert calls in libstdc++ to build the verification data
> structures and to verify the virtual calls in libstdc++ itself.
> However libstdc++ itself contains 'stub' (do-nothing) versions of the
> functions that build the data structures and perform the verification.
>  So if you want to turn on verification with libstdc++, you link it
> with libvtv (which contains the "real" versions of those functions)
> and if you don't want verification with libstdc++ you just don't link
> in libvtv.  There is no need to multiple versions of libstdc++.

But presumably libstdc++ with the extra calls is less efficient than 
libstdc++ without them, even if the calls are just to stub functions?

A GNU/Linux distributor would likely want to enable their users to use 
this functionality, meaning distributing a GCC build with libvtv included 
and a version of libstdc++ with the calls present for users who wish to 
build programs (linking with libstdc++ and libvtv) for which the calls in 
libstdc++ are checked.  But they might not want to have even the stub 
calls executed for all installed C++ programs.  So it would seem natural 
to provide both copies of libstdc++ - and desirable for this to be 
possible without needing separate GCC builds.

> I supposed the libgcc files could be built all the time (on systems
> that support libvtv).  Would there be any down side to this?

In my model, it's appropriate to build those independent of the libstdc++ 
configure option - and likewise to build everything in libvtv independent 
of the configure option.  There should be no need for the configure option 
to affect anything other than, at most, libstdc++ - that is, the handling 
of the option in other configure scripts should be removed, with the case 
where the relevant support is built being enabled by default.

> For certain use cases the current working directory is not our
> preferred place to put the log files.
> I have recently submitted a patch that tries to use environment
> variables to determine where to put the log files.  It first checks to
> see if the user has defined VTV_LOGS_DIR, in which case it will use
> that.  If that fails, it tries to find and use HOME.  If that also
> fails, it falls back on using the working directory.  I hope that is
> ok?
> 
> I have modified the call to open to take O_NOFOLLOW, but O_EXCL will
> do the wrong thing, as we sometimes wish to append to the log files
> and O_EXCL fails if you attempt to open an existing file (according to
> the documentation I read).

The list of directories seems plausible (better than using /tmp, anyway).

Classically O_EXCL was needed in such cases (if the log might end up 
getting written to a file in a directory also writable by an attacker) not 
just because of symlink attacks but also because it was possible to create 
a hard link to another user's file, with similar potential for attacks as 
with symlinks.  Nowadays systems are more likely to restrict such hard 
link creation (but they are also likely to have similar restrictions to 
make symlink attacks harder as well).

-- 
Joseph S. Myers
jos...@codesourcery.com

Fix PHI IDs in LTO streaming

[Jan Hubicka]

Hi,
LTO streaming renumbers gimple statements. It however forgets about PHIs that 
causes
duplicated gimple ids and later fun everywhere (especially for me when I want to
use them for reference streaming).
This patch assign numbers to SSA names too.  Since virtual PHIs are not stored, 
we
have to ignore them on stream-out time.  I do not think it is cool that they get
duplicated ids so I just renumber them afterwards.

Bootstrapped/regtested x86_64-linux, comitted as obvious.

Honza

* lto-streamer-out.c (output_function): Renumber PHIs.
* lto-streamer-in.c (input_function): Likewise.
Index: lto-streamer-out.c
===
--- lto-streamer-out.c  (revision 201568)
+++ lto-streamer-out.c  (working copy)
@@ -1792,12 +1792,32 @@ output_function (struct cgraph_node *nod
   FOR_ALL_BB (bb)
{
  gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+   {
+ gimple stmt = gsi_stmt (gsi);
+
+ /* Virtual PHIs are not going to be streamed.  */
+ if (!virtual_operand_p (gimple_phi_result (stmt)))
+   gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun));
+   }
  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
  gimple stmt = gsi_stmt (gsi);
  gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun));
}
}
+  /* To avoid keeping duplicate gimple IDs in the statements, renumber
+virtual phis now.  */
+  FOR_ALL_BB (bb)
+   {
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+   {
+ gimple stmt = gsi_stmt (gsi);
+ if (virtual_operand_p (gimple_phi_result (stmt)))
+   gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun));
+   }
+   }
 
   /* Output the code for the function.  */
   FOR_ALL_BB_FN (bb, fn)
Index: lto-streamer-in.c
===
--- lto-streamer-in.c   (revision 201568)
+++ lto-streamer-in.c   (working copy)
@@ -908,6 +908,11 @@ input_function (tree fn_decl, struct dat
   FOR_ALL_BB (bb)
 {
   gimple_stmt_iterator gsi;
+  for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+   {
+ gimple stmt = gsi_stmt (gsi);
+ gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun));
+   }
   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
  gimple stmt = gsi_stmt (gsi);
@@ -917,7 +922,14 @@ input_function (tree fn_decl, struct dat
   stmts = (gimple *) xcalloc (gimple_stmt_max_uid (fn), sizeof (gimple));
   FOR_ALL_BB (bb)
 {
-  gimple_stmt_iterator bsi = gsi_start_bb (bb);
+  gimple_stmt_iterator bsi = gsi_start_phis (bb);
+  while (!gsi_end_p (bsi))
+   {
+ gimple stmt = gsi_stmt (bsi);
+ gsi_next (&bsi);
+ stmts[gimple_uid (stmt)] = stmt;
+   }
+  bsi = gsi_start_bb (bb);
   while (!gsi_end_p (bsi))
{
  gimple stmt = gsi_stmt (bsi);

Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv

[Caroline Tice]

On Thu, Aug 8, 2013 at 3:26 PM, Mike Stump  wrote:
> On Aug 8, 2013, at 3:09 PM, Caroline Tice  wrote:
>> This patch changes where the logging file mechanism in libvtv tries to
>> write its log files.  Instead of trying to use /tmp, it now first
>> looks for an environment variable "VTV_LOGS_DIR".  If it can't find
>> that it looks for the environment variable "HOME".
>
> Hum…  I kinda don't think HOME should be used.

Why not?

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

On Wed, Aug 7, 2013 at 6:03 PM, Joseph S. Myers  wrote:
> Looking at the patch as committed, there seems to be some confusion about
> the nature of the --enable-vtable-verify configure option.

Yes, there is a bit.

>
> It's documented only for libstdc++.  Now, I still consider the existence
> of separate documentation for libstdc++ configure options to be
> unfortunate - I think all configure options for GCC should be documented
> in one place - but that's a separate matter.  Although only documented for
> libstdc++, it actually appears to be used in the libgcc and libvtv
> configure scripts.

The original intent of this flag was that if --enable-vtable-verify
was NOT used, then NOTHING having to do with vtable verification would
be built anywhere in the compiler, i.e. the binaries, libraries, etc.
would not contain anything that wasn't there before the vtable
verification patch was committed.  The idea (and implementation) has
evolved a bit, and I am not sure what the "right" way to handle this
option is at the moment.

>
> Given that it has effects on more than just libstdc++, it needs
> documenting in gcc/doc/install.texi, the main documentation of configure
> options for GCC.

I recently submitted a patch that adds the documentation for my
current understanding of the way this option works.

> Then there's the question of what the semantics should
> be.  My presumption is that the feature should be available for all GCC
> builds, at least by default on platforms where it can work, and the only
> thing needing a configure option should be whether the checks are enabled
> for libstdc++ itself (and ideally that would work by multilibbing
> libstdc++ rather than needing separate GCC builds to get libstdc++ with
> and without the checks).

Actually if you ever want to use the feature with your compiler you
should build your compiler with --enable-vtable-verify.  This will, as
you noted,  insert calls in libstdc++ to build the verification data
structures and to verify the virtual calls in libstdc++ itself.
However libstdc++ itself contains 'stub' (do-nothing) versions of the
functions that build the data structures and perform the verification.
 So if you want to turn on verification with libstdc++, you link it
with libvtv (which contains the "real" versions of those functions)
and if you don't want verification with libstdc++ you just don't link
in libvtv.  There is no need to multiple versions of libstdc++.

>Thus if the platform supports the feature, all
> relevant libgcc files should be built, and anything for libstdc++ needed
> for user code to use the feature should be built - the only thing not
> enabled by default would be checks for libstdc++'s classes' own vtables.
> (And unless there are difficulties in building the libgcc files on some
> systems, they could be built unconditionally, whether or not any other
> support needed for libvtv is present.

I supposed the libgcc files could be built all the time (on systems
that support libvtv).  Would there be any down side to this?

> Actually, it looks like they may
> depend on an ELF target, but not on anything more.)
>
> Could you confirm that the libstdc++ ABI is not affected by the configure
> option - that the same symbols, at the same symbol versions, with the same
> semantics, are exported from libstdc++.so for builds with and without the
> feature enabled?
>

The libstdc++ ABI has been enhanced to export the vtable verification
functions for which it contains stub versions (otherwise they could
never be overwritten by the versions in libvtv).  Other than that the
libstdc++ ABI exports the same symbols at the same symbol versions
with the same semantics.  I believe this export is unconditional.

> The file cp/vtable-class-hierarchy.c includes "tm.h".  Includes of tm.h
> from front ends are discouraged, and should have comments on them listing
> what target macros are used by the file in question (and so need to be
> converted to hooks before the include can be removed).  Could you add such
> a comment to the #include (or if it's redundant, remove all redundant
> #includes from that file)?
>
> You have a
>
> +#define MAX_SET_SIZE 5000
>
> which superficially looks like an arbitrary limit.  Could you add a
> comment explaining why no input files, not matter how extreme, could ever
> exceed the limit of 5000?  You have a couple of gcc_asserts regarding this
> limit, and an on-stack array for which it's at least not immediately
> obvious that all accesses are checked to ensure that a buffer overrun is
> impossible.
>
> If you have an arbitrary limit, and some input *can* exceed it (so
> triggering the gcc_asserts or buffer overrun), the right fix is of course
> to remove it, probably using vec.h to produce a dynamically growing array
> instead of hardcoding a size at all.  But failing that, exceeding the
> limit must result in a sorry () call, not an assertion failure or buffer
> overrun, neither of which is acceptable behavior for any compiler inp

Re: [FIXED] Generic lambda symbol table bug

[Adam Butcher]

On 07.08.2013 20:56, Adam Butcher wrote:

On 07.08.2013 16:59, Jason Merrill wrote:

On 08/07/2013 03:52 AM, Adam Butcher wrote:

But a cleaner way might be to extend the "processing
template declaration" state from lambda declarator all the way to 
the end of the
lambda body.  This would match with the scenario that occurs with a 
standard
in-class member function template definition.  To do that elegantly 
would

require a bit of refactoring of the lambda parser code.


It isn't already set through the whole lambda body?


No.  It is within cp_parser_lambda_declarator_opt.


That seems
necessary to support non-trivial lambda bodies; otherwise we won't 
be

able to handle dependence properly.


Agreed.  Okay, I will produce three patches:

  1) Refactor existing monomorphic implementation to give a cleaner
 parser separation of concerns; extract the fco creation and
 provide begin and end for the lambda function operator.

I did this---well not exactly as written---but I extended the template 
decl state through until the end of the lambda body.  And it works with 
my dependence test.  However, the previous version does too.  The 
following program works fine (conv ops an all) with the refactored 
version and without it.  I'm now questioning whether it is worth making 
any change in this area.  What do you think?  The test code is as 
follows and the resulting program correctly executes S::N::test() 4 
times as expected -- were you thinking of some other dependence case?


  #include 

  struct S
  {
 struct N
 {
float test () { std::cout << "S::N::test() -> 7.f\n"; return 
7.f; }

 };
  };

  int main()
  {
 auto f = []  (T const& s) {
typename T::N x;
return x.test ();
 };
 auto g = [] (auto const& s) {
typename std::decay::type::N x;
return x.test ();
 };

 S i;

 f(i);
 g(i);

 float (*pfn) (S const&) = f;

 pfn(i);

 pfn = g;

 return pfn(i);
  }

Error messages are reasonable also.  Omitting the necessary 'typename's 
gives:


   : need ‘typename’ before ‘T::N’ because ‘T’ is a dependent scope
   : need ‘typename’ before ‘typename std::decay(s)>::type::N’ because ‘typename std::decay::type’ is a 
dependent scope


Passing a local 'struct X {}' instead of 'S' gives:

  : In instantiation of ‘auto main():: const [with T 
= main()::X]’:

  :24:9:   required from here
  :14:23: error: no type named ‘N’ in ‘struct main()::X’
  ...

So all seems to be okay with both versions.  Any ideas why?

Cheers,
Adam

Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv

[Mike Stump]

On Aug 8, 2013, at 3:09 PM, Caroline Tice  wrote:
> This patch changes where the logging file mechanism in libvtv tries to
> write its log files.  Instead of trying to use /tmp, it now first
> looks for an environment variable "VTV_LOGS_DIR".  If it can't find
> that it looks for the environment variable "HOME".

Hum…  I kinda don't think HOME should be used.

[PATCH, vtv update] Change fixed size array to a vector; fix diagnostic messages.

[Caroline Tice]

This patch replaces the fixed sized array that was holding vtable
pointers for a particular class hierarchy with a vector, allowing for
dynamic resizing.  It also fixes issues with the warning diagnostics.
I am in the process of running regression tests with this patch;
assuming they all pass, is this patch OK to commit?

-- Caroline Tice
cmt...@google.com

2013-08-08  Caroline Tice  

* vtable-class-hierarchy.c: Remove unnecessary include statements.
(MAX_SET_SIZE): Remove unnecessary constant.
(register_construction_vtables):  Make vtable_ptr_array parameter
into a vector; remove num_args parameter. Change array accesses to
vector accesses.
(register_other_binfo_vtables): Ditto.
(insert_call_to_register_set): Ditto.
(insert_call_to_register_pair): Ditto.
(output_set_info):  Ditto.  Also change warning calls to warning_at
calls, and fix format of warning messages.
(register_all_pairs): Change vtbl_ptr_array from an array into a
vector.  Remove num_vtable_args (replace with calls to vector length).
Change array stores & accesses to vector functions. Change calls to
register_construction_vtables, register_other_binfo_vtables,
insert_call_to_register_set, insert_call_to_register_pair and
output_set_info to match their new signatures.  Change warning to
warning_at and fix the format of the warning message.


vtv-update.patch
Description: Binary data


vtv-update.patch
Description: Binary data

Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Joseph S. Myers]

On Thu, 8 Aug 2013, Caroline Tice wrote:

> The attached patch adds documentation for the --enable-vtable-verify
> configure option to  install.texi.  Is this ok to commit?

Could you please answer the questions I raised in the first four 
paragraphs of  at 
greater length?  This patch appears to document it just as a libstdc++ 
configure option, but as noted there it appears to affect more than 
libstdc++ (even though it would be more sensible if it *only* affected 
libstdc++, and even better if it just enabled extra multilibs).

-- 
Joseph S. Myers
jos...@codesourcery.com

[PATCH, vtv update] Fix /tmp directory issues in libvtv

[Caroline Tice]

This patch changes where the logging file mechanism in libvtv tries to
write its log files.  Instead of trying to use /tmp, it now first
looks for an environment variable "VTV_LOGS_DIR".  If it can't find
that it looks for the environment variable "HOME".  If it can't find
that either, it uses the current directory.  This also adds O_NOFOLLOW
to the open command.  There was a request that we also use O_EXCL, but
that would cause a problem because we occasionally want to be able to
append to existing log files, and adding O_EXCL would cause that to
fail.  I also added the uid and pid to the log file names.

Is this patch OK to commit?

-- Caroline Tice
cmt...@google.com


2013-08-08  Caroline Tice  

* vtv_utils.cc : Include stdlib.h
(log_dirs): Remove file static constant.
(__vtv_open_log):  Increase size of log file name.  Add the user
and process ids to the file name. Do not put the log files in /tmp.
Instead try to get the directory name from environment variables; if
that fails use the current directory.  Add O_NOFOLLOW to the flags
for 'open'.  Update function comment.


vtv-update-tmpdir.patch
Description: Binary data

[PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Caroline Tice]

The attached patch adds documentation for the --enable-vtable-verify
configure option to  install.texi.  Is this ok to commit?

-- Caroline TIce
cmt...@google.com

2013-08-08  Caroline Tice  

* doc/install.texi: Add documentation for the --enable-vtable-verify
configure option.


vtv-update-doc.patch
Description: Binary data

[patch][PR/42955] Don't install $(target)/bin/gcc, gfortran, etc.

[Brooks Moses]

As discussed in PR/42955, when GCC is built as a cross-compiler, it
will install "gcc", "g++", "c++", and "gfortran" binaries in
$(target)/bin, as well as installing the $target-gcc and so forth in
bin.  However, these binaries in $(target)/bin do not work; they
cannot find libexec.

More to the point, this bug has been open for three years with no
traffic, and the failure started significantly before that.  Clearly,
making these work is not a priority.  Further, these binaries are real
files, not symlinks or hard links; they take up actual space.

As discussed on the bug, Joseph argues that $(target)/bin "contains
executables from binutils for internal use by GCC; that's its sole
purpose. The files installed by GCC there aren't used by GCC (rather,
the public installed copy of the driver gets used when collect2 needs
to call back to the driver), so shouldn't be installed."

Thus, this patch, which simply removes these broken executables.
Tested by building a cross-compiler and confirming that they are gone,
and by building a native compiler and confirming that the expected
bin/gcc, bin/g++, bin/c++, and bin/gfortran are still present.

Ok to commit?

- Brooks


2013-08-08  Brooks Moses  

PR driver/42955
* Makefile.in: Do not install driver binaries in $(target)/bin.

PR driver/42955
* Make-lang.in: Do not install driver binaries in $(target)/bin.


2013-08-08_remove-target-gcc-bins.diff
Description: Binary data

patch for correct mode use by LRA for save/restore generation

[Vladimir Makarov]

The following patch implements correct mode use for save/restore generation.

The patch was successfully bootstrapped and tested on x86/x86-64, ppc64, 
s390.


The patch also makes some tuning in alternative matching and adds more 
debugging printing.


Committed as rev. 201611.

2013-08-08  Vladimir Makarov  

* lra-constraints.c (emit_spill_move): Remove assert.
(process_alt_operands): Add more debugging
output.  Increase reject for spilling into memory.  Decrease
reject for reloading scratch.
(split_reg): Use HARD_REGNO_CALLER_SAVE_MODE.

Index: lra-constraints.c
===
--- lra-constraints.c   (revision 201544)
+++ lra-constraints.c   (working copy)
@@ -859,8 +859,9 @@
 {
   if (GET_MODE (mem_pseudo) != GET_MODE (val))
 {
-  lra_assert (GET_MODE_SIZE (GET_MODE (mem_pseudo))
- >= GET_MODE_SIZE (GET_MODE (val)));
+  /* Usually size of mem_pseudo is greater than val size but in
+rare cases it can be less as it can be defined by target
+dependent macro HARD_REGNO_CALLER_SAVE_MODE.  */
   if (! MEM_P (val))
{
  val = gen_rtx_SUBREG (GET_MODE (mem_pseudo),
@@ -1423,8 +1424,14 @@
 
   overall = losers = reject = reload_nregs = reload_sum = 0;
   for (nop = 0; nop < n_operands; nop++)
-   reject += (curr_static_id
-  ->operand_alternative[nalt * n_operands + nop].reject);
+   {
+ int inc = (curr_static_id
+->operand_alternative[nalt * n_operands + nop].reject);
+ if (lra_dump_file != NULL && inc != 0)
+   fprintf (lra_dump_file,
+"Staticly defined alt reject+=%d\n", inc);
+ reject += inc;
+   }
   early_clobbered_regs_num = 0;
 
   for (nop = 0; nop < n_operands; nop++)
@@ -1598,7 +1605,14 @@
|| (find_regno_note (curr_insn, REG_DEAD,
 REGNO (operand_reg[nop]))
 == NULL_RTX))
- reject += 2;
+ {
+   if (lra_dump_file != NULL)
+ fprintf
+   (lra_dump_file,
+"%d Matching alt: reject+=2\n",
+nop);
+   reject += 2;
+ }
  }
/* If we have to reload this operand and some
   previous operand also had to match the same
@@ -1854,16 +1868,35 @@
{
  if (in_hard_reg_set_p (this_costly_alternative_set,
 mode, hard_regno[nop]))
-   reject++;
+   {
+ if (lra_dump_file != NULL)
+   fprintf (lra_dump_file,
+"%d Costly set: reject++\n",
+nop);
+ reject++;
+   }
}
  else
{
  /* Prefer won reg to spilled pseudo under other equal
 conditions.  */
+ if (lra_dump_file != NULL)
+   fprintf
+ (lra_dump_file,
+  "%d Non pseudo reload: reject++\n",
+  nop);
  reject++;
  if (in_class_p (operand_reg[nop],
  this_costly_alternative, NULL))
-   reject++;
+   {
+ if (lra_dump_file != NULL)
+   fprintf
+ (lra_dump_file,
+  "%d Non pseudo costly reload:"
+  " reject++\n",
+  nop);
+ reject++;
+   }
}
  /* We simulate the behaviour of old reload here.
 Although scratches need hard registers and it
@@ -1872,7 +1905,13 @@
 might cost something but probably less than old
 reload pass believes.  */
  if (lra_former_scratch_p (REGNO (operand_reg[nop])))
-   reject += LRA_LOSER_COST_FACTOR;
+   {
+ if (lra_dump_file != NULL)
+   fprintf (lra_dump_file,
+"%d Scratch win: reject+=3\n",
+nop);
+ reject += 3;
+   }
}
}
  else if (did_match)
@@ -1914,7 +1953,12 @@
 
  t

[C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)

[Paolo Carlini]

Hi,

this is, IMHO, a rather interesting issue. I was working on PR 54080, 
where currently with ICE pretty badly for Error reporting routines 
re-entered without producing any sensible error message. Figured out 
that the core of the issue are the error messages in push_tinst_level 
about template instantiation depth exceeded. Drafted the attached, the 
error message is great with no ICE:


54080.C: In instantiation of ‘decltype (foo class 
vector>(foo(input, func1), funcrest)) foo(const vector&, const 
Func1&, FuncRest) [with OutType = vector; Func1 = int; FuncRest = int; 
decltype (foo(foo(input, func1), funcrest)) = vector]’:

54080.C:25:22: required from here
54080.C:19:2: error: return-statement with no value, in function 
returning ‘vector’ [-fpermissive]

return;

when I ran the testsuite, I found something (tidied):

FAIL: g++.dg/cpp0x/decltype26.C (test for errors, line 6)
FAIL: g++.dg/cpp0x/decltype26.C (test for excess errors)
FAIL: g++.dg/cpp0x/decltype28.C (test for errors, line 11)
FAIL: g++.dg/cpp0x/decltype28.C (test for warnings, line 15)
FAIL: g++.dg/cpp0x/decltype29.C (test for errors, line 12)
FAIL: g++.dg/cpp0x/decltype29.C (test for excess errors)

the really interesting one is decltype28.C, which we don't reject 
anymore, we simply accept it. What is happening is that the overload 
which leads to excessive template instantiation depth is SFINAE-ed away 
and the other one "wins"! Thus, this is the core of my message: it seems 
that we behave wrt this issue - SFINAE vs template instantiation depth - 
in a different way vs current clang++ and icc: we produce hard error 
messages in SFINAE contexts. Is that intended? I find the issue 
interesting, arguably a template instantiation depth exceeded isn't just 
like any other error.


With the attached draft we handle quite a few other testcases I have 
seen around in a different way, for example c++/52875 is automatically 
fixed (assuming we want to behave like clang++ and icc).


Thanks!
Paolo.
Index: cp-tree.h
===
--- cp-tree.h   (revision 201588)
+++ cp-tree.h   (working copy)
@@ -5541,7 +5541,7 @@ extern tree fold_non_dependent_expr_sfinae(tree,
 extern bool alias_type_or_template_p(tree);
 extern bool alias_template_specialization_p (const_tree);
 extern bool explicit_class_specialization_p (tree);
-extern int push_tinst_level (tree);
+extern int push_tinst_level (tree, tsubst_flags_t);
 extern void pop_tinst_level (void);
 extern struct tinst_level *outermost_tinst_level(void);
 extern void init_template_processing   (void);
Index: mangle.c
===
--- mangle.c(revision 201588)
+++ mangle.c(working copy)
@@ -3429,7 +3429,7 @@ mangle_decl_string (const tree decl)
 {
   struct tinst_level *tl = current_instantiation ();
   if ((!tl || tl->decl != decl)
- && push_tinst_level (decl))
+ && push_tinst_level (decl, tf_warning_or_error))
{
  template_p = true;
  saved_fn = current_function_decl;
Index: pt.c
===
--- pt.c(revision 201588)
+++ pt.c(working copy)
@@ -6995,7 +6995,7 @@ add_pending_template (tree d)
   level = !current_tinst_level || current_tinst_level->decl != d;
 
   if (level)
-push_tinst_level (d);
+push_tinst_level (d, tf_warning_or_error);
 
   pt = ggc_alloc_pending_template ();
   pt->next = NULL;
@@ -8021,24 +8021,28 @@ static GTY(()) struct tinst_level *last_error_tins
for diagnostics and to restore it later.  */
 
 int
-push_tinst_level (tree d)
+push_tinst_level (tree d, tsubst_flags_t complain)
 {
   struct tinst_level *new_level;
 
   if (tinst_depth >= max_tinst_depth)
 {
   last_error_tinst_level = current_tinst_level;
-  if (TREE_CODE (d) == TREE_LIST)
-   error ("template instantiation depth exceeds maximum of %d (use "
-  "-ftemplate-depth= to increase the maximum) substituting %qS",
-  max_tinst_depth, d);
-  else
-   error ("template instantiation depth exceeds maximum of %d (use "
-  "-ftemplate-depth= to increase the maximum) instantiating %qD",
-  max_tinst_depth, d);
 
-  print_instantiation_context ();
+  if (complain & tf_error)
+   {
+ if (TREE_CODE (d) == TREE_LIST)
+   error ("template instantiation depth exceeds maximum of %d (use "
+  "-ftemplate-depth= to increase the maximum) "
+  "substituting %qS", max_tinst_depth, d);
+ else
+   error ("template instantiation depth exceeds maximum of %d (use "
+  "-ftemplate-depth= to increase the maximum) "
+  "instantiating %qD", max_tinst_depth, d);
 
+ print_instantiation_context ();
+   }
+
   ret

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

On 08/08/2013 08:31 PM, Paolo Carlini wrote:

On 08/08/2013 08:18 PM, David Edelsohn wrote:

Why does the patch and fix have any architecture or OS-dependency?
It's tricky, but it depends on the way are internally stored and 
handled *multiple* TYPE_DECL for essentially the same typedef. The 
class layout must involve both such typedef and a few data members.
... and at least a virtual function, forgot this important detail. I 
don't think you can construct a reject-valid of this type on Linux 
without a virtual function.


Paolo.

Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)

[Aldy Hernandez]

On 08/08/13 18:42, Richard Henderson wrote:

On 08/08/2013 02:06 AM, Aldy Hernandez wrote:

The hash is not really mapping the simd DECL to the simduid, since that's just
a matter of DECL_UID(simduid), but the OMP simd array to the index used to
reference it (simduid), like thus:

 _7 = GOMP_SIMD_LANE (simduid.0)
 ...
 ...
 D.1737[_7] = stuff;

decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP simd
array).

Agreed, or am I missing something?


Agreed.  Yeah, that's a bit confusing.


Ok, I will change the hash name and add some comments.

Re: [patch, mips] Add nan2008 multilibs to mips-mti-* targets.

[Richard Sandiford]

"Steve Ellcey "  writes:
> I would like to add new variations to the mips-mti-elf and mips-mti-linux
> targets to support the new NaN format on MIPS (mnan=2008 for IEEE 754-2008).
> While I was doing this I noticed that I was handling mips16 and micromips
> in a dumb manner.  I had them in MULTILIB_OPTIONS as non-exclusive flags
> and then used MULTILIB_EXCEPTIONS to not allow them to both be used.
> So I also changed MULTILIB_OPTIONS to make mips16 and micromips exclusive and
> then I could get rid of one of the MULTILIB_EXCEPTIONS entries.

Good spot.  Looks like you could do the same thing with -mabi=64, but it
isn't as logical as the mips16/micromips thing and probably wouldn't win much.

> I also make the new mnan=2008 flag exclusive of msoft-float since it
> is only useful for hard-float.

Well, it's useful for both, because -msoft-float uses the same NaN
format as -mhard-float.  But not having the multilib is fine.

> 2013-08-07  Steve Ellcey  
>
>   * config/mips/mti-linux.h (SYSROOT_SUFFIX_SPEC): Add nan2008.
>   * config/mips/t-mti-elf (MULTILIB_OPTIONS): Make mips16 and
>   micromips incompatible.  Add nan2008.
>   (MULTILIB_DIRNAMES): Add nan2008.
>   (MULTILIB_EXCEPTIONS): Remove mips16/micromips entry.
>   * config/mips/t-mti-linux (MULTILIB_OPTIONS): Make mips16
>   and micromips incompatible.  Add nan2008.
>   (MULTILIB_DIRNAMES): Add nan2008.
>   (MULTILIB_EXCEPTIONS): Remove mips16/micromips entry.

OK, thanks.

Richard

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ramana Radhakrishnan]

arm*-*-linux* is broken currently for what looks like the same reasons as
the powerpc port.




Have you tried Benjamin Kosnik's patch?  Does it fix the problem?




I hadn't but that seems to have fixed the issue. Thanks.


I spent some time this morning looking into what it would take to enable
this for arm*-*-linux* today and the first thing that I noticed is that the
testsuite is essentially driven by a shell script that caters to native
testing on the only port that this was developed for. It also expects -m32
and -m64 to be a standard option in all ports that want to turn this on.
Also because the tests are not using dejagnu it's going to be a right pain
to get this to work for cross-testing and unless that works reliably one
isn't going to be able to get this feature working easily.

Also whatever is target specific in the testsuite like environment-fail-32.s
and environment-fail-64.s needs to live in it's own target folders.
Basically this feature needs to follow conventions that exist already in
other parts of the source base or atleast have plans to get there surely ?

I haven't seen this in the reviews that I've seen so far, so apologies if
this has already been raised.



I know that the testsuite, as committed, is not in DejaGnu format and
that it will need to be converted to DejaGnu format.  I also know that
it is currently specific to the platform on which the feature was
developed.  The plan has always been to fix this.  There was some
private discussion about whether it would be better to commit the
testsuite in its current state or not to commit a testsuite at all
with the initial project commit.  I was told that it would be better
to show that there is some way of testing this feature, even if the
testsuite is not in its final form, than to not show any way of
testing it at all.


Good to know there is atleast a plan to fix this. It would have been 
better to state this in a public review or as part of your merge request 
or indeed fix this properly before merging .




I will work on getting the testsuite into a better form and also on
writing up some documentation on the platform-specific pieces of the
vtable verification feature (for those who wish to port it to other
platforms).



Cool - thanks .

So I tried playing with it on native arm*-*-*linux builds for sometime 
and ran into the first problem which came while just building vtv_start.c .



/bin/bash ./libtool --tag=CC   --mode=compile 
/home/ramrad01/build-vtv1/./gcc/xgcc -B/home/ramrad01/build-vtv1/./gcc/ 
-B/usr/local/armv7l-unknown-linux-gnueabihf/bin/ 
-B/usr/local/armv7l-unknown-linux-gnueabihf/lib/ -isystem 
/usr/local/armv7l-unknown-linux-gnueabihf/include -isystem 
/usr/local/armv7l-unknown-linux-gnueabihf/sys-include -I. 
-I../../../gcc/libvtv  -I../../../gcc/libvtv/../include  -D_GNU_SOURCE 
-Wall -Wextra -fno-exceptions -g -O2 -MT vtv_start.lo -MD -MP -MF 
.deps/vtv_start.Tpo -c -o vtv_start.lo vtv_start.c
libtool: compile:  /home/ramrad01/build-vtv1/./gcc/xgcc 
-B/home/ramrad01/build-vtv1/./gcc/ 
-B/usr/local/armv7l-unknown-linux-gnueabihf/bin/ 
-B/usr/local/armv7l-unknown-linux-gnueabihf/lib/ -isystem 
/usr/local/armv7l-unknown-linux-gnueabihf/include -isystem 
/usr/local/armv7l-unknown-linux-gnueabihf/sys-include -I. 
-I../../../gcc/libvtv -I../../../gcc/libvtv/../include -D_GNU_SOURCE 
-Wall -Wextra -fno-exceptions -g -O2 -MT vtv_start.lo -MD -MP -MF 
.deps/vtv_start.Tpo -c vtv_start.c  -fPIC -DPIC -o .libs/vtv_start.o
vtv_start.c:57:1: warning: constructor priorities from 0 to 100 are 
reserved for the implementation [enabled by default]

 {
 ^
vtv_start.c:65:3: internal compiler error: Segmentation fault
   = { };
   ^
0x3aa3f5 crash_signal
../../gcc/gcc/toplev.c:335
0x52a669 default_elf_asm_named_section(char const*, unsigned int, 
tree_node*)

../../gcc/gcc/varasm.c:6219
0x52c041 switch_to_section
../../gcc/gcc/varasm.c:7035
0x52c5b1 output_object_block
../../gcc/gcc/varasm.c:7209
0x52c5b1 output_object_block_htab
../../gcc/gcc/varasm.c:7270
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
make[2]: *** [vtv_start.lo] Error 1
make[2]: Leaving directory 
`/home/ramrad01/build-vtv1/armv7l-unknown-linux-gnueabihf/libvtv'

make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/home/ramrad01/build-vtv1/armv7l-unknown-linux-gnueabihf/libvtv'

make: *** [all] Error 2


It might be because of inconsistencies for handling comdat linkage with 
regards to the ARM C++ ABI variations but it's too late for me to dig 
further tonight.


regards
Ramana

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

On 08/08/2013 08:18 PM, David Edelsohn wrote:

Why does the patch and fix have any architecture or OS-dependency?
It's tricky, but it depends on the way are internally stored and handled 
*multiple* TYPE_DECL for essentially the same typedef. The class layout 
must involve both such typedef and a few data members. I had a look to 
testresults, and the patch appears to work for other -linux targets too, 
not just x86_64-linux, and it's elementary, doesn't add complexity to 
the code, thus unless it proves to cause regressions on the targets 
where it appears to work, it would be a pity to revert it. But as I said 
the issue is weird, after all remained unfixed for 3-4 years, 
realistically, I don't know if together with all the other issues I have 
in my TODO, I can promise to understand it in such detail to fix it on 
AIX too.


Paolo.

Re: [C++ Patch / RFC] PR 46206

[David Edelsohn]

Why does the patch and fix have any architecture or OS-dependency?

- David

On Thu, Aug 8, 2013 at 1:57 PM, Paolo Carlini  wrote:
> On 08/08/2013 07:35 PM, David Edelsohn wrote:
>>
>> I'm now seeing a new testsuite failure:
>>
>> FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
>> Excess errors:
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
>> error: using typedef-name 'Foo1::Bar' after 'struct'
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
>> error: invalid type in declaration before ';' token
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
>> error: using typedef-name 'Foo2::Bar' after 'struct'
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
>> error: invalid type in declaration before ';' token
>
> Which essentially means that my patchlet fixes the problem on x86_64-linux,
> but not on AIX. Because you would see the exact same fails if I revert the
> patch and keep the testcase in.
>
> I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason call
> I guess.
>
> Paolo.

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

On 08/08/2013 07:35 PM, David Edelsohn wrote:

I'm now seeing a new testsuite failure:

FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
Excess errors:
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
error: using typedef-name 'Foo1::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
error: invalid type in declaration before ';' token
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
error: using typedef-name 'Foo2::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
error: invalid type in declaration before ';' token
Which essentially means that my patchlet fixes the problem on 
x86_64-linux, but not on AIX. Because you would see the exact same fails 
if I revert the patch and keep the testcase in.


I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason 
call I guess.


Paolo.

Re: [C++ Patch / RFC] PR 46206

[David Edelsohn]

I'm now seeing a new testsuite failure:

FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
Excess errors:
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
error: using typedef-name 'Foo1::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
error: invalid type in declaration before ';' token
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
error: using typedef-name 'Foo2::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
error: invalid type in declaration before ';' token

Thanks, David

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

On 08/08/2013 06:51 PM, Tim Shen wrote:

On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini  wrote:

Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already 
have implementation experience about these flags, eg in iostream, and I dont 
think we want std::bitset everywhere.

So here's the change. It's under testing now, but could took several
hours. If someone has a faster machine, please tell the result :)

As a simple test, `g++ -m32 test.cc` didn't fail.

Thanks!

Paolo.

Re: [PING] [PATCH, AArch64] Skip gcc.dg/lower-subreg-1.c

[Yufeng Zhang]

Ping~

Thanks,
Yufeng

On 07/26/13 12:06, Yufeng Zhang wrote:

Hi,

This patch changes to skip gcc.dg/lower-subreg-1.c for aarch64*-*-*.
The word mode in aarch64 is 64-bit so the lower-subreg pass won't happen
in this test case.  The test is currently skipped on aarch64 with lp64
due to the directive of "dg-require-effective-target ilp32", but fails
when -mabi=ilp32 is in use.

OK to commit?

Thanks,
Yufeng

gcc/testsuite/

* gcc.dg/lower-subreg-1.c: Skip aarch64*-*-*.

Re: [Patch] Whole regex refactoring and current status

[Tim Shen]

On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini  wrote:
> Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we 
> already have implementation experience about these flags, eg in iostream, and 
> I dont think we want std::bitset everywhere.

So here's the change. It's under testing now, but could took several
hours. If someone has a faster machine, please tell the result :)

As a simple test, `g++ -m32 test.cc` didn't fail.


-- 
Tim Shen


a.patch
Description: Binary data

Re: [PATCH][ARM] FAIL: gcc.target/arm/pr58041.c scan-assembler ldrb

[Ramana Radhakrishnan]

On 08/08/13 15:44, Kyrylo Tkachov wrote:

Hi all,

The recently added gcc.target/arm/pr58041.c test exposed a bug in the backend.
When compiling for NEON and with -mno-unaligned-access we end up generating
the vld1.64 and vst1.64 instructions instead of doing the accesses one byte at
a time like -mno-unaligned-access expects. This patch fixes that by enabling
the NEON expander and insns that produce these instructions only when
unaligned accesses are allowed.

Bootstrapped on arm-linux-gnueabihf. Tested arm-none-eabi on qemu.

Ok for trunk and 4.8?


Ok for trunk and 4.8 .

regards
Ramana



Thanks,
Kyrill

2013-08-08  Kyrylo Tkachov  

* config/arm/neon.md (movmisalign): Disable when we
don't allow unaligned accesses.
(*movmisalign_neon_store): Likewise.
(*movmisalign_neon_load): Likewise.
(*movmisalign_neon_store): Likewise.
(*movmisalign_neon_load): Likewise.

Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)

[Richard Henderson]

On 08/08/2013 02:06 AM, Aldy Hernandez wrote:
> The hash is not really mapping the simd DECL to the simduid, since that's just
> a matter of DECL_UID(simduid), but the OMP simd array to the index used to
> reference it (simduid), like thus:
> 
> _7 = GOMP_SIMD_LANE (simduid.0)
> ...
> ...
> D.1737[_7] = stuff;
> 
> decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP simd
> array).
> 
> Agreed, or am I missing something?

Agreed.  Yeah, that's a bit confusing.


r~

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

On Thu, Aug 8, 2013 at 3:27 AM, Rainer Orth  
wrote:
> Caroline,
>
> your libgcc ChangeLog entries are all broken: they lack the initial "* "
> as can easily be seen in Emacs' Change Log Mode.
>
> Please fix.
>

Wow, I'm really sorry about that!  I don't know how that happened.
I'll fix it immediately.

> As an aside, I had a very quick look at libvtv to determine what's
> required for a port to a non-Linux platform.  It would be good if the
> requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly
> several more) could be documented to ease porters' task.
>


I will be working on some documentation for this.

> Thanks.
>
> Rainer
>
> --
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: New parameters to control stringop expansion libcall strategy

[Xinliang David Li]

Updated.

thanks,

David

On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers  wrote:
> On Wed, 7 Aug 2013, Xinliang David Li wrote:
>
>> Updated patch attached (fixed header, buffer overflow, and warning -->
>> error problems).
>
> You still have diagnostics starting with a capital letter, contrary to the
> GNU Coding Standards.
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
Index: testsuite/gcc.target/i386/memcpy-strategy-1.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-1.c   (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-1.c   (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=atom -mmemcpy-strategy=vector_loop:-1:align" } */
+/* { dg-final { scan-assembler-times "movdqa" 8 { target { ! { ia32 } } } } } 
*/
+/* { dg-final { scan-assembler-times "movdqa" 4 { target { ia32 } } } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+  __builtin_memcpy (a, b, 2048);
+}
+
Index: testsuite/gcc.target/i386/memcpy-strategy-2.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-2.c   (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-2.c   (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=atom 
-mmemcpy-strategy=vector_loop:3000:align,libcall:-1:align" } */
+/* { dg-final { scan-assembler-times "movdqa" 8 { target { ! { ia32 } } } } } 
*/
+/* { dg-final { scan-assembler-times "movdqa" 4 { target { ia32 } } } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+  __builtin_memcpy (a, b, 2048);
+}
+
Index: testsuite/gcc.target/i386/memset-strategy-1.c
===
--- testsuite/gcc.target/i386/memset-strategy-1.c   (revision 0)
+++ testsuite/gcc.target/i386/memset-strategy-1.c   (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=atom -mmemset-strategy=libcall:-1:align" } */
+/* { dg-final { scan-assembler-times "memset" 2  } } */
+
+char a[2048];
+void t (void)
+{
+  __builtin_memset (a, 1, 2048);
+}
+
Index: testsuite/gcc.target/i386/memcpy-strategy-3.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-3.c   (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-3.c   (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=atom 
-mmemcpy-strategy=vector_loop:2000:align,libcall:-1:align" } */
+/* { dg-final { scan-assembler-times "memcpy" 2  } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+  __builtin_memcpy (a, b, 2048);
+}
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 201581)
+++ doc/invoke.texi (working copy)
@@ -652,6 +652,7 @@ Objective-C and Objective-C++ Dialects}.
 -mbmi2 -mrtm -mlwp -mthreads @gol
 -mno-align-stringops  -minline-all-stringops @gol
 -minline-stringops-dynamically -mstringop-strategy=@var{alg} @gol
+-mmemcpy-strategy=@var{strategy} -mmemset-strategy=@var{strategy} 
 -mpush-args  -maccumulate-outgoing-args  -m128bit-long-double @gol
 -m96bit-long-double -mlong-double-64 -mlong-double-80 @gol
 -mregparm=@var{num}  -msseregparm @gol
@@ -14651,6 +14652,24 @@ Expand into an inline loop.
 Always use a library call.
 @end table
 
+@item -mmemcpy-strategy=@var{strategy}
+@opindex mmemcpy-strategy=@var{strategy}
+Override the internal decision heuristic to decide if @code{__builtin_memcpy}
+should be inlined and what inline algorithm to use when the expected size
+of the copy operation is known. @var{strategy} 
+is a comma-separated list of @var{alg}:@var{max_size}:@var{dest_align} 
triplets. 
+@var{alg} is specified in @option{-mstringop-strategy}, @var{max_size} 
specifies
+the max byte size with which inline algorithm @var{alg} is allowed.  For the 
last
+triplet, the @var{max_size} must be @code{-1}. The @var{max_size} of the 
triplets
+in the list must be specified in increasing order.  The minimal byte size for 
+@var{alg} is @code{0} for the first triplet and @code{@var{max_size} + 1} of 
the 
+preceding range.
+
+@item -mmemset-strategy=@var{strategy}
+@opindex mmemset-strategy=@var{strategy}
+The option is similar to @option{-mmemcpy-strategy=} except that it is to 
control
+@code{__builtin_memset} expansion.
+
 @item -momit-leaf-frame-pointer
 @opindex momit-leaf-frame-pointer
 Don't keep the frame pointer in a register for leaf functions.  This
Index: config/i386/stringop.def
===
--- config/i386/stringop.def(revision 0)
+++ config/i386/stringop.def(revision 0)
@@ -0,0 +1,37 @@
+/* Definitions for stringop strategy for IA-32.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the G

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

On Thu, Aug 8, 2013 at 5:55 AM, Ramana Radhakrishnan  wrote:
> On 08/06/13 22:39, Benjamin De Kosnik wrote:
>>
>>
 +# Filter out unsupported systems.
 +case "${target}" in
 +  x86_64-*-linux* | i?86-*-linux*)
 + VTV_SUPPORTED=yes
 + ;;
 +  powerpc*-*-linux*)
 + ;;
 +  sparc*-*-linux*)
 + ;;
 +  arm*-*-linux*)
 + ;;
>>>
>>>
>>> What about powerpc, sparc and arm?  Why are they mentioned here if no
>>> actual decision is made about support?
>>
>>
>> This is more a practical consideration: it's the middle of summer
>> break. Let's error on the side of caution for the moment, and get
>> this in causing minimal disruption on a convenient platform that I can
>> verify myself easily.
>>
>> Once this is in trunk, let a million flowers bloom! There is no
>> reason specific platform/target maintainers can't enable it at their
>> leisure on a per-setup manner and when they can verify testresults
>> easily.
>
>
>
> arm*-*-linux* is broken currently for what looks like the same reasons as
> the powerpc port.
>


Have you tried Benjamin Kosnik's patch?  Does it fix the problem?


> I spent some time this morning looking into what it would take to enable
> this for arm*-*-linux* today and the first thing that I noticed is that the
> testsuite is essentially driven by a shell script that caters to native
> testing on the only port that this was developed for. It also expects -m32
> and -m64 to be a standard option in all ports that want to turn this on.
> Also because the tests are not using dejagnu it's going to be a right pain
> to get this to work for cross-testing and unless that works reliably one
> isn't going to be able to get this feature working easily.
>
> Also whatever is target specific in the testsuite like environment-fail-32.s
> and environment-fail-64.s needs to live in it's own target folders.
> Basically this feature needs to follow conventions that exist already in
> other parts of the source base or atleast have plans to get there surely ?
>
> I haven't seen this in the reviews that I've seen so far, so apologies if
> this has already been raised.
>

I know that the testsuite, as committed, is not in DejaGnu format and
that it will need to be converted to DejaGnu format.  I also know that
it is currently specific to the platform on which the feature was
developed.  The plan has always been to fix this.  There was some
private discussion about whether it would be better to commit the
testsuite in its current state or not to commit a testsuite at all
with the initial project commit.  I was told that it would be better
to show that there is some way of testing this feature, even if the
testsuite is not in its final form, than to not show any way of
testing it at all.

I will work on getting the testsuite into a better form and also on
writing up some documentation on the platform-specific pieces of the
vtable verification feature (for those who wish to port it to other
platforms).

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

Hi,

>> In my humble opinion involving the whole std::bitset container for a
>> syntax option is way overkill.
>
>It's already used for match_flag_type anyway.

Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already 
have implementation experience about these flags, eg in iostream, and I dont 
think we want std::bitset everywhere.

Paolo

Paolo

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

Hi,

>So flag_type shall not be the same as size_t. I don't know if when I
>switch flag_type from unsigned int to, say, unsigned short, conflicts
>will appear in 16bit archtectures.

Well 16-bit archs aren't really supported these days, as long as the c++ 
runtime is concerned. Thus if unsigned short works let's go with it and move 
on. In any case, we don't have to make a super final decision, but breaking 32 
bit is bad.

In general, I would recommend being careful with adding templated containers, 
remember that eventually we want to have a lot of code in *.cc files and 
instantiations will be a big big annoyance.

Paolo

Re: [PATCH] Add vtable verification feature announcement to news on main page...

[Richard Earnshaw]

On 07/08/13 18:19, Caroline Tice wrote:
> As requested, here is the patch to announce the vtable verification
> feature on the main gcc.gnu.org web page.  Is this ok to commit?
> 

Please don't put this in until the issues that the patch have introduced
have been resolved.

R.

Re: Request to merge Undefined Behavior Sanitizer in

[Joseph S. Myers]

On Fri, 26 Jul 2013, Marc Glisse wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57324
> (that is using llvm's sanitizer)
> 
> and for a first patch (unreviewed):
> 
> http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01466.html
> (started at http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01402.html )

That patch is OK.  Of course we'll want problems that show up when the 
testsuite is run with a compiler built with sanitization enabled to be 
fixed, as well as those that show up in a simple build of the compiler and 
its libraries, but it makes sense to fix the latter first.  (And as the 
relevant functionality gets added to GCC's sanitizer, issues that show up 
in bootstrap with it enabled will need fixing as well.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [Patch] Whole regex refactoring and current status

[Andreas Schwab]

Paolo Carlini  writes:

> In my humble opinion involving the whole std::bitset container for a
> syntax option is way overkill.

It's already used for match_flag_type anyway.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH/Merge Request] Vtable Verification feature.

[Joseph S. Myers]

On Thu, 8 Aug 2013, Ramana Radhakrishnan wrote:

> I spent some time this morning looking into what it would take to enable this
> for arm*-*-linux* today and the first thing that I noticed is that the
> testsuite is essentially driven by a shell script that caters to native
> testing on the only port that this was developed for. It also expects -m32 and
> -m64 to be a standard option in all ports that want to turn this on. Also
> because the tests are not using dejagnu it's going to be a right pain to get
> this to work for cross-testing and unless that works reliably one isn't going
> to be able to get this feature working easily.

Clearly it's got to be converted to DejaGnu (and must avoid depending on a 
build directory at all, installed testing with runtest --tool libvtv, 
having created an appropriate site.exp, should work as well).  It's 
looking rather like this whole feature hasn't been adequately reviewed 
before commit

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA

[Joseph S. Myers]

On Thu, 8 Aug 2013, Ilya Enkovich wrote:

> > That is not a big issue to rename generic names. But I'm just still
> > trying to choose proper names. I looked into -fbounds-check but its
> > description already mention C/C++ and its semantics differs from what
> > new instrumentation does. I consider using -fcheck=pointer (currently
> > valid for Fortran) and 'chkp' instead of 'mpx' for generic things.
> > Does it look OK?
> I just realized that usage of option which is already defined for
> other languages may be problematic when this option is passed to
> MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers?

Seems reasonable to me.

> > I made an attempt to use multilibs instead. I tried to add mpx variant
> > to target libraries build but got fail for libgfortran build. Does
> > multilib support partial library rebuild? Actually I do not need
> > libgfortran library (an many other libraries) to be in mpx version. Is
> > it possible to get some libs from one place and some libs from another
> > place?

I'm not sure why the libgfortran build would have failed ... maybe some 
libraries don't in fact do anything with pointers for which the checks 
would help, but if so then I'd expect the option simply not to have any 
effect on the code generated for those libraries.  Multilibs are expected 
to be the same for all libraries (but packagers could no doubt optimize 
things in their packages, if in fact some libraries are identical when 
built both with and without MPX).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [Patch] Whole regex refactoring and current status

[Tim Shen]

On Thu, Aug 8, 2013 at 11:14 PM, Paolo Carlini  wrote:
> In my humble opinion involving the whole std::bitset container for a syntax 
> option is way overkill. Do you really have to do overloading between size_t 
> and that type? Or maybe you can use a type *smaller* than unsigned int.

The n3376 standard specified the following constructor:

...
explicit basic_regex(const charT* p, flag_type f = regex_constants::ECMAScript);
basic_regex(const charT* p, size_t len, flag_type f =
regex_constants::ECMAScript);
...

So flag_type shall not be the same as size_t. I don't know if when I
switch flag_type from unsigned int to, say, unsigned short, conflicts
will appear in 16bit archtectures.


-- 
Tim Shen

Re: New parameters to control stringop expansion libcall strategy

[Joseph S. Myers]

On Wed, 7 Aug 2013, Xinliang David Li wrote:

> Updated patch attached (fixed header, buffer overflow, and warning -->
> error problems).

You still have diagnostics starting with a capital letter, contrary to the 
GNU Coding Standards.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

Hi,

>There's a typedef in regex_constants.h:
>
>`typedef unsigned int syntax_option_type;`
>
>Which is a little bit naive. It possibly conflicts with size_t under
>i386 when overloading. I'm trying to change it to a bitset. Or is
>there any better solution?

In my humble opinion involving the whole std::bitset container for a syntax 
option is way overkill. Do you really have to do overloading between size_t and 
that type? Or maybe you can use a type *smaller* than unsigned int.

Paolo

Re: [Patch, Fortran] PR 58058: [4.7/4.8/4.9 Regression] Memory leak with transfer function

[Janus Weil]

ping!

2013/8/3 Janus Weil :
> Hi all,
>
> the attached patch plugs a memory leak of the TRANSFER intrinsic,
> which can occur when transferring to CHARACTER strings. For details
> see the PR.
>
> Regtested on x86_64-unknown-linux-gnu. Ok for trunk/4.8/4.7?
>
> Cheers,
> Janus
>
>
> 2013-08-03  Janus Weil  
>
> PR fortran/58058
> * trans-intrinsic.c (gfc_conv_intrinsic_transfer): Free the temporary
> string, if necessary.
>
> 2013-08-03  Janus Weil  
>
> PR fortran/58058
> * gfortran.dg/transfer_intrinsic_6.f90: New.

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Thu, 2013-08-08 10:43:53 -0400, David Malcolm  wrote:
> On Thu, 2013-08-08 at 16:36 +0200, Eric Botcazou wrote:
> > > This break Ada.
> > 
> > Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied.
> 
> Thanks; sorry again about the breakage.

Build Robot (http://toolchain.lug-owl.de/buildbot/) looks quite fine,
too. Thanks for fixing it that fast!

This was quite a major and intrusive change, so fallout is probably
somewhat expected.  It's probably even more a question of how it is
handled afterwards, and I guess this was quite fine.

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
Signature of:  What we do for ourselves dies with us. What we do for
the second  : others and the world remains and is immortal. (Albert 
Pine)


signature.asc
Description: Digital signature

[PATCH][ARM] FAIL: gcc.target/arm/pr58041.c scan-assembler ldrb

[Kyrylo Tkachov]

Hi all,

The recently added gcc.target/arm/pr58041.c test exposed a bug in the backend.
When compiling for NEON and with -mno-unaligned-access we end up generating
the vld1.64 and vst1.64 instructions instead of doing the accesses one byte at
a time like -mno-unaligned-access expects. This patch fixes that by enabling
the NEON expander and insns that produce these instructions only when
unaligned accesses are allowed.

Bootstrapped on arm-linux-gnueabihf. Tested arm-none-eabi on qemu.

Ok for trunk and 4.8?

Thanks,
Kyrill

2013-08-08  Kyrylo Tkachov  

* config/arm/neon.md (movmisalign): Disable when we
don't allow unaligned accesses.
(*movmisalign_neon_store): Likewise.
(*movmisalign_neon_load): Likewise.
(*movmisalign_neon_store): Likewise.
(*movmisalign_neon_load): Likewise.

neon_unaligned.patch
Description: Binary data

Re: [buildbot] r201508: Build failures after pass C++ conversion

[David Malcolm]

On Thu, 2013-08-08 at 16:36 +0200, Eric Botcazou wrote:
> > This break Ada.
> 
> Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied.

Thanks; sorry again about the breakage.

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Eric Botcazou]

> This break Ada.

Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied.


2013-08-08  Eric Botcazou  

* gcc-interface/Makefile.in (TOOLS_LIBS): Pick C object files from the
compiler build and use standard library variables.
(../../vxaddr2line$(exeext): Do not depend on targext.o and adjust.
(gnatmake-re): Do not depend on targext.o.
(gnatlink-re): Do not depend on link.o and targext.o.
(../../gnatmake$(exeext): Likewise.
(../../gnatlink$(exeext): Likewise.


-- 
Eric Botcazou
Index: gcc-interface/Makefile.in
===
--- gcc-interface/Makefile.in	(revision 201177)
+++ gcc-interface/Makefile.in	(working copy)
@@ -250,10 +250,9 @@ LIBS = $(LIBINTL) $(LIBICONV) $(LIBBACKT
 LIBDEPS = $(LIBINTL_DEP) $(LIBICONV_DEP) $(LIBBACKTRACE) $(LIBIBERTY)
 # Default is no TGT_LIB; one might be passed down or something
 TGT_LIB =
-TOOLS_LIBS = targext.o link.o ../../ggc-none.o ../../libcommon-target.a \
+TOOLS_LIBS = ../link.o ../targext.o ../../ggc-none.o ../../libcommon-target.a \
   ../../libcommon.a ../../../libcpp/libcpp.a $(LIBGNAT) $(LIBINTL) $(LIBICONV) \
-  ../../../libbacktrace/.libs/libbacktrace.a ../../../libiberty/libiberty.a \
-  $(SYSLIBS) $(TGT_LIB)
+  ../$(LIBBACKTRACE) ../$(LIBIBERTY) $(SYSLIBS) $(TGT_LIB)
 
 # Convert the target variable into a space separated list of architecture,
 # manufacturer, and operating system and assign each of those to its own
@@ -2491,12 +2490,12 @@ common-tools: ../stamp-tools
 	$(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatdll
 	$(GNATLINK) -v gnatdll -o $@ --GCC="$(GCC_LINK)" $(TOOLS_LIBS)
 
-../../vxaddr2line$(exeext): ../stamp-tools targext.o
+../../vxaddr2line$(exeext): ../stamp-tools
 	$(GNATMAKE) -c  $(ADA_INCLUDES) vxaddr2line --GCC="$(CC) $(ALL_ADAFLAGS)"
 	$(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) vxaddr2line
-	$(GNATLINK) -v vxaddr2line -o $@ --GCC="$(GCC_LINK)" targext.o $(CLIB)
+	$(GNATLINK) -v vxaddr2line -o $@ --GCC="$(GCC_LINK)" ../targext.o $(CLIB)
 
-gnatmake-re: ../stamp-tools link.o targext.o
+gnatmake-re: ../stamp-tools
 	$(GNATMAKE) -j0 $(ADA_INCLUDES) -u sdefault --GCC="$(CC) $(MOST_ADA_FLAGS)"
 	$(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatmake --GCC="$(CC) $(ALL_ADAFLAGS)"
 	$(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatmake
@@ -2507,7 +2506,7 @@ gnatmake-re: ../stamp-tools link.o targe
 # with the former version of gnatlink itself which cannot override itself.
 # gnatlink-re cannot be run at the same time as gnatmake-re, hence the
 # dependency
-gnatlink-re: ../stamp-tools link.o targext.o gnatmake-re
+gnatlink-re: ../stamp-tools gnatmake-re
 	$(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatlink --GCC="$(CC) $(ALL_ADAFLAGS)"
 	$(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatlink
 	$(GNATLINK) -v gnatlink -o ../../gnatlinknew$(exeext) \
@@ -2519,11 +2518,11 @@ gnatlink-re: ../stamp-tools link.o targe
 #  stamp target in the parent directory whenever gnat1 is rebuilt
 
 # Likewise for the tools
-../../gnatmake$(exeext): $(P) b_gnatm.o link.o targext.o $(GNATMAKE_OBJS)
+../../gnatmake$(exeext): $(P) b_gnatm.o $(GNATMAKE_OBJS)
 	+$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatm.o $(GNATMAKE_OBJS) \
 		$(TOOLS_LIBS)
 
-../../gnatlink$(exeext): $(P) b_gnatl.o link.o targext.o $(GNATLINK_OBJS)
+../../gnatlink$(exeext): $(P) b_gnatl.o $(GNATLINK_OBJS)
 	+$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatl.o $(GNATLINK_OBJS) \
 		$(TOOLS_LIBS)
 

Symtab cleanups 10/17 - ipa ref verifier

[Jan Hubicka]

Hi,
this patch implements (very basic) IPA REF verifier. It only checks if all 
references
corresopnds to a real statement. It does not yet check if all statements have 
proper
references attached to them. 
Even this simple testing found quite few positives where we leave stale 
references in
callgraph. Fixed thus.

Bootstrapped/regtested x86_64-linux, commited. Hope there won't be too much of 
other
positives ;)

* cgraphbuild.c (build_cgraph_edges): Do not walk into debugs.
(make_pass_rebuild_cgraph_edges): Also clear references.
* cgraph.c (verify_cgraph_node): Add basic ipa-ref verifier.
* ipa-inline-transform.c (inline_transform): Remove all references
after inlining.
* cgraphunit.c (expand_function): Remove all references after expansion.
* ipa-ref.c (ipa_ref_has_aliases_p): Fix formatting.
(ipa_find_reference): Rewrite to iterator.
(remove_stmt_references): Likewise.
(ipa_clear_stmts_in_references): New function.
* ipa-ref.h (ipa_clear_stmts_in_references): Declare.
* cgraphclones.c (cgraph_materialize_all_clones): Remove or clear 
references.
* ipa-split.c (split_function): Remove references in split function.
Index: cgraphbuild.c
===
*** cgraphbuild.c   (revision 201568)
--- cgraphbuild.c   (working copy)
*** build_cgraph_edges (void)
*** 318,323 
--- 318,326 
  gimple stmt = gsi_stmt (gsi);
  tree decl;
  
+ if (is_gimple_debug (stmt))
+   continue;
+ 
  if (is_gimple_call (stmt))
{
  int freq = compute_call_stmt_bb_frequency (current_function_decl,
*** make_pass_rebuild_cgraph_edges (gcc::con
*** 537,543 
  static unsigned int
  remove_cgraph_callee_edges (void)
  {
!   cgraph_node_remove_callees (cgraph_get_node (current_function_decl));
return 0;
  }
  
--- 540,548 
  static unsigned int
  remove_cgraph_callee_edges (void)
  {
!   struct cgraph_node *node = cgraph_get_node (current_function_decl);
!   cgraph_node_remove_callees (node);
!   ipa_remove_all_references (&node->symbol.ref_list);
return 0;
  }
  
Index: cgraph.c
===
*** cgraph.c(revision 201568)
--- cgraph.c(working copy)
*** verify_cgraph_node (struct cgraph_node *
*** 2537,2591 
  {
if (this_cfun->cfg)
{
  /* Reach the trees by walking over the CFG, and note the
 enclosing basic-blocks in the call edges.  */
  FOR_EACH_BB_FN (this_block, this_cfun)
!   for (gsi = gsi_start_bb (this_block);
!  !gsi_end_p (gsi);
!  gsi_next (&gsi))
! {
!   gimple stmt = gsi_stmt (gsi);
!   if (is_gimple_call (stmt))
! {
!   struct cgraph_edge *e = cgraph_edge (node, stmt);
!   tree decl = gimple_call_fndecl (stmt);
!   if (e)
! {
!   if (e->aux)
! {
!   error ("shared call_stmt:");
!   cgraph_debug_gimple_stmt (this_cfun, stmt);
!   error_found = true;
! }
!   if (!e->indirect_unknown_callee)
! {
!   if (verify_edge_corresponds_to_fndecl (e, decl))
! {
!   error ("edge points to wrong declaration:");
!   debug_tree (e->callee->symbol.decl);
!   fprintf (stderr," Instead of:");
!   debug_tree (decl);
!   error_found = true;
! }
! }
!   else if (decl)
! {
!   error ("an indirect edge with unknown callee "
!  "corresponding to a call_stmt with "
!  "a known declaration:");
!   error_found = true;
!   cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
! }
!   e->aux = (void *)1;
! }
!   else if (decl)
! {
!   error ("missing callgraph edge for call stmt:");
!   cgraph_debug_gimple_stmt (this_cfun, stmt);
!   error_found = true;
! }
! }
  }
}
else
/* No CFG available?!  */
--- 2537,2611 
  {
if (this_cfun->cfg)
{
+ pointer_set_t *stmts = pointer_set_create ();
+ int i;
+ struct ipa_ref *ref;
+ 
  /* Re

Re: [Patch] Whole regex refactoring and current status

[Tim Shen]

On Thu, Aug 8, 2013 at 10:04 PM, Tim Shen  wrote:
> On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini  
> wrote:
>> Tim, please address this ASAP, otherwise we have to revert the whole thing.
>
> I'm trying to reproduce the compilation failures.

There's a typedef in regex_constants.h:

`typedef unsigned int syntax_option_type;`

Which is a little bit naive. It possibly conflicts with size_t under
i386 when overloading. I'm trying to change it to a bitset. Or is
there any better solution?


-- 
Tim Shen

Re: [Patch] Whole regex refactoring and current status

[Tim Shen]

On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini  wrote:
> Tim, please address this ASAP, otherwise we have to revert the whole thing.

I'm trying to reproduce the compilation failures.


-- 
Tim Shen

[PATCH, ARM] Fix ICE when using Neon vld1_dup_[su]64()

[Richard Earnshaw]

PR 57431 is an ICE when compiling for Neon intrinsics.  For completeness
vld1_dup_u64 is defined to fill a single element vector from a single
element of data, but this is in reality degenerate.
Unfortunately, by expanding this into a vec_duplicate operation we
confuse the mid-end of the compiler, since it expects a vector mode to
result from the duplicate operation.

The fix is to special-case the expand into the appropriate direct load
operation and to then simplify the logic for the remaining cases (which
now really are all duplicate operations).

PR target/57431
* arm/neon.md (neon_vld1_dupdi): New expand pattern.
(neon_vld1_dup VD iterator): Iterate over VD not VDX.

R.--- neon.md (revision 201427)
+++ neon.md (local)
@@ -4593,19 +4593,20 @@ (define_insn "neon_vld1_lane"
 )
 
 (define_insn "neon_vld1_dup"
-  [(set (match_operand:VDX 0 "s_register_operand" "=w")
-(vec_duplicate:VDX (match_operand: 1 "neon_struct_operand" 
"Um")))]
+  [(set (match_operand:VD 0 "s_register_operand" "=w")
+(vec_duplicate:VD (match_operand: 1 "neon_struct_operand" 
"Um")))]
   "TARGET_NEON"
-{
-  if (GET_MODE_NUNITS (mode) > 1)
-return "vld1.\t{%P0[]}, %A1";
-  else
-return "vld1.\t%h0, %A1";
-}
-  [(set (attr "neon_type")
-  (if_then_else (gt (const_string "") (const_string "1"))
-(const_string "neon_vld2_2_regs_vld1_vld2_all_lanes")
-(const_string "neon_vld1_1_2_regs")))]
+  "vld1.\t{%P0[]}, %A1"
+  [(set_attr "neon_type" "neon_vld2_2_regs_vld1_vld2_all_lanes")]
+)
+
+;; Special case for DImode.  Treat it exactly like a simple load.
+(define_expand "neon_vld1_dupdi"
+  [(set (match_operand:DI 0 "s_register_operand" "")
+(unspec:DI [(match_operand:DI 1 "neon_struct_operand" "")]
+  UNSPEC_VLD1))]
+  "TARGET_NEON"
+  ""
 )
 
 (define_insn "neon_vld1_dup"

[PATCH, ARM] Fix handling of function arguments with excess alignment

[Richard Earnshaw]

PR target/56979 is a bug where a parameter to a function has an
alignment that is larger than its natural alignment.  In this case this
causes the mid-end to generate a mode for the argument that is
incompatible with the registers that are assigned for it.  We then end
up creating invalid RTL and subsequently abort when the pattern cannot
emit assembly code.

The fix is to decompose the assignment when this would happen in the
same way that we handle other block mode arguments and handle each piece
in turn.

PR target/56979
* arm.c (aapcs_vfp_allocate): Decompose the argument if the
suggested mode for the assignment isn't compatible with the
registers required.

Committed to trunk.

R.--- arm.c   (revision 201547)
+++ arm.c   (local)
@@ -4544,7 +4544,9 @@ aapcs_vfp_allocate (CUMULATIVE_ARGS *pcu
 if (((pcum->aapcs_vfp_regs_free >> regno) & mask) == mask)
   {
pcum->aapcs_vfp_reg_alloc = mask << regno;
-   if (mode == BLKmode || (mode == TImode && !TARGET_NEON))
+   if (mode == BLKmode
+   || (mode == TImode && !TARGET_NEON)
+   || ! arm_hard_regno_mode_ok (FIRST_VFP_REGNUM + regno, mode))
  {
int i;
int rcount = pcum->aapcs_vfp_rcount;

RE: [Patch] Whole regex refactoring and current status

[Kyrylo Tkachov]

> Hi Paolo,
> 
> >>This is already documented:
> >>
> >>http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#c
> oding_style.bad_identifiers
> >
> > Indeed. As a simple to remember rule never use single underscore +
> single
> > uppercase. Usually we add a p, like _Cp, etc. Plenty of examples
> > everywhere. At the moment I can't do the search&replace, any such patch
> is
> > preapproved though.
> 
> I wasn't certain about the right convention.  The following patch
> allowed bootstrap to finish on i386-pc-solaris2.10 and
> x86_64-unknown-linux-gnu.  I'll commit the patch once Solaris testing
> has finished.

I can confirm that this patch fixes the arm-none-eabi build as well.

Thanks,
Kyrill

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ramana Radhakrishnan]

On 08/06/13 22:39, Benjamin De Kosnik wrote:



+# Filter out unsupported systems.
+case "${target}" in
+  x86_64-*-linux* | i?86-*-linux*)
+ VTV_SUPPORTED=yes
+ ;;
+  powerpc*-*-linux*)
+ ;;
+  sparc*-*-linux*)
+ ;;
+  arm*-*-linux*)
+ ;;


What about powerpc, sparc and arm?  Why are they mentioned here if no
actual decision is made about support?


This is more a practical consideration: it's the middle of summer
break. Let's error on the side of caution for the moment, and get
this in causing minimal disruption on a convenient platform that I can
verify myself easily.

Once this is in trunk, let a million flowers bloom! There is no
reason specific platform/target maintainers can't enable it at their
leisure on a per-setup manner and when they can verify testresults
easily.



arm*-*-linux* is broken currently for what looks like the same reasons 
as the powerpc port.


I spent some time this morning looking into what it would take to enable 
this for arm*-*-linux* today and the first thing that I noticed is that 
the testsuite is essentially driven by a shell script that caters to 
native testing on the only port that this was developed for. It also 
expects -m32 and -m64 to be a standard option in all ports that want to 
turn this on. Also because the tests are not using dejagnu it's going to 
be a right pain to get this to work for cross-testing and unless that 
works reliably one isn't going to be able to get this feature working 
easily.


Also whatever is target specific in the testsuite like 
environment-fail-32.s and environment-fail-64.s needs to live in it's 
own target folders. Basically this feature needs to follow conventions 
that exist already in other parts of the source base or atleast have 
plans to get there surely ?


I haven't seen this in the reviews that I've seen so far, so apologies 
if this has already been raised.


regards
Ramana

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

Hi,

>I wasn't certain about the right convention.  The following patch
>allowed bootstrap to finish on i386-pc-solaris2.10 and
>x86_64-unknown-linux-gnu.  I'll commit the patch once Solaris testing
>has finished.

Thanks a lot.

>E.g. the first one is
>
>FAIL: 28_regex/algorithms/regex_match/basic/string_01.cc (test for
>excess errors)
>Excess errors:
>/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/basic/string_01.cc:34:47:
>error: call of overloaded 'basic_regex(const char [8], const
>flag_type&)' is ambiguous
>
>The other failures are similar.

Tim, please address this ASAP, otherwise we have to revert the whole thing.

Paolo

Re: [Patch] Whole regex refactoring and current status

[Rainer Orth]

Hi Paolo,

>>This is already documented:
>>
>>http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers
>
> Indeed. As a simple to remember rule never use single underscore + single
> uppercase. Usually we add a p, like _Cp, etc. Plenty of examples
> everywhere. At the moment I can't do the search&replace, any such patch is
> preapproved though.

I wasn't certain about the right convention.  The following patch
allowed bootstrap to finish on i386-pc-solaris2.10 and
x86_64-unknown-linux-gnu.  I'll commit the patch once Solaris testing
has finished.

2013-08-08  Rainer Orth  

* include/bits/regex.h: Replace _A, _B, _C, _R by _Ap, _Bp,
_Cp, _Rp.

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -995,18 +995,18 @@ namespace std _GLIBCXX_VISIBILITY(defaul
  const basic_regex<_CharT, _TraitsT>&,
  regex_constants::match_flag_type);
 
-  template
+  template
 friend bool
-regex_match(_B, _B,
-match_results<_B, _A>&,
-const basic_regex<_C, _R>&,
+regex_match(_Bp, _Bp,
+match_results<_Bp, _Ap>&,
+const basic_regex<_Cp, _Rp>&,
 regex_constants::match_flag_type);
 
-  template
+  template
 friend bool
-regex_search(_B, _B,
- match_results<_B, _A>&,
- const basic_regex<_C, _R>&,
+regex_search(_Bp, _Bp,
+ match_results<_Bp, _Ap>&,
+ const basic_regex<_Cp, _Rp>&,
  regex_constants::match_flag_type);
 
   flag_type _M_flags;
@@ -2111,16 +2111,16 @@ namespace std _GLIBCXX_VISIBILITY(defaul
   template
 friend class __detail::_BFSExecutor;
 
-  template
+  template
 friend bool
-regex_match(_B, _B, match_results<_B, _A>&,
+regex_match(_Bp, _Bp, match_results<_Bp, _Ap>&,
 const basic_regex<_Ch_type,
 _Rx_traits>&,
 regex_constants::match_flag_type);
 
-  template
+  template
 friend bool
-regex_search(_B, _B, match_results<_B, _A>&,
+regex_search(_Bp, _Bp, match_results<_Bp, _Ap>&,
  const basic_regex<_Ch_type,
  _Rx_traits>&,
  regex_constants::match_flag_type);


However, I see many 32-bit testsuite failures, both on Solaris and
Linux:

Running target unix/-m32
FAIL: 28_regex/algorithms/regex_match/basic/string_01.cc (test for excess 
errors)
WARNING: 28_regex/algorithms/regex_match/basic/string_01.cc compilation failed 
to produce executable
FAIL: 28_regex/algorithms/regex_match/basic/string_range_00_03.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_match/basic/string_range_00_03.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/basic/string_range_01_03.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_match/basic/string_range_01_03.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/basic/string_range_02_03.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_match/basic/string_range_02_03.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/53622.cc (test for excess errors)
WARNING: 28_regex/algorithms/regex_match/extended/53622.cc compilation failed 
to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/57173.cc (test for excess errors)
WARNING: 28_regex/algorithms/regex_match/extended/57173.cc compilation failed 
to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/cstring_bracket_01.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_match/extended/cstring_bracket_01.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/cstring_plus.cc (test for excess 
errors)
WARNING: 28_regex/algorithms/regex_match/extended/cstring_plus.cc compilation 
failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/cstring_questionmark.cc (test 
for excess errors)
WARNING: 28_regex/algorithms/regex_match/extended/cstring_questionmark.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/string_any.cc (test for excess 
errors)
WARNING: 28_regex/algorithms/regex_match/extended/string_any.cc compilation 
failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/string_range_00_03.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_match/extended/string_range_00_03.cc 
compilation failed to produce executable
FAIL: 28_regex/algorithms/regex_match/extended/string_range_01_03.cc (test for 
excess errors)
WARNING: 28_regex/algorithms/regex_mat

Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA

[Ilya Enkovich]

2013/8/8 Ilya Enkovich :
> 2013/8/8 Joseph S. Myers :
>> On Fri, 2 Aug 2013, Ilya Enkovich wrote:
>>
>>> Hi All,
>>>
>>> I've updated MPX Wiki page
>>> (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler).
>>> I added instrumentation description, programming model description,
>>> differences with other checkers, implementation details.
>>
>> Thanks.  As noted, there should be a clean separation between what's
>> generic and what's architecture-specific - the generic command-line
>> options, hooks etc. shouldn't mention "MPX" in their names.
>
> That is not a big issue to rename generic names. But I'm just still
> trying to choose proper names. I looked into -fbounds-check but its
> description already mention C/C++ and its semantics differs from what
> new instrumentation does. I consider using -fcheck=pointer (currently
> valid for Fortran) and 'chkp' instead of 'mpx' for generic things.
> Does it look OK?
I just realized that usage of option which is already defined for
other languages may be problematic when this option is passed to
MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers?

Thanks,
Ilya
>
>>
>> I'm unclear on the references to *_nobnd and *_nochk functions - are there
>> corresponding library (glibc etc.) changes to add additional function
>> variants?  Are all built-in functions that use pointers modified so that
>> GCC will insert the required checks when expanding inline?
> The idea was to add new versions of some functions into glibc and
> replace normal versions call with special versions call when possible
> (e.g. when we know memcpy call does not copy pointers). Currently in
> MPX mode expanding is allowed for _nochk_nobnd variant which is equal
> to regular call. I cannot be sure new version will be added into
> glibc. If not, we would probably create additional library in GCC.
>
>>
>> I'm also unclear on how much --enable-mpx does - in general, it's
>> desirable for a single compiler to be able to generate binaries both with
>> and without the checks, and so quite possibly to build libgcc, libstdc++
>> etc. as multilibs both with and without MPX, rather than needing to make a
>> static decision when GCC is built (so, any configure option should
>> preferably be about building *extra* library variants, for example, rather
>> than changing the options with which existing variants are built).
> Currently (in public mpx branch) --enable-mpx just adds compilation
> options to some libraries.
>
> I made an attempt to use multilibs instead. I tried to add mpx variant
> to target libraries build but got fail for libgfortran build. Does
> multilib support partial library rebuild? Actually I do not need
> libgfortran library (an many other libraries) to be in mpx version. Is
> it possible to get some libs from one place and some libs from another
> place?
>
> Thanks,
> Ilya
>
>>
>> --
>> Joseph S. Myers
>> jos...@codesourcery.com

Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)

[Aldy Hernandez]

+  hash_table  simduid_to_vf_htab;
+  hash_table  decl_to_simduid_htab;


Why two hashes?  Seems like you can map from decl to vf directly.  At what
point do you have a simduid integer, but not the decl from whence it came?


decl_to_simduid seems to be a misnomer.  What it really is, is a 
"omp_simd_array_to_simduid" map.  OMP simd array, being the temporary 
array used to hold simd lane private variable copies and such.


The hash is not really mapping the simd DECL to the simduid, since 
that's just a matter of DECL_UID(simduid), but the OMP simd array to the 
index used to reference it (simduid), like thus:


_7 = GOMP_SIMD_LANE (simduid.0)
...
...
D.1737[_7] = stuff;

decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP 
simd array).


Agreed, or am I missing something?

Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA

[Ilya Enkovich]

2013/8/8 Joseph S. Myers :
> On Fri, 2 Aug 2013, Ilya Enkovich wrote:
>
>> Hi All,
>>
>> I've updated MPX Wiki page
>> (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler).
>> I added instrumentation description, programming model description,
>> differences with other checkers, implementation details.
>
> Thanks.  As noted, there should be a clean separation between what's
> generic and what's architecture-specific - the generic command-line
> options, hooks etc. shouldn't mention "MPX" in their names.

That is not a big issue to rename generic names. But I'm just still
trying to choose proper names. I looked into -fbounds-check but its
description already mention C/C++ and its semantics differs from what
new instrumentation does. I consider using -fcheck=pointer (currently
valid for Fortran) and 'chkp' instead of 'mpx' for generic things.
Does it look OK?

>
> I'm unclear on the references to *_nobnd and *_nochk functions - are there
> corresponding library (glibc etc.) changes to add additional function
> variants?  Are all built-in functions that use pointers modified so that
> GCC will insert the required checks when expanding inline?
The idea was to add new versions of some functions into glibc and
replace normal versions call with special versions call when possible
(e.g. when we know memcpy call does not copy pointers). Currently in
MPX mode expanding is allowed for _nochk_nobnd variant which is equal
to regular call. I cannot be sure new version will be added into
glibc. If not, we would probably create additional library in GCC.

>
> I'm also unclear on how much --enable-mpx does - in general, it's
> desirable for a single compiler to be able to generate binaries both with
> and without the checks, and so quite possibly to build libgcc, libstdc++
> etc. as multilibs both with and without MPX, rather than needing to make a
> static decision when GCC is built (so, any configure option should
> preferably be about building *extra* library variants, for example, rather
> than changing the options with which existing variants are built).
Currently (in public mpx branch) --enable-mpx just adds compilation
options to some libraries.

I made an attempt to use multilibs instead. I tried to add mpx variant
to target libraries build but got fail for libgfortran build. Does
multilib support partial library rebuild? Actually I do not need
libgfortran library (an many other libraries) to be in mpx version. Is
it possible to get some libs from one place and some libs from another
place?

Thanks,
Ilya

>
> --
> Joseph S. Myers
> jos...@codesourcery.com

Re: [PATCH/Merge Request] Vtable Verification feature.

[Iain Sandoe]

Hi Caroline,

On 8 Aug 2013, at 11:27, Rainer Orth wrote:
> 
> As an aside, I had a very quick look at libvtv to determine what's
> required for a port to a non-Linux platform.

Likewise..

>  It would be good if the
> requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly
> several more) could be documented to ease porters' task.

+1

Also, in general, it would be very helpful if folks introducing new named 
sections would use a target hook to fetch them - since section naming/flag 
conventions are different between (at least) elf and mach-o.

thanks
Iain

Re: [PATCH/Merge Request] Vtable Verification feature.

[Rainer Orth]

Caroline,

your libgcc ChangeLog entries are all broken: they lack the initial "* "
as can easily be seen in Emacs' Change Log Mode.

Please fix.

As an aside, I had a very quick look at libvtv to determine what's
required for a port to a non-Linux platform.  It would be good if the
requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly
several more) could be documented to ease porters' task.

Thanks.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch] Whole regex refactoring and current status

[Paolo Carlini]

Hi,

>This is already documented:
>
>http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers

Indeed. As a simple to remember rule never use single underscore + single 
uppercase. Usually we add a p, like _Cp, etc. Plenty of examples everywhere. At 
the moment I can't do the search&replace, any such patch is preapproved though.

Thanks,
Paolo

RE: [Patch] Whole regex refactoring and current status

[Kyrylo Tkachov]

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Rainer Orth
> Sent: 08 August 2013 11:02
> To: Tim Shen
> Cc: libstdc++; gcc-patches
> Subject: Re: [Patch] Whole regex refactoring and current status
> 
> Tim Shen  writes:
> 
> > As metioned in this[1] email, I here propose a refactored version of
> > regex, and show the status:
> 
> This patch broke Solaris bootstrap:
> 
> In file included from /usr/include/ctype.h:18:0,
>  from /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-
> solaris2.10/libstdc++-v3/include/cctype:42,
>  from /vol/gcc/src/hg/trunk/local/libstdc++-
> v3/include/precompiled/stdc++.h:35:
> /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++-
> v3/include/bits/regex.h:998:25: error: expected nested-name-specifier
> before numeric constant
>template
>  ^
> 
> and several more instances.  This happens because  (via
> ) has:
> 
> #define   _B  0x0040  /* Blank */
> #define   _C  0x0020  /* Control character */
> 
> This is already documented:
> 
> http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#cod
> ing_style.bad_identifiers

We're also seeing the same error for ARM builds.

Kyrill

Re: [Patch] Whole regex refactoring and current status

[Rainer Orth]

Tim Shen  writes:

> As metioned in this[1] email, I here propose a refactored version of
> regex, and show the status:

This patch broke Solaris bootstrap:

In file included from /usr/include/ctype.h:18:0,
 from 
/var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++-v3/include/cctype:42,
 from 
/vol/gcc/src/hg/trunk/local/libstdc++-v3/include/precompiled/stdc++.h:35:
/var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++-v3/include/bits/regex.h:998:25:
 error: expected nested-name-specifier before numeric constant
   template
 ^

and several more instances.  This happens because  (via
) has:

#define _B  0x0040  /* Blank */
#define _C  0x0020  /* Control character */

This is already documented:

http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers

Please fix.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Eric Botcazou]

> Sorry.  How does the attached look?  (am bootstrapping now)

Thanks for devising the patch.  However, we are in the process of fixing the 
issue on the Ada side so please do not apply it for now.

-- 
Eric Botcazou

Re: [PATCH][tree-optimization] Fix PR58088

[Eric Botcazou]

> Also, the ChangeLog entries should be:
> 
> 2013-08-08  Kyrylo Tkachov  
> 
>   PR tree-optimization/58088
>   * gcc/fold-const.c (mask_with_trailing_zeros): New function.
>   (fold_binary_loc): Make sure we don't recurse infinitely
>   when the X in (X & C1) | C2 is a tree of the form (Y * K1) & K2.
>   Use mask_with_trailing_zeros where appropriate.

Without gcc/ prefix, paths are relative to the directory where ChangeLog lives.

> 2013-08-08  Kyrylo Tkachov  
> 
>   PR tree-optimization/58088
>   * gcc.c-torture/compile/pr58088.c: New test.

This one is good.

-- 
Eric Botcazou

Re: [PATCH, ARM] Fix unrecognizable vector comparisons

[Ramana Radhakrishnan]

On 08/01/13 03:04, Zhenqiang Chen wrote:

Thank you all for the comments. The patch is updated as:
1) Revert it to the original one.
2) For the testcase, replace the dg-options with
/* { dg-do compile } */
/* { dg-require-effective-target arm_neon } */
/* { dg-add-options arm_neon } */
/* { dg-options "-O3" } */

Bootstrap on Chromebook and Pandaboard.
No make check regression on Pandaboard.

Thanks!
-Zhenqiang


Ok for trunk and 4.8 branch.

Sorry about the delay.

Thanks,
Ramana

[Ping] Re: [PATCH, ARM] Fix unrecognizable vector comparisons

[Zhenqiang Chen]

Ping? Is it OK for 4.8 and trunk?

Thanks!
-Zhenqiang

On 1 August 2013 10:04, Zhenqiang Chen  wrote:
> Thank you all for the comments. The patch is updated as:
> 1) Revert it to the original one.
> 2) For the testcase, replace the dg-options with
> /* { dg-do compile } */
> /* { dg-require-effective-target arm_neon } */
> /* { dg-add-options arm_neon } */
> /* { dg-options "-O3" } */
>
> Bootstrap on Chromebook and Pandaboard.
> No make check regression on Pandaboard.
>
> Thanks!
> -Zhenqiang
>
> On 9 July 2013 02:49, Jakub Jelinek  wrote:
>> On Mon, Jul 08, 2013 at 11:44:04AM -0700, Janis Johnson wrote:
>>> >> @@ -0,0 +1,16 @@
>>> >> +/* { dg-do compile } */
>>> >> +/* { dg-options "-O3 -mfpu=neon -mcpu=cortex-a9 -mthumb
>>> >> -mfloat-abi=hard -S" } */
>>> >>
>>> >> dg-add-options arm_neon ?
>>> >> dg-require-effective-target arm_neon ?
>>> >
>>> > I will update it.
>>>
>>> Please skip the test for multilibs whose flags include -mfpu or -mcpu 
>>> options,
>>> which would conflict with or override the options in the test.
>>
>> Also the -S in dg-options looks wrong.  That should be derived from dg-do.
>>
>> Jakub

RE: [PATCH][tree-optimization] Fix PR58088

[Kyrylo Tkachov]

> Ok for trunk?
> 
> Bootstrapped on x86_64-linux-gnu and tested arm-none-eabi on qemu.
> 
> 
> Thanks,
> Kyrill
>   * gcc.c-torture/compile/pr58088.c: New test.

Also, the ChangeLog entries should be:

2013-08-08  Kyrylo Tkachov  

PR tree-optimization/58088
* gcc/fold-const.c (mask_with_trailing_zeros): New function.
(fold_binary_loc): Make sure we don't recurse infinitely
when the X in (X & C1) | C2 is a tree of the form (Y * K1) & K2.
Use mask_with_trailing_zeros where appropriate.


2013-08-08  Kyrylo Tkachov  

PR tree-optimization/58088
* gcc.c-torture/compile/pr58088.c: New test.

[PING][PATCH ARM]Extend thumb1_reorg to save more comparison instructions

[bin.cheng]

Ping this patch, http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01057.html

Thanks.
bin

Re: [PATCH] target/58065 ARM MALLOC_ABI_ALIGNMENT is wrong

[Ramana Radhakrishnan]

On 08/07/13 08:10, Bernd Edlinger wrote:

Hello,

in the discussion about the PR middle-end/57748 it became obvious that the ARM 
target architecture should define a value for MALLOC_ABI_ALIGNMENT, because 
otherwise the default is simply word aligned, which causes sub-optimal code, at 
least for the neon fpu.

This simple patch fixes PR target/58065 by defining the MALLOC_ABI_ALIGNMENT as 
BIGGEST_ALIGNMNET.

As a proof that this has indeed some subtle influence on the generated code
I created a new test case: The function foo is called by bar, and bar uses
malloc to allocate the memory, with compiler options "-O3 -g0 -mfpu=neon
-mfloat-abi=softfp" the function foo is inlined into bar,
but the inlined version does not use vstr instructions any more, because
the front-end does assume that malloc returns 4 byte aligned memory.



You don't mention whether this patch was regression tested. All patches 
must be regression tested. I've applied only the changes to the source 
after testing it overnight with a couple of my other patches. Read 
further on about why this testcase is not suitable and how you could 
make this better for next time.


For next time please read the contribution web page with respect to 
testing here


http://gcc.gnu.org/contribute.html




Regards
Bernd Edlinger  




Now moving on to the patch itself -

The testcase doesn't work because the vstr's that come out aren't 
because the alignment information doesn't match up rather than it being 
because the compiler has decided to use immediate offset addressing on 
storing a vector. Once you do that for a 128 bit vector there is no 
option but for the compiler to put out 2 vstr instructions which happens 
even after your patch for me.



+++ gcc/testsuite/gcc.target/arm/pr58065.c  2013-08-03 00:08:22.0 
+0200
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -g0 -mfpu=neon -mfloat-abi=softfp" } */


For next time please use dg-add-options arm_neon for the -mfpu=neon 
-mfloat-abi=softfp bits. You also shouldn't need -g0 here.


Instead I'd scan an intermedate rtl dump for alignment information 
rather than anything else. I haven't yet looked at the output but 
looking for A64 in an RTL dump instead of A32 might help in this 
particular case.


Also reading http://gcc.gnu.org/wiki/TestCaseWriting will be useful with 
respect to creating new testcases.




+/* { dg-final { scan-assembler-not "\[^v]str" } } */
+
+#include 
+
+typedef long long V
+  __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
+
+struct T { V a; V b[1]; };
+
+void
+foo (struct T *p)
+{
+  V a = { 3, 4 };
+  p->b[0] = a;
+}
+
+struct T *
+bar ()
+{
+  struct T *t = (struct T *) malloc (sizeof(*t));
+  foo (t);
+  return t;
+}


Please post the changelog inline in the future.

> 2013-08-07  Bernd Edlinger  
>
>PR target/58065
>Set MALLOC_ABI_ALIGNMENT to BIGGEST_ALIGNMENT for ARM.
>
>* gcc/config/arm/arm.h: Define MALLOC_ABI_ALIGNMENT.
>* gcc/testsuite/gcc.target/arm/pr58065.c: New testcase.

Also for next time there should be a changelog entry for each ChangeLog 
file you need to touch in this case with the file names in the changelog 
entry rooted at the file in which the ChangeLog file is present. In this 
particular case you need one for the source base and one for the 
testsuite and drop the gcc/ and gcc/testsuite prefixes to the file names 
below. Also notice how I've changed the Changelog to actually refer to 
MALLOC_ABI_ALIGNMENT rather than stating what's changed for arm.h.



Therefore this should read


2013-08-08  Bernd Edlinger  

PR target/58065
* config/arm/arm.h (MALLOC_ABI_ALIGNMENT): New,


2013-08-08  Bernd Edlinger  

PR target/58065
* gcc.target/arm/pr58065.c: New testcase.

This makes it easier for the person applying the patch to copy paste the 
Changelog into the relevant file. Thanks :)


And finally thank you for submitting the patch, if you intend on 
contributing to GCC in the medium to longer term, it would be worth 
getting the process for obtaining a copyright assignment sorted if you 
haven't done so already.


If you send a request for a copyright assignment form to 
g...@gcc.gnu.org, a maintainer who has access to the relevant forms will 
forward this on to you.


regards
Ramana