Re: [PATCH] i386: Always set cfun->machine->max_used_stack_alignment

[Uros Bizjak]

On Sun, Aug 5, 2018 at 12:48 AM, H.J. Lu  wrote:
> On Sat, Aug 04, 2018 at 11:48:15PM +0200, Uros Bizjak wrote:
>> On Sat, Aug 4, 2018 at 9:49 PM, H.J. Lu  wrote:
>> > On Sat, Aug 4, 2018 at 12:09 PM, Uros Bizjak  wrote:
>> >> On Sat, Aug 4, 2018 at 3:59 PM, H.J. Lu  wrote:
>> >>> On Sat, Aug 4, 2018 at 3:42 AM, Uros Bizjak  wrote:
>>  On Fri, Aug 3, 2018 at 12:55 AM, H.J. Lu  wrote:
>> > We should always set cfun->machine->max_used_stack_alignment if the
>> > maximum stack slot alignment may be greater than 64 bits.
>> >
>> > Tested on i686 and x86-64.  OK for master and backport for GCC 8?
>> 
>>  Can you explain why 64 bits, and what this value represents? Is this
>>  value the same for 64bit and 32bit targets?
>> 
>>  Should crtl->max_used_stack_slot_alignment be compared to
>>  STACK_BOUNDARY or even MIN_STACK_BOUNDARY instead?
>> >>>
>> >>> In this case, we don't need to realign the incoming stack since both
>> >>> crtl->max_used_stack_slot_alignment and crtl->preferred_stack_boundary
>> >>> are 128 bits.  We don't compute the largest alignment of stack slots to
>> >>> keep stack frame properly aligned for it.  Normally it is OK.   But if
>> >>> the largest alignment of stack slots > 64 bits and we don't keep stack
>> >>> frame proper aligned, we will get segfault if aligned vector load/store
>> >>> are used on these unaligned stack slots. My patch computes the
>> >>> largest alignment of stack slots, which are actually used,  if the
>> >>> largest alignment of stack slots allocated is > 64 bits, which is
>> >>> the smallest alignment for misaligned load/store.
>> >>
>> >> Does > 64 bits also hold for 32 bit targets, and -mabi=ms? I think
>> >> that we need to compare to STACK_BOUNDARY instead:
>> >
>> > 64 bits requirement is independent of any psABIs nor 32-bit vs 64-bit.
>> > cfun->machine->max_used_stack_alignment is used to decide how
>> > stack frame should be aligned.  It is always safe to compute it.  I used
>> >
>> > else if (crtl->max_used_stack_slot_alignment > 64)
>> >
>> > to compute cfun->machine->max_used_stack_alignment only if
>> > we have to.
>> >
>> >> --cut here--
>> >> Index: config/i386/i386.c
>> >> ===
>> >> --- config/i386/i386.c  (revision 263307)
>> >> +++ config/i386/i386.c  (working copy)
>> >> @@ -13281,8 +13281,7 @@
>> >>   recompute_frame_layout_p = true;
>> >> }
>> >>  }
>> >> -  else if (crtl->max_used_stack_slot_alignment
>> >> -  > crtl->preferred_stack_boundary)
>> >> +  else if (crtl->max_used_stack_slot_alignment > STACK_BOUNDARY)
>> >>  {
>> >
>> > This isn't correct..  cygming.h has
>> >
>> > #define STACK_BOUNDARY (TARGET_64BIT && ix86_abi == MS_ABI ? 128 :
>> > BITS_PER_WORD)
>> >
>> > darwin.h has
>> >
>> > #undef STACK_BOUNDARY
>> > #define STACK_BOUNDARY \
>> >  ((profile_flag || (TARGET_64BIT && ix86_abi == MS_ABI)) \
>> >   ? 128 : BITS_PER_WORD)
>> >
>> > i386.h has
>> >
>> > /* Boundary (in *bits*) on which stack pointer should be aligned.  */
>> > #define STACK_BOUNDARY \
>> >  (TARGET_64BIT && ix86_abi == MS_ABI ? 128 : BITS_PER_WORD)
>> >
>> > If STACK_BOUNDARY is 128 and max_used_stack_slot_alignment is 128,
>> > we will get segment when 128bit aligned load/store is generated on 
>> > misaligned
>> > stack slot.
>> >
>> >>/* We don't need to realign stack.  But we still need to keep
>> >>  stack frame properly aligned to satisfy the largest alignment
>> >> --cut here--
>> >>
>> >> (The testcase works OK with -mabi=ms, which somehow suggests that we
>> >> don't need realignment in this case).
>> >
>> > We may not hit 128bit aligned load/store on misaligned stack slot in this
>> > case.  It doesn't mean that won't happen.
>> >
>> > else if (crtl->max_used_stack_slot_alignment > 64)
>> >
>> > is the correct thing to do here.
>>
>> OK, but please add a comment, so in the future we will still know the
>> purpose of the magic number.
>>
>
> Like this?
>
> H.J.
> ---
> cfun->machine->max_used_stack_alignment is used to decide how stack frame
> should be aligned.  This is independent of any psABIs nor 32-bit vs 64-bit.
> It is always safe to compute max_used_stack_alignment.  We compute it only
> if 128-bit aligned load/store may be generated on misaligned stack slot
> which will lead to segfault.
>
> gcc/
>
> PR target/86386
> * config/i386/i386.c (ix86_finalize_stack_frame_flags): Set
> cfun->machine->max_used_stack_alignment if needed.
>
> gcc/testsuite/
>
> PR target/86386
> * gcc.target/i386/pr86386.c: New file.

OK, but please write the condition as ">= 128". The number 64 confused
me; I was thinking that it has something to do with minimum alignment
on 64bit target, while 128 clearly shows that alignment is related to
SSE moves. With ">= 128", I think that code is clear enough that a
long comment is not needed.

Thanks, and sorry for the confusion,
Uros.

>

Re: [PATCH] Check the STRING_CSTs in varasm.c

[Bernd Edlinger]

Hi,

I would like to do a minor tweak to the patch.
While staring at the other patch I realized that I should
better pass size and not thissize to the check
function, instead of making use of how thissize is
computed using MIN above.  This means that this condition

+  if ((unsigned)len != size && (unsigned)len != size + elts)
+return false;

should better and more readable be:

+  if (size < (unsigned)len && size != len - elts)
+return false;

Furthermore I wanted to have the check function static,
which I missed to do previously.

For completeness, these are again the precondition patches
for this patch:

[PATCH] Handle overlength strings in the C FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00142.html

[PATCH] Handle overlength strings in C++ FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00045.html
Approved: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00105.html

[PATCH] Make GO string literals properly NUL terminated
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01931.html

[PATCH] [Ada] Make middle-end string literals NUL terminated
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01929.html

[PATCH] Create internally nul terminated string literals in fortan FE
https://gcc.gnu.org/ml/fortran/2018-08/msg0.html


Bootstrapped, as usual.
Is it OK for trunk?


Thanks
Bernd.
2018-08-01  Bernd Edlinger  

	* doc/generic.texi (STRING_CST): Update.
	* varasm.c (check_string_literal): New checking function.
	(output_constant): Use it.

diff -pur gcc/varasm.c gcc/varasm.c
--- gcc/varasm.c	2018-07-17 11:19:27.0 +0200
+++ gcc/varasm.c	2018-07-31 10:16:12.058827505 +0200
@@ -4774,6 +4774,29 @@ initializer_constant_valid_for_bitfield_
   return false;
 }
 
+/* Check if a STRING_CST fits into the field.
+   Tolerate only the case when the NUL termination
+   does not fit into the field.   */
+
+static bool
+check_string_literal (tree string, unsigned HOST_WIDE_INT size)
+{
+  tree eltype = TREE_TYPE (TREE_TYPE (string));
+  unsigned HOST_WIDE_INT elts = tree_to_uhwi (TYPE_SIZE_UNIT (eltype));
+  const char *p = TREE_STRING_POINTER (string);
+  int len = TREE_STRING_LENGTH (string);
+
+  if (elts != 1 && elts != 2 && elts != 4)
+return false;
+  if (len <= 0 || len % elts != 0)
+return false;
+  if (size < (unsigned)len && size != len - elts)
+return false;
+  if (memcmp (p + len - elts, "\0\0\0\0", elts) != 0)
+return false;
+  return true;
+}
+
 /* output_constructor outer state of relevance in recursive calls, typically
for nested aggregate bitfields.  */
 
@@ -4942,6 +4965,7 @@ output_constant (tree exp, unsigned HOST
 	case STRING_CST:
 	  thissize
 	= MIN ((unsigned HOST_WIDE_INT)TREE_STRING_LENGTH (exp), size);
+	  gcc_checking_assert (check_string_literal (exp, size));
 	  assemble_string (TREE_STRING_POINTER (exp), thissize);
 	  break;
 	case VECTOR_CST:
Index: gcc/doc/generic.texi
===
--- gcc/doc/generic.texi	(revision 263072)
+++ gcc/doc/generic.texi	(working copy)
@@ -1162,9 +1162,13 @@ These nodes represent string-constants.  The @code
 returns the length of the string, as an @code{int}.  The
 @code{TREE_STRING_POINTER} is a @code{char*} containing the string
 itself.  The string may not be @code{NUL}-terminated, and it may contain
-embedded @code{NUL} characters.  Therefore, the
-@code{TREE_STRING_LENGTH} includes the trailing @code{NUL} if it is
-present.
+embedded @code{NUL} characters.  However, the
+@code{TREE_STRING_LENGTH} always includes a trailing @code{NUL} that
+is not part of the language string literal but appended by the front end.
+If the string shall not be @code{NUL}-terminated the @code{TREE_TYPE}
+is one character shorter than @code{TREE_STRING_LENGTH}.
+Excess caracters other than one trailing @code{NUL} character are not
+permitted.
 
 For wide string constants, the @code{TREE_STRING_LENGTH} is the number
 of bytes in the string, and the @code{TREE_STRING_POINTER}
@@ -1173,6 +1177,11 @@ target system (that is, as integers in the target
 non-wide string constants are distinguished only by the @code{TREE_TYPE}
 of the @code{STRING_CST}.
 
+String constants may also be used for other purpose like assember
+constraints or attributes.  These have no @code{TREE_TYPE} and
+need no explicit @code{NUL}-termination.  Note there is always
+another @code{NUL}-byte beyond @code{TREE_STRING_LENGTH}.
+
 FIXME: The formats of string constants are not well-defined when the
 target system bytes are not the same width as host system bytes.
 

Re: [PATCH] i386: Always set cfun->machine->max_used_stack_alignment

[H.J. Lu]

On Sun, Aug 5, 2018 at 12:15 AM, Uros Bizjak  wrote:
>>> OK, but please add a comment, so in the future we will still know the
>>> purpose of the magic number.
>>>
>>
>> Like this?
>>
>> H.J.
>> ---
>> cfun->machine->max_used_stack_alignment is used to decide how stack frame
>> should be aligned.  This is independent of any psABIs nor 32-bit vs 64-bit.
>> It is always safe to compute max_used_stack_alignment.  We compute it only
>> if 128-bit aligned load/store may be generated on misaligned stack slot
>> which will lead to segfault.
>>
>> gcc/
>>
>> PR target/86386
>> * config/i386/i386.c (ix86_finalize_stack_frame_flags): Set
>> cfun->machine->max_used_stack_alignment if needed.
>>
>> gcc/testsuite/
>>
>> PR target/86386
>> * gcc.target/i386/pr86386.c: New file.
>
> OK, but please write the condition as ">= 128". The number 64 confused
> me; I was thinking that it has something to do with minimum alignment
> on 64bit target, while 128 clearly shows that alignment is related to
> SSE moves. With ">= 128", I think that code is clear enough that a
> long comment is not needed.
>
> Thanks, and sorry for the confusion,
> Uros.
>

This is what I checked in.  I kept the comment change.  I will backport it
to GCC 8 after a few days.

Thanks.

-- 
H.J.
From 51af74d8d141aeceaacf2c9eb8e0b126e8f6c13b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Thu, 2 Aug 2018 10:43:03 -0700
Subject: [PATCH] i386: Set cfun->machine->max_used_stack_alignment if needed

cfun->machine->max_used_stack_alignment is used to decide how stack frame
should be aligned.  This is independent of any psABIs nor 32-bit vs 64-bit.
It is always safe to compute max_used_stack_alignment.  We compute it only
if 128-bit aligned load/store may be generated on misaligned stack slot
which will lead to segfault.

gcc/

	PR target/86386
	* config/i386/i386.c (ix86_finalize_stack_frame_flags): Set
	cfun->machine->max_used_stack_alignment if needed.

gcc/testsuite/

	PR target/86386
	* gcc.target/i386/pr86386.c: New file.
---
 gcc/config/i386/i386.c  | 14 +++--
 gcc/testsuite/gcc.target/i386/pr86386.c | 26 +
 2 files changed, 34 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr86386.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ee409cfe7e4..7554fd1f659 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13281,12 +13281,14 @@ ix86_finalize_stack_frame_flags (void)
 	  recompute_frame_layout_p = true;
 	}
 }
