Re: [PATCH] Improve avx512{f,bw} vec_set (PR middle-end/85090)

[Kirill Yukhin]

> On 31 Mar 2018, at 01:50, Jakub Jelinek  wrote:
> Hi!
> 
> The code we emit on the following testcases is really terrible, with both
> -mavx512f -mno-avx512bw as well as -mavx512bw, rather than doing e.g.
>vpinsrw $0, %edi, %xmm0, %xmm1
>vinserti32x4$0, %xmm1, %zmm0, %zmm0
> when trying to insert into low 128-bits or
>vextracti32x4   $1, %zmm0, %xmm1
>vpinsrw $3, %edi, %xmm1, %xmm1
>vinserti32x4$1, %xmm1, %zmm0, %zmm0
> when trying to insert into other 128-bits, we emit:
>pushq   %rbp
>vmovq   %xmm0, %rax
>movzwl  %di, %edi
>xorw%ax, %ax
>movq%rsp, %rbp
>orq %rdi, %rax
>andq$-64, %rsp
>vmovdqa64   %zmm0, -64(%rsp)
>movq%rax, -64(%rsp)
>vmovdqa64   -64(%rsp), %zmm0
>leave
> and furthermore there is some RA bug it triggers, so we miscompile it.
> All this is because while at least for AVX512BW we have ix86_expand_vector_set
> implemented for V64QImode and V32QImode, we actually don't have a pattern
> for it.  Fixed by adding those modes to V, which is used only by this
> vec_set pattern and some xop pattern, which is misusing it anyway (it really
> can't handle 512-bit vectors, so could result in assembly failures etc.
> with -mxop -mavx512f).  Furthermore, for AVX512F we can use the above
> extraction/insertion/insertion sequence, similarly to what we do for 256-bit
> insertions, just we have halves there rather than quarters.
> The splitters are added to match what vec_set_lo_* define_insn_and_split do,
> if we are trying to extract low 128-bit lane of a 512-bit vector, we really
> don't need vextracti32x4 instruction, we can use simple move or even nothing
> at all (if source and destination are the same register, or post RA register
> renaming can arrange that).
> 
> The RA bug still should be fixed, but at least this patch makes it latent
> and improves a lot the code.  Bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk? 
OK.

—
Thanks, K

Re: [PATCH, committed] Update my MAINTAINERS entries

[Gerald Pfeifer]

On Fri, 30 Mar 2018, Bill Schmidt wrote:
> Just updating my email address and making it a little clearer which 
> is which between Will Schmidt and me.  Committed.

Actually you can (should) remove your entry from Write after Approval
since you are now listed in one of the previous sections already.

Gerald

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

Hi Dominique,


If I am not mistaken, the patch causes:

FAIL: gfortran.dg/pr71935.f90   -O  (test for excess errors)
FAIL: gfortran.dg/substr_6.f90   -O0  execution test
FAIL: gfortran.dg/substr_6.f90   -O1  execution test
FAIL: gfortran.dg/substr_6.f90   -O2  execution test
FAIL: gfortran.dg/substr_6.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/substr_6.f90   -O3 -g  execution test
FAIL: gfortran.dg/substr_6.f90   -Os  execution test


These have been resolved now - I have removed the invalid code
from substr_6.f90 (PR85130), and the error is now suppressed
in the attached patch.

Re-regression-tested. OK for trunk?

Regards

Thomas

2018-03-25  Thomas Koenig  

PR fortran/51260
* resolve.c (resolve_variable): Simplify cases where access to a
parameter array results in a single constant.

2018-03-25  Thomas Koenig  

PR fortran/51260
* gfortran.dg/parameter_array_element_3.f90: New test.
! { dg-do compile }
! PR 51260 - an unneeded parameter found its way into the
! assembly code. Original test case by Tobias Burnus.
module x
contains
  subroutine foo(i)
integer, intent(in) :: i
  end subroutine foo
end module x

program main
  use x
  integer, parameter:: unneeded_parameter (1)=(/(i,i=1,1)/)
  call foo(unneeded_parameter (1))
  print *,unneeded_parameter (1)
