Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign

2014-11-11 Thread Eric Botcazou
> I just don't like all the as_a/is_a stuff enforced everywhere,
> it means more typing, more temporaries, more indentation.
> So, as I view it, instead of the checks being done cheaply (yes, I think
> the gimple checking as we have right now is very cheap) under the
> hood by the accessors (gimple_assign_{lhs,rhs1} etc.), those changes
> put the burden on the developers, who has to check that manually through
> the as_a/is_a stuff everywhere, more typing and uglier syntax.
> I just don't see that as a step forward, instead a huge step backwards.
> But perhaps I'm alone with this.

IMO that's the sort of things some of us were afraid of when the C++ switch 
was being discussed and IIRC we were told this would not happen...

-- 
Eric Botcazou


Re: Match-and-simplify and COND_EXPR

2014-11-11 Thread Richard Biener
On Mon, 10 Nov 2014, Andrew Pinski wrote:

> On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener  wrote:
> > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> >
> >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener  wrote:
> >> > On Thu, 6 Nov 2014, Richard Biener wrote:
> >> >
> >> >> On Wed, 5 Nov 2014, Andrew Pinski wrote:
> >> >>
> >> >> > Hi,
> >> >> >   I was trying to hook up tree-ssa-phiopt to match-and-simplify using
> >> >> > either gimple_build (or rather using gimple_simplify depending on if
> >> >> > we want to produce cond_expr for conditional move).  I ran into a
> >> >> > problem.
> >> >> > With the pattern below:
> >> >> > /* a ? 0 : 1 -> a if 0 and 1 are integral types. */
> >> >> > (simplify
> >> >> >   (cond_expr @0 integer_zerop integer_onep)
> >> >> >   (if (INTEGRAL_TYPE_P (type))
> >> >> > (convert @0)))
> >> >>
> >> >> Ok, so you are capturing a GENERIC expr here but nothing knows that.
> >> >> It would work if you'd do (ugh)
> >> >>
> >> >> (for op (lt le eq ne ge gt)
> >> >>  (simplify
> >> >>   (cond_expr (op @0 @1) integer_zerop integer_onep)
> >> >>   (if (INTEGRAL_TYPE_P (type))
> >> >>(convert (op @0 @1)
> >> >> (simplify
> >> >>  (cond_expr SSA_NAME@0 integer_zerop integer_onep)
> >> >>   (if (INTEGRAL_TYPE_P (type))
> >> >>(convert @0
> >> >>
> >> >> as a workaround.  To make your version work will require (quite)
> >> >> some special-casing in the code generator or maybe the resimplify
> >> >> helper.  Let me see if I can cook up a "simple" fix.
> >> >
> >> > Sth like below (for the real fix this has to be replicated in
> >> > all gimple_resimplifyN functions).  I'm missing a testcase
> >> > where the pattern would apply (and not be already folded by fold),
> >> > so I didn't check if it actually works.
> >>
> >> You do need to check if seq is NULL though as gimple_build depends on
> >> seq not being NULL.  But otherwise yes this works for me.
> >
> > Ok, here is a better and more complete fix.  The optimization
> > whether a captured expression may be a GENERIC condition isn't
> > implemented so a lot of checks are emitted right now.  Also
> > if gimple_build would fail this terminates the whole matching
> > process, not just matching of the current pattern (failing "late"
> > isn't supported with the style of code we emit - well, without
> > adding ugly gotos and labels).
> >
> > At least it avoids splitting up conditions if substituted into
> > a COND_EXPR, so simplifications of COND_EXPRs in-place (w/o seq)
> > should still work.
> >
> > Can you check whether this works for you?
> 
> This works for most cases but does not work for things like:
> /* a != b ? a : b -> a */
> (simplify
>   (cond_expr (ne @0 @1) @0 @1)
>   @0)
>
> In gimple_simplify after it matches COND_EXPR. It does not look into
> the operand 0 to see if it matches NE_EXPR, it just sees if it was a
> SSA_NAME.  In this case I was trying to do a simple replacement of
> value_replacement in tree-ssa-phiopt.c.

Yeah - I hoped you wouldn't notice.  It's not too difficult to fix.
Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
it is better to finally fix it with

Index: gcc/gimple-expr.c
===
--- gcc/gimple-expr.c   (revision 216973)
+++ gcc/gimple-expr.c   (working copy)
@@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
 bool
 is_gimple_condexpr (tree t)
 {
-  return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
-   && !tree_could_throw_p (t)
-   && is_gimple_val (TREE_OPERAND (t, 0))
-   && is_gimple_val (TREE_OPERAND (t, 1;
+  return is_gimple_val (t);
 }
 
 /* Return true if T is a gimple address.  */

I expect mostly fallout from passes building [VEC_]COND_EXPRs and
not gimplifying it and of course missed optimizations from the
vectorizer which will likely now try to vectorize the separate
comparisons in some maybe odd way (not sure).

I did this experiment once but didn't finish it.  Too bad :/

> But for my purpose right now this is sufficient and will be posting a
> patch which does not remove things from tree-ssa-phiopt.c just yet.

Fair enough.  I suppose for GCC 5 I will need to deal with the odd
GIMPLE IL.  At least I can consider it a "bug" and thus fix this
during stage3 ;)

Thanks,
Richard.


Re: How is libtool updated in GCC?

2014-11-11 Thread FX
> If you import rather than selectively merging one change you need (I 
> think) to revert libtool commit 3334f7ed5851ef1e96b052f2984c4acdbf39e20c 
> (incompatible with GCC handling of sysroots), as well as updating all 
> three relevant repositories (GCC, binutils-gdb, src - I'm not sure if 
> anything in binutils-gdb actually uses libtool but it has the files at 
> toplevel).

So I’ve actually chosen to avoid the mess and selectly merge the one change 
that we need. Looking at the history of libtool.m4 in our tree, it looks like 
others have done that in the past, too.

Thanks for the feedback. On the src repository, is it documented somewhere how 
to change it? (I have probably done it in the past, but don’t remember)

FX

Re: testing policy for C/C++ front end changes

2014-11-11 Thread Richard Biener
 On Mon, Nov 10, 2014 at 4:28 PM, Sandra Loosemore
 wrote:
> On 11/10/2014 05:03 AM, Richard Biener wrote:
>>
>> On Mon, Nov 10, 2014 at 5:50 AM, Jeff Law  wrote:
>>>
>>> On 11/09/14 16:13, Sandra Loosemore wrote:


 https://gcc.gnu.org/contribute.html#testing

 and noticed that the policy is to require a complete bootstrap for C
 changes, but not for C++.  Given that GCC's implementation language is
 now C++, isn't that backwards?  I'm not trying to weasel out of the
 extra work for my patch, just curious if the web site guidelines have
 gotten bit-rotten after the switch to C++, or if the SC did indeed
 consider the issue already and the published policy is accurate.
>>>
>>>
>>> They've bit-rotted a bit.  Interested in cons-ing up an update?
>>
>>
>> Bootstrap should now be required for both C and C++ FE changes
>> _and_ for libstdc++ changes as well given we start to pull in
>> libstdc++ headers during bootstrap.
>
>
> H.  How about the attached patch?  I also added a blurb about
> target-specific patches that I think reflects current practice.

I think you need to retain the fact that one needs to bootstrap, not just
build GCC.  Thus "If your change is to code that is not in a front
end, or is to the C or C++ front ends or libgcc or
libstdc++
libraries, you must perform a bootstrap of GCC with all languages enabled
by default, on at least one primary target,  and run all testsuites."

Ok with that change.

Thanks,
Richard.

> -Sandra
>


RE: arm/thumb broken on head

2014-11-11 Thread Terry Guo


> -Original Message-
> From: Terry Guo
> Sent: Tuesday, November 11, 2014 9:08 AM
> To: Richard Earnshaw; Joel Sherrill
> Cc: GCC Mailing List
> Subject: RE: arm/thumb broken on head
> 
> 
> 
> > -Original Message-
> > From: Richard Earnshaw
> > Sent: Tuesday, November 11, 2014 1:08 AM
> > To: Joel Sherrill; GCC Mailing List
> > Cc: Terry Guo
> > Subject: Re: arm/thumb broken on head
> >
> > On 10/11/14 15:26, Joel Sherrill wrote:
> > > Hi
> > >
> > > With gcc, newlib, and binutils head, arm-rtems and arm-eabi both die
> > > building libgcc2.c for the Thumb. I don't know if this is a recent
> > > gcc change or binutils having added some new error checking. Anyone
> > > got any ideas?
> > >
> > > /users/joel/test-gcc/b-arm-eabi-gcc/./gcc/xgcc
> > > -B/users/joel/test-gcc/b-arm-eabi-gcc/./gcc/ -nostdinc
> > > -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/newlib/ -isystem
> > > /users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/newlib/targ-include
> > > -isystem /users/joel/test-gcc/gcc/newlib/libc/include
> > > -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/libgloss/arm
> > > -L/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/libgloss/libnosys
> > > -L/users/joel/test-gcc/gcc/libgloss/arm
> > > -B/users/joel/test-gcc/install-head/arm-eabi/bin/
> > > -B/users/joel/test-gcc/install-head/arm-eabi/lib/ -isystem
> > > /users/joel/test-gcc/install-head/arm-eabi/include -isystem
> > > /users/joel/test-gcc/install-head/arm-eabi/sys-include-g -O2
-mthumb
> > > -O2  -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall
> > > -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes
> > > -Wmissing-prototypes -Wold-style-definition  -isystem ./include
> > > -fno-inline -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector
> > > -Dinhibit_libc  -fno-inline -I. -I. -I../../.././gcc
> > > -I../../../../gcc/libgcc -I../../../../gcc/libgcc/.
> > > -I../../../../gcc/libgcc/../gcc -I../../../../gcc/libgcc/../include
> > > -DHAVE_CC_TLS  -o _muldi3.o -MT _muldi3.o -MD -MP -MF _muldi3.dep
> > > -DL_muldi3 -c ../../../../gcc/libgcc/libgcc2.c -fvisibility=hidden
> > > -DHIDE_EXPORTS
> > > /tmp/cc9EfnXy.s: Assembler messages:
> > > /tmp/cc9EfnXy.s:69: Error: MOV Rd, Rs with two low registers is not
> > > permitted on this architecture -- `mov r6,r7'
> > >
> >
> > This is most likely fallout from the migration of thumb1 assembly to
> > unified syntax.  Terry, if this hasn't already been sorted can you take
a look?
> >
> > R.
> 
> Sorry for troubles. Indeed this is caused my recent Thumb-1 UAL patch.
> Some 'mov' instructions require special treatment. I am working on a
patch.
> 
> BR,
> Terry

Fix is committed to trunk at
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=217341.

BR,
Terry





[match-and-simplify] out of memory allocation error

2014-11-11 Thread Prathamesh Kulkarni
I moved definition of simplify::simplify outside the class (as in the
attached patch),
and I get following error during stage1 build:
out of memory allocating 16301357976 bytes after a total of 2026800 bytes
make[2]: *** [s-match] Error 1
make[1]: *** [all-gcc] Error 2
make: *** [all] Error 2

Why does moving definition of simplify constructor outside the class cause
out of memory allocation ?

Thanks,
Prathamesh
Index: genmatch.c
===
--- genmatch.c	(revision 217303)
+++ genmatch.c	(working copy)
@@ -549,11 +549,7 @@
   simplify (operand *match_, source_location match_location_,
 	struct operand *result_, source_location result_location_,
 	vec ifexpr_vec_, vec > for_vec_,
-	cid_map_t *capture_ids_)
-  : match (match_), match_location (match_location_),
-  result (result_), result_location (result_location_),
-  ifexpr_vec (ifexpr_vec_), for_vec (for_vec_),
-  capture_ids (capture_ids_), capture_max (capture_ids_->elements () - 1) {}
+	cid_map_t *capture_ids_);
 
   /* The expression that is matched against the GENERIC or GIMPLE IL.  */
   operand *match;
@@ -574,6 +570,20 @@
   int capture_max;
 };
 
+simplify::simplify (operand *match_, source_location match_location_,
+		operand *result_, source_location result_location_,
+		vec ifexpr_vec_, vec< vec > for_vec_,
+		cid_map_t *capture_ids_)
+{
+  match = match_;
+  match_location = match_location_;
+  result = result_;
+  result_location = result_location_;
+  ifexpr_vec = ifexpr_vec_;
+  for_vec = for_vec_;
+  capture_ids = capture_ids_;
+}
+
 /* Debugging routines for dumping the AST.  */
 
 DEBUG_FUNCTION void


Re: [match-and-simplify] out of memory allocation error

2014-11-11 Thread Prathamesh Kulkarni
On Tue, Nov 11, 2014 at 4:03 PM, Prathamesh Kulkarni
 wrote:
> I moved definition of simplify::simplify outside the class (as in the
> attached patch),
> and I get following error during stage1 build:
> out of memory allocating 16301357976 bytes after a total of 2026800 bytes
> make[2]: *** [s-match] Error 1
> make[1]: *** [all-gcc] Error 2
> make: *** [all] Error 2
>
> Why does moving definition of simplify constructor outside the class cause
> out of memory allocation ?
oops i didn't initialize capture_max.
sorry for the noise.
>
> Thanks,
> Prathamesh


Re: [match-and-simplify] out of memory allocation error

2014-11-11 Thread Florian Weimer

On 11/11/2014 11:33 AM, Prathamesh Kulkarni wrote:

Why does moving definition of simplify constructor outside the class cause
out of memory allocation ?


It's probably because you dropped the initialization of the capture_max 
member.


--
Florian Weimer / Red Hat Product Security


Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign

2014-11-11 Thread Richard Biener
On Tue, Nov 11, 2014 at 8:26 AM, Jakub Jelinek  wrote:
> On Mon, Nov 10, 2014 at 05:27:50PM -0500, David Malcolm wrote:
>> On Sat, 2014-11-08 at 14:56 +0100, Jakub Jelinek wrote:
>> > On Sat, Nov 08, 2014 at 01:07:28PM +0100, Richard Biener wrote:
>> > > To be constructive here - the above case is from within a
>> > > GIMPLE_ASSIGN case label
>> > > and thus I'd have expected
>> > >
>> > > case GIMPLE_ASSIGN:
>> > >   {
>> > > gassign *a1 = as_a  (s1);
>> > > gassign *a2 = as_a  (s2);
>> > >   lhs1 = gimple_assign_lhs (a1);
>> > >   lhs2 = gimple_assign_lhs (a2);
>> > >   if (TREE_CODE (lhs1) != SSA_NAME
>> > >   && TREE_CODE (lhs2) != SSA_NAME)
>> > > return (operand_equal_p (lhs1, lhs2, 0)
>> > > && gimple_operand_equal_value_p (gimple_assign_rhs1 (a1),
>> > >  gimple_assign_rhs1 
>> > > (a2)));
>> > >   else if (TREE_CODE (lhs1) == SSA_NAME
>> > >&& TREE_CODE (lhs2) == SSA_NAME)
>> > > return vn_valueize (lhs1) == vn_valueize (lhs2);
>> > >   return false;
>> > >   }
>> > >
>> > > instead.  That's the kind of changes I have expected and have approved 
>> > > of.
>> >
>> > But even that looks like just adding extra work for all developers, with no
>> > gain.  You only have to add extra code and extra temporaries, in switches
>> > typically also have to add {} because of the temporaries and thus extra
>> > indentation level, and it doesn't simplify anything in the code.
>>
>> The branch attempts to use the C++ typesystem to capture information
>> about the kinds of gimple statement we expect, both:
>>   (A) so that the compiler can detect type errors, and
>>   (B) as a comprehension aid to the human reader of the code
>>
>> The ideal here is when function params and struct field can be
>> strengthened from "gimple" to a subclass ptr.  This captures the
>> knowledge that every use of a function or within a struct has a given
>> gimple code.
>
> I just don't like all the as_a/is_a stuff enforced everywhere,
> it means more typing, more temporaries, more indentation.
> So, as I view it, instead of the checks being done cheaply (yes, I think
> the gimple checking as we have right now is very cheap) under the
> hood by the accessors (gimple_assign_{lhs,rhs1} etc.), those changes
> put the burden on the developers, who has to check that manually through
> the as_a/is_a stuff everywhere, more typing and uglier syntax.
> I just don't see that as a step forward, instead a huge step backwards.
> But perhaps I'm alone with this.
> Can you e.g. compare the size of - lines in your patchset combined, and
> size of + lines in your patchset?  As in, if your changes lead to less
> typing or more.

I see two ways out here.  One is to add overloads to all the functions
taking the special types like

tree
gimple_assign_rhs1 (gimple *);

or simply add

gassign *operator ()(gimple *g) { return as_a  (g); }

into a gimple-compat.h header which you include in places that
are not converted "nicely".

Both avoid manually making the compiler happy (which the
explicit as_a<> stuff is!  It doesn't add any "checking" - it's
just placing the as_a<> at the callers and thus make the
runtine ICE fire there).

As much as I don't like "global" conversion operators I don't
like adding overloads to all of the accessor functions even more.

Whether you enable them generally or just for selected files
via a gimple-compat.h will be up to you (but I'd rather get
rid of them at some point).

Note this allows seamless transform of "random" functions
taking a gimple now but really only expecting a single kind.

Note that we don't absolutely have to rush this all in for GCC 5.
Being the very first for GCC 6 stage1 is another possibility.
We just should get it right.

Thanks,
Richard.


> Jakub


Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign

2014-11-11 Thread Bernd Schmidt

On 11/11/2014 09:30 AM, Eric Botcazou wrote:

I just don't like all the as_a/is_a stuff enforced everywhere,
it means more typing, more temporaries, more indentation.
So, as I view it, instead of the checks being done cheaply (yes, I think
the gimple checking as we have right now is very cheap) under the
hood by the accessors (gimple_assign_{lhs,rhs1} etc.), those changes
put the burden on the developers, who has to check that manually through
the as_a/is_a stuff everywhere, more typing and uglier syntax.
I just don't see that as a step forward, instead a huge step backwards.
But perhaps I'm alone with this.


IMO that's the sort of things some of us were afraid of when the C++ switch
was being discussed and IIRC we were told this would not happen...


I'm with both of you on this.


Bernd




Re: Match-and-simplify and COND_EXPR

2014-11-11 Thread Richard Biener
On Tue, 11 Nov 2014, Richard Biener wrote:

> On Mon, 10 Nov 2014, Andrew Pinski wrote:
> 
> > On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener  wrote:
> > > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> > >
> > >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener  wrote:
> > >> > On Thu, 6 Nov 2014, Richard Biener wrote:
> > >> >
> > >> >> On Wed, 5 Nov 2014, Andrew Pinski wrote:
> > >> >>
> > >> >> > Hi,
> > >> >> >   I was trying to hook up tree-ssa-phiopt to match-and-simplify 
> > >> >> > using
> > >> >> > either gimple_build (or rather using gimple_simplify depending on if
> > >> >> > we want to produce cond_expr for conditional move).  I ran into a
> > >> >> > problem.
> > >> >> > With the pattern below:
> > >> >> > /* a ? 0 : 1 -> a if 0 and 1 are integral types. */
> > >> >> > (simplify
> > >> >> >   (cond_expr @0 integer_zerop integer_onep)
> > >> >> >   (if (INTEGRAL_TYPE_P (type))
> > >> >> > (convert @0)))
> > >> >>
> > >> >> Ok, so you are capturing a GENERIC expr here but nothing knows that.
> > >> >> It would work if you'd do (ugh)
> > >> >>
> > >> >> (for op (lt le eq ne ge gt)
> > >> >>  (simplify
> > >> >>   (cond_expr (op @0 @1) integer_zerop integer_onep)
> > >> >>   (if (INTEGRAL_TYPE_P (type))
> > >> >>(convert (op @0 @1)
> > >> >> (simplify
> > >> >>  (cond_expr SSA_NAME@0 integer_zerop integer_onep)
> > >> >>   (if (INTEGRAL_TYPE_P (type))
> > >> >>(convert @0
> > >> >>
> > >> >> as a workaround.  To make your version work will require (quite)
> > >> >> some special-casing in the code generator or maybe the resimplify
> > >> >> helper.  Let me see if I can cook up a "simple" fix.
> > >> >
> > >> > Sth like below (for the real fix this has to be replicated in
> > >> > all gimple_resimplifyN functions).  I'm missing a testcase
> > >> > where the pattern would apply (and not be already folded by fold),
> > >> > so I didn't check if it actually works.
> > >>
> > >> You do need to check if seq is NULL though as gimple_build depends on
> > >> seq not being NULL.  But otherwise yes this works for me.
> > >
> > > Ok, here is a better and more complete fix.  The optimization
> > > whether a captured expression may be a GENERIC condition isn't
> > > implemented so a lot of checks are emitted right now.  Also
> > > if gimple_build would fail this terminates the whole matching
> > > process, not just matching of the current pattern (failing "late"
> > > isn't supported with the style of code we emit - well, without
> > > adding ugly gotos and labels).
> > >
> > > At least it avoids splitting up conditions if substituted into
> > > a COND_EXPR, so simplifications of COND_EXPRs in-place (w/o seq)
> > > should still work.
> > >
> > > Can you check whether this works for you?
> > 
> > This works for most cases but does not work for things like:
> > /* a != b ? a : b -> a */
> > (simplify
> >   (cond_expr (ne @0 @1) @0 @1)
> >   @0)
> >
> > In gimple_simplify after it matches COND_EXPR. It does not look into
> > the operand 0 to see if it matches NE_EXPR, it just sees if it was a
> > SSA_NAME.  In this case I was trying to do a simple replacement of
> > value_replacement in tree-ssa-phiopt.c.
> 
> Yeah - I hoped you wouldn't notice.  It's not too difficult to fix.
> Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
> it is better to finally fix it with
> 
> Index: gcc/gimple-expr.c
> ===
> --- gcc/gimple-expr.c   (revision 216973)
> +++ gcc/gimple-expr.c   (working copy)
> @@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
>  bool
>  is_gimple_condexpr (tree t)
>  {
> -  return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
> -   && !tree_could_throw_p (t)
> -   && is_gimple_val (TREE_OPERAND (t, 0))
> -   && is_gimple_val (TREE_OPERAND (t, 1;
> +  return is_gimple_val (t);
>  }
>  
>  /* Return true if T is a gimple address.  */
> 
> I expect mostly fallout from passes building [VEC_]COND_EXPRs and
> not gimplifying it and of course missed optimizations from the
> vectorizer which will likely now try to vectorize the separate
> comparisons in some maybe odd way (not sure).
> 
> I did this experiment once but didn't finish it.  Too bad :/
> 
> > But for my purpose right now this is sufficient and will be posting a
> > patch which does not remove things from tree-ssa-phiopt.c just yet.
> 
> Fair enough.  I suppose for GCC 5 I will need to deal with the odd
> GIMPLE IL.  At least I can consider it a "bug" and thus fix this
> during stage3 ;)

Well.  Like the following which creates the expected code for

(simplify
 (cond_expr (eq @0 @1) @0 @1)
 @1)

I'll install this on the branch and work on the other patch to
clean it up a bit.

Richard.


2014-11-11  Richard Biener  

* genmatch.c (struct expr): Add is_generic member.
(cmp_operand): Also compare is_generic flag.
(dt_operand::gen_gimple_expr): If is_gen

Re: How is libtool updated in GCC?

2014-11-11 Thread Joseph Myers
On Tue, 11 Nov 2014, FX wrote:

> > If you import rather than selectively merging one change you need (I 
> > think) to revert libtool commit 3334f7ed5851ef1e96b052f2984c4acdbf39e20c 
> > (incompatible with GCC handling of sysroots), as well as updating all 
> > three relevant repositories (GCC, binutils-gdb, src - I'm not sure if 
> > anything in binutils-gdb actually uses libtool but it has the files at 
> > toplevel).
> 
> So I’ve actually chosen to avoid the mess and selectly merge the one 
> change that we need. Looking at the history of libtool.m4 in our tree, 
> it looks like others have done that in the past, too.
> 
> Thanks for the feedback. On the src repository, is it documented 
> somewhere how to change it? (I have probably done it in the past, but 
> don’t remember)

The src repository in this context is about newlib / libgloss.  Update the 
shared files, regenerate affected files in subdirectories with whatever 
version of auto* was previously used (which may not be fully consistent 
everywhere).

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

[RFC] UBSan unsafely uses VRP

2014-11-11 Thread Marat Zakirov

Hi all!

I found that UBSan uses vrp pass to optimize generated checks. Keeping 
in mind that vrp pass is about performance not stability I found example 
where UBSan may skip true positive.


Example came from spec2006 perlbench:

int ext;

int
Perl_do_sv_dump()
{
int freq[10];
int i;
int max = 0;
int t = INT_MAX - 20;

if (max < ext)
  max = ext;

for (i = 0; i <= max; i++)
if (freq[i])
  ext = 0;

t += i;  <<< (*)
return t;
}

vrp pass here sets vrp('i') to [0..10] in assumption that 'freq[i]' wont 
violate array bound (vrp uses loop iteration number calculation, see 
adjust_range_with_scev in tree-vrp.c). This means that UBSAN_CHECK_ADD 
build for (*) should be deleted as redundant (and actually it is deleted 
by vrp pass). So if at the execution max = 30, freq[5] != 0 uncaught 
overflow will occur.


There are also some unsafe code in functions 
ubsan_expand_si_overflow_addsub_check, 
ubsan_expand_si_overflow_mul_check which uses get_range_info to reduce 
checks number. As seen before vrp usage for sanitizers may decrease 
quality of error detection.


Potentially unsafe vrp decisions should preferably not be exploited by 
any sanitizer pass. I have several ideas how we could achieve that:

1) Do not use vrp info for sanitizers.
2) Do not use unsafe conclusions during vrp pass if sanitizers are enabled.
3) Make two versions of value range data safe and unsafe.

P.S.

To reproduce the issue use appropriate GCC options 
(-fsanitize=signed-integer-overflow -S -O3) and attached trivial patch 
which disables UBSan check build in inner loops. This patch is needed 
because currently iteration number calculation used by vrp pass do not 
supports yet UBSAN_CHECK_[ADD/SUB/MUL].


--Marat
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index dde0418..97dbf33 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -1014,6 +1014,10 @@ instrument_si_overflow (gimple_stmt_iterator gsi)
   || GET_MODE_BITSIZE (TYPE_MODE (lhstype)) != TYPE_PRECISION (lhstype))
 return;
 
+  // skip for inner loops 
+  if (gimple_bb(stmt)->loop_father->inner == NULL)
+return;
+
   switch (code)
 {
 case MINUS_EXPR:


Re: [RFC] UBSan unsafely uses VRP

2014-11-11 Thread Jakub Jelinek
On Tue, Nov 11, 2014 at 05:02:55PM +0300, Marat Zakirov wrote:
> I found that UBSan uses vrp pass to optimize generated checks. Keeping in
> mind that vrp pass is about performance not stability I found example where
> UBSan may skip true positive.
> 
> Example came from spec2006 perlbench:
> 
> int ext;
> 
> int
> Perl_do_sv_dump()
> {
> int freq[10];
> int i;
> int max = 0;
> int t = INT_MAX - 20;
> 
> if (max < ext)
>   max = ext;
> 
> for (i = 0; i <= max; i++)
> if (freq[i])
>   ext = 0;
> 
> t += i;  <<< (*)
> return t;
> }
> 
> vrp pass here sets vrp('i') to [0..10] in assumption that 'freq[i]' wont
> violate array bound (vrp uses loop iteration number calculation, see
> adjust_range_with_scev in tree-vrp.c). This means that UBSAN_CHECK_ADD build
> for (*) should be deleted as redundant (and actually it is deleted by vrp
> pass). So if at the execution max = 30, freq[5] != 0 uncaught overflow will
> occur.

Well, if max is >= 10, then you should get -fsanitize=bounds error already.
-fsanitize=undefined already disables -faggressive-loop-optimizations,
perhaps it can also disable other optimizations (I thought deriving number
of iterations from assuming undefined behavior doesn't occur in loop stmts
is already guarded by -faggressive-loop-optimizations though).

> There are also some unsafe code in functions
> ubsan_expand_si_overflow_addsub_check, ubsan_expand_si_overflow_mul_check
> which uses get_range_info to reduce checks number. As seen before vrp usage
> for sanitizers may decrease quality of error detection.

Using VRP is completely intentional there, we don't want to generate too
slow code if you decide you want to optimize your code (for -O0 VRP isn't
performed of course).

Jakub


Re: [RFC] UBSan unsafely uses VRP

2014-11-11 Thread Richard Biener
On Tue, Nov 11, 2014 at 3:15 PM, Jakub Jelinek  wrote:
> On Tue, Nov 11, 2014 at 05:02:55PM +0300, Marat Zakirov wrote:
>> I found that UBSan uses vrp pass to optimize generated checks. Keeping in
>> mind that vrp pass is about performance not stability I found example where
>> UBSan may skip true positive.
>>
>> Example came from spec2006 perlbench:
>>
>> int ext;
>>
>> int
>> Perl_do_sv_dump()
>> {
>> int freq[10];
>> int i;
>> int max = 0;
>> int t = INT_MAX - 20;
>>
>> if (max < ext)
>>   max = ext;
>>
>> for (i = 0; i <= max; i++)
>> if (freq[i])
>>   ext = 0;
>>
>> t += i;  <<< (*)
>> return t;
>> }
>>
>> vrp pass here sets vrp('i') to [0..10] in assumption that 'freq[i]' wont
>> violate array bound (vrp uses loop iteration number calculation, see
>> adjust_range_with_scev in tree-vrp.c). This means that UBSAN_CHECK_ADD build
>> for (*) should be deleted as redundant (and actually it is deleted by vrp
>> pass). So if at the execution max = 30, freq[5] != 0 uncaught overflow will
>> occur.
>
> Well, if max is >= 10, then you should get -fsanitize=bounds error already.
> -fsanitize=undefined already disables -faggressive-loop-optimizations,
> perhaps it can also disable other optimizations (I thought deriving number
> of iterations from assuming undefined behavior doesn't occur in loop stmts
> is already guarded by -faggressive-loop-optimizations though).

You could use -fno-strict-overflow ...

>> There are also some unsafe code in functions
>> ubsan_expand_si_overflow_addsub_check, ubsan_expand_si_overflow_mul_check
>> which uses get_range_info to reduce checks number. As seen before vrp usage
>> for sanitizers may decrease quality of error detection.
>
> Using VRP is completely intentional there, we don't want to generate too
> slow code if you decide you want to optimize your code (for -O0 VRP isn't
> performed of course).

Indeed.  Note that the strict-overflow warnings are already a bad burden
on VRP quality - a way out to me was always to track two lattices,
one assuming strict-overflow and one assuming wrapping overflow.
For strict-overflow warnings you then can compare simplification outcome
against two lattices and warn if the result differs.  Instead of that odd
+-INF(OVF) saturation.

Richard.

> Jakub


Re: Match-and-simplify and COND_EXPR

2014-11-11 Thread Richard Biener
On Tue, 11 Nov 2014, Richard Biener wrote:

> On Tue, 11 Nov 2014, Richard Biener wrote:
> 
> > On Mon, 10 Nov 2014, Andrew Pinski wrote:
> > 
> > > On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener  wrote:
> > > > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> > > >
> > > >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener  
> > > >> wrote:
> > > >> > On Thu, 6 Nov 2014, Richard Biener wrote:
> > > >> >
> > > >> >> On Wed, 5 Nov 2014, Andrew Pinski wrote:
> > > >> >>
> > > >> >> > Hi,
> > > >> >> >   I was trying to hook up tree-ssa-phiopt to match-and-simplify 
> > > >> >> > using
> > > >> >> > either gimple_build (or rather using gimple_simplify depending on 
> > > >> >> > if
> > > >> >> > we want to produce cond_expr for conditional move).  I ran into a
> > > >> >> > problem.
> > > >> >> > With the pattern below:
> > > >> >> > /* a ? 0 : 1 -> a if 0 and 1 are integral types. */
> > > >> >> > (simplify
> > > >> >> >   (cond_expr @0 integer_zerop integer_onep)
> > > >> >> >   (if (INTEGRAL_TYPE_P (type))
> > > >> >> > (convert @0)))
> > > >> >>
> > > >> >> Ok, so you are capturing a GENERIC expr here but nothing knows that.
> > > >> >> It would work if you'd do (ugh)
> > > >> >>
> > > >> >> (for op (lt le eq ne ge gt)
> > > >> >>  (simplify
> > > >> >>   (cond_expr (op @0 @1) integer_zerop integer_onep)
> > > >> >>   (if (INTEGRAL_TYPE_P (type))
> > > >> >>(convert (op @0 @1)
> > > >> >> (simplify
> > > >> >>  (cond_expr SSA_NAME@0 integer_zerop integer_onep)
> > > >> >>   (if (INTEGRAL_TYPE_P (type))
> > > >> >>(convert @0
> > > >> >>
> > > >> >> as a workaround.  To make your version work will require (quite)
> > > >> >> some special-casing in the code generator or maybe the resimplify
> > > >> >> helper.  Let me see if I can cook up a "simple" fix.
> > > >> >
> > > >> > Sth like below (for the real fix this has to be replicated in
> > > >> > all gimple_resimplifyN functions).  I'm missing a testcase
> > > >> > where the pattern would apply (and not be already folded by fold),
> > > >> > so I didn't check if it actually works.
> > > >>
> > > >> You do need to check if seq is NULL though as gimple_build depends on
> > > >> seq not being NULL.  But otherwise yes this works for me.
> > > >
> > > > Ok, here is a better and more complete fix.  The optimization
> > > > whether a captured expression may be a GENERIC condition isn't
> > > > implemented so a lot of checks are emitted right now.  Also
> > > > if gimple_build would fail this terminates the whole matching
> > > > process, not just matching of the current pattern (failing "late"
> > > > isn't supported with the style of code we emit - well, without
> > > > adding ugly gotos and labels).
> > > >
> > > > At least it avoids splitting up conditions if substituted into
> > > > a COND_EXPR, so simplifications of COND_EXPRs in-place (w/o seq)
> > > > should still work.
> > > >
> > > > Can you check whether this works for you?
> > > 
> > > This works for most cases but does not work for things like:
> > > /* a != b ? a : b -> a */
> > > (simplify
> > >   (cond_expr (ne @0 @1) @0 @1)
> > >   @0)
> > >
> > > In gimple_simplify after it matches COND_EXPR. It does not look into
> > > the operand 0 to see if it matches NE_EXPR, it just sees if it was a
> > > SSA_NAME.  In this case I was trying to do a simple replacement of
> > > value_replacement in tree-ssa-phiopt.c.
> > 
> > Yeah - I hoped you wouldn't notice.  It's not too difficult to fix.
> > Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
> > it is better to finally fix it with
> > 
> > Index: gcc/gimple-expr.c
> > ===
> > --- gcc/gimple-expr.c   (revision 216973)
> > +++ gcc/gimple-expr.c   (working copy)
> > @@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
> >  bool
> >  is_gimple_condexpr (tree t)
> >  {
> > -  return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
> > -   && !tree_could_throw_p (t)
> > -   && is_gimple_val (TREE_OPERAND (t, 0))
> > -   && is_gimple_val (TREE_OPERAND (t, 1;
> > +  return is_gimple_val (t);
> >  }
> >  
> >  /* Return true if T is a gimple address.  */
> > 
> > I expect mostly fallout from passes building [VEC_]COND_EXPRs and
> > not gimplifying it and of course missed optimizations from the
> > vectorizer which will likely now try to vectorize the separate
> > comparisons in some maybe odd way (not sure).
> > 
> > I did this experiment once but didn't finish it.  Too bad :/
> > 
> > > But for my purpose right now this is sufficient and will be posting a
> > > patch which does not remove things from tree-ssa-phiopt.c just yet.
> > 
> > Fair enough.  I suppose for GCC 5 I will need to deal with the odd
> > GIMPLE IL.  At least I can consider it a "bug" and thus fix this
> > during stage3 ;)
> 
> Well.  Like the following which creates the expected code for
> 
> (simplify
>  (cond_expr (eq @0 @1) 

Re: arm/thumb broken on head

2014-11-11 Thread Joel Sherrill

> Fix is committed to trunk at
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=217341.
>
> BR,
> Terry
>
That got past libgcc2 but now it fails building newlib for arm-eabi.
This is just a build of gcc with newlib and libgloss symlinked into
the source. You should be able to duplicate this. But if you need a
preprocessed file, I will send one.

/users/joel/test-gcc/b-arm-eabi-gcc/./gcc/xgcc
-B/users/joel/test-gcc/b-arm-eabi-gcc/./gcc/ -nostdinc
-B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/ -isystem
/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/targ-include
-isystem /users/joel/test-gcc/gcc/newlib/libc/include
-B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/arm
-L/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/libnosys
-L/users/joel/test-gcc/gcc/libgloss/arm
-B/users/joel/test-gcc/install-head/arm-eabi/bin/
-B/users/joel/test-gcc/install-head/arm-eabi/lib/ -isystem
/users/joel/test-gcc/install-head/arm-eabi/include -isystem
/users/joel/test-gcc/install-head/arm-eabi/sys-include  -mthumb
-DPACKAGE_NAME=\"newlib\" -DPACKAGE_TARNAME=\"newlib\"
-DPACKAGE_VERSION=\"2.1.0\" -DPACKAGE_STRING=\"newlib\ 2.1.0\"
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -I.
-I../../../../../../gcc/newlib/libc/stdlib -DARM_RDI_MONITOR
-fno-builtin  -g -O2  -mthumb -c -o lib_a-ecvtbuf.o `test -f
'ecvtbuf.c' || echo '../../../../../../gcc/newlib/libc/stdlib/'`ecvtbuf.c
In file included from
../../../../../../gcc/newlib/libc/stdlib/ecvtbuf.c:70:0:
/users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h: In function
'fcvtbuf':
/users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: error:
non-trivial conversion at assignment
 char * _EXFUN(fcvtbuf,(double, int, int*, int*, char *));
^
sizetype
int
_64 = _37;
/users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: internal
compiler error: verify_gimple failed
0x9602d1 verify_gimple_in_cfg(function*, bool)
../../gcc/gcc/tree-cfg.c:5039
0x86fd06 execute_function_todo
../../gcc/gcc/passes.c:1868
0x870733 execute_todo
../../gcc/gcc/passes.c:1925
Please submit a full bug report,
with preprocessed source if appropriate.

-- 
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherr...@oarcorp.comOn-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available(256) 722-9985



Re: arm/thumb broken on head

2014-11-11 Thread Ramana Radhakrishnan
On Tue, Nov 11, 2014 at 3:16 PM, Joel Sherrill
 wrote:
>
>> Fix is committed to trunk at
>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=217341.
>>
>> BR,
>> Terry
>>
> That got past libgcc2 but now it fails building newlib for arm-eabi.
> This is just a build of gcc with newlib and libgloss symlinked into
> the source. You should be able to duplicate this. But if you need a
> preprocessed file, I will send one.
>
> /users/joel/test-gcc/b-arm-eabi-gcc/./gcc/xgcc
> -B/users/joel/test-gcc/b-arm-eabi-gcc/./gcc/ -nostdinc
> -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/ -isystem
> /users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/targ-include
> -isystem /users/joel/test-gcc/gcc/newlib/libc/include
> -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/arm
> -L/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/libnosys
> -L/users/joel/test-gcc/gcc/libgloss/arm
> -B/users/joel/test-gcc/install-head/arm-eabi/bin/
> -B/users/joel/test-gcc/install-head/arm-eabi/lib/ -isystem
> /users/joel/test-gcc/install-head/arm-eabi/include -isystem
> /users/joel/test-gcc/install-head/arm-eabi/sys-include  -mthumb
> -DPACKAGE_NAME=\"newlib\" -DPACKAGE_TARNAME=\"newlib\"
> -DPACKAGE_VERSION=\"2.1.0\" -DPACKAGE_STRING=\"newlib\ 2.1.0\"
> -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -I.
> -I../../../../../../gcc/newlib/libc/stdlib -DARM_RDI_MONITOR
> -fno-builtin  -g -O2  -mthumb -c -o lib_a-ecvtbuf.o `test -f
> 'ecvtbuf.c' || echo '../../../../../../gcc/newlib/libc/stdlib/'`ecvtbuf.c
> In file included from
> ../../../../../../gcc/newlib/libc/stdlib/ecvtbuf.c:70:0:
> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h: In function
> 'fcvtbuf':
> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: error:
> non-trivial conversion at assignment
>  char * _EXFUN(fcvtbuf,(double, int, int*, int*, char *));
> ^
> sizetype
> int
> _64 = _37;
> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: internal
> compiler error: verify_gimple failed
> 0x9602d1 verify_gimple_in_cfg(function*, bool)
> ../../gcc/gcc/tree-cfg.c:5039
> 0x86fd06 execute_function_todo
> ../../gcc/gcc/passes.c:1868
> 0x870733 execute_todo
> ../../gcc/gcc/passes.c:1925
> Please submit a full bug report,
> with preprocessed source if appropriate.

That certainly looks like a different issue to the one with Terry's
patch. It does look like something else is broken in one of the tree
passes, maybe worth a bugzilla with pre-processed source and configury
options.


Ramana

>
> --
> Joel Sherrill, Ph.D. Director of Research & Development
> joel.sherr...@oarcorp.comOn-Line Applications Research
> Ask me about RTEMS: a free RTOS  Huntsville AL 35805
> Support Available(256) 722-9985
>


Re: arm/thumb broken on head

2014-11-11 Thread Joel Sherrill

On 11/11/2014 9:27 AM, Ramana Radhakrishnan wrote:
> On Tue, Nov 11, 2014 at 3:16 PM, Joel Sherrill
>  wrote:
>>> Fix is committed to trunk at
>>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=217341.
>>>
>>> BR,
>>> Terry
>>>
>> That got past libgcc2 but now it fails building newlib for arm-eabi.
>> This is just a build of gcc with newlib and libgloss symlinked into
>> the source. You should be able to duplicate this. But if you need a
>> preprocessed file, I will send one.
>>
>> /users/joel/test-gcc/b-arm-eabi-gcc/./gcc/xgcc
>> -B/users/joel/test-gcc/b-arm-eabi-gcc/./gcc/ -nostdinc
>> -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/ -isystem
>> /users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/newlib/targ-include
>> -isystem /users/joel/test-gcc/gcc/newlib/libc/include
>> -B/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/arm
>> -L/users/joel/test-gcc/b-arm-eabi-gcc/arm-eabi/thumb/libgloss/libnosys
>> -L/users/joel/test-gcc/gcc/libgloss/arm
>> -B/users/joel/test-gcc/install-head/arm-eabi/bin/
>> -B/users/joel/test-gcc/install-head/arm-eabi/lib/ -isystem
>> /users/joel/test-gcc/install-head/arm-eabi/include -isystem
>> /users/joel/test-gcc/install-head/arm-eabi/sys-include  -mthumb
>> -DPACKAGE_NAME=\"newlib\" -DPACKAGE_TARNAME=\"newlib\"
>> -DPACKAGE_VERSION=\"2.1.0\" -DPACKAGE_STRING=\"newlib\ 2.1.0\"
>> -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -I.
>> -I../../../../../../gcc/newlib/libc/stdlib -DARM_RDI_MONITOR
>> -fno-builtin  -g -O2  -mthumb -c -o lib_a-ecvtbuf.o `test -f
>> 'ecvtbuf.c' || echo '../../../../../../gcc/newlib/libc/stdlib/'`ecvtbuf.c
>> In file included from
>> ../../../../../../gcc/newlib/libc/stdlib/ecvtbuf.c:70:0:
>> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h: In function
>> 'fcvtbuf':
>> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: error:
>> non-trivial conversion at assignment
>>  char * _EXFUN(fcvtbuf,(double, int, int*, int*, char *));
>> ^
>> sizetype
>> int
>> _64 = _37;
>> /users/joel/test-gcc/gcc/newlib/libc/include/stdlib.h:175:8: internal
>> compiler error: verify_gimple failed
>> 0x9602d1 verify_gimple_in_cfg(function*, bool)
>> ../../gcc/gcc/tree-cfg.c:5039
>> 0x86fd06 execute_function_todo
>> ../../gcc/gcc/passes.c:1868
>> 0x870733 execute_todo
>> ../../gcc/gcc/passes.c:1925
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
> That certainly looks like a different issue to the one with Terry's
> patch. It does look like something else is broken in one of the tree
> passes, maybe worth a bugzilla with pre-processed source and configury
> options.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63817

I think that has enough detail. If not, just ask.
>
> Ramana
>
>> --
>> Joel Sherrill, Ph.D. Director of Research & Development
>> joel.sherr...@oarcorp.comOn-Line Applications Research
>> Ask me about RTEMS: a free RTOS  Huntsville AL 35805
>> Support Available(256) 722-9985
>>

-- 
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherr...@oarcorp.comOn-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available(256) 722-9985



Re: [RFC] UBSan unsafely uses VRP

2014-11-11 Thread Marat Zakirov

On 11/11/2014 05:26 PM, Richard Biener wrote:

On Tue, Nov 11, 2014 at 3:15 PM, Jakub Jelinek  wrote:

On Tue, Nov 11, 2014 at 05:02:55PM +0300, Marat Zakirov wrote:

I found that UBSan uses vrp pass to optimize generated checks. Keeping in
mind that vrp pass is about performance not stability I found example where
UBSan may skip true positive.

Example came from spec2006 perlbench:

int ext;

int
Perl_do_sv_dump()
{
 int freq[10];
 int i;
 int max = 0;
 int t = INT_MAX - 20;

 if (max < ext)
   max = ext;

 for (i = 0; i <= max; i++)
 if (freq[i])
   ext = 0;

 t += i;  <<< (*)
 return t;
}

vrp pass here sets vrp('i') to [0..10] in assumption that 'freq[i]' wont
violate array bound (vrp uses loop iteration number calculation, see
adjust_range_with_scev in tree-vrp.c). This means that UBSAN_CHECK_ADD build
for (*) should be deleted as redundant (and actually it is deleted by vrp
pass). So if at the execution max = 30, freq[5] != 0 uncaught overflow will
occur.

Well, if max is >= 10, then you should get -fsanitize=bounds error already.
-fsanitize=undefined already disables -faggressive-loop-optimizations,
perhaps it can also disable other optimizations (I thought deriving number
of iterations from assuming undefined behavior doesn't occur in loop stmts
is already guarded by -faggressive-loop-optimizations though).

You could use -fno-strict-overflow ...


There are also some unsafe code in functions
ubsan_expand_si_overflow_addsub_check, ubsan_expand_si_overflow_mul_check
which uses get_range_info to reduce checks number. As seen before vrp usage
for sanitizers may decrease quality of error detection.

Using VRP is completely intentional there, we don't want to generate too
slow code if you decide you want to optimize your code (for -O0 VRP isn't
performed of course).

Indeed.  Note that the strict-overflow warnings are already a bad burden
on VRP quality - a way out to me was always to track two lattices,
one assuming strict-overflow and one assuming wrapping overflow.
For strict-overflow warnings you then can compare simplification outcome
against two lattices and warn if the result differs.  Instead of that odd
+-INF(OVF) saturation.

Richard.


 Jakub

It is seems that -fsanitize=something do not set 
flag_aggressive_loop_optimizations to 0 in current GCC version. I made a 
watchpoint on it but changes after init_options_struct weren't found. I 
will make fix for both 
flag_aggressive_loop_optimizationsno-strict-overflow and 
flag_strict_overflow.


--Marat


Re: More useful support for low-end ARM architecture

2014-11-11 Thread Joey Ye
Markus,

-mmcu probably will not work for ARM architectured MCUs. Reason are
* Confusion. -mcpu is encouraged and already widely used for ARM
architectures. Introducing -mmcu will be very confusing.
* Expensive effort. Either supporting none, or supporting all. There
are large number of MCUs from ARM eco-system partners. Supporting all
of them is a large project.
* Maintance nightmare. Having GCC developers to input and maintain
vendor specific configurations is inefficient and error-prone.
* Failed schedule dependence. Having -mmcu actually means whenever
there is a new MCU introduced into market, GCC has to be updated to
describe it. Given GCC's release cycle (once per year) and the
frequency of MCU release (could be tens each year), GCC will never be
able to catch up.

Further more all the board/MCU specific configurations are already
described in CMSIS and variant of GUI based toolchain for ARM MCU.
Replicating then in GCC does not sounds a right thing to do for me.

From the link you shared I also noticed some other discussion
regarding makefile, libraries and other options, which are very
interesting but off scope of GCC mail list. Shall we please continue
the discuss at https://answers.launchpad.net/gcc-arm-embedded/+question/257326
?

Thanks,
Joey

On Sun, Nov 9, 2014 at 3:42 AM, Markus Hitter  wrote:
> Hello gcc folks,
>
> recently I started to expand a project of mine running mainly on AVR ATmega 
> to low end ARM chips. To my enlightment, gcc supports these thingies already. 
> To my disappointment, this support is pretty complicated. One faces at least 
> a much steeper learning curve that on AVR. Accordingly I suggested on the 
> avr-libc mailing list to do similar work for ARM, Cortex-M0 to Cortex-M4. At 
> least four people expressed interest, it looks like arm-libc is about to be 
> born.
>
> To those not knowing what this is, I talk here about all-in-one CPUs (MCUs) 
> with memory and some peripherals already on the chip. Program memory can be 
> as low as 8 kB, RAM as low as 1 kB. Usually they're programmed bare-metal, 
> this is, without any operating system.
>
> If you want to take a look at a simple Hello World application, here is one:
>
> https://bugs.launchpad.net/gcc-arm-embedded/+bug/1387906
>
> Looking at its Makefile, it requires quite a number of flags, among them nine 
> -I with custom paths, --specs, -T and also auto-generated C files. Lots of 
> stuff average programmers probably don't even know it exists. One of the 
> interested persons on the avr-libc mailing list explained what's missing, 
> much better than I could:
>
>> I think what the other responders missed is that avr-libc (via its
>> integration with binutils and gcc) gives you two key pieces of
>> functionality:
>>
>> -mmcu=atmega88
>> #include 
>>
>> You *also* get classic libc functionality (printf, etc) that's provided
>> on ARM by newlib etc, but that's not the big deal IMO.
>>
>> The #include is *relatively* easy, [... no topic for gcc ...]
>>
>> The -mmcu= functionality is even more deeply useful, although less
>> easily repeatable on ARM. It brings in the relevant linker script,
>> startup code, vector tables, and all the other infrastructure. *THAT*
>> is what makes it possible to write a program like:
>>
>> #include 
>> int main() {
>>   DDRD = 0x01;PORTD = 0x01;
>> }
>>
>> # avr-gcc -mmcu=atmega88 -o test test.c
>> # avrdude
>>
>> Writing a program for your random ARM chip requires digging *deeply*
>> into the various websites or IDEs of the manufacturer, trying to find
>> the right files (the filenames alone of which vary in strange ways),
>> probably determining the right way to alter them because the only
>> example you found was for a different chip in the same line, and then
>> hoping you've got everything glued together properly.
>>
>> I want to be able to write the above program (modified for the right
>> GPIO) and run:
>>
>> # arm-none-eabi-gcc -mmcu=stm32f405 -o test test.c
>
> This is why I joined here, we'd like to get -mmcu for all the ARM flavours. 
> It should pick up a linker script which works in most cases on its own. It 
> should also bring in startup code, so code writers don't have to mess with 
> stuff happening before main(). And not to forget, pre-set #defines like 
> __ARM_LPC1114__, __ARM_STM32F405__, etc.
>
> - How would we proceed in general?
>
> - Many flavours at once, or better start with one or two, adding more when 
> these work?
>
> - Did AVR support make things we should not repeat?
>
>
> Thanks for discussing,
>
> Markus
>
> P.S.: arm-libc discussion so far can be followed here:
> http://lists.nongnu.org/archive/html/avr-libc-dev/2014-11/threads.html
>
> --
> - - - - - - - - - - - - - - - - - - -
> Dipl. Ing. (FH) Markus Hitter
> http://www.jump-ing.de/


Re: [RFC] UBSan unsafely uses VRP

2014-11-11 Thread Jakub Jelinek
On Tue, Nov 11, 2014 at 07:12:55PM +0300, Marat Zakirov wrote:
> It is seems that -fsanitize=something do not set
> flag_aggressive_loop_optimizations to 0 in current GCC version. I made a
> watchpoint on it but changes after init_options_struct weren't found. I will
> make fix for both flag_aggressive_loop_optimizationsno-strict-overflow and
> flag_strict_overflow.

Ah, you're right, we only have:
/* When instrumenting the pointers, we don't want to remove
   the null pointer checks.  */
if (opts->x_flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE
 | SANITIZE_RETURNS_NONNULL_ATTRIBUTE))
  opts->x_flag_delete_null_pointer_checks = 0;
and as Joseph said, even that is misplaced, it should be done after all
options are processed, so that it isn't dependent on whether
-fsanitize=undefined or
--fdelete-null-pointer-checks/-faggressive-loop-optimizations come first.

Jakub


Re: More useful support for low-end ARM architecture

2014-11-11 Thread Joern Rennecke
On 11 November 2014 16:22, Joey Ye  wrote:
> Markus,
>
> -mmcu probably will not work for ARM architectured MCUs. Reason are
> * Confusion. -mcpu is encouraged and already widely used for ARM
> architectures. Introducing -mmcu will be very confusing.

It' just a matter of bundling specifications.  -mcpu specifies an
architecture and
tuning option.
-mmcu specifies that (it you have an -mcpu option, you best translate
it to that,
otherwise you can skip that step), plus additional things that pertain the the
parts an mcu consists of besides an mcu.

> * Expensive effort. Either supporting none, or supporting all. There
> are large number of MCUs from ARM eco-system partners. Supporting all
> of them is a large project.
> * Maintance nightmare. Having GCC developers to input and maintain
> vendor specific configurations is inefficient and error-prone.
> * Failed schedule dependence. Having -mmcu actually means whenever
> there is a new MCU introduced into market, GCC has to be updated to
> describe it. Given GCC's release cycle (once per year) and the
> frequency of MCU release (could be tens each year), GCC will never be
> able to catch up.

These kinds of issues were why I re-implemented the avr -mmcu option
with the SELF_SPEC mechanism to read a device-specific spec file.

As long as a new mcu can be described with the existing toolchain facilities
(e.g. a new combination of existing I/O support, some different parameters
 describing RAM / flash sizes), a new spec file can be distributed together
with a few hardware-specific library/crt files to support a new mcu with
an existing compiler.

> Further more all the board/MCU specific configurations are already
> described in CMSIS and variant of GUI based toolchain for ARM MCU.
> Replicating then in GCC does not sounds a right thing to do for me.

Yes, better to have a script that translates this info into a suitable spec file
and copies any required files in the appropriate installation places.
Not sure what kind of Copyright/licensing issues that would entail, though.


GCC Bugzilla disables caching of linked content

2014-11-11 Thread Jonathan Wakely
Hi Frédéric,

At some point GCC's bugzilla started taking ages to load, because
every single .css and .js file gets a query appended to the URL:

skins/contrib/Dusk/global.css?1368269827

This causes Firefox to constantly re-fetch those pages again and
again, so it takes several seconds to load every. single. page.

Do you know why this is?

Can we turn it off?


Re: GCC Bugzilla disables caching of linked content

2014-11-11 Thread Frédéric Buclin
Le 11. 11. 14 20:11, Jonathan Wakely a écrit :
> At some point GCC's bugzilla started taking ages to load, because
> every single .css and .js file gets a query appended to the URL:
> 
> skins/contrib/Dusk/global.css?1368269827
> 
> This causes Firefox to constantly re-fetch those pages again and
> again, so it takes several seconds to load every. single. page.
> 
> Do you know why this is?

1368269827 is the timestamp when these files were last modified. It's
appended to the URL so that your web browser doesn't refetch them if it
already has them in cache with this timestamp. So if your web browser
refetches them all the time, then you have something wrong with your
browser. In my case, pages load very quickly because none of the CSS or
JS files are reloaded. You cannot disable this feature.


Frédéric



Inconsistent GOT base pointer register usage in x86-64 psABI

2014-11-11 Thread H.J. Lu
Register Usage table in x86-64 psABI has

%rbx  callee-saved register; optionally used as base pointer

However, everywhere it uses %r15 to store the GOT address,
including PLT for large model.

It is a typo to mark RBX as GOT base register in Register Usage table.
GCC large model Linux implementation isn't tested nor verified.  The
only real usage is

rdos64.h:#define REAL_PIC_OFFSET_TABLE_REGNUM  R15_REG

which is correct.

We should fix Register Usage table in x86-64 psABI with the enclosed patch.

 Any comments?

-- 
H.J.
---
diff --git a/low-level-sys-info.tex b/low-level-sys-info.tex
index ef901d2..95a3f6f 100644
--- a/low-level-sys-info.tex
+++ b/low-level-sys-info.tex
@@ -572,7 +572,7 @@ bit.}.
 \RAX & temporary register; with variable arguments passes
 information about the number of vector registers used; 1$^{\rm st}$
 return register & No \\
-\RBX & callee-saved register; optionally used as base pointer & Yes \\
+\RBX & callee-saved register & Yes \\
 \RCX & used to pass 4$^{\rm th}$ integer argument to functions & No \\
 \RDX & used to pass 3$^{\rm rd}$ argument to functions; 2$^{\rm nd}$
return register & No \\
 \RSP & stack pointer & Yes \\
@@ -584,7 +584,8 @@ return register & No \\
 \reg{r10} & temporary register, used for passing a function's static
 chain pointer & No \\
 \reg{r11} & temporary register & No\\
-\reg{r12--r15} & callee-saved registers & Yes \\
+\reg{r12--r14} & callee-saved registers & Yes \\
+\reg{r15} & callee-saved register; optionally used as GOT base pointer & Yes \\
 \reg{xmm0}--\reg{xmm1} & used to pass and return floating point
 arguments & No\\
 \reg{xmm2}--\reg{xmm7} & used to pass floating point arguments & No\\


Re: GCC Bugzilla disables caching of linked content

2014-11-11 Thread Jonathan Wakely
On 11 November 2014 19:45, Frédéric Buclin wrote:
> Le 11. 11. 14 20:11, Jonathan Wakely a écrit :
>> At some point GCC's bugzilla started taking ages to load, because
>> every single .css and .js file gets a query appended to the URL:
>>
>> skins/contrib/Dusk/global.css?1368269827
>>
>> This causes Firefox to constantly re-fetch those pages again and
>> again, so it takes several seconds to load every. single. page.
>>
>> Do you know why this is?
>
> 1368269827 is the timestamp when these files were last modified. It's
> appended to the URL so that your web browser doesn't refetch them if it
> already has them in cache with this timestamp. So if your web browser
> refetches them all the time, then you have something wrong with your
> browser. In my case, pages load very quickly because none of the CSS or
> JS files are reloaded. You cannot disable this feature.

I'm just using Firefox from Fedora 20's repository, and I'm not the
only person seeing these pages being reloaded constantly.


Merge of jit branch to svn trunk as r217374

2014-11-11 Thread David Malcolm
I've merged the dmalcolm/jit git branch into svn trunk, as r217374. [1]
The git branch is now retired; future bugfixing will happen on svn
trunk.

I've created a "jit" component within the gcc.gnu.org/bugzilla, within
the "gcc" product, with dmalc...@gcc.gnu.org as the default assignee.

Please let me know if anything is broken, or if I broke any non-jit
builds.

Preliminary documentation can be seen at:
https://dmalcolm.fedorapeople.org/gcc/libgccjit-api-docs/index.html

In theory, that documentation ought to appear below
  https://gcc.gnu.org/onlinedocs
at some point, though I don't know the precise mechanism for making this
happen.  Note that the HTML docs can be built with Sphinx if available,
falling back to using texinfo if not.  The former gives superior output,
so if possible can Sphinx be installed on the relevant machine? [2]

Thanks everyone for their help with the jit effort.
Dave


[1] Specifically, I merged trunk r216746->r217317 into the jit branch as
497923878ef34b71d2dd995a76f41986f7d73ea, and ensured that the generated
libgccjit.texi is up-to-date.

I then generated the diff of the git branch against trunk as a patch,
and verified it on top of trunk, with a successful bootstrap and
regrtest on top of r217304 on x86_64-unknown-linux-gnu, with 16
common .sum files having identical results relative to a control
bootstrap of r217304, with a new jit.sum with: # of expected passes 4711
(both with --enable-host-shared)

I then doublechecked the build on top of an svn checkout of
trunk, both with and without the jit enabled, and committed it (to svn
trunk) as r217374.


[2] Specifically, gcc subdir's "configure" looks for a "sphinx-build"
executable via AC_CHECK_PROG.  I believe sphinx 1.0 or later is needed.




Re: Merge of jit branch to svn trunk as r217374

2014-11-11 Thread Joseph Myers
On Tue, 11 Nov 2014, David Malcolm wrote:

> In theory, that documentation ought to appear below
>   https://gcc.gnu.org/onlinedocs
> at some point, though I don't know the precise mechanism for making this
> happen.  Note that the HTML docs can be built with Sphinx if available,

maintainer-scripts/update_web_docs_svn needs updating for any new manual 
(and then htdocs/onlinedocs/index.html, in wwwdocs CVS, needs updating 
manually once the new manual has been generated).  It does not use the 
makefiles.

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


ubsan, asan testing is broken due to coloring

2014-11-11 Thread Andrew Pinski
With some configurations (looks like out of tree testing more than in
tree testing), all of ubsan and asan tests fail due to the
libsanitizer using coloring and that confuses the dejagnu pattern
matching. I don't have time to look fully into how to fix this issue
and I don't care much to coloring anyways so I disabled in the source
for my own use so the tests now pass.

Thanks,
Andrew