Re: [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

[Denis Chertykov]

2016-07-15 18:26 GMT+03:00 Georg-Johann Lay :
> This patch needs new hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE:
> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00839.html
>
> This patch turns attribute progmem into a working feature for AVR_TINY
> cores.
>
> It boils down to adding 0x4000 to all symbols with progmem:  Flash memory
> can be seen in the RAM address space starting at 0x4000, i.e. data in flash
> can be read by means of LD instruction if we add offsets of 0x4000.  There
> is no need for special access macros like pgm_read_* or special address
> spaces as there is nothing like a LPM instruction.
>
> This is simply achieved by setting a respective symbol_ref_flag, and when
> such a symbol has to be printed, then plus_constant with 0x4000 is used.
>
> Diagnosing of unsupported address spaces is now performed by
> TARGET_ADDR_SPACE_DIAGNOSE_USAGE which has exact location information.
> Hence there is no need to scan all decls for invalid address spaces.
>
> For AVR_TINY, alls address spaces have been disabled.  They are of no use.
> Supporting __flash would just make the backend more complicated without any
> gains.
>
>
> Ok for trunk?
>
> Johann
>
>
> gcc/
> * doc/extend.texi (AVR Variable Attributes) [progmem]: Add
> documentation how it works on reduced Tiny cores.
> (AVR Named Address Spaces): No support for reduced Tiny.
> * avr-protos.h (avr_addr_space_supported_p): New prototype.
> * avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
> (avr_address_tiny_pm_p): New static function.
> (avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
> if the address is in progmem.
> (avr_assemble_integer): Same.
> (avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
> for symbol_ref in progmem.
> (TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
> (avr_addr_space_diagnose_usage): ...and implementation.
> (avr_addr_space_supported_p): New function.
> (avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
> report bad address space usage if that space is supported.
> (avr_insert_attributes): Same.  No more complain about unsupported
> address spaces.
> * avr.h (AVR_TINY_PM_OFFSET): New macro.
> * avr-c.c (tm_p.h): Include it.
> (avr_cpu_cpp_builtins) [__AVR_TINY_PM_BASE_ADDRESS__]: Use
> AVR_TINY_PM_OFFSET instead of magic 0x4000 when built-in def'ing.
> Only define addr-space related built-in macro if
> avr_addr_space_supported_p.
> gcc/testsuite/
> * gcc.target/avr/torture/tiny-progmem.c: New test.
>

Approved.

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[Trevor Saunders]

On Sun, Jul 17, 2016 at 10:04:13PM -0400, Patrick Palka wrote:
> On Sun, 17 Jul 2016, David Edelsohn wrote:
> 
> > On Sun, Jul 17, 2016 at 10:43 AM, Patrick Palka  
> > wrote:
> > > On Sun, Jul 17, 2016 at 9:15 AM, David Edelsohn  wrote:
> > >>> Patrick Palka writes:
> > >>
> > >>> Hmm, this is only a problem due to command line length limits right?
> > >>> Fortunately the change makes the longest command line only about 10%
> > >>> longer than the previous longest command line.  With the change to
> > >>> avoid building libbackend.a altogether, the longest command line is
> > >>> now the cc1plus link command which is 4430 bytes long.  Without this
> > >>> change, the longest command line is the "ar rc libbackend.a"
> > >>> invocation at 4095 bytes (from what I can tell).  It's still pretty
> > >>> far away from the 8k limit imposed by Windows.
> > >>
> > >> Windows is not the only non-Linux system on which GCC runs.
> > >>
> > >> You repeatedly are making bad assumptions and assertions without
> > >> having studied much about GCC. You assume that GNU ar is the only
> > >> archiver in use. You propose removing libbackend.a without having
> > >> investigated when it was introduced and why.
> > >>
> > >> Your patches would be a lot more compelling if you invested the time
> > >> to learn some context.
> > >
> > > The only patch I officially proposed is the one elides the invocation
> > > of ranlib if the current ar is GNU ar (thanks to those who kindly
> > > mentioned that other ar's are supported).  Do you have any comments
> > > about this patch?
> > 
> > Testing for and utilizing a feature of GNU ar that speeds up builds
> > seems like a nice improvement, but it obviously adds complexity and
> > increases the risk that an independent change silently will introduce
> > build problems on some systems.
> > 
> > > And you're right.  I am sorry for suggesting ways to improve rebuild
> > > times without first uncovering the undocumented intricacies of the
> > > build system.  Shame on me!
> > 
> > Yes, GCC is a large and intricate Free Software project.  And
> > contributing patches requires some archaeology to understand the
> > motivation and reason for the design choices as part of the software
> > engineering process.  GCC supports and runs on a very large and
> > diverse set of systems, which is one facet of its strength.
> > 
> > One goal of GCC is evolution toward a more modular design:
> > 
> > https://gcc.gnu.org/wiki/ModularGCC
> > 
> > Thanks, David
> > 
> 
> I did some digging to figure out the origin of libbackend.a.  It was was
> created to work around a command line length limit on a VAX system
> (https://gcc.gnu.org/ml/gcc-bugs/2000-06/msg00438.html).  The 1600 byte
> command line that links cc1plus was too large for this system, so
> libbackend.a was introduced and used in place of $(OBJS) to reduce the
> size of this command line.
> 
> It's probably not safe to remove this archive even today since it
> currently stands for about 5000 bytes of object file names.  Simply
> removing it and having the build system refer to $(OBJS) will likely
> make us exceed 8k bytes in a command line somewhere.

yeah, though using response files where supported, and libraries
elsewhere is an option though somewhat complicated.

> I suppose rebuild times could instead be further reduced by splitting
> libbackend.a into multiple logical archives.  It would also be a small
> step towards making GCC more modular.

I'm not sure grouping object files really makes things more modular, but
if you could split some independent part of gcc into a reusable library
that could be built as its own shared object that seems useful for both
goals.

Trev

Re: Fix fir PR71696 in Libiberty Demangler (6)

[Marcel Böhme]

Hi,

This patch is still pending a full review.

Best regards,
- Marcel

> On 4 Jul 2016, at 8:54 PM, Bernd Schmidt  wrote:
> 
> On 06/30/2016 08:46 AM, Marcel Böhme wrote:
>> The attached patch fixes the stack overflow in the demangler due to
>> cycles in the references of “remembered” mangled types
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71696).
>> 
>> The methods demangle_signature and do_arg in cplus-dem.c allow to
>> “remember” mangled type names that can later be referenced and will
>> also be demangled. The method demangle_args demangles those types
>> following any references. So, if there is a cycle in the referencing
>> (or in the simplest case a self-reference), the method enters
>> infinite recursion.
>> 
>> The patch tracks the mangled types that are currently being demangled
>> in a new variable called work->proctypevec. If a referenced type is
>> currently being demangled, the demangling is marked as not
>> successful.
> 
> I'll defer reviewing these to someone who understands demangling better. You 
> might want to Cc Jason.
> 
> 
> Bernd
> 
> 

Re: Fix for PR70909 in Libiberty Demangler (4)

[Marcel Böhme]

Hi,

This patch is still pending a full review.

Best regards,
- Marcel

> On 30 Jun 2016, at 12:09 AM, Pedro Alves  wrote:
> 
> On 06/29/2016 08:43 AM, Marcel Böhme wrote:
>> Hi Jason,
>> 
>> These test cases are generated by fuzzing which produces a lot of 
>> nonsensical input data. 
>> I think, "Garbage In, Garbage Out" is quite applicable here.
>> With the patch at least it doesn’t crash and fixes the vulnerability.
> 
> Note that demangling shows up high in gdb profiles when loading
> huge programs.  If we can avoid quadratic or worse complexity,
> it'd preferred.
> 
> Thanks,
> Pedro Alves
> 

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[Patrick Palka]

On Sun, 17 Jul 2016, David Edelsohn wrote:

> On Sun, Jul 17, 2016 at 10:43 AM, Patrick Palka  wrote:
> > On Sun, Jul 17, 2016 at 9:15 AM, David Edelsohn  wrote:
> >>> Patrick Palka writes:
> >>
> >>> Hmm, this is only a problem due to command line length limits right?
> >>> Fortunately the change makes the longest command line only about 10%
> >>> longer than the previous longest command line.  With the change to
> >>> avoid building libbackend.a altogether, the longest command line is
> >>> now the cc1plus link command which is 4430 bytes long.  Without this
> >>> change, the longest command line is the "ar rc libbackend.a"
> >>> invocation at 4095 bytes (from what I can tell).  It's still pretty
> >>> far away from the 8k limit imposed by Windows.
> >>
> >> Windows is not the only non-Linux system on which GCC runs.
> >>
> >> You repeatedly are making bad assumptions and assertions without
> >> having studied much about GCC. You assume that GNU ar is the only
> >> archiver in use. You propose removing libbackend.a without having
> >> investigated when it was introduced and why.
> >>
> >> Your patches would be a lot more compelling if you invested the time
> >> to learn some context.
> >
> > The only patch I officially proposed is the one elides the invocation
> > of ranlib if the current ar is GNU ar (thanks to those who kindly
> > mentioned that other ar's are supported).  Do you have any comments
> > about this patch?
> 
> Testing for and utilizing a feature of GNU ar that speeds up builds
> seems like a nice improvement, but it obviously adds complexity and
> increases the risk that an independent change silently will introduce
> build problems on some systems.
> 
> > And you're right.  I am sorry for suggesting ways to improve rebuild
> > times without first uncovering the undocumented intricacies of the
> > build system.  Shame on me!
> 
> Yes, GCC is a large and intricate Free Software project.  And
> contributing patches requires some archaeology to understand the
> motivation and reason for the design choices as part of the software
> engineering process.  GCC supports and runs on a very large and
> diverse set of systems, which is one facet of its strength.
> 
> One goal of GCC is evolution toward a more modular design:
> 
> https://gcc.gnu.org/wiki/ModularGCC
> 
> Thanks, David
> 

I did some digging to figure out the origin of libbackend.a.  It was was
created to work around a command line length limit on a VAX system
(https://gcc.gnu.org/ml/gcc-bugs/2000-06/msg00438.html).  The 1600 byte
command line that links cc1plus was too large for this system, so
libbackend.a was introduced and used in place of $(OBJS) to reduce the
size of this command line.

It's probably not safe to remove this archive even today since it
currently stands for about 5000 bytes of object file names.  Simply
removing it and having the build system refer to $(OBJS) will likely
make us exceed 8k bytes in a command line somewhere.

I suppose rebuild times could instead be further reduced by splitting
libbackend.a into multiple logical archives.  It would also be a small
step towards making GCC more modular.

Re: [patch, fortran] PR66310 Problems with intrinsic repeat for large number of copies

[Jerry DeLisle]

On 07/12/2016 06:26 AM, Dominique d'Humières wrote:
>>> 2016-07-11  Jerry DeLisle  
>>>
>>> PR fortran/66310
>>> * simplify.c (gfc_simplify_repeat): Set max repeat to huge - 1 to allow
>>> one byte for null terminating the resulting string constant.
>>
>> OK, thanks
> Please hold on. I still see several problem with the patch applied. One is
> 
>program p
>   character :: z = 'z'
>   print *, repeat(z, huge(1)-2**9)
>end

Please test this revised patch. See my comments in the PR.

I think we should commit this one.

Jerry
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 77831ab..dcacae8 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -59,6 +59,8 @@ not after.
 
 #define MAX_SUBRECORD_LENGTH 2147483639   /* 2**31-9 */
 
+/* Practical limit on size of character constants.  */
+#define MAX_CHAR_LENGTH 268435455/* 2**28-1  */
 
 #define gfc_is_whitespace(c) ((c==' ') || (c=='\t'))
 
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 8096a92..4010753 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -5085,24 +5085,29 @@ gfc_simplify_repeat (gfc_expr *e, gfc_expr *n)
   /* Check that NCOPIES isn't too large.  */
   if (len)
 {
-  mpz_t max, mlen;
+  mpz_t max, mlen, limit;
   int i;
 
-  /* Compute the maximum value allowed for NCOPIES: huge(cl) / len.  */
+  /* Compute the maximum value allowed for NCOPIES:
+	huge(cl) - 1 / len.  */
   mpz_init (max);
   i = gfc_validate_kind (BT_INTEGER, gfc_charlen_int_kind, false);
 
+  /* Set the limit on size to avoid unsigned integer
+	 wrapping and resource limits.  */
+  mpz_init_set_si (limit, MAX_CHAR_LENGTH);
+
   if (have_length)
 	{
-	  mpz_tdiv_q (max, gfc_integer_kinds[i].huge,
-		  e->ts.u.cl->length->value.integer);
+	  mpz_tdiv_q (max, limit, e->ts.u.cl->length->value.integer);
 	}
   else
 	{
 	  mpz_init_set_si (mlen, len);
-	  mpz_tdiv_q (max, gfc_integer_kinds[i].huge, mlen);
+	  mpz_tdiv_q (max, limit, mlen);
 	  mpz_clear (mlen);
 	}
+  mpz_clear (limit);
 
   /* The check itself.  */
   if (mpz_cmp (ncopies, max) > 0)
diff --git a/gcc/testsuite/gfortran.dg/repeat_4.f90 b/gcc/testsuite/gfortran.dg/repeat_4.f90
index e5b5acc..a6ee75b 100644
--- a/gcc/testsuite/gfortran.dg/repeat_4.f90
+++ b/gcc/testsuite/gfortran.dg/repeat_4.f90
@@ -22,17 +22,17 @@ program test
 
   ! Check for too large NCOPIES argument and limit cases
   print *, repeat(t0, huge(0))
-  print *, repeat(t1, huge(0))
+  print *, repeat(t1, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
   print *, repeat(t2, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
   print *, repeat(s2, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
 
-  print *, repeat(t0, huge(0)/2)
-  print *, repeat(t1, huge(0)/2)
-  print *, repeat(t2, huge(0)/2)
+  print *, repeat(t0, huge(0)/8)
+  print *, repeat(t1, huge(0)/8)
+  print *, repeat(t2, huge(0)/16)
 
   print *, repeat(t0, huge(0)/2+1)
-  print *, repeat(t1, huge(0)/2+1)
-  print *, repeat(t2, huge(0)/2+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
-  print *, repeat(s2, huge(0)/2+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(t1, huge(0)/8+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(t2, huge(0)/16+1)! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(s2, huge(0)/16+1)! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
 
 end program test

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[David Edelsohn]

On Sun, Jul 17, 2016 at 10:43 AM, Patrick Palka  wrote:
> On Sun, Jul 17, 2016 at 9:15 AM, David Edelsohn  wrote:
>>> Patrick Palka writes:
>>
>>> Hmm, this is only a problem due to command line length limits right?
>>> Fortunately the change makes the longest command line only about 10%
>>> longer than the previous longest command line.  With the change to
>>> avoid building libbackend.a altogether, the longest command line is
>>> now the cc1plus link command which is 4430 bytes long.  Without this
>>> change, the longest command line is the "ar rc libbackend.a"
>>> invocation at 4095 bytes (from what I can tell).  It's still pretty
>>> far away from the 8k limit imposed by Windows.
>>
>> Windows is not the only non-Linux system on which GCC runs.
>>
>> You repeatedly are making bad assumptions and assertions without
>> having studied much about GCC. You assume that GNU ar is the only
>> archiver in use. You propose removing libbackend.a without having
>> investigated when it was introduced and why.
>>
>> Your patches would be a lot more compelling if you invested the time
>> to learn some context.
>
> The only patch I officially proposed is the one elides the invocation
> of ranlib if the current ar is GNU ar (thanks to those who kindly
> mentioned that other ar's are supported).  Do you have any comments
> about this patch?

Testing for and utilizing a feature of GNU ar that speeds up builds
seems like a nice improvement, but it obviously adds complexity and
increases the risk that an independent change silently will introduce
build problems on some systems.

> And you're right.  I am sorry for suggesting ways to improve rebuild
> times without first uncovering the undocumented intricacies of the
> build system.  Shame on me!

Yes, GCC is a large and intricate Free Software project.  And
contributing patches requires some archaeology to understand the
motivation and reason for the design choices as part of the software
engineering process.  GCC supports and runs on a very large and
diverse set of systems, which is one facet of its strength.

One goal of GCC is evolution toward a more modular design:

https://gcc.gnu.org/wiki/ModularGCC

Thanks, David

Re: [patch, Fortran] Fix some string temporaries

[Thomas Koenig]

Hi Mikael,


Do we actually want to backport this? Technically, it is a regression,
but people are not likely to notice much.


It is not an ICE, neither a code correctness issue as far as I can see,
so I would rather not backport.


Fine with me.



+case GFC_DEP_FORWARD:
+  return 0;



This is doubtfull, but not worse than before I guess.


0 in this case means that you need no array temporary.  This is fine.


+case GFC_DEP_BACKWARD:
+  return 1;
+
+case GFC_DEP_OVERLAP:
+  return 1;
+
+case GFC_DEP_NODEP:
+  return 0;
+
+case GFC_DEP_ERROR:
+  return 0;

Can we put a gcc_unreachable here instead?


Unfortunately not.  The original code (before I lifted out the
functionality) sometimes had GFC_DEP_ERROR at the end of the
function, which was then removed by

  return fin_dep == GFC_DEP_OVERLAP;

(In other words, if you put a gcc_unreachable there, you get
regressions.).

I can add a comment that this is intentional.


+

same here, make it unreachable please.

OK with this and the other unreachable change above.


So, OK with a comment why this appears?  Or should I simply
rename GFC_DEP_ERROR to GFC_DEP_NODEPFOUND to make this a bit
clearer?

Regards

Thomas

Re: Importing gnulib into the gcc tree

[Manuel López-Ibáñez]

On 16 July 2016 at 10:54, ayush goel  wrote:
> Hi,
> Thanks for the feedbacks.
>
> —> I’m already configuring gcc with multiple languages and multilib enabled
>
> —> The changes have been bootstrapped and regression tested (complete check, 
> make -k -j20 check).
>
> —> As mentioned, I have locally removed obstack.[ch] from libiberty and built 
> and tested the entire thing.
>
> PFA the patch

This sounds great to me, but I cannot approve it. I hope some of the
people who can will comment on it.

One thing that I miss is documenting gnulib in doc/sourcebuild.texi.
It would be good to document in particular how to add a new module.
GDB has: 
https://sourceware.org/gdb/wiki/DeveloperTips#Updating_GDB.27s_import_of_gnulib
but I think this info should be in sourcebuild.texi (or somewhere else
under doc/).

 I see several other mentions of libiberty in doc/, but some of them
may be just using libiberty as an example, thus not relevant.

Cheers,

Manuel.

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

[Manuel López-Ibáñez]

On 15/07/16 18:05, Aldy Hernandez wrote:

+case OPT_Walloca_larger_than_:
+  if (!value)
+   inform (loc, "-Walloca-larger-than=0 is meaningless");
+  break;
+
+case OPT_Wvla_larger_than_:
+  if (!value)
+   inform (loc, "-Wvla-larger-than=0 is meaningless");
+  break;
+

We don't give similar notes for any of the other Wx-larger-than= options. If 
-Wvla-larger-than=0 suppresses a previous -Wvla-larger-than=, then it doesn't 
seem meaningless, but a useful thing to have.


+  if (is_vla)
+gcc_assert (warn_vla_limit > 0);
+  if (!is_vla)
+gcc_assert (warn_alloca_limit > 0);

if-else ? Or perhaps:

gcc_assert (!is_vla || warn_vla_limit > 0);
gcc_assert (is_vla || warn_alloca_limit > 0);

--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -275,6 +275,15 @@ Wall
 C ObjC C++ ObjC++ Warning
 Enable most warning messages.

+Walloca
+C ObjC C++ ObjC++ Var(warn_alloca) Warning
+
+Walloca-larger-than=
+C ObjC C++ ObjC++ Var(warn_alloca_limit) Warning Joined RejectNegative UInteger
+-Walloca-larger-than= Warn on unbounded uses of
+alloca, and on bounded uses of alloca whose bound can be larger than
+ bytes.

No description for Walloca.

+ if (warn_alloca)
+   warning_at (loc, OPT_Walloca, "use of alloca");
+ continue;

Since alloca is a source code entity, it would be good to quote it using %< %> 
(this hints translators to not translate it).



+ const char *alloca_str
+   = is_vla ? "variable-length array" : "alloca";
+ char buff[WIDE_INT_MAX_PRECISION / 4 + 4];
+ switch (w)
+   {
+   case ALLOCA_OK:
+ break;
+   case ALLOCA_BOUND_MAYBE_LARGE:
+ gcc_assert (assumed_limit != 0);
+ if (warning_at (loc, wcode,
+ "argument to %s may be too large", alloca_str))
+   {
+ print_decu (assumed_limit, buff);
+ inform (loc, "limit is '%u' bytes, but argument may be '%s'",
+ is_vla ? warn_vla_limit : warn_alloca_limit, buff);
+   }
+ break;
+   case ALLOCA_BOUND_DEFINITELY_LARGE:
+ gcc_assert (assumed_limit != 0);
+ if (warning_at (loc, wcode,
+ "argument to %s is too large", alloca_str))
+   {
+ print_decu (assumed_limit, buff);
+ inform (loc, "limit is %u' bytes, but argument is '%s'",
+ is_vla ? warn_vla_limit : warn_alloca_limit, buff);
+   }
+ break;

https://gcc.gnu.org/codingconventions.html#Diagnostics :

All diagnostics should be full sentences without English fragments substituted 
in them, to facilitate translation.


Example:

if (warning_at (loc, wcode,
is_vla ? "argument to variable-length array may be too large"
   : "argument to % may be too large"))

+ print_decu (assumed_limit, buff);
+ inform (loc, "limit is '%u' bytes, but argument may be '%s'",
+ is_vla ? warn_vla_limit : warn_alloca_limit, buff);
+ }

https://gcc.gnu.org/wiki/DiagnosticsGuidelines#Quoting :
Other elements such as numbers that do no refer to numeric constants that 
appear in the source code should not be quoted.


+ warning_at (loc, wcode, "argument to %s may be too large due to "
+ "conversion from '%T' to '%T'",
+ alloca_str, invalid_casted_type, size_type_node);

From the same link:

Text should be quoted by either using the q modifier in a directive such as 
%qE, or by enclosing the quoted text in a pair of %< and %> directives, and 
never by using explicit quote characters. The directives handle the appropriate 
quote characters for each language and apply the correct color or highlighting.


I don't think the above are critical problems, they could be fixed by a follow 
up patch.


Cheers,
Manuel.

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[Manuel López-Ibáñez]

On 17/07/16 15:43, Patrick Palka wrote:

On Sun, Jul 17, 2016 at 9:15 AM, David Edelsohn  wrote:

You repeatedly are making bad assumptions and assertions without
having studied much about GCC. You assume that GNU ar is the only
archiver in use. You propose removing libbackend.a without having
investigated when it was introduced and why.

Your patches would be a lot more compelling if you invested the time
to learn some context.


The only patch I officially proposed is the one elides the invocation
of ranlib if the current ar is GNU ar (thanks to those who kindly
mentioned that other ar's are supported).  Do you have any comments
about this patch?

And you're right.  I am sorry for suggesting ways to improve rebuild
times without first uncovering the undocumented intricacies of the
build system.  Shame on me!


"Look at the bright side. Technical discussions sometimes appear harsh and dry 
to newcomers. Moreover, negative opinions are more vocal than positive ones. 
Thus, something that most people think is a good idea or they are indifferent 
may only get negative feedback from a few. Take this into account when judging 
how other people evaluate your ideas."

Point 3 https://gcc.gnu.org/wiki/Community

(I will extend the above with a few sentences about how tone is lost in email 
and one should always interpret criticism in the most friendly manner. Perhaps 
also extract anything useful from: 
http://producingoss.com/en/communications.html#writing-tone)


For what is worth, I commend your attempts at improving build times and I don't 
think you need to be sorry. On the other hand, it is to be expected that people 
may not welcome with open arms suggestions that may break GCC for them.


You are certainly right that there is a lot of undocumented/cargo-cult stuff in 
GCC. Unfortunately, what usually happens is that once the "newbie" becomes an 
"expert", the desire to document evaporates (I'm guilty also of this).


Why not get your "elide ranlib if possible" patch in first? Then, start a 
different discussion about whether libbackend.a is really necessary. I would 
actually prefer if GCC built some parts as libraries that could be re-used by 
other free-software projects rather than as a work-around:


https://gcc.gnu.org/wiki/rearch
https://gcc.gnu.org/wiki/ModularGCC#Middle-end_modules

Happy hacking,

Manuel.

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[Patrick Palka]

On Sun, Jul 17, 2016 at 9:15 AM, David Edelsohn  wrote:
>> Patrick Palka writes:
>
>> Hmm, this is only a problem due to command line length limits right?
>> Fortunately the change makes the longest command line only about 10%
>> longer than the previous longest command line.  With the change to
>> avoid building libbackend.a altogether, the longest command line is
>> now the cc1plus link command which is 4430 bytes long.  Without this
>> change, the longest command line is the "ar rc libbackend.a"
>> invocation at 4095 bytes (from what I can tell).  It's still pretty
>> far away from the 8k limit imposed by Windows.
>
> Windows is not the only non-Linux system on which GCC runs.
>
> You repeatedly are making bad assumptions and assertions without
> having studied much about GCC. You assume that GNU ar is the only
> archiver in use. You propose removing libbackend.a without having
> investigated when it was introduced and why.
>
> Your patches would be a lot more compelling if you invested the time
> to learn some context.

The only patch I officially proposed is the one elides the invocation
of ranlib if the current ar is GNU ar (thanks to those who kindly
mentioned that other ar's are supported).  Do you have any comments
about this patch?

And you're right.  I am sorry for suggesting ways to improve rebuild
times without first uncovering the undocumented intricacies of the
build system.  Shame on me!

>
> Thanks, David

Re: [RFC][IPA-VRP] Add support for IPA VRP in ipa-cp/ipa-prop

[Prathamesh Kulkarni]

On 15 July 2016 at 05:46, kugan  wrote:
> Hi,
>
>
>
> This patch extends ipa-cp/ipa-prop infrastructure to handle propagation of
> VR.
Hi Kugan,
Just a small nit - perhaps you should modify
ipa_print_node_jump_functions_for_edge () to pretty-print
value ranges associated with the jump function.

Thanks,
Prathamesh
>
>
>
> Thanks,
>
> Kugan
>
>
>
>
>
> gcc/testsuite/ChangeLog:
>
>
>
> 2016-07-14  Kugan Vivekanandarajah  
>
>
>
> * gcc.dg/ipa/vrp1.c: New test.
>
> * gcc.dg/ipa/vrp2.c: New test.
>
> * gcc.dg/ipa/vrp3.c: New test.
>
>
>
>
>
> gcc/ChangeLog:
>
>
>
> 2016-07-14  Kugan Vivekanandarajah  
>
>
>
> * common.opt: New option -fipa-vrp.
>
> * ipa-cp.c (ipa_get_vr_lat): New.
>
> (ipcp_vr_lattice::print): Likewise.
>
> (print_all_lattices): Call ipcp_vr_lattice::print.
>
> (ipcp_vr_lattice::meet_with): New.
>
> (ipcp_vr_lattice::meet_with_1): Likewise.
>
> (ipcp_vr_lattice::top_p): Likewise.
>
> (ipcp_vr_lattice::bottom_p): Likewsie.
>
> (ipcp_vr_lattice::set_to_bottom): Likewise.
>
> (set_all_contains_variable): Call VR set_to_bottom.
>
> (initialize_node_lattices): Init VR lattices.
>
> (propagate_vr_accross_jump_function): New.
>
> (propagate_constants_accross_call): Call
>
> propagate_vr_accross_jump_function.
>
> (ipcp_store_alignment_results): Rename to
>
> ipcp_store_alignment_and_vr_results and handke VR.
>
> * ipa-prop.c (ipa_set_jf_unknown):
>
> (ipa_compute_jump_functions_for_edge): Handle Value Range.
>
> (ipa_node_params_t::duplicate): Likewise.
>
> (ipa_write_jump_function): Likewise.
>
> (ipa_read_jump_function): Likewise.
>
> (write_ipcp_transformation_info): Likewise.
>
> (read_ipcp_transformation_info): Likewise.
>
> (ipcp_update_alignments): Rename to ipcp_update_vr_and_alignments
>
> and handle VR.
>
>
>
>
>

Re: [PATCH] Avoid invoking ranlib on libbackend.a

[David Edelsohn]

> Patrick Palka writes:

> Hmm, this is only a problem due to command line length limits right?
> Fortunately the change makes the longest command line only about 10%
> longer than the previous longest command line.  With the change to
> avoid building libbackend.a altogether, the longest command line is
> now the cc1plus link command which is 4430 bytes long.  Without this
> change, the longest command line is the "ar rc libbackend.a"
> invocation at 4095 bytes (from what I can tell).  It's still pretty
> far away from the 8k limit imposed by Windows.

Windows is not the only non-Linux system on which GCC runs.

You repeatedly are making bad assumptions and assertions without
having studied much about GCC. You assume that GNU ar is the only
archiver in use. You propose removing libbackend.a without having
investigated when it was introduced and why.

Your patches would be a lot more compelling if you invested the time
to learn some context.

Thanks, David

Re: [patch, Fortran] Fix some string temporaries

[Mikael Morin]

Hello,

Le 16/07/2016 à 15:38, Thomas Koenig a écrit :

Hello world,

this fixes PR 71902. The recent fix for PR 71783 introduced a
performance and code size regression a string temporary was created for
the test case when it was not actually needed.

I also took the opportunity of renaming the misnomed temporary
variable.

Regression-tested.

OK for trunk?

Do we actually want to backport this? Technically, it is a regression,
but people are not likely to notice much.

It is not an ICE, neither a code correctness issue as far as I can see, 
so I would rather not backport.


comments below.


p3.diff

Index: dependency.c
===
--- dependency.c(Revision 238223)
+++ dependency.c(Arbeitskopie)
@@ -54,6 +54,8 @@ enum gfc_dependency
 static gfc_dependency check_section_vs_section (gfc_array_ref *,
gfc_array_ref *, int);

+static gfc_dependency dep_ref (gfc_ref *, gfc_ref *, gfc_reverse *);
+
 /* Returns 1 if the expr is an integer constant value 1, 0 if it is not or
def if the value could not be determined.  */

@@ -1316,14 +1318,34 @@ gfc_check_dependency (gfc_expr *expr1, gfc_expr *e
  return 0;
}

-  if (identical)
-   return 1;
-
   /* Identical and disjoint ranges return 0,
 overlapping ranges return 1.  */
   if (expr1->ref && expr2->ref)
-   return gfc_dep_resolver (expr1->ref, expr2->ref, NULL);
+   {
+ gfc_dependency dep;
+ dep = dep_ref (expr1->ref, expr2->ref, NULL);
+ switch (dep)
+   {
+   case GFC_DEP_EQUAL:
+ return identical;

+   case GFC_DEP_FORWARD:
+ return 0;
+

This is doubtfull, but not worse than before I guess.


+   case GFC_DEP_BACKWARD:
+ return 1;
+
+   case GFC_DEP_OVERLAP:
+ return 1;
+
+   case GFC_DEP_NODEP:
+ return 0;
+
+   case GFC_DEP_ERROR:
+ return 0;

Can we put a gcc_unreachable here instead?


+   }
+   }
+
   return 1;

 case EXPR_FUNCTION:
@@ -2052,11 +2074,44 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref
2 : array references are overlapping but reversal of one or
more dimensions will clear the dependency.
1 : array references are overlapping.
-   0 : array references are identical or not overlapping.  */
+   0 : array references are identical or can be handled in a forward loop. 
 */

 int
 gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse)
 {
+  enum gfc_dependency dep;
+  dep = dep_ref (lref, rref, reverse);
+  switch (dep)
+{
+case GFC_DEP_EQUAL:
+  return 0;
+
+case GFC_DEP_FORWARD:
+  return 0;
+
+case GFC_DEP_BACKWARD:
+  return 2;
+
+case GFC_DEP_OVERLAP:
+  return 1;
+
+case GFC_DEP_NODEP:
+  return 0;
+
+case GFC_DEP_ERROR:
+  return 0;
+

same here, make it unreachable please.

OK with this and the other unreachable change above.

Thanks,
Mikael


+default:
+  gcc_unreachable();
+}
+}
+
+/* Compare two references, returning an enum gfc_dependency depending on the
+   "worst" type of dependency found.  */
+
+static gfc_dependency
+dep_ref (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse)
+{
   int n;
   int m;
   gfc_dependency fin_dep;