end program
! { dg-final { scan-assembler-times "unneeded_parameter" 0 } }
Index: resolve.c
===
--- resolve.c	(Revision 258845)
+++ resolve.c	(Arbeitskopie)
@@ -5577,6 +5577,16 @@ resolve_procedure:
   if (t && flag_coarray == GFC_FCOARRAY_LIB && gfc_is_coindexed (e))
 add_caf_get_intrinsic (e);
 
+  /* Simplify cases where access to a parameter array results in a
+ single constant.  Suppress errors since those will have been
+ issued before, as warnings.  */
+  if (e->rank == 0 && sym->as && sym->attr.flavor == FL_PARAMETER)
+{
+  gfc_push_suppress_errors ();
+  gfc_simplify_expr (e, 1);
+  gfc_pop_suppress_errors ();
+}
+
   return t;
 }
 

Re: [PATCH] [PR c++/85039] no type definitions in builtin offsetof

[Alexandre Oliva]

On Mar 30, 2018, Jason Merrill  wrote:

> On Fri, Mar 30, 2018 at 3:55 AM, Alexandre Oliva  wrote:
>> Types defined within a __builtin_offsetof argument don't always get
>> properly recorded as members of their context types

>> I suppose this means I should look for another solution that doesn't
>> involve rejecting these definitions, eh?

> Hmm, I'm afraid so

Ok, but...  tricky...

If I were to make the anonymous types members of the enclosing class,
their members would be promoted to members of the enclosing class, which
doesn't sound like the right thing to do.

But I wonder, should they even be regarded as members of the enclosing
class, when they do not appear inside the body of the class.  Perhaps
their context should be something else, say the innermost enclosing
namespace, the global namespace, some anonymous namespace introduced for
the offsetof...

One thing that's not clear to me is whether types defined in offsetof
can be referenced outside their own definition.  In C, that would be
natural, since the struct namespace is flat, but in C++, structs belong
to a context, and it's not obvious to me that offsetof should inject
names in an enclosing context, or in a separate invisible namespace.  I
lean towards the latter, but then I tried:

template  class foo { };
int j = __builtin_offsetof(struct a { int i; static foo x = foo(); }, i);

and found (through debug info) that foo is mangled as if struct a was
defined in the global namespace.  And, indeed, a is recorded in the
global namespace:

a b;

which was slightly surprising to me.

But it seems to be consistent: when the offsetof expression is within a
function, the type will be defined within the function; when it is used
in the initializer of a data member, the context is the class containing
the data member, even if the initializer is outside the class body, but
the type is defined in the global namespace:

struct a {
  static int const z, i = __builtin_offsetof(struct b { int j; }, j);
  b c;
};
int const a::z = __builtin_offsetof(struct d { int k; }, k);
d e;
b f; // b is not defined
a::b g; // ok

So, the problem with anon types defined in offsetof appears to be the
inconsistency between the scope in which d is introduced, namely the
global namespace because that's the current scope, and the DECL_CONTEXT,
copied to TYPE_CONTEXT, that is taken as class a because we're parsing
the initializer for a::z.  Since they disagree, we don't find the type
defined in the context named as enclosing it.

Since d is visible, I suppose the most conservative solution would be to
name the global namespace as the context for type d, rather than
defining it as a member of a.  Right?


Now, I really don't want to think of offsetof appearing in default
arguments or array lengths in function prototypes or even in template
parameters.  Types are not supposed to be defined in such contexts,
but...  should offsetof make an exception?  Currently, we reject them,
which is quite a relief, because otherwise it might appear in
expressions in templates and that would be really really hairy ;-)



> Incidentally, it would be nice to replace all the
> type_definition_forbidden stuff with defining-type-specifier as per DR
> 2141... (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50169)

*nod*

Is this in scope for GCC 8?  It's not marked as a regression, but I
guess I could try and tackle it if it's intended to make it anyway,
given that I've got some context on this issue.  LMK about the plans,
and whether you envision any pitfalls.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84979] improve auto handling in explicit tmpl args for concepts

[Alexandre Oliva]

