Re: [PATCH] Fix i?86 *mov*insv_1* patterns (PR target/54436)
On Sat, Sep 1, 2012 at 8:32 AM, Jakub Jelinek wrote: > The following testcase results in an assembler warning on movb $700415, %ch > The problem is that the *mov*_insv_1* patterns use SImode or DImode for the > source operand, accept CONST_INTs in the constraints and nothing truncates > the constants to QImode. While the b modifier in %b1 handles changing the > printout of registers and memory (forcing it to be 8-bit low register or > in Intel syntax 8-bit memory), it doesn't handle CONST_INTs this way. > I've checked other such uses of b, w, k modifiers and usually they are used > either in widening of the operand (which is fine), or with constraints not > allowing integers. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk/4.7? > > 2012-09-01 Jakub Jelinek > > PR target/54436 > * config/i386/i386.md (*mov_insv_1_rex64, *movsi_insv_1): If > operands[1] is CONST_INT_P, convert it to QImode before printing. > > * gcc.dg/torture/pr54436.c: New test. OK. Thanks, Uros.
Re: [PATCH] Handle truncate of a memory location
Sorry for the slow review.
Andrew Pinski writes:
> Index: simplify-rtx.c
> ===
> --- simplify-rtx.c(revision 190730)
> +++ simplify-rtx.c(working copy)
> @@ -869,6 +869,14 @@ simplify_unary_operation_1 (enum rtx_cod
> && COMPARISON_P (op)
> && (STORE_FLAG_VALUE & ~GET_MODE_MASK (mode)) == 0)
> return rtl_hooks.gen_lowpart_no_emit (mode, op);
> +
> + /* A truncate of a memory is just loading the low part of the memory
> + if are not changing the meaning of the address. */
> + if (GET_CODE (op) == MEM
> + && !MEM_VOLATILE_P (op)
> + && !mode_dependent_address_p (XEXP (op, 0)))
> + return rtl_hooks.gen_lowpart_no_emit (mode, op);
"if we are not..."
> Index: testsuite/gcc.target/mips/truncate-8.c
> ===
> --- testsuite/gcc.target/mips/truncate-8.c(revision 0)
> +++ testsuite/gcc.target/mips/truncate-8.c(revision 0)
> @@ -0,0 +1,17 @@
> +/* { dg-options "-mgp64" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler "lw\t" } } */
"\tlw\t". Might be better to use:
/* { dg-final { scan-assembler-times "\tlw\t" 1 } } */
and add:
/* { dg-final { scan-assembler-not "\tld" } } */
just to make sure that there really is only one load in the output
(which there ought to be for something as simple as this).
> +/* { dg-final { scan-assembler-not "sll" } } */
"\td?sll"
OK with those changes, thanks.
Richard
Re: [middle-end] Add machine_mode to address_cost target hook
Oleg Endo writes:
> While experimenting a little bit with an idea for an address mode
> selection RTL pass for SH, I realized that SH's sh_address_cost function
> is quite broken. When trying to fix it, I ran against a wall, since the
> mode of the MEM is not passed to the target hook function, as it is e.g.
> in legitimate_address. This circumstance makes it a bit difficult to
> return useful answers in the address_cost hook. Like on SH,
> displacement address modes for anything < SImode are considered slightly
> more expensive due to increased pressure on R0.
>
> Since everything in the middle-end already seems to pass the mode to the
> 'address_cost' function in rtlanal.c, I'd like to propose to forward the
> mode arg to the target hook. The change is quite obvious, as it only
> adds one new (mostly) unused argument to the various address_cost
> functions in the targets.
Thanks for doing this. We should perhaps add the address space too,
but if you don't feel like redoing the whole patch, that can wait until
someone wants it.
> /* If the target doesn't override, compute the cost as with arithmetic. */
>
> int
> -default_address_cost (rtx x, bool speed)
> +default_address_cost (rtx x, enum machine_mode mode, bool speed)
> {
>return rtx_cost (x, MEM, 0, speed);
> }
Remove the "mode" parameter name because it's unused. I think this
would trigger a warning otherwise.
OK for the target-independent bits otherwise. For MIPS:
> Index: gcc/config/mips/mips.c
> ===
> --- gcc/config/mips/mips.c(revision 190780)
> +++ gcc/config/mips/mips.c(working copy)
> @@ -3943,7 +3943,8 @@
> /* Implement TARGET_ADDRESS_COST. */
>
> static int
> -mips_address_cost (rtx addr, bool speed ATTRIBUTE_UNUSED)
> +mips_address_cost (rtx addr, enum machine_mode mode ATTRIBUTE_UNUSED,
> +bool speed ATTRIBUTE_UNUSED)
> {
>return mips_address_insns (addr, SImode, false);
Please change this to:
static int
mips_address_cost (rtx addr, enum machine_mode mode,
bool speed ATTRIBUTE_UNUSED)
{
return mips_address_insns (addr, mode, false);
}
MIPS is one of those targets that wanted to know the mode all along.
Richard
Re: [TILE-Gx, committed] support -mcmodel=MODEL
On Tue, 28 Aug 2012, Walter Lee wrote:
> This patch adds support for the -mcmodel=MODEL flag on TILE-Gx.
At which point I cannot help asking for an update to the
release notes at http://gcc.gnu.org/gcc-4.8/changes.html. ;-)
Let me know if you need help with that.
> Index: gcc/doc/invoke.texi
> ===
> @emph{TILE-Gx Options}
> -@gccoptlist{-mcpu=CPU -m32 -m64}
> +@gccoptlist{-mcpu=CPU -m32 -m64 -mcmodel=@var{code-model}}
Not added by this change, but should CPU be @var{CPU} here?
> @emph{TILEPro Options}
> @gccoptlist{-mcpu=CPU -m32}
Why are only -mcpu and -m32 listed here?
> +Generate code for the small model. Distance for direct calls is
"The distance..."?
> +@item -mcmodel=large
> +@opindex mcmodel=large
> +Generate code for the large model. There is no limiation on call
"limitation"
Gerald
[PATCH, C] Mixed pointer types in call to streamer_tree_cache_lookup() in gcc/lto-streamer-out.c
uint32_t * is used as a 3rd parameter in call to streamer_tree_cache_lookup()
in 2 places in gcc/lto-streamer-out.c when the procedure prototype have
unsigned *. They are not guaranteed to be the same for all targets
(I got error when building for DJGPP)
Andris
ChangeLog entry
2012-09-01 Andris Pavenis
* lto-streamer-out.c (write_global_references,
lto_output_decl_state_refs):
Fix parameter type in call to streamer_tree_cache_lookup
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 2adae74..12335c5 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -1098,7 +1098,7 @@ write_global_references (struct output_block *ob,
for (index = 0; index < size; index++)
{
- uint32_t slot_num;
+ unsigned slot_num;
t = lto_tree_ref_encoder_get_tree (encoder, index);
streamer_tree_cache_lookup (ob->writer_cache, t, &slot_num);
@@ -1131,7 +1131,7 @@ lto_output_decl_state_refs (struct output_block *ob,
struct lto_out_decl_state *state)
{
unsigned i;
- uint32_t ref;
+ unsigned ref;
tree decl;
/* Write reference to FUNCTION_DECL. If there is not function,
Re: [middle-end] Add machine_mode to address_cost target hook
On Sat, 2012-09-01 at 10:10 +0100, Richard Sandiford wrote:
> Thanks for doing this. We should perhaps add the address space too,
> but if you don't feel like redoing the whole patch, that can wait until
> someone wants it.
I just had a look at the address space thing...
There are already target hook overloads with address space parameters,
like legitimate_address_p and legitimize_address. Paolo suggested to do
something similar a while ago:
http://gcc.gnu.org/ml/gcc-patches/2009-06/msg01191.html
.. but somehow this never made it into mainline?!
Maybe it would be better to ...
a) Add an 'address_cost' overload to the existing onces.
(at least it would be consistent...)
b) Remove the overloads altogether and add the address space arg to the
original funcs.
Personally, I'd probably favor b). Some targets (e.g. rl78) already
override the address space overloads and just ignore the address space
arg. Either way, maybe it's better to do it in a separate patch. This
looks a bit like a pandora's box :)
> > /* If the target doesn't override, compute the cost as with arithmetic. */
> >
> > int
> > -default_address_cost (rtx x, bool speed)
> > +default_address_cost (rtx x, enum machine_mode mode, bool speed)
> > {
> >return rtx_cost (x, MEM, 0, speed);
> > }
>
> Remove the "mode" parameter name because it's unused. I think this
> would trigger a warning otherwise.
Removing won't do here, because of:
Index: gcc/target.def
===
--- gcc/target.def (revision 190840)
+++ gcc/target.def (working copy)
@@ -1758,7 +1758,7 @@
DEFHOOK
(address_cost,
"",
- int, (rtx address, bool speed),
+ int, (rtx address, enum machine_mode mode, bool speed),
default_address_cost)
Instead I've added ATTRIBUTE_UNUSED to avoid the warning.
> Please change this to:
>
> static int
> mips_address_cost (rtx addr, enum machine_mode mode,
> bool speed ATTRIBUTE_UNUSED)
> {
> return mips_address_insns (addr, mode, false);
> }
>
> MIPS is one of those targets that wanted to know the mode all along.
>
OK, done. Updated patch attached.
ACK status of the whole thing so far is below.
Somebody please check all the boxes :)
[x] target-independent bits
[ ] alpha [ ] arm [ ] avr [ ] bfin
[ ] cr16 [ ] cris [ ] epiphany[ ] i386
[ ] ia64 [ ] iq2000[ ] lm32[ ] m32c
[ ] m32r [ ] mcore [ ] mep [x] microblaze
[x] mips [ ] mmix [x] mn10300 [ ] pa
[ ] rs6000[ ] rx[ ] s390[ ] score
[x] sh[ ] sparc [ ] spu [ ] stormy16
[ ] v850 [ ] vax [ ] xtensa
Cheers,
Oleg
Index: gcc/rtlanal.c
===
--- gcc/rtlanal.c (revision 190840)
+++ gcc/rtlanal.c (working copy)
@@ -3820,13 +3820,14 @@
if (!memory_address_addr_space_p (mode, x, as))
return 1000;
- return targetm.address_cost (x, speed);
+ return targetm.address_cost (x, mode, speed);
}
/* If the target doesn't override, compute the cost as with arithmetic. */
int
-default_address_cost (rtx x, bool speed)
+default_address_cost (rtx x, enum machine_mode mode ATTRIBUTE_UNUSED,
+ bool speed)
{
return rtx_cost (x, MEM, 0, speed);
}
Index: gcc/output.h
===
--- gcc/output.h (revision 190840)
+++ gcc/output.h (working copy)
@@ -606,7 +606,7 @@
extern void default_elf_fini_array_asm_out_destructor (rtx, int);
extern int maybe_assemble_visibility (tree);
-extern int default_address_cost (rtx, bool);
+extern int default_address_cost (rtx, enum machine_mode, bool);
/* Output stack usage information. */
extern void output_stack_usage (void);
Index: gcc/doc/tm.texi
===
--- gcc/doc/tm.texi (revision 190840)
+++ gcc/doc/tm.texi (working copy)
@@ -6440,7 +6440,7 @@
processed, and false when @code{rtx_cost} should recurse.
@end deftypefn
-@deftypefn {Target Hook} int TARGET_ADDRESS_COST (rtx @var{address}, bool @var{speed})
+@deftypefn {Target Hook} int TARGET_ADDRESS_COST (rtx @var{address}, enum machine_mode @var{mode}, bool @var{speed})
This hook computes the cost of an addressing mode that contains
@var{address}. If not defined, the cost is computed from
the @var{address} expression and the @code{TARGET_RTX_COST} hook.
Index: gcc/target.def
===
--- gcc/target.def (revision 190840)
+++ gcc/target.def (working copy)
@@ -1758,7 +1758,7 @@
DEFHOOK
(address_cost,
"",
- int, (rtx address, bool speed),
+ int, (rtx address, enum machine_mode mode, bool speed),
default_address_cost)
/* Return where to allocate pseudo for a given hard register initial value. */
Index: gcc/hooks.c
===
--- gcc/hooks.c (revision 190840)
[PING] C++ conversion - pull in cstdlib
Ping! This allows one to include e.g. in GCC source files. Since the switch to C++ has been made, this should be OK to do now, I guess. Cheers, Oleg On Sat, 2012-08-25 at 23:59 +0200, Oleg Endo wrote: > Hello, > > This one makes system.h pull in when compiling as C++. > It fixes issues when e.g. including . > Tested on my SH cross compiler config with 'make all-gcc', after > including the following std headers in one of the RTL passes: > > > e > > e > > . > > My host GCC is: > gcc -v > Using built-in specs. > COLLECT_GCC=gcc > COLLECT_LTO_WRAPPER=/usr/lib/gcc/i686-linux-gnu/4.6/lto-wrapper > Target: i686-linux-gnu > Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro > 4.6.3-1ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs > --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr > --program-suffix=-4.6 --enable-shared --enable-linker-build-id > --with-system-zlib --libexecdir=/usr/lib --without-included-gettext > --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 > --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu > --enable-libstdcxx-debug --enable-libstdcxx-time=yes > --enable-gnu-unique-object --enable-plugin --enable-objc-gc > --enable-targets=all --disable-werror --with-arch-32=i686 > --with-tune=generic --enable-checking=release --build=i686-linux-gnu > --host=i686-linux-gnu --target=i686-linux-gnu > Thread model: posix > gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) > > Cheers, > Oleg > > ChangeLog: > > * system.h: Include instead of when > compiling as C++.
Re: [PATCH, middle-end]: Introduce TARGET_LEGITIMATE_COMBINED_INSN target hook
On Sun, Aug 26, 2012 at 11:00 AM, Uros Bizjak wrote:
> Actually a v3 of TARGET_REJECT_COMBINED_INSN target hook.
>
> Changes:
> - rename the hook and reverse the return value
>
> 2012-08-25 Uros Bizjak
>
> * target.def (legitimate_combined_insn): New target hook.
> * doc/tm.texi.in (TARGET_LEGITIMATE_COMBINED_INSN): New hook.
> * doc/tm.texi: Regenerated.
> * combine.c (recog_for_combine): Call targetm.legitimate_combined_insn
> to allow targets to reject combined insn.
> * hooks.h (hook_bool_rtx_true): New.
> * hooks.c (hook_bool_rtx_true): Ditto.
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32},
> also with target-dependant x86 patch that implements the hook.
I have committed the patch to mainline SVN, since the patch is
non-algorithmic and has no impact on non-x86 targets.
Uros.
Fold VEC_PERM_EXPR a little more
Hello,
I noticed while writing the patch posted at:
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01755.html
that fold_ternary_loc sometimes doesn't go all the way for VEC_PERM_EXPR,
calling it a second time would fold even more. Fixed by a simple
reordering.
(bootstrap and testsuite are ok)
2012-09-01 Marc Glisse
gcc/
* fold-const.c (fold_ternary_loc): Constant-propagate after
removing dead operands.
gcc/testsuite/
* gcc.dg/fold-perm.c: Improve test.
--
Marc GlisseIndex: fold-const.c
===
--- fold-const.c(revision 190845)
+++ fold-const.c(working copy)
@@ -14189,40 +14189,40 @@ fold_ternary_loc (location_t loc, enum t
}
if (maybe_identity)
{
if (all_in_vec0)
return op0;
if (all_in_vec1)
return op1;
}
- if ((TREE_CODE (arg0) == VECTOR_CST
- || TREE_CODE (arg0) == CONSTRUCTOR)
- && (TREE_CODE (arg1) == VECTOR_CST
- || TREE_CODE (arg1) == CONSTRUCTOR))
- {
- t = fold_vec_perm (type, arg0, arg1, sel);
- if (t != NULL_TREE)
- return t;
- }
-
if (all_in_vec0)
op1 = op0;
else if (all_in_vec1)
{
op0 = op1;
for (i = 0; i < nelts; i++)
sel[i] -= nelts;
need_mask_canon = true;
}
+ if ((TREE_CODE (op0) == VECTOR_CST
+ || TREE_CODE (op0) == CONSTRUCTOR)
+ && (TREE_CODE (op1) == VECTOR_CST
+ || TREE_CODE (op1) == CONSTRUCTOR))
+ {
+ t = fold_vec_perm (type, op0, op1, sel);
+ if (t != NULL_TREE)
+ return t;
+ }
+
if (op0 == op1 && !single_arg)
changed = true;
if (need_mask_canon && arg2 == op2)
{
tree *tsel = XALLOCAVEC (tree, nelts);
tree eltype = TREE_TYPE (TREE_TYPE (arg2));
for (i = 0; i < nelts; i++)
tsel[i] = build_int_cst (eltype, sel[i]);
op2 = build_vector (TREE_TYPE (arg2), tsel);
Index: testsuite/gcc.dg/fold-perm.c
===
--- testsuite/gcc.dg/fold-perm.c(revision 190845)
+++ testsuite/gcc.dg/fold-perm.c(working copy)
@@ -1,19 +1,20 @@
/* { dg-do compile } */
/* { dg-options "-O -fdump-tree-ccp1" } */
typedef int veci __attribute__ ((vector_size (4 * sizeof (int;
-void fun (veci *f, veci *g, veci *h)
+void fun (veci *f, veci *g, veci *h, veci *i)
{
veci m = { 7, 7, 4, 6 };
veci n = { 0, 1, 2, 3 };
veci p = { 1, 1, 7, 6 };
+ *i = __builtin_shuffle (*i, p, m);
*h = __builtin_shuffle (*h, *h, p);
*g = __builtin_shuffle (*f, *g, m);
*f = __builtin_shuffle (*f, *g, n);
}
/* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 3, 3, 0, 2 }" "ccp1" } } */
/* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 1, 1, 3, 2 }" "ccp1" } } */
/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "ccp1" } } */
/* { dg-final { cleanup-tree-dump "ccp1" } } */
Re: [Jiří Paleček] [PATCH][C++] Fix constant reference in a lambda (PR c++/53488)
On 08/31/2012 09:17 PM, Gabriel Dos Reis wrote: Hmm, would we not run the risk of doing the "wrong" capture if everything is postponed? We wouldn't postpone name lookup, just the capture transformation. The long-term solution is to implement the rules properly, which depend on how a variable is used, not just qualities of the variable itself. Jason
Re: [Jiří Paleček] [PATCH][C++] Fix constant reference in a lambda (PR c++/53488)
On Sat, Sep 1, 2012 at 10:50 AM, Jason Merrill wrote: > On 08/31/2012 09:17 PM, Gabriel Dos Reis wrote: >> >> Hmm, would we not run the risk of doing the "wrong" capture if everything >> is postponed? > > > We wouldn't postpone name lookup, just the capture transformation. > > The long-term solution is to implement the rules properly, which depend on > how a variable is used, not just qualities of the variable itself. OK, thanks! -- Gaby
Re: [PING] C++ conversion - pull in cstdlib
On Sat, 1 Sep 2012, Oleg Endo wrote: > Ping! > > This allows one to include e.g. in GCC source files. > Since the switch to C++ has been made, this should be OK to do now, I > guess. This is not a review, but have you tested building the Ada front end with this patch applied? Given recent issues relating to how Ada uses system.h, I think any such changes need testing for Ada. -- Joseph S. Myers jos...@codesourcery.com
Re: [PING] C++ conversion - pull in cstdlib
On Sat, 2012-09-01 at 16:17 +, Joseph S. Myers wrote: > On Sat, 1 Sep 2012, Oleg Endo wrote: > > > Ping! > > > > This allows one to include e.g. in GCC source files. > > Since the switch to C++ has been made, this should be OK to do now, I > > guess. > > This is not a review, but have you tested building the Ada front end with > this patch applied? Given recent issues relating to how Ada uses > system.h, I think any such changes need testing for Ada. > No I haven't. C and C++ only. Good to know, thanks. Will try. Cheers, Oleg
Re: [PATCH, middle-end]: Introduce TARGET_LEGITIMATE_COMBINED_INSN target hook
On Sat, Sep 1, 2012 at 7:32 AM, Uros Bizjak wrote:
> On Sun, Aug 26, 2012 at 11:00 AM, Uros Bizjak wrote:
>
>> Actually a v3 of TARGET_REJECT_COMBINED_INSN target hook.
>>
>> Changes:
>> - rename the hook and reverse the return value
>>
>> 2012-08-25 Uros Bizjak
>>
>> * target.def (legitimate_combined_insn): New target hook.
>> * doc/tm.texi.in (TARGET_LEGITIMATE_COMBINED_INSN): New hook.
>> * doc/tm.texi: Regenerated.
>> * combine.c (recog_for_combine): Call
>> targetm.legitimate_combined_insn
>> to allow targets to reject combined insn.
>> * hooks.h (hook_bool_rtx_true): New.
>> * hooks.c (hook_bool_rtx_true): Ditto.
>>
>> Bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32},
>> also with target-dependant x86 patch that implements the hook.
>
> I have committed the patch to mainline SVN, since the patch is
> non-algorithmic and has no impact on non-x86 targets.
>
> Uros.
I think it caused:
FAIL: gcc.dg/pr14475.c (internal compiler error)
FAIL: gcc.dg/pr14475.c forward ref (test for errors, line 6)
FAIL: gcc.dg/pr14475.c extension (test for errors, line 6)
FAIL: gcc.dg/pr14475.c narrower (test for warnings, line 6)
FAIL: gcc.dg/pr14475.c incomplete (test for errors, line 6)
FAIL: gcc.dg/pr14475.c (test for excess errors)
on Linux/x86-64.
--
H.J.
Re: [PATCH, middle-end]: Introduce TARGET_LEGITIMATE_COMBINED_INSN target hook
On Sat, Sep 1, 2012 at 7:32 PM, H.J. Lu wrote:
>>> Actually a v3 of TARGET_REJECT_COMBINED_INSN target hook.
>>>
>>> Changes:
>>> - rename the hook and reverse the return value
>>>
>>> 2012-08-25 Uros Bizjak
>>>
>>> * target.def (legitimate_combined_insn): New target hook.
>>> * doc/tm.texi.in (TARGET_LEGITIMATE_COMBINED_INSN): New hook.
>>> * doc/tm.texi: Regenerated.
>>> * combine.c (recog_for_combine): Call
>>> targetm.legitimate_combined_insn
>>> to allow targets to reject combined insn.
>>> * hooks.h (hook_bool_rtx_true): New.
>>> * hooks.c (hook_bool_rtx_true): Ditto.
>>>
>>> Bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32},
>>> also with target-dependant x86 patch that implements the hook.
>>
>> I have committed the patch to mainline SVN, since the patch is
>> non-algorithmic and has no impact on non-x86 targets.
>>
>> Uros.
>
> I think it caused:
>
> FAIL: gcc.dg/pr14475.c (internal compiler error)
> FAIL: gcc.dg/pr14475.c forward ref (test for errors, line 6)
> FAIL: gcc.dg/pr14475.c extension (test for errors, line 6)
> FAIL: gcc.dg/pr14475.c narrower (test for warnings, line 6)
> FAIL: gcc.dg/pr14475.c incomplete (test for errors, line 6)
> FAIL: gcc.dg/pr14475.c (test for excess errors)
>
> on Linux/x86-64.
The test compiles OK for me.
Uros.
[patch] PR bootstrap/54453 (libstdc++ doesn't build)
Hello, r190783 breaks bootstrap on powerpc64-unknown-linux-gnu. The problem is caused by a regexp that used to check for space|tab before the patch but now only looks for tab. The attached patch fixes this problem for me, but I'm not sure why (I haven't tried to look into the details of the problem, I've simply reverted parts of the commit that broke things for me). Can a libstdc++ maintainer please have a look? Ciao! Steven PR54453.diff Description: Binary data
combine BIT_FIELD_REF and VEC_PERM_EXPR
Hello,
this patch makes it so that instead of taking the k-th element of a
shuffle of V, we directly take the k'-th element of V.
Note that I am *not* checking that the shuffle is only used once. There
can be some circumstances where this optimization will make us miss other
opportunities later, but restricting it to the single use case would make
it much less useful.
This is also my first contact with BIT_FIELD_REF, I may have missed some
properties of that tree.
bootstrap+testsuite ok.
2012-09-01 Marc Glisse
gcc/
* tree-ssa-forwprop.c (simplify_bitfield): New function.
(ssa_forward_propagate_and_combine): Call it.
gcc/testsuite/
* gcc.dg/tree-ssa/forwprop-21.c: New testcase.
(-21 because I have another patch pending review that adds a -20 testcase)
(simplify_bitfield appears before simplify_permutation to minimize
conflicts with that same patch, I can put it back in order when I commit
if you prefer)
--
Marc GlisseIndex: testsuite/gcc.dg/tree-ssa/forwprop-21.c
===
--- testsuite/gcc.dg/tree-ssa/forwprop-21.c (revision 0)
+++ testsuite/gcc.dg/tree-ssa/forwprop-21.c (revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+typedef int v4si __attribute__ ((vector_size (4 * sizeof(int;
+
+int
+test (v4si *x, v4si *y)
+{
+ v4si m = { 2, 3, 6, 5 };
+ v4si z = __builtin_shuffle (*x, *y, m);
+ return z[2];
+}
+/* { dg-final { scan-tree-dump-not "VEC_PERM_EXPR" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Property changes on: testsuite/gcc.dg/tree-ssa/forwprop-21.c
___
Added: svn:eol-style
+ native
Added: svn:keywords
+ Author Date Id Revision URL
Index: tree-ssa-forwprop.c
===
--- tree-ssa-forwprop.c (revision 190847)
+++ tree-ssa-forwprop.c (working copy)
@@ -2570,20 +2570,88 @@ combine_conversions (gimple_stmt_iterato
gimple_assign_set_rhs_code (stmt, CONVERT_EXPR);
update_stmt (stmt);
return remove_prop_source_from_use (op0) ? 2 : 1;
}
}
}
return 0;
}
+/* Combine an element access with a shuffle. Returns true if there were
+ any changes made, else it returns false. */
+
+static bool
+simplify_bitfield (gimple_stmt_iterator *gsi)
+{
+ gimple stmt = gsi_stmt (*gsi);
+ gimple def_stmt;
+ tree op, op0, op1, op2;
+ unsigned idx, n, size;
+ enum tree_code code;
+
+ op = gimple_assign_rhs1 (stmt);
+ gcc_checking_assert (TREE_CODE (op) == BIT_FIELD_REF);
+
+ op0 = TREE_OPERAND (op, 0);
+ op1 = TREE_OPERAND (op, 1);
+ op2 = TREE_OPERAND (op, 2);
+
+ if (TREE_CODE (TREE_TYPE (op0)) != VECTOR_TYPE)
+return false;
+
+ size = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (TREE_TYPE (op0;
+ n = TREE_INT_CST_LOW (op1) / size;
+ idx = TREE_INT_CST_LOW (op2) / size;
+
+ if (n != 1)
+return false;
+
+ if (TREE_CODE (op0) != SSA_NAME)
+return false;
+
+ def_stmt = SSA_NAME_DEF_STMT (op0);
+ if (!def_stmt || !is_gimple_assign (def_stmt)
+ || !can_propagate_from (def_stmt))
+return false;
+
+ code = gimple_assign_rhs_code (def_stmt);
+
+ if (code == VEC_PERM_EXPR)
+{
+ tree p, m, index, tem;
+ unsigned nelts;
+ m = gimple_assign_rhs3 (def_stmt);
+ if (TREE_CODE (m) != VECTOR_CST)
+ return false;
+ nelts = VECTOR_CST_NELTS (m);
+ idx = TREE_INT_CST_LOW (VECTOR_CST_ELT (m, idx));
+ idx %= 2 * nelts;
+ if (idx < nelts)
+ {
+ p = gimple_assign_rhs1 (def_stmt);
+ }
+ else
+ {
+ p = gimple_assign_rhs2 (def_stmt);
+ idx -= nelts;
+ }
+ index = build_int_cst (TREE_TYPE (TREE_TYPE (m)), idx * size);
+ tem = fold_build3 (BIT_FIELD_REF, TREE_TYPE (op), p, op1, index);
+ gimple_assign_set_rhs1 (stmt, tem);
+ update_stmt (stmt);
+ return true;
+}
+
+ return false;
+}
+
/* Determine whether applying the 2 permutations (mask1 then mask2)
gives back one of the input. */
static int
is_combined_permutation_identity (tree mask1, tree mask2)
{
tree mask;
unsigned int nelts, i, j;
bool maybe_identity1 = true;
bool maybe_identity2 = true;
@@ -2828,20 +2896,22 @@ ssa_forward_propagate_and_combine (void)
cfg_changed = true;
changed = did_something != 0;
}
else if (code == VEC_PERM_EXPR)
{
int did_something = simplify_permutation (&gsi);
if (did_something == 2)
cfg_changed = true;
changed = did_something != 0;
}
+ else if (code == BIT_FIELD_REF)
+ changed = simplify_bitfield (&gsi);
break;
}
[Patch, Forttran] PR54426 - handle common_head in gfc_undo_symbols
In one of my recent patches, I delete the sym->common_head to plug a
memory leak.
However, for invalid commons, gfc_undo_symbols is called, which might
remove the common head while it is still referenced from
namespace->common_root, leading to invalid memory access and (on some
systems) to ICEs (segfaults).
In my memory leak patch, unused common_heads are deleted. However,
before with undo_symbols the symtree in wasn't deleted. This lead to
access to invalid memory for invalid programs - either visible via
valgrind or as ICE (segfault [e.g. for gfortran.dg/common_6.f90]).
Build and regtested on x86-64-linux. I also checked
gfortran.dg/common_6.f90 for invalid memory access with valgrind and the
Polyhedron test cases for memory leaks [there are no new onces].
I intent to commit the attached patch tomorrow as obvious.
Tobias
2012-09-01 Tobias Burnus
PR fortran/54426
* symbol.c (find_common_symtree): New function.
(gfc_undo_symbols): Use it; free common_head if needed.
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 5e97c40..8d3b56c 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -2867,6 +2867,30 @@ gfc_get_ha_symbol (const char *name, gfc_symbol **result)
return i;
}
+
+/* Search for the symtree belonging to a gfc_common_head; we cannot use
+ head->name as the common_root symtree's name might be mangled. */
+
+static gfc_symtree *
+find_common_symtree (gfc_symtree *st, gfc_common_head *head)
+{
+
+ gfc_symtree *result;
+
+ if (st == NULL)
+return NULL;
+
+ if (st->n.common == head)
+return st;
+
+ result = find_common_symtree (st->left, head);
+ if (!result)
+result = find_common_symtree (st->right, head);
+
+ return result;
+}
+
+
/* Undoes all the changes made to symbols in the current statement.
This subroutine is made simpler due to the fact that attributes are
never removed once added. */
@@ -2890,6 +2914,17 @@ gfc_undo_symbols (void)
needs to be removed to stop the resolver looking
for a (possibly) dead symbol. */
+ if (p->common_block->head == p && !p->common_next)
+ {
+ gfc_symtree st, *st0;
+ st0 = find_common_symtree (p->ns->common_root,
+ p->common_block);
+
+ st.name = st0->name;
+ gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
+ free (st0);
+ }
+
if (p->common_block->head == p)
p->common_block->head = p->common_next;
else
Re: [Fortran] PR37336 - FIINAL patch [1/n]: Implement the finalization wrapper subroutine
On 29/08/2012 21:53, Tobias Burnus wrote:
> Dear all,
>
> that's the revised version of patch at
> http://gcc.gnu.org/ml/fortran/2012-08/msg00095.html, taking the review
> comments into account.
>
> Reminder: This patch only generates the finalization wrapper, which is
> in the virtual table. It does not add the required calls; hence, it
> still doesn't allow to use finalization.
>
>
> The patch consists of three parts:
>
> a) The main patch, which implements the wrapper.
> I am asking for approval for that patch.
A few more nitpicks below.
>
> b) A patch which removes the gfc_error "not yet implemented"
> I suggest to only remove the error after finalization calls have been
> added
Sensible. By the way some (testsuite) parts of b) should be part of a).
>
> c) A patch which bumps the .mod version
>- or alternatively -
>a patch which disables the _final generation in the vtable.
>
> I have build and regtested (on x86-64-linux) the patch with (a) and
> (a)+(b) applied.
>
>
> I would like to include the patch (c) as modifying the vtable changes
> the ABI. Bumping the .mod version is a reliable way to force
> recompilation. The alternative is to wait until the final FINAL patch
> before bumping the .mod version (and disable the "_final" generation).
I don't like bumping the module version, for something not
module-related (vtypes are output as normal types in the module files),
but I guess it is the most user-friendly thing to do.
>
> One possibility, if deemed useful, is to combine the .mod version bump
> with backward compatible reading of .mod files, i.e., only error out
> when BT_CLASS is encountered in an old .mod file.
Let's keep things simple, let's not do that.
>
>
> Is the patch (a) OK for the trunk? With which version of (c)?
>
> (I am slightly inclined to do the .mod bump now. As a follow up, one can
> also commit Janus' proc-pointer patch,
> http://gcc.gnu.org/ml/fortran/2012-04/msg00033.html, though I think
> someone has still to review it.)
I am inclined to disable finalization completely (thus defer .mod bump),
because the new code is hardly tested and doesn't provide (yet) new
benefits such as reduced memory leaks as it is disabled.
We could do the bump now, but I'm afraid that we discover a bug when
finalization gets completely implemented, and we have to bump again to
fix it (though I don't see what kind of bug it could be).
I think Janus' patch is a much less serious problem in the sense that
people trying to link codes compiled with patched and non-patched
compiler will get a link error. I don't think it's worth a .mod bump.
Unless I miss something.
For (a), I noticed a few indenting issues (8+ spaces) and (mostly
wording) nits below to be fixed. Then OK.
> diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
> index 21a91ba..9d58aab 100644
> --- a/gcc/fortran/class.c
> +++ b/gcc/fortran/class.c
> @@ -689,6 +694,702 @@ copy_vtab_proc_comps (gfc_symbol *declared, gfc_symbol
> *vtype)
> }
>
>
> +/* Returns true if any of its nonpointer nonallocatable components or
> + their nonpointer nonallocatable subcomponents has a finalization
> + subroutine. */
> +
> +static bool
> +has_finalizer_component (gfc_symbol *derived)
Rename to has_finalizable_component ?
> +{
> + gfc_component *c;
> +
> + for (c = derived->components; c; c = c->next)
> +{
> + if (c->ts.type == BT_DERIVED && c->ts.u.derived->f2k_derived
> + && c->ts.u.derived->f2k_derived->finalizers)
> + return true;
> +
> + if (c->ts.type == BT_DERIVED
> + && !c->attr.pointer && !c->attr.allocatable
> + && has_finalizer_component (c->ts.u.derived))
> + return true;
> +}
> + return false;
> +}
> +
> +
> +/* Call DEALLOCATE for the passed component if it is allocatable, if it is
> + neither allocatable nor a pointer but has a finalizer, call it. If it
> + is a nonpointer component with allocatable or finalizes components, walk
s/finalizes/finalizable/ ?
> + them. Either of the is required; other nonallocatables and pointers aren't
s/the/them/ ?
> + handled gracefully.
> + Note: The DEALLOCATE handling takes care of finalizers, coarray
> + deregistering and allocatable components of the allocatable. */
"coarray deregistering and allocatable components" is confusing.
Note: If the component is allocatable, the DEALLOCATE handling takes
care of calling the appropriate finalizer(s), of coarray deregistering,
and of deallocating allocatable (sub)components.
> +
> +static void
> +finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
> + gfc_expr *stat, gfc_code **code)
[...]
> +
> +
> +/* Generate the wrapper finalization/polymorphic freeing subroutine for the
Difficult to read.
"Generate the finalization/polymorphic freeing wrapper subroutine..." or
something ?
> + derived type "derived". The function first calls the approriate FINAL
> + subroutine, then it DEALLOCATEs (finalizes/frees) the
[patch, fortran] Set -Wcompare-reals from -Wextra
Hello world, the attached patch sets -Wcompare-reals from -Wextra. It also dcouments a few cases (found while browsing the source) of options included in -Wall in invoke.texi. It also allows easy adding of other warning options to -Wextra. Regression-tested. OK for trunk? Thomas 2012-09-01 Thomas König * lang.opt (Wextra): Add. * invoke.texi: Document that -Wc-binding-type, -Wconversion and -Wline-truncation are implied by -Wall. Document that -Wcompare-reals is implied by -Wextra. * options.c (set_Wextra): New function. (gfc_handle_option): Handle -Wextra. 2012-09-01 Thomas König * gfortran.dg/wextra_1.f: New test.
Re: [PING] C++ conversion - pull in cstdlib
On Sat, 2012-09-01 at 18:25 +0200, Oleg Endo wrote: > On Sat, 2012-09-01 at 16:17 +, Joseph S. Myers wrote: > > On Sat, 1 Sep 2012, Oleg Endo wrote: > > > > > Ping! > > > > > > This allows one to include e.g. in GCC source files. > > > Since the switch to C++ has been made, this should be OK to do now, I > > > guess. > > > > This is not a review, but have you tested building the Ada front end with > > this patch applied? Given recent issues relating to how Ada uses > > system.h, I think any such changes need testing for Ada. > > > > No I haven't. C and C++ only. Good to know, thanks. Will try. > OK, now I have. ada, c, c++, fortran, go, java, objc, obj-c++ do build here. Cheers, Oleg
Re: VxWorks Patches Back from the Dead!
Hi all,
I have a new set of patches attached to this email. I've made a few
changes since last time, and a full build works (finally, again).
-The ioctl patch removes superfluous parens (thanks Paolo)
-The mkdir patch has a more precise (or uglier, depending on your point
of view :P) regex, and uses the variadic macro fix (thanks again Paolo)
-It adds another fixincludes patch for vxworks *NOT GCC* regs.h as it
doesn't include vxTypesOld.h, which it needs
-The patch that adds the AC_ARG_ENABLE option to explicitly
enable/disable libstdc++-v3 now has the argument changed from
--disable-libstdc++-v3 to --disable-libstdcxx, as I was having weird
issues and changing it seemed to work. If someone who has more
experience with this than me (so anyone) knows that this should not make
a difference than feel free to reject this change and I'll fall back to
the old version.
-The patch to allow machine_name to be skipped and enable fixincludes
for vxworks is now *much* simpler (thanks Bruce)
-There's a new patch to add NOMINMAX to
libstdc++-v3/config/os/vxworks/os_defines.h to keep vxworks headers from
defining min and max as macros.
Note that the open() patch is NOT changed as the suggested change does
not work when also compiling libstdc++-v3. Passing the third argument
explicitly won't break anything - it will just be ignored - and is the
only resolution that has worked so far.
They're also not in the same order as I was rebasing quite a bit to try
and keep history in sync.
Also, I'm hoping since this is the third (or fourth for some of these
patches) time submitting them, hopefully they can eventually make their
way in :D. Could someone please volunteer to commit once it is all
approved?
Thank you all for the work you do on GCC, and the help you've given me
to get my patches up to the standard!
Robert Mason
>From f1132398e72e73c549cb7f608a3a43c0f4801bc3 Mon Sep 17 00:00:00 2001
From: rbmj
Date: Mon, 4 Jun 2012 13:18:10 -0400
Subject: [PATCH 01/12] Added assert fixinclude hack for VxWorks.
VxWorks's assert.h relies on adjacent string tokens being joined,
and uses macros for some of the strings (e.g. __FILE__). However,
it does not put a space after the end quote and before the macro,
so instead of replacing the macro, gcc >= 4.7.x thinks it's a
user-defined literal token, and since the lookup obviously fails,
compilation of libstdc++ dies.
This patch just replaces the assert.h header with another one that
will work. It preserves the same format, just changes the spacing.
Proposed by Robert Mason:
http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00385.html
Approved by Nathan Sidwell:
http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00549.html
Approved by Bruce Korb: http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00552.html
Changes:
[fixincludes]
inclhack.def (AAB_vxworks_assert): Added fix.
fixincl.x: Regenerate
---
fixincludes/inclhack.def | 40
1 file changed, 40 insertions(+)
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 82792af..a9d582d 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -354,6 +354,46 @@ fix = {
_EndOfHeader_;
};
+/*
+ * Fix assert.h on VxWorks:
+ */
+fix = {
+ hackname= AAB_vxworks_assert;
+ files = assert.h;
+ mach= "*-*-vxworks*";
+
+ replace = <<- _EndOfHeader_
+ #ifndef _ASSERT_H
+ #define _ASSERT_H
+
+ #ifdef assert
+ #undef assert
+ #endif
+
+ #if defined(__STDC__) || defined(__cplusplus)
+ extern void __assert (const char*);
+ #else
+ extern void __assert ();
+ #endif
+
+ #ifdef NDEBUG
+ #define assert(ign) ((void)0)
+ #else
+
+ #define ASSERT_STRINGIFY(str) ASSERT_STRINGIFY_HELPER(str)
+ #define ASSERT_STRINGIFY_HELPER(str) #str
+
+ #define assert(test) ((void) \
+ ((test) ? ((void)0) : \
+ __assert("Assertion failed: " ASSERT_STRINGIFY(test) ", file " \
+ __FILE__ ", line " ASSERT_STRINGIFY(__LINE__) "\n")))
+
+ #endif
+
+ #endif
+ _EndOfHeader_;
+};
+
/*
* complex.h on AIX 5 and AIX 6 define _Complex_I and I in terms of __I,
--
1.7.10.4
>From 1220d34b665432ba238e1b58119576f66c015772 Mon Sep 17 00:00:00 2001
From: rbmj
Date: Mon, 4 Jun 2012 14:16:26 -0400
Subject: [PATCH 02/12] Add hack for ioctl() on VxWorks.
ioctl() is supposed to be variadic, but VxWorks only has a three
argument version with the third argument of type int. This messes
up when the third argument is not implicitly convertible to int.
This adds a macro which wraps around ioctl() and explicitly casts
the third argument to an int. This way, the most common use case
of ioctl (with a const char * for the third argument) will compile
in C++, where pointers must be explicitly casted to int.
Proposed by Robert Mason:
http://gcc.gnu.org/ml
