[patch, nios2] enable FRAME_GROWS_DOWNWARD

[Sandra Loosemore]

I discovered that the stack-smashing protection options 
(-fstack-protector and friends) were rejected on nios2 because this 
backend wasn't defining FRAME_GROWS_DOWNWARD.  There doesn't seem to be 
any particular reason not to enable that, so here I've switched it on.


Committed after regression testing on nios2-linux-gnu.  I've also got 
this working for nios2-elf on a local branch in conjunction with BSP 
library support and this patch


https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00537.html

which is still awaiting review.

-Sandra

2017-10-28  Sandra Loosemore  

	gcc/
	* config/nios2/nios2.h (FRAME_GROWS_DOWNWARD): Define to 1.
	* config/nios2/nios2.c (nios2_initial_elimination_offset):  Make
	FRAME_POINTER_REGNUM point at high end of local var area.
diff --git a/gcc/config/nios2/nios2.h b/gcc/config/nios2/nios2.h
index 420543e..9fdff02 100644
--- a/gcc/config/nios2/nios2.h
+++ b/gcc/config/nios2/nios2.h
@@ -252,6 +252,7 @@ enum reg_class
 
 /* Stack layout.  */
 #define STACK_GROWS_DOWNWARD 1
+#define FRAME_GROWS_DOWNWARD 1
 #define FIRST_PARM_OFFSET(FUNDECL) 0
 
 /* Before the prologue, RA lives in r31.  */
diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index cdd5e9a..5c70de7 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -1114,7 +1114,9 @@ nios2_initial_elimination_offset (int from, int to)
   switch (from)
 {
 case FRAME_POINTER_REGNUM:
-  offset = cfun->machine->args_size;
+  /* This is the high end of the local variable storage, not the
+	 hard frame pointer.  */
+  offset = cfun->machine->args_size + cfun->machine->var_size;
   break;
 
 case ARG_POINTER_REGNUM:

[v3 PATCH] Implement LWG 2485

[Ville Voutilainen]

2017-10-29  Ville Voutilainen  

Implement LWG 2485

* include/debug/array (get(const array<_Tp, _Nm>&&)): New.
* include/std/array (get(const array<_Tp, _Nm>&&)): Likewise.
* include/std/tuple (get(const tuple<_Elements...>&&)): Likewise.
(get(const tuple<_Types...>&&)): Likewise.
* include/std/utility
(__pair_get::__const_move_get(const std::pair<_Tp1, _Tp2>&&)):
Likewise.
(get(const std::pair<_Tp1, _Tp2>&&)): Likewise.
(get(const pair<_Tp, _Up>&&)): Likewise.
(get(const pair<_Up, _Tp>&&)): Likewise.
* testsuite/20_util/pair/astuple/get.cc: Add tests for
new overloads.
* testsuite/20_util/pair/astuple/get_by_type.cc: Likewise.
* testsuite/20_util/tuple/element_access/get2.cc: Likewise.
* testsuite/20_util/tuple/element_access/get2_by_type.cc: Likewise.
* testsuite/23_containers/array/tuple_interface/get.cc: Likewise.
* testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc:
Adjust.
* testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc:
Likewise.


lwg2485.diff.bz2
Description: BZip2 compressed data

[patch, fortran] KIND arguments for MINLOC and MAXLOC

[Thomas Koenig]

Hello world,

the attached patch allows KIND arguments to MINLOC and MAXLOC.
There was a bit of a choice to make here. Originally, I wanted to
run the calculation using index_type only and convert to another
integer kind if that was required. This ran into the issue that
bounds checking fails for this approach if there is a conversion
( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82660 ), and I
got regressions for that.

On the other hand, I wanted to avoid adding kind=1 and kind=2 versions
to the library. This approach had been rejected some time ago,
in 2009.

So, I chose a third path by using only pre-existing library
functions for kind=4, kind=8 and kind=16 and by doing a conversion
if the user specified kind=1 or kind=2.

This introduces a bug (array bounds violation not caught) if the user

- specifies bounds checking
- choses kind=1 or kind=2 for minloc or maxloc (it escapes me why
  anybody would want to do that)
- uses an array as return value whose bounds cannot be determined
  at compile-time, and gets the dimension of that array wrong

Frankly, if anybody would do this, the expression "deserves to lose"
comes to mind.

This would not be a regression, because kind=1 and kind=2 are
not supported at the moment.  This bug would be fixed together
with 82660.

Regression-tested. OK for trunk?

Regards

Thomas

2017-10-28  Thomas Koenig  

PR fortran/29600
* gfortran.h (gfc_check_f): Replace fm3l with fm4l.
* intrinsic.h (gfc_resolve_maxloc): Add gfc_expr * to argument
list in protoytpe.
(gfc_resolve_minloc): Likewise.
* check.c (gfc_check_minloc_maxloc): Handle kind argument.
* intrinsic.c (add_sym_3_ml): Rename to
(add_sym_4_ml): and handle kind argument.
(add_function): Replace add_sym_3ml with add_sym_4ml and add
extra arguments for maxloc and minloc.
(check_specific): Change use of check.f3ml with check.f4ml.
* iresolve.c (gfc_resolve_maxloc): Handle kind argument. If
the kind is smaller than the smallest library version available,
use gfc_default_integer_kind and convert afterwards.
(gfc_resolve_minloc): Likewise.

2017-10-28  Thomas Koenig  

PR fortran/29600
* gfortran.dg/minmaxloc_8.f90: New test.
Index: gfortran.h
===
--- gfortran.h	(Revision 253768)
+++ gfortran.h	(Arbeitskopie)
@@ -1989,7 +1989,7 @@ gfc_intrinsic_arg;
argument lists of intrinsic functions. fX with X an integer refer
to check functions of intrinsics with X arguments. f1m is used for
the MAX and MIN intrinsics which can have an arbitrary number of
-   arguments, f3ml is used for the MINLOC and MAXLOC intrinsics as
+   arguments, f4ml is used for the MINLOC and MAXLOC intrinsics as
these have special semantics.  */
 
 typedef union
@@ -1999,7 +1999,7 @@ typedef union
   bool (*f1m)(gfc_actual_arglist *);
   bool (*f2)(struct gfc_expr *, struct gfc_expr *);
   bool (*f3)(struct gfc_expr *, struct gfc_expr *, struct gfc_expr *);
-  bool (*f3ml)(gfc_actual_arglist *);
+  bool (*f4ml)(gfc_actual_arglist *);
   bool (*f3red)(gfc_actual_arglist *);
   bool (*f4)(struct gfc_expr *, struct gfc_expr *, struct gfc_expr *,
 	struct gfc_expr *);
Index: intrinsic.h
===
--- intrinsic.h	(Revision 253768)
+++ intrinsic.h	(Arbeitskopie)
@@ -537,7 +537,7 @@ void gfc_resolve_logical (gfc_expr *, gfc_expr *,
 void gfc_resolve_lstat (gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_matmul (gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_max (gfc_expr *, gfc_actual_arglist *);
-void gfc_resolve_maxloc (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
+void gfc_resolve_maxloc (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_maxval (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_mclock (gfc_expr *);
 void gfc_resolve_mclock8 (gfc_expr *);
@@ -545,7 +545,7 @@ void gfc_resolve_mask (gfc_expr *, gfc_expr *, gfc
 void gfc_resolve_merge (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_merge_bits (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_min (gfc_expr *, gfc_actual_arglist *);
-void gfc_resolve_minloc (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
+void gfc_resolve_minloc (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_minval (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_mod (gfc_expr *, gfc_expr *, gfc_expr *);
 void gfc_resolve_modulo (gfc_expr *, gfc_expr *, gfc_expr *);
Index: check.c
===
--- check.c	(Revision 253768)
+++ check.c	(Arbeitskopie)
@@ -3179,7 +3179,7 @@ gfc_check_matmul (gfc_expr *matrix_a, gfc_expr *ma
 bool
 gfc_check_minloc_maxloc (gfc_actual_arglist *ap)
 {
-  gfc_expr *a, *m, 

Re: Add -std=c17, -std=gnu17

[Richard Biener]

On October 27, 2017 11:56:47 PM GMT+02:00, Joseph Myers 
 wrote:
>C17, a bug-fix version of the C11 standard with DR resolutions
>integrated, will soon go to ballot.  This patch adds corresponding
>options -std=c17, -std=gnu17 (new default version, replacing
>-std=gnu11 as the default), -std=iso9899:2017.  As a bug-fix version
>of the standard, there is no need for flag_isoc17 or any options for
>compatibility warnings; however, there is a new __STDC_VERSION__
>value, so new cpplib languages CLK_GNUC17 and CLK_STDC17 are added to
>support using that new value with the new options.  (If the standard
>ends up being published in 2018 and being known as C18, option aliases
>can be added.  Note however that -std=iso9899:199409 corresponds to a
>__STDC_VERSION__ value rather than a publication date.)
>
>(There are a couple of DR resolutions needing implementing in GCC, but
>that's independent of the new options.)
>
>(I'd propose to add -std=c2x / -std=gnu2x / -Wc11-c2x-compat for the
>next major C standard revision once there are actually C2x drafts
>being issued with new features included.)
>
>Bootstrapped with no regressions for x86_64-pc-linux-gnu.  Are the
>non-front-end changes for the "GNU C17" language name OK?

Yes. 

Richard. 

>gcc:
>2017-10-27  Joseph Myers  
>
>   * doc/invoke.texi (C Dialect Options): Document -std=c17,
>   -std=iso9899:2017 and -std=gnu17.
>   * doc/standards.texi (C Language): Document C17 support.
>   * doc/cpp.texi (Overview): Mention -std=c17.
>   (Standard Predefined Macros): Document C11 and C17 values of
>   __STDC_VERSION__.  Do not refer to C99 support as incomplete.
>   * doc/extend.texi (Inline): Do not list individual options for
>   standards newer than C99.
>   * dwarf2out.c (highest_c_language, gen_compile_unit_die): Handle
>   "GNU C17".
>   * config/rl78/rl78.c (rl78_option_override): Handle "GNU C17"
>   language name.
>
>gcc/c-family:
>2017-10-27  Joseph Myers  
>
>   * c.opt (std=c17, std=gnu17, std=iso9899:2017): New options.
>   * c-opts.c (set_std_c17): New function.
>   (c_common_init_options): Use gnu17 as default C version.
>   (c_common_handle_option): Handle -std=c17 and -std=gnu17.
>
>gcc/testsuite:
>2017-10-27  Joseph Myers  
>
>   * gcc.dg/c17-version-1.c, gcc.dg/c17-version-2.c: New tests.
>
>libcpp:
>2017-10-27  Joseph Myers  
>
>   * include/cpplib.h (enum c_lang): Add CLK_GNUC17 and CLK_STDC17.
>   * init.c (lang_defaults): Add GNUC17 and STDC17 data.
>   (cpp_init_builtins): Handle C17 value of __STDC_VERSION__.
>
>Index: gcc/c-family/c-opts.c
>===
>--- gcc/c-family/c-opts.c  (revision 254145)
>+++ gcc/c-family/c-opts.c  (working copy)
>@@ -115,6 +115,7 @@ static void set_std_cxx2a (int);
> static void set_std_c89 (int, int);
> static void set_std_c99 (int);
> static void set_std_c11 (int);
>+static void set_std_c17 (int);
> static void check_deps_environment_vars (void);
> static void handle_deferred_opts (void);
> static void sanitize_cpp_opts (void);
>@@ -236,8 +237,8 @@ c_common_init_options (unsigned int decoded_option
> 
>   if (c_language == clk_c)
> {
>-  /* The default for C is gnu11.  */
>-  set_std_c11 (false /* ISO */);
>+  /* The default for C is gnu17.  */
>+  set_std_c17 (false /* ISO */);
> 
>  /* If preprocessing assembly language, accept any of the C-family
>front end options since the driver may pass them through.  */
>@@ -675,6 +676,16 @@ c_common_handle_option (size_t scode, const char *
>   set_std_c11 (false /* ISO */);
>   break;
> 
>+case OPT_std_c17:
>+  if (!preprocessing_asm_p)
>+  set_std_c17 (true /* ISO */);
>+  break;
>+
>+case OPT_std_gnu17:
>+  if (!preprocessing_asm_p)
>+  set_std_c17 (false /* ISO */);
>+  break;
>+
> case OPT_trigraphs:
>   cpp_opts->trigraphs = 1;
>   break;
>@@ -1559,6 +1570,21 @@ set_std_c11 (int iso)
>   lang_hooks.name = "GNU C11";
> }
> 
>+/* Set the C 17 standard (without GNU extensions if ISO).  */
>+static void
>+set_std_c17 (int iso)
>+{
>+  cpp_set_lang (parse_in, iso ? CLK_STDC17: CLK_GNUC17);
>+  flag_no_asm = iso;
>+  flag_no_nonansi_builtin = iso;
>+  flag_iso = iso;
>+  flag_isoc11 = 1;
>+  flag_isoc99 = 1;
>+  flag_isoc94 = 1;
>+  lang_hooks.name = "GNU C17";
>+}
>+
>+
> /* Set the C++ 98 standard (without GNU extensions if ISO).  */
> static void
> set_std_cxx98 (int iso)
>Index: gcc/c-family/c.opt
>===
>--- gcc/c-family/c.opt (revision 254145)
>+++ gcc/c-family/c.opt (working copy)
>@@ -1944,6 +1944,10 @@ std=c1x
> C ObjC Alias(std=c11)
> Deprecated in favor of -std=c11.
> 
>+std=c17
>+C ObjC
>+Conform to the ISO 2017 C standard.
>+
> std=c89
> C ObjC 

Re: [patch, fortran, RFC] Interchange indices for FORALL and DO CONCURRENT if profitable

[Richard Biener]

On October 28, 2017 12:03:58 AM GMT+02:00, Thomas Koenig 
 wrote:
>Hello world,
>
>this is a draft patch which interchanges the indices for FORALL and
>DO CONCURRENT loops for cases like PR 82471, where code like
>
>   DO CONCURRENT( K=1:N, J=1:M, I=1:L)
>  C(I,J,K) = A(I,J,K) + B(I,J,K)
>   END DO
>
>led to very poor code because of stride issues.  Currently,
>Graphite is not able to do this.
>
>Without the patch, the code above is translated as
>
>
> i.7 = 1;
> count.10 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.4;
> j.6 = 1;
> count.9 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.3;
> k.5 = 1;
> count.8 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.2;
>(*(real(kind=4)[0:] * restrict) c.data)[((c.offset 
>+ (integer(kind=8)) k.5 * c.dim[2].stride) + (integer(kind=8)) j.6 * 
>c.dim[1].stride) + (integer(kind=8)) i.7] = (*(real(kind=4)[0:] * 
>restrict) a.data)[((a.offset + (integer(kind=8)) k.5 * a.dim[2].stride)
>
>+ (integer(kind=8)) j.6 * a.dim[1].stride) + (integer(kind=8)) i.7] + 
>(*(real(kind=4)[0:] * restrict) b.data)[((b.offset + (integer(kind=8)) 
>k.5 * b.dim[2].stride) + (integer(kind=8)) j.6 * b.dim[1].stride) + 
>(integer(kind=8)) i.7];
> L.1:;
> k.5 = k.5 + 1;
> count.8 = count.8 + -1;
>   }
> L.2:;
> j.6 = j.6 + 1;
> count.9 = count.9 + -1;
>   }
> L.3:;
> i.7 = i.7 + 1;
> count.10 = count.10 + -1;
>   }
> L.4:;
>
>so the innermost loop has the biggest stride. With the patch
>(and front-end optimization turned on), this is turned into
>
> k.7 = 1;
> count.10 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.4;
> j.6 = 1;
> count.9 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.3;
> i.5 = 1;
> count.8 = 512;
> while (1)
>   {
> if (ANNOTATE_EXPR ) goto L.2;
>(*(real(kind=4)[0:] * restrict) c.data)[((c.offset 
>+ (integer(kind=8)) k.7 * c.dim[2].stride) + (integer(kind=8)) j.6 * 
>c.dim[1].stride) + (integer(kind=8)) i.5] = (*(real(kind=4)[0:] * 
>restrict) a.data)[((a.offset + (integer(kind=8)) k.7 * a.dim[2].stride)
>
>+ (integer(kind=8)) j.6 * a.dim[1].stride) + (integer(kind=8)) i.5] + 
>(*(real(kind=4)[0:] * restrict) b.data)[((b.offset + (integer(kind=8)) 
>k.7 * b.dim[2].stride) + (integer(kind=8)) j.6 * b.dim[1].stride) + 
>(integer(kind=8)) i.5];
> L.1:;
> i.5 = i.5 + 1;
> count.8 = count.8 + -1;
>   }
> L.2:;
> j.6 = j.6 + 1;
> count.9 = count.9 + -1;
>   }
> L.3:;
> k.7 = k.7 + 1;
> count.10 = count.10 + -1;
>   }
> L.4:;
>
>so the innermost loop is the one that gets traversed with unity stride
>(the way it should have been done).
>
>Although the algorithm here is quite simple, it resolves the issues
>raised in the PR so far, and it definitely worth fixing.
>
>If we do this kind of thing, it might also be possible to
>interchange normal DO loops in a similar way (which Graphite also
>cannot do at the moment, at least not if the bounds are variables).
>
>So, comments? Suggestions? Other ideas? Any ideas how to write
>a test case? Any volunteers to re-implement Graphite in the
>Fortran front end (didn't think so) or to make Graphite catch
>this sort of pattern (which it currently doesn't) instead?

Graphite has major issues with the scalarization of multidimensional arrays to 
a single array dimension. Indeed doing graphite optimization on the Fortran FE 
IL would be an interesting experiment.  Maybe list that as summer of code 
project idea. 

Richard. 

>Regards
>
>   Thomas
>
>2017-10-27  Thomas Koenig  
>
> * frontend-passes.c (index_interchange): New funciton,
> prototype.
> (optimize_namespace): Call index_interchange.
> (ind_type): New function.
> (has_var): New function.
> (index_cost): New function.
> (loop_comp): New function.

Re: [RFC PATCH] Fix pointer diff (was: -fsanitize=pointer-overflow support (PR sanitizer/80998))

[Richard Biener]

On October 28, 2017 2:53:56 PM GMT+02:00, Marc Glisse  
wrote:
>
>I am sending the new version of the patch in a separate email, to make
>it 
>more visible, and only replying to a few points here.
>
>On Thu, 19 Oct 2017, Richard Biener wrote:
>
>> On Mon, Oct 9, 2017 at 12:55 PM, Marc Glisse 
>wrote:
>>> On Mon, 3 Jul 2017, Richard Biener wrote:
>>>
 On Sat, 1 Jul 2017, Marc Glisse wrote:

> I wrote a quick prototype to see what the fallout would look like.
> Surprisingly, it actually passes bootstrap+testsuite on ppc64el
>with all
> languages with no regression. Sure, it is probably not a complete
> migration, there are likely a few places still converting to
>ptrdiff_t
> to perform a regular subtraction, but this seems to indicate that
>the
> work isn't as bad as using a signed type in pointer_plus_expr for
> instance.


 The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
 from POINTER_MINUS_EXPR in some cases I guess).

 The tree code needs documenting in tree.def and generic.texi.

 Otherwise ok(*).

 Thanks,
 Richard.

 (*) ok, just kidding -- or maybe not
>>>
>>>
>>> I updated the prototype a bit. Some things I noticed:
>>>
>>> - the C front-end has support for address spaces that seems to imply
>that
>>> pointer subtraction (and division by the size) may be done in a type
>larger
>>> than ptrdiff_t. Currently, it generates
>>> (ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is
>some type
>>> potentially larger than ptrdiff_t.
>>
>> It uses a ptrdiff_t corresponding to the respective address space,
>yes.
>> That we use sizetype elsewhere unconditionally is a bug :/
>>
>>> I am thus generating a POINTER_DIFF_EXPR
>>> with that type, while I was originally hoping its type would always
>be
>>> ptrdiff_t. It complicates code and I am not sure I didn't break
>address
>>> spaces anyway... (expansion likely needs to do the inttype dance)
>>
>> I think that's fine.  Ideally targets would provide a type to use for
>each
>> respective address space given we have targets that have sizetype
>smaller
>> than ptr_mode.
>>
>>> Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t
>(what
>>> POINTER_PLUS_EXPR takes as second argument) always the same type
>>> signed/unsigned?
>>
>> POINTER_PLUS_EXPR takes 'sizetype', not size_t.
>
>Ah, yes, that's the one I meant...
>
>> So the answer is clearly no.  And yes, that's ugly and broken and
>should be fixed.
>>
>>> Counter-examples: m32c (when !TARGET_A16), visium, darwin,
>>> powerpcspe, s390, vms... and it isn't even always the same that is
>bigger
>>> than the other. That's quite messy.
>>
>> Eh.  Yeah, targets are free to choose 'sizetype' and they do so for
>> efficiency.  IMHO we should get rid of this "feature".
>>
>>> - I had to use @@ in match.pd, not for constants, but because in
>GENERIC we
>>> sometimes ended up with pointers where operand_equal_p said yes but
>>> types_match disagreed.
>>>
>>> - It currently regresses 2 go tests: net/http runtime/debug
>>
>> Those are flaky for me and fail sometimes and sometimes not.
>
>Yes, I noticed that (there are 1 or 2 others in the go testsuite).
>
>> +@item POINTER_DIFF_EXPR
>> +This node represents pointer subtraction.  The two operands always
>> +have pointer/reference type.  The second operand is always an
>unsigned
>> +integer type compatible with sizetype.  It returns a signed integer.
>>
>> the 2nd sentence looks bogusly copied.
>
>Oups, removed.
>
>>
>> +  /* FIXME.  */
>> +  if (code == POINTER_DIFF_EXPR)
>> +   return int_const_binop (MINUS_EXPR,
>> +   fold_convert (ptrdiff_type_node,
>arg1),
>> +   fold_convert (ptrdiff_type_node,
>arg2));
>>
>>  wide_int_to_tree (type, wi::to_widest (arg1) - wi::to_widest
>(arg2));
>
>Before your reply, I wrote something similar using offset_int instead
>of 
>widest_int (and handling overflow, mostly for documentation purposes).
>I 
>wasn't sure which one to pick, I can change to widest_int if you think
>it 
>is better...

Offset_int is better. 

>> +case POINTER_DIFF_EXPR:
>> +  {
>> +   if (!POINTER_TYPE_P (rhs1_type)
>> +   || !POINTER_TYPE_P (rhs2_type)
>> +   // || !useless_type_conversion_p (rhs2_type, rhs1_type)
>>
>> types_compatible_p (rhs1_type, rhs2_type)?
>
>Makes sense.
>
>> +   // || !useless_type_conversion_p (ptrdiff_type_node,
>lhs_type))
>> +   || TREE_CODE (lhs_type) != INTEGER_TYPE
>> +   || TYPE_UNSIGNED (lhs_type))
>> + {
>> +   error ("type mismatch in pointer diff expression");
>> +   debug_generic_stmt (lhs_type);
>> +   debug_generic_stmt (rhs1_type);
>> +   debug_generic_stmt (rhs2_type);
>> +   return true;
>>
>> there's also verify_expr which could want adjustment for newly
>created
>> tree kinds.
>
>ok.
>

Re: [PATCH, Fortran, v1] Clarify error message of co_reduce

[Andre Vehreschild]

Hi all,

I got no direct objections therefore committed as r254197 to trunk and r254198
to gcc-7. 

@Steve, I will take a look at what you pointed. 

- Andre

On Fri, 27 Oct 2017 12:19:02 +0200
Andre Vehreschild  wrote:

> Hi all,
> 
> as noted on IRC is one of the error message in check.c co_reduce misleading.
> The attached patch fixes this. The intention of the error message is to tell
> that the type of the argument bound to parameter A is of wrong type and not
> that an unspecific argument has a wrong type.
> 
> If no one objects within 24 h I am planning to commit this patch as obvious to
> trunk gcc-7. Bootstrapped and regtested on x86_64-linux-gnu/f25.
> 
> Regards,
>   Andre


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog	(Revision 254196)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,7 @@
+2017-10-28  Andre Vehreschild  
+
+	* check.c (gfc_check_co_reduce): Clarify error message.
+
 2017-10-28  Paul Thomas  
 
 	PR fortran/81758
Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c	(Revision 254196)
+++ gcc/fortran/check.c	(Arbeitskopie)
@@ -1731,7 +1731,7 @@
 
   if (!gfc_compare_types (>ts, >result->ts))
 {
-  gfc_error ("A argument at %L has type %s but the function passed as "
+  gfc_error ("The A argument at %L has type %s but the function passed as "
 		 "OPERATOR at %L returns %s",
 		 >where, gfc_typename (>ts), >where,
 		 gfc_typename (>result->ts));
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog	(Revision 254197)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,7 @@
+2017-10-28  Andre Vehreschild  
+
+	* check.c (gfc_check_co_reduce): Clarify error message.
+
 2017-10-28  Paul Thomas  
 
 	Backported from trunk
Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c	(Revision 254197)
+++ gcc/fortran/check.c	(Arbeitskopie)
@@ -1731,7 +1731,7 @@
 
   if (!gfc_compare_types (>ts, >result->ts))
 {
-  gfc_error ("A argument at %L has type %s but the function passed as "
+  gfc_error ("The A argument at %L has type %s but the function passed as "
 		 "OPERATOR at %L returns %s",
 		 >where, gfc_typename (>ts), >where,
 		 gfc_typename (>result->ts));

[RFTesting] New POINTER_DIFF_EXPR

[Marc Glisse]

Hello,

first, if you are doing anything unusual with pointers (address spaces, 
pointer/sizetype with weird sizes, instrumentation, etc), it would be 
great if you could give this patch a try. It was bootstrapped and 
regtested on powerpc64le-unknown-linux-gnu (gcc112), and a slightly older 
version on x86_64-pc-linux-gnu (skylake laptop). I also built bare 
cross-compilers (no sysroot or anything) for avr, m32c, alpha64-vms, 
s390-linux, and visium to check that on trivial examples it behaves as 
expected (by the way, m32c seems broken for unrelated reasons at the 
moment), but I wouldn't count that as complete testing.


This was previously discussed in the thread "Fix pointer diff (was: 
-fsanitize=pointer-overflow support (PR sanitizer/80998))" ( 
https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02128.html for the latest 
message).


Front-ends other than C/C++ can be changed later (I took a quick look at 
fortran and ada, but they are way too unfamiliar to me), I did not remove 
any handling for the other representations.


2017-10-28  Marc Glisse  

gcc/c/
* c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR.
* c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR.

gcc/c-family/
* c-pretty-print.c (pp_c_additive_expression,
c_pretty_printer::expression): Handle POINTER_DIFF_EXPR.

gcc/cp/
* constexpr.c (cxx_eval_constant_expression,
potential_constant_expression_1): Handle POINTER_DIFF_EXPR.
* cp-gimplify.c (cp_fold): Likewise.
* error.c (dump_expr): Likewise.
* typeck.c (cp_build_binary_op): Likewise.
(pointer_diff): Use POINTER_DIFF_EXPR.

gcc/
* doc/generic.texi: Document POINTER_DIFF_EXPR, update
POINTER_PLUS_EXPR.
* cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR.
* expr.c (expand_expr_real_2): Likewise.
* fold-const.c (const_binop, const_binop,
fold_addr_of_array_ref_difference, fold_binary_loc): Likewise.
* match.pd (X-X, P+(Q-P), , (P+N)-P, P-(P+N), (P+M)-(P+N),
P-Q==0): New transformations for POINTER_DIFF_EXPR, based on
MINUS_EXPR transformations.
* optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR.
* tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise.
* tree-inline.c (estimate_operator_cost): Likewise.
* tree-pretty-print.c (dump_generic_node, op_code_prio,
op_symbol_code): Likewise.
* tree-vect-stmts.c (vectorizable_operation): Likewise.
* tree-vrp.c (extract_range_from_binary_expr): Likewise.
* varasm.c (initializer_constant_valid_p_1): Likewise.
* tree.def: New tree code POINTER_DIFF_EXPR.

--
Marc GlisseIndex: gcc/c/c-fold.c
===
--- gcc/c/c-fold.c	(revision 254183)
+++ gcc/c/c-fold.c	(working copy)
@@ -238,20 +238,21 @@ c_fully_fold_internal (tree expr, bool i
 case COMPOUND_EXPR:
 case MODIFY_EXPR:
 case PREDECREMENT_EXPR:
 case PREINCREMENT_EXPR:
 case POSTDECREMENT_EXPR:
 case POSTINCREMENT_EXPR:
 case PLUS_EXPR:
 case MINUS_EXPR:
 case MULT_EXPR:
 case POINTER_PLUS_EXPR:
+case POINTER_DIFF_EXPR:
 case TRUNC_DIV_EXPR:
 case CEIL_DIV_EXPR:
 case FLOOR_DIV_EXPR:
 case TRUNC_MOD_EXPR:
 case RDIV_EXPR:
 case EXACT_DIV_EXPR:
 case LSHIFT_EXPR:
 case RSHIFT_EXPR:
 case BIT_IOR_EXPR:
 case BIT_XOR_EXPR:
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(revision 254183)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3772,21 +3772,21 @@ parser_build_binary_op (location_t locat
   && TREE_CODE (type2) == ENUMERAL_TYPE
   && TYPE_MAIN_VARIANT (type1) != TYPE_MAIN_VARIANT (type2))
 warning_at (location, OPT_Wenum_compare,
 		"comparison between %qT and %qT",
 		type1, type2);
 
   return result;
 }
 
 /* Return a tree for the difference of pointers OP0 and OP1.
-   The resulting tree has type int.  */
+   The resulting tree has type ptrdiff_t.  */
 
 static tree
 pointer_diff (location_t loc, tree op0, tree op1)
 {
   tree restype = ptrdiff_type_node;
   tree result, inttype;
 
   addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op0)));
   addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op1)));
   tree target_type = TREE_TYPE (TREE_TYPE (op0));
@@ -3804,43 +3804,50 @@ pointer_diff (location_t loc, tree op0,
 	 to exist because the caller verified that comp_target_types
 	 returned non-zero.  */
   if (!addr_space_superset (as0, as1, _common))
 	gcc_unreachable ();
 
   common_type = common_pointer_type (TREE_TYPE (op0), TREE_TYPE (op1));
   op0 = convert (common_type, op0);
   op1 = convert (common_type, op1);
 }
 
-  /* Determine integer type to perform computations in.  This will usually
+  /* Determine integer type result of the subtraction.  This will usually

Re: [RFC PATCH] Fix pointer diff (was: -fsanitize=pointer-overflow support (PR sanitizer/80998))

[Marc Glisse]

I am sending the new version of the patch in a separate email, to make it 
more visible, and only replying to a few points here.


On Thu, 19 Oct 2017, Richard Biener wrote:


On Mon, Oct 9, 2017 at 12:55 PM, Marc Glisse  wrote:

On Mon, 3 Jul 2017, Richard Biener wrote:


On Sat, 1 Jul 2017, Marc Glisse wrote:


I wrote a quick prototype to see what the fallout would look like.
Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
languages with no regression. Sure, it is probably not a complete
migration, there are likely a few places still converting to ptrdiff_t
to perform a regular subtraction, but this seems to indicate that the
work isn't as bad as using a signed type in pointer_plus_expr for
instance.



The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
from POINTER_MINUS_EXPR in some cases I guess).

The tree code needs documenting in tree.def and generic.texi.

Otherwise ok(*).

Thanks,
Richard.

(*) ok, just kidding -- or maybe not



I updated the prototype a bit. Some things I noticed:

- the C front-end has support for address spaces that seems to imply that
pointer subtraction (and division by the size) may be done in a type larger
than ptrdiff_t. Currently, it generates
(ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is some type
potentially larger than ptrdiff_t.


It uses a ptrdiff_t corresponding to the respective address space, yes.
That we use sizetype elsewhere unconditionally is a bug :/


I am thus generating a POINTER_DIFF_EXPR
with that type, while I was originally hoping its type would always be
ptrdiff_t. It complicates code and I am not sure I didn't break address
spaces anyway... (expansion likely needs to do the inttype dance)


I think that's fine.  Ideally targets would provide a type to use for each
respective address space given we have targets that have sizetype smaller
than ptr_mode.


Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t (what
POINTER_PLUS_EXPR takes as second argument) always the same type
signed/unsigned?


POINTER_PLUS_EXPR takes 'sizetype', not size_t.


Ah, yes, that's the one I meant...


So the answer is clearly no.  And yes, that's ugly and broken and should be 
fixed.


Counter-examples: m32c (when !TARGET_A16), visium, darwin,
powerpcspe, s390, vms... and it isn't even always the same that is bigger
than the other. That's quite messy.


Eh.  Yeah, targets are free to choose 'sizetype' and they do so for
efficiency.  IMHO we should get rid of this "feature".


- I had to use @@ in match.pd, not for constants, but because in GENERIC we
sometimes ended up with pointers where operand_equal_p said yes but
types_match disagreed.

- It currently regresses 2 go tests: net/http runtime/debug


Those are flaky for me and fail sometimes and sometimes not.


Yes, I noticed that (there are 1 or 2 others in the go testsuite).


+@item POINTER_DIFF_EXPR
+This node represents pointer subtraction.  The two operands always
+have pointer/reference type.  The second operand is always an unsigned
+integer type compatible with sizetype.  It returns a signed integer.

the 2nd sentence looks bogusly copied.


Oups, removed.



+  /* FIXME.  */
+  if (code == POINTER_DIFF_EXPR)
+   return int_const_binop (MINUS_EXPR,
+   fold_convert (ptrdiff_type_node, arg1),
+   fold_convert (ptrdiff_type_node, arg2));

 wide_int_to_tree (type, wi::to_widest (arg1) - wi::to_widest (arg2));


Before your reply, I wrote something similar using offset_int instead of 
widest_int (and handling overflow, mostly for documentation purposes). I 
wasn't sure which one to pick, I can change to widest_int if you think it 
is better...



+case POINTER_DIFF_EXPR:
+  {
+   if (!POINTER_TYPE_P (rhs1_type)
+   || !POINTER_TYPE_P (rhs2_type)
+   // || !useless_type_conversion_p (rhs2_type, rhs1_type)

types_compatible_p (rhs1_type, rhs2_type)?


Makes sense.


+   // || !useless_type_conversion_p (ptrdiff_type_node, lhs_type))
+   || TREE_CODE (lhs_type) != INTEGER_TYPE
+   || TYPE_UNSIGNED (lhs_type))
+ {
+   error ("type mismatch in pointer diff expression");
+   debug_generic_stmt (lhs_type);
+   debug_generic_stmt (rhs1_type);
+   debug_generic_stmt (rhs2_type);
+   return true;

there's also verify_expr which could want adjustment for newly created
tree kinds.


ok.

So if the precision of the result is larger than that of the pointer 
operands we sign-extend the result, right?  So the subtraction is 
performed in precision of the pointer operands and then 
sign-extended/truncated to the result type? Which means it is _not_ a 
widening subtraction to get around the half-address-space issue.  The 
tree.def documentation should reflect this semantic detail.  Not sure if 
the RTL expansion follows it.


I actually have no idea what expansion 

Re: [patch, fortran, RFC] Interchange indices for FORALL and DO CONCURRENT if profitable

[Thomas Koenig]

Hi Steve,


On Sat, Oct 28, 2017 at 12:03:58AM +0200, Thomas Koenig wrote:

+/* Callback function to determine if an expression is the
+   corresponding variable.  */
+
+static int

static bool


Most of the functions in the patch are callback functions for
gfc_code_walker or gfc_expr_walker, respectively. Their
function arguments are given as

typedef int (*walk_code_fn_t) (gfc_code **, int *, void *);
typedef int (*walk_expr_fn_t) (gfc_expr **, int *, void *);

respectively, so the types of the functions are fixed.

Regards

Thomas