On Mar 30, 2018, Jason Merrill  wrote:

> True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an
> IDENTIFIER_NODE.  Looking at tsubst_copy_and_build, I see that we
> don't call finish_id_expression when substituting such a
> TEMPLATE_ID_EXPR.  So maybe lookup_template_function and
> lookup_template_variable are the right places for this test.

Nevermind the earlier email about multiple errors.  I realized that we
save the preparsed template_id right after the tests in
cp_parser_template_id, and if only I don't stop the rejected template
from being saved, we avoid the duplicate errors, as in the patch below.

A slight variant of this passed regstrap on i686- and x86_64-linux-gnu.
Ok to install, though it does not catch such cases as:

template 
void foo(T t) {
  typename T::template C u = t;
  T::template C (t);
  T::template C::f (t, u);
}

?


[PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

With concepts, we accept auto in explicit template arguments, but we
should only accept them for template classes.  Passing them to
template functions is not allowed.  So, reject it.


for  gcc/cp/ChangeLog

PR c++/84979
* parser.c (cp_parser_check_for_auto_in_templ_arguments): New.
(cp_parser_template_id): Use it to reject template args
referencing auto for non-type templates.

for  gcc/testsuite/ChangeLog

PR c++/84979
* g++.dg/concepts/pr84979.C: New.
---
 gcc/cp/parser.c |   39 +++
 gcc/testsuite/g++.dg/concepts/pr84979.C |9 +++
 2 files changed, 48 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e946d0b72292..c5ba2123ae96 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15690,6 +15690,42 @@ cp_parser_type_parameter (cp_parser* parser, bool 
*is_parameter_pack)
   return parameter;
 }
 
+/* If concepts are enabled and TEMPL does not identify a template
+   class, report occurrences of auto types in ARGUMENTS.  Return TRUE
+   if any such errors were reported.  */
+
+static bool
+cp_parser_check_for_auto_in_templ_arguments (cp_parser *parser 
ATTRIBUTE_UNUSED,
+tree templ,
+tree arguments)
+{
+  if (!flag_concepts)
+return false;
+
+  if (identifier_p (templ)
+  || DECL_TYPE_TEMPLATE_P (templ)
+  || DECL_TEMPLATE_TEMPLATE_PARM_P (templ))
+return false;
+
+  if (!arguments || TREE_CODE (arguments) != TREE_VEC)
+return false;
+
+  bool errors = false;
+
+  for (int i = 0; i < TREE_VEC_LENGTH (arguments); i++)
+{
+  tree xauto = type_uses_auto (TREE_VEC_ELT (arguments, i));
+  if (xauto)
+   {
+ error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)),
+   "invalid use of % in template argument");
+ errors = true;
+   }
+}
+
+  return errors;
+}
+
 /* Parse a template-id.
 
template-id:
@@ -15851,6 +15887,9 @@ cp_parser_template_id (cp_parser *parser,
   template_id
= finish_template_type (templ, arguments, entering_scope);
 }
+  else if (cp_parser_check_for_auto_in_templ_arguments (parser, templ,
+   arguments))
+template_id = error_mark_node;
   /* A template-like identifier may be a partial concept id. */
   else if (flag_concepts
&& (template_id = (cp_parser_maybe_partial_concept_id
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C 
b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index ..c70b3756f2b8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template void foo() {}
+
+void bar()
+{
+  foo(); // { dg-error "invalid|no match" }
+}


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

On Mar 30, 2018, Jason Merrill  wrote:

> I don't think we need this; if arg is overloaded, we take the
> type_unknown_p early exit, so the code lower down is always dealing
> with a single function.

Aah, that's why it seemed to me that we had already resolved overloads
when we got there.

As a bonus, I added the tf_conv test before another mark_used call I'd
missed in the previous patch.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


[PR c++/84943] mark function as used when taking its address

fn[0]() ICEd because we would fold the INDIRECT_REF used for the
array indexing while building the address for the call, after not
finding the decl hiding there at first.  But the decl would be exposed
by the folding, and then lower layers would complain we had the decl,
after all, but it wasn't one of the artificial or special functions
that could be called without being marked as used.

This patch arranges for a FUNCTION_DECL to be marked as used when
taking its address, just like we already did when taking the address
of a static function to call it as a member function (i.e. using the
obj.fn() notation).  However, we shouldn't mark functions as used when
just performing overload resolution, lest we might instantiate
templates we shouldn't, as in g++.dg/overload/template1.C.


for  gcc/cp/ChangeLog

PR c++/84943
* typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as
used.

for  gcc/testsuite/ChangeLog

PR c++/84943
* g++.dg/pr84943.C: New.
* g++.dg/pr84943-2.C: New.
---
 gcc/cp/typeck.c  |9 -
 gcc/testsuite/g++.dg/pr84943-2.C |   64 ++
 gcc/testsuite/g++.dg/pr84943.C   |8 +
 3 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C
 create mode 100644 gcc/testsuite/g++.dg/pr84943.C

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index d454c6c5a295..bdb2bb30a583 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5801,7 +5801,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, 
tsubst_flags_t complain)
 and the created OFFSET_REF.  */
   tree base = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg, 0)));
   tree fn = get_first_fn (TREE_OPERAND (arg, 1));
