Re: Reduce complette unrolling & peeling limits

[Eric Botcazou]

> this patch reduces max-peeled-insns and max-completely-peeled-insns from 400
> to 100.  The reason why I am doing this is that I want to reduce code bloat
> caused by my cunroll work that enabled a lot more unrolling then previously
> causing considerable code size regression at -O3.

Did you notice that gcc.c-torture/compile/pr43186.c regressed?  It now again 
takes a while to compile, so times out on slow machines:

FAIL: gcc.c-torture/compile/pr43186.c  -O3 -fomit-frame-pointer  (test for 
excess errors)
FAIL: gcc.c-torture/compile/pr43186.c  -O3 -fomit-frame-pointer -funroll-loops  
(test for excess errors)
FAIL: gcc.c-torture/compile/pr43186.c  -O3 -fomit-frame-pointer -funroll-all-
loops -finline-functions  (test for excess errors)
FAIL: gcc.c-torture/compile/pr43186.c  -O3 -g  (test for excess errors)
WARNING: program timed out.
WARNING: program timed out.
WARNING: program timed out.
WARNING: program timed out.

-- 
Eric Botcazou

Re: VEC re-write [patch 04/25]

[Eric Botcazou]

> 2012-11-15  Diego Novillo  
> 
>   Adjust for new vec API (http://gcc.gnu.org/wiki/cxx-conversion/cxx-vec)
> 
>   * gcc-interface/decl.c: Use new vec API in vec.h.
>   * gcc-interface/gigi.h: Likewise.
>   * gcc-interface/trans.c: Likewise.
>   * gcc-interface/utils.c: Likewise.
>   * gcc-interface/utils2.c: Likewise.

Thanks for the great work, everything seems to be working fine in Ada.

Minor nit: the patch has introduced a couple of trailing spaces.  No big deal, 
but we have a script that rejects the changes in this case.  Fixed thusly.


2012-11-18  Eric Botcazou  

* gcc-interface/decl.c: Remove trailing spaces.
* gcc-interface/trans.c: Remove trailing spaces.


-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 193596)
+++ gcc-interface/decl.c	(working copy)
@@ -7549,7 +7549,7 @@ build_position_list (tree gnu_type, bool
of operands to SUBSTITUTE_IN_EXPR.  DEFINITION is true if this is for
a definition of GNAT_SUBTYPE.  */
 
-static vec 
+static vec
 build_subst_list (Entity_Id gnat_subtype, Entity_Id gnat_type, bool definition)
 {
   vec gnu_list = vec();
@@ -7582,7 +7582,7 @@ build_subst_list (Entity_Id gnat_subtype
the substitutions described in SUBST_LIST.  GNU_LIST is a pre-existing
list to be prepended to the newly created entries.  */
 
-static vec 
+static vec
 build_variant_list (tree qual_union_type, vec subst_list,
 		vec gnu_list)
 {
Index: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 193596)
+++ gcc-interface/trans.c	(working copy)
@@ -3323,7 +3323,7 @@ Subprogram_Body_to_gnu (Node_Id gnat_nod
 {
   tree gnu_return_var = NULL_TREE;
 
-  vec_safe_push (gnu_return_label_stack, 
+  vec_safe_push (gnu_return_label_stack,
 		 create_artificial_label (input_location));
 
   start_stmt_group ();
@@ -4385,7 +4385,7 @@ Handled_Sequence_Of_Statements_to_gnu (N
   start_stmt_group ();
   gnat_pushlevel ();
 
-  vec_safe_push (gnu_except_ptr_stack, 
+  vec_safe_push (gnu_except_ptr_stack,
 		 create_var_decl (get_identifier ("EXCEPT_PTR"), NULL_TREE,
   build_pointer_type (except_type_node),
   build_call_n_expr (get_excptr_decl, 0),

Re: [patch] [mips] add multiarch definitions for mips-linux-gnu

[Richard Sandiford]

Matthias Klose  writes:
> 2012-11-14  Matthias Klose  
>
>   * config/mips/t-linux64: Add multiarch names in MULTILIB_OSDIRNAMES.

OK, thanks.

Richard

Re: [PATCH, PR55315] Don't assume a nonzero address plus a const is a nonzero address

[Richard Sandiford]

Tom de Vries  writes:
> 2012-11-17  Tom de Vries  
>
>   PR rtl-optimization/55315
>
>   * rtlanal.c (nonzero_address_p): Don't assume a nonzero address plus a
>   const is a nonzero address.
>
>   * gcc.target/mips/pr55315.c: New test.

OK, thanks.

Richard

Re: [PATCH][RFC] Bug handling SUBREG (MEM) - MEM having side-effects?

[Richard Sandiford]

Tejas Belagod  writes:

> Hi,
>
> I seem to have uncovered what seems to be a bug with handling SUBREG (MEM) 
> with
> the MEM having side-effects while testing aarch64-4.7. This bug seems to be
> latent on trunk.
>
> Here is a test case reduced from gcc.c-torture/execute/scal-to-vec1.c.
>
> #define vector(elcount, type)  \
> __attribute__((vector_size((elcount)*sizeof(type type
>
> #define vidx(type, vec, idx) (*((type *) &(vec) + idx))
>
> #define operl(a, b, op) (a op b)
> #define operr(a, b, op) (b op a)
>
> #define check(type, count, vec0, vec1, num, op, lr) \
> do {\
> int __i; \
> for (__i = 0; __i < count; __i++) {\
> if (vidx (type, vec1, __i) != oper##lr (num, vidx (type, vec0, 
> __i),
> op)) \
> __builtin_abort (); \
> }\
> } while (0)
>
> #define veccompare(type, count, v0, v1) \
> do {\
> int __i; \
> for (__i = 0; __i < count; __i++) { \
> if (vidx (type, v0, __i) != vidx (type, v1, __i)) \
> __builtin_abort (); \
> } \
> } while (0)
>
> volatile int one = 1;
>
> int main (int argc, char *argv[]) {
>
> vector(8, short) v0 = {one, 1, 2, 3, 4, 5, 6, 7};
> vector(8, short) v1;
> v1 = 2 << v0;   check (short, 8, v0, v1, 2, <<, l);
> return 0;
> }
>
> When compiled with -O1, the combiner tries to combine these two instructions:
>
> (insn 87 86 89 4 (set (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ])
> (sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113 [ ivtmp.23 ])) [0
> MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) scal-to-vec1.c:35 54
> {*extendhisi2_aarch64}
>  (expr_list:REG_INC (reg:DI 113 [ ivtmp.23 ])
> (nil)))
>
> (insn 89 87 90 4 (set (reg:SI 155)
> (ashift:SI (reg:SI 158)
> (subreg:QI (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ]) 
> 0)))
> scal-to-vec1.c:35 331 {*ashlsi3_insn}
>  (expr_list:REG_DEAD (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ])
> (nil)))
>
> It tries to forward-propagate reg 154 to insn 89 and the intermediate RTL 
> looks
> like this:
>
> (insn 89 87 90 4 (set (reg:SI 155)
> (ashift:SI (reg:SI 158)
> (subreg:QI ((sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113
>  [ivtmp.23 ])) [0 MEM[base:
>  D.2294_40, offset: 0B]+0 S2 A16])))
>
> The combiner then recursively tries to simplify this RTL. During simplifying 
> the
> subreg part of the RTL i.e.
>
> (subreg:QI ((sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0
>  MEM[base: D.2294_40, offset:
>  0B]+0 S2 A16])))
>
> combine_simplify_rtx () calls simplify_subreg () and this returns
>
> (subreg:QI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0
>  MEM[base: D.2294_40, offset: 0B]+0 S2 A16])))
>
> The code that does this is in combine.c:combine_simplify_rtx ()
>
> ...
>  rtx temp;
>  temp = simplify_subreg (mode, SUBREG_REG (x), op0_mode,
>  SUBREG_BYTE (x));
>  if (temp)
>return temp;
>   }
> ...
>
> So, the above tree is returned as is and insn 87 gets combined into insn 89 to
> become
>
> (insn 89 87 90 4 (set (reg:SI 155)
> (ashift:SI (reg:SI 158)
> (subreg:QI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0
>  MEM[base: D.2294_40, offset: 0B]+0 S2 A16])))
>
> This ends up in reload and in cleanup_subreg_operands () the subreg:QI 
> (mem:HI)
> gets narrowed to mem:QI, but the interesting bit is that the pre_inc stays the
> same, so becomes a pre_inc for a QI mode address rather than the original HI
> mode address and therefore instead of addressing a byte and incrementing the
> address by 2 to get the next half word, the address gets incremented by 1!
>
> Also, I feel this is a latent bug on the trunk. This is because while the
> combiner takes the same path on the trunk, the address expression is not a
> pre_inc there - the trunk wants to keep the addressing to a mem (plus (reg)
> (const)) and therefore combine_simplify_rtx () simplifies the subreg 
> expression
> right down to the narrowed memref i.e. mem:QI (plus:DI (reg:DI) (const)). This
> gets combined to insn 89 to become
>
> (insn 89 87 90 4 (set (reg:SI 155)
> (ashift:SI (reg:SI 158) (mem:QI (plus:DI (reg:DI) (const
>
> This then, gets later checked with recog () and since the predicate for 
> operand
> 2 does not allow memory operands for shifts in aarch64.md, does not get 
> combined
> and all is well.
>
> After the discussions Richard Earnshaw had on IRC with Andrew Pinski, it was
> felt that it was best to fix the standard predicate
> 'general_operand' to not allow SUBREG (MEM) with side-effects as it has known
> issues associated with it - particularly reload not being able to deal with 
> such
> cases. 'general_operand' currently outlaws cases like paradoxical SUBREG (MEM)
> on targets wit

Re: Reduce complette unrolling & peeling limits

[Dominique Dhumieres]

> Did you notice that gcc.c-torture/compile/pr43186.c regressed?  It now again
> takes a while to compile, so times out on slow machines:
> ...

On a 2.5Ghz Core2Duo, compiling the test with revision 192891 (2012-10-28)
takes a small fraction of a second, while with revision 193270 (2012-11-06)
it takes ~25s.

However this patch makes gfortran.dg/reassoc_4.f to fail

FAIL: gfortran.dg/reassoc_4.f  -O   scan-tree-dump-times reassoc1 "[0-9] * 
" 22

After it 22 should be replaced with 16 (thresshold 
max-completely-peeled-insns=138
gives 16, =139 gives 22).

Dominique

Re: VEC re-write [patch 05/25]

[Diego Novillo]

On Fri, Nov 16, 2012 at 6:29 PM, Ian Lance Taylor  wrote:
> On Thu, Nov 15, 2012 at 1:53 PM, Diego Novillo  wrote:
>> 2012-11-15  Diego Novillo  
>>
>> Adjust for new vec API 
>> (http://gcc.gnu.org/wiki/cxx-conversion/cxx-vec)
>>
>> * c-common.c: Use new vec API in vec.h.
>> * c-common.h: Likewise.
>> * c-gimplify.c: Likewise.
>> * c-pragma.c: Likewise.
>> * c-pretty-print.c: Likewise.
>> * c-pretty-print.h: Likewise.
>> * c-semantics.c: Likewise.
>> * c-decl.c: Likewise.
>> * c-parser.c: Likewise.
>> * c-tree.h: Likewise.
>> * c-typeck.c: Likewise.
>
>
>>  {
>>gcc_assert (decl && DECL_P (decl) && TREE_STATIC (decl));
>>
>> -  while (!VEC_empty (tree, types_used_by_cur_var_decl))
>> +  while (types_used_by_cur_var_decl && 
>> !types_used_by_cur_var_decl->is_empty ())
>
> vec_safe_is_empty?

Strictly speaking, yes.  But in this case, the call to ->is_empty() is
already protected by a non-NULL test for types_used_by_cur_var_decl.
So you can save yourself the duplicated test.


Diego.

Re: PATCH: Add --with-build-config=bootstrap-asan support

[Paolo Bonzini]

Il 18/11/2012 00:54, H.J. Lu ha scritto:
> +@if gcc-bootstrap
> +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
> +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
> +endif
> +@endif gcc-bootstrap

Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
bootstrapping, and they are defined with "=" so the definition could be
placed (or so it seems at first look) in bootstrap-asan.mk.

Paolo

>  # This is the list of variables to export in the environment when
>  # configuring any subdirectory.  It must also be exported whenever
>  # recursing into a build directory in case that directory's Makefile
> @@ -242,6 +248,7 @@ POSTSTAGE1_CXX_EXPORT = \
> -B$$r/$(HOST_SUBDIR)/prev-gcc/ -B$(build_tooldir)/bin/ -nostdinc++ \
> -B$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/src/.libs \
> -B$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/libsupc++/.libs \
> +   $(LIBASAN_LIBS) \
> -I$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/include/$(TARGET_SUBDIR) \
> -I$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/include \
> -I$$s/libstdc++-v3/libsupc++ \
> @@ -256,6 +263,7 @@ POSTSTAGE1_HOST_EXPORTS = \
>   $(HOST_EXPORTS) \
>   CC="$(STAGE_CC_WRAPPER) $$r/$(HOST_SUBDIR)/prev-gcc/xgcc$(exeext) \
> -B$$r/$(HOST_SUBDIR)/prev-gcc/ -B$(build_tooldir)/bin/ \
> +   $(LIBASAN_LIBS) \
> $(XGCC_FLAGS_FOR_TARGET) $$TFLAGS"; export CC; \
>   CC_FOR_BUILD="$$CC"; export CC_FOR_BUILD; \
>   $(POSTSTAGE1_CXX_EXPORT) \

Re: libquadmath: Update I/O parts from GLIBC

[Joseph S. Myers]

On Sun, 18 Nov 2012, Tobias Burnus wrote:

> I intent to commit the attached patch, but wouldn't mind if someone had first
> a look at the configure/host handling for the rounding.

The default in libquadmath should be to use fegetround to get the rounding 
mode, not fpu_control.h or other architecture-specific code.  Most of the 
extra complexity in glibc is to get it from libc without depending on libm 
(since fegetround is in libm), but libquadmath depends on libm anyway.

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

Re: PATCH: Add --with-build-config=bootstrap-asan support

[H.J. Lu]

On Sun, Nov 18, 2012 at 7:28 AM, Paolo Bonzini  wrote:
> Il 18/11/2012 00:54, H.J. Lu ha scritto:
>> +@if gcc-bootstrap
>> +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
>> +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
>> +endif
>> +@endif gcc-bootstrap
>
> Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
> bootstrapping, and they are defined with "=" so the definition could be
> placed (or so it seems at first look) in bootstrap-asan.mk.
>

It works. I will post a new patch to include a workaround for

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55380

Thanks.


-- 
H.J.

Re: PATCH: Add --with-build-config=bootstrap-asan support

[Hans-Peter Nilsson]

On Sun, 18 Nov 2012, H.J. Lu wrote:
> On Sun, Nov 18, 2012 at 7:28 AM, Paolo Bonzini  wrote:
> > Il 18/11/2012 00:54, H.J. Lu ha scritto:
> >> +@if gcc-bootstrap
> >> +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
> >> +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
> >> +endif
> >> +@endif gcc-bootstrap
> >
> > Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
> > bootstrapping, and they are defined with "=" so the definition could be
> > placed (or so it seems at first look) in bootstrap-asan.mk.
> >
>
> It works. I will post a new patch to include a workaround for
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55380
>
> Thanks.

Nice, but I agree with the other poster that this'd IMHO be
better as --enable-checking=asan (or actually,
--enable-checking=all,asan).

brgds, H-P

Re: PATCH: Add --with-build-config=bootstrap-asan support

[Paolo Bonzini]

Il 18/11/2012 16:59, Hans-Peter Nilsson ha scritto:
> On Sun, 18 Nov 2012, H.J. Lu wrote:
>> On Sun, Nov 18, 2012 at 7:28 AM, Paolo Bonzini  wrote:
>>> Il 18/11/2012 00:54, H.J. Lu ha scritto:
 +@if gcc-bootstrap
 +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
 +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
 +endif
 +@endif gcc-bootstrap
>>>
>>> Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
>>> bootstrapping, and they are defined with "=" so the definition could be
>>> placed (or so it seems at first look) in bootstrap-asan.mk.
>>>
>>
>> It works. I will post a new patch to include a workaround for
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55380
>>
>> Thanks.
> 
> Nice, but I agree with the other poster that this'd IMHO be
> better as --enable-checking=asan (or actually,
> --enable-checking=all,asan).

Yeah, that's a good thing to support too.  However, you still need a
toplevel build configuration to enable asan post-stage1.  Configuring
with --enable-checking=all,asan would require the bootstrap compiler to
support asan, too.

Paolo

Re: PATCH: Add --with-build-config=bootstrap-asan support

[H.J. Lu]

On Sun, Nov 18, 2012 at 7:59 AM, Hans-Peter Nilsson  wrote:
> On Sun, 18 Nov 2012, H.J. Lu wrote:
>> On Sun, Nov 18, 2012 at 7:28 AM, Paolo Bonzini  wrote:
>> > Il 18/11/2012 00:54, H.J. Lu ha scritto:
>> >> +@if gcc-bootstrap
>> >> +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
>> >> +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
>> >> +endif
>> >> +@endif gcc-bootstrap
>> >
>> > Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
>> > bootstrapping, and they are defined with "=" so the definition could be
>> > placed (or so it seems at first look) in bootstrap-asan.mk.
>> >
>>
>> It works. I will post a new patch to include a workaround for
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55380
>>
>> Thanks.
>
> Nice, but I agree with the other poster that this'd IMHO be
> better as --enable-checking=asan (or actually,
> --enable-checking=all,asan).
>

ASAN isn't a checking feature enabled in GCC source
without any compiler requirements.  It is a compiler
feature, similar to LTO. For example, you can't enable
ASAN in stage 1 since stage 1 compiler may not support
ASAN.  I believe  --with-build-config=bootstrap-asan
is the best fit.

-- 
H.J.

Re: Reduce complette unrolling & peeling limits

[Jan Hubicka]

> > this patch reduces max-peeled-insns and max-completely-peeled-insns from 400
> > to 100.  The reason why I am doing this is that I want to reduce code bloat
> > caused by my cunroll work that enabled a lot more unrolling then previously
> > causing considerable code size regression at -O3.
> 
> Did you notice that gcc.c-torture/compile/pr43186.c regressed?  It now again 
> takes a while to compile, so times out on slow machines:

I did not :(.  I am currently on a trip, but will take a look on tuesday.
If it seems to disturb testing, please just revert the patch for time being.

Honza

Re: [PATCH, PR 55260] Use info of the correct node in find_aggregate_values_for_callers_subset

[Jan Hubicka]

> Hi,
> 
> PR 55260 contains two testcases which trigger two different bugs in
> ipa-cp.c.  The first problem is that at one place in
> find_aggregate_values_for_callers_subset we use variable info instead
> of caller_info when determining whether a callee is a clone which then
> leads to an ICE.  Fixed thusly and I also renamed info to dest_info to
> make similar mistakes harder to make in future.
> 
> Fix of the second PR 55260 bug will probably involve some fiddling
> with vectors so I'll submit it next week when the new interface is in
> place.
> 
> Bootstrapped and tested on x86_64-linux, OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2012-11-16  Martin Jambor  
> 
>   PR tree-optimization/55260
>   * ipa-cp.c (find_aggregate_values_for_callers_subset): Rename info to
>   dest_info, use caller_info instead of info when determining whether
>   callee is a clone.
> 
>   * testsuite/g++.dg/torture/pr55260-1.C: New test.

OK, thanks!
Honza

Re: VEC re-write [patch 01/25]

[Hans-Peter Nilsson]

On Sat, 17 Nov 2012, Diego Novillo wrote:
> I have now committed all 25 parts of this patch as rev 193595.  Please
> CC me on any problems that you think may be related to this rewrite.

That seems to have trigged some bug in gcc-4.4-era.  See
PR55381.  There are a lot of suspicious warnings from vec.h.
It smells a bit like a host gcc bug, but I'll have to find a
newer version where it builds to confirm.  (If so, "hopefully"
it's as "simple" as upping the minimum host gcc version or
blacklisting gcc-4.4.x.)

brgds, H-P

Re: Reduce complette unrolling & peeling limits

[Jan Hubicka]

> > > this patch reduces max-peeled-insns and max-completely-peeled-insns from 
> > > 400
> > > to 100.  The reason why I am doing this is that I want to reduce code 
> > > bloat
> > > caused by my cunroll work that enabled a lot more unrolling then 
> > > previously
> > > causing considerable code size regression at -O3.
> > 
> > Did you notice that gcc.c-torture/compile/pr43186.c regressed?  It now 
> > again 
> > takes a while to compile, so times out on slow machines:
> 
> I did not :(.  I am currently on a trip, but will take a look on tuesday.
> If it seems to disturb testing, please just revert the patch for time being.

OK, here are multiple issues.
1) recursive inlining makes huge loop nest (of 18 loops)
2) SCEV is very slow on answering simple_iv tests in this case becuase it walks
   the nest
3) unroller is computing loop body size even when it is clear the body is much 
larger
   than the limit (the outer loop has 78000 instructions)

I will prepare patches to fix those issues. 

Honza
> 
> Honza

Re: VEC re-write [patch 01/25]

[Diego Novillo]

On Sun, Nov 18, 2012 at 12:05 PM, Hans-Peter Nilsson  wrote:
> On Sat, 17 Nov 2012, Diego Novillo wrote:
>> I have now committed all 25 parts of this patch as rev 193595.  Please
>> CC me on any problems that you think may be related to this rewrite.
>
> That seems to have trigged some bug in gcc-4.4-era.  See
> PR55381.  There are a lot of suspicious warnings from vec.h.
> It smells a bit like a host gcc bug, but I'll have to find a
> newer version where it builds to confirm.  (If so, "hopefully"
> it's as "simple" as upping the minimum host gcc version or
> blacklisting gcc-4.4.x.)

Yeah, I got those warnings in my sparc and hppa builds, but they are
harmless.  Strictly speaking offsetof cannot be applied to non-PODs.
The only thing that makes that class non-POD is the protected
attribute, but that does not alter the physical layout.  So the
compiler is emitting a harmless warning (newer versions have
tightened the check to warn when you are using offsetof on a non-base
class).

My cris-elf builds worked fine, but config-list.mk only builds stage
1, it does not build libgfortran.  Can you give me instructions on how
to build your target on my x86 workstation?


Thanks.  Diego.

Re: [4/8] Add bit_field_mode_iterator

[Richard Sandiford]

Richard Henderson  writes:
> On 11/15/2012 04:10 AM, Richard Sandiford wrote:
>> "next" was supposed to be "find and return another mode" rather than "++".
>> Did you think it was confusing because "next" sounded too much like
>> the latter?
>
> I wasn't keen on "next" being find-and-return, though I didn't
> actually find it confusing.  And perhaps rather than bikeshed
> this too much now, we should table this for revision in 4.9...
>
>> I hadn't thought about an operator bool terminator.  I agree that's
>> probably simpler, but do any libstdc++ classes have the same thing?
>> It doesn't feel any more standard than the "while (get_more)" idiom to me,
>> but that's probably just my ignorance of C++.
>
> ... when we can attack all the iterators.
>
> No, you're right that operator bool as a terminator isn't standard.
> Though for many purposes it seems better than the "!= fake_end_object"
> semantics that we'd have to use otherwise.
>
> That's a discussion that we should have generally as we find our 
> feet with C++ in GCC.
>
> Unless Eric has any strong objections, I think this patch is ok.
> And thus the entire patch set, as I havn't seen anything else that
> raises a red flag.

Thanks.  Committed with the changes Eric asked for after retesting
on x86_64-linux-gnu, powerpc64-linux-gnu and mipsisa64-elf.

Richard

Re: [4/8] Add bit_field_mode_iterator

[Richard Sandiford]

Eric Botcazou  writes:
>> get_best_mode has various checks to decide what counts as an acceptable
>> bitfield mode.  It actually has two copies of them, with slightly different
>> alignment checks:
>> 
>>   MIN (unit, BIGGEST_ALIGNMENT) > align
>> 
>> vs.
>> 
>>   unit <= MIN (align, BIGGEST_ALIGNMENT)
>> 
>> The second looks more correct, since we can't necessarily guarantee
>> larger alignments than BIGGEST_ALIGNMENT in all cases.
>
> Under the assumption that integer modes really require maximal
> alignment, i.e.  whatever BIGGEST_ALIGNMENT is, I agree.
>
>> This patch adds a new iterator class that can be used to walk through
>> the modes, and rewrites get_best_mode to use it.  I kept the existing
>> checks with two changes:
>> 
>> - bitregion_start is now tested independently of bitregion_end
>
> The comments needs to be updated then.
>
>> +  volatilep_ (volatilep), count_ (0)
>> +{
>> +  if (bitregion_end_)
>> +bitregion_end_ += 1;
>> +}
>
> IMO this is confusing.  I think bitregion_end/bitregion_end_ should have a 
> consistent meaning.
>
>> +/* Calls to this function return successively larger modes that can be used
>> +   to represent the bitfield.  Return true if another bitfield mode is +  
>> available, storing it in *OUT_MODE if so.  */
>> +
>> +bool bit_field_mode_iterator::next_mode (enum machine_mode *out_mode)
>
> 'bool' on its own line I think.

Thanks.  Here's the version I committed.

Richard


gcc/
* machmode.h (bit_field_mode_iterator): New class.
(get_best_mode): Change final parameter to bool.
* stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator)
(bit_field_mode_iterator::next_mode): New functions, split out from...
(get_best_mode): ...here.  Change final parameter to bool.
Use bit_field_mode_iterator.

Index: gcc/machmode.h
===
--- gcc/machmode.h  2012-11-18 17:22:01.515895170 +
+++ gcc/machmode.h  2012-11-18 17:22:43.313844317 +
@@ -259,13 +259,36 @@ extern enum machine_mode int_mode_for_mo
 
 extern enum machine_mode mode_for_vector (enum machine_mode, unsigned);
 
+/* A class for iterating through possible bitfield modes.  */
+class bit_field_mode_iterator
+{
+public:
+  bit_field_mode_iterator (HOST_WIDE_INT, HOST_WIDE_INT,
+  HOST_WIDE_INT, HOST_WIDE_INT,
+  unsigned int, bool);
+  bool next_mode (enum machine_mode *);
+  bool prefer_smaller_modes ();
+
+private:
+  enum machine_mode mode_;
+  /* We use signed values here because the bit position can be negative
+ for invalid input such as gcc.dg/pr48335-8.c.  */
+  HOST_WIDE_INT bitsize_;
+  HOST_WIDE_INT bitpos_;
+  HOST_WIDE_INT bitregion_start_;
+  HOST_WIDE_INT bitregion_end_;
+  unsigned int align_;
+  bool volatilep_;
+  int count_;
+};
+
 /* Find the best mode to use to access a bit field.  */
 
 extern enum machine_mode get_best_mode (int, int,
unsigned HOST_WIDE_INT,
unsigned HOST_WIDE_INT,
unsigned int,
-   enum machine_mode, int);
+   enum machine_mode, bool);
 
 /* Determine alignment, 1<=result<=BIGGEST_ALIGNMENT.  */
 
Index: gcc/stor-layout.c
===
--- gcc/stor-layout.c   2012-11-18 17:22:37.791271752 +
+++ gcc/stor-layout.c   2012-11-18 17:26:31.159273478 +
@@ -2624,14 +2624,103 @@ fixup_unsigned_type (tree type)
   layout_type (type);
 }
 
+/* Construct an iterator for a bitfield that spans BITSIZE bits,
+   starting at BITPOS.
+
+   BITREGION_START is the bit position of the first bit in this
+   sequence of bit fields.  BITREGION_END is the last bit in this
+   sequence.  If these two fields are non-zero, we should restrict the
+   memory access to that range.  Otherwise, we are allowed to touch
+   any adjacent non bit-fields.
+
+   ALIGN is the alignment of the underlying object in bits.
+   VOLATILEP says whether the bitfield is volatile.  */
+
+bit_field_mode_iterator
+::bit_field_mode_iterator (HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos,
+  HOST_WIDE_INT bitregion_start,
+  HOST_WIDE_INT bitregion_end,
+  unsigned int align, bool volatilep)
+: mode_ (GET_CLASS_NARROWEST_MODE (MODE_INT)), bitsize_ (bitsize),
+  bitpos_ (bitpos), bitregion_start_ (bitregion_start),
+  bitregion_end_ (bitregion_end), align_ (MIN (align, BIGGEST_ALIGNMENT)),
+  volatilep_ (volatilep), count_ (0)
+{
+}
+
+/* Calls to this function return successively larger modes that can be used
+   to represent the bitfield.  Return true if another bitfield mode is
+   available, storing it in *OUT_MODE if so.  */
+
+bool
+bit_field_mode_iterator::next_mode (enum machine_mode *out_mode)
+{
+  for (; mode_ != VOIDmod

Re: [5/8] Tweak bitfield alignment handling

[Richard Sandiford]

Eric Botcazou  writes:
>> This patch replaces:
>> 
>>   /* Stop if the mode is wider than the alignment of the containing
>>   object.
>> 
>>   It is tempting to omit the following line unless STRICT_ALIGNMENT
>>   is true.  But that is incorrect, since if the bitfield uses part
>>   of 3 bytes and we use a 4-byte mode, we could get a spurious segv
>>   if the extra 4th byte is past the end of memory.
>>   (Though at least one Unix compiler ignores this problem:
>>   that on the Sequent 386 machine.  */
>>   if (unit > align_)
>>  break;
>> 
>> with two checks: one for whether the final byte of the mode is known
>> to be mapped, and one for whether the bitfield is sufficiently aligned.
>> Later patches in the series rely on this in order not to pessimise
>> memory handling.
>> 
>> However, as described in the patch, I found that extending this
>> behaviour to get_best_mode affected code generation for x86_64-linux-gnu
>> and powerpc64-linux-gnu, and led to a failure in bb-slp-5.c on both.
>> I therefore chickened out and added the original check back to
>> get_best_mode.
>> 
>> I'm certainly interested in any feedback on the comment, but at the
>> same time I'd like this series to be a no-op on targets that keep
>> the traditional .md patterns.  Any change to get_best_mode is probably
>> best done as a separate patch.
>
> I agree with your conservative approach here.
>
>> gcc/
>>  * stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator):
>>  Set up a default value of bitregion_end_.
>>  (bit_field_mode_iterator::next_mode): Always apply bitregion_end_
>>  check.  Include SLOW_UNALIGNED_ACCESS in the alignment check.
>>  (get_best_mode): Ignore modes that are wider than the alignment.
>
> Fine with me, modulo:
>
>> @@ -2754,6 +2753,62 @@ get_best_mode (int bitsize, int bitpos,
>>enum machine_mode widest_mode = VOIDmode;
>>enum machine_mode mode;
>>while (iter.next_mode (&mode)
>> + /* ??? For compatiblity, reject modes that are wider than the
>> +alignment.  This has both advantages and disadvantages.
>
> "For compatibility" is a bit vague (compatibility with what?).  I'd write 
> "For 
> historical reasons" or something along these lines.

Yeah, that's better.  Here's what I committed after updating to the patch
for the inclusive bitregion_end_.

Richard


gcc/
* stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator):
Set up a default value of bitregion_end_.
(bit_field_mode_iterator::next_mode): Always apply bitregion_end_
check.  Include SLOW_UNALIGNED_ACCESS in the alignment check.
(get_best_mode): Ignore modes that are wider than the alignment.

Index: gcc/stor-layout.c
===
--- gcc/stor-layout.c   2012-11-18 17:26:31.159273478 +
+++ gcc/stor-layout.c   2012-11-18 17:28:17.481177253 +
@@ -2646,6 +2646,13 @@ fixup_unsigned_type (tree type)
   bitregion_end_ (bitregion_end), align_ (MIN (align, BIGGEST_ALIGNMENT)),
   volatilep_ (volatilep), count_ (0)
 {
+  if (!bitregion_end_)
+{
+  /* We can assume that any aligned chunk of ALIGN_ bits that overlaps
+the bitfield is mapped and won't trap.  */
+  bitregion_end_ = bitpos + bitsize + align_ - 1;
+  bitregion_end_ -= bitregion_end_ % align_ + 1;
+}
 }
 
 /* Calls to this function return successively larger modes that can be used
@@ -2676,23 +2683,15 @@ bit_field_mode_iterator::next_mode (enum
   if (count_ > 0 && unit > BITS_PER_WORD)
break;
 
-  /* Stop if the mode is wider than the alignment of the containing
-object.
-
-It is tempting to omit the following line unless STRICT_ALIGNMENT
-is true.  But that is incorrect, since if the bitfield uses part
-of 3 bytes and we use a 4-byte mode, we could get a spurious segv
-if the extra 4th byte is past the end of memory.
-(Though at least one Unix compiler ignores this problem:
-that on the Sequent 386 machine.  */
-  if (unit > align_)
-   break;
-
   /* Stop if the mode goes outside the bitregion.  */
   HOST_WIDE_INT start = bitpos_ - (bitpos_ % unit);
   if (bitregion_start_ && start < bitregion_start_)
break;
-  if (bitregion_end_ && start + unit > bitregion_end_ + 1)
+  if (start + unit > bitregion_end_ + 1)
+   break;
+
+  /* Stop if the mode requires too much alignment.  */
+  if (unit > align_ && SLOW_UNALIGNED_ACCESS (mode_, align_))
break;
 
   *out_mode = mode_;
@@ -2751,6 +2750,62 @@ get_best_mode (int bitsize, int bitpos,
   enum machine_mode widest_mode = VOIDmode;
   enum machine_mode mode;
   while (iter.next_mode (&mode)
+/* ??? For historical reasons, reject modes that are wider than
+   the alignment.  This has both advantages and disadvantages.
+   Removing this check means that something like:
+

Re: [6/8] Add strict volatile handling to bit_field_mode_iterator

[Richard Sandiford]

Eric Botcazou  writes:
>> OK.  The current recursive force-mem-to-reg cases in store_bit_field_1
>> and extract_bit_field_1 don't handle -fstrict-volatile-bitfields at all,
>> so this patch was trying to fix what seemed like an oversight.  Is it OK
>> to leave the code as-is (not handling -fstrict-volatile-bitfields),
>> or do I need to add new code to the expmed.c routines?
>
> The former, I think.

OK, I left this patch out and removed the associated constructor argument
from patch 7.

Richard

Re: VEC re-write [patch 01/25]

[Hans-Peter Nilsson]

On Sun, 18 Nov 2012, Diego Novillo wrote:
> My cris-elf builds worked fine, but config-list.mk only builds stage
> 1, it does not build libgfortran.  Can you give me instructions on how
> to build your target on my x86 workstation?

Better see the PR for cc1 command-line and preprocessed C-file.

brgds, H-P

Re: VEC re-write [patch 01/25]

[Diego Novillo]

On Sun, Nov 18, 2012 at 12:43 PM, Hans-Peter Nilsson  wrote:
> On Sun, 18 Nov 2012, Diego Novillo wrote:
>> My cris-elf builds worked fine, but config-list.mk only builds stage
>> 1, it does not build libgfortran.  Can you give me instructions on how
>> to build your target on my x86 workstation?
>
> Better see the PR for cc1 command-line and preprocessed C-file.

Ah, I had missed that.  Thanks.


Diego.

Re: VEC re-write [patch 01/25]

[Andreas Tobler]

On 18.11.12 18:05, Hans-Peter Nilsson wrote:
> On Sat, 17 Nov 2012, Diego Novillo wrote:
>> I have now committed all 25 parts of this patch as rev 193595.  Please
>> CC me on any problems that you think may be related to this rewrite.
> 
> That seems to have trigged some bug in gcc-4.4-era.  See
> PR55381.  There are a lot of suspicious warnings from vec.h.
> It smells a bit like a host gcc bug, but I'll have to find a
> newer version where it builds to confirm.  (If so, "hopefully"
> it's as "simple" as upping the minimum host gcc version or
> blacklisting gcc-4.4.x.)

Is there a minimum gcc to bootstrap current trunk?
I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6
succeeds. I see similar warnings as in the PR. But here on x86_64
FreeBSD genautomata gives a bus error.

build/genautomata
/export/devel/net/src/gcc/head/gcc/gcc/config/i386/i386.md \
  insn-conditions.md > tmp-automata.c
gmake[3]: *** [s-automata] Bus error (core dumped)

Andreas

Re: Reduce complette unrolling & peeling limits

[Eric Botcazou]

> OK, here are multiple issues.
> 1) recursive inlining makes huge loop nest (of 18 loops)
> 2) SCEV is very slow on answering simple_iv tests in this case becuase it
> walks the nest
> 3) unroller is computing loop body size even when it is clear the body is
> much larger than the limit (the outer loop has 78000 instructions)
> 
> I will prepare patches to fix those issues.

Thanks for the analysis (and don't worry, I won't revert anything :-).

-- 
Eric Botcazou

Re: [tsan] ThreadSanitizer instrumentation part

[Konstantin Serebryany]

Just a comment about tsan.
Today, tsan works *only* on x86_64 linux (no 32-bits, no non-linux).
Other 64-bit platforms may be doable, but not as easy as for asan.
Non-linux is harder than non-x86_64 (need to support tons of libc interceptors).
32-bit platforms are very hard to port to, I would not bother for now.
(this probably includes x32, which has cheap atomic 64-bit
loads/stores, but has too small address space for tsan)

Conclusion: when committing tsan code, please make sure it is enable
only on x86_64

--kcc

On Sat, Nov 17, 2012 at 3:13 AM, Wei Mi  wrote:
> Hi,
>
> Is it ok for the trunk?
>
> Thanks,
> Wei.
>
> On Tue, Nov 13, 2012 at 5:06 PM, Wei Mi  wrote:
>> Thanks for catching this. I update the patch.
>>
>> Regards,
>> Wei.
>>
>> On Tue, Nov 13, 2012 at 4:54 PM, Richard Henderson  wrote:
>>> On 11/13/2012 04:08 PM, Wei Mi wrote:
 +extern void tsan_finish_file (void);
 +
 +#endif /* TREE_TSAN */
 +/* ThreadSanitizer, a data race detector.
 +   Copyright (C) 2011 Free Software Foundation, Inc.
 +   Contributed by Dmitry Vyukov 
>>>
>>> Careful, you've got double applied patches there.
>>>
>>>
>>> r~

Re: VEC re-write [patch 01/25]

[Hans-Peter Nilsson]

On Sun, 18 Nov 2012, Andreas Tobler wrote:
> Is there a minimum gcc to bootstrap current trunk?
> I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6
> succeeds.

A gcc-4.1.2 has worked for me in the past, before this recent
vec.h change.  I think that's the minimum version reportedly
working.

brgds, H-P

Re: VEC re-write [patch 01/25]

[Andrew Pinski]

On Sun, Nov 18, 2012 at 9:12 AM, Diego Novillo  wrote:
> On Sun, Nov 18, 2012 at 12:05 PM, Hans-Peter Nilsson  
> wrote:
>> On Sat, 17 Nov 2012, Diego Novillo wrote:
>>> I have now committed all 25 parts of this patch as rev 193595.  Please
>>> CC me on any problems that you think may be related to this rewrite.
>>
>> That seems to have trigged some bug in gcc-4.4-era.  See
>> PR55381.  There are a lot of suspicious warnings from vec.h.
>> It smells a bit like a host gcc bug, but I'll have to find a
>> newer version where it builds to confirm.  (If so, "hopefully"
>> it's as "simple" as upping the minimum host gcc version or
>> blacklisting gcc-4.4.x.)
>
> Yeah, I got those warnings in my sparc and hppa builds, but they are
> harmless.  Strictly speaking offsetof cannot be applied to non-PODs.
> The only thing that makes that class non-POD is the protected
> attribute, but that does not alter the physical layout.  So the
> compiler is emitting a harmless warning (newer versions have
> tightened the check to warn when you are using offsetof on a non-base
> class).

But then we are no longer writing in C++.   Is there a reason why this
warning does not break the build?  We should not be using offsetof
with non-PODs.

Thanks,
Andrew Pinski

>
> My cris-elf builds worked fine, but config-list.mk only builds stage
> 1, it does not build libgfortran.  Can you give me instructions on how
> to build your target on my x86 workstation?
>
>
> Thanks.  Diego.

Re: VEC re-write [patch 01/25]

[Andreas Tobler]

On 18.11.12 20:11, Hans-Peter Nilsson wrote:
> On Sun, 18 Nov 2012, Andreas Tobler wrote:
>> Is there a minimum gcc to bootstrap current trunk?
>> I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6
>> succeeds.
> 
> A gcc-4.1.2 has worked for me in the past, before this recent
> vec.h change.  I think that's the minimum version reportedly
> working.

Up to 193594 a 4.2.1 did it fine on most of my platforms. But with
193595 it seems that I have to use a more recent gcc to bootstrap.

Andreas

Re: VEC re-write [patch 01/25]

[Hans-Peter Nilsson]

On Sun, 18 Nov 2012, Andreas Tobler wrote:

> On 18.11.12 20:11, Hans-Peter Nilsson wrote:
> > On Sun, 18 Nov 2012, Andreas Tobler wrote:
> >> Is there a minimum gcc to bootstrap current trunk?
> >> I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6
> >> succeeds.
> >
> > A gcc-4.1.2 has worked for me in the past, before this recent
> > vec.h change.  I think that's the minimum version reportedly
> > working.
>
> Up to 193594 a 4.2.1 did it fine on most of my platforms. But with
> 193595 it seems that I have to use a more recent gcc to bootstrap.

Yes, that's the vec.h-change revision.  I hope we can revert
that behavior (the *behavior*, not the patch).

brgds, H-P

Re: VEC re-write [patch 01/25]

[David Edelsohn]

Files were changed in gcc/c-family with no associated ChangeLog entry.
 And now bootstrap fails on AIX using GCC 4.6.3 with the error:

/nasfarm/dje/src/src/gcc/c-family/c-lex.c: In function 'c_fileinfo*
get_fileinfo(const char*)':
/nasfarm/dje/src/src/gcc/c-family/c-lex.c:107:39: error: overloaded
function with no contextual type information

which is

file_info_tree = splay_tree_new ((splay_tree_compare_fn) strcmp,
 0,
 (splay_tree_delete_value_fn) free);

- David

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Dominique Dhumieres]

> I think this should fix it. Can't test it right now, so help
> appreciated (Honza: hint hint! ;-)

The change at 
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01511/remove_dead_eq_notes.diff
(revision 192526) caused

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55006

Dominique

Re: PATCH: Add --with-build-config=bootstrap-asan support

[Hans-Peter Nilsson]

On Sun, 18 Nov 2012, Paolo Bonzini wrote:
> Il 18/11/2012 16:59, Hans-Peter Nilsson ha scritto:
> > Nice, but I agree with the other poster that this'd IMHO be
> > better as --enable-checking=asan (or actually,
> > --enable-checking=all,asan).
>
> Yeah, that's a good thing to support too.  However, you still need a
> toplevel build configuration to enable asan post-stage1.  Configuring
> with --enable-checking=all,asan would require the bootstrap compiler to
> support asan, too.

We seem to be able to add various flags to compilations *after*
the first stage (that are not valid at the first stage, for the
bootstrapping/host compiler) so I still don't get why this
should be conceptually different.  But whatever.

brgds, H-P

Keeping config.{guess,sub}, Makefile.* configure{m.*} etc. in sync

[Jan-Benedict Glaw]

Hi!

There was a discussion on the binutils mailing list recently, because
shared files were out of sync and a merge error showed up afterwards.

That made my write a small script to check the shared files between
the `src' and `gcc' repos, as well as the `config' repo to both of
them.

The consensus was that for the mentioned shared files between `src'
and `gcc', a global maintainer's ACK qualifies to sync changes between
these two repos, which I volunteer for.


Another story are config.{guess,sub} from the `config' repo. For GCC,
both are out of sync right now, missing a (small) number of commits.
How shall these files be handled? Send all changes separate to
gcc-patches, asking them to be ACKed? Or only import those when they
were actually needed? (And if they should be regularly imported, what
author and date attribution should be put into GCC's ChangeLog?)

Thanks, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
 Signature of:Don't believe in miracles: Rely on them!
 the second  :


signature.asc
Description: Digital signature

Re: Keeping config.{guess,sub}, Makefile.* configure{m.*} etc. in sync

[Andrew Pinski]

On Sun, Nov 18, 2012 at 4:37 PM, Jan-Benedict Glaw  wrote:
> Hi!
>
> There was a discussion on the binutils mailing list recently, because
> shared files were out of sync and a merge error showed up afterwards.
>
> That made my write a small script to check the shared files between
> the `src' and `gcc' repos, as well as the `config' repo to both of
> them.
>
> The consensus was that for the mentioned shared files between `src'
> and `gcc', a global maintainer's ACK qualifies to sync changes between
> these two repos, which I volunteer for.
>
>
> Another story are config.{guess,sub} from the `config' repo. For GCC,
> both are out of sync right now, missing a (small) number of commits.
> How shall these files be handled? Send all changes separate to
> gcc-patches, asking them to be ACKed? Or only import those when they
> were actually needed? (And if they should be regularly imported, what
> author and date attribution should be put into GCC's ChangeLog?)


config.sub and config.guess can be just copied (merged) without
applying each patch I think.  As we don't need to know the history of
them as they have been just merged in rather than each patch applied.
For the others, I don't know what we should do there.


Thanks,
Andrew Pinski

>
> Thanks, JBG
>
> --
>   Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
>  Signature of:Don't believe in miracles: Rely on them!
>  the second  :
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAlCpf00ACgkQHb1edYOZ4bthiACcDgQPpDPVsfi3JmVvrGvBGxB/
> JEMAnAsmA3OMwBZEjyuQP5yGVrTBF1Nx
> =CzKB
> -END PGP SIGNATURE-
>

Re: VEC re-write [patch 01/25]

[Diego Novillo]

On Sun, Nov 18, 2012 at 4:28 PM, David Edelsohn  wrote:
> Files were changed in gcc/c-family with no associated ChangeLog entry.

*gasp*

I mistakenly put them in c/ChangeLog.  Not that they carry any useful
information, but I'll move them.

>  And now bootstrap fails on AIX using GCC 4.6.3 with the error:
>
> /nasfarm/dje/src/src/gcc/c-family/c-lex.c: In function 'c_fileinfo*
> get_fileinfo(const char*)':
> /nasfarm/dje/src/src/gcc/c-family/c-lex.c:107:39: error: overloaded
> function with no contextual type information
>
> which is
>
> file_info_tree = splay_tree_new ((splay_tree_compare_fn) strcmp,
>  0,
>  (splay_tree_delete_value_fn) free);
>

Odd.  The patch did not touch c-lex.c.


Diego.

Re: VEC re-write [patch 01/25]

[David Edelsohn]

On Sun, Nov 18, 2012 at 8:03 PM, Diego Novillo  wrote:

>>  And now bootstrap fails on AIX using GCC 4.6.3 with the error:
>>
>> /nasfarm/dje/src/src/gcc/c-family/c-lex.c: In function 'c_fileinfo*
>> get_fileinfo(const char*)':
>> /nasfarm/dje/src/src/gcc/c-family/c-lex.c:107:39: error: overloaded
>> function with no contextual type information
>>
>> which is
>>
>> file_info_tree = splay_tree_new ((splay_tree_compare_fn) strcmp,
>>  0,
>>  (splay_tree_delete_value_fn) free);
>>
>
> Odd.  The patch did not touch c-lex.c.

I opened PR 55384 to track this

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55384

The problem is AIX stdlib.h defines

#define vec_free free

and vec.h

defines two functions named vec_free()

With the VEC changes, these now are renamed "free" and lead to
ambiguous name resolution.

I am not sure where

#undef vec_free

should be placed.  In vec.h or system.h?

Thanks, David

RFA: patch to fix PR19398

[Vladimir Makarov]

The following patch fixes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19398

Uros, there is i386.md part for which I need an approval.  Without this 
change, GCC will still generate the same code even if LRA uses an 
alternative with 'm' constraint.


2012-11-18  Vladimir Makarov  

PR target/19398
* lra-constraints.c (process_alt_operands): Discourage reloads
through secodnary memory.
* config/i386/i386.md (fix_trunc?f?i_sse): Remove peephole2
patterns.

Index: lra-constraints.c
===
--- lra-constraints.c   (revision 193567)
+++ lra-constraints.c   (working copy)
@@ -1942,6 +1942,19 @@
  if (no_regs_p && REG_P (op))
reject++;
 
+#ifdef SECONDARY_MEMORY_NEEDED
+ /* If reload requires moving value through secondary
+memory, it will need one more insn at least.  */
+ if (this_alternative != NO_REGS 
+ && REG_P (op) && (cl = get_reg_class (REGNO (op))) != NO_REGS
+ && ((curr_static_id->operand[nop].type != OP_OUT
+  && SECONDARY_MEMORY_NEEDED (cl, this_alternative,
+  GET_MODE (op)))
+ || (curr_static_id->operand[nop].type != OP_IN
+ && SECONDARY_MEMORY_NEEDED (this_alternative, cl,
+ GET_MODE (op)
+   losers++;
+#endif
  /* Input reloads can be inherited more often than output
 reloads can be removed, so penalize output
 reloads.  */
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 193557)
+++ config/i386/i386.md (working copy)
@@ -1875,7 +1875,7 @@
  (const_string "OI")))])
 
 (define_insn "*movti_internal_rex64"
-  [(set (match_operand:TI 0 "nonimmediate_operand" "=!r ,o  ,x,x ,m")
+  [(set (match_operand:TI 0 "nonimmediate_operand" "=!r ,!o  ,x,x ,m")
(match_operand:TI 1 "general_operand"  "riFo,riF,C,xm,x"))]
   "TARGET_64BIT && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
 {
@@ -4503,23 +4503,6 @@
&& peep2_reg_dead_p (2, operands[0])"
   [(set (match_dup 2) (fix:SWI48x (match_dup 1)))])
 
-;; Avoid vector decoded forms of the instruction.
-(define_peephole2
-  [(match_scratch:DF 2 "x")
-   (set (match_operand:SWI48x 0 "register_operand")
-   (fix:SWI48x (match_operand:DF 1 "memory_operand")))]
-  "TARGET_SSE2 && TARGET_AVOID_VECTOR_DECODE && optimize_insn_for_speed_p ()"
-  [(set (match_dup 2) (match_dup 1))
-   (set (match_dup 0) (fix:SWI48x (match_dup 2)))])
-
-(define_peephole2
-  [(match_scratch:SF 2 "x")
-   (set (match_operand:SWI48x 0 "register_operand")
-   (fix:SWI48x (match_operand:SF 1 "memory_operand")))]
-  "TARGET_AVOID_VECTOR_DECODE && optimize_insn_for_speed_p ()"
-  [(set (match_dup 2) (match_dup 1))
-   (set (match_dup 0) (fix:SWI48x (match_dup 2)))])
-
 (define_insn_and_split "fix_trunc_fisttp_i387_1"
   [(set (match_operand:SWI248x 0 "nonimmediate_operand")
(fix:SWI248x (match_operand 1 "register_operand")))]

PR55124 - tentative patch for ICE in PRE

[Tom de Vries]

Richard,

Consider the PR55124 example test.c:
...
int a, b;
long c;

static void inline
f2 (void)
{
  unsigned long k = 1;

  foo (b ? k = 0 : 0);

  b = (((c = b)
? (k ?: (c = 0))
: a)
   * c);
}

void
f1 (void)
{
  f2 ();
  a = b | c;
}
...

when compiling with -O2, we're running into the following assertion in pre:
...
test.c:18:1: internal compiler error: in find_or_generate_expression, at
tree-ssa-pre.c:2802
 f1 (void)
 ^
0xcf41d3 find_or_generate_expression
gcc/tree-ssa-pre.c:2802
0xcf42f6 create_expression_by_pieces
gcc/tree-ssa-pre.c:2861
0xcf4193 find_or_generate_expression
gcc/tree-ssa-pre.c:2799
0xcf42f6 create_expression_by_pieces
gcc/tree-ssa-pre.c:2861
0xcf4e28 insert_into_preds_of_block
gcc/tree-ssa-pre.c:3096
0xcf5c7d do_regular_insertion
gcc/tree-ssa-pre.c:3386
...

We're hitting the assert at the end of find_or_generate_expression:
...
static tree
find_or_generate_expression (basic_block block, tree op, gimple_seq *stmts)
{
  pre_expr expr = get_or_alloc_expr_for (op);
  unsigned int lookfor = get_expr_value_id (expr);
  pre_expr leader = bitmap_find_leader (AVAIL_OUT (block), lookfor);
  if (leader)
{
  if (leader->kind == NAME)
return PRE_EXPR_NAME (leader);
  else if (leader->kind == CONSTANT)
return PRE_EXPR_CONSTANT (leader);
}

  /* It must be a complex expression, so generate it recursively.  */
  bitmap exprset = VEC_index (bitmap, value_expressions, lookfor);
  bitmap_iterator bi;
  unsigned int i;
  EXECUTE_IF_SET_IN_BITMAP (exprset, 0, i, bi)
{
  pre_expr temp = expression_for_id (i);
  if (temp->kind != NAME)
return create_expression_by_pieces (block, temp, stmts,
get_expr_type (expr));
}

  gcc_unreachable ();
}
...

The state in which we're asserting is the following:
...
#5  0x00cf41d4 in find_or_generate_expression (block=0x76210f08,
op=0x762384c8, stmts=0x7fffdb78) at gcc/tree-ssa-pre.c:2802
2802  gcc_unreachable ();
(gdb) p block.index
$13 = 4
(gdb) call debug_generic_expr (op)
b.4_3
(gdb) call debug_pre_expr (expr)
b.4_3
(gdb) p lookfor
$11 = 7
(gdb) call debug_bitmap_set (((bb_value_sets_t) ((block)->aux))->avail_out)
debug[0] := { b.4_8 (0012), a.10_13 (0013), _14 (0014), iftmp.5_15 (0015) }
(gdb) p leader
$12 = (pre_expr) 0x0
(gdb) call debug_bitmap ( exprset )
first = 0x21fb058 current = 0x21fb058 indx = 0
0x21fb058 next = (nil) prev = (nil) indx = 0
bits = { 22 25 }
(gdb) call debug_pre_expr (expression_for_id (22))
b.4_3
(gdb) call debug_pre_expr (expression_for_id (25))
b.4_31
...
We're trying to find or generate an expr with value-id 0007 in block 4. We can't
find it (there's no leader) and we can't generate it because there are no exprs
with that value that are not names.

Higher up in the call stack and skipping create_expression_by_pieces, the state
is as follows:
...
#7  0x00cf4194 in find_or_generate_expression (block=0x76210f08,
op=0x76238558, stmts=0x7fffdb78) at gcc/tree-ssa-pre.c:2799
2799get_expr_type (expr));
(gdb) call debug_generic_expr (op)
c.6_5
(gdb) call debug_pre_expr (expr)
c.6_5
(gdb) p lookfor
$14 = 9
(gdb) p leader
$15 = (pre_expr) 0x0
(gdb) call debug_bitmap ( exprset )
first = 0x21fb0f8 current = 0x21fb0f8 indx = 0
0x21fb0f8 next = (nil) prev = (nil) indx = 0
bits = { 23 24 26 27 }
(gdb) call debug_pre_expr (temp)
{nop_expr,b.4_3}
(gdb) call debug_pre_expr (expression_for_id (23))
c.6_5
(gdb) call debug_pre_expr (expression_for_id (24))
{nop_expr,b.4_3}
(gdb) call debug_pre_expr (expression_for_id (26))
c.6_32
(gdb) call debug_pre_expr (expression_for_id (27))
{mem_ref<0B>,addr_expr<&c>}@.MEM_28
...
We're trying to find or generate an expr with value-id 0009 (in block 4). We
can't find it. We're trying to generate it using {nop_expr,b.4_3}, but as we've
seen above that won't work. The generation using expr 27 would work though.

Again higher up in the call stack and skipping create_expression_by_pieces, the
state is as follows:
...
(gdb) up
#9  0x00cf4e29 in insert_into_preds_of_block (block=0x76210f70,
exprnum=19, avail=0x22102e0) at gcc/tree-ssa-pre.c:3096
3096   &stmts, type);
(gdb) l
3091  eprime = VEC_index (pre_expr, avail, pred->dest_idx);
3092
3093  if (eprime->kind != NAME && eprime->kind != CONSTANT)
3094{
3095  builtexpr = create_expression_by_pieces (bprime, eprime,
3096   &stmts, type);
(gdb) p block.index
$17 = 5
(gdb) call debug_pre_expr (expr)
{convert_expr,c.7_16}
(gdb) p val
$18 = 8
(gdb) call debug_pre_expr (eprime)
{convert_expr,c.6_5}
(gdb) call get_expr_value_id (eprime)
$16 = 26
...
So we're trying to insert expr {convert_expr,c.6_5} with value-id 0026 int

Re: VEC re-write [patch 05/25]

[Ian Lance Taylor]

On Sun, Nov 18, 2012 at 7:09 AM, Diego Novillo  wrote:
> On Fri, Nov 16, 2012 at 6:29 PM, Ian Lance Taylor  wrote:
>> On Thu, Nov 15, 2012 at 1:53 PM, Diego Novillo  wrote:
>>> 2012-11-15  Diego Novillo  
>>>
>>> Adjust for new vec API 
>>> (http://gcc.gnu.org/wiki/cxx-conversion/cxx-vec)
>>>
>>> * c-common.c: Use new vec API in vec.h.
>>> * c-common.h: Likewise.
>>> * c-gimplify.c: Likewise.
>>> * c-pragma.c: Likewise.
>>> * c-pretty-print.c: Likewise.
>>> * c-pretty-print.h: Likewise.
>>> * c-semantics.c: Likewise.
>>> * c-decl.c: Likewise.
>>> * c-parser.c: Likewise.
>>> * c-tree.h: Likewise.
>>> * c-typeck.c: Likewise.
>>
>>
>>>  {
>>>gcc_assert (decl && DECL_P (decl) && TREE_STATIC (decl));
>>>
>>> -  while (!VEC_empty (tree, types_used_by_cur_var_decl))
>>> +  while (types_used_by_cur_var_decl && 
>>> !types_used_by_cur_var_decl->is_empty ())
>>
>> vec_safe_is_empty?
>
> Strictly speaking, yes.  But in this case, the call to ->is_empty() is
> already protected by a non-NULL test for types_used_by_cur_var_decl.
> So you can save yourself the duplicated test.

What I meant is: the line above could be replaced by

whlie (!vec_safe_is_empty (types_used_by_cur_var_decl))

Ian

Re: VEC re-write [patch 01/25]

[Ian Lance Taylor]

On Sun, Nov 18, 2012 at 5:14 PM, David Edelsohn  wrote:
>
> The problem is AIX stdlib.h defines
>
> #define vec_free free

Ouch.

> I am not sure where
>
> #undef vec_free
>
> should be placed.  In vec.h or system.h?

I think system.h.

Ian

Re: [RFA:] fix PR55030, wrong code from __builtin_setjmp_receiver

[Hans-Peter Nilsson]

On Mon, 12 Nov 2012, Eric Botcazou wrote:
> > This is a target-specific blockage insn, but with the general form
> > found in all targets defining it.  The default blockage is an empty
> > asm-volatile, which is what cse_insn recognized.  The blockage insn is
> > there to "prevent scheduling" of the critical insns and register
> > values.  It's almost obvious that an unspec_volatile should have that
> > effect "too"; at least that's intended by the code in
> > expand_builtin_setjmp_receiver.  Luckily (or unluckily regarding the
> > presence of the bug) *this* cse code is not run post-frame-layout
> > (post-reload-cse does not use this code), or it seems people would
> > soon notice register values used from the wrong side of the blockage,
> > considering its critical use at the restore of the stack-pointer.
> > As mentioned in the previous patch,
> > , clobbering
> > the frame-pointer (and then using it) does not seem the logical
> > alternative to the patch below; the blockage insn should just do its job.
>
> Agreed.
>
> > I updated comments and documentation so it's more apparent that
> > register uses should also not be moved across the blockage; see the
> > patched comments.
> >
> > Tested native x86_64-unknown-linux-gnu w./wo. -m32 and before/after
> > 192677.  Ok to commit?
> >
> > gcc:
> > PR middle-end/55030
> > * builtins.c (expand_builtin_setjmp_receiver): Update comment
> > regarding purpose of blockage.
> > * emit-rtl.c [!HAVE_blockage] (gen_blockage): Similarly for
> > the head comment.
> > * doc/md.texi (blockage): Update similarly.  Change wording to
> > require one of two forms, rather than implying a wider choice.
> > * cse.c (cse_insn): Where checking for blocking insns, treat
> > UNSPEC_VOLATILE as blocking, besides volatile ASM.
>
> That's fine on principle, but there is a predicate for this (volatile_insn_p)
> so I think we should use it here.  Moreover, cselib_process_insn has the same
> check so we should adjust it as well, which in turn means that dse.c:scan_insn
> should probably be adjusted too.  OK with these changes, thanks.

Doh, I should have looked for that construct in other places.
Thanks for the review.  Here's an updated patch with that.
Unfortunately, it causes regressions; read on for a very brief
analysis.

For x86_64-linux (base multilib):

Running 
/home/hp/gcctop/tmp/pr55030-2n/gcc/gcc/testsuite/gcc.dg/guality/guality.exp ...
...
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O3 -fomit-frame-pointer  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O3 -fomit-frame-pointer  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O3 -g  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O3 -g  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -Os  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -Os  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2 -flto -fno-use-linker-plugin 
-flto-partition=none  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2 -flto -fno-use-linker-plugin 
-flto-partition=none  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2 -flto -fuse-linker-plugin 
-fno-fat-lto-objects  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2 -flto -fuse-linker-plugin 
-fno-fat-lto-objects  line 14 arg7 == 30

And for -m32:
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg2 == 2
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg3 == 3
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg4 == 4
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg5 == 5
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg6 == 6
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg2 == 2
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg3 == 3
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg4 == 4
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg5 == 5
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg6 == 6
FAIL: gcc.dg/guality/pr36728-1.c  -O1  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg2 == 2
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg3 == 3
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg4 == 4
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg5 == 5
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg6 == 6
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 12 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg2 == 2
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg3 == 3
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg4 == 4
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg5 == 5
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg6 == 6
FAIL: gcc.dg/guality/pr36728-1.c  -O2  line 14 arg7 == 30
FAIL: gcc.dg/guality/pr36728-1.c  -O3 -fomit-fra

[doc] extend.texi copy-editing, 7/N (example markup)

[Sandra Loosemore]

This patch is another installment in my series of copy-edits to the GCC user 
documentation.  Here I've cleaned up several Texinfo markup issues relating to 
example environments.  Checked in under the free-for-all write access policy.

-Sandra


2012-11-18  Sandra Loosemore  

gcc/
* doc/extend.texi: Use @smallexample consistently.  Add @noindent
when continuing a sentence or paragraph past an example.  Change
tabs to spaces in examples.

Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 193610)
+++ gcc/doc/extend.texi	(working copy)
@@ -339,6 +339,7 @@ serves as a jump table:
 static void *array[] = @{ &&foo, &&bar, &&hack @};
 @end smallexample
 
+@noindent
 Then you can select a label with indexing, like this:
 
 @smallexample
@@ -1280,7 +1281,7 @@ Objects in this address space are locate
 
 @b{Example}
 
-@example
+@smallexample
 char my_read (const __flash char ** p)
 @{
 /* p is a pointer to RAM that points to a pointer to flash.
@@ -1301,7 +1302,7 @@ int main (void)
/* Return 17 by reading from flash memory */
return array[array[i]];
 @}
-@end example
+@end smallexample
 
 @noindent
 For each named address space supported by avr-gcc there is an equally
@@ -1309,7 +1310,7 @@ named but uppercase built-in macro defin
 The purpose is to facilitate testing if respective address space
 support is available or not:
 
-@example
+@smallexample
 #ifdef __FLASH
 const __flash int var = 1;
 
@@ -1327,7 +1328,7 @@ int read_var (void)
 return (int) pgm_read_word (&var);
 @}
 #endif /* __FLASH */
-@end example
+@end smallexample
 
 @noindent
 Notice that attribute @ref{AVR Variable Attributes,,@code{progmem}}
@@ -1367,10 +1368,12 @@ as immediates into operands of instructi
 @item
 The following code initializes a variable @code{pfoo}
 located in static storage with a 24-bit address:
-@example
+@smallexample
 extern const __memx char foo;
 const __memx void *pfoo = &foo;
-@end example
+@end smallexample
+
+@noindent
 Such code requires at least binutils 2.23, see
 @w{@uref{http://sourceware.org/PR13503,PR13503}}.
 
@@ -1404,6 +1407,7 @@ belonging to another address space by qu
 extern int __ea i;
 @end smallexample
 
+@noindent 
 The compiler generates special code to access the variable @code{i}.
 It may use runtime library
 support, or generate special machine instructions to access that address
@@ -1620,6 +1624,7 @@ example:
 #define debug(format, ...) fprintf (stderr, format, __VA_ARGS__)
 @end smallexample
 
+@noindent
 Here @samp{@dots{}} is a @dfn{variable argument}.  In the invocation of
 such a macro, it represents the zero or more tokens until the closing
 parenthesis that ends the invocation, including any commas.  This set of
@@ -1634,6 +1639,7 @@ argument.  Here is an example:
 #define debug(format, args...) fprintf (stderr, format, args)
 @end smallexample
 
+@noindent
 This is in all ways equivalent to the ISO C example above, but arguably
 more readable and descriptive.
 
@@ -1661,6 +1667,7 @@ used with the token paste operator, @sam
 #define debug(format, ...) fprintf (stderr, format, ## __VA_ARGS__)
 @end smallexample
 
+@noindent
 and if the variable arguments are omitted or empty, the @samp{##}
 operator causes the preprocessor to remove the comma before it.  If you
 do provide some variable arguments in your macro invocation, GNU CPP
@@ -2151,6 +2158,7 @@ void __f () @{ /* @r{Do something.} */; 
 void f () __attribute__ ((weak, alias ("__f")));
 @end smallexample
 
+@noindent
 defines @samp{f} to be a weak alias for @samp{__f}.  In C++, the
 mangled name for the target must be used.  It is an error if @samp{__f}
 is not defined in the same translation unit.
@@ -2198,6 +2206,7 @@ void* my_calloc(size_t, size_t) __attrib
 void my_realloc(void*, size_t) __attribute__((alloc_size(2)))
 @end smallexample
 
+@noindent
 declares that @code{my_calloc} returns memory of the size given by
 the product of parameter 1 and 2 and that @code{my_realloc} returns memory
 of the size given by parameter 2.
@@ -2324,6 +2333,7 @@ typedef int intfn ();
 extern const intfn square;
 @end smallexample
 
+@noindent
 This approach does not work in GNU C++ from 2.6.0 on, since the language
 specifies that the @samp{const} must be attached to the return value.
 
@@ -2762,6 +2772,7 @@ static void (*resolve_memcpy (void)) (vo
 @}
 @end smallexample
 
+@noindent
 The exported header file declaring the function the user calls would
 contain:
 
@@ -2769,6 +2780,7 @@ contain:
 extern void *memcpy (void *, const void *, size_t);
 @end smallexample
 
+@noindent
 allowing the user to call this as a regular function, unaware of the
 implementation.  Finally, the indirect function needs to be defined in
 the same translation unit as the resolver function:
@@ -3155,6 +3167,8 @@ optimized away, put
 @smallexample
 asm ("");
 @end smallexample
+
+@noindent
 (@pxref{Extended Asm}) in the called function, to serv

Re: PATCH: Add --with-build-config=bootstrap-asan support

[H.J. Lu]

On Sun, Nov 18, 2012 at 7:28 AM, Paolo Bonzini  wrote:
> Il 18/11/2012 00:54, H.J. Lu ha scritto:
>> +@if gcc-bootstrap
>> +ifneq ($(filter bootstrap-asan,$(BUILD_CONFIG)),)
>> +LIBASAN_LIBS=-B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/asan/.libs
>> +endif
>> +@endif gcc-bootstrap
>
> Do you need this to be here?  POSTSTAGE1_*_EXPORT is only used when
> bootstrapping, and they are defined with "=" so the definition could be
> placed (or so it seems at first look) in bootstrap-asan.mk.
>

It turns out to be quite tricky:

1. We can't use -faddress-sanitizer in libcpp due to

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55380

2. We can't use  -faddress-sanitizer in libiberty since
it will make all host binaries depend on libasan.
3. We can't use  -faddress-sanitizer in lto-plugin
since it will make linker depends on libasan.

So basically we can only use -faddress-sanitizer in
the gcc directory, which isn't easy to do.

Also I have to mark libsanitizer as bootstrap otherwise
multilib libsanitizer will get -funconfigured-libstcd++-v3.

I checked my patches into hjl/asan git branch:

http://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/hjl/asan

I will submit them after running some tests.


-- 
H.J.

[PATCH] Prepend -lasan and disallow -static with -faddress-sanitizer

[H.J. Lu]

Hi,

This patch prepens -lasan before any other libraries added by language.
It also disallows -static with -faddress-sanitizer.  OK to install?

Thanks.


H.J.
---
2012-11-18  H.J. Lu  

PR driver/55379
PR driver/55374
* gcc.c (prepend_lang_specific_driver): New function.
(process_command): Use it.

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 7a275e1..11279be4 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3609,6 +3609,87 @@ set_option_handlers (struct cl_option_handlers *handlers)
   handlers->handlers[2].mask = CL_TARGET;
 }
 
+/* Prepend command line before language-specific adjustment/addition of
+   flags.  */
+
+void
+prepend_lang_specific_driver (struct cl_decoded_option **in_decoded_options,
+ unsigned int *in_decoded_options_count,
+ int *in_added_libraries)
+{
+  unsigned int i, argc;
+
+  /* The new argument list will be contained in this.  */
+  struct cl_decoded_option *new_decoded_options;
+
+  /* The argument list.  */
+  struct cl_decoded_option *decoded_options;
+
+  bool static_link = false;
+  bool add_libasan = false;
+  bool static_libasan = false;
+
+  argc = *in_decoded_options_count;
+  decoded_options = *in_decoded_options;
+
+  for (i = 1; i < argc; i++)
+switch (decoded_options[i].opt_index)
+  {
+  case OPT_faddress_sanitizer:
+   add_libasan = true;
+   break;
+  case OPT_static:
+   static_link = true;
+   break;
+  case OPT_static_libasan:
+   static_libasan = true;
+   break;
+  }
+
+  if (add_libasan)
+{
+  /* Add -lasan before language-specific adjustment/addition.  */
+  unsigned int added_argc;
+
+  if (static_link)
+   fatal_error ("cannot specify -static with -faddress-sanitizer");
+
+  added_argc = 1;
+#ifdef HAVE_LD_STATIC_DYNAMIC
+  if (static_libasan)
+   added_argc += 2;
+#endif
+
+  new_decoded_options = XNEWVEC (struct cl_decoded_option,
+argc + added_argc);
+
+  i = 0;
+  do
+   {
+ new_decoded_options[i] = decoded_options[i];
+ i++;
+   }
+  while (i < argc);
+
+#ifdef HAVE_LD_STATIC_DYNAMIC
+  if (static_libasan)
+   generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
+&new_decoded_options[i++]);
+#endif
+  generate_option (OPT_l, "asan", 1, CL_DRIVER,
+  &new_decoded_options[i++]);
+#ifdef HAVE_LD_STATIC_DYNAMIC
+  if (static_libasan)
+   generate_option (OPT_Wl_, LD_DYNAMIC_OPTION, 1, CL_DRIVER,
+&new_decoded_options[i++]);
+#endif
+
+  *in_decoded_options_count = i;
+  *in_decoded_options = new_decoded_options;
+  *in_added_libraries = 1;
+}
+}
+
 /* Create the vector `switches' and its contents.
Store its length in `n_switches'.  */
 
@@ -3700,6 +3781,11 @@ process_command (unsigned int decoded_options_count,
  or an automatically created GCC_EXEC_PREFIX from
  decoded_options[0].arg.  */
 
+  /* Prepend command line before language-specific adjustment/addition of
+ flags.  */
+  prepend_lang_specific_driver (&decoded_options, &decoded_options_count,
+   &added_libraries);
+
   /* Do language-specific adjustment/addition of flags.  */
   lang_specific_driver (&decoded_options, &decoded_options_count,
&added_libraries);
-- 
1.7.11.7

[PATCH] Add STATIC_LIBASAN_LIBS for -static-libasan

[H.J. Lu]

Hi,

This patch adds STATIC_LIBASAN_LIBS so that one can simply use
"gcc -faddress-sanitizer -static-libasan".  OK to install?

Thanks.


H.J.
---
2012-11-18  H.J. Lu  

* gcc.c (ADD_STATIC_LIBASAN_LIBS): New macro.  Defined
with STATIC_LIBASAN_LIBS.
(LIBASAN_SPEC): Add STATIC_LIBASAN_LIBS.
* config/gnu-user.h (STATIC_LIBASAN_LIBS): New macro.

diff --git a/gcc/config/gnu-user.h b/gcc/config/gnu-user.h
index cb45749..8c4bbc6 100644
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -98,3 +98,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 
 #define TARGET_C99_FUNCTIONS 1
 #define TARGET_HAS_SINCOS 1
+
+/* Additional libraries needed by -static-libasan.  */
+#undef STATIC_LIBASAN_LIBS
+#define STATIC_LIBASAN_LIBS "-ldl -lpthread"
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 11279be4..5e68d71 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -545,11 +545,18 @@ proper position among the other output files.  */
 #define STACK_SPLIT_SPEC " %{fsplit-stack: --wrap=pthread_create}"
 
 #ifndef LIBASAN_SPEC
+#ifdef STATIC_LIBASAN_LIBS
+#define ADD_STATIC_LIBASAN_LIBS \
+  " %{static-libasan:" STATIC_LIBASAN_LIBS "}"
+#else
+#define ADD_STATIC_LIBASAN_LIBS
+#endif
 #ifdef HAVE_LD_STATIC_DYNAMIC
 #define LIBASAN_SPEC "%{static-libasan:" LD_STATIC_OPTION \
-"} -lasan %{static-libasan:" LD_DYNAMIC_OPTION "}"
+"} -lasan %{static-libasan:" LD_DYNAMIC_OPTION "}" \
+ADD_STATIC_LIBASAN_LIBS
 #else
-#define LIBASAN_SPEC "-lasan"
+#define LIBASAN_SPEC "-lasan" ADD_STATIC_LIBASAN_LIBS
 #endif
 #endif
 
-- 
1.7.11.7

[PATCH] Use explicit -I for libstdc++-v3 header files to build libsanitizer

[H.J. Lu]

Hi,

This patch adds explicit -I for libstdc++-v3 header files when building
libsanitizer so that it can be used for bootstrapping GCC.  Othewise,
-funconfigured-libstcd++-v3 will be used to compile multilib
libsanitizer.  OK to install?

Thanks.


H.J.
---
2012-11-18  H.J. Lu  

* Makefile.am (AM_MAKEFLAGS): Remove CC and CXX.
* configure.ac (ACX_NONCANONICAL_TARGET): New.
* asan/Makefile.am (AM_CXXFLAGS): Add -I for libstdc++-v3 header
files.
(AM_MAKEFLAGS): Remove CC and CXX.
* interception/Makefile.am: Likewise.
* sanitizer_common/Makefile.am: Likewise.
* Makefile.in: Regenerated.
* aclocal.m4: Likewise.
* configure: Likewise.
* asan/Makefile.in: Likewise.
* interception/Makefile.in: Likewise.
* sanitizer_common/Makefile.in: Likewise.

diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index 91e3434..aa8021a 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -33,8 +33,6 @@ AM_MAKEFLAGS = \
"includedir=$(includedir)" \
"AR=$(AR)" \
"AS=$(AS)" \
-   "CC=$(CC)" \
-   "CXX=$(CXX)" \
"LD=$(LD)" \
"LIBCFLAGS=$(LIBCFLAGS)" \
"NM=$(NM)" \
diff --git a/libsanitizer/Makefile.in b/libsanitizer/Makefile.in
index 8d685fd..4bb8ff2 100644
--- a/libsanitizer/Makefile.in
+++ b/libsanitizer/Makefile.in
@@ -41,7 +41,8 @@ DIST_COMMON = $(am__configure_deps) $(srcdir)/../config.guess 
\
$(srcdir)/../mkinstalldirs $(srcdir)/Makefile.am \
$(srcdir)/Makefile.in $(top_srcdir)/configure ChangeLog
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
-am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
+am__aclocal_m4_deps = $(top_srcdir)/../config/acx.m4 \
+   $(top_srcdir)/../config/depstand.m4 \
$(top_srcdir)/../config/lead-dot.m4 \
$(top_srcdir)/../config/multi.m4 \
$(top_srcdir)/../config/override.m4 \
@@ -236,6 +237,7 @@ sysconfdir = @sysconfdir@
 target = @target@
 target_alias = @target_alias@
 target_cpu = @target_cpu@
+target_noncanonical = @target_noncanonical@
 target_os = @target_os@
 target_vendor = @target_vendor@
 toolexecdir = @toolexecdir@
@@ -277,8 +279,6 @@ AM_MAKEFLAGS = \
"includedir=$(includedir)" \
"AR=$(AR)" \
"AS=$(AS)" \
-   "CC=$(CC)" \
-   "CXX=$(CXX)" \
"LD=$(LD)" \
"LIBCFLAGS=$(LIBCFLAGS)" \
"NM=$(NM)" \
diff --git a/libsanitizer/aclocal.m4 b/libsanitizer/aclocal.m4
index a52bc30..d6782f8 100644
--- a/libsanitizer/aclocal.m4
+++ b/libsanitizer/aclocal.m4
@@ -990,6 +990,7 @@ AC_SUBST([am__tar])
 AC_SUBST([am__untar])
 ]) # _AM_PROG_TAR
 
+m4_include([../config/acx.m4])
 m4_include([../config/depstand.m4])
 m4_include([../config/lead-dot.m4])
 m4_include([../config/multi.m4])
diff --git a/libsanitizer/asan/Makefile.am b/libsanitizer/asan/Makefile.am
index 3da1db3..45fb3b3 100644
--- a/libsanitizer/asan/Makefile.am
+++ b/libsanitizer/asan/Makefile.am
@@ -5,6 +5,10 @@ gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
 
 DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 
-DASAN_FLEXIBLE_MAPPING_AND_OFFSET=0 -DASAN_NEEDS_SEGV=1
 AM_CXXFLAGS = -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic 
-Wno-long-long  -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer 
-funwind-tables -fvisibility=hidden -Wno-variadic-macros -Wno-c99-extensions 
+## We require libstdc++-v3 to be in the same build tree.
+AM_CXXFLAGS += -I../../libstdc++-v3/include \
+  -I../../libstdc++-v3/include/$(target_noncanonical) \
+  -I$(srcdir)/../../libstdc++-v3/libsupc++
 ACLOCAL_AMFLAGS = -I $(top_srcdir) -I $(top_srcdir)/config
 
 toolexeclib_LTLIBRARIES = libasan.la
@@ -64,8 +68,6 @@ AM_MAKEFLAGS = \
"includedir=$(includedir)" \
"AR=$(AR)" \
"AS=$(AS)" \
-   "CC=$(CC)" \
-   "CXX=$(CXX)" \
"LD=$(LD)" \
"LIBCFLAGS=$(LIBCFLAGS)" \
"NM=$(NM)" \
diff --git a/libsanitizer/asan/Makefile.in b/libsanitizer/asan/Makefile.in
index e5e8d40..681fe5e 100644
--- a/libsanitizer/asan/Makefile.in
+++ b/libsanitizer/asan/Makefile.in
@@ -38,7 +38,8 @@ target_triplet = @target@
 subdir = asan
 DIST_COMMON = $(srcdir)/Makefile.am $(srcdir)/Makefile.in
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
-am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
+am__aclocal_m4_deps = $(top_srcdir)/../config/acx.m4 \
+   $(top_srcdir)/../config/depstand.m4 \
$(top_srcdir)/../config/lead-dot.m4 \
$(top_srcdir)/../config/multi.m4 \
$(top_srcdir)/../config/override.m4 \
@@ -228,6 +229,7 @@ sysconfdir = @sysconfdir@
 target = @target@
 target_alias = @target_alias@
 target_cpu = @target_cpu@
+target_noncanonical = @target_noncanonical@
 target_os = @target_os@
 target_vendor = @target_vendor@
 toolexecdir = @toolexecdir@
@@ -239,7 +241,13 @@ AM_CPPFLAGS = -I $(top_srcdir)/include -I $(top_s

Re: [PATCH] Fix part of PR bootstrap/55051 (issue6846063)

[Teresa Johnson]

On Fri, Nov 16, 2012 at 11:07 AM, Jan Hubicka  wrote:
>> This patch addresses the bogus "Invocation mismatch" messages seen in 
>> parallel
>> profiledbootstrap builds of gcc. See PR bootstrap/55051 for a discussion of
>> why this is occurring and why this checking is inaccurate.
>>
>> Profilebootstrapped and tested on x86_64-unknown-linux-gnu.  Ok for trunk?
>>
>> 2012-11-15  Teresa Johnson  
>>
>> PR bootstrap/55051
>>   * libgcov.c (gcov_exit): Remove checking against first
>> merged summary for program, as multiple simultaneous
>> processes attempting to update gcda files may cause
>> summaries to temporarily differ.
>>
>> Index: libgcov.c
>> ===
>> --- libgcov.c (revision 193522)
>> +++ libgcov.c (working copy)
>> @@ -365,7 +365,6 @@ gcov_exit (void)
>>struct gcov_info *gi_ptr;
>>const struct gcov_fn_info *gfi_ptr;
>>struct gcov_summary this_prg; /* summary for program.  */
>> -  struct gcov_summary all_prg;  /* summary for all instances of program.  */
>>struct gcov_ctr_summary *cs_ptr;
>>const struct gcov_ctr_info *ci_ptr;
>>unsigned t_ix;
>> @@ -382,7 +381,6 @@ gcov_exit (void)
>>if (gcov_dump_complete)
>>  return;
>>
>> -  memset (&all_prg, 0, sizeof (all_prg));
>>/* Find the totals for this execution.  */
>>memset (&this_prg, 0, sizeof (this_prg));
>>for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
>> @@ -469,7 +467,7 @@ gcov_exit (void)
>>unsigned n_counts;
>>struct gcov_summary prg; /* summary for this object over all
>> program.  */
>> -  struct gcov_ctr_summary *cs_prg, *cs_tprg, *cs_all;
>> +  struct gcov_ctr_summary *cs_prg, *cs_tprg;
>>int error = 0;
>>gcov_unsigned_t tag, length;
>>gcov_position_t summary_pos = 0;
>> @@ -684,7 +682,6 @@ gcov_exit (void)
>>   {
>> cs_prg = &prg.ctrs[t_ix];
>> cs_tprg = &this_prg.ctrs[t_ix];
>> -   cs_all = &all_prg.ctrs[t_ix];
>>
>> if (gi_ptr->merge[t_ix])
>>   {
>> @@ -702,24 +699,6 @@ gcov_exit (void)
>>   }
>> else if (cs_prg->runs)
>>   goto read_mismatch;
>> -
>> -   if (!cs_all->runs && cs_prg->runs)
>> - memcpy (cs_all, cs_prg, sizeof (*cs_all));
>> -   else if (!all_prg.checksum
>> -&& (!GCOV_LOCKED || cs_all->runs == cs_prg->runs)
>> -   /* Don't compare the histograms, which may have slight
>> -  variations depending on the order they were updated
>> -  due to the truncating integer divides used in the
>> -  merge.  */
>> -   && memcmp (cs_all, cs_prg,
>> -  sizeof (*cs_all) - (sizeof (gcov_bucket_type)
>> -  * GCOV_HISTOGRAM_SIZE)))
> Please keep the massage around with !GCOV_LOCKED.  In that case user should be
> informed that parallel exeuction is bad idea (tm).
>
> The memcpy/memcmp pair with sizeof (gcov_bucket_type) adjustment is ugly. Just
> copy those few relevant values into temporary vars.
>
> OK with this change.
>
> Honza

Ok, just committed the following:

012-11-18  Teresa Johnson  

PR bootstrap/55051
* libgcov.c (gcov_exit): Remove merged program summary
comparison unless !GCOV_LOCKED.

Index: libgcov.c
===
--- libgcov.c   (revision 193522)
+++ libgcov.c   (working copy)
@@ -365,7 +365,9 @@ gcov_exit (void)
   struct gcov_info *gi_ptr;
   const struct gcov_fn_info *gfi_ptr;
   struct gcov_summary this_prg; /* summary for program.  */
+#if !GCOV_LOCKED
   struct gcov_summary all_prg;  /* summary for all instances of program.  */
+#endif
   struct gcov_ctr_summary *cs_ptr;
   const struct gcov_ctr_info *ci_ptr;
   unsigned t_ix;
@@ -382,7 +384,9 @@ gcov_exit (void)
   if (gcov_dump_complete)
 return;

+#if !GCOV_LOCKED
   memset (&all_prg, 0, sizeof (all_prg));
+#endif
   /* Find the totals for this execution.  */
   memset (&this_prg, 0, sizeof (this_prg));
   for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
@@ -469,7 +473,10 @@ gcov_exit (void)
   unsigned n_counts;
   struct gcov_summary prg; /* summary for this object over all
  program.  */
-  struct gcov_ctr_summary *cs_prg, *cs_tprg, *cs_all;
+  struct gcov_ctr_summary *cs_prg, *cs_tprg;
+#if !GCOV_LOCKED
+  struct gcov_ctr_summary *cs_all;
+#endif
   int error = 0;
   gcov_unsigned_t tag, length;
   gcov_position_t summary_pos = 0;
@@ -684,7 +691,6 @@ gcov_exit (void)
{
  cs_prg = &prg.ctrs[t_ix];
  cs_tprg = &this_prg.ctrs[t_ix];
- cs_all = &all_prg.ctrs[t_ix];

  if (gi_ptr->merge[t_ix])
{
@@ -703,23 +709,34 @@ gcov_exit (void)
  else if (cs_prg->runs)
got

libgo patch committed: Fix reflect.valueInterface

[Ian Lance Taylor]

The gccgo frontend uses a slightly different representation for
interface values than the gc compiler: the gccgo frontend stores pointer
values directly, and all other values indirectly.  That requires some
changes in the reflect package.  There was a bug in those changes, which
showed up in that in some cases different reflect.Value values would
incorrectly point to the same underlying memory.  This patch fixes the
bug.  Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.7 branch.

Ian

diff -r 4bbe9e0d26e7 libgo/go/reflect/value.go
--- a/libgo/go/reflect/value.go	Sat Nov 17 23:53:21 2012 -0800
+++ b/libgo/go/reflect/value.go	Sun Nov 18 21:33:01 2012 -0800
@@ -342,7 +342,7 @@
 }
 
 // CallSlice calls the variadic function v with the input arguments in,
-// assigning the slice in[len(in)-1] to v's final variadic argument.  
+// assigning the slice in[len(in)-1] to v's final variadic argument.
 // For example, if len(in) == 3, v.Call(in) represents the Go call v(in[0], in[1], in[2]...).
 // Call panics if v's Kind is not Func or if v is not variadic.
 // It returns the output results as Values.
@@ -905,7 +905,7 @@
 
 	if safe && v.flag&flagRO != 0 {
 		// Do not allow access to unexported values via Interface,
-		// because they might be pointers that should not be 
+		// because they might be pointers that should not be
 		// writable or methods or function that should not be callable.
 		panic("reflect.Value.Interface: cannot return value obtained from unexported field or method")
 	}
@@ -928,7 +928,7 @@
 	eface.typ = v.typ.runtimeType()
 	eface.word = v.iword()
 
-	if v.flag&flagIndir != 0 && v.typ.size > ptrSize {
+	if v.flag&flagIndir != 0 && v.kind() != Ptr && v.kind() != UnsafePointer {
 		// eface.word is a pointer to the actual data,
 		// which might be changed.  We need to return
 		// a pointer to unchanging data, so make a copy.
@@ -1777,7 +1777,7 @@
 		default:
 			panic("reflect.Select: invalid Dir")
 
-		case SelectDefault: // default	
+		case SelectDefault: // default
 			if haveDefault {
 panic("reflect.Select: multiple default cases")
 			}

Merge from trunk to gccgo branch

[Ian Lance Taylor]

I've merged trunk revision 193614 to the gccgo branch.

Ian

Re: Patch ping

[Jakub Jelinek]

On Sat, Nov 17, 2012 at 11:10:04AM -0800, Richard Henderson wrote:
> On 11/16/2012 01:10 AM, Jakub Jelinek wrote:
> > Hi!
> > 
> > - PR54921 invalidate sp in cselib on fp setter insn
> >   http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02035.html
> >   (perhaps in light of PR54402 the single_succ (ENTRY_BLOCK_PTR)
> >   from the patch should be nuked)
> 
> > +  rtx expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX);
> > +  if (expr)
> > +   pat = XEXP (expr, 0);
> > +  if (GET_CODE (pat) == SET
> > + && SET_DEST (pat) == hard_frame_pointer_rtx)
> > +   cselib_invalidate_rtx (stack_pointer_rtx);
> > +  else if (GET_CODE (pat) == PARALLEL)
> 
> This is only one possible way that FP can be set.
> The others are with CFA_DEF_CFA or CFA_ADJUST_CFA.
> 
> Given 
> 
> +  && frame_pointer_needed
> +  && RTX_FRAME_RELATED_P (insn)
> 
> is there any reason to do more than
> 
>&& modified_in_p (hard_frame_pointer_rtx, insn)
> 
> ?

I'd prefer to only invalidate the stack pointer on the first assignment
to the hard pointer.  If the cselib link between sp and hfp is already
broken, invalidating sp will only result in worse code.  Dunno if there
are any targets that adjust the hard frame pointer after it has been set
once or similar.
Perhaps we could walk here CSELIB_VAL_PTR (hfpval)->locs here, and look
if any rtls in there have find_base_term (x->loc) == find_base_term
(stack_pointer_rtx), and only if yes, invalidate (and guard it by the
modified_in_p test).
BTW, var-tracking.c uses a similar test.

Jakub

Re: RFA: patch to fix PR19398

[Uros Bizjak]

On Mon, Nov 19, 2012 at 2:45 AM, Vladimir Makarov  wrote:
> The following patch fixes
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19398
>
> Uros, there is i386.md part for which I need an approval.  Without this
> change, GCC will still generate the same code even if LRA uses an
> alternative with 'm' constraint.
>
> 2012-11-18  Vladimir Makarov  
>
> PR target/19398
> * lra-constraints.c (process_alt_operands): Discourage reloads
> through secodnary memory.
> * config/i386/i386.md (fix_trunc?f?i_sse): Remove peephole2
> patterns.

Thanks!

Please note that i386.md change is not correct, it is peephole2 with
"Shorten x87->SSE reload sequences ..." comment that is not effective
anymore with your patch and should now be removed. The peephole2s that
your patch removes undo LRA transformation for targets that *do not*
benefit from MEM->REG operation for this particular FIX RTX. (Also,
please note that your patch includes movti_internal_rex64 change that
was already reverted due to better fix).

Please remove mentioned peephole2 instead. The test from the PR will
show effects of LRA change for all targets, other than core2i7_64, k8
and generic 64bit targets.

The patch that removes mentioned peephole2 from i386.md is pre-approved.

Thanks,
Uros.


>