[committed] Fix up pr56564*.c testcases
Hi!
The darwin and PE binds_local_p targhooks signal that even for -fpic
symbols can't be normally interposed, thus the testcases aren't
usable on these targets, because they rely on possible symbol interposition.
Fixed thusly, committed as obvious.
2013-06-12 Jakub Jelinek
PR target/56564
* gcc.target/i386/pr56564-1.c: Skip on darwin, mingw and cygwin.
* gcc.target/i386/pr56564-3.c: Likewise.
--- gcc/testsuite/gcc.target/i386/pr56564-1.c.jj2013-06-11
09:33:50.0 +0200
+++ gcc/testsuite/gcc.target/i386/pr56564-1.c 2013-06-11 21:25:41.208273724
+0200
@@ -1,5 +1,6 @@
/* PR target/56564 */
/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-skip-if "No symbol interposition for PIC" { *-*-mingw* *-*-cygwin*
*-*-darwin* } } */
/* { dg-options "-O3 -fpic -fdump-tree-optimized" } */
struct S { long a, b; } s = { 5, 6 };
--- gcc/testsuite/gcc.target/i386/pr56564-3.c.jj2013-06-11
09:33:50.0 +0200
+++ gcc/testsuite/gcc.target/i386/pr56564-3.c 2013-06-11 21:26:37.259270382
+0200
@@ -1,5 +1,6 @@
/* PR target/56564 */
/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-skip-if "No symbol interposition for PIC" { *-*-mingw* *-*-cygwin*
*-*-darwin* } } */
/* { dg-options "-O3 -fpic -fdump-tree-optimized" } */
__thread struct S { long a, b; } s = { 5, 6 };
Jakub
Re: lower-subreg and IBM long double
On Tue, Jun 11, 2013 at 04:58:07PM +, Joseph S. Myers wrote:
> It's preferred to put the hook documentation in the doc string in
> target.def, so tm.texi.in then only contains the @hook line indicating
> where the documentation will go in tm.texi.
Ah, thanks. Revised patch with testcase.
gcc/
* rs6000.c (TARGET_INIT_LOWER_SUBREG): Define.
(rs6000_init_lower_subreg): New function.
* lower-subreg.c (init_lower_subreg): Call targetm.init_lower_subreg.
* target.def (init_lower_subreg): New.
* doc/tm.texi.in (TARGET_INIT_LOWER_SUBREG): Document.
* doc/tm.texi: Regenerate.
gcc/testsuite/
* gcc.target/powerpc/fabsl.c: New test.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 199975)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -59,6 +59,7 @@
#include "opts.h"
#include "tree-vectorizer.h"
#include "dumpfile.h"
+#include "lower-subreg.h"
#if TARGET_XCOFF
#include "xcoffout.h" /* get declarations of xcoff_*_section_name */
#endif
@@ -1357,6 +1358,8 @@ static const struct attribute_spec rs6000_attribut
#define TARGET_RTX_COSTS rs6000_rtx_costs
#undef TARGET_ADDRESS_COST
#define TARGET_ADDRESS_COST hook_int_rtx_mode_as_bool_0
+#undef TARGET_INIT_LOWER_SUBREG
+#define TARGET_INIT_LOWER_SUBREG rs6000_init_lower_subreg
#undef TARGET_DWARF_REGISTER_SPAN
#define TARGET_DWARF_REGISTER_SPAN rs6000_dwarf_register_span
@@ -27430,6 +27523,20 @@ rs6000_memory_move_cost (enum machine_mode mode, r
return ret;
}
+static void
+rs6000_init_lower_subreg (void *data)
+{
+ if (!TARGET_IEEEQUAD
+ && TARGET_HARD_FLOAT
+ && (TARGET_FPRS || TARGET_E500_DOUBLE)
+ && TARGET_LONG_DOUBLE_128)
+{
+ struct target_lower_subreg *info = (struct target_lower_subreg *) data;
+ info->x_choices[0].move_modes_to_split[TFmode] = false;
+ info->x_choices[1].move_modes_to_split[TFmode] = false;
+}
+}
+
/* Returns a code for a target-specific builtin that implements
reciprocal of the function, or NULL_TREE if not available. */
Index: gcc/lower-subreg.c
===
--- gcc/lower-subreg.c (revision 199975)
+++ gcc/lower-subreg.c (working copy)
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h"
#include "df.h"
#include "lower-subreg.h"
+#include "target.h"
#ifdef STACK_GROWS_DOWNWARD
# undef STACK_GROWS_DOWNWARD
@@ -287,6 +288,9 @@ init_lower_subreg (void)
if (LOG_COSTS)
fprintf (stderr, "\nSpeed costs\n===\n\n");
compute_costs (true, &rtxes);
+
+ if (targetm.init_lower_subreg)
+targetm.init_lower_subreg (this_target_lower_subreg);
}
static bool
Index: gcc/target.def
===
--- gcc/target.def (revision 199975)
+++ gcc/target.def (working copy)
@@ -2926,6 +2926,14 @@ DEFHOOK
void, (int *code, rtx *op0, rtx *op1, bool op0_preserve_value),
default_canonicalize_comparison)
+/* Allow modification of subreg choices. */
+DEFHOOK
+(init_lower_subreg,
+ "This hook allows modification of the choices the lower_subreg pass \
+will make for particular subreg modes. @var{data} is a pointer to a \
+@code{struct target_lower_subreg}.",
+ void, (void *data), NULL)
+
DEFHOOKPOD
(atomic_test_and_set_trueval,
"This value should be set if the result written by\
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in (revision 199975)
+++ gcc/doc/tm.texi.in (working copy)
@@ -6384,6 +6384,8 @@ should probably only be given to addresses with di
registers on machines with lots of registers.
@end deftypefn
+@hook TARGET_INIT_LOWER_SUBREG
+
@node Scheduling
@section Adjusting the Instruction Scheduler
Index: gcc/testsuite/gcc.target/powerpc/fabsl.c
===
--- gcc/testsuite/gcc.target/powerpc/fabsl.c(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/fabsl.c(revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc*-*-darwin* powerpc*-*-aix* rs6000-*-*
powerpc*-*-linux* } } } */
+/* { dg-options "-O2 -mlong-double-128" } */
+/* { dg-final { scan-assembler-not "lwz" } } */
+/* { dg-final { scan-assembler-not "ld" } } */
+
+long double
+_fabsl (long double x)
+{
+ return __builtin_fabsl (x);
+}
--
Alan Modra
Australia Development Lab, IBM
Re: Improve uncprop and coalescing
On 06/07/13 03:14, Richard Biener wrote:
+/* Given SSA_NAMEs NAME1 and NAME2, return true if they are candidates for
+ coalescing together, false otherwise.
+
+ This must stay consistent with the code in tree-ssa-live.c which
+ sets up base values in the var map. */
+
+bool
+gimple_can_coalesce_p (tree name1, tree name2)
+{
+ /* First check the SSA_NAME's associated DECL. We only want to
+ coalesce if they have the same DECL or both have no associated DECL.
*/
+ if (SSA_NAME_VAR (name1) != SSA_NAME_VAR (name2))
+return false;
+
+ /* Now check the types. If the types are the same, then we should
+ try to coalesce V1 and V2. */
+ tree t1 = TREE_TYPE (name1);
+ tree t2 = TREE_TYPE (name2);
+ if (t1 == t2)
+return true;
+
+ /* If the types are not the same, check for a canonical type match. This
+ (for example) allows coalescing when the types are fundamentally the
+ same, but just have different names. */
+ if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2))
Please use types_compatible_p (t1, t2) here, that's the correct API to use
here.
No it isn't. types_compatible_p is far too permissive in this context
without more surgery to tree-ssa-live.c.
So let's take two objects, P1 and P2 which are pointers to types T1 and
T2 where T1 != T2. Assume P1 and P2 are somehow connected by a copy or
PHI node and that they are anonymously named objects.
types_compatible_p will return true for (T1, T2) unless T1 or T2 is a
pointer to a function. So if we used types_compatible_p we will try to
coalesce P1 and P2 (which is seemingly a good thing).
Now in tree-ssa-live.c::var_map_base_init we have:
/* Build the base variable list, and point partitions at their bases. */
for (x = 0; x < num_part; x++)
{
...
if (SSA_NAME_VAR (var))
m->base.from = SSA_NAME_VAR (var);
else
/* This restricts what anonymous SSA names we can coalesce
as it restricts the sets we compute conflicts for.
Using TREE_TYPE to generate sets is the easies as
type equivalency also holds for SSA names with the same
underlying decl. */
m->base.from = TREE_TYPE (var);
...
}
The way we set up base.from in var_map_base_init imposes requirements on
what objects can be added to the coalescing lists. We can only allow
coalescing of objects with the same underlying SSA_NAME_VAR or anonymous
objects with the same underlying TREE_TYPE (as the code is written
today). That's a side effect of restricting the sets we compute
conflicts for. If we don't compute the conflicts, then we'll assume P1
and P2 don't conflict and happily coalesce them, regardless of whether
or not they actually do conflict.
To use types_compatible_p to drive decisions in tree-ssa-coalesce.c,
ISTM we'd have to change this code from tree-ssa-live.c to put all
anonymous objects with a compatible type in the same hash entry. I
don't see a good way to do that without iterating over the hash table
entries looking for a compatible match or completely reimplementing the
hash.
By first checking if an anonymous variable's type has an underlying
canonical type and using that instead to key decisions in
var_map_base_init, we can allow more objects onto the coalescing lists
in tree-ssa-coalesce.c. In particular we can allow two anonymous
objects where both have an underlying TYPE_CANONICAL that is the same.
if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2)
&& types_compatible_p (t1, t2))
because looking at useless_type_conversion_p it looks like pointer types
with different address-spaces may have the same canonical type. A comment
on why we check both, refering to var_map_base_init should also be added.
Ok with that change.
Seems to make sense. Though we still have a point of contention around
whether or not we can use types_compatible_p to drive decisions in
tree-ssa-coalesce.c. I'm pretty sure we can't without some significant
surgery in tree-ssa-live.c.
Jeff
RE: new mul* patterns "U" constraint in rl78
>> +;; [(set_attr "valloc" "umul")] >I think this one needs to be macax also, since the constraints have "v" in >them but the opcodes use ax. > Other than that, it's OK. Committed with above change. Thanks for the quick review. Regards, Kaushik
expand_expr tweaks to fix PR57134
The following patch fixes PR57134 by
a) excluding bitfield expansion when EXPAND_MEMORY, and
b) passing down the EXPAND_MEMORY modifier in a couple of places where
this does not currently happen on recursive calls to expand_expr().
(a) has precedent elsewhere in expr.c, eg. see expand_expr_real_1
, (b) is a little difficult to justify except to claim
there's no logical reason why it should be excluded nowadays. Hand
waving argument follows:
Of the seven expand_expr() modifiers, recursive calls made to handle
inner references of aggregate types currently exclude EXPAND_SUM,
EXPAND_WRITE, and EXPAND_MEMORY from the modifier passed. I suppose
EXPAND_SUM is excluded so that the inner expand_expr() won't return a
PLUS or MULT when we might be adding a further offset, but I can't see
why the other two modifiers are not passed. Digging through the
history, it looks like EXPAND_WRITE may have been missed by accident,
and EXPAND_MEMORY used to be an entirely different animal. kenner was
responsible for the original recursive call that passed down
EXPAND_NORMAL, EXPAND_CONST_ADDRESS and EXPAND_INITIALIZER when expr.h
looked like:
14612 kennerEXPAND_MEMORY_USE_* are explained below. */
406 kenner enum expand_modifier {EXPAND_NORMAL, EXPAND_SUM,
14612 kenner EXPAND_CONST_ADDRESS, EXPAND_INITIALIZER,
14612 kenner EXPAND_MEMORY_USE_WO,
EXPAND_MEMORY_USE_RW,
14612 kenner EXPAND_MEMORY_USE_BAD,
EXPAND_MEMORY_USE_DONT};
406 kenner
14612 kenner /* Argument for chkr_* functions.
14612 kennerMEMORY_USE_RO: the pointer reads memory.
14612 kennerMEMORY_USE_WO: the pointer writes to memory.
14612 kennerMEMORY_USE_RW: the pointer modifies memory (ie it reads
and writes). An
14612 kenner example is (*ptr)++
14612 kennerMEMORY_USE_BAD: use this if you don't know the behavior of
the pointer, or
14612 kennerif you know there are no pointers. Using
an INDIRECT_REF
14612 kennerwith MEMORY_USE_BAD will abort.
14612 kennerMEMORY_USE_TW: just test for writing, without update.
Special.
14612 kennerMEMORY_USE_DONT: the memory is neither read nor written.
This is used by
14612 kenner '->' and '.'. */
So I think passing EXPAND_MEMORY and EXPAND_WRITE down ought to be
good. The VIEW_CONVERT_EXPR case really doesn't need to exclude
EXPAND_SUM either, since it doesn't allow an offset, but I made it the
same as the inner ref case so that when/if someone attends to
/* ??? We should work harder and deal with non-zero offsets. */
it doesn't cause a surprise. Really, a lot of this code needs
attention, for example, I think SSA made EXPAND_STACK_PARM and all of
the related code in calls.c dealing with libcalls when expanding args,
unnecessary.
Bootstrapped and regression tested powerpc64-linux. OK to apply?
PR middle-end/57134
* expr.c (expand_expr_real_1 ): Pass
EXPAND_MEMORY and EXPAND_WRITE to recursive call. Don't use
bitfield expansion when EXPAND_MEMORY.
(expand_expr_real_1 ): Pass modifier likewise.
Index: gcc/expr.c
===
--- gcc/expr.c (revision 199940)
+++ gcc/expr.c (working copy)
@@ -9918,12 +9919,8 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
&& modifier != EXPAND_STACK_PARM
? target : NULL_RTX),
VOIDmode,
-(modifier == EXPAND_INITIALIZER
- || modifier == EXPAND_CONST_ADDRESS
- || modifier == EXPAND_STACK_PARM)
-? modifier : EXPAND_NORMAL);
+modifier == EXPAND_SUM ? EXPAND_NORMAL : modifier);
-
/* If the bitfield is volatile, we want to access it in the
field's mode, not the computed mode.
If a MEM has VOIDmode (external with incomplete type),
@@ -10081,6 +10078,7 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
|| (MEM_P (op0)
&& (MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (mode1)
|| (bitpos % GET_MODE_ALIGNMENT (mode1) != 0
+&& modifier != EXPAND_MEMORY
&& ((modifier == EXPAND_CONST_ADDRESS
|| modifier == EXPAND_INITIALIZER)
? STRICT_ALIGNMENT
@@ -10279,10 +10277,7 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
&& modifier != EXPAND_STACK_PARM
? target : NULL_RTX),
VOIDmode,
-(modifier == EXPAND_INITIALIZER
- || modifier == EXPAND_CONST_ADDRESS
- || modifier == E
Re: [PATCH, rs6000] power8 patches, patch #7, quad/byte/half-word atomic instructions
I needed to rework the sync.md so that it would work correctly with no
optimization (using SUBREG's at -O0 did not give us the even registers for
holding PTImode values, so I created a PTImode temporary in load_lockedti and
store_conditionalti, which is normally optimized out.
[gcc]
2013-06-11 Michael Meissner
Pat Haugen
Peter Bergner
* config/rs6000/rs6000.c (emit_load_locked): Add support for
power8 byte, half-word, and quad-word atomic instructions.
(emit_store_conditional): Likewise.
(rs6000_expand_atomic_compare_and_swap): Likewise.
(rs6000_expand_atomic_op): Likewise.
* config/rs6000/sync.md (larx): Add new modes for power8.
(stcx): Likewise.
(AINT): New mode iterator to include TImode as well as normal
integer modes on power8.
(fetchop_pred): Use int_reg_operand instead of gpc_reg_operand so
that VSX registers are not considered. Use AINT mode iterator
instead of INT1 to allow inclusion of quad word atomic operations
on power8.
(load_locked): Likewise.
(store_conditional): Likewise.
(atomic_compare_and_swap): Likewise.
(atomic_exchange): Likewise.
(atomic_nand): Likewise.
(atomic_fetch_): Likewise.
(atomic_nand_fetch): Likewise.
(mem_thread_fence): Use gen_loadsync_ instead of enumerating
each type.
(ATOMIC): On power8, add QImode, HImode modes.
(load_locked_si): Varients of load_locked for QI/HI
modes that promote to SImode.
(load_lockedti): Convert TImode arguments to PTImode, so that we
get a guaranteed even/odd register pair.
(load_lockedpti): Likewise.
(store_conditionalti): Likewise.
(store_conditionalpti): Likewise.
* config/rs6000/rs6000.md (QHI): New mode iterator for power8
atomic load/store instructions.
(HSI): Likewise.
[gcc/testsuite]
2013-06-11 Michael Meissner
Pat Haugen
Peter Bergner
* gcc.target/powerpc/atomic-p7.c: New file, add tests for atomic
load/store instructions on power7, power8.
* gcc.target/powerpc/atomic-p8.c: Likewise.
Given these changes went beyond the original request to fix a spelling error
and improve the logic, I figured to send these patches out again. David, do
you have any problem with the new patches?
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 199965)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -17748,7 +17748,8 @@ emit_unlikely_jump (rtx cond, rtx label)
}
/* A subroutine of the atomic operation splitters. Emit a load-locked
- instruction in MODE. */
+ instruction in MODE. For QI/HImode, possibly use a pattern than includes
+ the zero_extend operation. */
static void
emit_load_locked (enum machine_mode mode, rtx reg, rtx mem)
@@ -17757,12 +17758,26 @@ emit_load_locked (enum machine_mode mode
switch (mode)
{
+case QImode:
+ fn = gen_load_lockedqi;
+ break;
+case HImode:
+ fn = gen_load_lockedhi;
+ break;
case SImode:
- fn = gen_load_lockedsi;
+ if (GET_MODE (mem) == QImode)
+ fn = gen_load_lockedqi_si;
+ else if (GET_MODE (mem) == HImode)
+ fn = gen_load_lockedhi_si;
+ else
+ fn = gen_load_lockedsi;
break;
case DImode:
fn = gen_load_lockeddi;
break;
+case TImode:
+ fn = gen_load_lockedti;
+ break;
default:
gcc_unreachable ();
}
@@ -17779,12 +17794,21 @@ emit_store_conditional (enum machine_mod
switch (mode)
{
+case QImode:
+ fn = gen_store_conditionalqi;
+ break;
+case HImode:
+ fn = gen_store_conditionalhi;
+ break;
case SImode:
fn = gen_store_conditionalsi;
break;
case DImode:
fn = gen_store_conditionaldi;
break;
+case TImode:
+ fn = gen_store_conditionalti;
+ break;
default:
gcc_unreachable ();
}
@@ -17931,7 +17955,7 @@ rs6000_expand_atomic_compare_and_swap (r
{
rtx boolval, retval, mem, oldval, newval, cond;
rtx label1, label2, x, mask, shift;
- enum machine_mode mode;
+ enum machine_mode mode, orig_mode;
enum memmodel mod_s, mod_f;
bool is_weak;
@@ -17943,22 +17967,29 @@ rs6000_expand_atomic_compare_and_swap (r
is_weak = (INTVAL (operands[5]) != 0);
mod_s = (enum memmodel) INTVAL (operands[6]);
mod_f = (enum memmodel) INTVAL (operands[7]);
- mode = GET_MODE (mem);
+ orig_mode = mode = GET_MODE (mem);
mask = shift = NULL_RTX;
if (mode == QImode || mode == HImode)
{
- mem = rs6000_adjust_atomic_subword (mem, &shift, &mask);
-
- /* Shift and mask OLDVAL
Re: [PATCH, committed] PR 57589: Fix logic error in driver-rs6000.c
On Wed, Jun 12, 2013 at 01:34:59AM +0200, Jakub Jelinek wrote: > On Tue, Jun 11, 2013 at 07:25:36PM -0400, Michael Meissner wrote: > > I committed the following patch as obvious, after making sure it builds and > > has > > no regressions. I was looking at driver-rs6000.c today, and I noticed that > > it > > was returning an address to an auto buffer for processing -mcpu=native. > > Does it? If I understand it right, the string itself isn't read into buf > (after all, what would translate the pointers in buf to point into other > parts of buf), /proc/self/auxv just contains the pairs of AT_* and the > corresponding values, for AT_PLATFORM that value is a char * pointing > somewhere into stack memory. Ah, you are correct. I misread the code, thinking it was returning a string, and not a pointer to the string in memory. I'll remove it tomorrow, and put in a comment for the next person. Sigh -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH, committed] PR 57589: Fix logic error in driver-rs6000.c
On Tue, Jun 11, 2013 at 07:25:36PM -0400, Michael Meissner wrote:
> I committed the following patch as obvious, after making sure it builds and
> has
> no regressions. I was looking at driver-rs6000.c today, and I noticed that it
> was returning an address to an auto buffer for processing -mcpu=native.
Does it? If I understand it right, the string itself isn't read into buf
(after all, what would translate the pointers in buf to point into other
parts of buf), /proc/self/auxv just contains the pairs of AT_* and the
corresponding values, for AT_PLATFORM that value is a char * pointing
somewhere into stack memory.
> 2013-06-11 Michael Meissner
>
> PR target/57589
> * config/rs6000/driver-rs6000.c (elf_platform): Make buffer static
> to allow returning address to AT_PLATFORM name.
>
> Index: gcc/config/rs6000/driver-rs6000.c
> ===
> --- gcc/config/rs6000/driver-rs6000.c (revision 199965)
> +++ gcc/config/rs6000/driver-rs6000.c (working copy)
> @@ -167,7 +167,7 @@ elf_platform (void)
>
>if (fd != -1)
> {
> - char buf[1024];
> + static char buf[1024];
>ElfW(auxv_t) *av;
>ssize_t n;
Jakub
[PATCH, committed] PR 57589: Fix logic error in driver-rs6000.c
I committed the following patch as obvious, after making sure it builds and has
no regressions. I was looking at driver-rs6000.c today, and I noticed that it
was returning an address to an auto buffer for processing -mcpu=native.
2013-06-11 Michael Meissner
PR target/57589
* config/rs6000/driver-rs6000.c (elf_platform): Make buffer static
to allow returning address to AT_PLATFORM name.
Index: gcc/config/rs6000/driver-rs6000.c
===
--- gcc/config/rs6000/driver-rs6000.c (revision 199965)
+++ gcc/config/rs6000/driver-rs6000.c (working copy)
@@ -167,7 +167,7 @@ elf_platform (void)
if (fd != -1)
{
- char buf[1024];
+ static char buf[1024];
ElfW(auxv_t) *av;
ssize_t n;
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Aw: Re: [PATCH] Basic support for MIPS r5900
Hello Richard,
> > How much other changes will be currently accepted here? There is other
> > stuff which I want to prepare and submit here, e.g.:
> > 1. disable use of dmult and ddiv (ABI n32).
> > 2. use trunc.w.s instead of cvt.w.s (to get single float working for
> > normal range calculations; i.e. calculating without inf or nan).
> > 3. fix use of ll/sc in libgomp, either increase mips ISA level or use
> > syscall (which is broken in Linux 2.6.35.4).
> > 4. fix libgcc to build a real muldi3 function for ABI n32 (not the
> > multi3 function which is stored in muldi3.o file).
> > 5. add support for configure parameters --float=single and
> > --float=double in addition to --float=soft and --float=hard.
> > 6. rework floating point to support single float with ABI n32 (either
> > break the ABI or store floating point values in general purpose
> > registers like soft float).
> > 7. change libgcc or mips.md in way so that the non IEEE 754 compatible
> > FPU of the r5900 gets compatible.
>
> Well, I'm afraid that's hard to say in advance. It really depends
> on what the changes look like. (1) and (2) sound harmless enough,
> although (1) should probably only be done in conjunction with (4).
> I'm not sure what (3) involves. (5) sounds like a good idea.
> (6) is worth doing, but anything ABI-related gets extra-paranoid
> treatment. :-)
The attached patch fixes (1) and (4). This makes mips64r5900el usable with
r5900. If (4) is a problem (i.e. patching libgcc/Makefile.in), it would be good
if at least (1) is accepted.
The patch for mips.md after line 1992 (adds TARGET_64BIT) is a more general
fix. This is not needed for r5900 support, but I think this should be fixed.
The same applies for patch after 2233 (adds ISA_HAS_DMULT). The fix here would
be also adding TARGET_64BIT, but for r5900 we need ISA_HAS_DMULT here. Other
changes in mips.md should not change the behaviour of GCC on non r5900
toolchains.
Best regards
JürgenIndex: gcc/config/mips/mips.h
===
--- gcc/config/mips/mips.h (Revision 199708)
+++ gcc/config/mips/mips.h (Arbeitskopie)
@@ -807,6 +807,7 @@
#define ISA_HAS_MUL3 ((TARGET_MIPS3900 \
|| TARGET_MIPS5400 \
|| TARGET_MIPS5500 \
+ || TARGET_MIPS5900 \
|| TARGET_MIPS7000 \
|| TARGET_MIPS9000 \
|| TARGET_MAD\
@@ -819,8 +820,28 @@
/* ISA has a three-operand multiplication instruction. */
#define ISA_HAS_DMUL3 (TARGET_64BIT\
&& TARGET_OCTEON \
+ && !TARGET_MIPS5900\
&& !TARGET_MIPS16)
+/* ISA supports instructions dmult and dmultu. */
+#define ISA_HAS_DMULT (TARGET_64BIT\
+ && !TARGET_MIPS5900)
+
+/* ISA supports instructions mult and multu.
+ This always supported, but the macro is needed for ISA_HAS_MULT
+ in mips.md. */
+#define ISA_HAS_MULT (1)
+
+/* ISA supports instructions ddiv and ddivu. */
+#define ISA_HAS_DDIV(TARGET_64BIT\
+ && !TARGET_MIPS5900)
+
+/* ISA supports instructions div and divu.
+ This always supported, but the macro is needed for ISA_HAS_DIV
+ in mips.md. */
+#define ISA_HAS_DIV (1)
+
+
#define ISA_HAS_DIV3 ((TARGET_LOONGSON_2EF \
|| TARGET_LOONGSON_3A) \
&& !TARGET_MIPS16)
Index: gcc/config/mips/mips.md
===
--- gcc/config/mips/mips.md (Revision 199708)
+++ gcc/config/mips/mips.md (Arbeitskopie)
@@ -1481,7 +1481,7 @@
[(set (match_operand:GPR 0 "register_operand")
(mult:GPR (match_operand:GPR 1 "register_operand")
(match_operand:GPR 2 "register_operand")))]
- ""
+ "ISA_HAS_MULT"
{
rtx lo;
@@ -1527,7 +1527,7 @@
{
if (which_alternative == 1)
return "mult\t%1,%2";
- if (mode == SImode && TARGET_MIPS3900)
+ if (mode == SImode && (TARGET_MIPS3900 || TARGET_MIPS5900))
return "mult\t%0,%1,%2";
return "mul\t%0,%1,%2";
}
@@ -1561,7 +1561,7 @@
[(set (match_operand:GPR 0 "muldiv_target_operand" "=l")
(mult:GPR (match_operand:GPR 1 "register_operand" "d")
(match_operand:GPR 2 "register_operand" "d")))]
- "!TARGET_FIX_R4000"
+ "ISA_HAS_MULT && !TARGET_FIX_R4000"
"mult\t%1,%2"
[(set_attr "type" "imul")
(set_attr "mode" "")])
@@ -1571,7 +1571,7 @@
(mult:GPR (match_operand:GPR 1 "register_operand" "d")
(match_operand:GPR 2 "register_operand" "d")))
(clobber (match_scratch:GPR 3 "=l"))]
- "TARGET_FIX_R4000"
+ "ISA_HAS_MULT && TARGET_FIX_R4000"
"mult\t%1,%2\;mflo\t%0"
[(set_attr "type" "imul")
(set_attr "mode" "")
@@ -1992,7 +1992,7 @@
(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
(any_extend:DI (match_operand:SI 2 "register_operand"
(clobber (match_operand:DI 3 "register_operand"))]
- ""
+ "TARGET_64BIT"
{
rtx hilo;
@@ -2038,7 +2038,7 @@
(mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "d"))
(sign_extend:DI (match_operand:SI 2
Symtab cleanups 5/17
Hi,
weakref is a static variable that is an alias of external symbol. Either by GCC
or by GAS it is translated to references to the target and depending on whether
target is used or not it is declared as weak or non-weak symbol.
Now the implementation in c-common.c sets DECL_EXTERNAL flag when building
weakref of variable, while it it keeps original flags (static) when building
alias of function.
While making cgraph implementation of weakref I run into problem with this
inconsistency and fixed it in cgraphunit.c by making all weakrefs external.
This is because dropping the external flag confused fold-const and made us
to miscompile attr-weakref testcase.
This is a lie to compiler - the symbol in question will never be defined by any
external source and it is purely local alias. This turned out to cause more
harm than benefits - at least I believe so. I had to hack around in visibility
code (faking weakrefs to be externally-visible), in unreachable code removal,
in LTO partitioning, in static var renaming, in symbol table.
Moreover sometimes this is done by testing DECL_EXTERNAL flag that I hoped to
be safe test for weakref alias. It is not. C++ FE produces normal external
aliases for same body/extra name machinery. Moreover because aliases used to be
extern at some point (again for wrong reason), the #pragma creates
DECL_EXTERNAL weak aliases in some scenarios, but not in others.
This patch fixes it by making weakrefs to appear local. I kept DECL_WEAK
flag on it that can be used by fold-const to avoid overzealous folding.
I added weakref flag to symtab node and commonized the code special casing
of weakrefs (a lot of it can go away).
I plan to followup with patch cleaning up varasm.c weak/weakref output logic
to be driven by symbol table.
It may be interesting to completely resolve weakref semantic before LTO
streaming
time; it is possible if we was able to rewrite weakref uses by the target decl,
but we lack support for it and also weakref symbls may be actualy used by an asm
statements.
Anyway, this patch should solve good part of the problems we are runing into.
Bootstrapped/regtested ppc64-linux with and without weakref support
and x86_64-linux (in earlier version of the patch), comitted.
Honza
* c-family/c-common.c (handle_alias_ifunc_attribute): Do not set
DECL_EXTERNAL for weakref variables.
* c-family/c-pragma.c (handle_pragma_weak): Make sure aliases
are not declared as external.
* cgraph.c (cgraph_create_function_alias): Set weakref flag.
* cgraph.h (symtab_node_base): Add weakref flag.
* cgraphunit.c (cgraph_reset_node): Clear weakref flag.
(handle_alias_pairs): Set weakref flag, do not set DECL_EXTERNAL.
(output_weakrefs): Use weakref flag.
* fold-const.c (simple_operand_p): Handle WEAK.
* gimple-fold.c (can_refer_decl_in_current_unit_p): Drop weakref.
* ipa.c (varpool_externally_visible_p): Drop weakref.
(function_and_variable_visibility): Update comment; fix weakref
sanity checks; do not clear DECL_WEAK on them.
* lto-cgraph.c (lto_output_node): update.
(lto_output_varpool_node): Update.
(input_overwrite_node): Update.
(input_node): Update.
(input_varpool_node): Update.
* lto-symtab.c (lto_symtab_symbol_p): Do not special case weakrefs.
(lto_symtab_merge_symbols): Add sanity check.
(lto_symtab_prevailing_decl): Do not special case weakrefs.
* passes.c (rest_of_decl_compilation): Set static flag, too.
* symtab.c (dump_symtab_base): Dump weakref.
(verify_symtab_base): Sanity check weakrefs.
(symtab_make_decl_local): Remove duplicated code.
(symtab_alias_ultimate_target): Simplify.
* varpool.c (varpool_create_variable_alias): Set weakref flag.
* lto-partition.c (get_symbol_class): Simplify weakref handling.
(add_symbol_to_partition_1): Likewise.
(contained_in_symbol): Likewise.
(lto_balanced_map): Likewise.
(rename_statics): Drop weakref.
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 199959)
+++ c-family/c-common.c (working copy)
@@ -7576,13 +7576,7 @@ handle_alias_ifunc_attribute (bool is_al
if (TREE_CODE (decl) == FUNCTION_DECL)
DECL_INITIAL (decl) = error_mark_node;
else
- {
- if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
- DECL_EXTERNAL (decl) = 1;
- else
- DECL_EXTERNAL (decl) = 0;
- TREE_STATIC (decl) = 1;
- }
+ TREE_STATIC (decl) = 1;
if (!is_alias)
/* ifuncs are also aliases, so set that attribute too. */
Index: c-family/c-pragma.c
===
--- c-family/c-pragma.c (revision 199959)
+++ c-family/c-pragma.c (working copy)
@@ -367,7 +367,12 @@ handle_pragma
[google/gcc-4_8] Backport upstream patch to fix non-deterministic order of DWARF output
This patch fixes a testsuite failure when -fdebug-types-section is
turned on, where the order of type stub DIEs is dependent on hash
table traversal order.
Backported from trunk at r199927. Bootstrapped, tested, and committed.
-cary
2013-06-10 Cary Coutant
gcc/
* dwarf2out.c (hash_external_ref): Use die_symbol or signature
for hash so that hash table traversal order is deterministic.
Index: dwarf2out.c
===
--- dwarf2out.c (revision 199902)
+++ dwarf2out.c (working copy)
@@ -7405,7 +7405,17 @@ static hashval_t
hash_external_ref (const void *p)
{
const struct external_ref *r = (const struct external_ref *)p;
- return htab_hash_pointer (r->type);
+ dw_die_ref die = r->type;
+ hashval_t h = 0;
+
+ if (! die->comdat_type_p)
+h = htab_hash_string (die->die_id.die_symbol);
+ else
+{
+ comdat_type_node_ref type_node = die->die_id.die_type_node;
+ memcpy (&h, type_node->signature, sizeof (h));
+}
+ return h;
}
/* Compare external_refs. */
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 10:44:12PM +0200, Jakub Jelinek wrote: > There is another thing to solve BTW, op0 and/or op1 might have side-effects, > if you are going to evaluate them more than once, they need to be surrounded > into cp_save_expr resp. c_save_expr. I see. Thanks for the notice. Marek
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 10:40:12PM +0200, Marek Polacek wrote:
> On Tue, Jun 11, 2013 at 10:33:25PM +0200, Jakub Jelinek wrote:
> > That means you probably should move the function call down in
> > cp_build_binary_op (resp. C counterpart), after the arguments are converted
> > to result_type?
>
> Ok, certainly. Seems the arguments are converted here:
>
> if (! converted)
> {
> if (TREE_TYPE (op0) != result_type)
> op0 = cp_convert_and_check (result_type, op0, complain);
> if (TREE_TYPE (op1) != result_type)
> op1 = cp_convert_and_check (result_type, op1, complain);
>
> if (op0 == error_mark_node || op1 == error_mark_node)
> return error_mark_node;
> }
>
> I'll move the instrumentation after the hunk above. And then
> in ubsan_instrument_division I might want to have just
> tree type = TREE_TYPE (op0);, maybe together with an assert like
> gcc_assert (TREE_TYPE (op0) == TREE_TYPE (op1).
There is another thing to solve BTW, op0 and/or op1 might have side-effects,
if you are going to evaluate them more than once, they need to be surrounded
into cp_save_expr resp. c_save_expr.
Jakub
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 10:33:25PM +0200, Jakub Jelinek wrote:
> That means you probably should move the function call down in
> cp_build_binary_op (resp. C counterpart), after the arguments are converted
> to result_type?
Ok, certainly. Seems the arguments are converted here:
if (! converted)
{
if (TREE_TYPE (op0) != result_type)
op0 = cp_convert_and_check (result_type, op0, complain);
if (TREE_TYPE (op1) != result_type)
op1 = cp_convert_and_check (result_type, op1, complain);
if (op0 == error_mark_node || op1 == error_mark_node)
return error_mark_node;
}
I'll move the instrumentation after the hunk above. And then
in ubsan_instrument_division I might want to have just
tree type = TREE_TYPE (op0);, maybe together with an assert like
gcc_assert (TREE_TYPE (op0) == TREE_TYPE (op1).
Marek
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 10:20:24PM +0200, Marek Polacek wrote: > On Tue, Jun 11, 2013 at 10:09:00PM +0200, Jakub Jelinek wrote: > > On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote: > > > > >+ tree type0 = TREE_TYPE (op0); > > > > >+ tree type1 = TREE_TYPE (op1); > > > > > > > > Can the 2 types be different? I thought divisions had homogeneous > > > > arguments, and the instrumentation was done late enough to avoid any > > > > potential issue, but maybe not... > > > > > > Yeah, they can; they only have to be of arithmetic type. > > > > Nope, if this is after conversion to result_type (resp. orig_type), > > then they both have result_type resp. orig_type type. > > > > Shift is different, there the two arguments can have different type. > > But currently I'm cp_convert-ing the arguments to orig_type only if > we were performing the shortening which changed the result_type. > If, with current patch, I put debug_tree (type0); debug_tree (type1); > into ubsan_instrument_division, I see different types (int vs. > unsigned int etc.). That means you probably should move the function call down in cp_build_binary_op (resp. C counterpart), after the arguments are converted to result_type? Jakub
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 10:09:00PM +0200, Jakub Jelinek wrote: > On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote: > > > >+ tree type0 = TREE_TYPE (op0); > > > >+ tree type1 = TREE_TYPE (op1); > > > > > > Can the 2 types be different? I thought divisions had homogeneous > > > arguments, and the instrumentation was done late enough to avoid any > > > potential issue, but maybe not... > > > > Yeah, they can; they only have to be of arithmetic type. > > Nope, if this is after conversion to result_type (resp. orig_type), > then they both have result_type resp. orig_type type. > > Shift is different, there the two arguments can have different type. But currently I'm cp_convert-ing the arguments to orig_type only if we were performing the shortening which changed the result_type. If, with current patch, I put debug_tree (type0); debug_tree (type1); into ubsan_instrument_division, I see different types (int vs. unsigned int etc.). Marek
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote: > > >+ tree type0 = TREE_TYPE (op0); > > >+ tree type1 = TREE_TYPE (op1); > > > > Can the 2 types be different? I thought divisions had homogeneous > > arguments, and the instrumentation was done late enough to avoid any > > potential issue, but maybe not... > > Yeah, they can; they only have to be of arithmetic type. Nope, if this is after conversion to result_type (resp. orig_type), then they both have result_type resp. orig_type type. Shift is different, there the two arguments can have different type. Jakub
[C++ Patch] PR 42021
Hi,
this issue, which originally was about spelling out in the error message
the ambiguous candidates, is by and large fixed. Only, we produce the
whole diagnostic twice for the testcase at issue: first in
cp_parser_lookup_name and then again in
cp_parser_nested_name_specifier_opt. The below fixes that.
Tested x86_64-linux.
Thanks,
Paolo.
///
2013-06-11 Paolo Carlini
PR c++/42021
* parser.c (cp_parser_nested_name_specifier_opt): Avoid emitting
again diagnostic already emitted by cp_parser_lookup_name.
Index: parser.c
===
--- parser.c(revision 199944)
+++ parser.c(working copy)
@@ -5230,10 +5230,14 @@ cp_parser_nested_name_specifier_opt (cp_parser *pa
decl);
else if (ambiguous_decls)
{
- error_at (token->location,
- "reference to %qD is ambiguous",
- token->u.value);
- print_candidates (ambiguous_decls);
+ if (cp_parser_uncommitted_to_tentative_parse_p
+ (parser))
+ {
+ error_at (token->location,
+ "reference to %qD is ambiguous",
+ token->u.value);
+ print_candidates (ambiguous_decls);
+ }
decl = error_mark_node;
}
else
Re: Remove self-assignments
On Tue, Jun 11, 2013 at 09:30:29PM +0200, Marc Glisse wrote:
> >I'd be curious how often this triggers in GCC itself as well.
>
> Do you know a convenient way to test that?
Perhaps you could put in the
if (gimple_assign_rhs_code (stmt) == TREE_CODE (gimple_assign_lhs (stmt))
&& operand_equal_p (gimple_assign_rhs1 (stmt),
gimple_assign_lhs (stmt), 0))
{
...
}
something like
FILE *f = fopen ("/tmp/self", "a");
fprintf (f, "%s ", main_input_filename);
print_gimple_stmt (f, stmt, 0, TDF_VOPS|TDF_MEMSYMS);
fclose (f);
(completely untested)
Marek
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
Hi!
On Tue, Jun 11, 2013 at 09:14:36PM +0200, Marc Glisse wrote:
> Hello,
>
> couple comments (not a true review)
Thanks anyway ;).
> On Tue, 11 Jun 2013, Marek Polacek wrote:
>
> >+tree
> >+ubsan_instrument_division (location_t loc, tree op0, tree op1)
> >+{
> >+ tree t, tt;
> >+ tree type0 = TREE_TYPE (op0);
> >+ tree type1 = TREE_TYPE (op1);
>
> Can the 2 types be different? I thought divisions had homogeneous
> arguments, and the instrumentation was done late enough to avoid any
> potential issue, but maybe not...
Yeah, they can; they only have to be of arithmetic type.
> >+ tree type1_zero_cst = build_int_cst (type1, 0);
>
> It is a bit funny to do that before the following test ;-)
Perhaps, yes. Moved it...
> >+ if (TREE_CODE (type0) != INTEGER_TYPE
> >+ || TREE_CODE (type1) != INTEGER_TYPE)
> >+return NULL_TREE;
..here.
> >+ /* We check INT_MIN / -1 only for signed types. */
> >+ if (!TYPE_UNSIGNED (type0) && !TYPE_UNSIGNED (type1))
> >+{
> >+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
> >+build_int_cst (type1, -1));
> >+ t = fold_build2 (EQ_EXPR, boolean_type_node, op0,
> >+ TYPE_MIN_VALUE (type0));
> >+ t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt);
> >+}
> >+ else
> >+t = type1_zero_cst;
> >+ tt = fold_build2 (EQ_EXPR, boolean_type_node,
> >+op1, type1_zero_cst);
> >+ t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, tt, t);
>
> If you wrote the comparison with 0 first, you could put the OR in
> the signed branch instead of relying on folding |0, no?
Duh, indeed. Will adjust. Thanks!
Marek
Re: More forwprop for vectors
On Tue, 11 Jun 2013, Jeff Law wrote: On 06/09/13 13:43, Marc Glisse wrote: Hello, just adapting yet another function so it also works with vectors. It seemed convenient to add a new macro. The name sucks (it doesn't match the semantics of INTEGRAL_TYPE_P), but I didn't want to name it INTEGER_SCALAR_OR_VECTOR_CONSTANT_P and didn't have any good idea for a short name. I'd just use a long name. I can easily see someone getting easily not being aware that INTEGRAL_CST_P returns true for vectors and as a result doing something inappropriate. INTEGER_CST_OR_VECTOR_INTEGER_TYPE_P? Having TYPE in there seems confusing, and INTEGER_SCALAR_OR_VECTOR_CONSTANT_P is at least one character shorter ;-) Oh, you probably meant INTEGER_CST_OR_VECTOR_INTEGER_CST_P? Compacting could give INT_OR_VECINT_CST_P (or INTVEC instead of VECINT, I don't know which order sounds best). I don't really mind the name, so if you want INTEGER_CST_OR_VECTOR_INTEGER_CST_P that's ok with me. Thanks for the comments on the 2 patches, -- Marc Glisse
Re: Remove self-assignments
On Tue, 11 Jun 2013, Jeff Law wrote: On 06/09/13 10:25, Marc Glisse wrote: Hello, this patch removes some self-assignments. I don't know if this is the best way, but it passes a bootstrap and the testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse PR tree-optimization/57361 gcc/ * tree-ssa-dse.c (dse_possible_dead_store_p): Handle self-assignment. gcc/testsuite/ * gcc.dg/tree-ssa/pr57361.c: New file. So dse_optimize_stmt will verify the statement does not have volatile operands. operand_equal_p also does it, so we are well covered there ;-) However, it doesn't verify the statement does not potentially throw (think about a segfault on the store when async exceptions are enabled). I think you need to test for that explicitly. Hmm, I am not at all familiar with that. Google drowns me in C# and javascript links, and grepping through the sources only pointed me to the -fasynchronous-unwind-tables flag. Richard noticed in the PR that expand_assignment already does: /* Optimize away no-op moves without side-effects. */ if (operand_equal_p (to, from, 0)) return; so it looks like the operand_equal_p test should be sufficient (or the compiler already breaks that code). Is there some reason this won't work in tree-ssa-dce.c? That gets run more often and these stores may be preventing code motion opportunities, so getting them out of the IL stream as early as possible would be good. In the first version of the patch: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57303#c6 I was doing it in fold_stmt_1, which should be called often enough. Richard suggested DSE in the PR, but that might be because he had a more subtle test in mind. I am certainly ok with moving it to DCE or anywhere else... I'd be curious how often this triggers in GCC itself as well. Do you know a convenient way to test that? -- Marc Glisse
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
Hello,
couple comments (not a true review)
On Tue, 11 Jun 2013, Marek Polacek wrote:
+tree
+ubsan_instrument_division (location_t loc, tree op0, tree op1)
+{
+ tree t, tt;
+ tree type0 = TREE_TYPE (op0);
+ tree type1 = TREE_TYPE (op1);
Can the 2 types be different? I thought divisions had homogeneous
arguments, and the instrumentation was done late enough to avoid any
potential issue, but maybe not...
+ tree type1_zero_cst = build_int_cst (type1, 0);
It is a bit funny to do that before the following test ;-)
+ if (TREE_CODE (type0) != INTEGER_TYPE
+ || TREE_CODE (type1) != INTEGER_TYPE)
+return NULL_TREE;
+
+ /* If we *know* that the divisor is not -1 or 0, we don't have to
+ instrument this expression.
+ ??? We could use decl_constant_value to cover up more cases. */
+ if (TREE_CODE (op1) == INTEGER_CST
+ && integer_nonzerop (op1)
+ && !integer_minus_onep (op1))
+return NULL_TREE;
+
+ /* We check INT_MIN / -1 only for signed types. */
+ if (!TYPE_UNSIGNED (type0) && !TYPE_UNSIGNED (type1))
+{
+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
+ build_int_cst (type1, -1));
+ t = fold_build2 (EQ_EXPR, boolean_type_node, op0,
+ TYPE_MIN_VALUE (type0));
+ t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt);
+}
+ else
+t = type1_zero_cst;
+ tt = fold_build2 (EQ_EXPR, boolean_type_node,
+ op1, type1_zero_cst);
+ t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, tt, t);
If you wrote the comparison with 0 first, you could put the OR in the
signed branch instead of relying on folding |0, no?
+ tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW);
+ tt = build_call_expr_loc (loc, tt, 0);
+ t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
+
+ return t;
+}
--
Marc Glisse
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
> On Tue, Jun 11, 2013 at 08:39:07PM +0200, Jan Hubicka wrote: > > > @@ -6802,10 +6802,20 @@ decl_binds_to_current_def_p (tree decl) > > > return resolution_to_local_definition_p (node->symbol.resolution); > > > } > > >/* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks > > > - binds locally but still can be overwritten). > > > + binds locally but still can be overwritten), DECL_COMMON (can be > > > merged > > > + with a non-common definition somewhere in the same module) or > > > + DECL_EXTERNAL. > > > This rely on fact that binds_local_p behave as decl_replaceable_p > > > for all other declaration types. */ > > > - return !DECL_WEAK (decl); > > > + if (DECL_WEAK (decl)) > > > +return false; > > > + if (DECL_COMMON (decl) > > > + && (DECL_INITIAL (decl) == NULL > > > + || DECL_INITIAL (decl) == error_mark_node)) > > > +return false; > > > > As discussed on IRC, this will return unnecesarily conservative answer for > > HIDDEN visibility and (resolution == LDPR_PREVAILING_DEF_IRONLY or > > resolution == LDPR_PREVAILING_DEF) symbols. > > If resolution is not LDPR_UNKNOWN, then we don't enter this code at all. > In that case we simply check binds_local_p (it returns true for those), > !TREE_PUBLIC (false), and then just look at at the resolution (so > preexisting code). Ah, sorry, you are right. I overlooked that you kept the existing resolution code around. Except for the independent problem with default visibility on external symbols in default_binds_local_p_1 I do not see any other issue. I will send separate patch for that tomorrow. Thanks, Honza > > Jakub
Re: More forwprop for vectors
On 06/09/13 13:43, Marc Glisse wrote: Hello, just adapting yet another function so it also works with vectors. It seemed convenient to add a new macro. The name sucks (it doesn't match the semantics of INTEGRAL_TYPE_P), but I didn't want to name it INTEGER_SCALAR_OR_VECTOR_CONSTANT_P and didn't have any good idea for a short name. I'd just use a long name. I can easily see someone getting easily not being aware that INTEGRAL_CST_P returns true for vectors and as a result doing something inappropriate. INTEGER_CST_OR_VECTOR_INTEGER_TYPE_P? Jeff
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On Tue, Jun 11, 2013 at 08:39:07PM +0200, Jan Hubicka wrote: > > @@ -6802,10 +6802,20 @@ decl_binds_to_current_def_p (tree decl) > > return resolution_to_local_definition_p (node->symbol.resolution); > > } > >/* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks > > - binds locally but still can be overwritten). > > + binds locally but still can be overwritten), DECL_COMMON (can be > > merged > > + with a non-common definition somewhere in the same module) or > > + DECL_EXTERNAL. > > This rely on fact that binds_local_p behave as decl_replaceable_p > > for all other declaration types. */ > > - return !DECL_WEAK (decl); > > + if (DECL_WEAK (decl)) > > +return false; > > + if (DECL_COMMON (decl) > > + && (DECL_INITIAL (decl) == NULL > > + || DECL_INITIAL (decl) == error_mark_node)) > > +return false; > > As discussed on IRC, this will return unnecesarily conservative answer for > HIDDEN visibility and (resolution == LDPR_PREVAILING_DEF_IRONLY or resolution > == LDPR_PREVAILING_DEF) symbols. If resolution is not LDPR_UNKNOWN, then we don't enter this code at all. In that case we simply check binds_local_p (it returns true for those), !TREE_PUBLIC (false), and then just look at at the resolution (so preexisting code). Jakub
[rl78] define TARGET_UNWIND_WORD_MODE
Fixes a libstdc++v3 build failure. Committed.
* config/rl78/rl78.c (TARGET_UNWIND_WORD_MODE): Define.
(rl78_unwind_word_mode): New.
Index: config/rl78/rl78.c
===
--- config/rl78/rl78.c (revision 199963)
+++ config/rl78/rl78.c (working copy)
@@ -2736,9 +2736,19 @@ rl78_return_in_memory (const_tree type,
{
const HOST_WIDE_INT size = int_size_in_bytes (type);
return (size == -1 || size > 8);
}
+
+#undef TARGET_UNWIND_WORD_MODE
+#define TARGET_UNWIND_WORD_MODE rl78_unwind_word_mode
+
+static enum machine_mode
+rl78_unwind_word_mode (void)
+{
+ return HImode;
+}
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-rl78.h"
Re: [RFC] Implement Undefined Behavior Sanitizer (take 2)
On Sat, Jun 08, 2013 at 08:22:33PM +0200, Jakub Jelinek wrote:
> > >+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
> > >+ integer_minus_one_node);
> >
> > Don't we usually try to have both operands of a comparison of the
> > same type?
>
> Not just usually, it really has to be build_int_cst (TREE_TYPE (op1), -1).
> And, more importantly, at least in cp_build_binary_op the calls need to be
> moved further down in the function, at least after if
> (processing_template_decl)
> but e.g. for division the trouble is that shorten_binary_op is performed
> before actually promoting one or both operand to the result_type. I guess
> for the diagnostics which prints the types, it would be best to diagnose
> using the promoted types and result_type constructed out of that, but
> without shorten_binary_op etc., that is just an optimization I think.
> So, maybe record the original result_type before shortening, and if
> shortening changed that, convert the arguments for the instrumentation only
> to the original result_type, otherwise use the conversion done normally.
> For shifts this isn't a big deal, because they always use result_type of the
> first operand after promotion, and the ubsan handler wants to see two types
> there (the question is, does it want for the shift amount look for the
> original shift count type, or the one converted to int)?
>
> Also, perhaps it would be better if these ubsan_instrument* functions
> didn't return a COMPOUND_EXPR, but instead just the lhs of that (i.e. the
> actual instrumentation) and let the caller set some var to that and if that
> var is non-NULL, after building the binary operation build a COMPOUND_EXPR
> with lhs being the instrumentation and rhs the binary operation itself.
All should be resolved.
> > >+ t = fold_build2 (EQ_EXPR, boolean_type_node, op0,
> > >+ TYPE_MIN_VALUE (TREE_TYPE (op0)));
> >
> > I didn't see where this test was restricted to the signed case
> > (0u/-1 is well defined)?
> >
> > >+ t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt);
> > >+ tt = build2 (EQ_EXPR, boolean_type_node,
> > >+ op1, integer_zero_node);
> >
> > Why not fold this one?
>
> Sure. And yeah, the INT_MIN/-1 checking needs to be done for signed types
> only.
Done.
> > >+tree
> > >+ubsan_instrument_shift (location_t loc, enum tree_code code,
> > >+ tree op0, tree op1)
> > >+{
> > >+ tree t, tt = NULL_TREE;
> > >+ tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
> > >+ tree uprecm1 = build_int_cst (unsigned_type_for (TREE_TYPE (op1)),
> > >+ TYPE_PRECISION (TREE_TYPE (op0)) - 1);
> > >+ tree precm1 = build_int_cst (TREE_TYPE (op1),
> > >+ TYPE_PRECISION (TREE_TYPE (op0)) - 1);
> >
> > (if we later want to extend this to vector-scalar shifts,
> > element_precision will be better than TYPE_PRECISION)
> >
> > Name unsigned_type_for (TREE_TYPE (op1)) and TYPE_PRECISION
> > (TREE_TYPE (op0)) that are used several times?
Done.
Thanks for the review.
Here's another version, hopefully all issues are fixed. During
the rewriting I had to fix a few ICEs, so this patch took more time.
I guess I might've misunderstood the cp_convert part, so sorry if
I did it wrong.
Lightly tested, I'm really starting to miss the ubsan testsuite ;).
Regtested on x86_64-linux.
2013-06-11 Marek Polacek
* Makefile.in: Add ubsan.c.
* common.opt: Add -fsanitize=undefined option.
* doc/invoke.texi: Document the new flag.
* sanitizer.def (DEF_SANITIZER_BUILTIN): Define.
* builtin-attrs.def (ATTR_COLD): Define.
* asan.c (initialize_sanitizer_builtins): Build
BT_FN_VOID_PTR_PTR_PTR.
* builtins.def (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS): Define.
c-family/
* c-ubsan.c: New file.
* c-ubsan.h: New file.
cp/
* typeck.c (cp_build_binary_op): Add division by zero and shift
instrumentation.
c/
* c-typeck.c (build_binary_op): Add division by zero and shift
instrumentation.
--- gcc/c-family/c-ubsan.c.mp 2013-06-11 19:51:55.555492466 +0200
+++ gcc/c-family/c-ubsan.c 2013-06-11 19:29:16.925551907 +0200
@@ -0,0 +1,126 @@
+/* UndefinedBehaviorSanitizer, undefined behavior detector.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+ Contributed by Marek Polacek
+
+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
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
> 2013-06-11 Jakub Jelinek
>
> PR target/56564
> * varasm.c (decl_binds_to_current_def_p): Call binds_local_p
> target hook even for !TREE_PUBLIC decls. If no resolution info
> is available, return false for common and external decls.
>
> --- gcc/varasm.c.jj 2013-06-11 20:11:34.0 +0200
> +++ gcc/varasm.c 2013-06-11 20:15:15.729363893 +0200
> @@ -6781,10 +6781,10 @@ bool
> decl_binds_to_current_def_p (tree decl)
> {
>gcc_assert (DECL_P (decl));
> - if (!TREE_PUBLIC (decl))
> -return true;
>if (!targetm.binds_local_p (decl))
> return false;
> + if (!TREE_PUBLIC (decl))
> +return true;
>/* When resolution is available, just use it. */
>if (TREE_CODE (decl) == VAR_DECL
>&& (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
> @@ -6802,10 +6802,20 @@ decl_binds_to_current_def_p (tree decl)
> return resolution_to_local_definition_p (node->symbol.resolution);
> }
>/* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks
> - binds locally but still can be overwritten).
> + binds locally but still can be overwritten), DECL_COMMON (can be merged
> + with a non-common definition somewhere in the same module) or
> + DECL_EXTERNAL.
> This rely on fact that binds_local_p behave as decl_replaceable_p
> for all other declaration types. */
> - return !DECL_WEAK (decl);
> + if (DECL_WEAK (decl))
> +return false;
> + if (DECL_COMMON (decl)
> + && (DECL_INITIAL (decl) == NULL
> + || DECL_INITIAL (decl) == error_mark_node))
> +return false;
As discussed on IRC, this will return unnecesarily conservative answer for
HIDDEN visibility and (resolution == LDPR_PREVAILING_DEF_IRONLY or resolution
== LDPR_PREVAILING_DEF) symbols.
I belive in these cases for both returns above can be true. here we know
symbol binds locally from linker.
We however can not trust resolution == LDPR_PREVAILING_DEF completely because
DEFAULT visibility symbols should still return false for shlib.
One case to handle it would bo whole_program_visibility to look for this case
and remove
DECL_WEAK, DECL_COMMON and COMDAT flags. It already does that for
LDPR_PREVAILING_DEF_IRONLY
via make_decl_local, but I am not quite convinced we can safely do it for
COMDATs that are
resolution == LDPR_PREVAILING_DEF, since it will make the symbol to change its
type for
linker. Linker API specification do not speak about possibility of such changes.
While looking into the details, I also convinced myself that the following test:
/* Variables defined outside this object might not be local. */
else if (DECL_EXTERNAL (exp) && !resolved_locally)
local_p = false;
/* If defined in this object and visibility is not default, must be
local. */
else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
local_p = true;
should really stand
/* Variables defined outside this object might not be local. */
else if (DECL_EXTERNAL (exp) && !resolved_locally)
local_p = false;
/* If defined in this object and visibility is not default, must be
local. */
else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT && !DECL_EXTERNAL (exp))
local_p = true;
We do not know if the defining unit is defining the symbol with hidden
visibility of not.
Perhaps linker can tell us via the resolution info, too?
At the moment I can not think of other problems here.
Honza
Re: Remove self-assignments
On 06/09/13 10:25, Marc Glisse wrote: Hello, this patch removes some self-assignments. I don't know if this is the best way, but it passes a bootstrap and the testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse PR tree-optimization/57361 gcc/ * tree-ssa-dse.c (dse_possible_dead_store_p): Handle self-assignment. gcc/testsuite/ * gcc.dg/tree-ssa/pr57361.c: New file. So dse_optimize_stmt will verify the statement does not have volatile operands. However, it doesn't verify the statement does not potentially throw (think about a segfault on the store when async exceptions are enabled). I think you need to test for that explicitly. Is there some reason this won't work in tree-ssa-dce.c? That gets run more often and these stores may be preventing code motion opportunities, so getting them out of the IL stream as early as possible would be good. I'd be curious how often this triggers in GCC itself as well. jeff
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On 06/11/2013 11:20 AM, Jakub Jelinek wrote: > 2013-06-11 Jakub Jelinek > > PR target/56564 > * varasm.c (decl_binds_to_current_def_p): Call binds_local_p > target hook even for !TREE_PUBLIC decls. If no resolution info > is available, return false for common and external decls. Looks good. Ok if it passes testing. r~
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On Tue, Jun 11, 2013 at 10:18:17AM -0700, Richard Henderson wrote:
> > 2) DECL_COMMON with non-default visibility - the old version would return
> >true, now it returns false unless linker plugin tells us the current
> >common was used
> >
> > Or do you think we should just do what we did before and just
> > handle the 1) and 2) cases explicitly in the decl_binds_to_current_def_p
> > function?
>
> This (2) sounds like something that we could handle in d_b_t_c_d_p without
> having to duplicate all of binds_local_p.
Ok, here are all the changes to d_b_t_c_d_p. Any holes in this? Untested
so far.
2013-06-11 Jakub Jelinek
PR target/56564
* varasm.c (decl_binds_to_current_def_p): Call binds_local_p
target hook even for !TREE_PUBLIC decls. If no resolution info
is available, return false for common and external decls.
--- gcc/varasm.c.jj 2013-06-11 20:11:34.0 +0200
+++ gcc/varasm.c2013-06-11 20:15:15.729363893 +0200
@@ -6781,10 +6781,10 @@ bool
decl_binds_to_current_def_p (tree decl)
{
gcc_assert (DECL_P (decl));
- if (!TREE_PUBLIC (decl))
-return true;
if (!targetm.binds_local_p (decl))
return false;
+ if (!TREE_PUBLIC (decl))
+return true;
/* When resolution is available, just use it. */
if (TREE_CODE (decl) == VAR_DECL
&& (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
@@ -6802,10 +6802,20 @@ decl_binds_to_current_def_p (tree decl)
return resolution_to_local_definition_p (node->symbol.resolution);
}
/* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks
- binds locally but still can be overwritten).
+ binds locally but still can be overwritten), DECL_COMMON (can be merged
+ with a non-common definition somewhere in the same module) or
+ DECL_EXTERNAL.
This rely on fact that binds_local_p behave as decl_replaceable_p
for all other declaration types. */
- return !DECL_WEAK (decl);
+ if (DECL_WEAK (decl))
+return false;
+ if (DECL_COMMON (decl)
+ && (DECL_INITIAL (decl) == NULL
+ || DECL_INITIAL (decl) == error_mark_node))
+return false;
+ if (DECL_EXTERNAL (decl))
+return false;
+ return true;
}
/* A replaceable function or variable is one which may be replaced
Jakub
Re: C++: fix ICE with CONST_DECLs
On 06/11/2013 12:39 PM, Paolo Carlini wrote: On 06/11/2013 06:27 PM, Mike Stump wrote: Here is a simple one. When processing CONST_DECLs after an error, we can ICE. This avoid the ICE. No testcase? Yep; the patch is fine, but needs a testcase. Jason
Re: [c++-concepts] code review
On 06/11/2013 11:09 AM, Andrew Sutton wrote: And you need to do this even when the call is type-dependent? Yes. Especially when the call is type-dependent. That's how I generate the constraint sets, which are used to determine the most constrained template during overload resolution. Ah, that makes sense. Then I guess what you're doing in resolve_constraint_check is what you want; since the function takes no parameters and returns bool, there's no substitution to do into the function type, so SFINAE doesn't matter. I'd just ask again to rename substitute_template_parameters; perhaps just calling it coerce_template_parms makes sense. And also please pass tmpl down to the is_decl parameter. Jason
Re: [patch, mips] Micromips delay slot fix
"Moore, Catherine" writes: > I'm testing a slightly different patch from the one that Steve posted: > > Index: mips.md > === > --- mips.md (revision 199648) > +++ mips.md (working copy) > @@ -703,8 +703,13 @@ > > ;; Is it a single instruction? > (define_attr "single_insn" "no,yes" > - (symbol_ref "(get_attr_length (insn) == (TARGET_MIPS16 ? 2 : 4) > - ? SINGLE_INSN_YES : SINGLE_INSN_NO)")) > + (if_then_else (ior (and (match_test "TARGET_MIPS16") > + (match_test "get_attr_length (insn) == 2")) > +(and (eq_attr "compression" "micromips,all") > + (match_test "TARGET_MICROMIPS")) > +(match_test "get_attr_length (insn) == 4")) > + (const_string "yes") > + (const_string "no"))) 4 isn't OK for MIPS16 though. There's also the problem that Maciej pointed out: a length of 4 doesn't imply a single insn on microMIPS. E.g. an unsplit doubleword move to or from the accumulator registers is a pair of 2-byte microMIPS instructions, so although its overall length is 4, it isn't a single insn. The original code has the same problem. In practice, the split should have happened by dbr_schedule time, but it seems bad practice to rely on that. (FWIW, the MIPS16 definition comes from the historical attitude that extended instructions count as 2 instructions. The *_insns functions also follow this counting.) I'm going to try redefining the length attribute after: ;; "Ghost" instructions occupy no space. (eq_attr "type" "ghost") (const_int 0) in terms of an "insn_count" attribute. This will conervatively count 4 for each microMIPS instruction in an unsplit multi-instruction sequence, just as we do now. Any attempt to change that should be a separate patch anyway. Thanks, Richard
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On 06/11/2013 10:03 AM, Jakub Jelinek wrote: > On Tue, Jun 11, 2013 at 09:56:28AM -0700, Richard Henderson wrote: >> I'm having trouble picking out what's supposed to be different between this >> function and default_binds_local_p_1. It appears to be the same set of >> tests, >> just rearranged into a possibly more efficient form requiring less runtime >> evaluation. > > What is different is > 1) weakref/ifunc handling without !TREE_PUBLIC (not sure about that though; >the old version would return true for !TREE_PUBLIC right away, but >e.g. default_binds_local_p_1 checks TREE_PUBLIC only after testing >for weakref) This sounds wrong even for binds_local_p. (1a) The documentation says that weakrefs must be static, which implies that TREE_PUBLIC must be 0. I guess that assumption could be invalidated by manipulations in the code, but I don't see them in handle_weakref_attribute. If someone knows better, please chime in... (1b) One *can* declare a static ifunc, but that only means that the resolver isn't reachable from outside the translation unit. The resolver, being user code, is free to return absolutely anything, so we can't be sure about whence it binds. > 2) DECL_COMMON with non-default visibility - the old version would return >true, now it returns false unless linker plugin tells us the current >common was used > > Or do you think we should just do what we did before and just > handle the 1) and 2) cases explicitly in the decl_binds_to_current_def_p > function? This (2) sounds like something that we could handle in d_b_t_c_d_p without having to duplicate all of binds_local_p. r~
Re: [PATCH] final.c: make debug_prefix_maps static
On Tue, 2013-06-11 at 09:34 -0700, Richard Henderson wrote: > On 06/11/2013 07:57 AM, David Malcolm wrote: > > Usage of "debug_prefix_maps" is hidden behind an API, with the only > > direct uses of the ptr being within final.c. Indeed the declaration of > > its type is local to that file. > > > > Hence it can be made static. > > > > Successfully bootstrapped on x86_64-unknown-linux-gnu > > > > OK for trunk? > > > > 2013-06-11 David Malcolm > > > > * final.c (debug_prefix_maps): Make static. > > > > Ok. Thanks; committed to svn trunk as r199963.
Re: [PATCH] function.c: Remove stray copy of initial_trampoline
On Tue, 2013-06-11 at 09:56 -0600, Jeff Law wrote: > On 06/11/13 09:16, David Malcolm wrote: > > > > 2013-06-11 David Malcolm > > > > * function.c (initial_trampoline): Remove stray copy; see > > varasm.c for the real copy. > This is fine. However, I'd just make the ChangeLog > > * function.c (initial_trampoline): Remove stray copy. Thanks; committed to svn trunk with the above ChangeLog as r199962.
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On Tue, Jun 11, 2013 at 09:56:28AM -0700, Richard Henderson wrote:
> On 06/11/2013 04:31 AM, Jakub Jelinek wrote:
> > +bool
> > +default_binds_to_current_def_p_1 (const_tree exp, int shlib)
> > +{
> ...
> > +}
>
> I'm having trouble picking out what's supposed to be different between this
> function and default_binds_local_p_1. It appears to be the same set of tests,
> just rearranged into a possibly more efficient form requiring less runtime
> evaluation.
What is different is
1) weakref/ifunc handling without !TREE_PUBLIC (not sure about that though;
the old version would return true for !TREE_PUBLIC right away, but
e.g. default_binds_local_p_1 checks TREE_PUBLIC only after testing
for weakref)
2) DECL_COMMON with non-default visibility - the old version would return
true, now it returns false unless linker plugin tells us the current
common was used
Or do you think we should just do what we did before and just
handle the 1) and 2) cases explicitly in the decl_binds_to_current_def_p
function?
Jakub
Re: [PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
On 06/11/2013 04:31 AM, Jakub Jelinek wrote:
> +bool
> +default_binds_to_current_def_p_1 (const_tree exp, int shlib)
> +{
...
> +}
I'm having trouble picking out what's supposed to be different between this
function and default_binds_local_p_1. It appears to be the same set of tests,
just rearranged into a possibly more efficient form requiring less runtime
evaluation.
r~
Re: [gomp4] Some progress on #pragma omp simd
On Sat, Apr 27, 2013 at 08:17:35PM +0200, Jakub Jelinek wrote:
> One way we could implement the SIMD private/lastprivate/reduction
> vars and for Cilk+ also firstprivate ones might be:
> - query the target what the maximum possible vectorization factor for the
> loop is (and min that with simdlen if any), let's call it MAXVF
...
I've made some progress on this, but am still far from being there, so
looking for comments. Known unhandled things are that the inliner doesn't
remap the simd uids (neither in loop structure nor in
__builtin_GOMP.simd_{vf,lane} arguments), the
adjust_simduid_builtins subpass doesn't actually try to shrink the sizes
of the arrays back to the decided vectorization factor, LTO doesn't handle
simd uids, and most importantly that the vectorizer still does a poor job on
it. Also, not sure what to do for lastprivate, probably use the magic
arrays and just in the epilogue of the loop compute which of the array items
belonged to the last iteration somehow.
I was using:
int a, b[1024];
struct S { S (); ~S (); int s; };
#pragma omp declare simd
__attribute__((noinline, noclone)) void
bar (int &x, int &y)
{
x += 4;
y += 4;
}
int
foo (void)
{
int i, j = 0, x;
S s;
#pragma omp simd private(x) private(s) reduction(+:j)
for (i = 0; i < 1024; i++)
{
a = 6;
x = 8;
s.s += 1;
b[i] += a + x + s.s;
j += b[i];
}
return j;
}
as a testcase, x being non-addressable scalar is just handled
normally, it shouldn't be observable if we have just one copy or more.
s, being addressable class that also needs to be constructed/destructed is
using the magic arrays and so is reduction. While the vectorizer can
recognize some reductions, e.g. without -ffast-math it will not vectorize
any floating point ones because that means changing the order of
computations, while when they are mandated to be one copy per simd lane,
the order of computations is clear and thus can be vectorized.
Also, especially with user defined reductions the reduction can be a
function call etc.
So, what I get out of ompexp on this is an initializing loop like:
:
D.2719 = __builtin_GOMP.simd_vf (1);
D.2713 = 0;
goto ;
:
D.2715[D.2713] = 0;
D.2718 = &D.2717[D.2713];
S::S (D.2718);
D.2713 = D.2713 + 1;
:
if (D.2713 < D.2719)
goto ;
else
goto ;
then the main loop:
:
i = 0;
goto ;
:
D.2714 = __builtin_GOMP.simd_lane (1);
a = 6;
x = 8;
D.2701 = D.2717[D.2714].s;
D.2702 = D.2701 + 1;
D.2717[D.2714].s = D.2702;
D.2703 = b[i];
a.0 = a;
D.2705 = a.0 + x;
D.2701 = D.2717[D.2714].s;
D.2706 = D.2705 + D.2701;
D.2707 = D.2703 + D.2706;
b[i] = D.2707;
D.2703 = b[i];
D.2727 = D.2715[D.2714];
D.2728 = D.2703 + D.2727;
D.2715[D.2714] = D.2728;
i = i + 1;
:
if (i < 1024)
goto ;
else
goto ;
and lastly a destruction/reduction loop:
:
D.2723 = __builtin_GOMP.simd_vf (1);
D.2713 = 0;
goto ;
:
D.2716 = D.2715[D.2713];
j = j + D.2716;
D.2718 = &D.2717[D.2713];
S::~S (D.2718);
D.2713 = D.2713 + 1;
:
if (D.2713 < D.2723)
goto ;
else
goto ;
I've discussed with richi on IRC the vectorizer data ref stuff somewhat,
but in the end kept this special function simd_lane, as the array indexes
modulo vectorization factor are nothing close to what the vectorizer was able
to handle. What I get in *.optimized is:
:
:
# ivtmp.41_45 = PHI
vect__17.10_7 = MEM[(struct S[32] *)&D.2717];
vect__18.11_1 = vect__17.10_7 + { 1, 1, 1, 1, 1, 1, 1, 1 };
MEM[(struct S[32] *)&D.2717] = vect__18.11_1;
_65 = (void *) ivtmp.41_45;
vect__20.17_50 = MEM[base: _65, offset: 0B];
vect__21.18_52 = vect__17.10_7 + { 15, 15, 15, 15, 15, 15, 15, 15 };
vect__22.20_53 = vect__20.17_50 + vect__21.18_52;
MEM[base: _65, offset: 0B] = vect__22.20_53;
vect__24.25_58 = MEM[(int[32] *)&D.2715];
vect__25.26_59 = vect__22.20_53 + vect__24.25_58;
MEM[(int[32] *)&D.2715] = vect__25.26_59;
ivtmp.41_32 = ivtmp.41_45 + 32;
if (ivtmp.41_32 != _20)
goto ;
else
goto ;
though, there is no pass that would be able to turn the memory accesses
into a vector load before the loop and just operations on vector register
followed by vector store at the end of the loop, and while CSE is able to
improve it somewhat at the RTL level, I still end up with unnecessary stack
stores inside of the loop.
vmovdqa 160(%rsp), %ymm0
movl$b, %eax
vmovdqa .LC0(%rip), %ymm3
vmovdqa .LC1(%rip), %ymm2
.L8:
vmovdqa %ymm0, %ymm1
addq$32, %rax
vpaddd %ymm3, %ymm0, %ymm0
vmovdqa %ymm0, 160(%rsp)! This is not needed
vpaddd %ymm2, %ymm1, %ymm1
vpaddd -32(%rax), %ymm1, %ymm1
vmovdqa %ymm1, -32(%rax)
cmpq$b+4096, %rax
vpaddd 32(%rsp), %ymm1, %ymm1 ! 32(%rsp) could be read before loop
vmovdqa %ymm1, 32(%rsp) ! This is not needed
jne .L8
! and we could just store %ymm0 to 160(
Re: C++: fix ICE with CONST_DECLs
On 06/11/2013 06:27 PM, Mike Stump wrote: Here is a simple one. When processing CONST_DECLs after an error, we can ICE. This avoid the ICE. No testcase? Paolo.
Re: [PATCH] final.c: make debug_prefix_maps static
On 06/11/2013 07:57 AM, David Malcolm wrote: > Usage of "debug_prefix_maps" is hidden behind an API, with the only > direct uses of the ptr being within final.c. Indeed the declaration of > its type is local to that file. > > Hence it can be made static. > > Successfully bootstrapped on x86_64-unknown-linux-gnu > > OK for trunk? > > 2013-06-11 David Malcolm > > * final.c (debug_prefix_maps): Make static. > Ok. r~
RE: [patch, mips] Micromips delay slot fix
> -Original Message-
> From: Maciej W. Rozycki [mailto:ma...@codesourcery.com]
> Sent: Monday, June 10, 2013 8:50 PM
> To: Steve Ellcey; Moore, Catherine
> Cc: Richard Sandiford; gcc-patches@gcc.gnu.org
> Subject: Re: [patch, mips] Micromips delay slot fix
>
> On Tue, 11 Jun 2013, Steve Ellcey wrote:
>
> > Sorry, it should have been 'main(void) {return 0; }. Then you get
> > (with the patch):
> >
> > j $31
> > move$2,$0
> >
> > instead of:
> >
> > move$2,$0
> > j $31
>
> Hmm, something must have been missed then from the microMIPS patches
> as our toolchain seems to get this case right:
>
> .setnoreorder
> .setnomacro
> jr $31
> move$2,$0
>
> .setmacro
> .setreorder
>
> (no idea where the extraneous newline comes from, hmm). Catherine, can
> you please double-check you haven't got anything outstanding yet?
There isn't anything outstanding. This is a bug in the mainline implementation
due to the introduction of microMIPS instruction length calculations. In the
original, all microMIPS insns had a length of 4.
>
> > > Is it safe to assume an RTL insn whose length is 4 has only a
> > > single machine instruction in the microMIPS mode? What's the
> > > difference to the
> > > MIPS16 instruction set here?
> >
> > I think so, I don't know of any microMIPS RTL instructions whose
> > length is 4 that would not be a single instruction. Maybe I should
> > add Catherine in to the discussion to see if she knows differently.
>
> I advise care here, even if we currently have no such pattern. It's easy to
> miss the dependency and any such bug introduced could trigger very rarely
> only. Also (unlike with the standard ISA) some otherwise ordinary
> instructions can't be scheduled into a delay slot, e.g. MOVEP or
> LWP/LDP/SWP/SDP.
>
There are only a handful of microMIPS-specific instructions. They are single
instructions, but they can't be placed in a delay slot. These instructions
explicitly set the can_delay attribute to "no".
I'm testing a slightly different patch from the one that Steve posted:
Index: mips.md
===
--- mips.md (revision 199648)
+++ mips.md (working copy)
@@ -703,8 +703,13 @@
;; Is it a single instruction?
(define_attr "single_insn" "no,yes"
- (symbol_ref "(get_attr_length (insn) == (TARGET_MIPS16 ? 2 : 4)
- ? SINGLE_INSN_YES : SINGLE_INSN_NO)"))
+ (if_then_else (ior (and (match_test "TARGET_MIPS16")
+ (match_test "get_attr_length (insn) == 2"))
+(and (eq_attr "compression" "micromips,all")
+ (match_test "TARGET_MICROMIPS"))
+(match_test "get_attr_length (insn) == 4"))
+ (const_string "yes")
+ (const_string "no")))
I'll let you know how it goes.
Catherine
C++: fix ICE with CONST_DECLs
Here is a simple one. When processing CONST_DECLs after an error, we can ICE.
This avoid the ICE.
Ok?
2013-06-11 Mike Stump
* init.c (constant_value_1): Protect CONST_DECLs better in the
face of errors.
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 44e558e..133a162 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1988,7 +1988,7 @@ constant_value_1 (tree decl, bool integral_p, bool
return_aggregate_cst_ok_p)
init = DECL_INITIAL (decl);
if (init == error_mark_node)
{
- if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
+ if (VAR_P (decl) && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
/* Treat the error as a constant to avoid cascading errors on
excessively recursive template instantiation (c++/9335). */
return init;
Re: [PATCH] tree-object-size.c: mark "unknown" as const
On 06/07/13 11:40, David Malcolm wrote: The array "unknown" within tree-object-size.c is only ever read from, not written to, so it can be marked as const. Successfully bootstrapped on x86_64-unknown-linux-gnu (using gcc-4.7.2-2.fc17.x86_64). OK for trunk? 2013-06-07 David Malcolm * tree-object-size.c (unknown): Make const. OK for the trunk. Thanks, jeff
Re: [PATCH] tree-into-ssa.c: make interesting_blocks static
On 06/07/13 11:36, David Malcolm wrote: "interesting_blocks" is only used inside tree-into-ssa.c, so it can be made static. Successfully bootstrapped on x86_64-unknown-linux-gnu (using gcc-4.7.2-2.fc17.x86_64). OK for trunk? 2013-06-07 David Malcolm * tree-into-ssa.c (interesting_blocks): Make static. OK for trunk. jeff
Re: PING - Sort SH / H8300 ISR tests
On 06/10/13 15:36, Oleg Endo wrote: Hello, Any comments on that one? Original message containing the attachment is here: http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01086.html On Mon, 2013-05-20 at 18:06 +0200, Oleg Endo wrote: Hi, I've noticed that testsuite/gcc.dg has some ISR related test cases that probably should have went to testsuite/gcc.target. The tests in question were mainly for SH, with two of them being used also for H8/300. Attached patch is supposed to fix that, also fixing some formatting issues and target checking for some of the SH tests. Unfortunately there's no H8/300 target test collection, so I took the liberty of adding one. The H8 bits are fine too. Please install onto the trunk, Jeff
Re: [PATCH] function.c: Remove stray copy of initial_trampoline
On 06/11/13 09:16, David Malcolm wrote: 2013-06-11 David Malcolm * function.c (initial_trampoline): Remove stray copy; see varasm.c for the real copy. This is fine. However, I'd just make the ChangeLog * function.c (initial_trampoline): Remove stray copy. Jeff
Re: new mul* patterns "U" constraint in rl78
> ; end of mulqi macro" > +;; [(set_attr "valloc" "umul")] I think this one needs to be macax also, since the constraints have "v" in them but the opcodes use ax. Other than that, it's OK.
Re: [PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)
Thanks!
[PATCH] function.c: Remove stray copy of initial_trampoline
Both function.c and varasm.c contain their own copies of this declaration: static GTY(()) rtx initial_trampoline; Only the one in varasm.c is used; the one in function.c appears to be a missing removal from this move: 2004-05-13 Diego Novillo Merge from tree-ssa-20020619-branch. (...snip...) * function.c (...snip...) (initial_trampoline): Move to varasm.c. * varasm.c (...snip...) (initial_trampoline): Move from function.c. as part of this commit on that branch: 2004-02-16 Richard Henderson * tree-nested.c: New file. (...snip...) (I'm not sure if the error occurred on the branch or during merger). I'm attaching a patch to remove the stray copy from function.c Successfully bootstrapped on x86_64-unknown-linux-gnu OK for trunk? 2013-06-11 David Malcolm * function.c (initial_trampoline): Remove stray copy; see varasm.c for the real copy. Index: gcc/function.c === --- gcc/function.c (revision 199957) +++ gcc/function.c (working copy) @@ -4980,8 +4980,6 @@ warning (OPT_Wunused_parameter, "unused parameter %q+D", decl); } -static GTY(()) rtx initial_trampoline; - /* Generate RTL for the end of the current function. */ void
Re: [c++-concepts] code review
> And you need to do this even when the call is type-dependent? Yes. Especially when the call is type-dependent. That's how I generate the constraint sets, which are used to determine the most constrained template during overload resolution.
Re: [PATCH] ARMv6-M MI thunk fix
On Tue, 11 Jun 2013 15:42:24 +0100 Julian Brown wrote: > Ahh -- so actually, maybe the right thing to do is use this last style > of thunk for v6-m, the same as v7-m -- is there any reason that > wouldn't work? (I wrote this patch to start with, but I don't remember > if I considered that possibility at the time...) Scrub that -- I mistakenly thought that v6m had long branch instructions. It doesn't (only long BL), so that won't work. Julian
Re: [c++-concepts] code review
On 06/11/2013 10:49 AM, Andrew Sutton wrote: After investigating, neither call_expr nor resolve_nondeduced_context do what I need. I need a resolution of a call expression that does not return overload sets, especially in the case where the initial call expression is already dependent. Does this have to do with restrictions on overloading of concept functions? Very much so. I need a single function because I'm inlining its body at the call site. And you need to do this even when the call is type-dependent? Jason
Re: [AArch64] Split simd combine
On 11/06/13 14:44, Sofiane Naci wrote: Hi, This patch split the simd aarch64_combine pattern. This passes the full regression test suite in aarch64-elf. OK for trunk? - Thanks Sofiane OK /Marcus
[PATCH] final.c: make debug_prefix_maps static
Usage of "debug_prefix_maps" is hidden behind an API, with the only direct uses of the ptr being within final.c. Indeed the declaration of its type is local to that file. Hence it can be made static. Successfully bootstrapped on x86_64-unknown-linux-gnu OK for trunk? 2013-06-11 David Malcolm * final.c (debug_prefix_maps): Make static. Index: gcc/final.c === --- gcc/final.c (revision 199957) +++ gcc/final.c (working copy) @@ -1471,7 +1471,7 @@ } debug_prefix_map; /* Linked list of such structures. */ -debug_prefix_map *debug_prefix_maps; +static debug_prefix_map *debug_prefix_maps; /* Record a debug file prefix mapping. ARG is the argument to
Re: SPE detection broken on Linux (bits/predefs.h: No such file or directory)
On Tue, Jun 11, 2013 at 4:36 AM, Roland Stigge wrote: > [Forwarding from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57578 , as > suggested] > > Hi! > > The build of powerpc spe on Linux aborts like this: > > [...] > /«PKGBUILDDIR»/build/./gcc/xgcc -B/«PKGBUILDDIR»/build/./gcc/ > -B/usr/lib/gcc-snapshot/powerpc-linux-gnuspe/bin/ > -B/usr/lib/gcc-snapshot/powerpc-linux-gnuspe/lib/ -isystem > /usr/lib/gcc-snapshot/powerpc-linux-gnuspe/include -isystem > /usr/lib/gcc-snapshot/powerpc-linux-gnuspe/sys-include-g -O2 -O2 -g > -O2 -DIN_GCC -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes > -Wmissing-prototypes -Wold-style-definition -isystem ./include -fPIC > -mlong-double-128 -mno-minimal-toc -g -DIN_LIBGCC2 -fbuilding-libgcc > -fno-stack-protector -fPIC -mlong-double-128 -mno-minimal-toc -I. -I. > -I../.././gcc -I../../../src/libgcc -I../../../src/libgcc/. > -I../../../src/libgcc/../gcc -I../../../src/libgcc/../include > -I../../../src/libgcc/../libdecnumber/dpd > -I../../../src/libgcc/../libdecnumber -DHAVE_CC_TLS -o > _gcov_merge_single.o -MT _gcov_merge_single.o -MD -MP -MF > _gcov_merge_single.dep -DL_gcov_merge_single -c > ../../../src/libgcc/libgcov.c > In file included from /usr/include/stdio.h:28:0, > from ../../../src/libgcc/../gcc/tsystem.h:87, > from ../../../src/libgcc/libgcov.c:27: > /usr/include/features.h:323:26: fatal error: bits/predefs.h: No such > file or directory > #include > ^ > compilation terminated. > [...] > > Turns out that the detection of the SPE case is done via > rs6000/e500-double.h in $(tm_file_list), but e500-double.h was removed > recently, and hence not added anymore to $(tm_file_list). However, in > the SPE (i.e. e500v2) case, with_cpu is set exactly to 8548 in > config.gcc. > > I solved this in gcc/config/rs6000/t-linux by replacing the line > > MULTIARCH_DIRNAME = powerpc-linux-gnuspe$(if $(findstring > rs6000/e500-double.h, $(tm_file_list)),,v1) > > with > > MULTIARCH_DIRNAME = powerpc-linux-gnuspe$(if $(findstring > 8548,$(with_cpu)),,v1) Olivier was the person who removed e500-double.h and added 8548 support. I would like to hear his and Eric's comment since they seem to be doing the most work on e500 at the moment. Thanks, David
Re: [c++-concepts] code review
>> After investigating, neither call_expr nor resolve_nondeduced_context >> do what I need. I need a resolution of a call expression that does not >> return overload sets, especially in the case where the initial call >> expression is already dependent. > > > Does this have to do with restrictions on overloading of concept functions? Very much so. I need a single function because I'm inlining its body at the call site. Andrew
Re: [PATCH] ARMv6-M MI thunk fix
On Mon, 10 Jun 2013 09:02:24 -0700
Cesar Philippidis wrote:
> On 6/10/13 8:32 AM, Richard Earnshaw wrote:
> > On 07/06/13 17:50, Cesar Philippidis wrote:
> >> On 6/6/13 9:00 AM, Richard Earnshaw wrote:
> >>> The pipeline offset is 4 for Thumb2 as well. So at the very
> >>> least you need to explain why your change doesn't apply then as
> >>> well.
> >>
> >> Yes some context is lost in that comment. Thunks are usually
> >> emitted in ARM mode, except for Thumb-only targets. Is the new
> >> comment OK? If so, please check it in since I do not have SVN
> >> write access.
> >>
> >
> > So what about ARMv7-M, which is thumb2 but no ARM state?
>
> Thumb2 is handled differently. This patch is inside an if
> (TARGET_THUMB1) block.
To expand, with examples (from g++.old-deja/g++.jason/thunk2.C):
-O2 -S -march=armv4t -fPIC -mthumb:
.code 32
.type _ZThn4_N8CExample9MixinFuncEi1A, %function
_ZThn4_N8CExample9MixinFuncEi1A:
.fnstart
.LFB19:
ldr r12, .LTHUMBFUNC0
.LTHUNKPC0:
add r12, pc, r12
sub r1, r1, #4
bx r12
.align 2
.LTHUMBFUNC0:
.word .LTHUNK0-(.LTHUNKPC0+7)
(Thumb-1 mode, but thunk is in ARM mode, PC offset is 8.)
-O2 -S -march=armv6-m -fPIC -mthumb:
.code 16
.thumb_func
.type _ZThn4_N8CExample9MixinFuncEi1A, %function
_ZThn4_N8CExample9MixinFuncEi1A:
.fnstart
.LFB19:
push {r3}
ldr r3, .LTHUMBFUNC0
.LTHUNKPC0:
add r3, pc, r3
mov r12, r3
sub r1, r1, #4
pop {r3}
bx r12
.align 2
.LTHUMBFUNC0:
.word .LTHUNK0-(.LTHUNKPC0+7)
(Current *wrong* behaviour: Thumb mode thunk, but the pipeline offset is
still 8.)
-O2 -S -march=armv7-m -fPIC -mthumb:
.thumb
.thumb_func
.type _ZThn4_N8CExample9MixinFuncEi1A, %function
_ZThn4_N8CExample9MixinFuncEi1A:
.fnstart
.LFB19:
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
sub r1, r1, #4
b .LTHUNK0(PLT)
(IIUC, wide branches are available, so there's no need for a synthesised
long PC-relative branch[-exchange]. Similar code is generated for ARM
mode on other architecture versions.)
Ahh -- so actually, maybe the right thing to do is use this last style
of thunk for v6-m, the same as v7-m -- is there any reason that
wouldn't work? (I wrote this patch to start with, but I don't remember
if I considered that possibility at the time...)
HTH,
Julian
Re: [Patch, Fortran] PR57535 - Fix class-array handling for function result variables
Le 06/06/2013 17:10, Tobias Burnus a écrit : > The attached test case failed with an ICE for function result variables > as in that case the function decl was used. > > Build and regtested on x86-64-gnu-linux. > OK for the trunk? > Yes, thanks Mikael
Re: [c++-concepts] code review
On 06/11/2013 09:45 AM, Andrew Sutton wrote: After investigating, neither call_expr nor resolve_nondeduced_context do what I need. I need a resolution of a call expression that does not return overload sets, especially in the case where the initial call expression is already dependent. Does this have to do with restrictions on overloading of concept functions? Jason
RE: new mul* patterns "U" constraint in rl78
Hi DJ, > "umul" is used when the insn takes two operands in A and X, and computes a > value into AX. > "macax" is used when the insn pattern itself reads from two virtual registers > and writes to a > virtual register (i.e. V = V op V), but clobbers AX anx BC in the process. Thanks for the clarification. I have updated the patch as below, adding "macax" for *mulhi3_rl78 pattern, while the other 2 use "umul" Let me know if below modified patch is OK to commit. Regards, Kaushik Index: gcc/config/rl78/constraints.md === --- gcc/config/rl78/constraints.md (revision 199949) +++ gcc/config/rl78/constraints.md (working copy) @@ -256,6 +256,19 @@ (match_test "!rl78_far_p (op) && rl78_as_legitimate_address (VOIDmode, XEXP (op, 0), true, ADDR_SPACE_GENERIC)")) ) +(define_memory_constraint "U" + "memory references valid with mov to/from a/ax" + (and (match_code "mem") + (match_test "rl78_virt_insns_ok () +|| satisfies_constraint_Wab (op) +|| satisfies_constraint_Wbc (op) +|| satisfies_constraint_Wde (op) +|| satisfies_constraint_Wd2 (op) +|| satisfies_constraint_Whl (op) +|| satisfies_constraint_Wh1 (op) +|| satisfies_constraint_Whb (op) +|| satisfies_constraint_Ws1 (op) +|| satisfies_constraint_Wfr (op) "))) (define_memory_constraint "Qbi" "built-in compare types" Index: gcc/config/rl78/rl78.md === --- gcc/config/rl78/rl78.md (revision 199949) +++ gcc/config/rl78/rl78.md (working copy) @@ -276,6 +276,7 @@ mova, x mov%h0, a ; end of mulqi macro" +;; [(set_attr "valloc" "umul")] ) (define_insn "*mulhi3_rl78" @@ -290,6 +291,7 @@ mulhu ; bcax = bc * ax movw%h0, ax ; end of mulhi macro" +;; [(set_attr "valloc" "macax")] ) (define_insn "*mulhi3_g13" @@ -309,6 +311,7 @@ movwax, 0x6 ; MDBL movw%h0, ax ; end of mulhi macro" +;; [(set_attr "valloc" "umul")] ) ;; 0x0 is MACR(L). 0x2 is MACR(H) but we don't care about it
[AArch64] Fix move_lo_quad_ move
Hi, This patch fixes a bug in the move_lo_quad_ pattern. The pattern, shown below, issues a scalar MOV instruction for vector modes: (define_insn "move_lo_quad_" [(set (match_operand:VQ 0 "register_operand" "=w,w,w") (vec_concat:VQ (match_operand: 1 "register_operand" "w,r,r") (vec_duplicate: (const_int 0] "TARGET_SIMD" "@ mov\\t%d0, %d1 fmov\t%d0, %1 ... This is fixed by using DUP for the first alternative instead. This passes the full regression test suite in aarch64-elf. OK for trunk? - Thanks Sofiane aarch64-mov-lo-quad-fix.patch Description: Binary data
[AArch64] Split simd combine
Hi, This patch split the simd aarch64_combine pattern. This passes the full regression test suite in aarch64-elf. OK for trunk? - Thanks Sofiane aarch64-split-simd-combine.diff Description: Binary data
Re: [c++-concepts] code review
>> I think I previously put constraint_info in lang_decl_min, right next >> to template_info no less. It was easy to manage there, and initialized >> as part of build_template_decl. But this obviously doesn't work for >> partial specializations unless they get template_decls. > > > Right. And we would want it to be specific to template_decls, not all decls > that use lang_decl_min. I'll have to check. I can't remember off the top of my head if non-template member functions have a corresponding template_decl. I think they do. auto declarations will also get constraints in the future. >> On the topic of logic.cc, I'm plan on rewriting this module to use a >> set instead of lists. There will be some pretty substantial changes to >> the internal interfaces. >> >> Would it be reasonable to commit this now (since it works correctly), >> and plan to rewrite it in the near future? > > > OK. I was experimenting with this over the weekend. I'm just going to rewrite it now, but without the optimizations I alluded to earlier. They didn't pan out the way I'd have liked. >> I think I see where the problem is. cp_parser_requires_clause is >> parsed non-optionally in a requires expression, but that's not >> included in the patch. I factored out the explicit parsing (hence the >> assertion) from the optional parsing. > > > The two situations seem pretty similar; you don't decide you're parsing a > requires expression until you see the keyword either, right? > > The usual pattern in the parser is for a function to try to parse a > particular construct and then return NULL_TREE if we're looking at something > else; it seems most straightforward to do that in this case as well. Yes, but I wasn't testing for the keyword prior to the call to cp_parser_requires_clause_opt. That's not quite true, I was in for member functions, but that was an oversight. Changing to test for requires won't be hard. >> What is the main entry point to overload resolution? > > > Perhaps finish_call_expr is what you want. After investigating, neither call_expr nor resolve_nondeduced_context do what I need. I need a resolution of a call expression that does not return overload sets, especially in the case where the initial call expression is already dependent. resolve_nondeduced_context looks like a good candidate to extend, but I hesitate to modify since it's used in a number of different places, and winnowing the overload set may not be appropriate in those contexts.
Re: [PATCH] dg-extract-results.sh tolerate spaces in variant names.
On 11/06/13 14:12, Jakub Jelinek wrote:
On Tue, Jun 11, 2013 at 02:07:50PM +0100, Marcus Shawcroft wrote:
On 11/06/13 13:55, Jakub Jelinek wrote:
On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
Hi,
The dg-extract-result.sh script is not tolerant of spaces in test variants.
Consider the following site.exp fragment:
set target_list [
list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
-mfloat-abi=softfp}" \
"arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
-mfloat-abi=hard}" \
]
Why don't you separate the variants with commas instead of spaces?
Jakub
That would test each of the listed options in isolation, one test
variant for each comma separated option, eight variants in total.
The example above tests just two variants, each with the set of
options as listed.
For that I think the syntax is slashes separating the options.
I.e.
RUNTESTFLAGS='--target_board=arm-eabi-aem\{-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp,-marm/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard\}'
Jakub
Aha, that works fine. Thank you!
/Marcus
Re: [PATCH] dg-extract-results.sh tolerate spaces in variant names.
On Tue, Jun 11, 2013 at 02:07:50PM +0100, Marcus Shawcroft wrote:
> On 11/06/13 13:55, Jakub Jelinek wrote:
> >On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
> >>Hi,
> >>
> >>The dg-extract-result.sh script is not tolerant of spaces in test variants.
> >>
> >>Consider the following site.exp fragment:
> >>
> >>
> >>set target_list [
> >> list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
> >>-mfloat-abi=softfp}" \
> >>"arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
> >>-mfloat-abi=hard}" \
> >> ]
> >
> >Why don't you separate the variants with commas instead of spaces?
> >
> > Jakub
> >
>
>
> That would test each of the listed options in isolation, one test
> variant for each comma separated option, eight variants in total.
> The example above tests just two variants, each with the set of
> options as listed.
For that I think the syntax is slashes separating the options.
I.e.
RUNTESTFLAGS='--target_board=arm-eabi-aem\{-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp,-marm/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard\}'
Jakub
Re: [PATCH] dg-extract-results.sh tolerate spaces in variant names.
On 11/06/13 13:55, Jakub Jelinek wrote:
On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
Hi,
The dg-extract-result.sh script is not tolerant of spaces in test variants.
Consider the following site.exp fragment:
set target_list [
list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
-mfloat-abi=softfp}" \
"arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
-mfloat-abi=hard}" \
]
Why don't you separate the variants with commas instead of spaces?
Jakub
That would test each of the listed options in isolation, one test
variant for each comma separated option, eight variants in total. The
example above tests just two variants, each with the set of options as
listed.
/Marcus
Re: [PATCH] dg-extract-results.sh tolerate spaces in variant names.
On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
> Hi,
>
> The dg-extract-result.sh script is not tolerant of spaces in test variants.
>
> Consider the following site.exp fragment:
>
>
> set target_list [
> list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
> -mfloat-abi=softfp}" \
>"arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
> -mfloat-abi=hard}" \
> ]
Why don't you separate the variants with commas instead of spaces?
Jakub
[PATCH] dg-extract-results.sh tolerate spaces in variant names.
Hi,
The dg-extract-result.sh script is not tolerant of spaces in test variants.
Consider the following site.exp fragment:
set target_list [
list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
-mfloat-abi=softfp}" \
"arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
-mfloat-abi=hard}" \
]
which results in a .sum file of the form:
=== gcc tests ===
Schedule of variations:
arm-eabi-aem/-marm -march=armv7-a -mfpu=vfpv3-d16 -mfloat-abi=softfp
arm-eabi-aem/-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
-mfloat-abi=hard
Running target arm-eabi-aem/-marm -march=armv7-a -mfpu=vfpv3-d16
-mfloat-abi=softfp
When the check target is executed in parallel (make -j) and RUNTESTFLAGS
is empty the Makefile arranges to split the tests into multiple groups
each run in parallel, the resulting .sum.sep and .log.sep files are then
stitched back into a single .sum and .log file using dg-extract-results.sh
The existing implementation of dg-extract-results.sh does not take
account of the spaces within a variation. Therefore in the above
example all bar the first option are stripped. Since the prefix of the
variation names are identical dg-extract-results.sh loses one set of
results.
Given the site.exp fragment above the output of make check is therefore
different depending on whether or not -j is specified.
I can find no definitive documentation that explicitly permits nor
prevents the use of space separated options within test variants.
However it seems like a reasonable use case and (bar the use of -j) it
currently works.
This patch adjusts dg-extract-results.sh to permit spaces within variant
names. The changes are rather ugly. Suggestions welcome.
I experimented with adjusting IFS to allow the shell scripts for loops
that iterate over lists of variants to ignore spaces, but this solution
was fragile (and ugly). The use of bash arrays would simplify the
change, however I believe this script was written with portability in
mind so I've attempted to stick to posix shell.
OK?
/Marcus
2013-06-11 Marcus Shawcroft
* dg-extract-results.sh (a_init, a_length, a_append, a_lookup): New.
* dg-extract-results.sh: Use array for VARIANTS.
Strip whitespace from temporary filenames. GAWK fragments to tolerate
whitespace embedded in variant names. Match variants in gawk
scripts using match().diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index 7029281..3253c51 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -52,15 +52,51 @@ msg() {
echo "$@" >&2
}
+# Provide the illusion of arrays. These arrays can store values that
+# can white space. We use an additional layer of indirection, each
+# value is stored in a shell variable with a name of the form
+# _. The array variable itself is just a
+# list of the underlying variable names.
+
+# Initialise an array to empty.
+a_init() {
+ local a="$1"
+ eval ${a}_n=0
+ eval ${a}=""
+}
+
+# Get the length of an array.
+a_length() {
+ local a="$1"
+ local nv=${a}_n
+ eval echo \$$nv
+}
+
+a_append() {
+ local a="$1"
+ local v="$2"
+ local nv=${a}_n
+ eval local count=\$$nv
+ eval ${a}_$count=\"$v\"
+ eval ${a}=\"\$${a} ${a}_$count\"
+ count=`expr $count + 1`
+ eval $nv=$count
+}
+
+a_lookup()
+{
+ eval echo \$$1
+}
+
# Parse the command-line options.
-VARIANTS=""
+a_init OPTVARIANTS
TOOL=""
MODE="sum"
while getopts "l:t:L" ARG; do
case $ARG in
- l) VARIANTS="${VARIANTS} ${OPTARG}";;
+ l) a_append OPTVARIANTS "${OPTARG}";;
t) test -z "$TOOL" || (msg "${PROGNAME}: only one tool can be specified"; exit 1);
TOOL="${OPTARG}";;
L) MODE="log";;
@@ -201,41 +237,39 @@ EOF
exit 0
fi
+a_init VARIANTS
+
# If no variants were specified, find all variants in the remaining
# summary files. Otherwise, ignore specified variants that aren't in
# any of those summary files.
-
-if test -z "$VARIANTS" ; then
+if test -z "$OPTVARIANTS" ; then
VAR_AWK=${TMP}/variants.awk
cat < $VAR_AWK
/^Schedule of variations:/ { in_vars=1; next }
/^$/{ in_vars=0 }
/^Running target/ { exit }
-{ if (in_vars==1) print \$1; else next }
+{ if (in_vars==1) print \$0; else next }
EOF
- touch ${TMP}/varlist
for FILE in $SUM_FILES; do
-$AWK -f $VAR_AWK $FILE >> ${TMP}/varlist
- done
- VARIANTS="`sort -u ${TMP}/varlist`"
+$AWK -f $VAR_AWK $FILE
+ done | sed 's/^[ ]*//' | sort -u > ${TMP}/varlist
+ while read line; do
+a_append VARIANTS "$line"
+ done < ${TMP}/varlist
else
- VARS="$VARIANTS"
- VARIANTS=""
- for VAR in $VARS
+ # Check the command line provided list of variants exists.
+ for v in $OPTVARIANTS
do
-grep "Running target $VAR" $SUM_FILES > /dev/null && VARIANTS="$VARIANTS $VAR"
+VAR=`a_lookup $v`
+if grep "Running target $VAR" $SUM_FILES > /dev/null; then
+
[PATCH] Fix up decl_binds_to_current_def_p (PR target/56564)
Hi!
As discussed on IRC, decl_binds_to_current_def_p doesn't handle
some cases properly, e.g. if a DECL_COMMON decl has hidden visibility,
then it returns true even without consulting linker plugin resolution,
even when the common might bind to a real definition of the var in some
other TU of the same module.
Fixed by adding new target hook for this. Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?
2013-06-11 Jakub Jelinek
PR target/56564
* output.h (default_binds_to_current_def_p,
default_binds_to_current_def_p_1): New prototypes.
* varasm.c (default_binds_to_current_def_p,
default_binds_to_current_def_p_1): New functions.
(decl_binds_to_current_def): Use binds_to_current_def_p
target hook.
* target.def (binds_to_current_def_p): New hook.
* doc/tm.texi.in (TARGET_BINDS_TO_CURRENT_DEF_P): Document.
* doc/tm.texi: Regenerated.
* config/i386/winnt.c (i386_pe_binds_to_current_def_p): New function.
* config/i386/i386-protos.h (i386_pe_binds_to_current_def_p): New
prototype.
* config/i386/i386.c (TARGET_BINDS_TO_CURRENT_DEF_P): Redefine for
PE and darwin.
* config/darwin-protos.h (darwin_binds_to_current_def_p): New
prototype.
* config/darwin.c (darwin_binds_to_current_def_p): New function.
* config/rs6000/rs6000.c (TARGET_BINDS_TO_CURRENT_DEF_P): Redefine for
darwin.
--- gcc/output.h.jj 2013-01-11 09:03:01.0 +0100
+++ gcc/output.h2013-06-11 08:40:22.247597454 +0200
@@ -583,6 +583,8 @@ extern void default_asm_output_anchor (r
extern bool default_use_anchors_for_symbol_p (const_rtx);
extern bool default_binds_local_p (const_tree);
extern bool default_binds_local_p_1 (const_tree, int);
+extern bool default_binds_to_current_def_p (const_tree);
+extern bool default_binds_to_current_def_p_1 (const_tree, int);
extern void default_globalize_label (FILE *, const char *);
extern void default_globalize_decl_name (FILE *, tree);
extern void default_emit_unwind_label (FILE *, tree, int, int);
--- gcc/varasm.c.jj12013-06-11 09:33:52.367820102 +0200
+++ gcc/varasm.c2013-06-11 11:17:53.162448015 +0200
@@ -6766,46 +6766,80 @@ default_binds_local_p_1 (const_tree exp,
return local_p;
}
-/* Return true when references to DECL must bind to current definition in
- final executable.
+/* Assume ELF-ish defaults, since that's pretty much the most liberal
+ wrt cross-module name binding. */
- The condition is usually equivalent to whether the function binds to the
- current module (shared library or executable), that is to binds_local_p.
- We use this fact to avoid need for another target hook and implement
- the logic using binds_local_p and just special cases where
- decl_binds_to_current_def_p is stronger than binds_local_p. In particular
- the weak definitions (that can be overwritten at linktime by other
- definition from different object file) and when resolution info is available
- we simply use the knowledge passed to us by linker plugin. */
bool
-decl_binds_to_current_def_p (tree decl)
+default_binds_to_current_def_p (const_tree exp)
{
- gcc_assert (DECL_P (decl));
- if (!TREE_PUBLIC (decl))
+ return default_binds_to_current_def_p_1 (exp, flag_shlib);
+}
+
+bool
+default_binds_to_current_def_p_1 (const_tree exp, int shlib)
+{
+ /* Weakrefs may not bind to the current def, even though the weakref itself
is
+ always static and therefore local. Similarly, the resolver for ifunc
functions
+ might resolve to a non-local function.
+ FIXME: We can resolve the weakref case more curefuly by looking at the
+ weakref alias. */
+ if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
+ || (TREE_CODE (exp) == FUNCTION_DECL
+ && lookup_attribute ("ifunc", DECL_ATTRIBUTES (exp
+return false;
+ /* Static variables are always local. */
+ if (! TREE_PUBLIC (exp))
return true;
- if (!targetm.binds_local_p (decl))
+ /* If PIC, then assume that any global name can be overridden by
+ symbols resolved from other modules, unless it has non-default
+ visibility. */
+ if (shlib && DECL_VISIBILITY (exp) == VISIBILITY_DEFAULT)
return false;
- /* When resolution is available, just use it. */
- if (TREE_CODE (decl) == VAR_DECL
- && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
+ /* With resolution file in hands, take look into resolutions. */
+ if (TREE_CODE (exp) == VAR_DECL && TREE_PUBLIC (exp)
+ && (TREE_STATIC (exp) || DECL_EXTERNAL (exp)))
{
- struct varpool_node *vnode = varpool_get_node (decl);
+ struct varpool_node *vnode = varpool_get_node (exp);
if (vnode
- && vnode->symbol.resolution != LDPR_UNKNOWN)
- return resolution_to_local_definition_p (vnode->symbol.resolution);
+ && resolution_to_local_definition_p (vnode->symbol.resolution))
+ return true;
}
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
Le 11/06/2013 12:00, Tobias Burnus a écrit :
> Hello Mikael,
>
> Mikael Morin wrote:
>> Le 03/06/2013 16:06, Tobias Burnus a écrit :
>>> diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
>>> index b2e8fdc..655d3c1 100644
>>> --- a/gcc/fortran/resolve.c
>>> +++ b/gcc/fortran/resolve.c
>>> @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace
>>> *ns)
>>> }
>>> }
>>> /* Add the attributes and the arrayspec to the temporary. */
>>> /* Add the attributes and the arrayspec to the temporary. */
>>> tmp->n.sym->attr = gfc_expr_attr (e);
>>> + tmp->n.sym->attr.function = 0;
>>> + tmp->n.sym->attr.result = 0;
>>> + tmp->n.sym->attr.flavor = FL_VARIABLE;
>>> +
>>> if (as)
>>> {
>>> tmp->n.sym->as = gfc_copy_array_spec (as);
>> This fixes the problem, but shouldn't the fix be in gfc_expr_attr
>> instead?
>> It seems to me that most symbol attributes don't make sense in any case
>> for non-variables, except for some of the standard ones
>> (allocatable,...) and possibly a couple more.
>
> I will audit the use of gfc_expr_attr and send an updated patch later.
>
> Is the current patch okay for GCC 4.8? I prefer simpler patches for the
> branch.
>
Yes.
Re: Document Intel Silvermont support in invoke.texi
On Tue, Jun 11, 2013 at 01:42:47PM +0400, Kirill Yukhin wrote: > > Ok, thanks. > > > Checked into MT: http://gcc.gnu.org/ml/gcc-cvs/2013-06/msg00376.html > I think we also should port core-avx2 into 4.8.x If you mean the documentation of it plus atom MOVBE listing, yes. Patch preapproved. Jakub
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
Hello Mikael,
Mikael Morin wrote:
Le 03/06/2013 16:06, Tobias Burnus a écrit :
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index b2e8fdc..655d3c1 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns)
}
}
/* Add the attributes and the arrayspec to the temporary. */
/* Add the attributes and the arrayspec to the temporary. */
tmp->n.sym->attr = gfc_expr_attr (e);
+ tmp->n.sym->attr.function = 0;
+ tmp->n.sym->attr.result = 0;
+ tmp->n.sym->attr.flavor = FL_VARIABLE;
+
if (as)
{
tmp->n.sym->as = gfc_copy_array_spec (as);
This fixes the problem, but shouldn't the fix be in gfc_expr_attr instead?
It seems to me that most symbol attributes don't make sense in any case
for non-variables, except for some of the standard ones
(allocatable,...) and possibly a couple more.
I will audit the use of gfc_expr_attr and send an updated patch later.
Is the current patch okay for GCC 4.8? I prefer simpler patches for the
branch.
Tobias
PS: Still pending review: [Patch, Fortran] PR57535 - Fix class-array
handling for function result variables,
http://gcc.gnu.org/ml/fortran/2013-06/msg00053.html
[v3] Fix libstdc++/56019
Hi,
tested x86_64-linux, committed to mainline.
Thanks,
Paolo.
2013-06-11 Paolo Carlini
PR libstdc++/56019
* include/c_global/cstddef (max_align_t): Add to namespace std.
* include/c_std/cstddef: Likewise.
* testsuite/18_support/max_align_t/requirements/1.cc: New.
* testsuite/18_support/max_align_t/requirements/2.cc: Likewise.
Index: include/c_global/cstddef
===
--- include/c_global/cstddef(revision 199937)
+++ include/c_global/cstddef(working copy)
@@ -36,7 +36,20 @@
// ISO C++ 14882: 18.1 Types
//
+#ifndef _GLIBCXX_CSTDDEF
+#define _GLIBCXX_CSTDDEF 1
+
#pragma GCC system_header
#include
#include
+
+#if __cplusplus >= 201103L
+namespace std
+{
+ // We handle size_t, ptrdiff_t, and nullptr_t in c++config.h.
+ using ::max_align_t;
+}
+#endif
+
+#endif // _GLIBCXX_CSTDDEF
Index: include/c_std/cstddef
===
--- include/c_std/cstddef (revision 199937)
+++ include/c_std/cstddef (working copy)
@@ -44,4 +44,12 @@
#include
#include
+#if __cplusplus >= 201103L
+namespace std
+{
+ // We handle size_t, ptrdiff_t, and nullptr_t in c++config.h.
+ using ::max_align_t;
+}
#endif
+
+#endif // _GLIBCXX_CSTDDEF
Index: testsuite/18_support/max_align_t/requirements/1.cc
===
--- testsuite/18_support/max_align_t/requirements/1.cc (revision 0)
+++ testsuite/18_support/max_align_t/requirements/1.cc (working copy)
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }
+//
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: Document Intel Silvermont support in invoke.texi
> Ok, thanks. > Checked into MT: http://gcc.gnu.org/ml/gcc-cvs/2013-06/msg00376.html I think we also should port core-avx2 into 4.8.x Thanks, K
Re: [GOOGLE] Avoid emitting duplicated weakref
> On 9 June 2013 05:31, Xinliang David Li wrote:
> > Guard also with L_IPO_COMP_MODE as this is lipo specific.
>
> Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537
>
> See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the
> alias in the first place (back then, honza may have changed that
> recently on trunk).
>
> Honza, Joseph,
> Are there rules for handling weakrefs in some standard? If so, which ones?
Not that I would know of. It is an extension engineered for glibc pthreads. in
mainline I quite changed way weakrefs are handled and I still plan to do more.
What LTO does now is to handle weakref symbol as static symbol despite it has
DECL_EXTERNAL on it. (I have patch in queue to really make it static).
This is because the symbol never hits the exported symbol table anyhow.
I.e. you can have:
cat f1.i
static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
$ cat f2.i
static __gthrw_pthread_once __attribute__ ((__weakref__
("comething_completely_else")));
that will break with your do_assemble_alias change. with LTO you will now get
__gthrw_pthread_once.1 and __gthrw_pthread_once.2 with proper weakref targets.
Honza
Re: [PATCH] PR32219, weak hidden reference segfault
ping, CCing middle-end maintainers for review. On 31 May 2013 10:13, Chung-Lin Tang wrote: > On 13/5/15 8:12 PM, Richard Sandiford wrote: >> Chung-Lin Tang writes: >>> On 13/5/10 6:37 PM, Richard Sandiford wrote: Chung-Lin Tang writes: > +case UNSPEC: > + /* Reach for a contained symbol. */ > + return nonzero_address_p (XVECEXP (x, 0, 0)); I don't think this is safe. UNSPECs really are unspecified :-), so we can't assume that (unspec X) is nonzero simply because X is. >>> >>> Attached is a modified patch (not yet tested but just for demonstration) >>> with a more specific test, hopefully regarded as more safe. >>> >>> The point is in recognizing (const (unspec [symbol] XYZ)) offsets in PIC >>> references, which seems quite idiomatic across all targets by now. >> >> I agree this is safer. However, there used to be some ports that >> use (plus pic_offset_table_rtx (symbol_ref X)) -- i.e. without an unspec -- >> to represent a PIC reference to X. That always seemed semantically wrong, >> since you're not actually adding the address of X and the PIC register, >> but the patch wouldn't handle that case correctly. > > Well I can't help those targets then, but at least nothing will be > changed for them by this patch. It will just continue to return 'true'. > >> An alternative might be to remove the pic_offset_table_rtx case altogether >> and rely on targetm.delegitimize_address instead. FWIW, I'd prefer that >> if it works, but it's not me you need to convince. :-) > > Like we discussed below, I think the direction should be towards making > things more machine-independent, rather then pushing more into the backend. > >>> I would suggest that this probably means there should be a new, more >>> specific construct in RTL to represent relocation values of this kind, >>> instead of (const (unspec)) serving an unofficial role; possibly some >>> real support for reasoning about PIC references could also be considered. >> >> Yeah, maybe we could try to introduce some target-independent knowledge >> of certain reloc types, a bit like the generic BFD_RELOC_*s in bfd. > > > FWIW, I've ran tests on the newer patch on i686-linux, with no > regressions. Testcase has been moved to gcc.dg/torture by recommendation > of Bernhard. If any of the RTL maintainers can give an eye of merciful > approval, this old PR could be resolved :) > > Thanks, > Chung-Lin >
Re: [GOOGLE] Avoid emitting duplicated weakref
On 9 June 2013 05:31, Xinliang David Li wrote:
> Guard also with L_IPO_COMP_MODE as this is lipo specific.
Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537
See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the
alias in the first place (back then, honza may have changed that
recently on trunk).
Honza, Joseph,
Are there rules for handling weakrefs in some standard? If so, which ones?
Just curious..
Thanks,
>
> David
>
> On Sat, Jun 8, 2013 at 8:29 PM, Dehao Chen wrote:
>> This patch fixes a bug when two weakref symbols are mapped to a same
>> assembler name.
>>
>> Testing on going.
>>
>> OK for google branches if test is fine?
>>
>> Thanks,
>> Dehao
>>
>> Index: gcc/varasm.c
>> ===
>> --- gcc/varasm.c (revision 199844)
>> +++ gcc/varasm.c (working copy)
>> @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target)
>>if (TREE_ASM_WRITTEN (decl))
>> return;
>>
>> + if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl))
>> + && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)))
>> +return;
>> +
>>/* We must force creation of DECL_RTL for debug info generation, even
>> though
>> we don't use it here. */
>>make_decl_rtl (decl);
Re: [patch] [python libstdc++ printers] Fix gdb/15195
On 21/03/13 14:20, Tom Tromey wrote:
>> "Phil" == Phil Muldoon writes:
>
> Phil> 2013-03-21 Phil Muldoon
> Phil> PR gdb/15195
>
> I think this should use a full URL.
> Phil> def to_string (self):
>
> Why doesn't the 'children' method also need a fix?
>
> Also, Phil and I discussed this on irc, and he is going to write a
> regression test.
Hi,
Attached is an updated patch correcting the issues that you pointed
out.
Cheers,
Phil
2013-06-11 Phil Muldoon
http://sourceware.org/bugzilla/show_bug.cgi?id=15195
* python/libstdcxx/v6/printers.py (StdTuplePrinter.__init__):
Acquire referenced value if tuple is a reference.
* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
flag. Add tuple and referenced tuple tests.
--
Index: python/libstdcxx/v6/printers.py
===
--- python/libstdcxx/v6/printers.py (revision 199642)
+++ python/libstdcxx/v6/printers.py (working copy)
@@ -313,6 +313,9 @@
def __init__ (self, typename, val):
self.typename = typename
self.val = val;
+type = self.val.type
+if type.code == gdb.TYPE_CODE_REF:
+ self.val = self.val.referenced_value()
def children (self):
return self._iterator (self.val)
Index: testsuite/libstdc++-prettyprinters/cxx11.cc
===
--- testsuite/libstdc++-prettyprinters/cxx11.cc (revision 199706)
+++ 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)
@@ -100,6 +102,10 @@
uptr->i = 23;
// { dg-final { regexp-test uptr {std::unique_ptr.datum. containing 0x.*} } }
+ ExTuple tpl(6,7);
+// { dg-final { note-test tpl {std::tuple containing = {[1] = 6, [2] = 7}} } }
+ ExTuple &rtpl = tpl;
+// { dg-final { note-test rtpl {std::tuple containing = {[1] = 6, [2] = 7}} }
}
placeholder(""); // Mark SPOT
use(efl);
use(fl);
SPE detection broken on Linux (bits/predefs.h: No such file or directory)
[Forwarding from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57578 , as suggested] Hi! The build of powerpc spe on Linux aborts like this: [...] /«PKGBUILDDIR»/build/./gcc/xgcc -B/«PKGBUILDDIR»/build/./gcc/ -B/usr/lib/gcc-snapshot/powerpc-linux-gnuspe/bin/ -B/usr/lib/gcc-snapshot/powerpc-linux-gnuspe/lib/ -isystem /usr/lib/gcc-snapshot/powerpc-linux-gnuspe/include -isystem /usr/lib/gcc-snapshot/powerpc-linux-gnuspe/sys-include-g -O2 -O2 -g -O2 -DIN_GCC -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -fPIC -mlong-double-128 -mno-minimal-toc -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -fPIC -mlong-double-128 -mno-minimal-toc -I. -I. -I../.././gcc -I../../../src/libgcc -I../../../src/libgcc/. -I../../../src/libgcc/../gcc -I../../../src/libgcc/../include -I../../../src/libgcc/../libdecnumber/dpd -I../../../src/libgcc/../libdecnumber -DHAVE_CC_TLS -o _gcov_merge_single.o -MT _gcov_merge_single.o -MD -MP -MF _gcov_merge_single.dep -DL_gcov_merge_single -c ../../../src/libgcc/libgcov.c In file included from /usr/include/stdio.h:28:0, from ../../../src/libgcc/../gcc/tsystem.h:87, from ../../../src/libgcc/libgcov.c:27: /usr/include/features.h:323:26: fatal error: bits/predefs.h: No such file or directory #include ^ compilation terminated. [...] Turns out that the detection of the SPE case is done via rs6000/e500-double.h in $(tm_file_list), but e500-double.h was removed recently, and hence not added anymore to $(tm_file_list). However, in the SPE (i.e. e500v2) case, with_cpu is set exactly to 8548 in config.gcc. I solved this in gcc/config/rs6000/t-linux by replacing the line MULTIARCH_DIRNAME = powerpc-linux-gnuspe$(if $(findstring rs6000/e500-double.h, $(tm_file_list)),,v1) with MULTIARCH_DIRNAME = powerpc-linux-gnuspe$(if $(findstring 8548,$(with_cpu)),,v1) Thanks in advance! Roland
Re: [announce] New scalar-storage-order branch in GCC repository
> I experimented with something like this a while ago. The goal was to
> have a -f{big,little}-endian switch that changes the default byteorder,
> and also to add endianness attributes. It didn't go very far, but I
> could compile some simple testcases and it seemed to behave as expected
> for those.
OK, that seems to be more ambitious than what we have done. As a matter of
fact, when we were discussing the specification of the feature, we tried to
narrow it to something that we believe was reasonably implementable and
maintainable in the long term, hence the more limited scope of our version.
> The patch is attached for reference. The interesting problems are
> byteswapping pointers and how to deal with function arguments; neither
> is dealt with in this patch (except for a crummy attempt to get varargs
> right). At the moment I don't intend to do further work on this.
Thanks for posting the patch in any case!
--
Eric Botcazou
Ping: Re: [DWARF] Fix multiple register spanning location.
Hello, May I have a review from the DWARF, the ARM and RS6000 maintainers for comments/approval ? http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01613.html Needed to fix the powerpc-spe bootstrap referenced in bugzilla #57389 Many Thanks Christian Christian Bruel wrote: >> However I feel a little bit uncomfortable with this solution that >> doesn't seem to fix the root cause. The dbx_register_number hooks is >> called basically from two places : dwarf2cfi.c and dwarf2out.c. That >> show different uses: either we want to refer to the hard regno when >> dealing with the cfa description (whereas we want DWARF_FRAME_REGNUM, >> not DBX_REGISTER_NUMBER). or we use the DBX_REGISTER_NUMBER for output >> register locations. >> >> Since this information cannot be detected contextually, I'd like to >> extend the dwarf_register_span target hook to return a dbx number or >> not. This is dwarf-span-target-dbx.patch >> >> build tested with the configurations that failed at one time or the other: >> - sh64-unknown-elf (The original sh64-elf build failure assertion in >> dwarf2cfi is fixed.) >> - arm-none-eabi -with-fpu=neon-vfpv4 >> - powerpc-e500v2-linux-gnuspe >> - x86_64-unknown-linux-gnu sanity build OK >> >> Is dwarf-span-target-dbx.patch OK for trunk ?. More comments ?