-  if (!mark_used (fn, complain) && !(complain & tf_error))
+  if (!(complain & tf_conv)
+ && !mark_used (fn, complain) && !(complain & tf_error))
return error_mark_node;
 
   if (! flag_ms_extensions)
@@ -5971,6 +5972,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, 
tsubst_flags_t complain)
  so we can just form an ADDR_EXPR with the correct type.  */
   if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF)
 {
+  if (TREE_CODE (arg) == FUNCTION_DECL && !(complain & tf_conv)
+ && !mark_used (arg, complain) && !(complain & tf_error))
+   return error_mark_node;
   val = build_address (arg);
   if (TREE_CODE (arg) == OFFSET_REF)
PTRMEM_OK_P (val) = PTRMEM_OK_P (arg);
@@ -5983,7 +5987,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, 
tsubst_flags_t complain)
 function.  */
   gcc_assert (TREE_CODE (fn) == FUNCTION_DECL
  && DECL_STATIC_FUNCTION_P (fn));
-  if (!mark_used (fn, complain) && !(complain & tf_error))
+  if (!(complain & tf_conv)
+ && !mark_used (fn, complain) && !(complain & tf_error))
return error_mark_node;
   val = build_address (fn);
   if (TREE_SIDE_EFFECTS (TREE_OPERAND (arg, 0)))
diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C
new file mode 100644
index ..d1ef012b9155
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr84943-2.C
@@ -0,0 +1,64 @@
+// { dg-do run }
+
+// Avoid -pedantic-error default
+// { dg-options "" }
+
+// Make sure the functions referenced by various forms of
+// address-taking are marked as used and compiled in.
+
+static void ac() {}
+void a() {
+  ac[0](); // { dg-warning "arithmetic" }
+}
+
+static void bc() {}
+void b() {
+  (&*&*&*)();
+}
+
+template  U cc() {}
+void (*c())() {
+  return cc;
+}
+
+template 
+struct x {
+  void a(int);
+  template  static U a(x*) {}
+  static void a(long) {}
+  static void a(void *) {}
+  static void a() {
+void (*p0)(void*) = x().a;
+p0(0);
+void (*p1)(long) = a;
+p1(0);
+void (*p2)() = a;
+p2();
+void (*p3)(x*) = a;
+p3(0);
+  }
+};
+
+struct z {
+  void a(int);
+  template  static U a(z*) {}
+  static void a(long) {}
+  static void a(void *) {}
+  static void a() {
+void (*p0)(void*) = z().a;
+p0(0);
+void (*p1)(long) = a;
+p1(0);
+void (*p2)() = a;
+p2();
+void (*p3)(z*) = a;
+p3(0);
+  }
+};
+
+int main(int argc, char *argv[]) {
+  if (argc > 1) {
+x().a();
+z().a();
+  }
+}
diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C
new file mode 100644
index ..36f75a164119
---