Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
On 08/25/2013 09:33 PM, Gerald Pfeifer wrote: On Tue, 20 Aug 2013, Florian Weimer wrote: As the libvtv reviewer, you don't need permission to commit your changes. :-) Actually, reviewers do need someone else's approval for their own changes (unlike maintainers and of course not for trivial changes). Ah, but I'm not a global reviewer, so I couldn't approve the change anyway. :-) Caroline, I think your patch in http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00619.html doesn't match the ChangeLog update and what was approved by Jeff. It should have gone into the Various Maintainers section. -- Florian Weimer / Red Hat Product Security Team
Re: Fix OBJ_TYPE_REF handling in ipa-cp
Hi, On Thu, Aug 22, 2013 at 05:33:50PM +0200, Jan Hubicka wrote: Hi, this problem was noticed by my verifier that binfo walks are not across type hiearchy. ipa_intraprocedural_devirtualization is one remaining place where we take class of object from OBJ_TYPE_REF_OBJECT instead of obj_type_ref_class_type. Unforutnately I noticed that this problem is propagated quite further across ipa-prop design. We assume that types of pointers taken from gimple call arguments are types of pointers to classes pass down to the callee. This is not true after propagation. I did not fix the all places, only places needed to get parameter for ipa_set_jf_known_type and detect_type_change_ssa right. Also I modified ipa_set_jf_known_type to not record non-polymorphic type. It is a waste of memory and LTO streaming bandwidth. Bootstrapped/regtesed x86_64-linux. Martin, please can you review the change? * ipa-prop.c (ipa_set_jf_known_type): Check that component type is a record type with BINFO. (detect_type_change_ssa): Add comp_type argument. (compute_complex_assign_jump_func): Add param_type argument; pass it down to detect_type_change_ssa. (compute_known_type_jump_func): Add expected_type parameter. Do not bother tracking a non-polymorphic type. (ipa_get_callee_param_type): New function. (ipa_compute_jump_functions_for_edge): Pass down calle parm types. (ipa_analyze_virtual_call_uses): Use class typee as argument of detect_type_change_1. (ipa_intraprocedural_devirtualization): Pass down class type. Hopefully I'll get rid of the component_types in the jump functions and most of this won't be necesary. Meanwhile, this is OK. Thanks, Martin
Re: [PATCH 0/2] Port symtab/cgraph/varpool nodes to use C++ inheritance
On Tue, 2013-08-20 at 23:01 +0200, Jan Hubicka wrote:
[...]
In summary,
struct GTY(()) symtab_node_base
becomes:
class GTY((user)) symtab_node_base
and the subclasses:
struct GTY(()) cgraph_node
and:
struct GTY(()) varpool_node
become (respectively):
struct GTY((user)) cgraph_node : public symtab_node_base
and:
class GTY((user)) varpool_node : public symtab_node_base
I am not quite sure why we need symtab_node_base.
symtab_node is meant to be basetype of the symbols. Symbols can be either
functions or variables.
I would expect then
struct GTY((user)) cgraph_node : public symtab_node
class GTY((user)) varpool_node : public symtab_node
Indeed, I would have used symtab_node, but it's already in use as the
typedef of the *pointer* type - to quote the current ipa-ref.h:
typedef union symtab_node_def *symtab_node;
So I kept the name symtab_node_base.
I was also holding off on renames since you have other changes
in-flight.
The symtab_node_def union goes away, as do the symbol fields at the
top of the two subclasses.
Yes, this is very nice.
I kept the existing names for things, and the struct for cgraph_node
since it is often referred to as struct cgraph_node.
In fact I think we should take the chance to rename
cgraph_node - symtab_function
varpool_node- symtab_variable
(or symtab_func/symtab_var as suggested by Martin today, I am fine with both)
symtab_node may also be better as symtab_symbol. So the hearchy would be
that symbol table consists of symbols and symbols can be function or entries.
The original naming comes from callgraph code that is over decade old now.
node is because graph has nodes and edges.
We can also do this incrementally on the top of the hard work of getting the
hiearchy in at first place, if that seems easier for you. Indeed there are
some references to it in comments, but they should be updated.
Given how much of this is done by a script (with its own test suite),
I'd be up for getting in such other changes at the same time.
Let the wild rumpus^Wbikesheddery commence!
How about:
class symtab_sym {};
class symtab_func : public symtab_sym {};
class symtab_var : public symtab_sym {};
typedef symtab_sym *symtab_node;
In an ideal world I'd use a namespace for this, but I'm wary about
confusions between the symtab meaning of function vs the cfun meaning
of function.
I am OK with temporary inconsistency with naming and I will be happy to
continue with incremental cleanups.
The patch is in three parts; all three are needed.
* a fix for a bug in gengtype:
http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00882.html
[...]
I can't approve this change, unfortunately.
This one is already approved and committed.
* Patch 1 of the 2 in this series is the hand-written part of the
[...]
* Patch 2 of the 2 in this series is the autogenerated part.
Currently to access the base symtab fields of a cgraph or varpool
node, the code has e.g.
node-symbol.decl
whereas with C++ inheritance, the symbol field is no more, and we
directly use the base-class field:
node-decl
Indeed, this is very nice. We also use
(symtab_node)node whenver we need to go from cgraph/varpool node back
to basetype. These should go, too.
Finally I introduced cgraph(node)/varpool(node) functions that converts
symtab node to cgraph/varpool node and ICEs when failed.
We probably should use our new template conversions. We have is_a
predicate and dyn_cast convertor that returns NULL on failure. Do
we have variant that ICEs when conversion is not possible?
How does this look? Are there other automated changes you'd like me
to make? How should I go about getting this into trunk, given that you
have many other changes to this code on the way?
I like the general direction and I am grateful you work on it :) Lets go with
this as first step. Incrementally I think the ipa-ref API should be cleaned
up
(it worked around model before we had basetype for cgraph/varpool) and I am
having quite long TODO list on the top of that.
Honza
Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
Thank you for pointing out my error. I will commit the following patch. -- Caroline Tice cmt...@google.com 2013-08-26 Caroline Tice cmt...@google.com * MAINTAINERS: Correct earliers update: Move myself from libvtv Various Reviewers to libvtv Various Maintainers. Index: MAINTAINERS === --- MAINTAINERS (revision 201986) +++ MAINTAINERS (working copy) @@ -178,6 +178,7 @@ libobjc Nicola Pero nicola.pero@meta- libobjc Andrew Pinski pins...@gmail.com libquadmath Tobias Burnus bur...@net-b.de libquadmath Jakub Jelinek ja...@redhat.com +libvtv Caroline Tice cmt...@google.com loop discovery Michael Hayes m.ha...@elec.canterbury.ac.nz soft-fp Joseph Myers jos...@codesourcery.com scheduler (+ haifa) Jim Wilson wil...@tuliptree.org @@ -288,7 +289,6 @@ libsanitizer, asan.c Jakub Jelinek jaku libsanitizer, asan.c Dodji Seketeli do...@redhat.com libsanitizer, asan.c Kostya Serebryany k...@google.com libsanitizer, asan.c Dmitry Vyukov dvyu...@google.com -libvtv Caroline Tice cmt...@google.com loop optimizer Zdenek Dvorak o...@ucw.cz loop optimizer Daniel Berlin dber...@dberlin.org LTO Diego Novillo dnovi...@google.com On Mon, Aug 26, 2013 at 2:10 PM, Florian Weimer fwei...@redhat.com wrote: On 08/25/2013 09:33 PM, Gerald Pfeifer wrote: On Tue, 20 Aug 2013, Florian Weimer wrote: As the libvtv reviewer, you don't need permission to commit your changes. :-) Actually, reviewers do need someone else's approval for their own changes (unlike maintainers and of course not for trivial changes). Ah, but I'm not a global reviewer, so I couldn't approve the change anyway. :-) Caroline, I think your patch in http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00619.html doesn't match the ChangeLog update and what was approved by Jeff. It should have gone into the Various Maintainers section. -- Florian Weimer / Red Hat Product Security Team
Re: ELF interposition and One Definition Rule
On Aug 26, 2013, at 8:21 AM, Jan Hubicka hubi...@ucw.cz wrote:
My understanding of C++ One Definition Rule, in a strict sense, does not a
allow in to define two functions of the same name and different semantics in a
valid program . I also think that all DSOs eventually linked together or
dlopenned are part of the same program. So theoretically, for C++ produced
code, we may go with AVAIL_AVAILABLE everywhere.
So, I think you're on firm ground wrt the standard. I think LTO naturally
wants see and make use of semantics, and once you accept that as valid, which,
reasonably it is, I think you get to see and understand quite a lot about the
code. Replacing anything comes with a heavy constraint that it is reasonably
the same and the user will die if it is not. When an allocation function that
the LTO optimizer can see is 32 byte aligned on the returned pointer, it is
reasonable to make use of this on the client side code-gen. If the user
replaces that allocation function with one that was not 32-byte aligned, bad
things would happen.
I think what the optimizer can see is open ended, and any use it wants to make
of what it sees is fine. Functions, data, classes, methods, ctors, dtors,
templates, everything.
Now, that the standard perspective. From the QOI viewpoint, you will have
users that want to do various things, and they should explain what they want,
and we should document and vend them the good stuff. I defer to the
interposing types on what they want to do and why. Roughly, they need a way to
hide things from the optimizer. Assembly can do this, but, we'd also want to
hide (or mark as please don't peer into) any function, method or variable.
Separate compilation not using the -flto flag seems a reasonable way to do
this. I don't know if it is enough… I think those types of people will scream
when they discover they want more control.
On IRC we got into an agreement that we may disallow interposition for
virtuals,
Hum… I'm not one of those people that want to interpose virtuals, but as a
tool vendor, it would seem like some would like to be able to interpose
virtuals. I think separate compilation with no -flto should be enough to hide
enough details to make the interposition of virtuals possible. For example,
someone has a nice C++ abi that includes a virtual function for open, and one
wants to interpose open to trace it to merely help debug a problem. Doesn't
strike me as wrong.
For comdat (template functions), I can't help but think having a way to mark
definitions as, please don't peer into this, would be nice to have. One can
separate declaration and definition and explicitly instantiate, but doing this
might be a pain. I'd defer, again, to the interposers.
Now, when the cost of allowing interposing is high (dynamic relocs for
example), disallowing interposition by default is fine, not arguing that one
must always have the cost. Just seems nice from a theoretic perspective to
allow the user to say, yes, we do want to allow interposing on these virtuals.
Does the following patch seems sane?
Easier to review the change in semantics of a sample bit of code… I think I
understand the effects of the change.
Of course I would be happier with a stronger rule - for instance allowing
interposition only on plain functions not on methods.
Hum, I like the orthogonal rules that apply generally. Meaning, I don't like
the notion of treating functions and methods (or virtual methods) differently.
For example, a don't peer into for a template function definition, should be
used to not peer into a normal inline function.
I think I like letting the optimizer do anything, and making the user
responsible for not using -flto, or ensuring enough separate compilation, or
otherwise marking the boundaries that don't want to peer though… I could also
be burned alive by a linux distributor with existing code if I tried this… :-)
Good luck.
Oh, so keep in mind, if you do something like
template class T
class actor {
ctor() {
static int i = 100;
printf(%p\n, i);
}
};
and don't smash all the ctors together, you (can) wind up with multiple ctor::i
objects. The standard says there is one of them. The usual way to ensure
there is only one of them is to collapse all the the ctors together into one,
then, trivially, that one can only reference one of the i's that exist. This
is one way (beyond equality) to tell if there are multiples (if the function is
replicated). Trying to think if there were any other ways… ah yes, here it is:
6 A static local variable in a member function always refers to the same
object, whether or not the member function is inline.
A static local variable in an extern inline function always refers to
the same object. A string literal in an extern inline function is the
same object in different translation units.
So, string literals can also be used to notice the uniqueness of the
Re: [GOOGLE] Update AutoFDO annotation
Can you add missing documentation on functions like ...:get_count_info
-- documenting return value etc. Also it might be better to avoid
using 'set' as the local variable name. Change it to something more
specific.
thanks,
David
On Thu, Aug 22, 2013 at 3:56 PM, Dehao Chen de...@google.com wrote:
This patch has 2 changes:
1. Now that we have discriminator for inlined callsite, we don't need
special handling for callsite location any more.
2. If a source line is mapped to multiple BBs, only the first BB will
be annotated.
3. Before actual annotation, mark everythin BB/edge as not annotated.
Bootstrapped and passed regression test.
OK for google branch?
Thanks,
Dehao
Index: gcc/auto-profile.c
===
--- gcc/auto-profile.c (revision 201927)
+++ gcc/auto-profile.c (working copy)
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see
#include string.h
#include map
#include vector
+#include set
#include config.h
#include system.h
@@ -100,6 +101,10 @@ typedef std::mapunsigned, gcov_type icall_target
(execution_count, value_profile_histogram). */
typedef std::pairgcov_type, icall_target_map count_info;
+/* Set of inline_stack. Used to track if the profile is already used to
+ annotate the program. */
+typedef std::setinline_stack location_set;
+
struct string_compare
{
bool operator() (const char *a, const char *b) const
@@ -202,7 +207,8 @@ class autofdo_source_profile {
const function_instance *get_function_instance_by_decl (tree decl) const;
/* Find profile info for a given gimple STMT. If found, store the profile
info in INFO, and return true; otherwise return false. */
- bool get_count_info (gimple stmt, count_info *info) const;
+ bool get_count_info (gimple stmt, count_info *info,
+ const location_set *set) const;
/* Find total count of the callee of EDGE. */
gcov_type get_callsite_total_count (struct cgraph_edge *edge) const;
@@ -284,17 +290,13 @@ static const char *get_original_name (const char *
/* Return the combined location, which is a 32bit integer in which
higher 16 bits stores the line offset of LOC to the start lineno
- of DECL, The lower 16 bits stores the discrimnator of LOC if
- USE_DISCR is true, otherwise 0. */
+ of DECL, The lower 16 bits stores the discrimnator. */
static unsigned
-get_combined_location (location_t loc, tree decl, bool use_discr)
+get_combined_location (location_t loc, tree decl)
{
- if (use_discr)
-return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) 16)
- | get_discriminator_from_locus (loc);
- else
-return (LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) 16;
+ return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) 16)
+ | get_discriminator_from_locus (loc);
}
/* Return the function decl of a given lexical BLOCK. */
@@ -316,7 +318,7 @@ get_function_decl_from_block (tree block)
}
static void
-get_inline_stack (gimple stmt, bool use_discr, inline_stack *stack)
+get_inline_stack (gimple stmt, inline_stack *stack)
{
location_t locus = gimple_location (stmt);
if (LOCATION_LOCUS (locus) == UNKNOWN_LOCATION)
@@ -337,14 +339,13 @@ static void
tree decl = get_function_decl_from_block (block);
stack-push_back (std::make_pair (
- decl, get_combined_location (locus, decl, level == 0 use_discr)));
+ decl, get_combined_location (locus, decl)));
locus = tmp_locus;
level++;
}
stack-push_back (std::make_pair (
current_function_decl,
- get_combined_location (locus, current_function_decl,
- level == 0 use_discr)));
+ get_combined_location (locus, current_function_decl)));
}
@@ -523,14 +524,16 @@ const function_instance *autofdo_source_profile::g
return ret == map_.end() ? NULL : ret-second;
}
-bool autofdo_source_profile::get_count_info (gimple stmt,
- count_info *info) const
+bool autofdo_source_profile::get_count_info (gimple stmt, count_info *info,
+ const location_set *set) const
{
if (LOCATION_LOCUS (gimple_location (stmt)) == cfun-function_end_locus)
return false;
inline_stack stack;
- get_inline_stack (stmt, true, stack);
+ get_inline_stack (stmt, stack);
+ if (set set-find(stack) != set-end())
+return false;
if (stack.size () == 0)
return false;
const function_instance *s = get_function_instance_by_inline_stack (stack);
@@ -544,7 +547,7 @@ gcov_type autofdo_source_profile::get_callsite_tot
{
inline_stack stack;
stack.push_back (std::make_pair(edge-callee-symbol.decl, 0));
- get_inline_stack (edge-call_stmt, false, stack);
+ get_inline_stack (edge-call_stmt, stack);
const function_instance *s = get_function_instance_by_inline_stack (stack);
if (s == NULL)
@@ -821,7 +824,7 @@ afdo_vpt (gimple stmt, const icall_target_map map
on statements. */
static gcov_type
Re: ELF interposition and One Definition Rule
On 08/26/2013 11:21 AM, Jan Hubicka wrote: Our default behaviour special case inline functions that are always AVAIL_AVAILABLE and, via decl_replaceable_p, also any COMDAT (that may be for functions since all COMDATs are also inlines, but makes difference for variables I think). Not all COMDAT functions are inlines; template instantiations are also COMDAT. My understanding of C++ One Definition Rule, in a strict sense, does not a allow in to define two functions of the same name and different semantics in a valid program. Strictly speaking, the same is true of C: If an identifier declared with external linkage is used in an expression (other than as part of the operand of a sizeof or _Alignof operator whose result is an integer constant), somewhere in the entire program there shall be exactly one external definition for the identifier; otherwise, there shall be no more than one. I also think that all DSOs eventually linked together or dlopenned are part of the same program. So theoretically, for C++ produced code, we may go with AVAIL_AVAILABLE everywhere. And for C code as well. ELF symbol replacement is an extension to C/C++ semantics, which does not include DSOs; it seems to me that it is up to us to define what assumptions we want the compiler to be able to make. On IRC we got into an agreement that we may disallow interposition for virtuals, since replacing these seems fishy - they don't even have address well defined and compiler can freely duplicate and/or unify them. I think same apply for C++ constructors and destructors now. But it would be simple to create a DSO which replaces a virtual function or a constructor, by simply providing a new definition. Since interposition happens outside the language, I don't think reasoning based on things that happen within the language makes much sense. Of course I would be happier with a stronger rule - for instance allowing interposition only on plain functions not on methods. Existing uses of interposition apply to plain functions, but that doesn't mean someone might not want to use it for member functions as well. I would be happy with an even stronger default that optimizes on the assumption that no interposition occurs; typically interposition is overriding a symbol found in a dynamic library (i.e. malloc) rather than a symbol defined in the same translation unit as the use. Jason
Re: [C++ Patch] Remove old bison hack?!
OK. Jason
Re: Make some comdats implicitly hidden
On 08/26/2013 03:57 PM, Jan Hubicka wrote: While analyzing the relocations of libreoffice I noticed that I can play the same game w/o LTO at linker level. Making those symbols hidden truns external relocations to internal and should improve runtime, too: comdat sharing by dynamic linker is expensive and won't save duplicated functions from the binary. Makes sense. There is ext/visibility/template2.C that fails with the patch. It tests that visibility pragma does not bring the symbol local, but now we do so based on logic above. Jason, do you have any idea how to fix the testcase? I tried to use different visility but that doesn't work, since we do not have corresponding scan-not-* Maybe change it to use a function that has its address taken, so this optimization doesn't apply. Jason
Re: ELF interposition and One Definition Rule
On Mon, Aug 26, 2013 at 7:58 PM, Jason Merrill ja...@redhat.com wrote: On 08/26/2013 11:21 AM, Jan Hubicka wrote: Our default behaviour special case inline functions that are always AVAIL_AVAILABLE and, via decl_replaceable_p, also any COMDAT (that may be for functions since all COMDATs are also inlines, but makes difference for variables I think). Not all COMDAT functions are inlines; template instantiations are also COMDAT. My understanding of C++ One Definition Rule, in a strict sense, does not a allow in to define two functions of the same name and different semantics in a valid program. Strictly speaking, the same is true of C: If an identifier declared with external linkage is used in an expression (other than as part of the operand of a sizeof or _Alignof operator whose result is an integer constant), somewhere in the entire program there shall be exactly one external definition for the identifier; otherwise, there shall be no more than one. I also think that all DSOs eventually linked together or dlopenned are part of the same program. So theoretically, for C++ produced code, we may go with AVAIL_AVAILABLE everywhere. And for C code as well. C++'s ODR is much stronger than C's though. For example, C allows definitions for the same inline function to differ -- the out-of-line extern definition does not need to be the same. That is anathema in C++. ELF symbol replacement is an extension to C/C++ semantics, which does not include DSOs; it seems to me that it is up to us to define what assumptions we want the compiler to be able to make. On IRC we got into an agreement that we may disallow interposition for virtuals, since replacing these seems fishy - they don't even have address well defined and compiler can freely duplicate and/or unify them. I think same apply for C++ constructors and destructors now. But it would be simple to create a DSO which replaces a virtual function or a constructor, by simply providing a new definition. Since interposition happens outside the language, I don't think reasoning based on things that happen within the language makes much sense. Of course I would be happier with a stronger rule - for instance allowing interposition only on plain functions not on methods. Existing uses of interposition apply to plain functions, but that doesn't mean someone might not want to use it for member functions as well. Agreed. I would be happy with an even stronger default that optimizes on the assumption that no interposition occurs; typically interposition is overriding a symbol found in a dynamic library (i.e. malloc) rather than a symbol defined in the same translation unit as the use. Jason Again, strongly agreed. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On 08/26/2013 10:45 AM, Gabriel Dos Reis wrote:
Hmm, let's not make it a default. Replacing global operator new (e.g. for
tracing purposes) is a valid C++ programming idiom.
Absolutely. What strikes me as vanishingly unlikely is the idea that
the replacement operator new would expose pointers to returned memory
*and* that code would refer to the memory both via one of those pointers
and via the value of the new-expression in the same function. That is,
void *last_new_ptr;
void *operator new (size_t) noexcept(false) {
last_new_ptr = ...;
return last_new_ptr;
}
int main()
{
int *a = new int;
int *b = (int*)last_new_ptr;
*a = 42;
*b = 24;
if (*a != 24) abort();
}
I'm happy to let this code break by assuming that the store to b
couldn't have affected *a.
Jason
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 8:17 PM, Jason Merrill ja...@redhat.com wrote:
On 08/26/2013 10:45 AM, Gabriel Dos Reis wrote:
Hmm, let's not make it a default. Replacing global operator new (e.g. for
tracing purposes) is a valid C++ programming idiom.
Absolutely. What strikes me as vanishingly unlikely is the idea that the
replacement operator new would expose pointers to returned memory *and* that
code would refer to the memory both via one of those pointers and via the
value of the new-expression in the same function. That is,
void *last_new_ptr;
void *operator new (size_t) noexcept(false) {
last_new_ptr = ...;
return last_new_ptr;
}
int main()
{
int *a = new int;
int *b = (int*)last_new_ptr;
*a = 42;
*b = 24;
if (*a != 24) abort();
}
I'm happy to let this code break by assuming that the store to b couldn't
have affected *a.
Amen!
-- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On 08/22/2013 09:19 AM, Jan Hubicka wrote: - I tried to track functions that lead to terminate() and not mark them as ECF_LEAF. This is because user can set handler. If the handler can resonably expect the static vars defined in its unit to be in the final form, we can not consider it ECF_LEAF. Perhaps there are cases where terminate() is called only for programs already after undefined effect? Not really; terminate() is an alternative to undefined behavior. - Is do_end_catch nothrow? It does not seem to be declared so in libsupc++ No, destroying the exception object might throw. Bootstrapped/regtested x86_64-linux, OK? OK. Jason
[patch/djgpp]: Add ASM_DECLARE_FUNCTION_NAME
Needed to build i586-pc-msdosdjgpp target, committed.
* config/i386/djgpp.h (ASM_DECLARE_FUNCTION_NAME): New.
Index: config/i386/djgpp.h
===
--- config/i386/djgpp.h (revision 202015)
+++ config/i386/djgpp.h (working copy)
@@ -117,6 +117,17 @@
#define ASM_OUTPUT_ALIGNED_BSS(FILE, DECL, NAME, SIZE, ALIGN) \
asm_output_aligned_bss ((FILE), (DECL), (NAME), (SIZE), (ALIGN))
+/* Write the extra assembler code needed to declare a function properly. */
+
+#ifndef ASM_DECLARE_FUNCTION_NAME
+#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL)\
+ do \
+{ \
+ ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);\
+} \
+ while (0)
+#endif
+
/* This is how to tell assembler that a symbol is weak */
#undef ASM_WEAKEN_LABEL
#define ASM_WEAKEN_LABEL(FILE,NAME) \
Re: [PATCH, libgcc] Fix licenses on several files
On 29/07/2013, at 10:03 AM, Maxim Kuvyrkov wrote: While verifying license compliance for GCC and its libraries I noticed that several libgcc files that end up in the final library are licensed under GPL-3.0+ instead of GPL-3.0-with-GCC-exception. This is, obviously, was not the intention of developers who just copied wrong boilerplate text, and this patch fixes the oversights. I wrote up a blog post about GCC licenses and ideas on their audit at http://www.kugelworks.com/blog/gcc-license-audit/ . The blog post mentions two improvements in handling licenses in GCC and I will follow up on them in separate threads. -- Maxim Kuvyrkov www.kugelworks.com
RE: [ping**4] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3
PING!
This issue is really important. It does not only affect bitfields but all kinds
of packed structures.
Starting from gcc 4.6.0 there is not a single released version that handles the
packed structures
correctly.
So could some one please approve Sandra's patch now?
Thanks
Bernd.
Date: Mon, 29 Jul 2013 00:33:23 -0600
From: san...@codesourcery.com
To: gcc-patches@gcc.gnu.org
CC: richard.guent...@gmail.com; ebotca...@adacore.com;
bernd.edlin...@hotmail.de
Subject: [ping**3] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3
On 07/20/2013 01:12 PM, Sandra Loosemore wrote:
On 07/09/2013 10:23 AM, Sandra Loosemore wrote:
On 06/30/2013 09:24 PM, Sandra Loosemore wrote:
Here is my third attempt at cleaning up -fstrict-volatile-bitfields.
Ping?
...and ping again.
...and again. Hmmm.
struct patch_status
{
volatile int approved:1;
volatile int rejected:1;
volatile int needs_changes:1;
int pinged;
};
extern struct patch_status s;
while (!s.approved !s.rejected !s.needs_changes)
{
sleep (a_week_or_two ());
pinged++;
}
Part 1 removes the warnings and packedp flag. It is the same as in the
last version, and has already been approved. I'll skip reposting it
since the patch is here already:
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00908.html
Part 2 replaces parts 2, 3, and 4 in the last version. I've re-worked
this code significantly to try to address Bernd Edlinger's comments on
the last version in PR56997.
Part 2: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg1.html
Part 3 is the test cases, which are the same as in the last version.
Nobody has reviewed these but I assume they are OK if Part 2 is
approved?
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00912.html
Part 4 is new; it makes -fstrict-volatile-bitfields not be the default
for any target any more. It is independent of the other changes.
Part 4: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg2.html
It seems that the change to the defaults in part 4 is still
controversial but my understanding based on discussion of the previous
version of the patches is that the maintainers were going to insist on
that as a condition of getting the other bug fixes in. From my
perspective, I'd be happy just to wrap up this patch series somehow or
another, so please let me know if there are additional changes I need to
make before this is suitable to check in.
Please note that I'm pinging my own 4-part patch series, not the Bernd's
followup patch confusingly posted in the same thread.
-Sandra
Re: powerpc64le multilibs and multiarch dir
On Sun, Aug 25, 2013 at 10:40:30PM -0700, Mike Stump wrote: On Aug 25, 2013, at 8:32 PM, Alan Modra amo...@gmail.com wrote: We (IBM) don't intend to support running both big and little-endian processes on the same system in the near future. Perhaps I'm jumping the gun in defining the multi-os dirs like /lible and /lib64le. I'd recommend against multilibs, unless you have a need for them… These multlibs are only added if you ask for them via --enable-targets. -- Alan Modra Australia Development Lab, IBM
Re: [C++ patch] Set attributes for C++ runtime library calls
On Aug 23, 2013, at 9:36 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
You made a statement. It was not clear whether it was what you want
or whether it is what the standards mandate.
Both.
(2) what you believe the standards mandate, with appropriate quote; and
The life of the original object does not end. See below for the quote.
Your assertion or a quote from the standards?
A quote. You can tell it is a quote, because the wording comes from the
standard.
And yet memcpy is exactly defined and all interactions with the entire rest
of the standard are as specified. Those semantics that are defined and
required, are, by definition. You can't make them go away by claiming they
are undefined.
I have no idea what exactly you are talking about.
You said memcpy was undefined. You've not quoted the standard that said it was
undefined. I've quoted the definition from the standard.
If you have a rule in that standards that say that mempcy is assignment,
please share it with us.
I have. Please read the thread once more.
int i = 1, j = 2;
{
memcpy (i, j)
// here i exists and is 2.
}
3.7.1 Static storage duration [basic.stc.static]
1 All objects which neither have dynamic storage duration nor are local
have static storage duration. The storage for these objects shall
last for the duration of the program (_basic.start.init_,
_basic.start.term_).
This covers why i exists.
The storage duration of an object is not necessarily the same
as its lifetime.
Is this relevant in this case? If so, please quote the standard. It is not.
Yes, but what is the relevance?
If you don't understand the relevance of the standard that mandates the
behavior of the code in question, I can't help you.
Right, because it doesn't define taking the bytes of one object and
assigning another object to have those bytes. The definition of memcpy does
that.
Chapter and verse, please.
Again, I've already quoted it. You need to be able to read and understand
quotes from the standard, why they apply and how the requirements drive the
semantics.
char c1, c2;
c1 = c2;
This is defined to take the bytes that comprise c2 (1 byte, by definition),
and slam them into c1. After this runs, c1 will have the value of c2.
This, is, by definition. We can plainly see the assignment operator. This
is assignment, the hint is the spelling of the operator. It is spelled =.
This is what the C standard means, when they say copies. copies is defined
to mean, =.
so? What is the relevance exactly?
See above.
Which was the point I was making.
Sure it does. Directly, please read the statement again:
The memcpy function copies n characters from the object
pointed to by s2 into the object pointed to by s1.
Yes, but why is that assignment not copy construction as in
new (p) T (v);
?
That is the question.
I can cover that; but, it would be a waste of mine time if you are unable to
see such simple things like memcpy is defined and that memcpy does an
assignment. The case for this is harder to follow. Essentially, the short
version, placement creates an object. The address of the data of that object
is identical to the backing store. Therefore, the two are indistinguishable.
Since the access type is the same, the original object refers to the object
from the new expression; therefore, it exists. There is no end of it's
lifetime[1], so it continues to exist, post the block the new was in. The
value of that object must compare == to the new object, since the address and
the type are the same and the storage isn't volatile. QED. Trivial enough to
dig out all the quotes to make that happen from the standard. If you don't buy
that, feel free to quote the standard that mandates the semantics are other
than the above.
1 - One can equally view the lifetime of the old object ending and the lifetime
of a new object beginning, since the required semantics in both cases are
identical. Since the difference can't be seen, there is none. Since there is
none, it is irrelevant what words we actually use to describe it in this case.
I said what the words mean. I'd said copy means the = operator.
Copy in C++ does not necessarily mean assignment.
Sorry, can't help you then. It does, that's just the way it is. If it meant
increment, you'd be able to quote a definition of copy that said it meant
increment. You can't, because it doesn't. Also, in this case, there is no
difference in semantics, even if you assume copy meant copy construction. So,
doesn't matter in the least which particular copy you want to claim it is.
Here is the text for =:
[#3] An assignment operator stores a value in the object
designated by the left operand.
This does not say that storying a value in an object is assignment.
Sure it does:
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 2:44 AM, Mike Stump mikest...@comcast.net wrote: but I fear you won't understand this and how it applies. You must be right. I cannot match the entertaining value of your message, so you win. -- Gaby
Clean up pretty printers [18/n]
Same topic as patch 17/n. For more expressing printers.
Tested on an x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-26 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::unary_expression): Now a
virtual member function.
(c_pretty_printer::multiplicative_expression): Likewise.
(c_pretty_printer::conditional_expression): Likewise.
(c_pretty_printer::assignment_expression): Likewise.
(c_pretty_printer::expression): Likewise.
(pp_unary_expression): Adjust.
(pp_multiplicative_expression): Likewise.
(pp_assignment_expression): Likewise.
(pp_conditional_expression): Likewise.
(pp_expression): Likewise.
* c-pretty-print.c (c_pretty_printer::unary_expression): Rename
from pp_c_unary_expression. Adjust.
(c_pretty_printer::multiplicative_expression): Rename from
pp_c_multiplicative_expression. Adjust.
(c_pretty_printer::conditional_expression): Rename from
pp_c_conditional_expression. Adjust.
(c_pretty_printer::assignment_expression): Rename from
pp_c_assignment_expression. Adjust.
(c_pretty_printer::expression): Rename from pp_c_expression. Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
unary_expression, multiplicative_expression,
conditional_expression, expression.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::unary_expression):
Declare as overrider.
(cxx_pretty_printer::multiplicative_expression): Likewise.
(cxx_pretty_printer::conditional_expression): Likewise.
(cxx_pretty_printer::assignment_expression): Likewise.
(cxx_pretty_printer::expression): Likewise.
* cxx-pretty-print.c (cxx_pretty_printer::unary_expression):
Rename from pp_cxx_unary_expression. Adjust.
(cxx_pretty_printer::multiplicative_expression): Rename from
pp_cxx_multiplicative_expression. Adjust.
(cxx_pretty_printer::conditional_expression): Rename from
pp_cxx_conditional_expression. Adjust.
(cxx_pretty_printer::assignment_expression): Rename from
pp_cxx_assignment_expression. Adjust.
(cxx_pretty_printer::expression): Rename from pp_cxx_expression.
Adjust.
(cxx_pretty_printer::cxx_pretty_printer): Dot not assign to
unary_expression, multiplicative_expression,
conditional_expression, assignment_expression, expression.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201985)
+++ c-family/c-pretty-print.c (working copy)
@@ -50,7 +50,6 @@
static void pp_c_initializer_list (c_pretty_printer *, tree);
static void pp_c_brace_enclosed_initializer_list (c_pretty_printer *, tree);
-static void pp_c_multiplicative_expression (c_pretty_printer *, tree);
static void pp_c_additive_expression (c_pretty_printer *, tree);
static void pp_c_shift_expression (c_pretty_printer *, tree);
static void pp_c_relational_expression (c_pretty_printer *, tree);
@@ -59,8 +58,6 @@
static void pp_c_exclusive_or_expression (c_pretty_printer *, tree);
static void pp_c_inclusive_or_expression (c_pretty_printer *, tree);
static void pp_c_logical_and_expression (c_pretty_printer *, tree);
-static void pp_c_conditional_expression (c_pretty_printer *, tree);
-static void pp_c_assignment_expression (c_pretty_printer *, tree);
/* declarations. */
@@ -1255,7 +1252,7 @@
if (TREE_OPERAND (e, 2))
{
pp_separate_with (this, ',');
- pp_c_expression (this, TREE_OPERAND (e, 2));
+ expression (TREE_OPERAND (e, 2));
}
pp_c_right_paren (this);
break;
@@ -1619,7 +1616,7 @@
break;
case MEM_REF:
- pp_c_expression (this, e);
+ expression (e);
break;
case COMPLEX_CST:
@@ -1641,7 +1638,7 @@
case VA_ARG_EXPR:
pp_c_ws_string (this, __builtin_va_arg);
pp_c_left_paren (this);
- pp_assignment_expression (this, TREE_OPERAND (e, 0));
+ assignment_expression (TREE_OPERAND (e, 0));
pp_separate_with (this, ',');
pp_type_id (this, TREE_TYPE (e));
pp_c_right_paren (this);
@@ -1721,15 +1718,15 @@
__imag__ unary-expression */
void
-pp_c_unary_expression (c_pretty_printer *pp, tree e)
+c_pretty_printer::unary_expression (tree e)
{
enum tree_code code = TREE_CODE (e);
switch (code)
{
case PREINCREMENT_EXPR:
case PREDECREMENT_EXPR:
- pp_string (pp, code == PREINCREMENT_EXPR ? ++ : --);
- pp_c_unary_expression (pp, TREE_OPERAND (e, 0));
+ pp_string (this, code == PREINCREMENT_EXPR ? ++ : --);
+ unary_expression (TREE_OPERAND (e, 0));
break;
case ADDR_EXPR:
@@ -1740,53 +1737,53 @@
case CONJ_EXPR:
/* String literal are used by address. */
if (code == ADDR_EXPR
[PATCH, i386]: Remove unneeded ATTRIBUTE_UNUSED decorations
Hello!
2013-08-26 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_debug_options): Remove prototype.
(x86_64_elf_select_section): Ditto.
(ix86_handle_tm_regparm_attribute): Remove ATTRIBUTE_UNUSED on used
arguments.
(ix86_pass_by_reference): Ditto.
(ix86_return_in_memory): Ditto.
(output_set_got): Ditto.
(ix86_unary_operator_ok): Ditto.
(ix86_expand_builtin): Ditto.
Tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 201986)
+++ config/i386/i386.c (working copy)
@@ -2321,7 +2321,6 @@
static char *ix86_target_string (HOST_WIDE_INT, int, const char *,
const char *, enum fpmath_unit, bool);
-static void ix86_debug_options (void) ATTRIBUTE_UNUSED;
static void ix86_function_specific_save (struct cl_target_option *);
static void ix86_function_specific_restore (struct cl_target_option *);
static void ix86_function_specific_print (FILE *, int,
@@ -2754,7 +2753,7 @@
/* Function that is callable from the debugger to print the current
options. */
-void
+void ATTRIBUTE_UNUSED
ix86_debug_options (void)
{
char *opts = ix86_target_string (ix86_isa_flags, target_flags,
@@ -4848,10 +4847,7 @@
RELOC indicates whether forming the initial value of DECL requires
link-time relocations. */
-static section * x86_64_elf_select_section (tree, int, unsigned HOST_WIDE_INT)
- ATTRIBUTE_UNUSED;
-
-static section *
+static section * ATTRIBUTE_UNUSED
x86_64_elf_select_section (tree decl, int reloc,
unsigned HOST_WIDE_INT align)
{
@@ -5303,8 +5299,7 @@
static tree
ix86_handle_tm_regparm_attribute (tree *node, tree name ATTRIBUTE_UNUSED,
tree args ATTRIBUTE_UNUSED,
- int flags ATTRIBUTE_UNUSED,
- bool *no_add_attrs)
+ int flags, bool *no_add_attrs)
{
tree alt;
@@ -7228,8 +7223,7 @@
appropriate for passing a pointer to that type. */
static bool
-ix86_pass_by_reference (cumulative_args_t cum_v ATTRIBUTE_UNUSED,
- enum machine_mode mode ATTRIBUTE_UNUSED,
+ix86_pass_by_reference (cumulative_args_t cum_v, enum machine_mode mode,
const_tree type, bool named ATTRIBUTE_UNUSED)
{
CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
@@ -7762,7 +7756,7 @@
}
static bool
-ix86_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
+ix86_return_in_memory (const_tree type, const_tree fntype)
{
#ifdef SUBTARGET_RETURN_IN_MEMORY
return SUBTARGET_RETURN_IN_MEMORY (type, fntype);
@@ -8950,7 +8944,7 @@
/* Emit code for the SET_GOT patterns. */
const char *
-output_set_got (rtx dest, rtx label ATTRIBUTE_UNUSED)
+output_set_got (rtx dest, rtx label)
{
rtx xops[3];
@@ -18060,7 +18054,7 @@
bool
ix86_unary_operator_ok (enum rtx_code code ATTRIBUTE_UNUSED,
enum machine_mode mode ATTRIBUTE_UNUSED,
- rtx operands[2] ATTRIBUTE_UNUSED)
+ rtx operands[2])
{
/* If one of operands is memory, source and destination must match. */
if ((MEM_P (operands[0])
@@ -32154,9 +32148,8 @@
IGNORE is nonzero if the value is to be ignored. */
static rtx
-ix86_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED,
-enum machine_mode mode ATTRIBUTE_UNUSED,
-int ignore ATTRIBUTE_UNUSED)
+ix86_expand_builtin (tree exp, rtx target, rtx subtarget,
+enum machine_mode mode, int ignore)
{
const struct builtin_description *d;
size_t i;
Re: [PATCH, i386]: Remove unneeded ATTRIBUTE_UNUSED decorations
On Mon, Aug 26, 2013 at 11:04:42AM +0200, Uros Bizjak wrote:
static bool
-ix86_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
+ix86_return_in_memory (const_tree type, const_tree fntype)
{
#ifdef SUBTARGET_RETURN_IN_MEMORY
return SUBTARGET_RETURN_IN_MEMORY (type, fntype);
This doesn't look right, because the SUBTARGET_RETURN_IN_MEMORY
definitions (interix and i386elf) don't mention the FNTYPE anywhere.
Jakub
Re: [PATCH, i386]: Remove unneeded ATTRIBUTE_UNUSED decorations
On Mon, Aug 26, 2013 at 11:13 AM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Aug 26, 2013 at 11:04:42AM +0200, Uros Bizjak wrote:
static bool
-ix86_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
+ix86_return_in_memory (const_tree type, const_tree fntype)
{
#ifdef SUBTARGET_RETURN_IN_MEMORY
return SUBTARGET_RETURN_IN_MEMORY (type, fntype);
This doesn't look right, because the SUBTARGET_RETURN_IN_MEMORY
definitions (interix and i386elf) don't mention the FNTYPE anywhere.
Uh, I missed the #else.
Fixed in a followup patch.
Thanks,
Uros.
Re: Clean up pretty printers [18/n]
Paolo Carlini paolo.carl...@oracle.com writes: | Hi Gaby. | | On 08/26/2013 10:42 AM, Gabriel Dos Reis wrote: | Same topic as patch 17/n. For more expressing printers. | Tested on an x86_64-suse-linux. Applied to mainline. | Just got this: | | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c: In function ‘bool | c_tree_printer(pretty_printer*, text_info*, const char*, int, bool, | bool, bool)’: | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c:123:31: error: | ‘pp_c_expression’ was not declared in this scope | make[2]: *** [c/c-objc-common.o] Error 1 | | Yesterday things were fine. What happened was I inadvertently committed only a fraction of the patch. Now, all changes should be in. Sorry for the breakage. I guess it is time to go to bed. -- Gaby
Re: Clean up pretty printers [18/n]
Paolo Carlini paolo.carl...@oracle.com writes: | Hi Gaby, | | bootstrap is currently broken: | | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c: In function ‘bool | c_tree_printer(pretty_printer*, text_info*, const char*, int, bool, | bool, bool)’: | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c:123:31: error: | ‘pp_c_expression’ was not declared in this scope | make[2]: *** [c/c-objc-common.o] Error 1 | | If I don't hear from you I'm going to commit as obvious the below. You need more than that. The rest of patch is in trunk now. Sorry for the breakage -- as you probably guessed, I specified only c-family directory on the commit command line. -- Gaby
Re: Clean up pretty printers [18/n]
On 08/26/2013 11:20 AM, Gabriel Dos Reis wrote: You need more than that. The rest of patch is in trunk now. Sorry for the breakage -- as you probably guessed, I specified only c-family directory on the commit command line. Thanks! Paolo.
[c++-concepts] MErge from trunk
at revision 201992. There were conflicts with the recent pretty printing patches. Fixed. (Mostly due to the new concept-specific nodes printing.) Modulo the asan stuff. -- Gaby
Re: [PATCH] Create pass through and ancestor jump functions even there is dynamic type change
2013-08-23 Martin Jambor mjam...@suse.cz
* ipa-prop.h (ipa_pass_through_data): New field type_preserved.
(ipa_ancestor_jf_data): Likewise.
(ipa_get_jf_pass_through_agg_preserved): Fix comment typo.
(ipa_get_jf_pass_through_type_preserved): New function.
(ipa_get_jf_ancestor_agg_preserved): Fix comment typo.
(ipa_get_jf_ancestor_type_preserved): New function.
* ipa-cp.c (ipa_get_jf_pass_through_result): Honor type_preserved
flag.
(ipa_get_jf_ancestor_result): Likewise.
(propagate_vals_accross_pass_through): Use
ipa_get_jf_pass_through_result to do all the value mappings.
* ipa-prop.c (ipa_print_node_jump_functions_for_edge): Dump the
type_preserved flag.
(ipa_set_jf_cst_copy): New function.
(ipa_set_jf_simple_pass_through): Set the type_preserved flag.
(ipa_set_jf_arith_pass_through): Likewise.
(ipa_set_ancestor_jf): Likewise.
(compute_complex_assign_jump_func): Set type_preserved instead of
punting.
(ipa_compute_jump_functions_for_edge): Likewise.
(combine_known_type_and_ancestor_jfs): Honor type_preserved.
(update_jump_functions_after_inlining): Update type_preserved.
Explicitely create jump functions when combining one with
pass_through.
(ipa_write_jump_function): Stream the type_preserved flags.
(ipa_read_jump_function): Likewise.
+ if (TREE_CODE (input) == TREE_BINFO)
+{
+ if (ipa_get_jf_pass_through_type_preserved (jfunc))
+ {
+ gcc_checking_assert (ipa_get_jf_pass_through_operation (jfunc)
+== NOP_EXPR);
+ return input;
+ }
+ return NULL_TREE;
+}
To handle the types in construction well, I plan to extend
possible_polymorphic_call_targets to get an OUTER_TYPE, OFFSET, INCLUDE_BASES,
INCLUDE_DERIVED_TYPES arguments (in addition to existing OTR_TYPE and
OTR_TOKEN). OUTER_TYPE and OFFSET is what we now pass into get_binfo_at_offset
(i.e. the dynamic type of object we can track and offset of our subobject we
have virtual call for), where INCLUDE_BASES if true will make it to record all
the mathcing virtual methods of basetypes of OTR_TYPE, while
INCLUDE_DERIVED_TYPES will make it to record all matchng virtual methods of
derived types of OUTER_TYPE. If both are NULL, it will do what
get_binfo_at_offset does currently. Does it seem to work for you?
In the case you have !ipa_get_jf_pass_through_type_preserved, I believe you
will just set INCLUDE_BASES (since you do not know if outer_type is in
construction)
The INCLUDE_DERIVED_TYPES will be useful when we know the outer_object comes
from THIS pointer of a method that is not a constructor. I this case we know
that the type is either type of object the method is defined for or one of its
derivations.
Again if outer type is in construction (i.e. the method is constructor or
destructor) we will have both INCLUDE_DERIVED_TYPES and INCLUDE_BASES set.
The list will still be smaller than what we get now purely on the type of the
virtual call object.
Does this seem work for you?
the patch is OK,
thank you!
Honza
[ubsan] Introduce pointer_sized_int_node
I noticed I forgot to apply this old patch, already acked by Jason.
It introduces new pointer_sized_int_node, thus we can get rid of
uptr_type function in ubsan, and it allows us to do some clean-up
in asan.c, too.
Tested x86_64-linux, applying to ubsan branch.
2013-08-26 Marek Polacek pola...@redhat.com
* tree.h (enum tree_index): Add TI_POINTER_SIZED_TYPE.
(pointer_sized_int_node): Define.
* tree.c (build_common_tree_nodes): Initialize
pointer_sized_int_node.
* ubsan.c (ubsan_encode_value): Use pointer_sized_int_node instead
calling uptr_type.
(uptr_type): Remove function.
* asan.c (asan_global_struct): Use pointer_sized_int_node instead
calling build_nonstandard_integer_type.
(initialize_sanitizer_builtins): Likewise.
(asan_finish_file): Likewise.
--- gcc/tree.c.mp 2013-07-21 19:54:35.416986756 +0200
+++ gcc/tree.c 2013-07-21 19:56:58.347562787 +0200
@@ -9638,6 +9638,8 @@ build_common_tree_nodes (bool signed_cha
= build_pointer_type (build_type_variant (void_type_node, 1, 0));
fileptr_type_node = ptr_type_node;
+ pointer_sized_int_node = build_nonstandard_integer_type (POINTER_SIZE, 1);
+
float_type_node = make_node (REAL_TYPE);
TYPE_PRECISION (float_type_node) = FLOAT_TYPE_SIZE;
layout_type (float_type_node);
--- gcc/ubsan.c.mp 2013-07-21 20:04:59.469653493 +0200
+++ gcc/ubsan.c 2013-07-21 20:07:00.227178083 +0200
@@ -123,14 +123,6 @@ ubsan_typedesc_new (tree type, tree decl
return desc;
}
-/* Build the ubsan uptr type. */
-
-static tree
-uptr_type (void)
-{
- return build_nonstandard_integer_type (POINTER_SIZE, 1);
-}
-
/* Helper routine, which encodes a value in the uptr type.
Arguments with precision = POINTER_SIZE are passed directly,
the rest is passed by reference. T is a value we are to encode. */
@@ -143,7 +135,7 @@ ubsan_encode_value (tree t)
{
case INTEGER_TYPE:
if (TYPE_PRECISION (type) = POINTER_SIZE)
- return fold_build1 (NOP_EXPR, uptr_type (), t);
+ return fold_build1 (NOP_EXPR, pointer_sized_int_node, t);
else
return build_fold_addr_expr (t);
case REAL_TYPE:
@@ -153,7 +145,7 @@ ubsan_encode_value (tree t)
{
tree itype = build_nonstandard_integer_type (bitsize, true);
t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
- return fold_convert (uptr_type (), t);
+ return fold_convert (pointer_sized_int_node, t);
}
else
{
--- gcc/tree.h.mp 2013-07-21 19:54:35.441986868 +0200
+++ gcc/tree.h 2013-07-21 19:56:05.128353854 +0200
@@ -4227,6 +4227,7 @@ enum tree_index
TI_VA_LIST_FPR_COUNTER_FIELD,
TI_BOOLEAN_TYPE,
TI_FILEPTR_TYPE,
+ TI_POINTER_SIZED_TYPE,
TI_DFLOAT32_TYPE,
TI_DFLOAT64_TYPE,
@@ -4383,6 +4384,7 @@ extern GTY(()) tree global_trees[TI_MAX]
#define va_list_fpr_counter_field
global_trees[TI_VA_LIST_FPR_COUNTER_FIELD]
/* The C type `FILE *'. */
#define fileptr_type_node global_trees[TI_FILEPTR_TYPE]
+#define pointer_sized_int_node global_trees[TI_POINTER_SIZED_TYPE]
#define boolean_type_node global_trees[TI_BOOLEAN_TYPE]
#define boolean_false_node global_trees[TI_BOOLEAN_FALSE]
--- gcc/asan.c.mp 2013-07-21 20:07:15.013237456 +0200
+++ gcc/asan.c 2013-07-21 20:16:10.929376734 +0200
@@ -1954,7 +1954,7 @@ asan_global_struct (void)
= build_decl (UNKNOWN_LOCATION, FIELD_DECL,
get_identifier (field_names[i]),
(i == 0 || i == 3) ? const_ptr_type_node
- : build_nonstandard_integer_type (POINTER_SIZE, 1));
+ : pointer_sized_int_node);
DECL_CONTEXT (fields[i]) = ret;
if (i)
DECL_CHAIN (fields[i - 1]) = fields[i];
@@ -2039,8 +2039,7 @@ initialize_sanitizer_builtins (void)
ptr_type_node, ptr_type_node, NULL_TREE);
tree BT_FN_VOID_PTR_PTRMODE
= build_function_type_list (void_type_node, ptr_type_node,
- build_nonstandard_integer_type (POINTER_SIZE,
- 1), NULL_TREE);
+ pointer_sized_int_node, NULL_TREE);
tree BT_FN_VOID_INT
= build_function_type_list (void_type_node, integer_type_node, NULL_TREE);
tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5];
@@ -2197,7 +2196,6 @@ asan_finish_file (void)
if (gcount)
{
tree type = asan_global_struct (), var, ctor;
- tree uptr = build_nonstandard_integer_type (POINTER_SIZE, 1);
tree dtor_statements = NULL_TREE;
vecconstructor_elt, va_gc *v;
char buf[20];
@@ -2226,15 +2224,16 @@ asan_finish_file (void)
varpool_assemble_decl (varpool_node_for_decl (var));
fn = builtin_decl_implicit (BUILT_IN_ASAN_REGISTER_GLOBALS);
+ tree gcount_tree = build_int_cst (pointer_sized_int_node,
Committed: fix i686 bootstrap: x86_64_elf_select_section is unused
bootstrap for i686-pc-linux-gnu was failing with:
/ssd/fsf/boot-201992/./prev-gcc/xg++
-B/ssd/fsf/boot-201992/./prev-gcc/ -B/usr/local/i686-pc-linux-gnu/bin/
-nostdinc++
-B/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/src/.libs
-B/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/libsupc++/.libs
-I/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/include/i686-pc-linux-gnu -I/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/include -I/ssd/fsf/gcc/libstdc++-v3/libsupc++ -L/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/src/.libs -L/ssd/fsf/boot-201992/prev-i686-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -c -DIN_GCC_FRONTEND -g -O2 -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Icp -I../../gcc/gcc -I../../gcc/gcc/cp -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../gcc/gcc/../libbacktrace../../gcc/gcc/cp/cp-gimplify.c -o
cp/cp-gimplify.o
../../gcc/gcc/config/i386/i386.c:4851:1: error: section*
x86_64_elf_select_section(tree, int, long long unsigned int) defined
but not used [-Werror=unused-function]
committed as obvious.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 201992)
+++ config/i386/i386.c (working copy)
@@ -4847,7 +4847,7 @@ ix86_in_large_data_p (tree exp)
RELOC indicates whether forming the initial value of DECL requires
link-time relocations. */
-static section * ATTRIBUTE_UNUSED
+ATTRIBUTE_UNUSED static section *
x86_64_elf_select_section (tree decl, int reloc,
unsigned HOST_WIDE_INT align)
{
Re: [Patch, Fortran, OOP] PR 57305: ICE when calling SIZEOF on an unlimited polymorphic variable
Hi Mikael, the SIZEOF intrinsic currently returns the size according to the *declared* type for polymorphic variables. I think this doesn't really make much sense and it also causes ICEs when SIZEOF is called on CLASS(*) variables (which don't really have a declared type). Therefore I'm proposing to make SIZEOF return the size according to the *dynamic* type instead. The same is done by STORAGE_SIZE (F08), which however gives the size in bits. SIZEOF is a GNU extension, so we are free to decide on its behavior. I don't remember why the declared type was chosen in the first place, and I hope that no one seriously depends on the current behavior (the size of the declared type is probably not really useful after all). I'm slightly inclined to kindly invite the user to switch to STORAGE_SIZE+SIZE instead. Any other opinion? Since the SIZEOF intrinsic has been around for some time in gfortran (before STORAGE_SIZE was available), I would say we should at least continue to support it for backward compatibility. And I guess we should also make it behave reasonably for all inputs. However, it might be worth to add a note in the documentation that STORAGE_SIZE and SIZE should be used instead in standard-conforming code. The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk? Independently of the above, the patch seems to be forgetting the arg-rank != 0 case. Right. Also I just noticed that STORAGE_SIZE does not seem to behave correctly for class-array arguments, either ... Cheers, Janus
Fix some issues with speculative devirtualization machinery
Hi,
by making speculative indirect call machinery to be used by ipa-devirt I put it
on
wild on firefox converting about 30% of all polymorphic calls in the progrma
(3).
This has provoked some issues I missed previously this patch fixes:
1) dumping - we no longer do only indirect calls from other units.
2) Adds sanity check to cgraph_speculative_call_info with explanation comment
since it already confused Maritn (Liska). I also noticed that
direct/indirect
names are swapped that is very confusing
3) I noticed that when we resolve speculation in a way so direct edge wins
and the edge is already inline, its frequency increases that ought to rescale
all callees of the node. I want to do this as part of edge scaling in
ipa-inline-transform, but droped a FIXME.
4) cgraph_redirect_edge_call_stmt_to_callee ICEd when especulation was
wrong and dumping was enabled.
5) ipa-inline needs to reset caches more carefuly because of the effect
described above: resolving a speculation in the middle of function has
cascading effect that eventually lead to sanity check failure.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
* cgraph.c (cgraph_turn_edge_to_speculative): Fix debug output.
(cgraph_speculative_call_info): Fix parameter order and formating;
add sanity check.
(cgraph_resolve_speculation): Add FIXME about scaling profiles.
(cgraph_redirect_edge_call_stmt_to_callee): Fix ICE in debug dump.
* ipa-inline.c (heap_edge_removal_hook): Reset node growth cache.
(resolve_noninline_speculation): Update callee keys, too.
Index: cgraph.c
===
--- cgraph.c(revision 201966)
+++ cgraph.c(working copy)
@@ -1076,8 +1076,8 @@ cgraph_turn_edge_to_speculative (struct
if (dump_file)
{
- fprintf (dump_file, Indirect call - direct call from
- other module %s/%i = %s/%i\n,
+ fprintf (dump_file, Indirect call - speculative call
+ %s/%i = %s/%i\n,
xstrdup (cgraph_node_name (n)), n-symbol.order,
xstrdup (cgraph_node_name (n2)), n2-symbol.order);
}
@@ -1113,8 +1113,8 @@ cgraph_turn_edge_to_speculative (struct
void
cgraph_speculative_call_info (struct cgraph_edge *e,
- struct cgraph_edge *indirect,
struct cgraph_edge *direct,
+ struct cgraph_edge *indirect,
struct ipa_ref *reference)
{
struct ipa_ref *ref;
@@ -1137,16 +1137,18 @@ cgraph_speculative_call_info (struct cgr
}
else
for (e = e-caller-callees;
-e2-call_stmt != e-call_stmt || e2-lto_stmt_uid !=
e-lto_stmt_uid;
+e2-call_stmt != e-call_stmt
+|| e2-lto_stmt_uid != e-lto_stmt_uid;
e = e-next_callee)
;
}
gcc_assert (e-speculative e2-speculative);
- indirect = e;
- direct = e2;
+ direct = e;
+ indirect = e2;
reference = NULL;
- for (i = 0; ipa_ref_list_reference_iterate (e-caller-symbol.ref_list, i,
ref); i++)
+ for (i = 0; ipa_ref_list_reference_iterate (e-caller-symbol.ref_list,
+ i, ref); i++)
if (ref-speculative
((ref-stmt ref-stmt == e-call_stmt)
|| (ref-lto_stmt_uid == e-lto_stmt_uid)))
@@ -1154,6 +1156,11 @@ cgraph_speculative_call_info (struct cgr
reference = ref;
break;
}
+
+ /* Speculative edge always consist of all three components - direct edge,
+ indirect and reference. */
+
+ gcc_assert (e e2 ref);
}
/* Redirect callee of E to N. The function does not update underlying
@@ -1209,6 +1216,8 @@ cgraph_resolve_speculation (struct cgrap
fprintf (dump_file, Speculative call turned into direct call.\n);
edge = e2;
e2 = tmp;
+ /* FIXME: If EDGE is inlined, we should scale up the frequencies and
counts
+ in the functions inlined through it. */
}
edge-count += e2-count;
edge-frequency += e2-frequency;
@@ -1305,12 +1314,12 @@ cgraph_redirect_edge_call_stmt_to_callee
else if (!gimple_check_call_matching_types (e-call_stmt,
e-callee-symbol.decl,
true))
{
- e = cgraph_resolve_speculation (e, NULL);
if (dump_file)
fprintf (dump_file, Not expanding speculative call of %s/%i -
%s/%i\n
Type mismatch.\n,
xstrdup (cgraph_node_name (e-caller)),
e-caller-symbol.order,
xstrdup (cgraph_node_name (e-callee)),
e-callee-symbol.order);
+ e = cgraph_resolve_speculation (e, NULL);
}
/* Expand speculation into GIMPLE code. */
else
Index: ipa-inline.c
===
--- ipa-inline.c(revision 201966)
+++ ipa-inline.c
RFA: fix avr gcc.dg/fixed-point/convert-accum-neg.c execution failure
The gcc.dg/fixed-point/convert-accum-neg.c execution test fails for avr
because for fractional integer to accumulator / integer conversions,
the avr target rounds towards -infinity, whereas we are supposed to round
towards 0.
The attached patch implements rounding towards 0, and adds an option
-mfract-convert-truncate to revert to the previous behaviour (for
situations where smaller code is more important than proper rounding).
Tested for atmega128-sim.
OK to apply?
2013-06-25 Joern Rennecke joern.renne...@embecosm.com
* config/avr/avr.opt (mfract-convert-truncate): New option.
* config/avr/avr.c (avr_out_fract): Unless TARGET_FRACT_CONV_TRUNC
is set, round negative fractional integers according to n1169
when converting to integer types.
Index: config/avr/avr.c
===
--- config/avr/avr.c(revision 201989)
+++ config/avr/avr.c(working copy)
@@ -7030,7 +7030,9 @@ avr_out_fract (rtx insn, rtx operands[],
RTX_CODE shift = UNKNOWN;
bool sign_in_carry = false;
bool msb_in_carry = false;
+ bool lsb_in_tmp_reg = false;
bool lsb_in_carry = false;
+ bool frac_rounded = false;
const char *code_ashift = lsl %0;
@@ -7038,6 +7040,7 @@ #define MAY_CLOBBER(RR)
/* Shorthand used below. */ \
((sign_bytes \
IN_RANGE (RR, dest.regno_msb - sign_bytes + 1, dest.regno_msb)) \
+ || (offset IN_RANGE (RR, dest.regno, dest.regno_msb))\
|| (reg_unused_after (insn, all_regs_rtx[RR])\
!IN_RANGE (RR, dest.regno, dest.regno_msb)))
@@ -7112,13 +7115,119 @@ #define MAY_CLOBBER(RR)
else
gcc_unreachable();
+ /* If we need to round the fraction part, we might need to save/round it
+ before clobbering any of it in Step 1. Also, we might to want to do
+ the rounding now to make use of LD_REGS. */
+ if (SCALAR_INT_MODE_P (GET_MODE (xop[0]))
+ SCALAR_ACCUM_MODE_P (GET_MODE (xop[1]))
+ !TARGET_FRACT_CONV_TRUNC)
+{
+ bool overlap
+ = (src.regno =
+ (offset ? dest.regno_msb - sign_bytes : dest.regno + zero_bytes - 1)
+ dest.regno - offset -1 = dest.regno);
+ unsigned s0 = dest.regno - offset -1;
+ bool use_src = true;
+ unsigned sn;
+ unsigned copied_msb = src.regno_msb;
+ bool have_carry = false;
+
+ if (src.ibyte dest.ibyte)
+ copied_msb -= src.ibyte - dest.ibyte;
+
+ for (sn = s0; sn = copied_msb; sn++)
+ if (!IN_RANGE (sn, dest.regno, dest.regno_msb)
+!reg_unused_after (insn, all_regs_rtx[sn]))
+ use_src = false;
+ if (use_src TEST_HARD_REG_BIT (reg_class_contents[LD_REGS], s0))
+ {
+ avr_asm_len (tst %0 CR_TAB brpl 0f,
+ all_regs_rtx[src.regno_msb], plen, 2);
+ sn = src.regno;
+ if (sn s0)
+ {
+ if (TEST_HARD_REG_BIT (reg_class_contents[LD_REGS], sn))
+ avr_asm_len (cpi %0,1, all_regs_rtx[sn], plen, 1);
+ else
+ avr_asm_len (sec CR_TAB cpc %0,__zero_reg__,
+all_regs_rtx[sn], plen, 2);
+ have_carry = true;
+ }
+ while (++sn s0)
+ avr_asm_len (cpc %0,__zero_reg__, all_regs_rtx[sn], plen, 1);
+ avr_asm_len (have_carry ? sbci %0,128 : subi %0,129,
+ all_regs_rtx[s0], plen, 1);
+ for (sn = src.regno + src.fbyte; sn = copied_msb; sn++)
+ avr_asm_len (sbci %0,255, all_regs_rtx[sn], plen, 1);
+ avr_asm_len (\n0:, NULL, plen, 0);
+ frac_rounded = true;
+ }
+ else if (use_src overlap)
+ {
+ avr_asm_len (clr __tmp_reg__ CR_TAB
+ sbrc %1,0 CR_TAB dec __tmp_reg__, xop, plen, 1);
+ sn = src.regno;
+ if (sn s0)
+ {
+ avr_asm_len (add %0,__tmp_reg__, all_regs_rtx[sn], plen, 1);
+ have_carry = true;
+ }
+ while (++sn s0)
+ avr_asm_len (adc %0,__tmp_reg__, all_regs_rtx[sn], plen, 1);
+ if (have_carry)
+ avr_asm_len (clt CR_TAB bld __tmp_reg__,7 CR_TAB
+adc %0,__tmp_reg__,
+all_regs_rtx[s0], plen, 1);
+ else
+ avr_asm_len (lsr __tmp_reg CR_TAB add %0,__tmp_reg__,
+all_regs_rtx[s0], plen, 2);
+ for (sn = src.regno + src.fbyte; sn = copied_msb; sn++)
+ avr_asm_len (adc %0,__zero_reg__, all_regs_rtx[sn], plen, 1);
+ frac_rounded = true;
+ }
+ else if (overlap)
+ {
+ bool use_src
+ = (TEST_HARD_REG_BIT (reg_class_contents[LD_REGS], s0)
+ (IN_RANGE (s0, dest.regno, dest.regno_msb)
+ || reg_unused_after (insn, all_regs_rtx[s0])));
+ xop[2] = all_regs_rtx[s0];
+
Re: [PATCH] Intrinsics for fxsave[,64], xsave[,64], xsaveopt[,64]
Hi,
I've added references to fxsr, xsave and xsaveopt options and builtins
to doc/[invoke,extend].texi.
Is it OK?
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 29a30ee..3aad294 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2013-08-23 Alexander Ivchenko alexander.ivche...@intel.com
+
+ * doc/invoke.texi: Document fxsr, xsave and xsaveopt options.
+ * doc/extend.texi: Document fxsr, xsave and xsaveopt builtins.
+
2013-08-23 Gabriel Dos Reis g...@integrable-solutions.net
* pretty-print.h (pp_newline_and_flush): Declare. Remove macro
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 68d9426..35561a1 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10957,6 +10957,31 @@ unsigned int __builtin_ia32_lzcnt_u32(unsigned int);
unsigned long long __builtin_ia32_lzcnt_u64 (unsigned long long);
@end smallexample
+The following built-in functions are available when @option{-mfxsr} is used.
+All of them generate the machine instruction that is part of the name.
+@smallexample
+void __builtin_ia32_fxsave (void *)
+void __builtin_ia32_fxrstor (void *)
+void __builtin_ia32_fxsave64 (void *)
+void __builtin_ia32_fxrstor64 (void *)
+@end smallexample
+
+The following built-in functions are available when @option{-mxsave} is used.
+All of them generate the machine instruction that is part of the name.
+@smallexample
+void __builtin_ia32_xsave (void *, long long)
+void __builtin_ia32_xrstor (void *, long long)
+void __builtin_ia32_xsave64 (void *, long long)
+void __builtin_ia32_xrstor64 (void *, long long)
+@end smallexample
+
+The following built-in functions are available when
@option{-mxsaveopt} is used.
+All of them generate the machine instruction that is part of the name.
+@smallexample
+void __builtin_ia32_xsaveopt (void *, long long)
+void __builtin_ia32_xsaveopt64 (void *, long long)
+@end smallexample
+
The following built-in functions are available when @option{-mtbm} is used.
Both of them generate the immediate form of the bextr machine instruction.
@smallexample
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 411c8be..77923f3 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -651,10 +651,10 @@ Objective-C and Objective-C++ Dialects}.
-mavx2 -mavx512f -mavx512pf -mavx512er -mavx512cd @gol
-maes -mpclmul -mfsgsbase -mrdrnd -mf16c -mfma @gol
-msse4a -m3dnow -mpopcnt -mabm -mbmi -mtbm -mfma4 -mxop -mlzcnt @gol
--mbmi2 -mrtm -mlwp -mthreads @gol
+-mbmi2 -mfxsr -mxsave -mxsaveopt -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}
+-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
@@ -14412,6 +14412,9 @@ preferred alignment to
@option{-mpreferred-stack-boundary=2}.
@itemx -mno-bmi2
@itemx -mlzcnt
@itemx -mno-lzcnt
+@itemx -mfxsr
+@itemx -mxsave
+@itemx -mxsaveopt
@itemx -mrtm
@itemx -mtbm
@itemx -mno-tbm
@@ -14424,7 +14427,7 @@ preferred alignment to
@option{-mpreferred-stack-boundary=2}.
These switches enable or disable the use of instructions in the MMX, SSE,
SSE2, SSE3, SSSE3, SSE4.1, AVX, AVX2, AVX512F, AVX512PF, AVX512ER, AVX512CD,
AES, PCLMUL, FSGSBASE, RDRND, F16C, FMA, SSE4A, FMA4, XOP, LWP, ABM, BMI, BMI2,
-LZCNT, RTM or 3DNow!@:
+FXSR, XSAVE, XSAVEOPT, LZCNT, RTM or 3DNow!@:
extended instruction sets.
These extensions are also available as built-in functions: see
@ref{X86 Built-in Functions}, for details of the functions enabled and
2013/8/18 Alexander Ivchenko aivch...@gmail.com:
Hi Gerald,
I can certainly address that, will take a look on Mon. Thank you for
pointing this out!
Alexander
2013/8/18 Gerald Pfeifer ger...@pfeifer.com:
Hi Alexander and colleagues,
On Thu, 18 Oct 2012, Alexander Ivchenko wrote:
this patch adds new intrinsics for fxsave, fxsave64, xsave, xsave64,
xsaveopt and xsaveopt64 instructions
I noticed you covered this in the GCC 4.8 release notes (changes.html),
but could not find any reference in the regular GCC documentation
(gcc/doc/*.texi) now.
Is this an omission you are planning to address?
Gerald
wide-int branch updated
fixed fits_uhwi_p.
tested on x86-64.
kenny
Index: gcc/wide-int.h
===
--- gcc/wide-int.h (revision 201985)
+++ gcc/wide-int.h (working copy)
@@ -1650,7 +1650,7 @@ wide_int_ro::fits_shwi_p () const
inline bool
wide_int_ro::fits_uhwi_p () const
{
- return len == 1 || (len == 2 val[1] == 0);
+ return (len == 1 val[0] = 0) || (len == 2 val[1] == 0);
}
/* Return the signed or unsigned min of THIS and C. */
Re: Clean up pretty printers [18/n]
Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza
Re: [Patch, Fortran, OOP] PR 57305: ICE when calling SIZEOF on an unlimited polymorphic variable
Le 26/08/2013 13:22, Janus Weil a écrit : Hi Mikael, the SIZEOF intrinsic currently returns the size according to the *declared* type for polymorphic variables. I think this doesn't really make much sense and it also causes ICEs when SIZEOF is called on CLASS(*) variables (which don't really have a declared type). Therefore I'm proposing to make SIZEOF return the size according to the *dynamic* type instead. The same is done by STORAGE_SIZE (F08), which however gives the size in bits. SIZEOF is a GNU extension, so we are free to decide on its behavior. I don't remember why the declared type was chosen in the first place, and I hope that no one seriously depends on the current behavior (the size of the declared type is probably not really useful after all). I'm slightly inclined to kindly invite the user to switch to STORAGE_SIZE+SIZE instead. Any other opinion? Since the SIZEOF intrinsic has been around for some time in gfortran (before STORAGE_SIZE was available), I would say we should at least continue to support it for backward compatibility. And I guess we should also make it behave reasonably for all inputs. However, it might be worth to add a note in the documentation that STORAGE_SIZE and SIZE should be used instead in standard-conforming code. I thought about it again, and what I'm actually in favor of is diagnosing by default _all_ extensions having a standard replacement. That's an independant question, and not generating wrong code for SIZEOF is certainly good in any case. Mikael
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. -- Gaby
Re: [ping**4] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3
On 08/26/2013 01:16 AM, Bernd Edlinger wrote: PING! This issue is really important. It does not only affect bitfields but all kinds of packed structures. Starting from gcc 4.6.0 there is not a single released version that handles the packed structures correctly. So could some one please approve Sandra's patch now? Indeed. I understand that some people are still unhappy about changing the intended behavior on targets where -fstrict-volatile-bitfields is currently the default, like ARM. But right now all that default means is that we are generating wrong code by default for those targets for volatile bitfield and packed structure accesses -- and by wrong code, that means misaligned memory accesses or losing bits of the field, not just code that doesn't conform to either the C++ memory model or AAPCS. Moreover, -fstrict-volatile-bitfields currently doesn't behave as documented on any target, whether or not it is the default. -Sandra
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. It seems to reproduce quite uniformlny and doesn't seem to be specific to my firefox builds I am doing right now... Thanks! Honza -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. I'll ask to reconsider the priority at the next meeting. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. Jason
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 Ilya Enkovich ilya.enkov...@intel.com * mode-classes.def (MODE_BOUND): New. * tree.def (BOUND_TYPE): New. * genmodes.c (complete_mode): Support MODE_BOUND. (BOUND_MODE): New. (make_bound_mode): New. * machmode.h (BOUND_MODE_P): New. * stor-layout.c (int_mode_for_mode): Support MODE_BOUND. (layout_type): Support BOUND_TYPE. * tree-pretty-print.c (dump_generic_node): Support BOUND_TYPE. * tree.c (build_int_cst_wide): Support BOUND_TYPE. (type_contains_placeholder_1): Likewise. * tree.h (BOUND_TYPE_P): New. * varasm.c (output_constant): Support BOUND_TYPE. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. * config/i386/predicates.md (lea_address_operand): Rename to... (address_no_seg_operand): ... this. (address_mpx_no_base_operand): New. (address_mpx_no_index_operand): New. (bnd_mem_operator): New. * config/i386/i386.opt (mmpx): New. * doc/invoke.texi: Add documentation for the
Re: [Patch, Fortran, OOP] PR 57305: ICE when calling SIZEOF on an unlimited polymorphic variable
I'm slightly inclined to kindly invite the user to switch to STORAGE_SIZE+SIZE instead. Any other opinion? Since the SIZEOF intrinsic has been around for some time in gfortran (before STORAGE_SIZE was available), I would say we should at least continue to support it for backward compatibility. And I guess we should also make it behave reasonably for all inputs. However, it might be worth to add a note in the documentation that STORAGE_SIZE and SIZE should be used instead in standard-conforming code. I thought about it again, and what I'm actually in favor of is diagnosing by default _all_ extensions having a standard replacement. By 'diagnosing' you mean to give a warning? This can already be accomplished by compiling with -std=f2008 -Wintrinsics-std, I guess (not by default, though). Using only -std=f2008 currently results in errors like: undefined reference to `sizeof_' Cheers, Janus
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:00 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? It seems to reproduce quite uniformlny and doesn't seem to be specific to my firefox builds I am doing right now… I think I may have an idea about which existing leak is making itself noticed but I am looking for a (semi-)automatic way to regtest it. Thanks! Honza -- Gaby
Re: Add overload for register_pass
On 08/24/2013 02:33 PM, Oleg Endo wrote: gcc/ChangeLog: * passes.c (register_pass): Add overload. * tree-pass.h (register_pass): Forward declare it. Add comment. Ok. r~
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:00 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . You need plugin enabled buinutils for that. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza
Re: [C++ patch] Set attributes for C++ runtime library calls
On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. I'll ask to reconsider the priority at the next meeting. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. That sounds good to me. Of course it would be really nice to go for this by default and let users overwritting toplevel new to use an option (like we have -freestanding). Can we consider the original patch (that does not change malloc attributes) and then I can make this as an followup? Also if any of these runtime parts are known to be cold in a way that is not obvious to the backend (i.e. that it is reachable only by EH edges or leading to noreturn), it may be good idea to add cold attribute annotations or use use PREDICT_EXPR to mark the unlikely path. I never tought too much about this, but especially the EH machinery seems to generate a lot of code that is executed only for EH. I would like to try to get reorder-blocks to split those parts away to cold text section just as we do with the profile. Thank you, Honza Jason
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 9:38 AM, Jan Hubicka hubi...@ucw.cz wrote: On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. I'll ask to reconsider the priority at the next meeting. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. That sounds good to me. Of course it would be really nice to go for this by default and let users overwritting toplevel new to use an option (like we have -freestanding). Hmm, let's not make it a default. Replacing global operator new (e.g. for tracing purposes) is a valid C++ programming idiom. Can we consider the original patch (that does not change malloc attributes) and then I can make this as an followup? Also if any of these runtime parts are known to be cold in a way that is not obvious to the backend (i.e. that it is reachable only by EH edges or leading to noreturn), it may be good idea to add cold attribute annotations or use use PREDICT_EXPR to mark the unlikely path. I never tought too much about this, but especially the EH machinery seems to generate a lot of code that is executed only for EH. I would like to try to get reorder-blocks to split those parts away to cold text section just as we do with the profile. Thank you, Honza Jason
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:32 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 9:00 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . I got this part. The question I asked relates to the when you said then add -fdump-tree-all into the final link command line options. You need plugin enabled buinutils for that. ah. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza I think the problem is in tree gimple dumpers. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 9:07 AM, Jason Merrill ja...@redhat.com wrote: On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? 3.8/1 says that the object lifetime ends when the storage is reused. So, just returning the pointer itself is not the problem, but returning the problem and reusing the storage (e.g. through construction) may be the problem. The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. Ugh, priority 2 isn't good. I'll ask to reconsider the priority at the next meeting. Yes, please. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. Jason
RFA: fix some avr stdint issues
This patch fixes the gcc.dg/c99-stdint-5.c and gcc.dg/c99-stdint-6.c excess error failures. Tested for atmega128-sim. OK to apply? 2013-05-26 Joern Rennecke joern.renne...@embecosm.com * config/avr/avr-stdint.h (INT16_TYPE): Change default to int. (UINT16_TYPE): Change default to unsigned int. Index: config/avr/avr-stdint.h === --- config/avr/avr-stdint.h (revision 201989) +++ config/avr/avr-stdint.h (working copy) @@ -34,11 +34,11 @@ the Free Software Foundation; either ver #define SIG_ATOMIC_TYPE char #define INT8_TYPE signed char -#define INT16_TYPE (INT_TYPE_SIZE == 16 ? short int : long int) +#define INT16_TYPE (INT_TYPE_SIZE == 16 ? int : long int) #define INT32_TYPE (INT_TYPE_SIZE == 16 ? long int : long long int) #define INT64_TYPE (INT_TYPE_SIZE == 16 ? long long int : 0) #define UINT8_TYPE unsigned char -#define UINT16_TYPE (INT_TYPE_SIZE == 16 ? short unsigned int : long unsigned int) +#define UINT16_TYPE (INT_TYPE_SIZE == 16 ? unsigned int : long unsigned int) #define UINT32_TYPE (INT_TYPE_SIZE == 16 ? long unsigned int : long long unsigned int) #define UINT64_TYPE (INT_TYPE_SIZE == 16 ? long long unsigned int : 0)
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:32 AM, Jan Hubicka hubi...@ucw.cz wrote: modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . You need plugin enabled buinutils for that. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza by the way, we should be leaking less now that we release the memory acquire in obstack for local pretty printers. and discharged pretty printers. However, I think we still have problems with: (1) global pretty printers, (2) when we do ggc_strdup on the formatted text. One of the local patches I have avoids the copy and GGC involvement. Please let me know which of the above Make linker variables I should modify. -- Gaby
Re: RFA: fix find_valid_class to accept classes w/out last hard reg
Joern Rennecke wrote: 2013-05-02 Joern Rennecke joern.renne...@embecosm.com * reload.c (find_valid_class): Allow classes that do not include FIRST_PSEUDO_REGISTER - 1. This is OK. Thanks, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain ulrich.weig...@de.ibm.com
ELF interposition and One Definition Rule
Hi, cgraph_function_body_availability determine if body of a given function is available in current compilation unit and if so, if it may be overwritten by completely different semantic by (dynamic)linker. Function that is AVAIL_AVAILABLE can still be repleaced by a function fron different unit, but its effects must be the same. Our default behaviour special case inline functions that are always AVAIL_AVAILABLE and, via decl_replaceable_p, also any COMDAT (that may be for functions since all COMDATs are also inlines, but makes difference for variables I think). In the variable initializer case we also special case DECL_VIRTUAL and assume that there is only one possible initializer for every virtual variable. The performance implications of cgraph_function_body_availability are important; -fPIC costs over 10% of performance but making everything AVAIL_AVAILABLE cuts the costs well under 1%. My understanding of C++ One Definition Rule, in a strict sense, does not a allow in to define two functions of the same name and different semantics in a valid program . I also think that all DSOs eventually linked together or dlopenned are part of the same program. So theoretically, for C++ produced code, we may go with AVAIL_AVAILABLE everywhere. After some discussion on mainline list Michael Matz pointed out that one can understand ODR in a sense that the interposed function was never part of the program, since it is completely replaced. On IRC we got into an agreement that we may disallow interposition for virtuals, since replacing these seems fishy - they don't even have address well defined and compiler can freely duplicate and/or unify them. I think same apply for C++ constructors and destructors now. Does the following patch seems sane? If there will be no opposition till end of week, I will go ahead with it. Of course I would be happier with a stronger rule - for instance allowing interposition only on plain functions not on methods. Main benefits of AVAILABLE is inlining, but it also helps to avoid expensive dynamic relocations. Making virtual functions AVAILABLE will allow us to hide them fron interaces of shared libraries. This should turn good part of non-local dynamic relocations into local dynamic relocations in practice. Bootstrapped/regtested ppc64-linux. Honza * cgraph.c (cgraph_function_body_availability): Also return AVAIL_AVAILABLE for virtual functions, constructors and destructors. Index: cgraph.c === --- cgraph.c(revision 201996) +++ cgraph.c(working copy) @@ -2035,6 +2052,29 @@ cgraph_function_body_availability (struc behaviour on replacing inline function by different body. */ else if (DECL_DECLARED_INLINE_P (node-symbol.decl)) avail = AVAIL_AVAILABLE; + /* C++ One Deifnition Rule states that in the entire program, an object or + non-inline function cannot have more than one definition; if an object or + function is used, it must have exactly one definition. You can declare an + object or function that is never used, in which case you don't have to + provide a definition. In no event can there be more than one definition. + + In the interposition done by linking or dynamic linking one function + is replaced by another. Direct interpretation of C++ ODR would imply + that both functions must origin from the same definition and thus be + semantically equivalent and we should always return AVAIL_AVAILABLE. + + We however opted to be more conservative and allow interposition + for common (noninline) functions and methods based on interpretation + of ODR that simply consider the replaced function definition to not + be part of the program. + + We however do allow interposition of virtual functions on the basis that + their address is not well defined and compiler is free to duplicate their + bodies same way as it does with inline functions. */ + else if (DECL_VIRTUAL_P (node-symbol.decl) + || DECL_CXX_CONSTRUCTOR_P (node-symbol.decl) + || DECL_CXX_DESTRUCTOR_P (node-symbol.decl)) +avail = AVAIL_AVAILABLE; /* If the function can be overwritten, return OVERWRITABLE. Take care at least of two notable extensions - the COMDAT functions
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:32 AM, Jan Hubicka hubi...@ucw.cz wrote: modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . You need plugin enabled buinutils for that. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza by the way, we should be leaking less now that we release the memory acquire in obstack for local pretty printers. and discharged pretty printers. However, I think we still have problems with: (1) global pretty printers, (2) when we do ggc_strdup on the formatted text. One of the local patches I have avoids the copy and GGC involvement. Please let me know which of the above Make linker variables I should modify. My favorite way to do so is to wait for compilation to get into linking stage and then simply cutpaste the command line and modify it myself. Otherwise it is bit inpractical: we link many times (every binary twice just for checksum :( ) and thus you would end up with quite massive disk usage I would guess. Honza -- Gaby
Fix edge redirection in ipa-inline-transform
Hi,
this patch fixes bug that triggers an ICE when building firefox;
cgraph_redirect_edge_call_stmt_to_callee can now remove an edge it is
given and thus inline_transform must keep track of next pointer.
Bootstrapped/regtested ppc64-linux, commited.
Index: ChangeLog
===
--- ChangeLog (revision 201996)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
2013-08-26 Jan Hubicka j...@suse.cz
+ * ipa-inline-transform.c (inline_transform): Be ready for edge
+ to be changed by edge redirection.
+
+2013-08-26 Jan Hubicka j...@suse.cz
+
* cgraph.c (cgraph_speculative_call_info): Fix parameter order and
formating;
add sanity check.
(cgraph_resolve_speculation): Add FIXME about scaling profiles.
Index: ipa-inline-transform.c
===
--- ipa-inline-transform.c (revision 201995)
+++ ipa-inline-transform.c (working copy)
@@ -412,7 +412,7 @@ unsigned int
inline_transform (struct cgraph_node *node)
{
unsigned int todo = 0;
- struct cgraph_edge *e;
+ struct cgraph_edge *e, *next;
/* FIXME: Currently the pass manager is adding inline transform more than
once to some clones. This needs revisiting after WPA cleanups. */
@@ -424,8 +424,11 @@ inline_transform (struct cgraph_node *no
if (preserve_function_body_p (node))
save_inline_function_body (node);
- for (e = node-callees; e; e = e-next_callee)
-cgraph_redirect_edge_call_stmt_to_callee (e);
+ for (e = node-callees; e; e = next)
+{
+ next = e-next_callee;
+ cgraph_redirect_edge_call_stmt_to_callee (e);
+}
ipa_remove_all_references (node-symbol.ref_list);
timevar_push (TV_INTEGRATION);
Fix ICE with -fdump-ipa-cp
Hi,
this patch fixes fallout of my patch to remove DECL_ARGUMENT from
WPA stage. Fixed thus.
Regtested/bootstrapped x86_64-linux, comitted.
Honza
Index: ChangeLog
===
--- ChangeLog (revision 201997)
+++ ChangeLog (working copy)
@@ -1,5 +1,9 @@
2013-08-26 Jan Hubicka j...@suse.cz
+ * ipa-prop.c (ipa_print_node_params): Do not ICE during WPA.
+
+2013-08-26 Jan Hubicka j...@suse.cz
+
* ipa-inline-transform.c (inline_transform): Be ready for basic block
to be changed by edge redirection.
Index: ipa-prop.c
===
--- ipa-prop.c (revision 201995)
+++ ipa-prop.c (working copy)
@@ -3041,7 +3041,6 @@ void
ipa_print_node_params (FILE *f, struct cgraph_node *node)
{
int i, count;
- tree temp;
struct ipa_node_params *info;
if (!node-symbol.definition)
@@ -3054,12 +3053,7 @@ ipa_print_node_params (FILE *f, struct c
{
int c;
- temp = ipa_get_param (info, i);
- if (TREE_CODE (temp) == PARM_DECL)
- fprintf (f, param %d : %s, i,
- (DECL_NAME (temp)
- ? (*lang_hooks.decl_printable_name) (temp, 2)
- : (unnamed)));
+ ipa_dump_param (f, info, i);
if (ipa_is_param_used (info, i))
fprintf (f, used);
c = ipa_get_controlled_uses (info, i);
Fix cgraph_redirect_edge_call_stmt_to_callee
Hi,
this patch fixes interesting problem in
cgraph_redirect_edge_call_stmt_to_callee.
Before expanding the speculative call we do type checking ensuring that type of
callee
is compatible with a call. This fails when ipa-cp decides to redirect the call
and
change function signature, because the call type is not updated yet. We need
to compare with type of function we have in reference (that is original function
before ipa-cp change).
This allows us to do 400 more speculative calls on firefox. Still 10 of them
fail
for reasons I do not quite understand and will try to debug next.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
Index: ChangeLog
===
--- ChangeLog (revision 201998)
+++ ChangeLog (working copy)
@@ -1,5 +1,11 @@
2013-08-26 Jan Hubicka j...@suse.cz
+ * cgraph.c (cgraph_redirect_edge_call_stmt_to_callee): Fix formatting;
+ fix edge count/frequency when speculation failed; fix type check
+ for the direct call.
+
+2013-08-26 Jan Hubicka j...@suse.cz
+
* ipa-prop.c (ipa_print_node_params): Do not ICE during WPA.
2013-08-26 Jan Hubicka j...@suse.cz
Index: cgraph.c
===
--- cgraph.c(revision 201996)
+++ cgraph.c(working copy)
@@ -1306,29 +1306,44 @@ cgraph_redirect_edge_call_stmt_to_callee
struct ipa_ref *ref;
cgraph_speculative_call_info (e, e, e2, ref);
- /* If there already is an direct call (i.e. as a result of inliner's
substitution),
-forget about speculating. */
+ /* If there already is an direct call (i.e. as a result of inliner's
+substitution), forget about speculating. */
if (decl)
e = cgraph_resolve_speculation (e, decl);
- /* If types do not match, speculation was likely wrong. */
- else if (!gimple_check_call_matching_types (e-call_stmt,
e-callee-symbol.decl,
+ /* If types do not match, speculation was likely wrong.
+ The direct edge was posisbly redirected to the clone with a different
+signature. We did not update the call statement yet, so compare it
+with the reference that still points to the proper type. */
+ else if (!gimple_check_call_matching_types (e-call_stmt,
+ ref-referred-symbol.decl,
true))
{
if (dump_file)
fprintf (dump_file, Not expanding speculative call of %s/%i -
%s/%i\n
Type mismatch.\n,
-xstrdup (cgraph_node_name (e-caller)),
e-caller-symbol.order,
-xstrdup (cgraph_node_name (e-callee)),
e-callee-symbol.order);
+xstrdup (cgraph_node_name (e-caller)),
+e-caller-symbol.order,
+xstrdup (cgraph_node_name (e-callee)),
+e-callee-symbol.order);
e = cgraph_resolve_speculation (e, NULL);
+ /* We are producing the final function body and will throw away the
+callgraph edges really soon. Reset the counts/frequencies to
+keep verifier happy in the case of roundoff errors. */
+ e-count = gimple_bb (e-call_stmt)-count;
+ e-frequency = compute_call_stmt_bb_frequency
+ (e-caller-symbol.decl, gimple_bb (e-call_stmt));
}
/* Expand speculation into GIMPLE code. */
else
{
if (dump_file)
- fprintf (dump_file, Expanding speculative call of %s/%i - %s/%i
count:
+ fprintf (dump_file,
+Expanding speculative call of %s/%i - %s/%i count:
HOST_WIDEST_INT_PRINT_DEC\n,
-xstrdup (cgraph_node_name (e-caller)),
e-caller-symbol.order,
-xstrdup (cgraph_node_name (e-callee)),
e-callee-symbol.order,
+xstrdup (cgraph_node_name (e-caller)),
+e-caller-symbol.order,
+xstrdup (cgraph_node_name (e-callee)),
+e-callee-symbol.order,
(HOST_WIDEST_INT)e-count);
gcc_assert (e2-speculative);
push_cfun (DECL_STRUCT_FUNCTION (e-caller-symbol.decl));
@@ -1342,11 +1357,12 @@ cgraph_redirect_edge_call_stmt_to_callee
: REG_BR_PROB_BASE / 2,
e-count, e-count + e2-count);
e-speculative = false;
- cgraph_set_call_stmt_including_clones (e-caller, e-call_stmt,
new_stmt, false);
- e-frequency = compute_call_stmt_bb_frequency (e-caller-symbol.decl,
-gimple_bb
(e-call_stmt));
- e2-frequency = compute_call_stmt_bb_frequency
(e2-caller-symbol.decl,
- gimple_bb
(e2-call_stmt));
+
Minor gimple_get_virt_method_for_binfo tweek
Hi,
any time we look for constructor of a given readonly static var, we should use
ctor_for_folding that knows about interposition rules and knows how to walk
aliases.
gimple_get_virt_method_for_binfo is hopefully last place I forgot to update.
Bootstrapped/regtested ppc64-linux, committed.
* gimple-fold.c (gimple_get_virt_method_for_binfo): Use
ctor_for_folding.
Index: gimple-fold.c
===
--- gimple-fold.c (revision 202000)
+++ gimple-fold.c (working copy)
@@ -3097,7 +3097,7 @@ tree
gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree known_binfo)
{
unsigned HOST_WIDE_INT offset, size;
- tree v, fn, vtable;
+ tree v, fn, vtable, init;
vtable = v = BINFO_VTABLE (known_binfo);
/* If there is no virtual methods table, leave the OBJ_TYPE_REF alone. */
@@ -3117,14 +3117,24 @@ gimple_get_virt_method_for_binfo (HOST_W
v = TREE_OPERAND (v, 0);
if (TREE_CODE (v) != VAR_DECL
- || !DECL_VIRTUAL_P (v)
- || !DECL_INITIAL (v)
- || DECL_INITIAL (v) == error_mark_node)
+ || !DECL_VIRTUAL_P (v))
return NULL_TREE;
+ init = ctor_for_folding (v);
+
+ /* The virtual tables should always be born with constructors.
+ and we always should assume that they are avaialble for
+ folding. At the moment we do not stream them in all cases,
+ but it should never happen that ctor seem unreachable. */
+ gcc_assert (init);
+ if (init == error_mark_node)
+{
+ gcc_assert (in_lto_p);
+ return NULL_TREE;
+}
gcc_checking_assert (TREE_CODE (TREE_TYPE (v)) == ARRAY_TYPE);
size = tree_low_cst (TYPE_SIZE (TREE_TYPE (TREE_TYPE (v))), 1);
offset += token * size;
- fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v),
+ fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), init,
offset, size, vtable);
if (!fn || integer_zerop (fn))
return NULL_TREE;
C++ constructors/destructors can not have address taken.
Hi,
this patch tesch comdat_can_be_unshared_p_1 about the fact that C++
constructors and destructors can not have address taken that can be used for
equivalence comparsion. This mean that we can privatize them into the DSO.
Bootstrapped/regtested x86_64-linux, comitted.
* ipa.c (comdat_can_be_unshared_p_1): C++ constructors and destructors
can be unshared.
Index: ipa.c
===
--- ipa.c (revision 201995)
+++ ipa.c (working copy)
@@ -574,9 +574,13 @@ static bool
comdat_can_be_unshared_p_1 (symtab_node node)
{
/* When address is taken, we don't know if equality comparison won't
- break eventually. Exception are virutal functions and vtables,
- where this is not possible by language standard. */
+ break eventually. Exception are virutal functions, C++
+ constructors/destructors and vtables, where this is not possible by
+ language standard. */
if (!DECL_VIRTUAL_P (node-symbol.decl)
+ (TREE_CODE (node-symbol.decl) != FUNCTION_DECL
+ || (!DECL_CXX_CONSTRUCTOR_P (node-symbol.decl)
+ !DECL_CXX_DESTRUCTOR_P (node-symbol.decl)))
address_taken_from_non_vtable_p (node))
return false;
Do not turn virtual methods cold based on absence of direct calls
Hi,
on firefox we turn some virtual methods cold just because we think that if they
are not called directly and not having address taken, we won't need them.
Unless we track call targets of polymorphic calls better, we should not try
to touch them until after inlining (when those will just disappear).
Bootstrapped/regtested x86_64-linux, comitted.
Index: ChangeLog
===
--- ChangeLog (revision 202002)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
2013-08-26 Jan Hubicka j...@suse.cz
+ * cgraph.c (cgraph_propagate_frequency): Do not assume that virtual
+ methods can not be called indirectly when their address is not taken.
+
+2013-08-26 Jan Hubicka j...@suse.cz
+
* gimple-fold.c (gimple_get_virt_method_for_binfo): Use
ctor_for_folding.
2013-08-26 Jan Hubicka j...@suse.cz
Index: cgraph.c
===
--- cgraph.c(revision 202000)
+++ cgraph.c(working copy)
@@ -2348,7 +2348,10 @@ cgraph_propagate_frequency (struct cgrap
struct cgraph_propagate_frequency_data d = {true, true, true, true};
bool changed = false;
- if (!node-local.local)
+ /* We can not propagate anything useful about externally visible functions
+ nor about virtuals. */
+ if (!node-local.local
+ || (flag_devirtualize DECL_VIRTUAL_P (node-symbol.decl)))
return false;
gcc_assert (node-symbol.analyzed);
if (dump_file (dump_flags TDF_DETAILS))
RFA: prefer double over same-size float as conversion result
This fixes the following avr failures:
FAIL: gcc.dg/Wdouble-promotion.c (test for warnings, line 45)
FAIL: gcc.dg/Wdouble-promotion.c (test for warnings, line 52)
FAIL: gcc.dg/Wdouble-promotion.c (test for warnings, line 53)
FAIL: gcc.dg/Wdouble-promotion.c (test for warnings, line 56)
FAIL: gcc.dg/pr44214-2.c scan-tree-dump-times original \\* 1
bootstrapped/regtested on i686-pc-linux-gnu
OK to apply?
2013-05-14 Joern Rennecke joern.renne...@embecosm.com
* c-typeck.c (c_common_type): Prefer double_type_node over
other REAL_TYPE types with the same precision.
(convert_arguments): Likewise.
Index: c/c-typeck.c
===
--- c/c-typeck.c(revision 201992)
+++ c/c-typeck.c(working copy)
@@ -919,6 +919,13 @@ c_common_type (tree t1, tree t2)
|| TYPE_MAIN_VARIANT (t2) == long_double_type_node)
return long_double_type_node;
+ /* Likewise, prefer double to float even if same size.
+ We got a couple of embedded targets with 32 bit doubles, and the
+ pdp11 might have 64 bit floats. */
+ if (TYPE_MAIN_VARIANT (t1) == double_type_node
+ || TYPE_MAIN_VARIANT (t2) == double_type_node)
+return double_type_node;
+
/* Otherwise prefer the unsigned one. */
if (TYPE_UNSIGNED (t1))
@@ -3156,7 +3163,9 @@ convert_arguments (tree typelist, vectr
}
else if (TREE_CODE (valtype) == REAL_TYPE
(TYPE_PRECISION (valtype)
- TYPE_PRECISION (double_type_node))
+ = TYPE_PRECISION (double_type_node))
+ valtype != double_type_node
+ valtype != long_double_type_node
!DECIMAL_FLOAT_MODE_P (TYPE_MODE (valtype)))
{
if (type_generic)
Re: [PATCH v3 12/18] convert the Go front end to automatic dependencies
Ian == Ian Lance Taylor i...@google.com writes: Ian I assume that dropping $(OUTPUT_OPTION) is correct--I haven't looked Ian at the new definition of $(COMPILE). I believe the depcomp script takes care of this. Tom
Re: [Ping^4] [Patch, AArch64, ILP32] 3/5 Minor change in function.c:assign_parm_find_data_types()
On 08/15/2013 11:21 AM, Yufeng Zhang wrote: Ping^4~ I am aware that it is currently holiday season, but it would be really nice if this tiny patch can get some further comments even if it is not an approval. The original RFA email is here: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01485.html Ok. It probably would have helped to have included the link or the patch itself in the various pings. It would have also been helpful to note that this makes the callee side match up with the caller side in calls.c:1247, initialize_argument_information. r~
Re: RFA: prefer double over same-size float as conversion result
On 08/26/2013 09:07 AM, Joern Rennecke wrote: 2013-05-14 Joern Rennecke joern.renne...@embecosm.com * c-typeck.c (c_common_type): Prefer double_type_node over other REAL_TYPE types with the same precision. (convert_arguments): Likewise. Ok. r~
Re: [PATCH i386 2/8] [AVX512] Add mask registers.
On 22 Aug 08:49, Richard Henderson wrote:
Hello,
You can always split away the clobber after reload, as we do for
when add gets implemented with lea.
I've refactored the patch, making mask logic insn patterns non-unspec.
Unfortunately I was unable to use '*' in mask alternative, since it is
not working. So, I've put '!' and all gooes fine. '?' is not working
as well.
Vlad, could you pls clarify, if it is ok to use '!'?
ChangeLog:
2013-07-24 Alexander Ivchenko alexander.ivche...@intel.com
Maxim Kuznetsov maxim.kuznet...@intel.com
Sergey Lega sergey.s.l...@intel.com
Anna Tikhonova anna.tikhon...@intel.com
Ilya Tocar ilya.to...@intel.com
Andrey Turetskiy andrey.turets...@intel.com
Ilya Verbin ilya.ver...@intel.com
Kirill Yukhin kirill.yuk...@intel.com
Michael Zolotukhin michael.v.zolotuk...@intel.com
* config/i386/constraints.md (k): New.
(Yk): Ditto.
* config/i386/i386.c (const regclass_map): Add new mask registers.
(dbx_register_map): Ditto.
(dbx64_register_map): Ditto.
(svr4_dbx_register_map): Ditto.
(ix86_conditional_register_usage): Squash mask registers if AVX512F is
disabled.
(ix86_preferred_reload_class): Disable constants for mask registers.
(ix86_secondary_reload): Do spill of mask register using 32-bit insn.
(ix86_hard_regno_mode_ok): Support new mask registers.
(x86_order_regs_for_local_alloc): Ditto.
* config/i386/i386.h (FIRST_PSEUDO_REGISTER): Update.
(FIXED_REGISTERS): Add new mask registers.
(CALL_USED_REGISTERS): Ditto.
(REG_ALLOC_ORDER): Ditto.
(VALID_MASK_REG_MODE): New.
(FIRST_MASK_REG): Ditto.
(LAST_MASK_REG): Ditto.
(reg_class): Add MASK_EVEX_REGS, MASK_REGS.
(MAYBE_MASK_CLASS_P): New.
(REG_CLASS_NAMES): Add MASK_EVEX_REGS, MASK_REGS.
(REG_CLASS_CONTENTS): Ditto.
(MASK_REGNO_P): New.
(ANY_MASK_REG_P): Ditto.
(HI_REGISTER_NAMES): Add new mask registers.
* config/i386/i386.md (MASK0_REG, MASK1_REG, MASK2_REG,
MASK3_REG, MASK4_REG, MASK5_REG, MASK6_REG,
MASK7_REG): Constants for new mask registers.
(attribute type): Add mskmov, msklog.
(attribute length_immediate): Support them.
(attribute memory): Ditto.
(attribute prefix_0f): Ditto.
(*movhi_internal): Support new mask registers.
(*movqi_internal): Ditto.
(define_split): Split out clobber pattern is a logic
insn on mask registers.
(*klogicmode): New.
(andhi_1): Make code visible, extend to support mask regs.
(*andqi_1): Extend to support mask regs.
(kandnmode): New.
(define_split): Split and-not to and and not if operands
are not mask regs.
(*codemode_1): Separate HI mode to new pattern...
(codehi_1): This.
(*codeqi_1): Extend to support mask regs.
(kxnormode): New.
(define_split): New split for not-xor.
(kortestzhi): new.
(kortestchi): Ditto.
(kunpckhi): Ditto.
(*one_cmplmode2_1): Remove HImode and handle it...
(*one_cmplhi2_1): ...Here, now with mask registers support.
(*one_cmplqi2_1): Support new mask registers.
(HI/QImode arithmetics splitter): Don't split if mask registers are
used.
(HI/QImode not splitter): Ditto.
Testing:
1. Bootstrap pass.
2. make check shows no regressions.
3. Spec 2000 2006 build show no regressions both with and without -mavx512f
option.
4. Spec 2000 2006 run shows no stability regressions without -mavx512f
option.
Is it ok?
Thanks, K
---
gcc/config/i386/constraints.md | 8 +-
gcc/config/i386/i386.c | 34 +++--
gcc/config/i386/i386.h | 40 --
gcc/config/i386/i386.md| 279 ++---
gcc/config/i386/predicates.md | 4 +
5 files changed, 306 insertions(+), 59 deletions(-)
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 28e626f..92e0c05 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -19,7 +19,7 @@
;;; Unused letters:
;;; B H T
-;;; h jk
+;;; h j
;; Integer register constraints.
;; It is not necessary to define 'r' here.
@@ -78,6 +78,12 @@
TARGET_80387 || TARGET_FLOAT_RETURNS_IN_80387 ? FP_SECOND_REG : NO_REGS
Second from top of 80387 floating-point stack (@code{%st(1)}).)
+(define_register_constraint k TARGET_AVX512F ? MASK_EVEX_REGS : NO_REGS
+@internal Any mask register that can be used as predicate, i.e. k1-k7.)
+
+(define_register_constraint Yk TARGET_AVX512F ? MASK_REGS : NO_REGS
+@internal Any mask register.)
+
;; Vector registers (also used for plain floating point nowadays).
(define_register_constraint y TARGET_MMX ? MMX_REGS : NO_REGS
Any MMX
RFA: Consider int and same-size short as equivalent vector components
avr currently shows the following failure: FAIL: c-c++-common/vector-scalar.c -Wc++-compat (test for excess errors) Excess errors: /home/amylaar/atmel/4.8/unisrc-mainline/gcc/testsuite/c-c++-common/vector-scalar .c:9:34: error: invalid operands to binary | (have '__vector(8) int' and 'veci') The issue is that when we compute the result of an operatiopn of a veci and an int, we get a __vector(8) int result (int is the same size as short), yet the typechecking later does not accept the vectors as compatible. Fixed by relaxing the latter for the case that int and short are the same size. bootstrapped / regtested on i686-pc-linux-gnu. OK to apply? 2013-07-17 Joern Rennecke joern.renne...@embecosm.com * c-common.c (same_scalar_type_ignoring_signedness): Also accept short/int as equivalent if they have the same precision. Index: c-family/c-common.c === --- c-family/c-common.c (revision 201992) +++ c-family/c-common.c (working copy) @@ -10700,10 +10700,22 @@ same_scalar_type_ignoring_signedness (tr (c2 == INTEGER_TYPE || c2 == REAL_TYPE || c2 == FIXED_POINT_TYPE)); + t1 = c_common_signed_type (t1); + t2 = c_common_signed_type (t2); /* Equality works here because c_common_signed_type uses TYPE_MAIN_VARIANT. */ - return c_common_signed_type (t1) -== c_common_signed_type (t2); + if (t1 == t2) +return true; + if (TYPE_PRECISION (t1) != TYPE_PRECISION (t2)) +return false; + /* When short and int are the same size, we promote vectors of short + to vectors of int when doing arithmetic with scalars. Hence, + we also have to accept mixing short / int vectors in this case. + Example: c-c++-common/vector-scalar.c for target avr. */ + if ((t1 == integer_type_node t2 == short_integer_type_node) + || (t2 == integer_type_node t1 == short_integer_type_node)) +return true; + return false; } /* Check for missing format attributes on function pointers. LTYPE is
Re: RFA: fix some avr stdint issues
2013/8/26 Joern Rennecke joern.renne...@embecosm.com: This patch fixes the gcc.dg/c99-stdint-5.c and gcc.dg/c99-stdint-6.c excess error failures. Tested for atmega128-sim. OK to apply? 2013-05-26 Joern Rennecke joern.renne...@embecosm.com * config/avr/avr-stdint.h (INT16_TYPE): Change default to int. (UINT16_TYPE): Change default to unsigned int. It seems reasonable for me. Please apply. Denis.
Re: RFA: prefer double over same-size float as conversion result
In convert_arguments I think you should be comparing TYPE_MAIN_VARIANT (valtype) against double_type_node and long_double_type_node, rather than just valtype. This is PR c/35649 (so include that number in your ChangeLog entry and close that bug as fixed). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH i386 2/8] [AVX512] Add mask registers.
On 08/26/2013 09:13 AM, Kirill Yukhin wrote:
+(define_split
+ [(set (match_operand:SWI12 0 mask_reg_operand)
+ (any_logic:SWI12 (match_operand:SWI12 1 mask_reg_operand)
+ (match_operand:SWI12 2 mask_reg_operand)))
+ (clobber (reg:CC FLAGS_REG))]
+ TARGET_AVX512F reload_completed
+ [(set (match_operand:SWI12 0 mask_reg_operand)
+ (any_logic:SWI12 (match_operand:SWI12 1 mask_reg_operand)
+ (match_operand:SWI12 2 mask_reg_operand)))])
You must you match_dup on the new pattern half of define_split.
This pattern must never have triggered during your tests, since
it should have resulted in garbage rtl, and an ICE.
+(define_insn kandnmode
+ [(set (match_operand:SWI12 0 register_operand =r,!Yk)
+ (and:SWI12
+ (not:SWI12
+ (match_operand:SWI12 1 register_operand 0,Yk))
+ (match_operand:SWI12 2 register_operand r,Yk)))]
+ TARGET_AVX512F
+ @
+ #
+ kandnw\t{%2, %1, %0|%0, %1, %2}
+ [(set_attr type *,msklog)
+ (set_attr prefix *,vex)
+ (set_attr mode MODE)])
What happened to the bmi andn alternative we discussed?
(define_insn *andqi_1
- [(set (match_operand:QI 0 nonimmediate_operand =qm,q,r)
- (and:QI (match_operand:QI 1 nonimmediate_operand %0,0,0)
- (match_operand:QI 2 general_operand qn,qmn,rn)))
+ [(set (match_operand:QI 0 nonimmediate_operand =qm,q,r,!Yk)
+ (and:QI (match_operand:QI 1 nonimmediate_operand %0,0,0,Yk)
+ (match_operand:QI 2 general_operand qn,qmn,rn,Yk)))
(clobber (reg:CC FLAGS_REG))]
ix86_binary_operator_ok (AND, QImode, operands)
@
and{b}\t{%2, %0|%0, %2}
and{b}\t{%2, %0|%0, %2}
- and{l}\t{%k2, %k0|%k0, %k2}
- [(set_attr type alu)
- (set_attr mode QI,QI,SI)])
+ and{l}\t{%k2, %k0|%k0, %k2}
+ #
+ [(set_attr type alu,alu,alu,msklog)
+ (set_attr mode QI,QI,SI,HI)])
Why force the split? You can write the kand here...
+(define_insn codehi_1
+ [(set (match_operand:HI 0 nonimmediate_operand =r,rm,!Yk)
+ (any_or:HI
+ (match_operand:HI 1 nonimmediate_operand %0,0,Yk)
+ (match_operand:HI 2 general_operand g,ri,Yk)))
+ (clobber (reg:CC FLAGS_REG))]
+ ix86_binary_operator_ok (CODE, HImode, operands)
+ @
+ logic{w}\t{%2, %0|%0, %2}
+ logic{w}\t{%2, %0|%0, %2}
+ #
+ [(set_attr type alu,alu,msklog)
+ (set_attr mode HI)])
Likewise.
The point being that with optimization enabled, we will have run the split and
gotten all of the performance benefit of eliding the clobber. But with
optimization disabled, we don't need the split for correctness.
+(define_insn kunpckhi
+ [(set (match_operand:HI 0 register_operand =Yk)
+ (ior:HI
+ (ashift:HI
+ (match_operand:HI 1 register_operand Yk)
+ (const_int 8))
+ (zero_extend:HI (subreg:QI (match_operand:HI 2 register_operand
Yk) 0]
+ TARGET_AVX512F
+ kunpckbw\t{%2, %1, %0|%0, %1, %2}
+ [(set_attr mode HI)
+ (set_attr type msklog)
+ (set_attr prefix vex)])
Don't write the subreg explicitly. Instead, use a match_operand:QI, which will
match the whole (subreg (reg)) expression, and also something that the combiner
could simplify out of that.
+(define_insn *one_cmplhi2_1
+ [(set (match_operand:HI 0 nonimmediate_operand =rm,Yk)
+ (not:HI (match_operand:HI 1 nonimmediate_operand 0,Yk)))]
+ ix86_unary_operator_ok (NOT, HImode, operands)
...
(define_insn *one_cmplqi2_1
- [(set (match_operand:QI 0 nonimmediate_operand =qm,r)
- (not:QI (match_operand:QI 1 nonimmediate_operand 0,0)))]
+ [(set (match_operand:QI 0 nonimmediate_operand =qm,r,*Yk)
+ (not:QI (match_operand:QI 1 nonimmediate_operand 0,0,*Yk)))]
Forgotten ! for Yk alternatives.
+ TARGET_AVX512F !ANY_MASK_REG_P (operands [0])
...
+;; Do not split instructions with mask registers.
(define_split
...
+(! ANY_MASK_REG_P (operands[0])
+ || ! ANY_MASK_REG_P (operands[1])
+ || ! ANY_MASK_REG_P (operands[2]))
This ugliness is why I suggested adding a general_reg_operand in our last
conversation.
r~
Re: RFA: fix avr gcc.dg/fixed-point/convert-accum-neg.c execution failure
2013/8/26 Joern Rennecke joern.renne...@embecosm.com: The gcc.dg/fixed-point/convert-accum-neg.c execution test fails for avr because for fractional integer to accumulator / integer conversions, the avr target rounds towards -infinity, whereas we are supposed to round towards 0. The attached patch implements rounding towards 0, and adds an option -mfract-convert-truncate to revert to the previous behaviour (for situations where smaller code is more important than proper rounding). Tested for atmega128-sim. OK to apply? 2013-06-25 Joern Rennecke joern.renne...@embecosm.com * config/avr/avr.opt (mfract-convert-truncate): New option. * config/avr/avr.c (avr_out_fract): Unless TARGET_FRACT_CONV_TRUNC is set, round negative fractional integers according to n1169 when converting to integer types. Approved. Denis.
Re: [PATCH] libitm: Add custom HTM fast path for RTM on x86_64.
On 08/22/2013 02:57 PM, Torvald Riegel wrote: On Thu, 2013-08-22 at 12:05 -0700, Richard Henderson wrote: On 08/22/2013 11:39 AM, Torvald Riegel wrote: + /* Store edi for future HTM fast path retries. We use a stack slot + lower than the jmpbuf so that the jmpbuf's rip field will overlap + with the proper return address on the stack. */ + movl%edi, -64(%rsp) You havn't allocated the stack frame here, and you're storing outside the redzone. This is invalid. Two possibilities: (1) always allocate the stack frame on entry to the function (adds two register additions to the htm fast path -- in the noise i'd think) (2) store the edi value in the non-htm path, with the pr_HTMRetryableAbort bit or'd in. (adds an extra store to the non-htm path; probably noise). You'd want to mask out that bit when you reload it. Oops. Picked fix (2). Better now? Move the andl of edi down into the HAVE_AS_RTM block, above the orl of HTMRetriedAfterAbort. Ok with that change. r~
Re: PING: Fwd: Re: [patch] implement Cilk Plus simd loops on trunk
+static tree
+c_check_cilk_loop_incr (location_t loc, tree decl, tree incr)
+{
+ if (EXPR_HAS_LOCATION (incr))
+loc = EXPR_LOCATION (incr);
+
+ if (!incr)
+{
+ error_at (loc, missing increment);
+ return error_mark_node;
+}
Either these tests are swapped, or the second one isn't really needed.
+ switch (TREE_CODE (incr))
+{
+case POSTINCREMENT_EXPR:
+case PREINCREMENT_EXPR:
+case POSTDECREMENT_EXPR:
+case PREDECREMENT_EXPR:
+ if (TREE_OPERAND (incr, 0) != decl)
+ break;
+
+ // Bah... canonicalize into whatever OMP_FOR_INCR needs.
+ if (POINTER_TYPE_P (TREE_TYPE (decl))
+TREE_OPERAND (incr, 1))
+ {
+ tree t = fold_convert_loc (loc,
+ sizetype, TREE_OPERAND (incr, 1));
+
+ if (TREE_CODE (incr) == POSTDECREMENT_EXPR
+ || TREE_CODE (incr) == PREDECREMENT_EXPR)
+ t = fold_build1_loc (loc, NEGATE_EXPR, sizetype, t);
+ t = fold_build_pointer_plus (decl, t);
+ incr = build2 (MODIFY_EXPR, void_type_node, decl, t);
+ }
+ return incr;
Handling pointer types and pointer_plus_expr here (p++) ...
+case MODIFY_EXPR:
+ {
+ tree rhs;
+
+ if (TREE_OPERAND (incr, 0) != decl)
+ break;
+
+ rhs = TREE_OPERAND (incr, 1);
+ if (TREE_CODE (rhs) == PLUS_EXPR
+ (TREE_OPERAND (rhs, 0) == decl
+ || TREE_OPERAND (rhs, 1) == decl)
+ INTEGRAL_TYPE_P (TREE_TYPE (rhs)))
+ return incr;
+ else if (TREE_CODE (rhs) == MINUS_EXPR
+ TREE_OPERAND (rhs, 0) == decl
+ INTEGRAL_TYPE_P (TREE_TYPE (rhs)))
+ return incr;
+ // Otherwise fail because only PLUS_EXPR and MINUS_EXPR are
+ // allowed.
+ break;
... but not here (p += 1)?
+c_validate_cilk_plus_loop (tree *tp, int *walk_subtrees, void *data)
+{
+ if (!tp || !*tp)
+return NULL_TREE;
+
+ bool *valid = (bool *) data;
+
+ switch (TREE_CODE (*tp))
+{
+case CALL_EXPR:
+ {
+ tree fndecl = CALL_EXPR_FN (*tp);
+
+ if (TREE_CODE (fndecl) == ADDR_EXPR)
+ fndecl = TREE_OPERAND (fndecl, 0);
+ if (TREE_CODE (fndecl) == FUNCTION_DECL)
+ {
+ if (setjmp_call_p (fndecl))
+ {
+ error_at (EXPR_LOCATION (*tp),
+ calls to setjmp are not allowed within loops
+ annotated with #pragma simd);
+ *valid = false;
+ *walk_subtrees = 0;
+ }
+ }
+ break;
Why bother checking for setjmp? While I agree it makes little sense, there are
plenty of other standard functions which also make no sense to use from within
#pragma simd. What's likely to go wrong with a call to setjmp, as opposed to
getcontext, pthread_create, or even printf?
+ if (DECL_REGISTER (decl))
+{
+ error_at (loc, induction variable cannot be declared register);
+ return false;
+}
Why?
All of the actual gimple changes look good. You could commit those now if you
like to reduce the size of the next patch.
r~
[wide-int] Add missing ()
[wide-int] Add missing () diff --git a/gcc/genmodes.c b/gcc/genmodes.c index a27993a..946dcc9 100644 --- a/gcc/genmodes.c +++ b/gcc/genmodes.c @@ -874,14 +874,14 @@ emit_max_int (void) max = i-bytesize; if (max mmax) mmax = max; - printf (#define MAX_BITSIZE_MODE_ANY_INT %d*BITS_PER_UNIT\n, mmax); + printf (#define MAX_BITSIZE_MODE_ANY_INT (%d*BITS_PER_UNIT)\n, mmax); mmax = 0; for (j = 0; j MAX_MODE_CLASS; j++) for (i = modes[j]; i; i = i-next) if (mmax i-bytesize) mmax = i-bytesize; - printf (#define MAX_BITSIZE_MODE_ANY_MODE %d*BITS_PER_UNIT\n, mmax); + printf (#define MAX_BITSIZE_MODE_ANY_MODE (%d*BITS_PER_UNIT)\n, mmax); } static void
Re: [RFA] Type inheritance graph analysis speculative devirtualization, part 4/7, ODR at LTO time
Hi,
On Mon, Aug 19, 2013 at 04:01:16PM +0200, Jan Hubicka wrote:
[...]
* Makefile.in (ipa-devirt.o): Add dependency on diagnostic.h
* ipa-devirt.c: Include diganostic.h
(odr_type_d): Add types and types_set.
(hash_type_name): Work for types with vtables during LTO.
(odr_hasher::remove): Fix comment; destroy types_set.
(add_type_duplicate): New function,
(get_odr_type): Use it.
(dump_type_inheritance_graph): Dump type duplicates.
* ipa.c (symtab_remove_unreachable_nodes): Build type inheritance
graph.
* tree.c (types_same_for_odr): Give exact answers on types with
virtual tables.
[...]
Index: ipa-devirt.c
===
*** ipa-devirt.c (revision 201836)
--- ipa-devirt.c (working copy)
*** static odr_hash_type odr_hash;
*** 222,227
--- 251,382
static GTY(()) vec odr_type, va_gc *odr_types_ptr;
#define odr_types (*odr_types_ptr)
+ /* TYPE is equivalent to VAL by ODR, but its tree representation differs
+from VAL-type. This may happen in LTO where tree merging did not merge
+all variants of the same type. It may or may not mean the ODR violation.
+Add it to the list of duplicates and warn on some violations. */
+
+ void
+ add_type_duplicate (odr_type val, tree type)
+ {
It seems the function can be made static. If not, it should probably
have a name that would be less prone to clashes.
Thanks,
Martin
Re: [RFA] Type inheritance graph analysis speculative devirtualization, part 4/7, ODR at LTO time
Hi,
On Mon, Aug 19, 2013 at 04:01:16PM +0200, Jan Hubicka wrote:
[...]
* Makefile.in (ipa-devirt.o): Add dependency on diagnostic.h
* ipa-devirt.c: Include diganostic.h
(odr_type_d): Add types and types_set.
(hash_type_name): Work for types with vtables during LTO.
(odr_hasher::remove): Fix comment; destroy types_set.
(add_type_duplicate): New function,
(get_odr_type): Use it.
(dump_type_inheritance_graph): Dump type duplicates.
* ipa.c (symtab_remove_unreachable_nodes): Build type inheritance
graph.
* tree.c (types_same_for_odr): Give exact answers on types with
virtual tables.
[...]
Index: ipa-devirt.c
===
*** ipa-devirt.c(revision 201836)
--- ipa-devirt.c(working copy)
*** static odr_hash_type odr_hash;
*** 222,227
--- 251,382
static GTY(()) vec odr_type, va_gc *odr_types_ptr;
#define odr_types (*odr_types_ptr)
+ /* TYPE is equivalent to VAL by ODR, but its tree representation differs
+from VAL-type. This may happen in LTO where tree merging did not
merge
+all variants of the same type. It may or may not mean the ODR
violation.
+Add it to the list of duplicates and warn on some violations. */
+
+ void
+ add_type_duplicate (odr_type val, tree type)
+ {
It seems the function can be made static. If not, it should probably
have a name that would be less prone to clashes.
Yes, it can be static. It is anoying we no longer have those missing static
keyword warnings like before switch to c++. I updated my local tree.
Thanks,
Honza
Thanks,
Martin
Re: v2 of GDB hooks for debugging GCC
On Wed, 2013-08-21 at 15:01 -0600, Tom Tromey wrote:
David == David Malcolm dmalc...@redhat.com writes:
[...]
David How would one go about toggling the enabledness of a prettyprinter? Is
David this something that can only be done from python?
You can use enable pretty-printer and disable pretty-printer.
Yes, using .* to match the current executable:
(gdb) disable pretty-printer .* gcc
7 printers disabled
128 of 135 printers enabled
and this does indeed disable/enable the prettyprinters.
David I did see references to gdb.parameter(verbose) in gdb.printing
David - how would one go about setting this?
set verbose on
I think few people use this setting though; probably best to do what
you're doing now.
David +# Convert enum tree_code (tree.def and tree.h) to a dict:
David +tree_code_dict = gdb.types.make_enum_dict(gdb.lookup_type('enum
tree_code'))
One other subtlety is that this doesn't interact well with all kinds of
uses of gdb. For example if you have a running gdb, then modify enum
tree_code and rebuild, then the pretty-printers won't notice this
change.
I guess it would be nice if we had pre-built caches for this kind of
this available upstream. But meanwhile, if you care, you can roll your
own using events to notice when to invalidate data.
As they say, the two fundamental problems in Computer Science are cache
invalidation, naming things, and off-by-one errors.
I'm inclined not to care for now: if you've rebuilt gcc with a new enum
tree code, then you should restart gdb.
Is there a precanned event provided by gdb that I can connect to for
when the underlying code has changed and my caches need to be
invalidated?
David +def __call__(self, gdbval):
David +type_ = gdbval.type.unqualified()
David +str_type_ = str(type_)
FWIW I think for RegexpCollectionPrettyPrinter you could write a
subclass whose __call__ first dereferenced a pointer, then called
super's __call__. But your approach is just fine.
Thanks
v3 of GDB hooks for debugging GCC
On Wed, 2013-08-21 at 15:01 -0600, Tom Tromey wrote:
David == David Malcolm dmalc...@redhat.com writes:
Tom Naughty.
David We chatted about this at Cauldron; I haven't yet had a chance to
David implement the magic bullet approach we discussed there. In the
David meantime, is there a API I can call to determine how safe this kludge
David is?
Not right now. You can just call the function and catch the exception
that occurs if it can't be done.
I think you can still run into trouble sometimes. For example if the
user puts a breakpoint in one of the functions used by the
pretty-printer, and then does bt, hitting the breakpoint while
printing the backtrace... not sure what happens then, maybe a crash.
Tom I think you could set up the safe-path in the gcc .gdbinit.
David Interesting idea - but .gdbinit itself seems to get declined, so I
don't
David think this can help.
Haha, I didn't think of that :-)
But you were on the right track... if one marks gcc's .gdbinit as
loadable, then gdb can happily *import* (rather than autoload) the
python hooks without needing the user to grant extra permission.
I'm attaching a revised patch that reworks things to use this scheme, by
adding a python import line into the configure[.ac] hook that generates
builddir/gcc/.gdbinit : all that a gcc hacker has to do to use the
python hooks now is to ensure that their ~/.gdbinit script contains a
line like this:
add-auto-load-safe-path /absolute/path/to/build/gcc
and it all should just work. You need this already to get the
pre-existing gdbinit hooks to work with a recent gdb that has the
autoload protection. [I renamed the file from gdb-hooks.py to
gdbhooks.py so that it's importable as a python module. Doing it from
the srcdir avoids having to copy the file to the builddir].
So in this scheme, the Python hooks piggyback on top of the older gdb
hooks. Hope that's OK. As noted earlier in the thread, the Python
hooks can be disabled by running:
(gdb) disable pretty-printer .* gcc
7 printers disabled
The patch also adds me a maintainer of gdbhooks.py into the MAINTAINERS
file. (There doesn't seem to be any sort order to the maintainer part
of that file, should there be?)
Finally, I added a copyright header to the new file (part of GCC, FSF
assignee, GPLv3 or later).
OK for trunk?
Dave
commit 9ef4a9c7474b56f19bfa49905944931e52e95514
Author: David Malcolm dmalc...@redhat.com
Date: Wed Aug 21 15:45:55 2013 -0400
initial version of gdb hooks
* MAINTAINERS (gdbhooks.py): Add myself as maintainer
gcc/
* gdbhooks.py: New.
* configure.ac (gdbinit.in): Add import of gcc/gdbhooks.py.
* configure: Regenerate.
diff --git a/MAINTAINERS b/MAINTAINERS
index 78b288f..50ede75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -256,6 +256,7 @@ testsuite Rainer Orth r...@cebitec.uni-bielefeld.de
testsuite Mike Stump mikest...@comcast.net
testsuite Janis Johnson jani...@codesourcery.com
register allocation Vladimir Makarov vmaka...@redhat.com
+gdbhooks.py David Malcolm dmalc...@redhat.com
Note that individuals who maintain parts of the compiler need approval to
check in changes outside of the parts of the compiler they maintain.
diff --git a/gcc/configure b/gcc/configure
index ec662f5..c6bc3a6 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -27397,6 +27397,7 @@ if test x$subdirs != x; then
done
fi
echo source ${srcdir}/gdbinit.in .gdbinit
+echo python import sys; sys.path.append('${srcdir}'); import gdbhooks .gdbinit
gcc_tooldir='$(libsubdir)/$(libsubdir_to_prefix)$(target_noncanonical)'
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 62d3053..5d3e5ad 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5181,6 +5181,7 @@ if test x$subdirs != x; then
done
fi
echo source ${srcdir}/gdbinit.in .gdbinit
+echo python import sys; sys.path.append('${srcdir}'); import gdbhooks .gdbinit
gcc_tooldir='$(libsubdir)/$(libsubdir_to_prefix)$(target_noncanonical)'
AC_SUBST(gcc_tooldir)
diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py
new file mode 100644
index 000..3d69b11
--- /dev/null
+++ b/gcc/gdbhooks.py
@@ -0,0 +1,397 @@
+# Python hooks for gdb for debugging GCC
+# Copyright (C) 2013 Free Software Foundation, Inc.
+
+# Contributed by David Malcolm dmalc...@redhat.com
+
+# This file is part of GCC.
+
+# GCC is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation; either version 3, or (at your option) any later
+# version.
+
+# GCC is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+# for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3. If not see
+# http://www.gnu.org/licenses/.
+
+
+Enabling
Re: GDB hooks for debugging GCC
On Mon, 2013-08-19 at 13:56 -0600, Jeff Law wrote: On 08/02/2013 07:48 PM, David Malcolm wrote: GDB 7.0 onwards supports hooks written in Python to improve the quality-of-life within the debugger. The best known are the pretty-printing hooks [1], which we already use within libstdc++ for printing better representations of STL containers. So as I mentioned during the Cauldron, I really like this and feel it could simplify certain aspects of debugging. What's even better is we can easily twiddle this stuff to further improve the experience as we use it more. Of course I'm particularly interested in blocks edges, so that's where you'll probably see further feedback from me. [ ... ] Note that this doesn't eliminate the .gdbinit script that some people use, but it's arguably far superior, in that it happens automatically, and for all values that are printed e.g. in backtraces, viewing struct fields, etc. Right. I'd like to get this into trunk. Some remaining issues: * installation; currently one has to manually copy it into place, as noted above; it would be nice to automate things so that during a build it gets copied to cc1-gdb.py, cc1plus-gdb.py etc Perhaps add -iex add-auto-load-safe-path gcc source path in .gdbinit? I can't imagine typing that every time I start up... I came up with a simpler approach to this that doesn't require copying things into place, and reuses the permissions already granted to $builddir/gcc/.gdbinit; see: http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01532.html This other approach does link enablement of the Python hooks to that of the older hooks. I thought this is worth mentioning, though I don't see it as a problem. * it hardcoded values in a few places rather than correctly looking up enums * the kludge in the RTX printer mentioned above. Given this stuff is in python, I'm comfortable letting you own it and decide if/when/how to fix these warts. Thanks; v3 of the patch (linked to above) would add me to the MAINTAINERS file for this aspect.
Re: RFA: Consider int and same-size short as equivalent vector components
On Mon, 26 Aug 2013, Joern Rennecke wrote: avr currently shows the following failure: FAIL: c-c++-common/vector-scalar.c -Wc++-compat (test for excess errors) Excess errors: /home/amylaar/atmel/4.8/unisrc-mainline/gcc/testsuite/c-c++-common/vector-scalar .c:9:34: error: invalid operands to binary | (have '__vector(8) int' and 'veci') The issue is that when we compute the result of an operatiopn of a veci and an int, we get a __vector(8) int result (int is the same size as short), Maybe we could change that? yet the typechecking later does not accept the vectors as compatible. Fixed by relaxing the latter for the case that int and short are the same size. If you do this, maybe rename the function, or at least expand the comment, to make it clear that it should only be used for comparison operators with vectors? The issue seems larger than just short/int. On x86, (ll)l fails to compile for a vector of long, with ll that has opaque type vector of int, that seems wrong. -- Marc Glisse
Re: RFA: Consider int and same-size short as equivalent vector components
Quoting Marc Glisse marc.gli...@inria.fr: The issue seems larger than just short/int. On x86, (ll)l fails to compile for a vector of long, with ll that has opaque type vector of int, that seems wrong. I don't understand what you mean here. Could you post the actual code sample?
Re: RFA: Consider int and same-size short as equivalent vector components
On Mon, 26 Aug 2013, Joern Rennecke wrote:
Quoting Marc Glisse marc.gli...@inria.fr:
The issue seems larger than just short/int. On x86, (ll)l fails to
compile for a vector of long, with ll that has opaque type vector of
int, that seems wrong.
I don't understand what you mean here. Could you post the actual code
sample?
typedef long vl __attribute__((vector_size(16)));
void f(vl l){
(ll)l;
}
it compiles fine on x86_64 but not on x86.
(btw, my sentence was ambiguous, what I find wrong is the failure to
compile, not the type of ll)
--
Marc Glisse
Make some comdats implicitly hidden
Hi,
for -flto (and for -fhwhole-program as well) I for a while privatize comdat
symbols based on fact that I know their address can not be compared for
equivality (virtuals/ctors/dtors and functions with no address taken).
While analyzing the relocations of libreoffice I noticed that I can play
the same game w/o LTO at linker level. Making those symbols hidden truns
external relocations to internal and should improve runtime, too: comdat
sharing by dynamic linker is expensive and won't save duplicated functions
from the binary.
There is ext/visibility/template2.C that fails with the patch. It tests that
visibility pragma does not bring the symbol local, but now we do so based
on logic above.
Jason, do you have any idea how to fix the testcase? I tried to use different
visility but that doesn't work, since we do not have corresponding scan-not-*
Bootstrapped/regtested x86_64-linux
Honza
* ipa.c (function_and_variable_visibility): Make comdats that can be
unshared hidden.
Index: ipa.c
===
--- ipa.c (revision 202001)
+++ ipa.c (working copy)
@@ -873,6 +873,17 @@ function_and_variable_visibility (bool w
segfault though. */
symtab_dissolve_same_comdat_group_list ((symtab_node) node);
}
+ if (node-symbol.externally_visible
+ DECL_COMDAT (node-symbol.decl)
+ comdat_can_be_unshared_p ((symtab_node) node))
+ {
+ if (dump_file
+ DECL_VISIBILITY (node-symbol.decl) != VISIBILITY_HIDDEN)
+ fprintf (dump_file, Promoting visibility to hidden: %s/%i\n,
+cgraph_node_name (node), node-symbol.order);
+ DECL_VISIBILITY (node-symbol.decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED (node-symbol.decl) = true;
+ }
if (node-thunk.thunk_p
TREE_PUBLIC (node-symbol.decl))
@@ -980,6 +991,17 @@ function_and_variable_visibility (bool w
symtab_dissolve_same_comdat_group_list ((symtab_node) vnode);
vnode-symbol.resolution = LDPR_PREVAILING_DEF_IRONLY;
}
+ if (vnode-symbol.externally_visible
+ DECL_COMDAT (vnode-symbol.decl)
+ comdat_can_be_unshared_p ((symtab_node) vnode))
+ {
+ if (dump_file
+ DECL_VISIBILITY (vnode-symbol.decl) == VISIBILITY_HIDDEN)
+ fprintf (dump_file, Promoting visibility to hidden: %s/%i\n,
+varpool_node_name (vnode), vnode-symbol.order);
+ DECL_VISIBILITY (vnode-symbol.decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED (vnode-symbol.decl) = true;
+ }
}
if (dump_file)
[C++ Patch] Remove old bison hack?!
Hi,
a few days ago I noticed this comment and code. Removing it passes
testing on x86_64-linux (I would in any case also boot and test multilib
before committing).
Admittedly, a bit risky, but Stage 1 seems the right time to try.
Thanks!
Paolo.
2013-08-26 Paolo Carlini paolo.carl...@oracle.com
* decl.c (grokfndecl): Remove old bison hack.
Index: decl.c
===
--- decl.c (revision 202008)
+++ decl.c (working copy)
@@ -7427,17 +7449,6 @@ grokfndecl (tree ctype,
the information in the TEMPLATE_ID_EXPR. */
SET_DECL_IMPLICIT_INSTANTIATION (decl);
- if (TREE_CODE (fns) == COMPONENT_REF)
- {
- /* Due to bison parser ickiness, we will have already looked
-up an operator_name or PFUNCNAME within the current class
-(see template_id in parse.y). If the current class contains
-such a name, we'll get a COMPONENT_REF here. Undo that. */
-
- gcc_assert (TREE_TYPE (TREE_OPERAND (fns, 0))
- == current_class_type);
- fns = TREE_OPERAND (fns, 1);
- }
gcc_assert (identifier_p (fns) || TREE_CODE (fns) == OVERLOAD);
DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args);