-  else if (crtl->max_used_stack_slot_alignment
-	   > crtl->preferred_stack_boundary)
-{
-  /* We don't need to realign stack.  But we still need to keep
-	 stack frame properly aligned to satisfy the largest alignment
-	 of stack slots.  */
+  else if (crtl->max_used_stack_slot_alignment >= 128)
+{
+  /* We don't need to realign stack.  max_used_stack_alignment is
+	 used to decide how stack frame should be aligned.  This is
+	 independent of any psABIs nor 32-bit vs 64-bit.  It is always
+	 safe to compute max_used_stack_alignment.  We compute it only
+	 if 128-bit aligned load/store may be generated on misaligned
+	 stack slot which will lead to segfault.   */
   if (ix86_find_max_used_stack_alignment (stack_alignment, true))
 	cfun->machine->max_used_stack_alignment
 	  = stack_alignment / BITS_PER_UNIT;
diff --git a/gcc/testsuite/gcc.target/i386/pr86386.c b/gcc/testsuite/gcc.target/i386/pr86386.c
new file mode 100644
index 000..a67cf45444e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr86386.c
@@ -0,0 +1,26 @@
+/* PR target/86386 */
+/* { dg-do run { target { avx_runtime && int128 } } } */
+/* { dg-options "-Os -fno-tree-dce -mstringop-strategy=vector_loop -mavx" } */
+
+unsigned c, d, e, f;
+
+unsigned __attribute__((noipa))
+foo (unsigned char g, unsigned short h, unsigned i, unsigned long long j,
+ unsigned char k, unsigned short l, unsigned m, unsigned __int128 n)
+{
+  __builtin_memset (&e, 0, 3);
+  n <<= m;
+  __builtin_memcpy (&m, 2 + (char *) &n, 1);
+  m >>= 0;
+  d ^= __builtin_mul_overflow (l, n, &m);
+  return m;
+}
+
+int
+main ()
+{
+  unsigned __int128 x = foo (0, 0, 0, 0, 0, 4, 1, 3);
+  if (x != 24)
+__builtin_abort ();
+  return 0;
+}
-- 
2.17.1

[Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

[Janus Weil]

Hi all,

the attached patch fixes PR 86116 by splitting up the function
'compare_type' into two variants: One that is used for checking
generic interfaces and operators (keeping the old name), and one that
is used for checking dummy functions and procedure pointer assignments
('compare_type_characteristics'). The latter calls the former, but
includes an additional check that must not be done when checking
generics.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-08-05  Janus Weil  

PR fortran/86116
* interface.c (compare_type): Remove a CLASS/TYPE check.
(compare_type_characteristics): New function that behaves like the old
'compare_type'.
(gfc_check_dummy_characteristics, gfc_check_result_characteristics):
Call 'compare_type_characteristics' instead of 'compare_type'.

2018-08-05  Janus Weil  

PR fortran/86116
* gfortran.dg/generic_34.f90: New test case.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 263308)
+++ gcc/fortran/interface.c	(working copy)
@@ -735,6 +735,13 @@ compare_type (gfc_symbol *s1, gfc_symbol *s2)
   if (s2->attr.ext_attr & (1 << EXT_ATTR_NO_ARG_CHECK))
 return true;
 
+  return gfc_compare_types (&s1->ts, &s2->ts) || s2->ts.type == BT_ASSUMED;
+}
+
+
+static bool
+compare_type_characteristics (gfc_symbol *s1, gfc_symbol *s2)
+{
   /* TYPE and CLASS of the same declared type are type compatible,
  but have different characteristics.  */
   if ((s1->ts.type == BT_CLASS && s2->ts.type == BT_DERIVED)
@@ -741,7 +748,7 @@ compare_type (gfc_symbol *s1, gfc_symbol *s2)
   || (s1->ts.type == BT_DERIVED && s2->ts.type == BT_CLASS))
 return false;
 
-  return gfc_compare_types (&s1->ts, &s2->ts) || s2->ts.type == BT_ASSUMED;
+  return compare_type (s1, s2);
 }
 
 
@@ -1309,7 +1316,8 @@ gfc_check_dummy_characteristics (gfc_symbol *s1, g
   /* Check type and rank.  */
   if (type_must_agree)
 {
-  if (!compare_type (s1, s2) || !compare_type (s2, s1))
+  if (!compare_type_characteristics (s1, s2)
+	  || !compare_type_characteristics (s2, s1))
 	{
 	  snprintf (errmsg, err_len, "Type mismatch in argument '%s' (%s/%s)",
 		s1->name, gfc_typename (&s1->ts), gfc_typename (&s2->ts));
@@ -1528,7 +1536,7 @@ gfc_check_result_characteristics (gfc_symbol *s1,
 return true;
 
   /* Check type and rank.  */
-  if (!compare_type (r1, r2))
+  if (!compare_type_characteristics (r1, r2))
 {
   snprintf (errmsg, err_len, "Type mismatch in function result (%s/%s)",
 		gfc_typename (&r1->ts), gfc_typename (&r2->ts));
! { dg-do compile }
!
! PR 86116: [6/7/8/9 Regression] Ambiguous generic interface not recognised
!
! Contributed by martin 

module mod

   type :: t
   end type t

   interface sub
  module procedure s1
  module procedure s2
   end interface

contains

   subroutine s1(x)  ! { dg-error "Ambiguous interfaces in generic interface" }
  type(t) :: x
   end subroutine

   subroutine s2(x)  ! { dg-error "Ambiguous interfaces in generic interface" }
  class(*), allocatable :: x
   end subroutine

end

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill]

On Tue, Jul 24, 2018 at 6:49 AM, Marek Polacek  wrote:
> On Tue, Jul 03, 2018 at 04:27:33PM -0400, Jason Merrill wrote:
>> On Tue, Jul 3, 2018 at 3:41 PM, Jason Merrill  wrote:
>> > On Tue, Jul 3, 2018 at 2:58 PM, Marek Polacek  wrote:
>> >> On Tue, Jul 03, 2018 at 12:40:51PM -0400, Jason Merrill wrote:
>> >>> On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek  
>> >>> wrote:
>> >>> > On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>> >>> >> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek  
>> >>> >> wrote:
>> >>> >> > This PR complains about us accepting invalid code like
>> >>> >> >
>> >>> >> >   template struct A {};
>> >>> >> >   A<-1> a;
>> >>> >> >
>> >>> >> > Where we should detect the narrowing: [temp.arg.nontype] says
>> >>> >> > "A template-argument for a non-type template-parameter shall be a 
>> >>> >> > converted
>> >>> >> > constant expression ([expr.const]) of the type of the 
>> >>> >> > template-parameter."
>> >>> >> > and a converted constant expression can contain only
>> >>> >> > - integral conversions other than narrowing conversions,
>> >>> >> > - [...]."
>> >>> >> > It spurred e.g.
>> >>> >> > 
>> >>> >> > and has >=3 dups so it has some visibility.
>> >>> >> >
>> >>> >> > I think build_converted_constant_expr needs to set check_narrowing.
>> >>> >> > check_narrowing also always mentions that it's in { } but that is 
>> >>> >> > no longer
>> >>> >> > true; in the future it will also apply to <=>.  We'd probably have 
>> >>> >> > to add a new
>> >>> >> > flag to struct conversion if wanted to distinguish between these.
>> >>> >> >
>> >>> >> > This does not yet fix detecting narrowing in function templates 
>> >>> >> > (78244).
>> >>> >> >
>> >>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >>> >> >
>> >>> >> > 2018-06-27  Marek Polacek  
>> >>> >> >
>> >>> >> > PR c++/57891
>> >>> >> > * call.c (build_converted_constant_expr): Set 
>> >>> >> > check_narrowing.
>> >>> >> > * decl.c (compute_array_index_type): Add warning sentinel.  
>> >>> >> > Use
>> >>> >> > input_location.
>> >>> >> > * pt.c (convert_nontype_argument): Return NULL_TREE if any 
>> >>> >> > errors
>> >>> >> > were reported.
>> >>> >> > * typeck2.c (check_narrowing): Don't mention { } in 
>> >>> >> > diagnostic.
>> >>> >> >
>> >>> >> > * g++.dg/cpp0x/Wnarrowing6.C: New test.
>> >>> >> > * g++.dg/cpp0x/Wnarrowing7.C: New test.
>> >>> >> > * g++.dg/cpp0x/Wnarrowing8.C: New test.
>> >>> >> > * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>> >>> >> > * g++.dg/init/new43.C: Adjust dg-error.
>> >>> >> > * g++.dg/other/fold1.C: Likewise.
>> >>> >> > * g++.dg/parse/array-size2.C: Likewise.
>> >>> >> > * g++.dg/other/vrp1.C: Add dg-error.
>> >>> >> > * g++.dg/template/char1.C: Likewise.
>> >>> >> > * g++.dg/ext/builtin12.C: Likewise.
>> >>> >> > * g++.dg/template/dependent-name3.C: Adjust dg-error.
>> >>> >> >
>> >>> >> > diff --git gcc/cp/call.c gcc/cp/call.c
>> >>> >> > index 209c1fd2f0e..956c7b149dc 100644
>> >>> >> > --- gcc/cp/call.c
>> >>> >> > +++ gcc/cp/call.c
>> >>> >> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, 
>> >>> >> > tree expr, tsubst_flags_t complain)
>> >>> >> >  }
>> >>> >> >
>> >>> >> >if (conv)
>> >>> >> > -expr = convert_like (conv, expr, complain);
>> >>> >> > +{
>> >>> >> > +  conv->check_narrowing = !processing_template_decl;
>> >>> >>
>> >>> >> Why !processing_template_decl?  This needs a comment.
>> >>> >
>> >>> > Otherwise we'd warn for e.g.
>> >>> >
>> >>> > template struct S { char a[N]; };
>> >>> > S<1> s;
>> >>> >
>> >>> > where compute_array_index_type will try to convert the size of the 
>> >>> > array (which
>> >>> > is a template_parm_index of type int when parsing the template) to 
>> >>> > size_type.
>> >>> > So I guess I can say that we need to wait for instantiation?
>> >>>
>> >>> We certainly shouldn't give a narrowing diagnostic about a
>> >>> value-dependent expression.  It probably makes sense to check that at
>> >>> the top of check_narrowing, with all the other early exit conditions.
>> >>> But if we do know the constant value in the template, it's good to
>> >>> complain then rather than wait for instantiation.
>> >>
>> >> Makes sense; how about this then?  (Regtest/bootstrap running.)
>> >>
>> >> 2018-07-03  Marek Polacek  
>> >>
>> >> PR c++/57891
>> >> * call.c (build_converted_constant_expr): Set check_narrowing.
>> >> * decl.c (compute_array_index_type): Add warning sentinel.  Use
>> >> input_location.
>> >> * pt.c (convert_nontype_argument): Return NULL_TREE if any errors
>> >> were reported.
>> >> * typeck2.c (check_narrowing): Don't warn for 
>> >> instantiation-dependent
>> >> expressions o

Re: [C++ PATCH] Fix constexpr ICE with poor man's offsetof (PR c++/86738)

[Jason Merrill]

OK

On Sat, Aug 4, 2018 at 2:18 AM, Jakub Jelinek  wrote:
> Hi!
>
> Some time ago, I've moved the poor man's offsetof recognizing hack to
> cp_fold.  On the following testcase that means we don't fold it early during
> parsing.  Now, if we try to evaluate those inside of constexpr contexts
> with !ctx->quiet, it is diagnosed as invalid (but *non_constant_p is not
> set).  Worse, with ctx->quiet, we pretend it is a constant expression but
> don't actually fold it, and then we have e.g. in array index evaluation
>   VERIFY_CONSTANT (index);
> ...
>   VERIFY_CONSTANT (nelts);
>   if ((lval
>? !tree_int_cst_le (index, nelts)
>: !tree_int_cst_lt (index, nelts))
>   || tree_int_cst_sgn (index) < 0)
> {
>   diag_array_subscript (ctx, ary, index);
>   *non_constant_p = true;
>   return t;
> }
> where VERIFY_CONSTANT is happy about it, even when index is not an
> INTEGER_CST, but large complex TREE_CONSTANT expression.  The above though
> assumes INTEGER_CST.  Perhaps we should check for INTEGER_CST somewhere (and
> in other similar code too), but it isn't clear to me what exactly we should
> do if those trees aren't INTEGER_CSTs, especially with !ctx->quiet.
>
> This patch changes a different thing, the usual case (including other spots
> for NULL pointer dereferences or arith) in constexpr.c is
>   if (some condition)
> {
>   if (!ctx->quiet)
> error* (...);
>   *non_constant_p = true;
>   return t;
> }
> but the following two spots were different and that caused the array
> handling to see those complex unsimplified constant expressions.
> With this, it is not treated as constant for maybe_constant_value etc.
> purposes, though following cp_fold can still fold it into a constant.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (including
> check-c++-all testing on both), ok for trunk and 8.3 after a while?
>
> 2018-08-03  Jakub Jelinek  
>
> PR c++/86738
> * constexpr.c (cxx_eval_binary_expression): For arithmetics involving
> NULL pointer set *non_constant_p to true.
> (cxx_eval_component_reference): For dereferencing of a NULL pointer,
> set *non_constant_p to true and return t.
>
> * g++.dg/opt/pr86738.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2018-07-31 23:57:24.193432388 +0200
> +++ gcc/cp/constexpr.c  2018-08-03 14:54:13.302817282 +0200
> @@ -2082,6 +2082,7 @@ cxx_eval_binary_expression (const conste
>  {
>if (!ctx->quiet)
> error ("arithmetic involving a null pointer in %qE", lhs);
> +  *non_constant_p = true;
>return t;
>  }
>else if (code == POINTER_PLUS_EXPR)
> @@ -2522,9 +2523,13 @@ cxx_eval_component_reference (const cons
>  lval,
>  non_constant_p, overflow_p);
>if (INDIRECT_REF_P (whole)
> -  && integer_zerop (TREE_OPERAND (whole, 0))
> -  && !ctx->quiet)
> -error ("dereferencing a null pointer in %qE", orig_whole);
> +  && integer_zerop (TREE_OPERAND (whole, 0)))
> +{
> +  if (!ctx->quiet)
> +   error ("dereferencing a null pointer in %qE", orig_whole);
> +  *non_constant_p = true;
> +  return t;
> +}
>
>if (TREE_CODE (whole) == PTRMEM_CST)
>  whole = cplus_expand_constant (whole);
> --- gcc/testsuite/g++.dg/opt/pr86738.C.jj   2018-08-03 15:03:51.477358712 
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr86738.C  2018-08-03 15:02:51.940201694 +0200
> @@ -0,0 +1,12 @@
> +// PR c++/86738
> +// { dg-do compile }
> +
> +struct S { int s; };
> +unsigned char a[20];
> +unsigned char *p = &a[(__UINTPTR_TYPE__) &((S *) 0)->s];
> +
> +void
> +foo ()
> +{
> +  __builtin_memcpy (&a[15], &a[(__UINTPTR_TYPE__) &((S *) 0)->s], 2);
> +}
>
> Jakub

Re: [C++ PATCH] Fix tsubst of structured bindings (PR c++/86836)

[Jason Merrill]

OK.

On Sat, Aug 4, 2018 at 1:54 AM, Jakub Jelinek  wrote:
> Hi!
>
> As mentioned in the PR, for valid structured bindings this patch should be
> unnecessary, because the identifiers from the structured binding shouldn't
> be used in the initializer of the structured binding, but for invalid source
> it can matter.  When tsubst_init is called before tsubst_decomp_names,
> the local specializations for the decomp id VAR_DECLs aren't created and
> so the tsubst of those VAR_DECLs gives the PARM_DECL in this testcase, or
> something else unrelated to the decomp.
>
> Fixed by doing tsubst_decomp_names first, then tsubst_init the initializer
> and then the rest.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and 8.3
> after a while?
>
> 2018-08-03  Jakub Jelinek  
>
> PR c++/86836
> * pt.c (tsubst_expr): For structured bindings, call 
> tsubst_decomp_names
> before tsubst_init, not after it.
>
> * g++.dg/cpp1z/decomp46.C: New test.
>
> --- gcc/cp/pt.c.jj  2018-08-03 11:36:25.550755429 +0200
> +++ gcc/cp/pt.c 2018-08-03 11:48:51.144567965 +0200
> @@ -16740,7 +16740,17 @@ tsubst_expr (tree t, tree args, tsubst_f
> else
>   {
> int const_init = false;
> +   unsigned int cnt = 0;
> +   tree first = NULL_TREE, ndecl = error_mark_node;
> maybe_push_decl (decl);
> +
> +   if (VAR_P (decl)
> +   && DECL_DECOMPOSITION_P (decl)
> +   && TREE_TYPE (pattern_decl) != error_mark_node)
> + ndecl = tsubst_decomp_names (decl, pattern_decl, args,
> +  complain, in_decl, &first,
> +  &cnt);
> +
> if (VAR_P (decl)
> && DECL_PRETTY_FUNCTION_P (decl))
>   {
> @@ -16756,23 +16766,14 @@ tsubst_expr (tree t, tree args, tsubst_f
> if (VAR_P (decl))
>   const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
> (pattern_decl));
> -   if (VAR_P (decl)
> -   && DECL_DECOMPOSITION_P (decl)
> -   && TREE_TYPE (pattern_decl) != error_mark_node)
> - {
> -   unsigned int cnt;
> -   tree first;
> -   tree ndecl
> - = tsubst_decomp_names (decl, pattern_decl, args,
> -complain, in_decl, &first, 
> &cnt);
> -   if (ndecl != error_mark_node)
> - cp_maybe_mangle_decomp (ndecl, first, cnt);
> -   cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
> -   if (ndecl != error_mark_node)
> - cp_finish_decomp (ndecl, first, cnt);
> - }
> -   else
> - cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
> +
> +   if (ndecl != error_mark_node)
> + cp_maybe_mangle_decomp (ndecl, first, cnt);
> +
> +   cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
> +
> +   if (ndecl != error_mark_node)
> + cp_finish_decomp (ndecl, first, cnt);
>   }
>   }
>   }
> --- gcc/testsuite/g++.dg/cpp1z/decomp46.C.jj2018-08-03 12:00:10.524066454 
> +0200
> +++ gcc/testsuite/g++.dg/cpp1z/decomp46.C   2018-08-03 11:59:49.925018174 
> +0200
> @@ -0,0 +1,25 @@
> +// PR c++/86836
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +struct A {
> +  int operator*();
> +  void operator++();
> +  bool operator!=(A);
> +};
> +template  class map {
> +public:
> +  A begin();
> +  A end();
> +};
> +
> +template  void mergemap(map orig, map toadd) {
> +  for (auto p : toadd)
> +auto [orig] = orig;// { dg-error "use of 'orig' before 
> deduction of 'auto'" }
> +}  // { dg-warning "structured bindings only 
> available with" "" { target c++14_down } .-1 }
> +
> +int
> +main() {
> +  map x, y;
> +  mergemap(x, y);
> +}
>
> Jakub

Re: [PATCH] Make strlen range computations more conservative

[Jeff Law]

On 08/05/2018 12:51 AM, Bernd Edlinger wrote:
> On 08/04/18 22:52, Martin Sebor wrote:
>> On 08/03/2018 01:43 AM, Jakub Jelinek wrote:
>>> On Thu, Aug 02, 2018 at 09:59:13PM -0600, Martin Sebor wrote:
> If I call this with foo (2, 1), do you still claim it is not valid C?

 String functions like strlen operate on character strings stored
 in character arrays.  Calling strlen (&s[1]) is invalid because
 &s[1] is not the address of a character array.  The fact that
 objects can be represented as arrays of bytes doesn't change
 that.  The standard may be somewhat loose with words on this
 distinction but the intent certainly isn't for strlen to traverse
 arbitrary sequences of bytes that cross subobject boundaries.
 (That is the intent behind the raw memory functions, but
 the current text doesn't make the distinction clear.)
>>>
>>> But the standard doesn't say that right now.
>>
>> It does, in the restriction on multi-dimensional array accesses.
>> Given the array 'char a[2][2];' it's only valid to access a[0][0]
>> and a[0][1], and a[1][0], and a[1][1].  It's not valid to access
>> a[2][0] or a[2][1], even though they happen to be located at
>> the same addresses as a[1][0] and a[1][1].
>>
>> There is no exception for distinct struct members.  So in
>> a struct { char a[2], b[2]; }, even though a and b and laid
>> out the same way as char[2][2] would be, it's not valid to
>> treat a as such.  There is no distinction between array
>> subscripting and pointer arithmetic, so it doesn't matter
>> what form the access takes.
>>
>> Yes, the standard could be clearer.  There probably even are
>> ambiguities and contradictions (the authors of the Object Model
>> proposal believe there are and are trying to clarify/remove
>> them).  But the intent is clearly there.  It's especially
>> important for adjacent members of different types (say a char[8]
>> followed by a function pointer.  We definitely don't want writes
>> to the array to be allowed to change the function pointer.)
>>
>>> Plus, at least from the middle-end POV, there is also the case of
>>> placement new and stores changing the dynamic type of the object,
>>> previously say a struct with two fields, then a placement new with a single
>>> char array over it (the placement new will not survive in the middle-end, so
>>> it will be just a memcpy or strcpy or some other byte copy over the original
>>> object, and due to the CSE/SCCVN etc. of pointer to pointer conversions
>>> being in the middle-end useless means you can see a pointer to the struct
>>> with two fields rather than pointer to char array.
>>
>> There may be challenges in the middle-end, you would know much
>> better than me.  All I'm saying is that it's not valid to access
>> [sub]objects by dereferencing pointers to other subobjects.  All
>> the examples in this discussion have been of that form.
>>
> 
> These examples do not aim to be valid C, they just point out limitations
> of the middle-end design, and a good deal of the problems are due
> to trying to do things that are not safe within the boundaries given
> by the middle-end design.
I really think this is important -- and as such I think we need to move
away from trying to describe scenarios in C because doing so keeps
bringing us back to the "C doesn't allow XYZ" kinds of arguments when
what we're really discussing are GIMPLE semantic issues.

So examples should be GIMPLE.  You might start with (possibly invalid) C
code to generate the GIMPLE, but the actual discussion needs to be
looking at GIMPLE.  We might include the C code in case someone wants to
look at things in a debugger, but bringing the focus to GIMPLE is really
important here.

jeff

Re: [PATCH] Make strlen range computations more conservative

[Jeff Law]

On 08/05/2018 12:08 AM, Bernd Edlinger wrote:
> On 08/04/18 23:56, Martin Sebor wrote:
>> On 08/03/2018 01:00 AM, Jeff Law wrote:
>>> On 07/24/2018 05:18 PM, Bernd Edlinger wrote:
 On 07/24/18 23:46, Jeff Law wrote:
> On 07/24/2018 01:59 AM, Bernd Edlinger wrote:
>> Hi!
>>
>> This patch makes strlen range computations more conservative.
>>
>> Firstly if there is a visible type cast from type A to B before passing
>> then value to strlen, don't expect the type layout of B to restrict the
>> possible return value range of strlen.
> Why do you think this is the right thing to do?  ie, is there language
> in the standards that makes you think the code as it stands today is
> incorrect from a conformance standpoint?  Is there a significant body of
> code that is affected in an adverse way by the current code?  If so,
> what code?
>
>

 I think if you have an object, of an effective type A say char[100], then
 you can cast the address of A to B, say typedef char (*B)[2] for instance
 and then to const char *, say for use in strlen.  I may be wrong, but I 
 think
 that we should at least try not to pick up char[2] from B, but instead
 use A for strlen ranges, or leave this range open.  Currently the range
 info for strlen is [0..1] in this case, even if we see the type cast
 in the generic tree.
>>> ISTM that you're essentially saying that the cast to const char *
>>> destroys any type information we can exploit here.  But if that's the
>>> case, then I don't think we can even derive a range of [0,99].  What's
>>> to say that "A" didn't result from a similar cast of some object that
>>> was char[200] that happened out of the scope of what we could see during
>>> the strlen range computation?
>>>
>>> If that is what you're arguing, then I think there's a re-evaluation
>>> that needs to happen WRT strlen range computation/
>>>
>>> And just to be clear, I do see this as a significant correctness question.
>>>
>>> Martin, thoughts?
>>
>> The argument is that given:
>>
>>    struct S { char a[4], b; };
>>
>>    char a[8] = "1234567";
>>
>> this is valid and should pass:
>>
>>    __attribute__ ((noipa))
>>    void f (struct S *p)
>>    {
>>      assert (7 == strlen (p->a));
>>    }
>>
>>    int main (void)
>>    {
>>      f ((struct S*)a);
>>    }
>>
>> (This is the basic premise behind pr86259.)
>>
>> This argument is wrong and the code is invalid.  For the access
>> to p->a to be valid p must point to an object of struct S (it
>> doesn't) and the p->a array must hold a nul-terminated string
>> (it also doesn't).
>>
>> This should not be surprising because the following equivalent
>> code behaves the same way:
>>
>>    __attribute__ ((noipa))
>>    void f (struct S *p)
>>    {
>>      int n = 0;
>>      for (int i = 0; p->a[i]; ++i)
>>    ++n;
>>      if (3 != n)
>>    __builtin_abort ();
>>    }
>>
>> and also because for write accesses, GCC (helpfully) enforces
>> the restriction with _FORTIFY_SOURCE=2:
>>
>>    __attribute__ ((noipa))
>>    void f (struct S *p)
>>    {
>>      strcpy (p->a, "1234");   // aborts
>>    }
>>
>> I care less about the optimization than I do about the basic
>> premise that it's essential to respect subobject boundaries(*).
>> It would make little sense to undo the strlen optimization
>> without also undoing the optimization for the direct array
>> access case.  Undoing either would raise the question about
>> the validity of the _FORRTIFY_SOURCE=2 behavior.  That would
>> be a huge step backwards in terms of code security.  If we
>> did some of these but not others, it would make the behavior
>> inconsistent and surprising, all to accommodate one instance
>> of invalid code.
>>
>> If we had a valid test case where the strlen optimization
>> leads to invalid code, or even if there were a significant
>> number of bug reports showing that it breaks an invalid
>> but common idiom, I would certainly feel compelled to
>> make it right somehow.  But there has been just one bug
>> report with clearly invalid code that should be easily
>> corrected.
>>
> 
> 
> I see this from a software engineering POV.
> 
> If we have code like this:
> 
> void test (const char *x)
> {
>assert (strlen (x) < 10);
> }
> 
> One would usually expect the program to abort (or at least abort with
> a near 100% likelihood) if x is not a valid string with length less
> than 10.
I would not expect the program to abort if this function were called
with an invalid (ie unterminated) string.  In that scenario I know
enough to not expect anything because my program is broken, plain and
simple.

> 
> But if lto and other optimizations show that this code is invoked with
> an invalid, non-zero terminated string the assertion is suddenly gone.
And the program is invalid as it exhibits undefined behavior.  One
undefined behavior is exhibited I have no expectations.  I *like* when
we do something like trap as soon as undef

Re: [PATCH] assume sprintf formatting of wide characters may fail (PR 86853)

[Eric Gallager]

On 8/4/18, Martin Sebor  wrote:
> The sprintf handling of wide characters neglects to consider
> that calling the function may fail due to a conversion error
> (when the wide character is invalid or not representable in
> the current locale).  The handling also misinterprets
> the POSIX %S wide string directive as a plain narrow %s and
> doesn't include %C (the POSIX equivalent of %lc).

I was worried about portability to non-POSIX platforms, but after
checking the documentation for the gnulib sprintf-posix module,
apparently there are no portability issues with platforms that don't
support the %C directive, so I guess it should be fine:
https://www.gnu.org/software/gnulib/manual/html_node/sprintf.html

> The attached patch corrects these oversights by extending the data
> structures to indicate when a directive may fail, and extending the
> UNDER4K member of the format_result structure to also encode
> calls with such directives.
>
> Tested on x86_64-linux.
>
> Besides the trunk, since this bug can affect code correctness
> I'd like to backport this patch to both release branches (7
> and 8).
>
> Martin
>

[PATCH] Fix not properly nul-terminated string constants in JIT

[Bernd Edlinger]

Hi!


My other patch with adds assertions to varasm.c regarding correct
nul termination of sting literals did make these incorrect string
constants in JIT frontend fail.

The string constants are not nul terminated if their length exceeds
200 characters.  The test cases do not use strings of that size where
that would make a difference.  But using a fixed index type is clearly
wrong.

This patch removes the fixed char[200] array type from playback::context,
and uses build_string_literal instead of using build_string directly.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-08-05  Bernd Edlinger  

	* jit-playback.c (playback::context::context): Remove
	m_char_array_type_node.
	(playback::context::new_string_literal): Use
	build_string_literal.
	(playback::context::replay): Remove m_char_array_type_node.
	* jit-playback.h (playback::context::m_char_array_type_node): Remove.

diff -pur gcc/jit/jit-playback.c gcc/jit/jit-playback.c
--- gcc/jit/jit-playback.c	2018-06-28 09:08:01.0 +0200
+++ gcc/jit/jit-playback.c	2018-08-05 15:58:15.815403219 +0200
@@ -81,7 +81,6 @@ playback::context::context (recording::c
   : log_user (ctxt->get_logger ()),
 m_recording_ctxt (ctxt),
 m_tempdir (NULL),
-m_char_array_type_node (NULL),
 m_const_char_ptr (NULL)
 {
   JIT_LOG_SCOPE (get_logger ());
@@ -617,16 +616,9 @@ playback::rvalue *
 playback::context::
 new_string_literal (const char *value)
 {
-  tree t_str = build_string (strlen (value), value);
-  gcc_assert (m_char_array_type_node);
-  TREE_TYPE (t_str) = m_char_array_type_node;
-
-  /* Convert to (const char*), loosely based on
- c/c-typeck.c: array_to_pointer_conversion,
- by taking address of start of string.  */
-  tree t_addr = build1 (ADDR_EXPR, m_const_char_ptr, t_str);
+  tree t_str = build_string_literal (strlen (value) + 1, value);
 
-  return new rvalue (this, t_addr);
+  return new rvalue (this, t_str);
 }
 
 /* Construct a playback::rvalue instance (wrapping a tree) for a
@@ -2633,10 +2625,6 @@ playback::context::
 replay ()
 {
   JIT_LOG_SCOPE (get_logger ());
-  /* Adapted from c-common.c:c_common_nodes_and_builtins.  */
-  tree array_domain_type = build_index_type (size_int (200));
-  m_char_array_type_node
-= build_array_type (char_type_node, array_domain_type);
 
   m_const_char_ptr
 = build_pointer_type (build_qualified_type (char_type_node,
diff -pur gcc/jit/jit-playback.h gcc/jit/jit-playback.h
--- gcc/jit/jit-playback.h	2018-01-03 11:03:58.0 +0100
+++ gcc/jit/jit-playback.h	2018-08-05 15:58:52.988918367 +0200
@@ -316,7 +316,6 @@ private:
 
   auto_vec m_functions;
   auto_vec m_globals;
-  tree m_char_array_type_node;
   tree m_const_char_ptr;
 
   /* Source location handling.  */

[PATCH] Optimize logarithm addition and subtraction

[MCC CS]

Hi everyone,

this patch reduces calls to logarithm functions by
merging log a + log b => log a*b and
log a - log b => log a/b
This is my first contribution, so have I forgot something?
I think a contributor agreement is not needed since it's
a 20-line patch, and I may not have time to contribute in the
future.

Thank you for your comments.

2018-08-05  Deswurstes  

gcc/
PR tree-optimization/86710
* match.pd: Add logarithm addition optimization

gcc/testsuite/
PR tree-optimization/86710
* gcc.dg/pr86710.c: Add logarithm tests

Index: gcc/match.pd
===
--- gcc/match.pd(revision 263305)
+++ gcc/match.pd(working copy)
@@ -4015,6 +4015,18 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(exps (logs @0))
@0))

+(for logs (LOG LOG2 LOG10)
+ /* x * logN(a) + y * logN(b) -> x * y * logN(a * b). */
+ (simplify
+  (plus:c (mult:c @0 (logs:s @1)) (mult:c @2 (logs:s @3)))
+  (mult (mult @0 @2) (logs (mult @1 @3
+
+(for logs (LOG LOG2 LOG10)
+ /* x * logN(a) - y * logN(b) -> x * logN(a / b) / y. */
+ (simplify
+  (sub (mult:c @0 (logs:s @1)) (mult:c @2 (logs:s @3)))
+  (rdiv (mult @0 (logs (rdiv @1 @3))) @3))
+
  /* Optimize logN(func()) for various exponential functions.  We
 want to determine the value "x" and the power "exponent" in
 order to transform logN(x**exponent) into exponent*logN(x).  */
Index: gcc/testsuite/gcc.dg/pr86710.c
===
--- gcc/testsuite/gcc.dg/pr86710.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/pr86710.c  (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-final { scan-assembler-not "log(.*)log(.*)log" } } */
+
+double c;
+
+void f1 (double x, double s)
+{
+  c = 5 * __builtin_log(x) + __builtin_log(s) / 2;
+}
+
+void f2 (double x, double s)
+{
+  c = __builtin_log(x) * 3 - 7 * __builtin_log(s);
+}

Besides, if you think optimizing
/* logN(a) + logN(b) -> logN(a * b) */
would be enough, without the coefficients,
here's a simpler patch:

Index: gcc/match.pd
===
--- gcc/match.pd(revision 263305)
+++ gcc/match.pd(working copy)
@@ -4015,6 +4015,18 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(exps (logs @0))
@0))

+(for logs (LOG LOG2 LOG10)
+ /* logN(a) + logN(b) -> logN(a * b). */
+ (simplify
+  (plus:c (logs:s (@0)) (logs:s (@1)))
+  (logs (mult (@0) (@1
+
+(for logs (LOG LOG2 LOG10)
+ /* logN(a) - logN(b) -> logN(a / b). */
+ (simplify
+  (sub (logs:s (@0)) (logs:s (@1)))
+  (logs (rdiv (@0) (@1
+
  /* Optimize logN(func()) for various exponential functions.  We
 want to determine the value "x" and the power "exponent" in
 order to transform logN(x**exponent) into exponent*logN(x).  */
Index: gcc/testsuite/gcc.dg/pr86710.c
===
--- gcc/testsuite/gcc.dg/pr86710.c(nonexistent)
+++ gcc/testsuite/gcc.dg/pr86710.c(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "log(.*)log(.*)log" } } */
+
+double c;
+
+void f1 (double x, double s)
+{
+  c = __builtin_log(x) + __builtin_log(s);
+}
+
+void f2 (double x, double s)
+{
+  c = __builtin_log(x) - __builtin_log(s);
+}

Re: [PATCH] Make strlen range computations more conservative

[Jeff Law]

On 08/04/2018 02:52 PM, Martin Sebor wrote:
> On 08/03/2018 01:43 AM, Jakub Jelinek wrote:
>> On Thu, Aug 02, 2018 at 09:59:13PM -0600, Martin Sebor wrote:
 If I call this with foo (2, 1), do you still claim it is not valid C?
>>>
>>> String functions like strlen operate on character strings stored
>>> in character arrays.  Calling strlen (&s[1]) is invalid because
>>> &s[1] is not the address of a character array.  The fact that
>>> objects can be represented as arrays of bytes doesn't change
>>> that.  The standard may be somewhat loose with words on this
>>> distinction but the intent certainly isn't for strlen to traverse
>>> arbitrary sequences of bytes that cross subobject boundaries.
>>> (That is the intent behind the raw memory functions, but
>>> the current text doesn't make the distinction clear.)
>>
>> But the standard doesn't say that right now.
> 
> It does, in the restriction on multi-dimensional array accesses.
> Given the array 'char a[2][2];' it's only valid to access a[0][0]
> and a[0][1], and a[1][0], and a[1][1].  It's not valid to access
> a[2][0] or a[2][1], even though they happen to be located at
> the same addresses as a[1][0] and a[1][1].
> 
> There is no exception for distinct struct members.  So in
> a struct { char a[2], b[2]; }, even though a and b and laid
> out the same way as char[2][2] would be, it's not valid to
> treat a as such.  There is no distinction between array
> subscripting and pointer arithmetic, so it doesn't matter
> what form the access takes.
Understood WRT what the C language says.

Let's bring it back to GIMPLE though.  In GIMPLE we allow those some
crossing of subobject boundaries as explained earlier in the thread.
It's not ideal, but that's the way things are.

So with that in mind, I think we need to reevaluate some of the
assumptions we're making in this code.


Jeff

Re: [PATCH] Make strlen range computations more conservative

[Richard Biener]

On August 4, 2018 10:52:02 PM GMT+02:00, Martin Sebor  wrote:
>On 08/03/2018 01:43 AM, Jakub Jelinek wrote:
>> On Thu, Aug 02, 2018 at 09:59:13PM -0600, Martin Sebor wrote:
 If I call this with foo (2, 1), do you still claim it is not valid
>C?
>>>
>>> String functions like strlen operate on character strings stored
>>> in character arrays.  Calling strlen (&s[1]) is invalid because
>>> &s[1] is not the address of a character array.  The fact that
>>> objects can be represented as arrays of bytes doesn't change
>>> that.  The standard may be somewhat loose with words on this
>>> distinction but the intent certainly isn't for strlen to traverse
>>> arbitrary sequences of bytes that cross subobject boundaries.
>>> (That is the intent behind the raw memory functions, but
>>> the current text doesn't make the distinction clear.)
>>
>> But the standard doesn't say that right now.
>
>It does, in the restriction on multi-dimensional array accesses.
>Given the array 'char a[2][2];' it's only valid to access a[0][0]
>and a[0][1], and a[1][0], and a[1][1].  It's not valid to access
>a[2][0] or a[2][1], even though they happen to be located at
>the same addresses as a[1][0] and a[1][1].
>
>There is no exception for distinct struct members.  So in
>a struct { char a[2], b[2]; }, even though a and b and laid
>out the same way as char[2][2] would be, it's not valid to
>treat a as such.  There is no distinction between array
>subscripting and pointer arithmetic, so it doesn't matter
>what form the access takes.

What does the standard say to comparing & s. a[2] and & s. b[0] and what does 
that mean when you consider converting those to uintptr_t and back and then 
access the data pointed to? 
Points-to analysis considers the first pointer to point to both subobjects 
while the second only to the second. (just pointing out other maybe 
inconsistent itself within GIMPLE handling of subobjects in points-to analysis) 

Richard. 

>Yes, the standard could be clearer.  There probably even are
>ambiguities and contradictions (the authors of the Object Model
>proposal believe there are and are trying to clarify/remove
>them).  But the intent is clearly there.  It's especially
>important for adjacent members of different types (say a char[8]
>followed by a function pointer.  We definitely don't want writes
>to the array to be allowed to change the function pointer.)
>
>> Plus, at least from the middle-end POV, there is also the case of
>> placement new and stores changing the dynamic type of the object,
>> previously say a struct with two fields, then a placement new with a
>single
>> char array over it (the placement new will not survive in the
>middle-end, so
>> it will be just a memcpy or strcpy or some other byte copy over the
>original
>> object, and due to the CSE/SCCVN etc. of pointer to pointer
>conversions
>> being in the middle-end useless means you can see a pointer to the
>struct
>> with two fields rather than pointer to char array.
>
>There may be challenges in the middle-end, you would know much
>better than me.  All I'm saying is that it's not valid to access
>[sub]objects by dereferencing pointers to other subobjects.  All
>the examples in this discussion have been of that form.
>
>>
>> Consider e.g.
>> typedef __typeof__ (sizeof 0) size_t;
>> void *operator new (size_t, void *p) { return p; }
>> void *operator new[] (size_t, void *p) { return p; }
>> struct S { char a; char b[64]; };
>> void baz (char *);
>>
>> size_t
>> foo (S *p)
>> {
>>   baz (&p->a);
>>   char *q = new (p) char [16];
>>   baz (q);
>>   return __builtin_strlen (q);
>> }
>>
>> I don't think it is correct to say that strlen must be 0.  In this
>testcase
>> the pointer passed to strlen is still S *, though I think with enough
>> tweaking you could also have something where the argument is &p->a.
>
>I think the problem here is changing the type of p->a.  I'm
>not up on the latest C++ changes here but I think it's a known
>problem with the specification.  A similar (known) problem also
>comes in the case of dynamically allocated objects:
>
>   char *p = (char*)operator new (2);
>   char *p1 = new (p) char ('a');
>   char *p2 = new (p) char ('\0');
>   strlen (p1);
>
>Is the strlen(p) call valid when there's no string or array
>at p: there is a singlelton char object that just happens
>to be followed by another singleton char object.  It's not
>an array of two elements.  Each is [an array of] one char.
>
>This is a (specification) problem for sequence containers like
>vector where strictly speaking, it's not valid to iterate over
>them because of the array restriction.
>
>>
>> I have no problem for strlen to return 0 if it sees a toplevel object
>of
>> size 1, but note that if it is extern, it already might be a problem
>in some
>> cases:
>> struct T { char a; char a2[]; } b;
>> extern struct T c;
>> void foo (int *p) { p[0] = strlen (b); p[1] = strlen (c); }
>> If c's definition is struct T c = { ' ', "abcde" };
>> then the object doesn't have

Re: [PATCH] Optimize logarithm addition and subtraction

[Marc Glisse]

On Sun, 5 Aug 2018, MCC CS wrote:


this patch reduces calls to logarithm functions by
merging log a + log b => log a*b and


this makes sense.


+ /* x * logN(a) + y * logN(b) -> x * y * logN(a * b). */


this on the other hand... Can you explain the math?

--
Marc Glisse

Re: [PATCH] Optimize logarithm addition and subtraction

[Bernd Edlinger]

> merging log a + log b => log a*b and

Maybe a*b could overflow, while adding the logarithms would not?


Bernd.

Re: [PATCH] Optimize logarithm addition and subtraction

[Andreas Schwab]

On Aug 05 2018, Marc Glisse  wrote:

> On Sun, 5 Aug 2018, MCC CS wrote:
>
>> this patch reduces calls to logarithm functions by
>> merging log a + log b => log a*b and
>
> this makes sense.

Even when a*b may overflow?

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Optimize logarithm addition and subtraction

[Marc Glisse]

On Sun, 5 Aug 2018, Bernd Edlinger wrote:


merging log a + log b => log a*b and


Maybe a*b could overflow, while adding the logarithms would not?


Well, that's a discussion that happens every time a new transformation is 
added to -funsafe-math-optimizations (I assume this one is under that 
umbrella?). We already may get extra overflow with -fassociative-math for 
instance. Sure, the overflow (or underflow!) is more likely for log than 
for addition, but this still seems like the kind of transformation that 
-ffast-math should enable, in my opinion. Now we can see if I am in the 
minority...


--
Marc Glisse

Re: [PATCH] Optimize logarithm addition and subtraction

[Bernd Edlinger]

On 08/05/18 20:35, Marc Glisse wrote:
> On Sun, 5 Aug 2018, Bernd Edlinger wrote:
> 
>>> merging log a + log b => log a*b and
>>
>> Maybe a*b could overflow, while adding the logarithms would not?
> 
> Well, that's a discussion that happens every time a new transformation is 
> added to -funsafe-math-optimizations (I assume this one is under that 
> umbrella?). We already may get extra overflow with -fassociative-math for 
> instance. Sure, the overflow (or underflow!) is more likely for log than for 
> addition, but this still seems like the kind of transformation that 
> -ffast-math should enable, in my opinion. Now we can see if I am in the 
> minority...
> 

Yes, this should be unsafe-math

Regarding the second transformation, I think it is invalid.

I tried in sage:

sage: a=2
sage: b=3
sage: x=5
sage: y=6
sage: p = x*log(a) + y*log(b)
sage: q = x*y*log(a*b)
sage: print p
6*log(3) + 5*log(2)
sage: print q
30*log(6)
sage: print numerical_approx(p)
10.0574096348084
sage: print numerical_approx(q)
53.7527840768416


Bernd.

Re: [PATCH] Optimize logarithm addition and subtraction

[MCC CS]

> Sent: Sunday, August 05, 2018 at 8:17 PM
> From: "Marc Glisse" 
> To: "MCC CS" 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Optimize logarithm addition and subtraction
>
> On Sun, 5 Aug 2018, MCC CS wrote:
> 
> > this patch reduces calls to logarithm functions by
> > merging log a + log b => log a*b and
> 
> this makes sense.
> 
> > + /* x * logN(a) + y * logN(b) -> x * y * logN(a * b). */
> 
> this on the other hand... Can you explain the math?
> 
> -- 
> Marc Glisse
> 


Oops, please ignore the patch at the top and 
review the second patch. I must have done 
A mistake somewhere. It's actually x * y * log(a^(1/y)b^(1/x))

Re: [PATCH] Optimize logarithm addition and subtraction

[Marc Glisse]

On Sun, 5 Aug 2018, MCC CS wrote:


Besides, if you think optimizing
/* logN(a) + logN(b) -> logN(a * b) */
would be enough, without the coefficients,
here's a simpler patch:

Index: gcc/match.pd
===
--- gcc/match.pd(revision 263305)
+++ gcc/match.pd(working copy)
@@ -4015,6 +4015,18 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (exps (logs @0))
   @0))

+(for logs (LOG LOG2 LOG10)


You seem to be missing some indentation, see neighboring
transformations.


+ /* logN(a) + logN(b) -> logN(a * b). */
+ (simplify
+  (plus:c (logs:s (@0)) (logs:s (@1)))
+  (logs (mult (@0) (@1


No parentheses around @0 and @1. No :c here, the pattern is symmetric
and a permutation would be redundant.


+
+(for logs (LOG LOG2 LOG10)
+ /* logN(a) - logN(b) -> logN(a / b). */
+ (simplify
+  (sub (logs:s (@0)) (logs:s (@1)))
+  (logs (rdiv (@0) (@1


It would be possible to merge the 2 with a second 'for', but that's not a 
requirement.



+
 /* Optimize logN(func()) for various exponential functions.  We
want to determine the value "x" and the power "exponent" in
order to transform logN(x**exponent) into exponent*logN(x).  */
Index: gcc/testsuite/gcc.dg/pr86710.c
===
--- gcc/testsuite/gcc.dg/pr86710.c(nonexistent)
+++ gcc/testsuite/gcc.dg/pr86710.c(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */


The transformation should not happen at -O2, it should require
-funsafe-math-optimizations or similar.


+/* { dg-final { scan-assembler-not "log(.*)log(.*)log" } } */


I think I'd rather scan-tree-dump-times the "optimized" dump.


+
+double c;
+
+void f1 (double x, double s)
+{
+  c = __builtin_log(x) + __builtin_log(s);
+}
+
+void f2 (double x, double s)
+{
+  c = __builtin_log(x) - __builtin_log(s);
+}


You need to specify how you tested the patch (bootstrap on what platform, 
run make check, whatever). And then we need to see if other people think 
-funsafe-math-optimization is enough protection or the transformation is 
too dangerous...

Re: [patch] improve internals documentation for nested function descriptors

[Sandra Loosemore]

On 08/03/2018 12:16 PM, Jeff Law wrote:

On 07/27/2018 03:44 PM, Eric Botcazou wrote:

Apropos of the discussion about improving the docs for
TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
submission,

https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html

here is the patch I've come up with based on reading the source.  Is
this technically accurate?  Any suggestions on how to improve it further?


""Define this hook to 0 if the target implements custom support"

"custom" was precisely chosen to distinguish this kind of descriptors from the
standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover
I would really start with the "custom" case and not the standard case as was
originally written, the "custom" case being the common case for targets.

I'm not really convinced by the substitution misalignment -> tag either, but
if others find the new version more understandable, then OK with me.

I think the new version is an improvement, but I don't think it's ideal
due to the issues noted above.

We're trying to distinguish between the ABI mandated function
descriptors and those generated internally for supporting nested
functions.  Wordsmithing that into the docs might make things clearer.
I'm not sure how to express that clearly in the name of the target hook
though.


Hmmm.  I agree that the name of the hook is confusing.  Part of the 
problem is that it does two different things, but I don't have any good 
ideas for how to fix that.


Here's another attempt at documenting what's there now, this time 
avoiding using the ambiguous term "custom" at all in the text.  I've 
also tried to address the other comments.  Is this any better?


-Sandra
2018-08-05  Sandra Loosemore  

	gcc/
	* target.def (custom_function_descriptors): Improve documentation.
	* doc/tm.texi.in (Trampolines): Expand discussion of function
	descriptors and move TARGET_CUSTOM_FUNCTION_DESCRIPTORS to the
	beginning of the section.
	* doc/tm.texi: Regenerated.
Index: gcc/doc/tm.texi
===
--- gcc/doc/tm.texi	(revision 263300)
+++ gcc/doc/tm.texi	(working copy)
@@ -5267,24 +5267,78 @@ into the stack.  Arguments meaning is si
 @end deftypefn
 
 @node Trampolines
-@section Trampolines for Nested Functions
+@section Support for Nested Functions
+@cindex support for nested functions
 @cindex trampolines for nested functions
-@cindex nested functions, trampolines for
+@cindex descriptors for nested functions
+@cindex nested functions, support for
 
-A @dfn{trampoline} is a small piece of code that is created at run time
-when the address of a nested function is taken.  It normally resides on
-the stack, in the stack frame of the containing function.  These macros
-tell GCC how to generate code to allocate and initialize a
-trampoline.
-
-The instructions in the trampoline must do two things: load a constant
-address into the static chain register, and jump to the real address of
-the nested function.  On CISC machines such as the m68k, this requires
-two instructions, a move immediate and a jump.  Then the two addresses
-exist in the trampoline as word-long immediate operands.  On RISC
-machines, it is often necessary to load each address into a register in
-two parts.  Then pieces of each address form separate immediate
-operands.
+Taking the address of a nested function requires special compiler
+handling to ensure that the static chain register is loaded when
+the function is invoked via an indirect call.
+
+GCC has traditionally supported nested functions by creating an
+executable @dfn{trampoline} at run time when the address of a nested
+function is taken.  This is a small piece of code which normally
+resides on the stack, in the stack frame of the containing function.
+The trampoline loads the static chain register and then jumps to the
+real address of the nested function.
+
+The use of trampolines requires an executable stack, which is a
+security risk.  To avoid this problem, GCC also supports another
+strategy: using descriptors for nested functions.  Under this model,
+taking the address of a nested function results in a pointer to a
+non-executable function descriptor object.  Initializing the static chain
+from the descriptor is handled at indirect call sites.
+
+On some targets, including HPPA and IA-64, function descriptors may be
+mandated by the ABI or be otherwise handled in a target-specific way
+by the back end in its code generation strategy for indirect calls.
+GCC also provides its own generic descriptor implementation to support the
+@option{-fno-trampolines} option.  In this case runtime detection of
+function descriptors at indirect call sites relies on descriptor
+pointers being tagged with a bit that is never set in bare function
+addresses.  Since GCC's generic function descriptors are
+not ABI-compliant, this option is typically used only on a
+per-language basis (notably by Ada) or when it can otherwise be
+appl

[0/5] C-SKY port v3

[Sandra Loosemore]

This is the third iteration of the C-SKY port patch set.  The previous 
versions were posted here:


https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01289.html
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01861.html

Changes since V2 are:

- The cse_cc pass is gone, replaced by a no-op move of the cc register 
to allow the regular CSE pass to work.


- The backend defines the new TARGET_HAVE_SPECULATION_SAFE_VALUE hook 
(this architecture isn't affected by speculative-execution bugs).


Same 5 pieces in the series as before:

[1] Configury
[2] Backend implementation
[3] Documentation
[4] Testsuite
[5] libgcc

-Sandra

[1/5] C-SKY port v3: Configury

[Sandra Loosemore]

 2018-08-05  Jojo  
	Huibin Wang  
	Sandra Loosemore  
	Chung-Lin Tang  
	Andrew Jenner  

	C-SKY port: Configury

	gcc/
	* config.gcc (csky-*-*): New.
	* configure.ac: Add csky to targets for dwarf2 debug_line support.
	* configure: Regenerated.

	contrib/
	* config-list.mk (LIST): Add csky-elf and csky-linux-gnu.


diff --git a/contrib/config-list.mk b/contrib/config-list.mk
index c3537d2..d9e48a9 100644
--- a/contrib/config-list.mk
+++ b/contrib/config-list.mk
@@ -40,6 +40,7 @@ LIST = aarch64-elf aarch64-linux-gnu aarch64-rtems \
   arm-symbianelf avr-elf \
   bfin-elf bfin-uclinux bfin-linux-uclibc bfin-rtems bfin-openbsd \
   c6x-elf c6x-uclinux cr16-elf cris-elf cris-linux crisv32-elf crisv32-linux \
+  csky-elf csky-linux-gnu \
   epiphany-elf epiphany-elfOPT-with-stack-offset=16 fido-elf \
   fr30-elf frv-elf frv-linux ft32-elf h8300-elf hppa-linux-gnu \
   hppa-linux-gnuOPT-enable-sjlj-exceptions=yes hppa64-linux-gnu \
diff --git a/gcc/config.gcc b/gcc/config.gcc
index b17fdba..351b2cf 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1278,6 +1278,70 @@ crisv32-*-linux* | cris-*-linux*)
 		;;
 	esac
 	;;
+csky-*-*)
+	if test x${with_endian} != x; then
+	case ${with_endian} in
+		big|little)		;;
+		*)
+		echo "with_endian=${with_endian} not supported."
+		exit 1
+		;;
+	esac
+	fi
+	if test x${with_float} != x; then
+	case ${with_float} in
+		soft | hard) ;;
+		*) echo
+		"Unknown floating point type used in --with-float=$with_float"
+		exit 1
+		;;
+	esac
+	fi
+	tm_file="csky/csky.h"
+	md_file="csky/csky.md"
+	out_file="csky/csky.c"
+	tm_p_file="${tm_p_file} csky/csky-protos.h"
+	extra_options="${extra_options} csky/csky_tables.opt"
+
+	if test x${enable_tpf_debug} = xyes; then
+	tm_defines="${tm_defines} ENABLE_TPF_DEBUG"
+	fi
+
+	case ${target} in
+	csky-*-elf*)
+		tm_file="dbxelf.h elfos.h newlib-stdint.h ${tm_file} csky/csky-elf.h"
+		tmake_file="csky/t-csky csky/t-csky-elf"
+		default_use_cxa_atexit=no
+		;;
+	csky-*-linux*)
+		tm_file="dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h ${tm_file} csky/csky-linux-elf.h"
+		tmake_file="${tmake_file} csky/t-csky csky/t-csky-linux"
+
+		if test "x${enable_multilib}" = xyes ; then
+		tm_file="$tm_file ./sysroot-suffix.h"
+		tmake_file="${tmake_file} csky/t-sysroot-suffix"
+		fi
+
+		case ${target} in
+		csky-*-linux-gnu*)
+			tm_defines="$tm_defines DEFAULT_LIBC=LIBC_GLIBC"
+			;;
+		csky-*-linux-uclibc*)
+			tm_defines="$tm_defines DEFAULT_LIBC=LIBC_UCLIBC"
+			default_use_cxa_atexit=no
+			;;
+		*)
+			echo "Unknown target $target"
+			exit 1
+			;;
+		esac
+		;;
+	*)
+		echo "Unknown target $target"
+		exit 1
+		;;
+	esac
+	;;
 epiphany-*-elf | epiphany-*-rtems*)
 	tm_file="${tm_file} dbxelf.h elfos.h"
 	tmake_file="${tmake_file} epiphany/t-epiphany"
@@ -3831,6 +3895,10 @@ case "${target}" in
 		fi
 		;;
 
+csky-*-*)
+	supported_defaults="cpu endian float"
+	;;
+
 	arm*-*-*)
 		supported_defaults="arch cpu float tune fpu abi mode tls"
 		for which in cpu tune arch; do
diff --git a/gcc/configure b/gcc/configure
index 80ac4a3..b7a8e36 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -27838,7 +27838,7 @@ esac
 # ??? Once 2.11 is released, probably need to add first known working
 # version to the per-target configury.
 case "$cpu_type" in
-  aarch64 | alpha | arc | arm | avr | bfin | cris | i386 | m32c | m68k \
+  aarch64 | alpha | arc | arm | avr | bfin | cris | csky | i386 | m32c | m68k \
   | microblaze | mips | nios2 | pa | riscv | rs6000 | score | sparc | spu \
   | tilegx | tilepro | visium | xstormy16 | xtensa)
 insn="nop"
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 4fc851c..65f9c92 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4932,7 +4932,7 @@ esac
 # ??? Once 2.11 is released, probably need to add first known working
 # version to the per-target configury.
 case "$cpu_type" in
-  aarch64 | alpha | arc | arm | avr | bfin | cris | i386 | m32c | m68k \
+  aarch64 | alpha | arc | arm | avr | bfin | cris | csky | i386 | m32c | m68k \
   | microblaze | mips | nios2 | pa | riscv | rs6000 | score | sparc | spu \
   | tilegx | tilepro | visium | xstormy16 | xtensa)
 insn="nop"

[2/5] C-SKY port v3: Backend implementation

[Sandra Loosemore]

 2018-08-05  Jojo  
	Huibin Wang  
	Sandra Loosemore  
	Chung-Lin Tang  

	C-SKY port: Backend implementation

	gcc/
	* config/csky/*: New.
	* common/config/csky/*: New.


csky-gcc-2.patch.gz
Description: application/gzip

[3/5] C-SKY port v3: Documentation

[Sandra Loosemore]

2018-08-05  Sandra Loosemore  

	C-SKY port: Documentation

	gcc/
	* doc/extend.texi (C-SKY Function Attributes): New section.
	* doc/invoke.texi (Option Summary): Add C-SKY options.
	(C-SKY Options): New section.
	* doc/md.texi (Machine Constraints): Document C-SKY constraints.
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index bf465d7..4bab786 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2324,6 +2324,7 @@ GCC plugins may provide their own attributes.
 * AVR Function Attributes::
 * Blackfin Function Attributes::
 * CR16 Function Attributes::
+* C-SKY Function Attributes::
 * Epiphany Function Attributes::
 * H8/300 Function Attributes::
 * IA-64 Function Attributes::
@@ -4145,6 +4146,38 @@ function entry and exit sequences suitable for use in an interrupt handler
 when this attribute is present.
 @end table
 
+@node C-SKY Function Attributes
+@subsection C-SKY Function Attributes
+
+These function attributes are supported by the C-SKY back end:
+
+@table @code
+@item interrupt
+@itemx isr
+@cindex @code{interrupt} function attribute, C-SKY
+@cindex @code{isr} function attribute, C-SKY
+Use these attributes to indicate that the specified function
+is an interrupt handler.
+The compiler generates function entry and exit sequences suitable for
+use in an interrupt handler when either of these attributes are present.
+
+Use of these options requires the @option{-mistack} command-line option
+to enable support for the necessary interrupt stack instructions.  They
+are ignored with a warning otherwise.  @xref{C-SKY Options}.
+
+@item naked
+@cindex @code{naked} function attribute, C-SKY
+This attribute allows the compiler to construct the
+requisite function declaration, while allowing the body of the
+function to be assembly code. The specified function will not have
+prologue/epilogue sequences generated by the compiler. Only basic
+@code{asm} statements can safely be included in naked functions
+(@pxref{Basic Asm}). While using extended @code{asm} or a mixture of
+basic @code{asm} and C code may appear to work, they cannot be
+depended upon to work reliably and are not supported.
+@end table
+
+
 @node Epiphany Function Attributes
 @subsection Epiphany Function Attributes
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6047d82..f6016d2 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -722,6 +722,16 @@ Objective-C and Objective-C++ Dialects}.
 -msim  -mint32  -mbit-ops
 -mdata-model=@var{model}}
 
+@emph{C-SKY Options}
+@gccoptlist{-march=@var{arch}  -mcpu=@var{cpu} @gol
+-mbig-endian  -EB  -mlittle-endian  -EL @gol
+-mhard-float  -msoft-float  -mfpu=@var{fpu}  -mdouble-float  -mfdivdu @gol
+-melrw  -mistack  -mmp  -mcp  -mcache  -msecurity  -mtrust @gol
+-mdsp  -medsp  -mvdsp @gol
+-mdiv  -msmart  -mhigh-registers  -manchor @gol
+-mpushpop  -mmultiple-stld  -mconstpool  -mstack-size  -mccrt @gol
+-mbranch-cost=@var{n}  -mcse-cc  -msched-prolog}
+
 @emph{Darwin Options}
 @gccoptlist{-all_load  -allowable_client  -arch  -arch_errors_fatal @gol
 -arch_only  -bind_at_load  -bundle  -bundle_loader @gol
@@ -14605,6 +14615,7 @@ platform.
 * C6X Options::
 * CRIS Options::
 * CR16 Options::
+* C-SKY Options::
 * Darwin Options::
 * DEC Alpha Options::
 * FR30 Options::
@@ -17729,6 +17740,198 @@ However, @samp{far} is not valid with @option{-mcr16c}, as the
 CR16C architecture does not support the far data model.
 @end table
 
+@node C-SKY Options
+@subsection C-SKY Options
+@cindex C-SKY Options
+
+GCC supports these options when compiling for C-SKY V2 processors.
+
+@table @gcctabopt
+
+@item -march=@var{arch}
+@opindex march=
+Specify the C-SKY target architecture.  Valid values for @var{arch} are:
+@samp{ck801}, @samp{ck802}, @samp{ck803}, @samp{ck807}, and @samp{ck810}.
+The default is @samp{ck810}.
+
+@item -mcpu=@var{cpu}
+@opindex mcpu=
+Specify the C-SKY target processor.  Valid values for @var{cpu} are:
+@samp{ck801}, @samp{ck801t},
+@samp{ck802}, @samp{ck802t}, @samp{ck802j},
+@samp{ck803}, @samp{ck803h}, @samp{ck803t}, @samp{ck803ht},
+@samp{ck803f}, @samp{ck803fh}, @samp{ck803e}, @samp{ck803eh},
+@samp{ck803et}, @samp{ck803eht}, @samp{ck803ef}, @samp{ck803efh},
+@samp{ck803ft}, @samp{ck803eft}, @samp{ck803efht}, @samp{ck803r1},
+@samp{ck803hr1}, @samp{ck803tr1}, @samp{ck803htr1}, @samp{ck803fr1},
+@samp{ck803fhr1}, @samp{ck803er1}, @samp{ck803ehr1}, @samp{ck803etr1},
+@samp{ck803ehtr1}, @samp{ck803efr1}, @samp{ck803efhr1}, @samp{ck803ftr1},
+@samp{ck803eftr1}, @samp{ck803efhtr1},
+@samp{ck803s}, @samp{ck803st}, @samp{ck803se}, @samp{ck803sf},
+@samp{ck803sef}, @samp{ck803seft},
+@samp{ck807e}, @samp{ck807ef}, @samp{ck807}, @samp{ck807f},
+@samp{ck810e}, @samp{ck810et}, @samp{ck810ef}, @samp{ck810eft},
+@samp{ck810}, @samp{ck810v}, @samp{ck810f}, @samp{ck810t}, @samp{ck810fv},
+@samp{ck810tv}, @samp{ck810ft}, and @samp{ck810ftv}.
+
+@item -mbig-endian
+@opindex mbig-endian
+@itemx -EB
+@opindex -EB
+@itemx -mlittle-endian
+@opindex mlittle-endian

[4/5] C-SKY port v3: Testsuite

[Sandra Loosemore]

 2018-08-05  Sandra Loosemore  
	Chung-Lin Tang  
	Xianmiao Qu  

	C-SKY port: Testsuite

	gcc/testsuite/
	* g++.dg/Wno-frame-address.C: Adjust for C-SKY.
	* g++.dg/torture/type-generic-1.C: Likewise.
	* gcc.c-torture/compile/2804-1.c: Likewise.
	* gcc.c-torture/execute/20101011-1.c: Likewise.
	* gcc.c-torture/execute/ieee/mul-subnormal-single-1.x: Likewise.
	* gcc.dg/20020312-2.c: Likewise.
	* gcc.dg/Wno-frame-address.c: Likewise.
	* gcc.dg/c11-true_min-1.c: Likewise.
	* gcc.dg/sibcall-10.c: Likewise.
	* gcc.dg/sibcall-9.c: Likewise.
	* gcc.dg/stack-usage-1.c: Likewise.
	* gcc.dg/torture/float32-tg-3.c: Likewise.
	* gcc.dg/torture/float32x-tg-3.c: Likewise.
	* gcc.dg/torture/float64-tg-3.c: Likewise.
	* gcc.dg/torture/float64x-tg-3.c: Likewise.
	* gcc.dg/torture/type-generic-1.c: Likewise.
	* gcc.target/csky/*: New.
	* lib/target-supports.exp (check_profiling_available): Add
	csky-*-elf.
	(check_effective_target_hard_float): Handle C-SKY targets with
	single-precision hard float only.
	(check_effective_target_logical_op_short_circuit): Handle C-SKY.
diff --git a/gcc/testsuite/g++.dg/Wno-frame-address.C b/gcc/testsuite/g++.dg/Wno-frame-address.C
index a2df034..54a02fe 100644
--- a/gcc/testsuite/g++.dg/Wno-frame-address.C
+++ b/gcc/testsuite/g++.dg/Wno-frame-address.C
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-skip-if "Cannot access arbitrary stack frames." { arm*-*-* hppa*-*-* ia64-*-* } }
+// { dg-skip-if "Cannot access arbitrary stack frames." { arm*-*-* hppa*-*-* ia64-*-* csky*-*-* } }
 // { dg-options "-Werror" }
 // { dg-additional-options "-mbackchain" { target s390*-*-* } }
 
diff --git a/gcc/testsuite/g++.dg/torture/type-generic-1.C b/gcc/testsuite/g++.dg/torture/type-generic-1.C
index 4d82592..7708724 100644
--- a/gcc/testsuite/g++.dg/torture/type-generic-1.C
+++ b/gcc/testsuite/g++.dg/torture/type-generic-1.C
@@ -4,6 +4,7 @@
 /* { dg-do run } */
 /* { dg-add-options ieee } */
 /* { dg-skip-if "No Inf/NaN support" { spu-*-* } } */
+/* { dg-skip-if "No subnormal support" { csky-*-* } { "-mhard-float" } } */
 
 #include "../../gcc.dg/tg-tests.h"
 
diff --git a/gcc/testsuite/gcc.c-torture/compile/2804-1.c b/gcc/testsuite/gcc.c-torture/compile/2804-1.c
index 5c6b731..35464c2 100644
--- a/gcc/testsuite/gcc.c-torture/compile/2804-1.c
+++ b/gcc/testsuite/gcc.c-torture/compile/2804-1.c
@@ -4,6 +4,7 @@
 /* { dg-skip-if "" { { i?86-*-* x86_64-*-* } && { ia32 && { ! nonpic } } } } */
 /* { dg-skip-if "No 64-bit registers" { m32c-*-* } } */
 /* { dg-skip-if "Not enough 64-bit registers" { pdp11-*-* } { "-O0" } { "" } } */
+/* { dg-xfail-if "Inconsistent constraint on asm" { csky-*-* } { "-O0" } { "" } } */
 /* { dg-xfail-if "" { h8300-*-* } } */
 
 /* Copyright (C) 2000, 2003 Free Software Foundation */
diff --git a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
index dda49a5..f95d900 100644
--- a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
@@ -93,6 +93,10 @@ __aeabi_idiv0 (int return_value)
 #elif defined (__nvptx__)
 /* There isn't even a signal function.  */
 # define DO_TEST 0
+#elif defined (__csky__)
+  /* This presently doesn't raise SIGFPE even on csky-linux-gnu, much
+ less bare metal.  See the implementation of __divsi3 in libgcc.  */
+# define DO_TEST 0
 #else
 # define DO_TEST 1
 #endif
diff --git a/gcc/testsuite/gcc.c-torture/execute/ieee/mul-subnormal-single-1.x b/gcc/testsuite/gcc.c-torture/execute/ieee/mul-subnormal-single-1.x
index 16df951..ee40863 100644
--- a/gcc/testsuite/gcc.c-torture/execute/ieee/mul-subnormal-single-1.x
+++ b/gcc/testsuite/gcc.c-torture/execute/ieee/mul-subnormal-single-1.x
@@ -1,3 +1,8 @@
+if {[istarget "csky-*-*"] && [check_effective_target_hard_float]} {
+# The C-SKY hardware FPU only supports flush-to-zero mode.
+set torture_execute_xfail "csky-*-*"
+return 1
+}
 if [istarget "epiphany-*-*"] {
 # The Epiphany single-precision floating point format does not
 # support subnormals.
diff --git a/gcc/testsuite/gcc.dg/20020312-2.c b/gcc/testsuite/gcc.dg/20020312-2.c
index f5929e0..f8be3ce 100644
--- a/gcc/testsuite/gcc.dg/20020312-2.c
+++ b/gcc/testsuite/gcc.dg/20020312-2.c
@@ -111,6 +111,11 @@ extern void abort (void);
 /* No pic register.  */
 #elif defined (__nvptx__)
 /* No pic register.  */
+#elif defined (__csky__)
+/* Pic register is r28, but some cores only have r0-r15.  */
+# if defined (__CK807__) || defined (__CK810__)
+#   define PIC_REG  "r28"
+# endif
 #else
 # error "Modify the test for your target."
 #endif
diff --git a/gcc/testsuite/gcc.dg/Wno-frame-address.c b/gcc/testsuite/gcc.dg/Wno-frame-address.c
index e6dfe52..9fe4d07 100644
--- a/gcc/testsuite/gcc.dg/Wno-frame-address.c
+++ b/gcc/testsuite/gcc.dg/Wno-frame-address.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-skip-if "Cannot access arbitrary stack frames" { arm*-*-* avr-*-* hppa*-*-* ia64-*-* visium-*-* } } */
+/* { dg-skip-if

[5/5] C-SKY port v3: libgcc

[Sandra Loosemore]

 2018-08-05  Jojo  
	Huibin Wang  
	Sandra Loosemore  
	Chung-Lin Tang  

	C-SKY port: libgcc

	libgcc/
	* config.host: Add C-SKY support.
	* config/csky/*: New.
diff --git a/libgcc/config.host b/libgcc/config.host
index 18cabaf..bd4ef1e 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -108,6 +108,9 @@ cr16-*-*)
 crisv32-*-*)
 	cpu_type=cris
 	;;
+csky*-*-*)
+	cpu_type=csky
+	;;
 fido-*-*)
 	cpu_type=m68k
 	;;
@@ -507,6 +510,15 @@ cris-*-elf)
 cris-*-linux* | crisv32-*-linux*)
 	tmake_file="$tmake_file cris/t-cris t-softfp-sfdf t-softfp cris/t-linux"
 	;;
+csky-*-elf*)
+	tmake_file="csky/t-csky t-fdpbit"
+	extra_parts="$extra_parts crti.o crtn.o"
+	;;
+csky-*-linux*)
+	tmake_file="$tmake_file csky/t-csky t-slibgcc-libgcc t-fdpbit csky/t-linux-csky"
+	extra_parts="$extra_parts crti.o crtn.o"
+	md_unwind_header=csky/linux-unwind.h
+	;;
 epiphany-*-elf* | epiphany-*-rtems*)
 	tmake_file="$tmake_file epiphany/t-epiphany t-fdpbit epiphany/t-custom-eqsf"
 	extra_parts="$extra_parts crti.o crtint.o crtrunc.o crtm1reg-r43.o crtm1reg-r63.o crtn.o"
diff --git a/libgcc/config/csky/crti.S b/libgcc/config/csky/crti.S
new file mode 100644
index 000..3e4beb9
--- /dev/null
+++ b/libgcc/config/csky/crti.S
@@ -0,0 +1,140 @@
+# Define _init and _fini entry points for C-SKY.
+# Copyright (C) 2018 Free Software Foundation, Inc.
+# Contributed by C-SKY Microsystems and Mentor Graphics.
+#
+# This file is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 3, or (at your option) any
+# later version.
+#
+# This file is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# Under Section 7 of GPL version 3, you are granted additional
+# permissions described in the GCC Runtime Library Exception, version
+# 3.1, as published by the Free Software Foundation.
+#
+# You should have received a copy of the GNU General Public License and
+# a copy of the GCC Runtime Library Exception along with this program;
+# see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+# .
+
+
+# This file just makes a stack frame for the contents of the .fini and
+# .init sections.  Users may put any desired instructions in those
+# sections.
+
+	.file"crti.S"
+
+/* We use more complicated versions of this code with GLIBC.  */
+#if defined(__gnu_linux__)
+
+#ifndef PREINIT_FUNCTION
+# define PREINIT_FUNCTION __gmon_start__
+#endif
+
+#ifndef PREINIT_FUNCTION_WEAK
+# define PREINIT_FUNCTION_WEAK 1
+#endif
+
+#if PREINIT_FUNCTION_WEAK
+	.global PREINIT_FUNCTION
+	.weak PREINIT_FUNCTION
+	.align 4
+	.type call_weak_fn, %function
+call_weak_fn:
+	// push  lr
+	subisp, 4
+	stw lr, (sp)
+#ifdef  __PIC__
+	lrw a2, PREINIT_FUNCTION@GOT
+	addua2, gb
+	ldw a2, (a2)
+#else
+	lrw a2, PREINIT_FUNCTION
+#endif
+	cmpnei  a2, 0
+	bf  1f
+	jsr a2
+1:
+	// pop lr
+	ldw lr, (sp)
+	addisp, 4
+	rts
+
+	.align 4
+#else
+	.hidden PREINIT_FUNCTION
+#endif /* PREINIT_FUNCTION_WEAK */
+
+	.section .init,"ax",@progbits
+	.align 4
+	.globl _init
+	.type _init, @function
+_init:
+	subisp, 8
+	stw lr, (sp, 0)
+#ifdef __PIC__
+	//  stw gb, (sp, 4)
+	bsr .Lgetpc
+.Lgetpc:
+	lrw gb, .Lgetpc@GOTPC
+	add gb, lr
+#endif
+#if PREINIT_FUNCTION_WEAK
+#ifdef __PIC__
+	lrw a2, call_weak_fn@GOTOFF
+	add a2, gb
+	jsr a2
+#else
+	jsricall_weak_fn
+#endif
+#else /* !PREINIT_FUNCTION_WEAK */
+#ifdef  __PIC__
+	lrw a2, PREINIT_FUNCTION@PLT
+	addua2, gb
+	ldw a2, (a2)
+	jsr a2
+#else
+	jsriPREINIT_FUNCTION
+#endif
+#endif /* PREINIT_FUNCTION_WEAK */
+
+	br  2f
+	.literals
+	.align  4
+2:
+	.section .fini,"ax",@progbits
+	.align 4
+	.globl _fini
+	.type _fini, @function
+_fini:
+	subisp,8
+	stw lr, (sp, 0)
+	br  2f
+	.literals
+	.align  4
+2:
+
+/* These are the non-GLIBC versions.  */
+#else  /* !defined(__gnu_linux__) */
+	.section  ".init"
+	.global  _init
+	.type  _init,@function
+	.align  2
+_init:
+	subi  sp, 16
+	st.w  lr, (sp, 12)
+	mov r0, r0
+
+	.section  ".fini"
+	.global  _fini
+	.type  _fini,@function
+	.align  2
+_fini:
+	subi  sp, 16
+	st.w  lr, (sp, 12)
+	mov r0, r0
+#endif /* defined(__gnu_linux__) */
diff --git a/libgcc/config/csky/crtn.S b/libgcc/config/csky/crtn.S
new file mode 100644
index 000..8bef996
--- /dev/null
+++ b/libgcc/config/csky/crtn.S
@@ -0,0 +1,55 @@
+# Terminate C-SKY .init and .fini sections.
+# Copyright (C) 2018 Free Software Foundation, Inc.
+# Contributed by C-SKY Microsystems and Mentor Graphics.
+#
+# This file is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation