libtool update for powerpc64le-linux
I'd like to apply the following patch to the gcc repository (well,
excluding the libgo part which I'm hoping someone will apply for me to
the master go repository). I know the normal procedure for autotools
is to submit upstream then update when the patch is in the upstream
autotools repository, but this simple libtool patch has been awaiting
review for over two months.
This adds support for building gcc on little-endian powerpc linux
hosts, and tidies the existing host match for powerpc. config.sub
won't return ppc*-*linux* so there isn't much point in matching that.
* libtool.m4 (ld -m flags): Remove non-canonical ppc host match.
Support little-endian powerpc linux hosts.
libgo/
* config/libtool.m4 (ld -m flags): Remove non-canonical ppc host
match. Support little-endian powerpc linux hosts.
* libgo/configure: Regenerate.
libjava/libltdl/
* acinclude.m4 (ld -m flags): Remove non-canonical ppc host
match. Support little-endian powerpc linux hosts.
* configure: Regenerate.
gcc/
* configure: Regenerate.
libobjc/
* configure: Regenerate.
libgfortran/
* configure: Regenerate.
libffi/
* configure: Regenerate.
libssp/
* configure: Regenerate.
libitm/
* configure: Regenerate.
libgomp/
* configure: Regenerate.
libquadmath/
* configure: Regenerate.
libsanitizer/
* configure: Regenerate.
zlib/
* configure: Regenerate.
libstdc++-v3/
* configure: Regenerate.
libmudflap/
* configure: Regenerate.
boehm-gc/
* configure: Regenerate.
lto-plugin/
* configure: Regenerate.
libatomic/
* configure: Regenerate.
libbacktrace/
* configure: Regenerate.
libjava/
* configure: Regenerate.
libjava/classpath/
* configure: Regenerate.
Index: libtool.m4
===
--- libtool.m4 (revision 200501)
+++ libtool.m4 (working copy)
@@ -1220,7 +1220,7 @@
rm -rf conftest*
;;
-x86_64-*kfreebsd*-gnu|x86_64-*linux*|ppc*-*linux*|powerpc*-*linux*| \
+x86_64-*kfreebsd*-gnu|x86_64-*linux*|powerpc*-*linux*| \
s390*-*linux*|s390*-*tpf*|sparc*-*linux*)
# Find out which ABI we are using.
echo 'int i;' > conftest.$ac_ext
@@ -1241,7 +1241,10 @@
;;
esac
;;
- ppc64-*linux*|powerpc64-*linux*)
+ powerpc64le-*)
+ LD="${LD-ld} -m elf32lppclinux"
+ ;;
+ powerpc64-*)
LD="${LD-ld} -m elf32ppclinux"
;;
s390x-*linux*)
@@ -1260,7 +1263,10 @@
x86_64-*linux*)
LD="${LD-ld} -m elf_x86_64"
;;
- ppc*-*linux*|powerpc*-*linux*)
+ powerpcle-*)
+ LD="${LD-ld} -m elf64lppc"
+ ;;
+ powerpc-*)
LD="${LD-ld} -m elf64ppc"
;;
s390*-*linux*|s390*-*tpf*)
Index: libgo/config/libtool.m4
===
--- libgo/config/libtool.m4 (revision 200501)
+++ libgo/config/libtool.m4 (working copy)
@@ -1225,7 +1225,7 @@
rm -rf conftest*
;;
-x86_64-*kfreebsd*-gnu|x86_64-*linux*|ppc*-*linux*|powerpc*-*linux*| \
+x86_64-*kfreebsd*-gnu|x86_64-*linux*|powerpc*-*linux*| \
s390*-*linux*|s390*-*tpf*|sparc*-*linux*)
# Find out which ABI we are using.
echo 'int i;' > conftest.$ac_ext
@@ -1239,7 +1239,10 @@
x86_64-*linux*)
LD="${LD-ld} -m elf_i386"
;;
- ppc64-*linux*|powerpc64-*linux*)
+ powerpc64le-*)
+ LD="${LD-ld} -m elf32lppclinux"
+ ;;
+ powerpc64-*)
LD="${LD-ld} -m elf32ppclinux"
;;
s390x-*linux*)
@@ -1258,7 +1261,10 @@
x86_64-*linux*)
LD="${LD-ld} -m elf_x86_64"
;;
- ppc*-*linux*|powerpc*-*linux*)
+ powerpcle-*)
+ LD="${LD-ld} -m elf64lppc"
+ ;;
+ powerpc-*)
LD="${LD-ld} -m elf64ppc"
;;
s390*-*linux*|s390*-*tpf*)
Index: libjava/libltdl/acinclude.m4
===
--- libjava/libltdl/acinclude.m4(revision 200501)
+++ libjava/libltdl/acinclude.m4(working copy)
@@ -519,7 +519,7 @@
rm -rf conftest*
;;
-x86_64-*linux*|ppc*-*linux*|powerpc*-*linux*|s390*-*linux*|sparc*-*linux*)
+x86_64-*linux*|powerpc*-*linux*|s390*-*linux*|sparc*-*linux*)
# Find out which ABI we are using.
echo 'int i;' > conftest.$ac_ext
if AC_TRY_EVAL(ac_compile); then
@@ -529,7 +529,10 @@
x86_64-*linux*)
LD="${LD-ld} -m elf_i386"
;;
-ppc64-*linux*|powerpc64-*linux*)
+powerpc64le-*)
+ LD="${LD-ld} -m elf32lppclinux"
+ ;;
+powerpc64-*)
LD="${LD-ld} -m elf32ppclinux"
;;
s390x-*linux*)
@@ -545,7 +548,10 @@
x86_64-*linux*)
LD="${LD-ld}
Re: Two testcases failing for check-debug
On 08/16/2013 02:40 AM, Tim Shen wrote: Could you please commit this patch for me? Done. Paolo.
Re: [patch] [python libstdc++ printers] Fix gdb/15195
On 23/07/13 15:23, Tom Tromey wrote:
>> "Phil" == Phil Muldoon writes:
>
> Phil> On 03/07/13 08:33, Phil Muldoon wrote:
>>> This new patch replaces and obsoletes the previous. On further
>>> inspection of some other pretty printer related bugs, it seems that
>>> all of the printers need to fetch the referenced value where the value
>>> type is a reference. This patch applies that change, and adds a
>>> number of reference based tests.
>
> Phil> ping
>
> I sent a reply last Thursday:
>
> http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00713.html
Weirdly I did not get this email in my inbox (but I see it right there
in the archives).
Anyway, I have regenerated the patch with the fixes requested.
Cheers,
Phil
2013-08-16 Phil Muldoon
PR gcc/53477
http://sourceware.org/bugzilla/show_bug.cgi?id=15195
* python/libstdcxx/v6/printers.py (Printer.__call__): If a value
is a reference, fetch referenced value.
(RxPrinter.invoke): Ditto.
* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
flag. Add referenced value tests.
--
Index: libstdc++-v3/python/libstdcxx/v6/printers.py
===
--- libstdc++-v3/python/libstdcxx/v6/printers.py(revision 199642)
+++ libstdc++-v3/python/libstdcxx/v6/printers.py(working copy)
@@ -786,6 +786,11 @@
def invoke(self, value):
if not self.enabled:
return None
+
+if value.type.code == gdb.TYPE_CODE_REF:
+if hasattr(gdb.Value,"referenced_value"):
+value = value.referenced_value()
+
return self.function(self.name, value)
# A pretty-printer that conforms to the "PrettyPrinter" protocol from
@@ -841,6 +846,11 @@
return None
basename = match.group(1)
+
+if val.type.code == gdb.TYPE_CODE_REF:
+if hasattr(gdb.Value,"referenced_value"):
+val = val.referenced_value()
+
if basename in self.lookup:
return self.lookup[basename].invoke(val)
Index: libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
===
--- libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc(revision
199706)
+++ libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc(working copy)
@@ -1,5 +1,5 @@
// { dg-do run }
-// { dg-options "-std=gnu++11 -g" }
+// { dg-options "-std=gnu++11 -g -O0" }
// Copyright (C) 2011-2013 Free Software Foundation, Inc.
//
@@ -25,6 +25,8 @@
#include
#include
+typedef std::tuple ExTuple;
+
template
void
placeholder(const T &s)
@@ -63,43 +65,75 @@
std::forward_list efl;
// { dg-final { note-test efl "empty std::forward_list" } }
+ std::forward_list &refl = efl;
+// { dg-final { note-test refl "empty std::forward_list" } }
+
std::forward_list fl;
fl.push_front(2);
fl.push_front(1);
// { dg-final { note-test fl {std::forward_list = {[0] = 1, [1] = 2}} } }
+ std::forward_list &rfl = fl;
+// { dg-final { note-test rfl {std::forward_list = {[0] = 1, [1] = 2}} } }
+
std::unordered_map eum;
// { dg-final { note-test eum "std::unordered_map with 0 elements" } }
+ std::unordered_map &reum = eum;
+// { dg-final { note-test reum "std::unordered_map with 0 elements" } }
+
std::unordered_multimap eumm;
// { dg-final { note-test eumm "std::unordered_multimap with 0 elements" } }
+ std::unordered_multimap &reumm = eumm;
+// { dg-final { note-test reumm "std::unordered_multimap with 0 elements" } }
+
std::unordered_set eus;
// { dg-final { note-test eus "std::unordered_set with 0 elements" } }
+ std::unordered_set &reus = eus;
+// { dg-final { note-test reus "std::unordered_set with 0 elements" } }
+
std::unordered_multiset eums;
// { dg-final { note-test eums "std::unordered_multiset with 0 elements" } }
+ std::unordered_multiset &reums = eums;
+// { dg-final { note-test reums "std::unordered_multiset with 0 elements" } }
std::unordered_map uom;
uom[5] = "three";
uom[3] = "seven";
// { dg-final { note-test uom {std::unordered_map with 2 elements = {[3] =
"seven", [5] = "three"}} } }
+ std::unordered_map &ruom = uom;
+// { dg-final { note-test ruom {std::unordered_map with 2 elements = {[3] =
"seven", [5] = "three"}} } }
+
std::unordered_multimap uomm;
uomm.insert(std::pair (5, "three"));
uomm.insert(std::pair (5, "seven"));
// { dg-final { note-test uomm {std::unordered_multimap with 2 elements = {[5]
= "seven", [5] = "three"}} } }
+ std::unordered_multimap &ruomm = uomm;
+// { dg-final { note-test ruomm {std::unordered_multimap with 2 elements =
{[5] = "seven", [5] = "three"}} } }
std::unordered_set uos;
uos.insert(5);
// { dg-final { note-test uos {std::unordered_set with 1 elements = {[0] = 5}}
} }
+ std::unordered_set &ruos = uos;
+// { dg-final { note-test ruos {std::unordered_set with 1 elements = {[0] =
5}} } }
std::unordered_
Clean up pretty printers [7/n]
Same topic as previous patch in this series.
-- Gaby
2013-08-16 Gabriel Dos Reis
* sched-vis.c (rtl_slim_pp_initialized): Remove.
(rtl_slim_pp): Likewise.
(init_rtl_slim_pretty_print): Likewise.
(dump_value_slim): Don't call it. Use local pretty printer.
(dump_insn_slim): Likewise.
(dump_rtl_slim): Likewise.
(str_pattern_slim): Likewise.
* tree-mudflap.c (mf_varname_tree): Use local pretty printer.
Simplify.
Index: sched-vis.c
===
--- sched-vis.c (revision 201784)
+++ sched-vis.c (working copy)
@@ -47,10 +47,6 @@
It is also possible to obtain a string for a single pattern as a string
pointer, via str_pattern_slim, but this usage is discouraged. */
-/* A pretty-printer for slim rtl printing. */
-static bool rtl_slim_pp_initialized = false;
-static pretty_printer rtl_slim_pp;
-
/* For insns we print patterns, and for some patterns we print insns... */
static void print_insn_with_notes (pretty_printer *, const_rtx);
@@ -760,24 +756,6 @@
}
}
-/* Return a pretty-print buffer set up to print to file F. */
-
-static pretty_printer *
-init_rtl_slim_pretty_print (FILE *f)
-{
- if (! rtl_slim_pp_initialized)
-{
- pp_construct (&rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
- rtl_slim_pp_initialized = true;
-}
- else
-/* Clean out any data that str_insn_slim may have left here. */
-pp_clear_output_area (&rtl_slim_pp);
-
- rtl_slim_pp.buffer->stream = f;
- return &rtl_slim_pp;
-}
-
/* Print X, an RTL value node, to file F in slim format. Include
additional information if VERBOSE is nonzero.
@@ -787,9 +765,11 @@
void
dump_value_slim (FILE *f, const_rtx x, int verbose)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
- print_value (pp, x, verbose);
- pp_flush (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (&rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer->stream = f;
+ print_value (&rtl_slim_pp, x, verbose);
+ pp_flush (&rtl_slim_pp);
}
/* Emit a slim dump of X (an insn) to the file F, including any register
@@ -797,9 +777,11 @@
void
dump_insn_slim (FILE *f, const_rtx x)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
- print_insn_with_notes (pp, x);
- pp_flush (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (&rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer->stream = f;
+ print_insn_with_notes (&rtl_slim_pp, x);
+ pp_flush (&rtl_slim_pp);
}
/* Same as above, but stop at LAST or when COUNT == 0.
@@ -810,19 +792,21 @@
int count, int flags ATTRIBUTE_UNUSED)
{
const_rtx insn, tail;
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
+ pretty_printer rtl_slim_pp;
+ pp_construct (&rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer->stream = f;
tail = last ? NEXT_INSN (last) : NULL_RTX;
for (insn = first;
(insn != NULL) && (insn != tail) && (count != 0);
insn = NEXT_INSN (insn))
{
- print_insn_with_notes (pp, insn);
+ print_insn_with_notes (&rtl_slim_pp, insn);
if (count > 0)
count--;
}
- pp_flush (pp);
+ pp_flush (&rtl_slim_pp);
}
/* Dumps basic block BB to pretty-printer PP in slim form and without and
@@ -857,9 +841,10 @@
const char *
str_pattern_slim (const_rtx x)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (NULL);
- print_pattern (pp, x, 0);
- return pp_formatted_text (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (&rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ print_pattern (&rtl_slim_pp, x, 0);
+ return ggc_strdup (pp_formatted_text (&rtl_slim_pp));
}
/* Emit a slim dump of X (an insn) to stderr. */
Index: tree-mudflap.c
===
--- tree-mudflap.c (revision 201784)
+++ tree-mudflap.c (working copy)
@@ -106,20 +106,14 @@
static tree
mf_varname_tree (tree decl)
{
- static pretty_printer buf_rec;
- static int initialized = 0;
- pretty_printer *buf = & buf_rec;
const char *buf_contents;
tree result;
gcc_assert (decl);
- if (!initialized)
-{
- pp_construct (buf, /* prefix */ NULL, /* line-width */ 0);
- initialized = 1;
-}
- pp_clear_output_area (buf);
+ pretty_printer buf;
+ pp_construct (&buf, /* prefix */ NULL, /* line-width */ 0);
+ pp_clear_output_area (&buf);
/* Add FILENAME[:LINENUMBER[:COLUMNNUMBER]]. */
{
@@ -134,17 +128,17 @@
if (sourcefile == NULL)
sourcefile = "";
-pp_string (buf, sourcefile);
+pp_string (&buf, sourcefile);
if (sourceline != 0)
{
-pp_colon (buf);
-pp_decimal_int (buf, sourceline);
+pp_colon (&buf);
+pp_decimal_int (&buf, sourceline);
if (sourcecolumn != 0)
{
-pp_colon (buf);
-pp_decimal_int (buf, sourcecolumn);
+
Re: Fix class type lookup from OBJ_TYPE_REF
Thanks,
this is version I commited after testing at ppc64-linux. The difference is
that it also updates code suffering from the same problem in
gimple_extract_devirt_binfo_from_cst. I originally intended to rewrite the
function but after yesterday discussion with Martin I think it will take me
little more time. Moreover this really can trigger bad code so I think
having self contained patch is better.
Honza
* gimple-fold.c (gimple_extract_devirt_binfo_from_cst): Add new
arugment expected_type.
(gimple_fold_call): Use it.
* gimple.h (gimple_extract_devirt_binfo_from_cst): Update prototype.
* ipa-cp.c (ipa_get_indirect_edge_target_1): Update.
* ipa-prop.c (ipa_analyze_virtual_call_uses): Use
obj_type_ref_class.
(try_make_edge_direct_virtual_call): Likewise.
* tree.c (obj_type_ref_class): New.
* tree.h (obj_type_ref_class): Use it.
.
Index: gimple-fold.c
===
--- gimple-fold.c (revision 201776)
+++ gimple-fold.c (working copy)
@@ -1007,13 +1007,14 @@ gimple_fold_builtin (gimple stmt)
represented by a declaration (i.e. a global or automatically allocated one)
or NULL if it cannot be found or is not safe. CST is expected to be an
ADDR_EXPR of such object or the function will return NULL. Currently it is
- safe to use such binfo only if it has no base binfo (i.e. no ancestors). */
+ safe to use such binfo only if it has no base binfo (i.e. no ancestors)
+ EXPECTED_TYPE is type of the class virtual belongs to. */
tree
-gimple_extract_devirt_binfo_from_cst (tree cst)
+gimple_extract_devirt_binfo_from_cst (tree cst, tree expected_type)
{
HOST_WIDE_INT offset, size, max_size;
- tree base, type, expected_type, binfo;
+ tree base, type, binfo;
bool last_artificial = false;
if (!flag_devirtualize
@@ -1022,7 +1023,6 @@ gimple_extract_devirt_binfo_from_cst (tr
return NULL_TREE;
cst = TREE_OPERAND (cst, 0);
- expected_type = TREE_TYPE (cst);
base = get_ref_base_and_extent (cst, &offset, &size, &max_size);
type = TREE_TYPE (base);
if (!DECL_P (base)
@@ -1108,7 +1108,8 @@ gimple_fold_call (gimple_stmt_iterator *
else
{
tree obj = OBJ_TYPE_REF_OBJECT (callee);
- tree binfo = gimple_extract_devirt_binfo_from_cst (obj);
+ tree binfo = gimple_extract_devirt_binfo_from_cst
+(obj, obj_type_ref_class (callee));
if (binfo)
{
HOST_WIDE_INT token
Index: gimple.h
===
--- gimple.h(revision 201776)
+++ gimple.h(working copy)
@@ -854,7 +854,7 @@ unsigned get_gimple_rhs_num_ops (enum tr
gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
const char *gimple_decl_printable_name (tree, int);
tree gimple_get_virt_method_for_binfo (HOST_WIDE_INT, tree);
-tree gimple_extract_devirt_binfo_from_cst (tree);
+tree gimple_extract_devirt_binfo_from_cst (tree, tree);
/* Returns true iff T is a scalar register variable. */
extern bool is_gimple_reg (tree);
Index: ipa-cp.c
===
--- ipa-cp.c(revision 201776)
+++ ipa-cp.c(working copy)
@@ -1541,7 +1541,8 @@ ipa_get_indirect_edge_target_1 (struct c
if (TREE_CODE (t) != TREE_BINFO)
{
tree binfo;
- binfo = gimple_extract_devirt_binfo_from_cst (t);
+ binfo = gimple_extract_devirt_binfo_from_cst
+(t, ie->indirect_info->otr_type);
if (!binfo)
return NULL_TREE;
binfo = get_binfo_at_offset (binfo, anc_offset, otr_type);
Index: ipa-prop.c
===
--- ipa-prop.c (revision 201776)
+++ ipa-prop.c (working copy)
@@ -1903,7 +1903,7 @@ ipa_analyze_virtual_call_uses (struct cg
ii = cs->indirect_info;
ii->offset = anc_offset;
ii->otr_token = tree_low_cst (OBJ_TYPE_REF_TOKEN (target), 1);
- ii->otr_type = TREE_TYPE (TREE_TYPE (OBJ_TYPE_REF_OBJECT (target)));
+ ii->otr_type = obj_type_ref_class (target);
ii->polymorphic = 1;
}
@@ -2453,7 +2453,8 @@ try_make_edge_direct_virtual_call (struc
if (TREE_CODE (binfo) != TREE_BINFO)
{
- binfo = gimple_extract_devirt_binfo_from_cst (binfo);
+ binfo = gimple_extract_devirt_binfo_from_cst
+(binfo, ie->indirect_info->otr_type);
if (!binfo)
return NULL;
}
Index: tree.c
===
--- tree.c (revision 201776)
+++ tree.c (working copy)
@@ -11864,6 +11864,21 @@ types_same_for_odr (tree type1, tree typ
return true;
}
+/* REF is OBJ_TYPE_REF, return the class the ref corresponds to. */
+
+tree
+obj_type_ref_class (tree ref)
+{
+ gcc_checking_assert (TREE_CODE (ref) == OBJ_TYPE_REF);
+ ref = TREE_TYPE (ref);
+ gcc_checking_assert (TREE_CODE (ref) == PO
Re: [PATCH] Fix bad interaction between GTY((user)) and incomplete declarations
On Fri, 2013-08-16 at 07:12 +0300, Laurynas Biveinis wrote: > > * gengtype.c (create_user_defined_type): Ensure that the kind > > is set to TYPE_USER_STRUCT, fixing a bug seen when an incomplete > > declaration is seen before the GTY((user)) marking. > > This is OK, thank you. Thanks; committed to trunk as r201791.
Re: [PATCH] Redesign pthread in LIB_SPEC for systems without libpthread
On Fri, Aug 16, 2013 at 3:21 AM, Maxim Kuvyrkov wrote:
> On 15/08/2013, at 10:49 PM, Alexander Ivchenko wrote:
>
>> Could anybody please take a look? This is important for building gcc for
>> android.
>>
>> ping^4
>
> [Sorry for being cranky]
>
> There is a reason why people are ignoring your patch -- the submission is not
> very well structured.
>
> First, the subject -- it is not accurate. "Redesign" implies a big change,
> you would be better off with a heading like "Fix LIB_SPEC for Android".
>
> Second, you do not fully describe the problem that you are trying to fix.
>
> Third, you do not say how your patch fixes the problem.
>
> Lastly, you do not mention which targets you have encountered the problem on
> and tested the fix on. Since you are from Intel, I can guess that you use
> x86.
>
>
>> 2013-04-02 Pavel Chupin
>>
>> Redesign pthread in LIB_SPEC for systems without libpthread
>> * gcc/config/gnu-user.h: Remove pthread from
>> GNU_USER_TARGET_LIB_SPEC
>> but keep in default LIB_SPEC
>> * gcc/config/linux-android.h: Add pthread to ANDROID_LIB_SPEC
>>
>> Is it OK for trunk?
>
>
>> --- a/gcc/config/gnu-user.h
>> +++ b/gcc/config/gnu-user.h
>> @@ -74,11 +74,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
>> If not, see
>> #define CPLUSPLUS_CPP_SPEC "-D_GNU_SOURCE %(cpp)"
>>
>> #define GNU_USER_TARGET_LIB_SPEC \
>> - "%{pthread:-lpthread} \
>> - %{shared:-lc} \
>> + "%{shared:-lc} \
>
> Indentation seems off here, please double-check.
>
>> %{!shared:%{mieee-fp:-lieee} %{profile:-lc_p}%{!profile:-lc}}"
>> #undef LIB_SPEC
>> -#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC
>> +#define LIB_SPEC "%{pthread:-lpthread} " GNU_USER_TARGET_LIB_SPEC
>
> You need to similarly add "%{pthread:-lpthread} " to LIB_SPEC in
> gcc/config/mips/gnu-user.h.
>
>>
>> #if defined(HAVE_LD_EH_FRAME_HDR)
>> #define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
>> diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h
>> index 831a19c..aaf1d34 100644
>> --- a/gcc/config/linux-android.h
>> +++ b/gcc/config/linux-android.h
>> @@ -49,7 +49,8 @@
>>"%{!frtti:%{!fno-rtti: -fno-rtti}}"
>>
>> #define ANDROID_LIB_SPEC \
>> - "%{!static: -ldl}"
>> + "%{!static: -ldl}" \
>> + "%{pthread: -lc}"
>
> Add a comment to explain why -pthread option triggers -lc for Android.
>
> Please post the updated patch along with notes on which targets you have
> tested it. Test coverage should include at least x86_64-linux-gnu and one of
> android targets.
>
> Thank you,
>
> --
> Maxim Kuvyrkov
> www.kugelworks.com
>
>
Sorry for not being clear. Thanks for review.
It's late to change subj I think to avoid producing new thread but I
got your point.
Problem is that all 3 Android compilers (arm, x86, mips) are failed to
build on trunk due to libgomp and libatomic configure errors like (arm
example):
configure:14403:
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/xgcc
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/bin/
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/lib/
-isystem
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/include
-isystem
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/sys-include
-o conftest -g -Os -fno-sync-libcalls -pthread conftest.c >&5
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-binutils-2.23-arm-linux-androideabi/bin/arm-linux-androideabi-ld:
error: cannot find -lpthread
There is no libpthread library in Bionic, pthreads is integrated into libc.
My fix removes -lpthread from GNU_USER_TARGET_LIB_SPEC definition
which is included in both bionic and linux but keeping it in linux
LIB_SPEC.
Please see modified patch attached.
I've built all 3 Android compilers, x86_64 and i686 and ran simple
test with -pthread.
Updated ChangeLog:
2013-08-16 Pavel Chupin
Fix LIB_SPEC for systems without libpthread
* config/gnu-user.h: Remove pthread from
GNU_USER_TARGET_LIB_SPEC but keep at LIB_SPEC.
* config/arm/linux-eabi.h: Add pthread to linux LIB_SPEC.
* config/i386/linux-common.h: Likewise.
* config/mips/gnu-user.h: Likewise.
* config/mips/linux-common.h: Likewise.
Is it OK now?
--
Pavel Chupin
Intel Corporation
0001-Fix-LIB_SPEC-for-systems-without-libpthread.patch
Description: Binary data
Re: [PATCH,ARM] fix testsuite failures for arm-none-linux-gnueabihf
Hi Kyrylo
Thanks for the comments. I've done version 2 of the patch with the
changes incorporated
2013-08-16 Charles Baylis
* gcc.dg/builtin-apply2.c: skip test on arm hardfloat ABI targets
* gcc.dg/tls/pr42894.c: Use -mfloat-abi=soft as Thumb1 does
not support hardfloat ABI
* gcc,target/arm/thumb-ltu.c: Use -mfloat-abi=soft as Thumb1
does not support hardfloat ABI
* lib/target-supports.exp
(check_effective_target_arm_fp16_ok_nocache): don't force
-mfloat-abi=soft when building for hardfloat target
On 15 August 2013 16:13, Kyrylo Tkachov wrote:
> Hi Charles,
>
> CC'ing some of the ARM maintainers...
>
>> Hi
>>
>> The attached patch fixes some tests which fail when testing gcc for a
>> arm-none-linux-gnueabihf target because they do not expect to be built
>> with a hard float ABI.
>>
>> The change in target-supports.exp fixes arm-fp16-ops-5.c and arm-fp16-
>> ops-6.c.
>>
>> Tested on arm-none-linux-gnueabihf using qemu-arm, and does not cause
>> any other tests to break.
>>
>> Comments? This is my first patch, so please point out anything wrong.
>>
>
> --- gcc/testsuite/gcc.dg/tls/pr42894.c (revision 201726)
> +++ gcc/testsuite/gcc.dg/tls/pr42894.c (working copy)
> @@ -1,6 +1,7 @@
> /* PR target/42894 */
> /* { dg-do compile } */
> /* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
> +/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-*hf
> } } */
> /* { dg-require-effective-target tls } */
>
> This test also fails for a bare-metal arm-none-eabi target configured with
> hard float, so perhaps this could be:
>
> -/* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
> +/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-* }
> } */
>
>
>>
>>
>>
>> 2013-08-15 Charles Baylis
>>
>> * gcc.dg/builtin-apply2.c: skip test on arm hardfloat ABI targets
>> * gcc.dg/tls/pr42894.c: Use -mfloat-abi=soft as Thumb1 does
>> not support hardfloat ABI
>> * arm/thumb-ltu.c: Use -mfloat-abi=soft as Thumb1 does not
>> support hardfloat ABI
>
> ChangeLog entries are specified relative to the ChangeLog location. This
> should be:
>
> * gcc.target/arm/thumb-ltu.c:...
>
>> * target-supports.exp: don't force -mfloat-abi=soft when
>> building for hardfloat target
>
> Likewise, and also it's a good idea to specify which function/construct you're
> changing. In this case I think you're modifying
> check_effective_target_arm_fp16_ok_nocache. So this entry would be:
>
> * lib/target-supports.exp
> (check_effective_target_arm_fp16_ok_nocache):...
>
>
> Thanks,
> Kyrill
>
>
>
>
Index: gcc/testsuite/gcc.dg/builtin-apply2.c
===
--- gcc/testsuite/gcc.dg/builtin-apply2.c (revision 201726)
+++ gcc/testsuite/gcc.dg/builtin-apply2.c (working copy)
@@ -1,6 +1,7 @@
/* { dg-do run } */
/* { dg-skip-if "Variadic funcs have all args on stack. Normal funcs have args
in registers." { "aarch64*-*-* avr-*-* " } { "*" } { "" } } */
/* { dg-skip-if "Variadic funcs use Base AAPCS. Normal funcs use VFP
variant." { "arm*-*-*" } { "-mfloat-abi=hard" } { "" } } */
+/* { dg-skip-if "Variadic funcs use Base AAPCS. Normal funcs use VFP
variant." { "arm*-*-gnueabihf" } { "*" } { "-mfloat-abi=soft*" } } */
/* PR target/12503 */
/* Origin: */
Index: gcc/testsuite/gcc.dg/tls/pr42894.c
===
--- gcc/testsuite/gcc.dg/tls/pr42894.c (revision 201726)
+++ gcc/testsuite/gcc.dg/tls/pr42894.c (working copy)
@@ -1,6 +1,6 @@
/* PR target/42894 */
/* { dg-do compile } */
-/* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
+/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-* }
} */
/* { dg-require-effective-target tls } */
extern __thread int t;
Index: gcc/testsuite/gcc.target/arm/thumb-ltu.c
===
--- gcc/testsuite/gcc.target/arm/thumb-ltu.c(revision 201726)
+++ gcc/testsuite/gcc.target/arm/thumb-ltu.c(working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-skip-if "incompatible options" { arm*-*-* } { "-march=*" } {
"-march=armv6" "-march=armv6j" "-march=armv6z" } } */
-/* { dg-options "-mcpu=arm1136jf-s -mthumb -O2" } */
+/* { dg-options "-mcpu=arm1136jf-s -mthumb -O2 -mfloat-abi=soft" } */
void f(unsigned a, unsigned b, unsigned c, unsigned d)
{
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 201726)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -2445,6 +2445,11 @@ proc check_effective_target_arm_fp16_ok_
# Must generate floating-point instructions.
return 0
}
+if [check-flags [list "" { *-*-gnueabihf } { "*" } { "" } ]] {
+# Use existing float-abi and f
Re: [patch] [python libstdc++ printers] Fix gdb/15195
> "Phil" == Phil Muldoon writes: Phil> Anyway, I have regenerated the patch with the fixes requested. Thanks. Phil> 2013-08-16 Phil Muldoon Phil> PR gcc/53477 I think this should say PR libstdc++/53477 Other than this nit, it looks good to me. I can't remember if I can approve these, but I think not. Tom
v3 of patch (was Re: [PATCH 11/11] Make opt_pass and gcc::pipeline be GC-managed)
On Mon, 2013-08-05 at 06:59 -1000, Richard Henderson wrote: > On 08/05/2013 05:18 AM, David Malcolm wrote: > > So I *think* the most efficient traversal is to do this first (with a > > suitable comment): > > > > for (int i = passes_by_id_size ; i > 0; ) > >::gt_ggc_mx (passes_by_id[--i]); > > > > That ought to visit all of the passes without triggering recursion > > (unless someone does something weird to the pass structure in a plugin). > > Reasonable. > > > I'm nervous about omitting the traversal for other pointers the class > > has (though I *think* the passes by id array captures it all) so for > > completeness I'd prefer to then to also do it for all of: > > > > ::gt_ggc_mx (all_passes); > > ::gt_ggc_mx (all_small_ipa_passes); > > ::gt_ggc_mx (all_lowering_passes); > > ::gt_ggc_mx (all_regular_ipa_passes); > > ::gt_ggc_mx (all_lto_gen_passes); > > ::gt_ggc_mx (all_late_ipa_passes); > > > > #define INSERT_PASSES_AFTER(PASS) > > #define PUSH_INSERT_PASSES_WITHIN(PASS) > > #define POP_INSERT_PASSES() > > #define NEXT_PASS(PASS, NUM) ::gt_ggc_mx (PASS ## _ ## NUM); > > #define TERMINATE_PASS_LIST() > > > > #include "pass-instances.def" > > > > #undef INSERT_PASSES_AFTER > > #undef PUSH_INSERT_PASSES_WITHIN > > #undef POP_INSERT_PASSES > > #undef NEXT_PASS > > #undef TERMINATE_PASS_LIST > > > > Is it standard in handwritten GC code to omit traversals based on this > > higher-level knowledge? Presumably it warrants a source comment? > > (having spent too much time lately debugging PCH issues I'm nervous > > about trying to optimize this). > > It's something that we should think about when doing any kind of GC. > The deep recursion has bitten us before, and lead to the chain_next > and chain_prev annotations for GTY. > > > AIUI we could do similar optimizations in the PCH object-noting hook, > > but can't do similar optimizations in the PCH *op* hook, since every > > field needs to reconstructed when reading the data back from disk. > > Correct. Sorry about the delay in following up on this. I'm attaching an implementation of patch which incorporates the ideas for optimizing the GC traversal. It turned out that the passes_by_id array only contains *most* of the passes, not all of them: it only contains passes that had register_one_dump_file called on them, thus omitting those that have a "*" at the front of their names (or a NULL name, but I don't think there are any of these). So the pass_manager traversal hooks need to do some extra work after the backwards traversal of the passes_by_id array to ensure that we visit everything. I also tweaked the traversal hooks for opt_pass to emulate "chain_next", since this is where the really deep callchains could otherwise occur. See the patch for details (given the subtleties I opted to put big comments in the relevant routines). (I also fixed the typo "overide" to "override" as noted by Bernhard) Successfully bootstrapped and tested on x86_64-unknown-linux-gnu, on top of the other patches (Note that the gengtype fix I committed just now as r201791 is also needed [1]). OK for trunk? [1] http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00882.html commit 5deba6060b8d9991b6298af2d1122310e0415043 Author: David Malcolm Date: Fri Aug 2 15:58:46 2013 -0400 Make opt_pass and gcc::pass_manager be GC-managed This patch makes gcc::pass_manager and opt_pass instances be allocated within the GC-heap, and adds traversal hooks for GC/PCH, so that passes can own refs to other GC-allocated objects. It also adds templates to ggc.h, to create boilerplate for GTY((user)) types that gengtype can't handle. gcc/ Make opt_pass and gcc::pass_manager be GC-managed, so that pass instances can own GC refs. * Makefile.in (GTFILES): Add pass_manager.h and tree-pass.h. * context.c (gcc::context::gt_ggc_mx): Traverse passes_. (gcc::context::gt_pch_nx): Likewise. (gcc::context::gt_pch_nx): Likewise. * ggc.h (gt_ggc_mx ): New. (gt_pch_nx_with_op ): New. (gt_pch_nx ): New. * passes.c (opt_pass::gt_ggc_mx): New. (opt_pass::gt_pch_nx): New. (opt_pass::gt_pch_nx_with_op): New. (pass_manager::gt_ggc_mx): New. (pass_manager::gt_pch_nx): New. (pass_manager::gt_pch_nx_with_op): New. (pass_manager::operator new): Use ggc_internal_cleared_alloc_stat rather than xcalloc. * pass_manager.h (class pass_manager): Add GTY((user)) marking. (pass_manager::gt_ggc_mx): New. (pass_manager::gt_pch_nx): New. (pass_manager::gt_pch_nx_with_op): New. * tree-pass.h (class opt_pass): Add GTY((user)) marking. (opt_pass::operator new): New. (opt_pass::gt_ggc_mx): New. (opt_pass::gt_pch_nx): New. (opt_pass::gt_pch_nx_with_op): New. diff --git a/gcc/Makefile.in b/gcc/Makefile.in index b874a6b..9917aec 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -3828,6 +3828,8 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h $(
[PING]RE: [PATCH] Fix for PR c/57490
Has anyone had a chance to look at this patch? Is it OK for trunk? Thanks, -Balaji V. Iyer. > -Original Message- > From: Iyer, Balaji V > Sent: Monday, August 12, 2013 1:17 PM > To: Rainer Orth > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > (pola...@redhat.com) > Subject: RE: [PATCH] Fix for PR c/57490 > > > > > -Original Message- > > From: Rainer Orth [mailto:r...@cebitec.uni-bielefeld.de] > > Sent: Friday, August 09, 2013 7:54 AM > > To: Iyer, Balaji V > > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > > (pola...@redhat.com) > > Subject: Re: [PATCH] Fix for PR c/57490 > > > > Rainer Orth writes: > > > > > Rainer Orth writes: > > > > > >> "Iyer, Balaji V" writes: > > >> > > -Original Message- > > From: Jakub Jelinek [mailto:ja...@redhat.com] > > Sent: Monday, July 01, 2013 1:09 PM > > To: Iyer, Balaji V > > Cc: gcc-patches@gcc.gnu.org; Rainer Orth > > Subject: Re: [PATCH] Fix for PR c/57490 > > > > On Mon, Jul 01, 2013 at 05:02:57PM +, Iyer, Balaji V wrote: > > > OK. The fixed patch is attached. Here are the ChangeLog entries: > > > > > > gcc/cp/ChangeLog > > > 2013-07-01 Balaji V. Iyer > > > > > > > Still > > PR c/57490 > > hasn't been added to cp/ChangeLog and c/ChangeLog entries. > > > --- /dev/null > > > +++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57490.c > > > @@ -0,0 +1,25 @@ > > > > >>> > > >>> Fixed as you suggested. Here is the fixed Changelogs and patch is > attached. > > >>> > > >>> gcc/cp/ChangeLog > > >>> 2013-07-01 Balaji V. Iyer > > >>> > > >>> PR c/57490 > > >>> * cp-array-notation.c (cp_expand_cond_array_notations): Added a > > >>> check for truth values. > > >>> (expand_array_notation_exprs): Added truth values case. Removed > an > > >>> unwanted else. Added for-loop to walk through subtrees in > > >>> default > > >>> case. > > >>> > > >>> gcc/c/ChangeLog > > >>> 2013-07-01 Balaji V. Iyer > > >>> > > >>> PR c/57490 > > >>> * c-array-notation.c (fix_conditional_array_notations_1): Added > > >>> a > > >>> check for truth values. > > >>> (expand_array_notation_exprs): Added truth values case. Removed > an > > >>> unwanted else. Added for-loop to walk through subtrees in > > >>> default > > >>> case. > > >>> > > >>> > > >>> gcc/testsuite/ChangeLog > > >>> 2013-07-01 Balaji V. Iyer > > >>> > > >>> PR c/57490 > > >>> * c-c++-common/cilk-plus/AN/pr57490.c: New test. > > >> > > >> I've just tested this patch on i386-pc-solaris2.10: > > >> > > >> The c-c++-common/cilk-plus/AN/an-if.c test still FAILs for C++: > > >> > > >> FAIL: c-c++-common/cilk-plus/AN/an-if.c -fcilkplus (internal > > >> compiler error) > > >> FAIL: c-c++-common/cilk-plus/AN/an-if.c -fcilkplus (test for > > >> excess > > >> errors) > > [...] > > > This is still unfixed almost three weeks later. Balaji, could you > > > please have a look? > > > > This bug is now unfixed for two months, and no reaction whatsoever on > > the report. This is getting annoying since it generates large amount > > of testsuite noise. > > > > Please fix ASAP! > > > > Hi Rainer, > >First off, my sincerest apologies for letting this bug slip the cracks. I > am > attaching a patch that seem to work fine with the .i file that you have > submitted > in bugzilla for both C and C++. Please let me know if this fix works for you > and if > it is OK for trunk. > > Here are the Changelog entries: > gcc/c/ChangeLog > 2013-08-12 Balaji V. Iyer > > PR c/57490 > * c-array-notation.c (fix_conditional_array_notations_1): Added a > check for truth values. > (expand_array_notation_exprs): Added truth values case. Removed an > unwanted else. Added for-loop to walk through subtrees in default > case. > > gcc/cp/ChangeLog > 2013-08-12 Balaji V. Iyer > > PR c/57490 > * cp-array-notation.c (cp_expand_cond_array_notations): Added a > check for truth values. > (expand_array_notation_exprs): Added truth values case. Removed an > unwanted else. Added for-loop to walk through subtrees in default > case. > * typeck.c (cp_build_binary_op): Inherited the type of the array > notation for built-in array notation functions. > > gcc/testsuite/ChangeLog > 2013-07-01 Balaji V. Iyer > > PR c/57490 > * c-c++-common/cilk-plus/AN/pr57490.c: New test. > > > Yours sincerely, > > Balaji V. Iyer. > > > > Rainer > > > > -- > > -- > > --- Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
The following patch fixes a bug in the rs6000 DFP handling of some _Decimal128
patterns, namely the unary patterns neg, abs and nabs. These patterns use
the legacy binary floating point instructions fneg, fabs and fnabs to
twiddle the signbit. However, these instructions only operate on the first
register in the even/odd register pair, so for destiniation and source
operands that do not overlap, the destination's odd register will be left
uninitialized. This patch fixes that.
I also noticed there is no current way for the ABS and NABS patterns to be
generated, so I created some generic builtins so they can be. I have also
created testcases to verify we generate correct code.
This bootstrapped and regtested with no regressions. Is this ok for trunk?
David, since this is a bad code generation bug, is it ok to backport just
the *negtd2_fpr portion of the patch to the open release branches?
Peter
gcc/
* builtins.def (BUILT_IN_FABSD32): New DFP ABS builtin.
(BUILT_IN_FABSD64): Likewise.
(BUILT_IN_FABSD128): Likewise.
* builtins.c (expand_builtin): Add support for new DFP ABS builtins.
(fold_builtin_1): Likewise.
* config/rs6000/dfp.md (*negtd2_fpr): Handle non-overlapping destination
and source operands.
(*abstd2_fpr): Likewise.
(*nabstd2_fpr): Likewise.
gcc/testsuite/
* gcc.target/powerpc/dfp-dd-2.c: New test.
* gcc.target/powerpc/dfp-td-2.c: Likewise.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 201779)
+++ gcc/builtins.c (working copy)
@@ -5865,6 +5865,9 @@ expand_builtin (tree exp, rtx target, rt
switch (fcode)
{
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
target = expand_builtin_fabs (exp, target, subtarget);
if (target)
return target;
@@ -10314,6 +10317,9 @@ fold_builtin_1 (location_t loc, tree fnd
return fold_builtin_strlen (loc, type, arg0);
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
return fold_builtin_fabs (loc, arg0, type);
case BUILT_IN_ABS:
Index: gcc/builtins.def
===
--- gcc/builtins.def(revision 201779)
+++ gcc/builtins.def(working copy)
@@ -257,6 +257,9 @@ DEF_C99_BUILTIN(BUILT_IN_EXPM1L,
DEF_LIB_BUILTIN(BUILT_IN_FABS, "fabs", BT_FN_DOUBLE_DOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSF, "fabsf", BT_FN_FLOAT_FLOAT,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSL, "fabsl", BT_FN_LONGDOUBLE_LONGDOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD32, "fabsd32", BT_FN_DFLOAT32_DFLOAT32,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD64, "fabsd64", BT_FN_DFLOAT64_DFLOAT64,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD128, "fabsd128",
BT_FN_DFLOAT128_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_BUILTIN(BUILT_IN_FDIM, "fdim", BT_FN_DOUBLE_DOUBLE_DOUBLE,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIMF, "fdimf", BT_FN_FLOAT_FLOAT_FLOAT,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIML, "fdiml",
BT_FN_LONGDOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
Index: gcc/config/rs6000/dfp.md
===
--- gcc/config/rs6000/dfp.md(revision 201779)
+++ gcc/config/rs6000/dfp.md(working copy)
@@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
(neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fneg %0,%1"
- [(set_attr "type" "fp")])
+ "*
+{
+ if (REGNO (operands[0]) == REGNO (operands[1]))
+return \"fneg %0,%1\";
+ else
+return \"fneg %0,%1\;fmr %L0,%L1\";
+}"
+ [(set_attr "type" "fp")
+ (set_attr "length" "8")])
(define_expand "abstd2"
[(set (match_operand:TD 0 "gpc_reg_operand" "")
@@ -148,15 +155,29 @@ (define_insn "*abstd2_fpr"
[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
(abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fabs %0,%1"
- [(set_attr "type" "fp")])
+ "*
+{
+ if (REGNO (operands[0]) == REGNO (operands[1]))
+return \"fabs %0,%1\";
+ else
+return \"fabs %0,%1\;fmr %L0,%L1\";
+}"
+ [(set_attr "type" "fp")
+ (set_attr "length" "8")])
(define_insn "*nabstd2_fpr"
[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
(neg:TD (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d"]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fnabs %0,%1"
- [(set_attr "type" "fp")])
+ "*
+{
+ if (REGNO (operands[0]) == REGNO (operands[1]))
+return \"fnabs %0,%1\";
+ else
+return \"fnabs %
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, Aug 16, 2013 at 12:06:56PM -0500, Peter Bergner wrote:
> --- gcc/config/rs6000/dfp.md (revision 201779)
> +++ gcc/config/rs6000/dfp.md (working copy)
> @@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
>[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
>"TARGET_HARD_FLOAT && TARGET_FPRS"
> - "fneg %0,%1"
> - [(set_attr "type" "fp")])
> + "*
> +{
> + if (REGNO (operands[0]) == REGNO (operands[1]))
> +return \"fneg %0,%1\";
> + else
> +return \"fneg %0,%1\;fmr %L0,%L1\";
> +}"
> + [(set_attr "type" "fp")
> + (set_attr "length" "8")])
Is the length right for the firt case though?
I mean, shouldn't the insn have two alternatives:
=d,d 0,d and length 4,8 ?
> @@ -148,15 +155,29 @@ (define_insn "*abstd2_fpr"
>[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
>"TARGET_HARD_FLOAT && TARGET_FPRS"
> - "fabs %0,%1"
> - [(set_attr "type" "fp")])
> + "*
> +{
> + if (REGNO (operands[0]) == REGNO (operands[1]))
> +return \"fabs %0,%1\";
> + else
> +return \"fabs %0,%1\;fmr %L0,%L1\";
> +}"
> + [(set_attr "type" "fp")
> + (set_attr "length" "8")])
Ditto here.
Jakub
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, 2013-08-16 at 19:11 +0200, Jakub Jelinek wrote:
> On Fri, Aug 16, 2013 at 12:06:56PM -0500, Peter Bergner wrote:
> > --- gcc/config/rs6000/dfp.md(revision 201779)
> > +++ gcc/config/rs6000/dfp.md(working copy)
> > @@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
> >[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> > (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
> >"TARGET_HARD_FLOAT && TARGET_FPRS"
> > - "fneg %0,%1"
> > - [(set_attr "type" "fp")])
> > + "*
> > +{
> > + if (REGNO (operands[0]) == REGNO (operands[1]))
> > +return \"fneg %0,%1\";
> > + else
> > +return \"fneg %0,%1\;fmr %L0,%L1\";
> > +}"
> > + [(set_attr "type" "fp")
> > + (set_attr "length" "8")])
>
> Is the length right for the firt case though?
> I mean, shouldn't the insn have two alternatives:
> =d,d 0,d and length 4,8 ?
To be honest, I basically just cut/pasted the negtf2_internal pattern
without looking too closely, but I agree you are right. I'll redo the
patch with your suggestion. negtf2_internal always generates two insns,
so it doesn't need any changes.
Peter
Re: [PATCH] Fix for PR c/57490
On 08/12/2013 01:16 PM, Iyer, Balaji V wrote: + /* If it is a built-in array notation function, then the return type of + the function is the type of the array passed in as array notation. */ How can the function return an array? Jason
RE: [PATCH] Fix for PR c/57490
> -Original Message- > From: Jason Merrill [mailto:ja...@redhat.com] > Sent: Friday, August 16, 2013 2:08 PM > To: Iyer, Balaji V; Rainer Orth > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > (pola...@redhat.com) > Subject: Re: [PATCH] Fix for PR c/57490 > > On 08/12/2013 01:16 PM, Iyer, Balaji V wrote: > > + /* If it is a built-in array notation function, then the return type of > > + the function is the type of the array passed in as array notation. */ > > How can the function return an array? > I guess I worded the comment poorly. Well, what I was going to say is this: float x, A[10]; x = __sec_reduce_add (A[:]); // The sec_reduce_add function's return type is the type of A[] which is float. int y, B[10]; y = __sec_reduce_add (B[:]); // ...the return type of __sec_reduce_add function is same as B[] which is int Thanks, Balaji V. Iyer. > Jason
[GOOGLE] bug in discriminator assignment
This patch fixes a bug in assigning discrminator. We should explicitly
call the hash function when finding the next discriminator.
Bootstrapped and passed regression tests.
OK for google branches?
Thanks,
Dehao
Index: gcc/tree-cfg.c
===
--- gcc/tree-cfg.c (revision 201632)
+++ gcc/tree-cfg.c (working copy)
@@ -709,7 +709,7 @@ next_discriminator_for_locus (location_t locus)
item.discriminator = 0;
slot = (struct locus_discrim_map **)
htab_find_slot_with_hash (discriminator_per_locus, (void *) &item,
-(hashval_t) locus, INSERT);
+locus_map_hash (&item), INSERT);
gcc_assert (slot);
if (*slot == HTAB_EMPTY_ENTRY)
{
Re: [PATCH 09/11] Support "gcc" namespace in gengtype
On Thu, 2013-08-01 at 11:01 -1000, Richard Henderson wrote: > On 07/26/2013 05:04 AM, David Malcolm wrote: > > +"/* Types with a \"gcc::\" prefix have the prefix stripped\n" > > +" during gengtype parsing. Provide a \"using\" directive\n" > > +" to ensure that the fully-qualified types are found. */\n" > > I'd rather not use the word "prefix"; the term "namespace" is both more exact > and less confusing in this context. > > Otherwise ok. Thanks; committed to trunk as r201800, having fixed up the wording, and bootstrapped and tested.
[PATCH] Fix PR58139 but correctly initializing reg_raw_mode[]
PR58139 shows a case where sched2 is scheduling the definition of a volatile
VSX register across a call. The volatile VSX register (62) is actually made
up the non-volatile FPR30 and another volatile doubleword, so it is actually
partially volatile. Since FPR30 is defined by the PPC{,64} ABIs as being
non-volatile, it is not marked in CALL_USED_REGISTERS[]. This means that
the DF infrastruture doesn't record the dependency between the call and
the VSX instruction, so sched2 freely schedules the VSX instruction past
the call.
There is code in sched-deps.c:deps_analyze_insn() that looks for partial
clobbers by testing HARD_REGNO_CALL_PART_CLOBBERED (when insn is a call insn).
HARD_REGNO_CALL_PART_CLOBBERED is correctly handled, meaning passing in 62
and a vector mode like V2DFmode returns true. However, we don't see the
clobber because the mode it uses when testing HARD_REGNO_CALL_PART_CLOBBERED
is reg_raw_mode[62], which is set to DImode, and hardreg 62 is not partially
clobbered in DImode.
The definition of reg_raw_mode[] is defined as:
/* For each hard register, the widest mode object that it can contain.
This will be a MODE_INT mode if the register can hold integers. Otherwise
it will be a MODE_FLOAT or a MODE_CC mode, whichever is valid for the
register. */
Now DImode is not the widest mode that can fit into hardreg 62, rather
it is the vector modes like V2DFmode, etc. The code that initializes
reg_raw_mode[] is in reginfo.c:init_reg_modes_target() which uses the
value returned by choose_hard_reg_mode(i, 1, false).
Looking at choose_hard_reg_mode(), it loops through several mode classes
looking for modes that are ok. The bug seems to be that it returns too
quickly when it finds a valid mode. To find the "widest" mode, it really
should scan through all mode classes, choosing the widest mode it encounters.
The patch below does that and I can confirm with this patch, the scheduler
sees the dependency between the call and the vsx instruction, correctly
blocking the move.
Pat Haugen SPEC tested this on powerpc64-linux and saw no degradations.
This also passed powerpc64-linux bootstrap and regtesting.
Is this Ok for mainline?
Since this is a wrong code bug, is this also ok on the 4.8 branch after
a couple of weeks, as long as no one finds any issues with it?
Peter
PR58139
* gcc/reginfo.c (choose_hard_reg_mode): San through all mode classes
looking for widest mode.
Index: gcc/reginfo.c
===
--- gcc/reginfo.c (revision 201779)
+++ gcc/reginfo.c (working copy)
@@ -620,40 +620,35 @@ choose_hard_reg_mode (unsigned int regno
mode = GET_MODE_WIDER_MODE (mode))
if ((unsigned) hard_regno_nregs[regno][mode] == nregs
&& HARD_REGNO_MODE_OK (regno, mode)
- && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode)))
+ && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
+ && GET_MODE_SIZE (mode) > GET_MODE_SIZE (found_mode))
found_mode = mode;
- if (found_mode != VOIDmode)
-return found_mode;
-
for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT);
mode != VOIDmode;
mode = GET_MODE_WIDER_MODE (mode))
if ((unsigned) hard_regno_nregs[regno][mode] == nregs
&& HARD_REGNO_MODE_OK (regno, mode)
- && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode)))
+ && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
+ && GET_MODE_SIZE (mode) > GET_MODE_SIZE (found_mode))
found_mode = mode;
- if (found_mode != VOIDmode)
-return found_mode;
-
for (mode = GET_CLASS_NARROWEST_MODE (MODE_VECTOR_FLOAT);
mode != VOIDmode;
mode = GET_MODE_WIDER_MODE (mode))
if ((unsigned) hard_regno_nregs[regno][mode] == nregs
&& HARD_REGNO_MODE_OK (regno, mode)
- && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode)))
+ && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
+ && GET_MODE_SIZE (mode) > GET_MODE_SIZE (found_mode))
found_mode = mode;
- if (found_mode != VOIDmode)
-return found_mode;
-
for (mode = GET_CLASS_NARROWEST_MODE (MODE_VECTOR_INT);
mode != VOIDmode;
mode = GET_MODE_WIDER_MODE (mode))
if ((unsigned) hard_regno_nregs[regno][mode] == nregs
&& HARD_REGNO_MODE_OK (regno, mode)
- && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode)))
+ && (! call_saved || ! HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
+ && GET_MODE_SIZE (mode) > GET_MODE_SIZE (found_mode))
found_mode = mode;
if (found_mode != VOIDmode)
[Google] fix a bug in lipo varpool node linking
This patch fixed a bug in lipo varpool node linking.
C++ FE drops the initializer if it's not used in this TU. For current
varpool linking may
resolve the varpool node to the one with null initializer.
-Rong
Index: l-ipo.c
===
--- l-ipo.c (revision 201800)
+++ l-ipo.c (working copy)
@@ -2151,6 +2151,19 @@ resolve_varpool_node (struct varpool_node **slot,
return;
}
+ if (DECL_INITIAL (decl1) && !DECL_INITIAL (decl2))
+{
+ merge_addressable_attr (decl1, decl2);
+ return;
+}
+
+ if (!DECL_INITIAL (decl1) && DECL_INITIAL (decl2))
+{
+ *slot = node;
+ merge_addressable_attr (decl2, decl1);
+ return;
+}
+
/* Either all complete or neither's type is complete. Just
pick the primary module's decl. */
if (!varpool_is_auxiliary (*slot))
[patch, google] Update powerpc64 xfail file.
I fixed the bits in our DejaGNU powerpc64 board file to transfer profile output back to the build system from the remote target, which means that a bunch of test failures now go away, and we uncover a small handful of new ones. This patch updates the xfail file accordingly. Committed as obvious. - Brooks 2013-08-16_powerpc64-xfail-update.diff Description: Binary data
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, 2013-08-16 at 19:11 +0200, Jakub Jelinek wrote:
> On Fri, Aug 16, 2013 at 12:06:56PM -0500, Peter Bergner wrote:
> > --- gcc/config/rs6000/dfp.md(revision 201779)
> > +++ gcc/config/rs6000/dfp.md(working copy)
> > @@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
> >[(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> > (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
> >"TARGET_HARD_FLOAT && TARGET_FPRS"
> > - "fneg %0,%1"
> > - [(set_attr "type" "fp")])
> > + "*
> > +{
> > + if (REGNO (operands[0]) == REGNO (operands[1]))
> > +return \"fneg %0,%1\";
> > + else
> > +return \"fneg %0,%1\;fmr %L0,%L1\";
> > +}"
> > + [(set_attr "type" "fp")
> > + (set_attr "length" "8")])
>
> Is the length right for the firt case though?
> I mean, shouldn't the insn have two alternatives:
> =d,d 0,d and length 4,8 ?
Is this better?
Peter
gcc/
* builtins.def (BUILT_IN_FABSD32): New DFP ABS builtin.
(BUILT_IN_FABSD64): Likewise.
(BUILT_IN_FABSD128): Likewise.
* builtins.c (expand_builtin): Add support for new DFP ABS builtins.
(fold_builtin_1): Likewise.
* config/rs6000/dfp.md (*negtd2_fpr): Handle non-overlapping destination
and source operands.
(*abstd2_fpr): Likewise.
(*nabstd2_fpr): Likewise.
gcc/testsuite/
* gcc.target/powerpc/dfp-dd-2.c: New test.
* gcc.target/powerpc/dfp-td-2.c: Likewise.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 201779)
+++ gcc/builtins.c (working copy)
@@ -5865,6 +5865,9 @@ expand_builtin (tree exp, rtx target, rt
switch (fcode)
{
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
target = expand_builtin_fabs (exp, target, subtarget);
if (target)
return target;
@@ -10314,6 +10317,9 @@ fold_builtin_1 (location_t loc, tree fnd
return fold_builtin_strlen (loc, type, arg0);
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
return fold_builtin_fabs (loc, arg0, type);
case BUILT_IN_ABS:
Index: gcc/builtins.def
===
--- gcc/builtins.def(revision 201779)
+++ gcc/builtins.def(working copy)
@@ -257,6 +257,9 @@ DEF_C99_BUILTIN(BUILT_IN_EXPM1L,
DEF_LIB_BUILTIN(BUILT_IN_FABS, "fabs", BT_FN_DOUBLE_DOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSF, "fabsf", BT_FN_FLOAT_FLOAT,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSL, "fabsl", BT_FN_LONGDOUBLE_LONGDOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD32, "fabsd32", BT_FN_DFLOAT32_DFLOAT32,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD64, "fabsd64", BT_FN_DFLOAT64_DFLOAT64,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD128, "fabsd128",
BT_FN_DFLOAT128_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_BUILTIN(BUILT_IN_FDIM, "fdim", BT_FN_DOUBLE_DOUBLE_DOUBLE,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIMF, "fdimf", BT_FN_FLOAT_FLOAT_FLOAT,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIML, "fdiml",
BT_FN_LONGDOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
Index: gcc/config/rs6000/dfp.md
===
--- gcc/config/rs6000/dfp.md(revision 201779)
+++ gcc/config/rs6000/dfp.md(working copy)
@@ -132,11 +132,14 @@ (define_expand "negtd2"
"")
(define_insn "*negtd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
+ [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
+ (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d,0")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fneg %0,%1"
- [(set_attr "type" "fp")])
+ "@
+ fneg %0,%1\;fmr %L0,%L1
+ fneg %0,%1"
+ [(set_attr "type" "fp")
+ (set_attr "length" "8,4")])
(define_expand "abstd2"
[(set (match_operand:TD 0 "gpc_reg_operand" "")
@@ -145,18 +148,24 @@ (define_expand "abstd2"
"")
(define_insn "*abstd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
+ [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
+ (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d,0")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fabs %0,%1"
- [(set_attr "type" "fp")])
+ "@
+ fabs %0,%1\;fmr %L0,%L1
+ fabs %0,%1"
+ [(set_attr "type" "fp")
+ (set_attr "length" "8,4")])
(define_insn "*nabstd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (neg:TD (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d"]
+ [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
+ (neg:TD (abs:TD (match_operand:
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, Aug 16, 2013 at 04:18:18PM -0500, Peter Bergner wrote: > --- gcc/config/rs6000/dfp.md (revision 201779) > +++ gcc/config/rs6000/dfp.md (working copy) > @@ -132,11 +132,14 @@ (define_expand "negtd2" >"") > > (define_insn "*negtd2_fpr" > - [(set (match_operand:TD 0 "gpc_reg_operand" "=d") > - (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))] > + [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d") > + (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d,0")))] I'm surprised if it works in this order, I'd expect that alternative 0 would match even for the same regno and that the second alternative wouldn't be tried. Jakub
Re: [Google] fix a bug in lipo varpool node linking
ok.
David
On Fri, Aug 16, 2013 at 1:28 PM, Rong Xu wrote:
> This patch fixed a bug in lipo varpool node linking.
>
> C++ FE drops the initializer if it's not used in this TU. For current
> varpool linking may
> resolve the varpool node to the one with null initializer.
>
> -Rong
>
>
> Index: l-ipo.c
> ===
> --- l-ipo.c (revision 201800)
> +++ l-ipo.c (working copy)
> @@ -2151,6 +2151,19 @@ resolve_varpool_node (struct varpool_node **slot,
>return;
> }
>
> + if (DECL_INITIAL (decl1) && !DECL_INITIAL (decl2))
> +{
> + merge_addressable_attr (decl1, decl2);
> + return;
> +}
> +
> + if (!DECL_INITIAL (decl1) && DECL_INITIAL (decl2))
> +{
> + *slot = node;
> + merge_addressable_attr (decl2, decl1);
> + return;
> +}
> +
>/* Either all complete or neither's type is complete. Just
> pick the primary module's decl. */
>if (!varpool_is_auxiliary (*slot))
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, 2013-08-16 at 23:25 +0200, Jakub Jelinek wrote: > On Fri, Aug 16, 2013 at 04:18:18PM -0500, Peter Bergner wrote: > > --- gcc/config/rs6000/dfp.md(revision 201779) > > +++ gcc/config/rs6000/dfp.md(working copy) > > @@ -132,11 +132,14 @@ (define_expand "negtd2" > >"") > > > > (define_insn "*negtd2_fpr" > > - [(set (match_operand:TD 0 "gpc_reg_operand" "=d") > > - (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))] > > + [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d") > > + (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d,0")))] > > I'm surprised if it works in this order, I'd expect that alternative 0 > would match even for the same regno and that the second alternative wouldn't > be tried. It did that way because I thought that was the order they had to be in due to this in the internals doc: ‘0’, ‘1’, ‘2’, ... ‘9’ An operand that matches the specified operand number is allowed. If a digit is used together with letters within the same alternative, the digit should come last. I see I missed the "within the same alternative" and yeah, I guess I'm a little surprised the first alternative didn't kick in now too. I'll swap them to be sure and I'll add a couple of extra routines in the tests that test both alternatives. Peter
Re: [PATCH] MIPS/libgcc: Avoid the PLT in MIPS16 stub calls
On Wed, 7 Aug 2013, Richard Sandiford wrote:
> > /* Define a function NAME that moves a return value of mode MODE from
> > FPRs to GPRs. */
> >
> > -#define RET_FUNCTION(NAME, MODE) \
> > +#define _RET_FUNCTION(NAME, MODE) \
> > STARTFN (NAME);\
> > MOVE_##MODE##_RET (t, $31); \
> > ENDFN (NAME)
> >
> > +#ifdef SHARED
> > +#define RET_FUNCTION(NAME, MODE) \
> > + _RET_FUNCTION (NAME##_compat, MODE);\
> > + .symver NAME##_compat, NAME##@GCC_4.4.0
> > +#else
> > +#define RET_FUNCTION(NAME, MODE) \
> > + _RET_FUNCTION (NAME, MODE); \
> > + .hidden NAME
> > +#endif
>
> Rather than repeat this, I think we should have:
>
> #ifdef SHARED
> #define CE_STARTFN(NAME) \
> STARTFN (NAME##_compat); \
> .symver NAME##_compat, NAME@GCC_4.4.0
> #define CE_ENDFN(NAME) ENDFN (NAME##_compat)
> #else
> #define CE_STARTFN(NAME) STARTFN (NAME); .hidden NAME
> #define CE_ENDFN(NAME) ENDFN (NAME)
> #endif
>
> below your new comment, with CE arbitrarily standing for "compat export".
Indeed, clever. I was wondering if it was possible to avoid the
repetition, but missed the ability to reuse STARTFN/ENDFN for this
purpose.
> Feel free to use different names if you can think of something better.
I think yours are as good as any other ones might be.
> Note no "##" before "@".
Indeed, the versioned symbol alias is not a valid preprocessor token.
> Please also delete the corresponding entries in libgcc.ver, which should
> no longer be needed.
Done, thanks for pointing it out.
> OK with those changes, thanks. No need for a full retest; checking
> that libgcc_s.so.1 and libgcc.a are unchanged from your posted version
> should be fine.
No differences found. Here's what I applied (note that I decided to keep
the formatting style consistent between the two CE_STARTFN variants).
2013-08-16 Maciej W. Rozycki
Catherine Moore
Richard Sandiford
libgcc/
* config/mips/mips16.S (CE_STARTFN, CE_ENDFN): New macros.
(RET_FUNCTION): Use them in place of STARTFN and ENDFN.
(CALL_STUB_NO_RET): Likewise.
(CALL_STUB_RET): Likewise.
* config/mips/libgcc-mips16.ver: Remove __mips16_call_stub and
__mips16_ret call/return stub symbols.
* config.host : For non-R5900 add t-slibgcc-libgcc
to tmake_file.
Maciej
gcc-mips16-hidden-stub.diff
Index: gcc-fsf-trunk-quilt/libgcc/config.host
===
--- gcc-fsf-trunk-quilt.orig/libgcc/config.host 2013-08-16 20:19:15.0
+0100
+++ gcc-fsf-trunk-quilt/libgcc/config.host 2013-08-16 20:21:23.568799943
+0100
@@ -741,13 +741,13 @@ mips*-*-linux*) # Linux MIPS,
either
extra_parts="$extra_parts crtfastmath.o"
tmake_file="${tmake_file} t-crtfm"
case ${host} in
- mips64r5900* | mipsr5900*)
- # The MIPS16 support code uses floating point
- # instructions that are not supported on r5900.
- ;;
- *)
- tmake_file="${tmake_file} mips/t-mips16"
- ;;
+ mips64r5900* | mipsr5900*)
+ # The MIPS16 support code uses floating point
+ # instructions that are not supported on r5900.
+ ;;
+ *)
+ tmake_file="${tmake_file} mips/t-mips16 t-slibgcc-libgcc"
+ ;;
esac
md_unwind_header=mips/linux-unwind.h
if test "${ac_cv_sizeof_long_double}" = 16; then
Index: gcc-fsf-trunk-quilt/libgcc/config/mips/libgcc-mips16.ver
===
--- gcc-fsf-trunk-quilt.orig/libgcc/config/mips/libgcc-mips16.ver
2013-08-16 22:41:01.0 +0100
+++ gcc-fsf-trunk-quilt/libgcc/config/mips/libgcc-mips16.ver2013-08-16
22:41:09.617705885 +0100
@@ -45,42 +45,4 @@ GCC_4.4.0 {
__mips16_floatsidf
__mips16_floatunsidf
__mips16_fix_truncdfsi
- __mips16_ret_sf
- __mips16_ret_sc
- __mips16_ret_df
- __mips16_ret_dc
- __mips16_call_stub_1
- __mips16_call_stub_5
- __mips16_call_stub_2
- __mips16_call_stub_6
- __mips16_call_stub_9
- __mips16_call_stub_10
- __mips16_call_stub_sf_0
- __mips16_call_stub_sf_1
- __mips16_call_stub_sf_5
- __mips16_call_stub_sf_2
- __mips16_call_stub_sf_6
- __mips16_call_stub_sf_9
- __mips16_call_stub_sf_10
- __mips16_call_stub_sc_0
- __mips16_call_stub_sc_1
- __mips16_call_stub_sc_5
- __mips16_call_stub_sc_2
- __mips16_call_stub_sc_6
- __mips16_call_stub_sc_9
- __mips16_call_stub_sc_10
- __mips16_call_stub_df_0
- __mips16_call_stub_df_1
- __mips16_call_stub_df_5
- __mips16_call_stub_df_2
- __mips16_call_stub_df_6
- __mips16_call_stub_df_9
- __mips16_call_stub_df_10
- __mips16_call_stub_dc_0
- __mips16_call_stub_dc_1
- __mips16_call_stub_dc_5
- __mips16_call_stub_dc_2
-
Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
OK, I *think* I have done as you requested. I have to try the
environment variable before falling back on stderr (there's a program
we want to use this on that disables the ability to write to stderr).
I have added the secure_getenv stuff as you requested. The fixed patch
is attached.
Please review the patch and let me know if this is OK to commit. Thanks!
-- Caroline Tice
cmt...@google.com
2013-08-16 Caroline Tice
* configure.ac: Add check for __secure_getenv and secure_getenv.
* configure: Regenerate.
* vtv_utils.cc : Include stdlib.h
(HAVE_SECURE_GETENV): Add checks and definitions for secure_getenv.
(log_dirs): Remove file static constant.
(__vtv_open_log): Increase size of log file name. Add the user
and process ids to the file name. Do not put the log files in /tmp.
Instead try to get the directory name from an environment variable; if
that fails try to use stderr. Add O_NOFOLLOW to the flags
for 'open'. Update function comment.
* vtv_rts.cc (log_memory_protection_data): Remove %d from file name.
On Wed, Aug 14, 2013 at 8:33 AM, Florian Weimer wrote:
> On 08/12/2013 07:07 PM, Caroline Tice wrote:
>
>> The feature is supposed to be active in production code (like the
>> stack protector).
>
>
> Okay, and it makes sense to enable this in programs that run with different
> privileges than the invoking user.
>
> If a trust transition occurred during the past execve, libvtv must not use
> the current directory to store files, or derive file paths from environment
> variables. Using shared directories such as /tmp is also tricky. (The
> current libvtv version in trunk has an arbitrary file creation vulnerability
> because of the hardcoded path in /tmp.)
>
> Realistically, I think libvtv can only log to standard error (or perhaps
> syslog/the journal) if it detects it runs with elevated privileges.
>
> One way to achieve that is to return dup(2) as the file descriptor if
> logs_dir is NULL (after changing getenv to secure_getenv), instead of
> setting logs_dir to ".". This means that unless the environment variable is
> set, nothing would be logged to disk. I'm not sure if this what you want.
> But it follows the principle of least surprise, though. Creating log files
> in strange places because of a crash is unusual.
>
> If you insist on dumping stuff into the current directory by default, you'll
> have to use getauxval(AT_SECURE) (glibc 2.17 and later),
> __libc_enable_secure (some glibc systems, but not Fedora and the ELs),
> issetugid (Solaris and the BSDs) or an explicit comparison between
> getuid()/geteuid() and getgid()/getegid() (all the rest, slightly unsafe on
> Linux). I can write down this approach in more detail, it should probably
> go into the Fedora security guide anyway.
>
> I also noticed this in log_memory_protection_data:
>
> log_fd = __vtv_open_log ("vtv_memory_protection_data_%d.log");
>
> The "_%d" should probably be dropped because the argument is not a format
> string.
>
>
>
> --
> Florian Weimer / Red Hat Product Security Team
vtv-update-tmpdir.patch
Description: Binary data
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, 2013-08-16 at 23:25 +0200, Jakub Jelinek wrote:
> On Fri, Aug 16, 2013 at 04:18:18PM -0500, Peter Bergner wrote:
> > --- gcc/config/rs6000/dfp.md(revision 201779)
> > +++ gcc/config/rs6000/dfp.md(working copy)
> > @@ -132,11 +132,14 @@ (define_expand "negtd2"
> >"")
> >
> > (define_insn "*negtd2_fpr"
> > - [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> > - (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
> > + [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
> > + (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d,0")))]
>
> I'm surprised if it works in this order, I'd expect that alternative 0
> would match even for the same regno and that the second alternative wouldn't
> be tried.
Ok, updated to switch the order of the alternatives. This works...just
like the previous one. I created two versions of the TD test case to
test both alternatives and to make sure we get fmrs on the one and
no fmrs on the other.
Peter
gcc/
* builtins.def (BUILT_IN_FABSD32): New DFP ABS builtin.
(BUILT_IN_FABSD64): Likewise.
(BUILT_IN_FABSD128): Likewise.
* builtins.c (expand_builtin): Add support for new DFP ABS builtins.
(fold_builtin_1): Likewise.
* config/rs6000/dfp.md (*negtd2_fpr): Handle non-overlapping destination
and source operands.
(*abstd2_fpr): Likewise.
(*nabstd2_fpr): Likewise.
gcc/testsuite/
* gcc.target/powerpc/dfp-dd-2.c: New test.
* gcc.target/powerpc/dfp-td-2.c: Likewise.
* gcc.target/powerpc/dfp-td-3.c: Likewise.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 201779)
+++ gcc/builtins.c (working copy)
@@ -5865,6 +5865,9 @@ expand_builtin (tree exp, rtx target, rt
switch (fcode)
{
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
target = expand_builtin_fabs (exp, target, subtarget);
if (target)
return target;
@@ -10314,6 +10317,9 @@ fold_builtin_1 (location_t loc, tree fnd
return fold_builtin_strlen (loc, type, arg0);
CASE_FLT_FN (BUILT_IN_FABS):
+case BUILT_IN_FABSD32:
+case BUILT_IN_FABSD64:
+case BUILT_IN_FABSD128:
return fold_builtin_fabs (loc, arg0, type);
case BUILT_IN_ABS:
Index: gcc/builtins.def
===
--- gcc/builtins.def(revision 201779)
+++ gcc/builtins.def(working copy)
@@ -257,6 +257,9 @@ DEF_C99_BUILTIN(BUILT_IN_EXPM1L,
DEF_LIB_BUILTIN(BUILT_IN_FABS, "fabs", BT_FN_DOUBLE_DOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSF, "fabsf", BT_FN_FLOAT_FLOAT,
ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSL, "fabsl", BT_FN_LONGDOUBLE_LONGDOUBLE,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD32, "fabsd32", BT_FN_DFLOAT32_DFLOAT32,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD64, "fabsd64", BT_FN_DFLOAT64_DFLOAT64,
ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN(BUILT_IN_FABSD128, "fabsd128",
BT_FN_DFLOAT128_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_C99_BUILTIN(BUILT_IN_FDIM, "fdim", BT_FN_DOUBLE_DOUBLE_DOUBLE,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIMF, "fdimf", BT_FN_FLOAT_FLOAT_FLOAT,
ATTR_MATHFN_FPROUNDING_ERRNO)
DEF_C99_BUILTIN(BUILT_IN_FDIML, "fdiml",
BT_FN_LONGDOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
Index: gcc/config/rs6000/dfp.md
===
--- gcc/config/rs6000/dfp.md(revision 201779)
+++ gcc/config/rs6000/dfp.md(working copy)
@@ -132,11 +132,14 @@ (define_expand "negtd2"
"")
(define_insn "*negtd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
+ [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
+ (neg:TD (match_operand:TD 1 "gpc_reg_operand" "0,d")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fneg %0,%1"
- [(set_attr "type" "fp")])
+ "@
+ fneg %0,%1
+ fneg %0,%1\;fmr %L0,%L1"
+ [(set_attr "type" "fp")
+ (set_attr "length" "4,8")])
(define_expand "abstd2"
[(set (match_operand:TD 0 "gpc_reg_operand" "")
@@ -145,18 +148,24 @@ (define_expand "abstd2"
"")
(define_insn "*abstd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
+ [(set (match_operand:TD 0 "gpc_reg_operand" "=d,d")
+ (abs:TD (match_operand:TD 1 "gpc_reg_operand" "0,d")))]
"TARGET_HARD_FLOAT && TARGET_FPRS"
- "fabs %0,%1"
- [(set_attr "type" "fp")])
+ "@
+ fabs %0,%1
+ fabs %0,%1\;fmr %L0,%L1"
+ [(set_attr "type" "fp")
+ (set_attr "length" "4,8")])
(define_insn "*nabstd2_fpr"
- [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
- (neg:TD (abs:T
[GOOGLE] Emit linkage_name when built with -gmlt and for abstract decls
This patch emits linkage_name at -gmlt. It also make sure abstract
decls' linkage_names are emitted so that inlined functions can also
find linkage name.
Bootstrapped and passed regression test.
OK for google branches?
Thanks,
Dehao
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 201632)
+++ gcc/dwarf2out.c (working copy)
@@ -16537,10 +16537,9 @@ add_src_coords_attributes (dw_die_ref die, tree de
static void
add_linkage_name (dw_die_ref die, tree decl)
{
- if (debug_info_level > DINFO_LEVEL_TERSE
+ if (debug_info_level > DINFO_LEVEL_NONE
&& (TREE_CODE (decl) == FUNCTION_DECL || TREE_CODE (decl) == VAR_DECL)
&& TREE_PUBLIC (decl)
- && !DECL_ABSTRACT (decl)
&& !(TREE_CODE (decl) == VAR_DECL && DECL_REGISTER (decl))
&& die->die_tag != DW_TAG_member)
{
[GOOGLE] Add discriminator for inlined callsite
This patch emits discriminator for inlined callsite. This is important
when there are two inlined callsites in the same line.
Bootstrapped, testing on going.
OK for google branches?
Thanks,
Dehao
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 201632)
+++ gcc/dwarf2out.c (working copy)
@@ -18807,12 +18807,16 @@ gen_label_die (tree decl, dw_die_ref context_die)
static inline void
add_call_src_coords_attributes (tree stmt, dw_die_ref die)
{
- expanded_location s = expand_location (BLOCK_SOURCE_LOCATION (stmt));
+ location_t locus = BLOCK_SOURCE_LOCATION (stmt);
+ expanded_location s = expand_location (locus);
if (dwarf_version >= 3 || !dwarf_strict)
{
add_AT_file (die, DW_AT_call_file, lookup_filename (s.file));
add_AT_unsigned (die, DW_AT_call_line, s.line);
+ unsigned discr = get_discriminator_from_locus (locus);
+ if (discr != 0)
+ add_AT_unsigned (die, DW_AT_discr, discr);
}
}
Re: [PATCH, rs6000, generic builtins] Fix unary TDmode patterns and add DFP ABS builtins
On Fri, 2013-08-16 at 17:32 -0500, Peter Bergner wrote: > Ok, updated to switch the order of the alternatives. This works...just > like the previous one. I created two versions of the TD test case to > test both alternatives and to make sure we get fmrs on the one and > no fmrs on the other. FYI, this did bootstrap and regtest with no errors. Ok for mainline? And is the *negtd2_fpr hunk ok for the 4.8 branch? Peter
Re: [patch] Make cxxfilt demangle internal-linkage templates
Ping?
On Wed, Aug 7, 2013 at 11:51 AM, Paul Pluzhnikov wrote:
> The following source:
>
> template static void f();
> void g() { f(); }
>
> results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.
>
> Richard Smith says:
>
> The ABI doesn't cover manglings for local symbols ...
> ... and c++filt is not able to cope with the L prefix here.
>
> I'm having a hard time seeing how this isn't a g++ bug and a matching
> c++filt bug.
--
Paul Pluzhnikov
Re: Fix class type lookup from OBJ_TYPE_REF
This patch causes an ICE when building libobjc, which is part of normal bootstrap. PR 58179 /nasfarm/edelsohn/src/src/libobjc/accessors.m: In function 'objc_getProperty': /nasfarm/edelsohn/src/src/libobjc/accessors.m:127:11: internal compiler error: in obj_type_ref_class, at tree.c:11876 result = RETAIN (*(pointer_to_ivar)); - David
Re: [wwwdocs] Add link to @gnutools on Twitter
On Aug 12, 2013, Gerald Pfeifer wrote: > Based on another suggestion by David I also added a link to the > gnutools account on Google+. Patch below, gcc.gnu.org updated. I don't think it's good for the GNU project to direct people towards proprietary spying networks. How about moving these to a freedom-respecting network based on GNU social, pump.io, diaspora*, and/or friendica, and directing people there instead? -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [PATCH] Fix for PR c/57490
On 08/16/2013 02:13 PM, Iyer, Balaji V wrote: + /* If it is a built-in array notation function, then the return type of + the function is the type of the array passed in as array notation. */ How can the function return an array? float x, A[10]; x = __sec_reduce_add (A[:]); // The sec_reduce_add function's return type is the type of A[] which is float. Ah, then the comment should say "...is the element type of the array...". Jason
Re: [PATCH] Redesign pthread in LIB_SPEC for systems without libpthread
On 17/08/2013, at 3:07 AM, Pavel Chupin wrote:
...
>
> It's late to change subj I think to avoid producing new thread but I
> got your point.
> Problem is that all 3 Android compilers (arm, x86, mips) are failed to
> build on trunk due to libgomp and libatomic configure errors like (arm
> example):
>
> configure:14403:
> /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/xgcc
> -B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/
> -B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/bin/
> -B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/lib/
> -isystem
> /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/include
> -isystem
> /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/sys-include
> -o conftest -g -Os -fno-sync-libcalls -pthread conftest.c >&5
> /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-binutils-2.23-arm-linux-androideabi/bin/arm-linux-androideabi-ld:
> error: cannot find -lpthread
>
> There is no libpthread library in Bionic, pthreads is integrated into libc.
> My fix removes -lpthread from GNU_USER_TARGET_LIB_SPEC definition
> which is included in both bionic and linux but keeping it in linux
> LIB_SPEC.
Thanks for the description. I thought a bit more about the problem, and it
might be possible to solve it more concisely:
1. In config/gnu-user.h you define GNU_USER_TARGET_NO_PTHREADS_LIB_SPEC to the
main part of GNU_USER_TARGET_LIB_SPEC. GNU_USER_TARGET_LIB_SPEC now becomes
'"%{pthread:-lpthread} " GNU_USER_TARGET_NO_PTHREADS_LIB_SPEC'.
2. In occurrences of LINUX_OR_ANDROID_LD you continue to use
GNU_USER_TARGET_LIB_SPEC for the first argument and
'GNU_USER_TARGET_NO_PTHREADS_LIB_SPEC " " ANDROID_LIB_SPEC' for the second
argument.
This way you are operating with named macros instead of verbatim
"%{pthread:-lpthread}", and, hopefully, this will be more clear to an outside
observer.
>
> Please see modified patch attached.
> I've built all 3 Android compilers, x86_64 and i686 and ran simple
> test with -pthread.
Thanks for good test coverage. I assume you have also tested a non-android
Linux target?
The patch is OK if adjusted to the comments above (or good arguments provided
why your current patch is more straigh-forward).
Thank you,
--
Maxim Kuvyrkov
www.kugelworks.com
Re: [PATCH] Sanitize block partitioning under -freorder-blocks-and-partition
On Fri, Aug 9, 2013 at 2:02 PM, Teresa Johnson wrote: > On Fri, Aug 9, 2013 at 8:28 AM, Jan Hubicka wrote: >>> > Do we sanity check that the cold partition does not contain any blocks of >>> > count 0? It may be that the profile is broken enough to make partitioning >>> > not work. >>> >>> Do you mean sanity check that the cold partition does not contain any >>> blocks of count > 0? (they should all be zero) I don't think that >>> sanity check is there, but I can try adding that. >> >> Thanks, lets start with this - I suppose we need to figure out if >> 1) the reachable blocks goes to cold section because partitioning decides >> so even if they have non-0 count. > > Right, this should be easy enough to check and should hopefully never happen. > >> 2) the reachable blocks goes to cold section because they have incorrectly >> updated count to 0 by someone > > A sanity check should find this too. But it can happen now for various > reasons like the comdat issue I described below. But it will be good > to flag these and fix them. > >> 3) profiling gets some blocks wrong. > > This is the one that will be tough to fix, if the training run isn't > representative. > >>> >>> The issue with such a sanity check may be due to the later fixup I >>> have in this patch (fixup_partitions). It is invoked after certain >>> optimizations on the cfg that may make hot blocks previously reached >>> by both hot and cold edges only reachable by cold blocks. These blocks >>> are remarked cold. If the profile data hasn't been updated correctly >>> it is possible that they would still have a non-0 count, although they >>> are essentially cold after the cfg transformation. >> >> Well, or the other posibility is that the edges was updated wrong >> and the blocks are really cold. We need to figure out if that happens >> commonly enough. >> >> I will try to think of some artificial testcases. >> >>> >>> But certainly such a sanity check should always succeed after the >>> original partitioning. >>> >>> > I can think of inlining where the count gets scaled all way down to 0. >>> > Perhaps >>> > count scaling code can be modified to never round towards 0 for block >>> > executing >>> > non-0 times... >>> >>> This reminds me of why this situation could happen. When I have been >>> testing this on the google branch I found situations where COMDAT >>> routines have 0 profile counts (if I remember correctly, this happens >>> when profile-gen binary has call to out-of-line copy of COMDAT in >>> module A, linker chooses the out-of-line copy from module B, therefore >>> the profile data for COMDAT in module A is 0). When the COMDAT gets >>> inlined, the 0 counts on its bbs are scaled to 0, even though the >>> callsite is non-zero. I have a patch that I was planning to send as a >>> follow-up that handles this case by propagating the callsite bb's >>> count to the inlined code when it has 0 counts, scaling by the edge >>> frequencies. I can either include that patch in this one, or send it >>> for review separately right now. Do you want to give it a try with >>> this one to see if it addresses the issue? >> >> This scenario should not happen with LTO setup: the LTO symbol tables >> contains >> code before early optimization and should be identical with profiling or >> without (modulo the new references and call from profile code). >> >> But this patch seems useful as a backup solution for non-LTO, so yes, please >> send it separately and I can try to double check that it really do not happen >> with LTO. >> (acutally LTO symtab may just chose COMDAT from module that has counts with >> it. >> It has all the info for it. I was thinkin about it few weeks back. It is >> bit hard to do - you need to verify that all references from the function are >> the same or linking might fail if you overwrite linker's decisiosns). > > I see, yes LTO can deal with this better since it has global > information. In non-LTO mode (including LIPO) we have the issue. > > I take it gimp is built with LTO and therefore shouldn't be hitting > this comdat issue? > > Let me do a couple things: Here is some status: > - port over my comdat inlining fix from the google branch to trunk and > send it for review. If you or Martin could try it to see if it helps > with function splitting to avoid the hits from the cold code that > would be great I have included the cleaned up patch below. I will send it to trunk for review, but included it below if you want to try it out before I send it pending a little more extensive testing I want to do after the cleanup (it passes bootstrap and some small test cases I checked manually). > - I'll add some new sanity checking to try to detect non-zero blocks > in the cold section, or 0 blocks reached by non-zero edges and see if > I can flush out any problems with my tests or a profiledbootstrap or > gimp. I added both of these and ran into issues due to profile maintenance. For example, there were non-zero blocks in t
