[RFC/RFA] [PATCH 10/12] Verify detected CRC loop with symbolic execution and LFSR matching

[Mariam Arutunian]

Symbolically execute potential CRC loops and check whether the loop
actually calculates CRC (uses LFSR matching).
Calculated CRC and created LFSR are compared on each iteration of the
potential CRC loop.

  gcc/

* Makefile.in (OBJS): Add crc-verification.o.
* crc-verification.cc: New file.
* crc-verification.h: New file.
* gimple-crc-optimization.cc (loop_calculates_crc): New function.
(is_output_crc): Likewise.
(swap_crc_and_data_if_needed): Likewise.
(get_output_phi): Likewise.
(execute): Add check whether potential CRC loop calculates CRC.

  gcc/sym-exec/

* state.cc (create_reversed_lfsr): New function.
(create_forward_lfsr): Likewise.
(last_set_bit): Likewise.
(create_lfsr): Likewise.
* state.h (is_bit_vector): Reorder, make the function public and static.
(create_reversed_lfsr): New static function declaration.
(create_forward_lfsr): New static function declaration.

Signed-off-by: Mariam Arutunian 
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index aab909c3510..1996a60078c 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1716,6 +1716,7 @@ OBJS = \
 	tree-iterator.o \
 	tree-logical-location.o \
 	tree-loop-distribution.o \
+	crc-verification.o \
 	gimple-crc-optimization.o \
 	sym-exec/expression.o \
 	sym-exec/state.o \
diff --git a/gcc/crc-verification.cc b/gcc/crc-verification.cc
new file mode 100644
index 000..0922199d377
--- /dev/null
+++ b/gcc/crc-verification.cc
@@ -0,0 +1,1331 @@
+/* Execute symbolically all paths of the loop.
+   Calculate the value of the polynomial by executing loop with real values to
+   create LFSR state.
+   After each iteration check that final states of calculated CRC values match
+   determined LFSR.
+   Copyright (C) 2022-2024 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.   */
+
+#include "crc-verification.h"
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "ssa.h"
+#include "gimple-iterator.h"
+#include "tree-cfg.h"
+#include "cfganal.h"
+#include "tree-ssa-loop.h"
+
+/* Check whether defined variable is used outside the loop, only
+   CRC's definition is allowed to be used outside the loop.  */
+
+bool
+crc_symbolic_execution::is_used_outside_the_loop (tree def)
+{
+  imm_use_iterator imm_iter;
+  gimple *use_stmt;
+  FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def)
+{
+  if (!flow_bb_inside_loop_p (m_crc_loop, use_stmt->bb))
+	{
+	  if (is_a (use_stmt)
+	  && as_a (use_stmt) == m_output_crc)
+	return false;
+	  if (dump_file)
+	fprintf (dump_file, "Defined variable is used outside the loop.\n");
+	  return true;
+	}
+}
+  return false;
+}
+
+/* Calculate value of the rhs operation of GS assigment statement
+   and assign it to lhs variable.  */
+
+bool
+crc_symbolic_execution::execute_assign_statement (const gassign *gs)
+{
+  enum tree_code rhs_code = gimple_assign_rhs_code (gs);
+  tree lhs = gimple_assign_lhs (gs);
+  if (dump_file && (dump_flags & TDF_DETAILS))
+fprintf (dump_file, "lhs type : %s \n",
+	 get_tree_code_name (TREE_CODE (lhs)));
+
+  /* This will filter some normal cases too.  Ex.  usage of array.  */
+  if (TREE_CODE (lhs) != SSA_NAME)
+return false;
+
+  /* Check uses only when m_output_crc is known.  */
+  if (m_output_crc)
+if (is_used_outside_the_loop (lhs))
+  return false;
+
+  state *current_state = m_states.last ();
+
+  if (gimple_num_ops (gs) == 2)
+{
+  tree op1 = gimple_assign_rhs1 (gs);
+  switch (rhs_code)
+	{
+	  case BIT_NOT_EXPR:
+	return current_state->do_complement (op1, lhs);
+	  case NOP_EXPR:
+	  case SSA_NAME:
+	  case VAR_DECL:
+	  case INTEGER_CST:
+	return current_state->do_assign (op1, lhs);
+	  default:
+	{
+	  if (dump_file)
+		fprintf (dump_file,
+			 "Warning, encountered unsupported unary operation "
+			 "with %s code while executing assign statement!\n",
+			 get_tree_code_name (rhs_code));
+	  return false;
+	}
+	}
+}
+  else if (gimple_num_ops (gs) == 3)
+{
+  tree op1 = gimple_assign_rhs1 (gs);
+  tree op2 = gimple_assign_rhs2 (gs);
+  switch (rhs_code)
+	{
+	  case LSHIFT_EXPR:
+	return current_state->do_shift_left (op1, op2, lhs);
+	  case RSHIFT_EXPR:
+	return current_state->do_shift_right (op1, op2, lhs);
+	  case 

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Hi,

I've now pushed the below change to the following branches with the 
corresponding commit id.


trunk: 9ddad76e98ac8f257f90b3814ed3c6ba78d0f3c7
releases/gcc-14: da3a6b0dda45bc676bb985d7940853b50803e11a
releases/gcc-13: 75d394c20b0ad85dfe8511324d61d13e453c9285
releases/gcc-12: d9c89402b54be4c15bb3c7bcce3465f534746204
releases/gcc-11: 08ca81e4b49bda153d678a372df7f7143a94f4ad

Kind regards,
Torbjörn


On 2024-05-22 13:54, Richard Earnshaw (lists) wrote:

On 22/05/2024 12:14, Torbjorn SVENSSON wrote:

Hello Richard,

Thanks for the reply.

 From my point of view, at least the -fshort-enums part should be on all 
branches. Just to be clean, maybe it's easier to backport the entire patch?


Yes, that's a fair point.  I was only thinking about the broadening of the test 
to the other argument registers when I said that.

So, just to be clear, OK all.

R.



Unless you have an objection, I would like to go ahead and just backport it to 
all branches.

Kind regards,
Torbjörn

On 2024-05-22 12:55, Richard Earnshaw (lists) wrote:

On 06/05/2024 12:50, Torbjorn SVENSSON wrote:

Hi,

Forgot to mention when I sent the patch that I would like to commit it to the 
following branches:

- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk



Well you can [commit it to the release branches], but I'm not sure it's 
essential.  It seems pretty unlikely to me that this would regress on a release 
branch without having first regressed on trunk.

R.


Kind regards,
Torbjörn

On 2024-05-02 12:50, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

  * gcc.target/arm/cmse/extend-param.c: Add regression test. Add
    -fshort-enums.
  * gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
    .../gcc.target/arm/cmse/extend-param.c    | 21 +++
    .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
    2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
    /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
    /* { dg-final { check-function-bodies "**" "" "" } } */
      #include 
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
  if (index >= ARRAY_SIZE)
    return 0;
  return array[index];
-
    }
      /*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
    **    ...
    */
    __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
-
  if (index >= ARRAY_SIZE)
    return 0;
  return array[index];
+}
    -}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+**    ...
+**    uxtb    r0, r0
+**    uxtb    r1, r1
+**    uxth    r2, r2
+**    uxtb    r3, r3
+**    ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+    return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
    /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
    /* { dg-final { check-function-bodies "**" "" "" } } */
      #include 
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)
    unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
    {
  return ns_foo_p ();
-}
\ No newline at end of file
+}

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

On 22/05/2024 12:14, Torbjorn SVENSSON wrote:
> Hello Richard,
> 
> Thanks for the reply.
> 
> From my point of view, at least the -fshort-enums part should be on all 
> branches. Just to be clean, maybe it's easier to backport the entire patch?

Yes, that's a fair point.  I was only thinking about the broadening of the test 
to the other argument registers when I said that.

So, just to be clear, OK all.

R.

> 
> Unless you have an objection, I would like to go ahead and just backport it 
> to all branches.
> 
> Kind regards,
> Torbjörn
> 
> On 2024-05-22 12:55, Richard Earnshaw (lists) wrote:
>> On 06/05/2024 12:50, Torbjorn SVENSSON wrote:
>>> Hi,
>>>
>>> Forgot to mention when I sent the patch that I would like to commit it to 
>>> the following branches:
>>>
>>> - releases/gcc-11
>>> - releases/gcc-12
>>> - releases/gcc-13
>>> - releases/gcc-14
>>> - trunk
>>>
>>
>> Well you can [commit it to the release branches], but I'm not sure it's 
>> essential.  It seems pretty unlikely to me that this would regress on a 
>> release branch without having first regressed on trunk.
>>
>> R.
>>
>>> Kind regards,
>>> Torbjörn
>>>
>>> On 2024-05-02 12:50, Torbjörn SVENSSON wrote:
>>>> Add regression test to the existing zero/sign extend tests for CMSE to
>>>> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>>>>
>>>> boolCharShortEnumSecureFunc test is done using -O0 to ensure the
>>>> instructions are in a predictable order.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>  * gcc.target/arm/cmse/extend-param.c: Add regression test. Add
>>>>    -fshort-enums.
>>>>  * gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.
>>>>
>>>> Signed-off-by: Torbjörn SVENSSON 
>>>> ---
>>>>    .../gcc.target/arm/cmse/extend-param.c    | 21 +++
>>>>    .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
>>>>    2 files changed, 19 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
>>>> b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>>> index 01fac786238..d01ef87e0be 100644
>>>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>>> @@ -1,5 +1,5 @@
>>>>    /* { dg-do compile } */
>>>> -/* { dg-options "-mcmse" } */
>>>> +/* { dg-options "-mcmse -fshort-enums" } */
>>>>    /* { dg-final { check-function-bodies "**" "" "" } } */
>>>>      #include 
>>>> @@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char 
>>>> enumSecureFunc (enum offset index) {
>>>>  if (index >= ARRAY_SIZE)
>>>>    return 0;
>>>>  return array[index];
>>>> -
>>>>    }
>>>>      /*
>>>> @@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char 
>>>> enumSecureFunc (enum offset index) {
>>>>    **    ...
>>>>    */
>>>>    __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
>>>> -
>>>>  if (index >= ARRAY_SIZE)
>>>>    return 0;
>>>>  return array[index];
>>>> +}
>>>>    -}
>>>> \ No newline at end of file
>>>> +/*
>>>> +**__acle_se_boolCharShortEnumSecureFunc:
>>>> +**    ...
>>>> +**    uxtb    r0, r0
>>>> +**    uxtb    r1, r1
>>>> +**    uxth    r2, r2
>>>> +**    uxtb    r3, r3
>>>> +**    ...
>>>> +*/
>>>> +__attribute__((cmse_nonsecure_entry,optimize(0))) char 
>>>> boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, 
>>>> enum offset d) {
>>>> +  size_t index = a + b + c + d;
>>>> +  if (index >= ARRAY_SIZE)
>>>> +    return 0;
>>>> +  return array[index];
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
>>>> b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>>>> index cf731ed33df..081de0d699f 100644
>>>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>>>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>>>> @@ -1,5 +1,5 @@
>>>>    /* { dg-do compile } */
>>>> -/* { dg-options "-mcmse" } */
>>>> +/* { dg-options "-mcmse -fshort-enums" } */
>>>>    /* { dg-final { check-function-bodies "**" "" "" } } */
>>>>      #include 
>>>> @@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
>>>> (ns_enum_foo_t * ns_foo_p)
>>>>    unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
>>>>    {
>>>>  return ns_foo_p ();
>>>> -}
>>>> \ No newline at end of file
>>>> +}
>>

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Hello Richard,

Thanks for the reply.

From my point of view, at least the -fshort-enums part should be on all 
branches. Just to be clean, maybe it's easier to backport the entire patch?


Unless you have an objection, I would like to go ahead and just backport 
it to all branches.


Kind regards,
Torbjörn

On 2024-05-22 12:55, Richard Earnshaw (lists) wrote:

On 06/05/2024 12:50, Torbjorn SVENSSON wrote:

Hi,

Forgot to mention when I sent the patch that I would like to commit it to the 
following branches:

- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk



Well you can [commit it to the release branches], but I'm not sure it's 
essential.  It seems pretty unlikely to me that this would regress on a release 
branch without having first regressed on trunk.

R.


Kind regards,
Torbjörn

On 2024-05-02 12:50, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

 * gcc.target/arm/cmse/extend-param.c: Add regression test. Add
   -fshort-enums.
 * gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
   .../gcc.target/arm/cmse/extend-param.c    | 21 +++
   .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
   2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
   /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
   /* { dg-final { check-function-bodies "**" "" "" } } */
     #include 
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
     if (index >= ARRAY_SIZE)
   return 0;
     return array[index];
-
   }
     /*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
   **    ...
   */
   __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
-
     if (index >= ARRAY_SIZE)
   return 0;
     return array[index];
+}
   -}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+**    ...
+**    uxtb    r0, r0
+**    uxtb    r1, r1
+**    uxth    r2, r2
+**    uxtb    r3, r3
+**    ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+    return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
   /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
   /* { dg-final { check-function-bodies "**" "" "" } } */
     #include 
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)
   unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
   {
     return ns_foo_p ();
-}
\ No newline at end of file
+}

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

On 06/05/2024 12:50, Torbjorn SVENSSON wrote:
> Hi,
> 
> Forgot to mention when I sent the patch that I would like to commit it to the 
> following branches:
> 
> - releases/gcc-11
> - releases/gcc-12
> - releases/gcc-13
> - releases/gcc-14
> - trunk
> 

Well you can [commit it to the release branches], but I'm not sure it's 
essential.  It seems pretty unlikely to me that this would regress on a release 
branch without having first regressed on trunk.

R.

> Kind regards,
> Torbjörn
> 
> On 2024-05-02 12:50, Torbjörn SVENSSON wrote:
>> Add regression test to the existing zero/sign extend tests for CMSE to
>> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>>
>> boolCharShortEnumSecureFunc test is done using -O0 to ensure the
>> instructions are in a predictable order.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/arm/cmse/extend-param.c: Add regression test. Add
>>   -fshort-enums.
>> * gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.
>>
>> Signed-off-by: Torbjörn SVENSSON 
>> ---
>>   .../gcc.target/arm/cmse/extend-param.c    | 21 +++
>>   .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
>>   2 files changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
>> b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> index 01fac786238..d01ef87e0be 100644
>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> @@ -1,5 +1,5 @@
>>   /* { dg-do compile } */
>> -/* { dg-options "-mcmse" } */
>> +/* { dg-options "-mcmse -fshort-enums" } */
>>   /* { dg-final { check-function-bodies "**" "" "" } } */
>>     #include 
>> @@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
>> (enum offset index) {
>>     if (index >= ARRAY_SIZE)
>>   return 0;
>>     return array[index];
>> -
>>   }
>>     /*
>> @@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
>> (enum offset index) {
>>   **    ...
>>   */
>>   __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
>> -
>>     if (index >= ARRAY_SIZE)
>>   return 0;
>>     return array[index];
>> +}
>>   -}
>> \ No newline at end of file
>> +/*
>> +**__acle_se_boolCharShortEnumSecureFunc:
>> +**    ...
>> +**    uxtb    r0, r0
>> +**    uxtb    r1, r1
>> +**    uxth    r2, r2
>> +**    uxtb    r3, r3
>> +**    ...
>> +*/
>> +__attribute__((cmse_nonsecure_entry,optimize(0))) char 
>> boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
>> offset d) {
>> +  size_t index = a + b + c + d;
>> +  if (index >= ARRAY_SIZE)
>> +    return 0;
>> +  return array[index];
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
>> b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>> index cf731ed33df..081de0d699f 100644
>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
>> @@ -1,5 +1,5 @@
>>   /* { dg-do compile } */
>> -/* { dg-options "-mcmse" } */
>> +/* { dg-options "-mcmse -fshort-enums" } */
>>   /* { dg-final { check-function-bodies "**" "" "" } } */
>>     #include 
>> @@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
>> (ns_enum_foo_t * ns_foo_p)
>>   unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
>>   {
>>     return ns_foo_p ();
>> -}
>> \ No newline at end of file
>> +}

[PING^2] [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Gentle ping!

Kind regards,
Torbjörn

On 2024-05-14 13:01, Torbjorn SVENSSON wrote:

Hi,

I'm not sure if the previous "ok" from Richard on the v1 is enough for 
this or if there needs another approval.


Adding extra maintainers since Richard Earnshaw appears to be busy the 
past weeks.


Kind regards,
Torbjörn

On 2024-05-06 13:50, Torbjorn SVENSSON wrote:

Hi,

Forgot to mention when I sent the patch that I would like to commit it 
to the following branches:


- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk

Kind regards,
Torbjörn

On 2024-05-02 12:50, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test. Add
  -fshort-enums.
* gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
  .../gcc.target/arm/cmse/extend-param.c    | 21 +++
  .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
  2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c

index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  #include 
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char 
enumSecureFunc (enum offset index) {

    if (index >= ARRAY_SIZE)
  return 0;
    return array[index];
-
  }
  /*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char 
enumSecureFunc (enum offset index) {

  **    ...
  */
  __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool 
index) {

-
    if (index >= ARRAY_SIZE)
  return 0;
    return array[index];
+}
-}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+**    ...
+**    uxtb    r0, r0
+**    uxtb    r1, r1
+**    uxth    r2, r2
+**    uxtb    r3, r3
+**    ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short 
c, enum offset d) {

+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+    return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c

index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  #include 
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)

  unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
  {
    return ns_foo_p ();
-}
\ No newline at end of file
+}

[PING] [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Hi,

I'm not sure if the previous "ok" from Richard on the v1 is enough for 
this or if there needs another approval.


Adding extra maintainers since Richard Earnshaw appears to be busy the 
past weeks.


Kind regards,
Torbjörn

On 2024-05-06 13:50, Torbjorn SVENSSON wrote:

Hi,

Forgot to mention when I sent the patch that I would like to commit it 
to the following branches:


- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk

Kind regards,
Torbjörn

On 2024-05-02 12:50, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test. Add
  -fshort-enums.
* gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
  .../gcc.target/arm/cmse/extend-param.c    | 21 +++
  .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
  2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c

index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  #include 
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char 
enumSecureFunc (enum offset index) {

    if (index >= ARRAY_SIZE)
  return 0;
    return array[index];
-
  }
  /*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char 
enumSecureFunc (enum offset index) {

  **    ...
  */
  __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool 
index) {

-
    if (index >= ARRAY_SIZE)
  return 0;
    return array[index];
+}
-}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+**    ...
+**    uxtb    r0, r0
+**    uxtb    r1, r1
+**    uxth    r2, r2
+**    uxtb    r3, r3
+**    ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short 
c, enum offset d) {

+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+    return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c

index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  #include 
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)

  unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
  {
    return ns_foo_p ();
-}
\ No newline at end of file
+}

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Hi,

Forgot to mention when I sent the patch that I would like to commit it 
to the following branches:


- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk

Kind regards,
Torbjörn

On 2024-05-02 12:50, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test. Add
  -fshort-enums.
* gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
  .../gcc.target/arm/cmse/extend-param.c| 21 +++
  .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
  2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  
  #include 

@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
if (index >= ARRAY_SIZE)
  return 0;
return array[index];
-
  }
  
  /*

@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
  **...
  */
  __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
-
if (index >= ARRAY_SIZE)
  return 0;
return array[index];
+}
  
-}

\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtbr0, r0
+** uxtbr1, r1
+** uxthr2, r2
+** uxtbr3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
  /* { dg-final { check-function-bodies "**" "" "" } } */
  
  #include 

@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)
  unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
  {
return ns_foo_p ();
-}
\ No newline at end of file
+}

[PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjörn SVENSSON]

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test. Add
  -fshort-enums.
* gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.

Signed-off-by: Torbjörn SVENSSON 
---
 .../gcc.target/arm/cmse/extend-param.c| 21 +++
 .../gcc.target/arm/cmse/extend-return.c   |  4 ++--
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
 /* { dg-final { check-function-bodies "**" "" "" } } */
 
 #include 
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
   if (index >= ARRAY_SIZE)
 return 0;
   return array[index];
-
 }
 
 /*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc 
(enum offset index) {
 ** ...
 */
 __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
-
   if (index >= ARRAY_SIZE)
 return 0;
   return array[index];
+}
 
-}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtbr0, r0
+** uxtbr1, r1
+** uxthr2, r2
+** uxtbr3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+return 0;
+  return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
 /* { dg-final { check-function-bodies "**" "" "" } } */
 
 #include 
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 
(ns_enum_foo_t * ns_foo_p)
 unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
 {
   return ns_foo_p ();
-}
\ No newline at end of file
+}
-- 
2.25.1

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

On 30/04/2024 16:37, Torbjorn SVENSSON wrote:
> 
> 
> On 2024-04-30 17:11, Richard Earnshaw (lists) wrote:
>> On 27/04/2024 15:13, Torbjörn SVENSSON wrote:
>>> Add regression test to the existing zero/sign extend tests for CMSE to
>>> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>>>
>>> Test is done using -O0 to ensure the instructions are in a predictable
>>> order.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/arm/cmse/extend-param.c: Add regression test.
>>>
>>> Signed-off-by: Torbjörn SVENSSON 
>>> ---
>>>   .../gcc.target/arm/cmse/extend-param.c    | 20 ++-
>>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
>>> b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> index 01fac786238..b8b8ecbff56 100644
>>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> @@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char 
>>> boolSecureFunc (bool index) {
>>>   return 0;
>>>     return array[index];
>>>   -}
>>> \ No newline at end of file
>>> +}
>>> +
>>> +/*
>>> +**__acle_se_boolCharShortEnumSecureFunc:
>>> +**    ...
>>> +**    uxtb    r0, r0
>>> +**    uxtb    r1, r1
>>> +**    uxth    r2, r2
>>> +**    uxtb    r3, r3
>>> +**    ...
>>> +*/
>>> +__attribute__((cmse_nonsecure_entry,optimize(0))) char 
>>> boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, 
>>> enum offset d) {
>>> +
>>> +  size_t index = a + b + c + d;
>>> +  if (index >= ARRAY_SIZE)
>>> +    return 0;
>>> +  return array[index];
>>> +
>>> +}
>>
>> Ok, but please can you add '-fshort-enums' to dg-options to ensure this test 
>> still behaves correctly if run with a different default (I missed that last 
>> time around).
> 
> Ok, I'll add that to extend-param.c. Do you want me to also add it to the 
> extend-return.c test case?
> 
> Kind regards,
> Torbjörn

Yes please, if it has the same issue.

R.

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

On 2024-04-30 17:11, Richard Earnshaw (lists) wrote:

On 27/04/2024 15:13, Torbjörn SVENSSON wrote:

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

Test is done using -O0 to ensure the instructions are in a predictable
order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test.

Signed-off-by: Torbjörn SVENSSON 
---
  .../gcc.target/arm/cmse/extend-param.c| 20 ++-
  1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..b8b8ecbff56 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc 
(bool index) {
  return 0;
return array[index];
  
-}

\ No newline at end of file
+}
+
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtbr0, r0
+** uxtbr1, r1
+** uxthr2, r2
+** uxtbr3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+return 0;
+  return array[index];
+
+}


Ok, but please can you add '-fshort-enums' to dg-options to ensure this test 
still behaves correctly if run with a different default (I missed that last 
time around).


Ok, I'll add that to extend-param.c. Do you want me to also add it to 
the extend-return.c test case?


Kind regards,
Torbjörn

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

On 27/04/2024 15:13, Torbjörn SVENSSON wrote:
> Add regression test to the existing zero/sign extend tests for CMSE to
> verify that r0, r1, r2 and r3 are properly extended, not just r0.
> 
> Test is done using -O0 to ensure the instructions are in a predictable
> order.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/arm/cmse/extend-param.c: Add regression test.
> 
> Signed-off-by: Torbjörn SVENSSON 
> ---
>  .../gcc.target/arm/cmse/extend-param.c| 20 ++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
> b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> index 01fac786238..b8b8ecbff56 100644
> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> @@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc 
> (bool index) {
>  return 0;
>return array[index];
>  
> -}
> \ No newline at end of file
> +}
> +
> +/*
> +**__acle_se_boolCharShortEnumSecureFunc:
> +**   ...
> +**   uxtbr0, r0
> +**   uxtbr1, r1
> +**   uxthr2, r2
> +**   uxtbr3, r3
> +**   ...
> +*/
> +__attribute__((cmse_nonsecure_entry,optimize(0))) char 
> boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
> offset d) {
> +
> +  size_t index = a + b + c + d;
> +  if (index >= ARRAY_SIZE)
> +return 0;
> +  return array[index];
> +
> +}

Ok, but please can you add '-fshort-enums' to dg-options to ensure this test 
still behaves correctly if run with a different default (I missed that last 
time around).

R.

[COMMITTED 09/16] Verify that reading back from vrange_storage doesn't drop bits.

[Aldy Hernandez]

We have a sanity check in the irange storage code to make sure that
reading back a cache entry we have just written to yields exactly the
same range.  There's no need to do this only for integers.  This patch
moves the code to a more generic place.

However, doing so tickles a latent bug in the frange code where a
range is being pessimized from [0.0, 1.0] to [-0.0, 1.0].  Exclude
checking frange's until this bug is fixed.

gcc/ChangeLog:

* value-range-storage.cc (irange_storage::set_irange): Move
verification code from here...
(vrange_storage::set_vrange): ...to here.
---
 gcc/value-range-storage.cc | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/gcc/value-range-storage.cc b/gcc/value-range-storage.cc
index f00474ad0e6..09a29776a0e 100644
--- a/gcc/value-range-storage.cc
+++ b/gcc/value-range-storage.cc
@@ -165,6 +165,19 @@ vrange_storage::set_vrange (const vrange )
 }
   else
 gcc_unreachable ();
+
+  // Verify that reading back from the cache didn't drop bits.
+  if (flag_checking
+  // FIXME: Avoid checking frange, as it currently pessimizes some ranges:
+  //
+  // gfortran.dg/pr49472.f90 pessimizes [0.0, 1.0] into [-0.0, 1.0].
+  && !is_a  (r)
+  && !r.undefined_p ())
+{
+  Value_Range tmp (r);
+  get_vrange (tmp, r.type ());
+  gcc_checking_assert (tmp == r);
+}
 }
 
 // Restore R from storage.
@@ -306,13 +319,6 @@ irange_storage::set_irange (const irange )
   irange_bitmask bm = r.m_bitmask;
   write_wide_int (val, len, bm.value ());
   write_wide_int (val, len, bm.mask ());
-
-  if (flag_checking)
-{
-  int_range_max tmp;
-  get_irange (tmp, r.type ());
-  gcc_checking_assert (tmp == r);
-}
 }
 
 static inline void
-- 
2.44.0

[PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Torbjörn SVENSSON]

Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.

Test is done using -O0 to ensure the instructions are in a predictable
order.

gcc/testsuite/ChangeLog:

* gcc.target/arm/cmse/extend-param.c: Add regression test.

Signed-off-by: Torbjörn SVENSSON 
---
 .../gcc.target/arm/cmse/extend-param.c| 20 ++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c 
b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..b8b8ecbff56 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc 
(bool index) {
 return 0;
   return array[index];
 
-}
\ No newline at end of file
+}
+
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtbr0, r0
+** uxtbr1, r1
+** uxthr2, r2
+** uxtbr3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char 
boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum 
offset d) {
+
+  size_t index = a + b + c + d;
+  if (index >= ARRAY_SIZE)
+return 0;
+  return array[index];
+
+}
-- 
2.25.1

[PATCH] tree-optimization/114464 - verify types in recurrence vectorization

[Richard Biener]

The following adds missing verification of vector type compatibility
to recurrence vectorization.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR tree-optimization/114464
* tree-vect-loop.cc (vectorizable_recurr): Verify the latch
vector type is compatible with what we chose for the recurrence.

* g++.dg/vect/pr114464.cc: New testcase.
---
 gcc/testsuite/g++.dg/vect/pr114464.cc | 11 +++
 gcc/tree-vect-loop.cc | 22 ++
 2 files changed, 33 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/vect/pr114464.cc

diff --git a/gcc/testsuite/g++.dg/vect/pr114464.cc 
b/gcc/testsuite/g++.dg/vect/pr114464.cc
new file mode 100644
index 000..0d872aae9d4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr114464.cc
@@ -0,0 +1,11 @@
+// { dg-do compile }
+
+void h(unsigned char *scratch, bool carry)
+{
+  for (int i = 0; i < 16; i++) {
+bool b = scratch[i] <<= 1;
+if (carry)
+  scratch[i] |= 1;
+carry = b;
+  }
+}
diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 302c92e6f31..42e78cc9c32 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -9212,6 +9212,28 @@ vectorizable_recurr (loop_vec_info loop_vinfo, 
stmt_vec_info stmt_info,
return false;
  }
}
+
+  /* Verify we have set up compatible types.  */
+  edge le = loop_latch_edge (LOOP_VINFO_LOOP (loop_vinfo));
+  tree latch_vectype = NULL_TREE;
+  if (slp_node)
+   {
+ slp_tree latch_def = SLP_TREE_CHILDREN (slp_node)[le->dest_idx];
+ latch_vectype = SLP_TREE_VECTYPE (latch_def);
+   }
+  else
+   {
+ tree latch_def = PHI_ARG_DEF_FROM_EDGE (phi, le);
+ if (TREE_CODE (latch_def) == SSA_NAME)
+   {
+ stmt_vec_info latch_def_info = loop_vinfo->lookup_def (latch_def);
+ latch_def_info = vect_stmt_to_vectorize (latch_def_info);
+ latch_vectype = STMT_VINFO_VECTYPE (latch_def_info);
+   }
+   }
+  if (!types_compatible_p (latch_vectype, vectype))
+   return false;
+
   /* The recurrence costs the initialization vector and one permute
 for each copy.  */
   unsigned prologue_cost = record_stmt_cost (cost_vec, 1, scalar_to_vec,
-- 
2.35.3

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Maciej W. Rozycki]

On Wed, 24 Jan 2024, Jeff Law wrote:

> >   Do we have consensus now to move forward with this change as posted?  I'd
> > like to get these patches ticked off ASAP.
> I think it should move forward.  I think having the RTL tests deals with
> Andrew's concern and the testcase adjustment has value as well.
> 
> I ACK's the RTL tests a few minutes ago and we should consider the 1/2 and 2/2
> of the original OK now as well.

 I have committed both patch series now, thank you for your assistance and 
review.

  Maciej

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Jeff Law]

On 1/24/24 04:26, Maciej W. Rozycki wrote:

On Tue, 16 Jan 2024, Maciej W. Rozycki wrote:


I don't have a strong opinion on this.  I certainly see Andrew's point, but
it's also the case that if some work earlier in the RTL or gimple pipeline
comes along and compromises the test, then we'd see the failure and deal with
it.  It's pretty standard procedure.


  I'll be happy to add an RTL test case, also for my recent complementary
cset-sext.c addition and maybe other if-conversion pieces recently added.
I think that does not preclude arming pr105314.c with RTL scanning though.


  I have made a buch of testcases as we discussed at the meeting last week
and the RTL parser did not blow up, so I have now submitted them.  See:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643802.html>
and the next two messages (threading broke with this submission for some
reason, probably due to a glitch in my mail client I've seen from time to
time; I guess it's not worth it to get the patch series resubmitted as
they are independent from each other really and can be applied in any
order).

  I haven't heard back from Andrew beyond his initial message, so it's not
clear to me whether he maintains his objection in spite the arguments
given.  Andrew?

  Do we have consensus now to move forward with this change as posted?  I'd
like to get these patches ticked off ASAP.
I think it should move forward.  I think having the RTL tests deals with 
Andrew's concern and the testcase adjustment has value as well.


I ACK's the RTL tests a few minutes ago and we should consider the 1/2 
and 2/2 of the original OK now as well.


Thanks,
Jeff

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Maciej W. Rozycki]

On Tue, 16 Jan 2024, Maciej W. Rozycki wrote:

> > I don't have a strong opinion on this.  I certainly see Andrew's point, but
> > it's also the case that if some work earlier in the RTL or gimple pipeline
> > comes along and compromises the test, then we'd see the failure and deal 
> > with
> > it.  It's pretty standard procedure.
> 
>  I'll be happy to add an RTL test case, also for my recent complementary 
> cset-sext.c addition and maybe other if-conversion pieces recently added.  
> I think that does not preclude arming pr105314.c with RTL scanning though.

 I have made a buch of testcases as we discussed at the meeting last week 
and the RTL parser did not blow up, so I have now submitted them.  See: 
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643802.html>
 
and the next two messages (threading broke with this submission for some 
reason, probably due to a glitch in my mail client I've seen from time to 
time; I guess it's not worth it to get the patch series resubmitted as 
they are independent from each other really and can be applied in any 
order).

 I haven't heard back from Andrew beyond his initial message, so it's not 
clear to me whether he maintains his objection in spite the arguments 
given.  Andrew?

 Do we have consensus now to move forward with this change as posted?  I'd 
like to get these patches ticked off ASAP.

  Maciej

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Maciej W. Rozycki]

On Tue, 16 Jan 2024, Jeff Law wrote:

> >   It's not clear to me what you mean by an "RTL testcase", i.e. how you'd
> > see the testcase changed (or an additional one produced instead) and why,
> > please elaborate.  Right now we verify that branches are absent from
> > output, but not how that happens.
> What I'm guessing Andrew is suggesting is the testcase be adjusted so that its
> source is RTL rather than C.  With that framework you can skip most of the
> pipeline and make the test more stable if something changes earlier in the
> pipeline.
> 
> There aren't a lot of great examples of this and the RTL parser is probably
> less stable than the gimple parser.  But if you look in gcc.dg/rtl you should
> see examples.
> 
> In theory you can take the RTL dump from a pass, massage it and feed it back
> into the compiler.  Perhaps a good example is rtl/x86_64/ira.c

 Thanks, I wasn't aware of this feature.

> >   How are the improvements going to affect the testcase?
> > 
> >   If they make it no longer relevant (in which case a replacement testcase
> > for the new arrangement will be needed) or require updates, then I think
> > it's an expected situation: one of the purposes of the testsuite is to
> > make sure we're in control and understand what the consequences of changes
> > made are.  It's not that the testsuite is cast in stone and not expected
> > to change.
> > 
> >   I.e. if we expect noce_try_store_flag_mask no longer to trigger, then
> > we'll see that in the test results (good!) and we can update the relevant
> > test case(s). e.g. by reversing the pass criteria so that we're still in
> > control.
> I think Andrew's point is that we can still test that the pass does what we
> want when presented with RTL in a particular form and isolate the pass from
> depending on prior passes in the pipeline either creating or not destroying
> the particular form we want to ensure is properly handled.

 It makes sense to me.

> I don't have a strong opinion on this.  I certainly see Andrew's point, but
> it's also the case that if some work earlier in the RTL or gimple pipeline
> comes along and compromises the test, then we'd see the failure and deal with
> it.  It's pretty standard procedure.

 I'll be happy to add an RTL test case, also for my recent complementary 
cset-sext.c addition and maybe other if-conversion pieces recently added.  
I think that does not preclude arming pr105314.c with RTL scanning though.

  Maciej

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Jeff Law]

On 1/12/24 06:59, Maciej W. Rozycki wrote:

On Fri, 12 Jan 2024, Andrew Pinski wrote:


Verify that if-conversion succeeded through noce_try_store_flag_mask, as
per PR rtl-optimization/105314, tightening the test case and making it
explicit.

 gcc/testsuite/
 * gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.


I have an objection for this, if we are checking the RTL pass and not
overall code generation, then maybe we change the testcase so that it
is a RTL testcase instead.


  It's not clear to me what you mean by an "RTL testcase", i.e. how you'd
see the testcase changed (or an additional one produced instead) and why,
please elaborate.  Right now we verify that branches are absent from
output, but not how that happens.
What I'm guessing Andrew is suggesting is the testcase be adjusted so 
that its source is RTL rather than C.  With that framework you can skip 
most of the pipeline and make the test more stable if something changes 
earlier in the pipeline.


There aren't a lot of great examples of this and the RTL parser is 
probably less stable than the gimple parser.  But if you look in 
gcc.dg/rtl you should see examples.


In theory you can take the RTL dump from a pass, massage it and feed it 
back into the compiler.  Perhaps a good example is rtl/x86_64/ira.c






Especially when there might be improvements going into GCC 15
specifically targeting ifcvt on the gimple level (I am planning on
doing some).


  How are the improvements going to affect the testcase?

  If they make it no longer relevant (in which case a replacement testcase
for the new arrangement will be needed) or require updates, then I think
it's an expected situation: one of the purposes of the testsuite is to
make sure we're in control and understand what the consequences of changes
made are.  It's not that the testsuite is cast in stone and not expected
to change.

  I.e. if we expect noce_try_store_flag_mask no longer to trigger, then
we'll see that in the test results (good!) and we can update the relevant
test case(s). e.g. by reversing the pass criteria so that we're still in
control.
I think Andrew's point is that we can still test that the pass does what 
we want when presented with RTL in a particular form and isolate the 
pass from depending on prior passes in the pipeline either creating or 
not destroying the particular form we want to ensure is properly handled.


I don't have a strong opinion on this.  I certainly see Andrew's point, 
but it's also the case that if some work earlier in the RTL or gimple 
pipeline comes along and compromises the test, then we'd see the failure 
and deal with it.  It's pretty standard procedure.


Jeff

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Maciej W. Rozycki]

On Fri, 12 Jan 2024, Andrew Pinski wrote:

> > Verify that if-conversion succeeded through noce_try_store_flag_mask, as
> > per PR rtl-optimization/105314, tightening the test case and making it
> > explicit.
> >
> > gcc/testsuite/
> > * gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.
> 
> I have an objection for this, if we are checking the RTL pass and not
> overall code generation, then maybe we change the testcase so that it
> is a RTL testcase instead.

 It's not clear to me what you mean by an "RTL testcase", i.e. how you'd 
see the testcase changed (or an additional one produced instead) and why, 
please elaborate.  Right now we verify that branches are absent from 
output, but not how that happens.

> Especially when there might be improvements going into GCC 15
> specifically targeting ifcvt on the gimple level (I am planning on
> doing some).

 How are the improvements going to affect the testcase?

 If they make it no longer relevant (in which case a replacement testcase 
for the new arrangement will be needed) or require updates, then I think 
it's an expected situation: one of the purposes of the testsuite is to 
make sure we're in control and understand what the consequences of changes 
made are.  It's not that the testsuite is cast in stone and not expected 
to change.

 I.e. if we expect noce_try_store_flag_mask no longer to trigger, then 
we'll see that in the test results (good!) and we can update the relevant 
test case(s). e.g. by reversing the pass criteria so that we're still in 
control.

  Maciej

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Andrew Pinski]

On Thu, Jan 11, 2024 at 3:37 PM Maciej W. Rozycki  wrote:
>
> Verify that if-conversion succeeded through noce_try_store_flag_mask, as
> per PR rtl-optimization/105314, tightening the test case and making it
> explicit.
>
> gcc/testsuite/
> * gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.

I have an objection for this, if we are checking the RTL pass and not
overall code generation, then maybe we change the testcase so that it
is a RTL testcase instead.
Especially when there might be improvements going into GCC 15
specifically targeting ifcvt on the gimple level (I am planning on
doing some).

Thanks,
Andrew Pinski

> ---
>  gcc/testsuite/gcc.target/riscv/pr105314.c |2 ++
>  1 file changed, 2 insertions(+)
>
> gcc-test-riscv-pr105314-rtl.diff
> Index: gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
> ===
> --- gcc.orig/gcc/testsuite/gcc.target/riscv/pr105314.c
> +++ gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
> @@ -1,6 +1,7 @@
>  /* PR rtl-optimization/105314 */
>  /* { dg-do compile } */
>  /* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +/* { dg-options "-fdump-rtl-ce1" } */
>
>  long
>  foo (long a, long b, long c)
> @@ -10,4 +11,5 @@ foo (long a, long b, long c)
>return a;
>  }
>
> +/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through 
> noce_try_store_flag_mask" 1 "ce1" } } */
>  /* { dg-final { scan-assembler-not "\\s(?:beq|bne)\\s" } } */

Re: [PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Kito Cheng]

LGTM

On Fri, Jan 12, 2024 at 7:37 AM Maciej W. Rozycki  wrote:
>
> Verify that if-conversion succeeded through noce_try_store_flag_mask, as
> per PR rtl-optimization/105314, tightening the test case and making it
> explicit.
>
> gcc/testsuite/
> * gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.
> ---
>  gcc/testsuite/gcc.target/riscv/pr105314.c |2 ++
>  1 file changed, 2 insertions(+)
>
> gcc-test-riscv-pr105314-rtl.diff
> Index: gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
> ===
> --- gcc.orig/gcc/testsuite/gcc.target/riscv/pr105314.c
> +++ gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
> @@ -1,6 +1,7 @@
>  /* PR rtl-optimization/105314 */
>  /* { dg-do compile } */
>  /* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +/* { dg-options "-fdump-rtl-ce1" } */
>
>  long
>  foo (long a, long b, long c)
> @@ -10,4 +11,5 @@ foo (long a, long b, long c)
>return a;
>  }
>
> +/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through 
> noce_try_store_flag_mask" 1 "ce1" } } */
>  /* { dg-final { scan-assembler-not "\\s(?:beq|bne)\\s" } } */

[PATCH 2/2] RISC-V/testsuite: Also verify if-conversion runs for pr105314.c

[Maciej W. Rozycki]

Verify that if-conversion succeeded through noce_try_store_flag_mask, as 
per PR rtl-optimization/105314, tightening the test case and making it 
explicit.

gcc/testsuite/
* gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.
---
 gcc/testsuite/gcc.target/riscv/pr105314.c |2 ++
 1 file changed, 2 insertions(+)

gcc-test-riscv-pr105314-rtl.diff
Index: gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
===
--- gcc.orig/gcc/testsuite/gcc.target/riscv/pr105314.c
+++ gcc/gcc/testsuite/gcc.target/riscv/pr105314.c
@@ -1,6 +1,7 @@
 /* PR rtl-optimization/105314 */
 /* { dg-do compile } */
 /* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+/* { dg-options "-fdump-rtl-ce1" } */
 
 long
 foo (long a, long b, long c)
@@ -10,4 +11,5 @@ foo (long a, long b, long c)
   return a;
 }
 
+/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through 
noce_try_store_flag_mask" 1 "ce1" } } */
 /* { dg-final { scan-assembler-not "\\s(?:beq|bne)\\s" } } */

Re: [PATCH] sel-sched: Verify change before replacing dest in EXPR_INSN_RTX [PR112995]

[Kewen.Lin]

Hi Jeff,

on 2023/12/21 04:30, Jeff Law wrote:
> 
> 
> On 12/15/23 01:52, Kewen.Lin wrote:
>> Hi,
>>
>> PR112995 exposed one issue in current try_replace_dest_reg
>> that the result rtx insn after replace_dest_with_reg_in_expr
>> is probably unable to match any constraints.  Although there
>> are some checks on the changes onto dest or src of orig_insn,
>> none is performed on the EXPR_INSN_RTX.
>>
>> This patch is to add the check before actually replacing dest
>> in expr with reg.
>>
>> Bootstrapped and regtested on x86_64-redhat-linux and
>> powerpc64{,le}-linux-gnu.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -
>> PR rtl-optimization/112995
>>
>> gcc/ChangeLog:
>>
>> * sel-sched.cc (try_replace_dest_reg): Check the validity of the
>> replaced insn before actually replacing dest in expr.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/powerpc/pr112995.c: New test.
> Setting aside whether or not we should just deprecate/remove sel-sched for 
> now
> 
> 
> 
> From the PR:
>> with moving up, we have:
>>
>> (insn 46 0 0 (set (reg:DI 64 0 [135])
>>     (sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 
>> {extendsidi2}
>>  (expr_list:REG_DEAD (reg/v:SI 9 9 [orig:119 c ] [119])
>>     (nil)))
>>
>> in try_replace_dest_reg, we updated the above EXPR_INSN_RTX to:
>>
>> (insn 48 0 0 (set (reg:DI 32 0)
>>     (sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 
>> {extendsidi2}
>>  (nil))
>>
>> This doesn't match any constraint and it's an unexpected modification.
> 
> 
> It would have been helpful to include that in the patch, along with the fact 
> that (reg 32) and (reg 64) are FP and VREGs respectively.  That makes it 
> clearer why the constraints might not match after the change.
> 

Good idea!

> OK for the trunk.

Thanks for the review, as suggested I updated the commit log
as below and committed it as r14-6768-g5fbc77726f68a3.

-
PR112995 exposed one issue in current try_replace_dest_reg
that the result rtx insn after replace_dest_with_reg_in_expr
is probably unable to match any constraints.  Although there
are some checks on the changes onto dest or src of orig_insn,
none is performed on the EXPR_INSN_RTX.

Initially we have:

(insn 31 6 10 2 (set (reg/v:SI 9 9 [orig:119 c ] [119])
 (reg/v:SI 64 0 [orig:119 c ] [119]))
"test.i":5:5 555 {*movsi_internal1} ... )
...
(insn 25 10 27 2 (set (reg:DI 64 0 [135])
  (sign_extend:DI
 (reg/v:SI 9 9 [orig:119 c ] [119])))
 "test.i":6:5 31 {extendsidi2} ...)

with moving up, we have:

(insn 46 0 0 (set (reg:DI 64 0 [135])
  (sign_extend:DI
  (reg/v:SI 64 0 [orig:119 c ] [119])))
   31 {extendsidi2} ...)

in try_replace_dest_reg, we updated the above EXPR_INSN_RTX to:

(insn 48 0 0 (set (reg:DI 32 0)
  (sign_extend:DI
  (reg/v:SI 64 0 [orig:119 c ] [119])))
   31 {extendsidi2} ...)

The dest (reg 64) is a VR (also VSX REG), the updating makes
it become to (reg 32) which is a FPR (also VSX REG), we have
an alternative to match "VR,VR" but no one to match "FPR/VSX,
VR/VSX", so it fails with ICE.

This patch is to add the check before actually replacing dest
in expr with reg.
-

BR,
Kewen

Re: [PATCH] sel-sched: Verify change before replacing dest in EXPR_INSN_RTX [PR112995]

[Jeff Law]

On 12/15/23 01:52, Kewen.Lin wrote:

Hi,

PR112995 exposed one issue in current try_replace_dest_reg
that the result rtx insn after replace_dest_with_reg_in_expr
is probably unable to match any constraints.  Although there
are some checks on the changes onto dest or src of orig_insn,
none is performed on the EXPR_INSN_RTX.

This patch is to add the check before actually replacing dest
in expr with reg.

Bootstrapped and regtested on x86_64-redhat-linux and
powerpc64{,le}-linux-gnu.

Is it ok for trunk?

BR,
Kewen
-
PR rtl-optimization/112995

gcc/ChangeLog:

* sel-sched.cc (try_replace_dest_reg): Check the validity of the
replaced insn before actually replacing dest in expr.

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/pr112995.c: New test.
Setting aside whether or not we should just deprecate/remove sel-sched 
for now




From the PR:

with moving up, we have:

(insn 46 0 0 (set (reg:DI 64 0 [135])
(sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 {extendsidi2}
 (expr_list:REG_DEAD (reg/v:SI 9 9 [orig:119 c ] [119])
(nil)))

in try_replace_dest_reg, we updated the above EXPR_INSN_RTX to:

(insn 48 0 0 (set (reg:DI 32 0)
(sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 {extendsidi2}
 (nil))

This doesn't match any constraint and it's an unexpected modification.



It would have been helpful to include that in the patch, along with the 
fact that (reg 32) and (reg 64) are FP and VREGs respectively.  That 
makes it clearer why the constraints might not match after the change.


OK for the trunk.
jeff

[PATCH] sel-sched: Verify change before replacing dest in EXPR_INSN_RTX [PR112995]

[Kewen.Lin]

Hi,

PR112995 exposed one issue in current try_replace_dest_reg
that the result rtx insn after replace_dest_with_reg_in_expr
is probably unable to match any constraints.  Although there
are some checks on the changes onto dest or src of orig_insn,
none is performed on the EXPR_INSN_RTX.

This patch is to add the check before actually replacing dest
in expr with reg.

Bootstrapped and regtested on x86_64-redhat-linux and
powerpc64{,le}-linux-gnu.

Is it ok for trunk?

BR,
Kewen
-
PR rtl-optimization/112995

gcc/ChangeLog:

* sel-sched.cc (try_replace_dest_reg): Check the validity of the
replaced insn before actually replacing dest in expr.

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/pr112995.c: New test.
---
 gcc/sel-sched.cc| 10 +-
 gcc/testsuite/gcc.target/powerpc/pr112995.c | 14 ++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr112995.c

diff --git a/gcc/sel-sched.cc b/gcc/sel-sched.cc
index 1925f4a9461..a35b5e16c91 100644
--- a/gcc/sel-sched.cc
+++ b/gcc/sel-sched.cc
@@ -1614,7 +1614,15 @@ try_replace_dest_reg (ilist_t orig_insns, rtx best_reg, 
expr_t expr)
   /* Make sure that EXPR has the right destination
  register.  */
   if (expr_dest_regno (expr) != REGNO (best_reg))
-replace_dest_with_reg_in_expr (expr, best_reg);
+{
+  rtx_insn *vinsn = EXPR_INSN_RTX (expr);
+  validate_change (vinsn, _DEST (PATTERN (vinsn)), best_reg, 1);
+  bool res = verify_changes (0);
+  cancel_changes (0);
+  if (!res)
+   return false;
+  replace_dest_with_reg_in_expr (expr, best_reg);
+}
   else
 EXPR_TARGET_AVAILABLE (expr) = 1;

diff --git a/gcc/testsuite/gcc.target/powerpc/pr112995.c 
b/gcc/testsuite/gcc.target/powerpc/pr112995.c
new file mode 100644
index 000..4adcb5f3851
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr112995.c
@@ -0,0 +1,14 @@
+/* { dg-require-effective-target float128 } */
+/* { dg-options "-O2 -mdejagnu-cpu=power9 -fselective-scheduling2" } */
+
+/* Verify there is no ICE.  */
+
+int a[10];
+int b(_Float128 e) {
+  int c;
+  _Float128 d;
+  c = e;
+  d = c;
+  d = a[c] + d;
+  return d;
+}
--
2.39.3

verify

[Saurabh Jha via Gcc-patches]

verify

[pushed] testsuite: add verify-sarif-file to some testcases that were missing it

[David Malcolm via Gcc-patches]

Pushed to trunk as r14-1420-ge4c8f7024f02d8.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/malloc-sarif-1.c: Add missing verify-sarif-file
directive.
* gcc.dg/analyzer/sarif-pr107366.c: Likewise.
---
 gcc/testsuite/gcc.dg/analyzer/malloc-sarif-1.c | 2 ++
 gcc/testsuite/gcc.dg/analyzer/sarif-pr107366.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-sarif-1.c 
b/gcc/testsuite/gcc.dg/analyzer/malloc-sarif-1.c
index 3d141b53f47..3d798e687e6 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-sarif-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-sarif-1.c
@@ -12,6 +12,8 @@ void test_1 (void)
 
 /* Verify SARIF output.
 
+ { dg-final { verify-sarif-file } }
+
The threadFlowLocation objects should have "kinds" properties
reflecting the meanings of the events:
  { dg-final { scan-sarif-file "\"kinds\": \\\[\"acquire\", \"memory\"\\\]" 
} }
diff --git a/gcc/testsuite/gcc.dg/analyzer/sarif-pr107366.c 
b/gcc/testsuite/gcc.dg/analyzer/sarif-pr107366.c
index 997cf56586d..ee156d4df23 100644
--- a/gcc/testsuite/gcc.dg/analyzer/sarif-pr107366.c
+++ b/gcc/testsuite/gcc.dg/analyzer/sarif-pr107366.c
@@ -18,3 +18,4 @@ hwloc_apply_diff_one() {
   }
 }
 
+/* { dg-final { verify-sarif-file } } */
-- 
2.26.3

Re: [PATCH v6 7/9] RISC-V: autovec: Verify that GET_MODE_NUNITS is a multiple of 2.

[Jeff Law via Gcc-patches]

On 5/5/23 09:46, Michael Collison wrote:

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is a multiple of 2.
I've pushed this to the trunk given it was acked by Richard S and he 
explicitly indicated it need not wait for all the patches in this kit.


jeff

[PATCH v6 7/9] RISC-V: autovec: Verify that GET_MODE_NUNITS is a multiple of 2.

[Michael Collison]

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is a multiple of 2.
---
 gcc/tree-vect-slp.cc | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index b299e209b5b..3b7a21724ec 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
(GET_MODE_BITSIZE (int_mode), 1);
  tree vector_type
= get_vectype_for_scalar_type (vinfo, int_type, count);
+ poly_int64 half_nelts;
  if (vector_type
  && VECTOR_MODE_P (TYPE_MODE (vector_type))
  && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
-  GET_MODE_SIZE (base_vector_mode)))
+  GET_MODE_SIZE (base_vector_mode))
+ && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
+2, _nelts))
{
  /* Try fusing consecutive sequences of COUNT / NVECTORS elements
 together into elements of type INT_TYPE and using the result
@@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
  poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
  vec_perm_builder sel1 (nelts, 2, 3);
  vec_perm_builder sel2 (nelts, 2, 3);
- poly_int64 half_nelts = exact_div (nelts, 2);
+
  for (unsigned int i = 0; i < 3; ++i)
{
  sel1.quick_push (i);
-- 
2.34.1

Re: [PATCH v5 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Richard Sandiford via Gcc-patches]

Michael Collison  writes:
> While working on autovectorizing for the RISCV port I encountered an issue
> where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
> where GET_MODE_NUNITS is equal to one.
>
> Tested on RISCV and x86_64-linux-gnu. Okay?
>
> 2023-03-09  Michael Collison  
>
>   * tree-vect-slp.cc (can_duplicate_and_interleave_p):
>   Check that GET_MODE_NUNITS is a multiple of 2.

OK, thanks.  Doesn't need to wait for any other of the other patches
in the series.

Richard

> ---
>  gcc/tree-vect-slp.cc | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index d73deaecce0..a64fe454e19 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> unsigned int count,
>   (GET_MODE_BITSIZE (int_mode), 1);
> tree vector_type
>   = get_vectype_for_scalar_type (vinfo, int_type, count);
> +   poly_int64 half_nelts;
> if (vector_type
> && VECTOR_MODE_P (TYPE_MODE (vector_type))
> && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> -GET_MODE_SIZE (base_vector_mode)))
> +GET_MODE_SIZE (base_vector_mode))
> +   && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
> +  2, _nelts))
>   {
> /* Try fusing consecutive sequences of COUNT / NVECTORS elements
>together into elements of type INT_TYPE and using the result
> @@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
> int count,
> poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
> vec_perm_builder sel1 (nelts, 2, 3);
> vec_perm_builder sel2 (nelts, 2, 3);
> -   poly_int64 half_nelts = exact_div (nelts, 2);
> +
> for (unsigned int i = 0; i < 3; ++i)
>   {
> sel1.quick_push (i);

[PATCH v5 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Michael Collison]

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is a multiple of 2.
---
 gcc/tree-vect-slp.cc | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index d73deaecce0..a64fe454e19 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
(GET_MODE_BITSIZE (int_mode), 1);
  tree vector_type
= get_vectype_for_scalar_type (vinfo, int_type, count);
+ poly_int64 half_nelts;
  if (vector_type
  && VECTOR_MODE_P (TYPE_MODE (vector_type))
  && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
-  GET_MODE_SIZE (base_vector_mode)))
+  GET_MODE_SIZE (base_vector_mode))
+ && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
+2, _nelts))
{
  /* Try fusing consecutive sequences of COUNT / NVECTORS elements
 together into elements of type INT_TYPE and using the result
@@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
  poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
  vec_perm_builder sel1 (nelts, 2, 3);
  vec_perm_builder sel2 (nelts, 2, 3);
- poly_int64 half_nelts = exact_div (nelts, 2);
+
  for (unsigned int i = 0; i < 3; ++i)
{
  sel1.quick_push (i);
-- 
2.34.1

Re: [PATCH] vect: Verify that GET_MODE_NUNITS is greater than one.

[Jeff Law via Gcc-patches]

On 3/14/23 15:52, Michael Collison wrote:

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is greater than one.
Is this still relevant?   I know other changes were made to deal with 
the case where GET_MODE_NUNITS returns 1, but I don't know if they made 
this obsolete.


Any chance we could get a testcase for this?  I realize it might depend 
on unmerged RVV bits.



jeff

Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Richard Sandiford via Gcc-patches]

 writes:
> Yes, like kito said.
> We won't enable VNx1DImode in auto-vectorization so it's meaningless to fix 
> it here.
> We dynamic adjust the minimum vector-length for different '-march' according 
> to RVV ISA specification.
> So we strongly suggest that we should drop this fix.

I think the patch should go in regardless.  If we have a port with
a VNx1 mode then the exact_div is at best dubious and at worst wrong.

Thanks,
Richard

Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Michael Collison]

Juzhe and Kito,

Thank you for the clarification.

On 4/18/23 18:48, juzhe.zh...@rivai.ai wrote:

Yes, like kito said.
We won't enable VNx1DImode in auto-vectorization so it's meaningless 
to fix it here.
We dynamic adjust the minimum vector-length for different '-march' 
according to RVV ISA specification.

So we strongly suggest that we should drop this fix.

Thanks.

juzhe.zh...@rivai.ai

*From:* Kito Cheng <mailto:kito.ch...@gmail.com>
*Date:* 2023-04-19 02:21
*To:* Richard Biener <mailto:richard.guent...@gmail.com>; Jeff Law
<mailto:jeffreya...@gmail.com>; Palmer Dabbelt
<mailto:pal...@dabbelt.com>
*CC:* Michael Collison <mailto:colli...@rivosinc.com>; gcc-patches
<mailto:gcc-patches@gcc.gnu.org>; 钟居哲 <mailto:juzhe.zh...@rivai.ai>
*Subject:* Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS
is a multiple of 2.
Few more background about RVV:
RISC-V has provide different VLEN configuration by different ISA
extension like `zve32x`, `zve64x` and `v`
zve32x just guarantee the minimal VLEN is 32 bits,
zve64x guarantee the minimal VLEN is 64 bits,
and v guarantee the minimal VLEN is 128 bits,
Current status (without that patch):
Zve32x: Mode for one vector register mode is VNx1SImode and VNx1DImode
is invalid mode
- one vector register could hold 1 + 1x SImode where x is 0~n, so it
might hold just one SI
Zve64x: Mode for one vector register mode is VNx1DImode or VNx2SImode
- one vector register could hold 1 + 1x DImode where x is 0~n, so it
might hold just one DI
- one vector register could hold 2 + 2x SImode where x is 0~n, so it
might hold just two SI
So what I want to say here is VNx1DImode is really NOT safe to assume
to have more than two DI in theory.
However `v` extension guarantees the minimal VLEN is 128 bits.
We are trying to introduce another type/mode mapping for this
configure:
v: Mode for one vector register mode is VNx2DImode or VNx4SImode
- one vector register could hold 2 + 2x DImode where x is 0~n, so it
will hold at least two DI
- one vector register could hold 4 + 4x SImode where x is 0~n, so it
will hold at least four DI
So GET_MODE_NUNITS for a single vector register with DI mode will
become 2 (VNx2DImode) if it is really possible, which is a more
precise way to model the vector extension for RISC-V .
On Tue, Apr 18, 2023 at 10:28 PM Kito Cheng 
wrote:
>
> Wait, VNx1DImode can be really evaluate to just one element if
> -march=rv64g_zve64x,
>
> I thinks this should be just fixed on backend by this patch:
>
>

https://patchwork.ozlabs.org/project/gcc/patch/20230414014518.15458-1-juzhe.zh...@rivai.ai/
>
> On Tue, Apr 18, 2023 at 2:12 PM Richard Biener via Gcc-patches
>  wrote:
> >
> > On Mon, Apr 17, 2023 at 8:42 PM Michael Collison
 wrote:
> > >
> > > While working on autovectorizing for the RISCV port I
encountered an issue
> > > where can_duplicate_and_interleave_p assumes that
GET_MODE_NUNITS is a
> > > evenly divisible by two. The RISC-V target has vector modes
(e.g. VNx1DImode),
> > > where GET_MODE_NUNITS is equal to one.
> > >
> > > Tested on RISCV and x86_64-linux-gnu. Okay?
> >
> > OK.
> >
> > > 2023-03-09  Michael Collison 
> > >
> > > * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> > > Check that GET_MODE_NUNITS is a multiple of 2.
> > > ---
> > >  gcc/tree-vect-slp.cc | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > > index d73deaecce0..a64fe454e19 100644
> > > --- a/gcc/tree-vect-slp.cc
> > > +++ b/gcc/tree-vect-slp.cc
> > > @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p
(vec_info *vinfo, unsigned int count,
> > > (GET_MODE_BITSIZE (int_mode), 1);
> > >   tree vector_type
> > > = get_vectype_for_scalar_type (vinfo, int_type,
count);
> > > + poly_int64 half_nelts;
> > >   if (vector_type
> > >   && VECTOR_MODE_P (TYPE_MODE (vector_type))
> > >   && known_eq (GET_MODE_SIZE (TYPE_MODE
(vector_type)),
> > > -  GET_MODE_SIZE (base_vector_mode)))
> > > +  GET_MODE_SIZE (base_vector_mode))
> > > + &

Re: Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[juzhe.zhong]

Yes, like kito said.
We won't enable VNx1DImode in auto-vectorization so it's meaningless to fix it 
here.
We dynamic adjust the minimum vector-length for different '-march' according to 
RVV ISA specification.
So we strongly suggest that we should drop this fix.

Thanks.


juzhe.zh...@rivai.ai
 
From: Kito Cheng
Date: 2023-04-19 02:21
To: Richard Biener; Jeff Law; Palmer Dabbelt
CC: Michael Collison; gcc-patches; 钟居哲
Subject: Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple 
of 2.
Few more background about RVV:
 
RISC-V has provide different VLEN configuration by different ISA
extension like `zve32x`, `zve64x` and `v`
zve32x just guarantee the minimal VLEN is 32 bits,
zve64x guarantee the minimal VLEN is 64 bits,
and v guarantee the minimal VLEN is 128 bits,
 
Current status (without that patch):
 
Zve32x: Mode for one vector register mode is VNx1SImode and VNx1DImode
is invalid mode
- one vector register could hold 1 + 1x SImode where x is 0~n, so it
might hold just one SI
 
Zve64x: Mode for one vector register mode is VNx1DImode or VNx2SImode
- one vector register could hold 1 + 1x DImode where x is 0~n, so it
might hold just one DI
- one vector register could hold 2 + 2x SImode where x is 0~n, so it
might hold just two SI
 
So what I want to say here is VNx1DImode is really NOT safe to assume
to have more than two DI in theory.
 
However `v` extension guarantees the minimal VLEN is 128 bits.
 
We are trying to introduce another type/mode mapping for this configure:
 
v: Mode for one vector register mode is VNx2DImode or VNx4SImode
- one vector register could hold 2 + 2x DImode where x is 0~n, so it
will hold at least two DI
- one vector register could hold 4 + 4x SImode where x is 0~n, so it
will hold at least four DI
 
So GET_MODE_NUNITS for a single vector register with DI mode will
become 2 (VNx2DImode) if it is really possible, which is a more
precise way to model the vector extension for RISC-V .
 
 
 
On Tue, Apr 18, 2023 at 10:28 PM Kito Cheng  wrote:
>
> Wait, VNx1DImode can be really evaluate to just one element if
> -march=rv64g_zve64x,
>
> I thinks this should be just fixed on backend by this patch:
>
> https://patchwork.ozlabs.org/project/gcc/patch/20230414014518.15458-1-juzhe.zh...@rivai.ai/
>
> On Tue, Apr 18, 2023 at 2:12 PM Richard Biener via Gcc-patches
>  wrote:
> >
> > On Mon, Apr 17, 2023 at 8:42 PM Michael Collison  
> > wrote:
> > >
> > > While working on autovectorizing for the RISCV port I encountered an issue
> > > where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> > > evenly divisible by two. The RISC-V target has vector modes (e.g. 
> > > VNx1DImode),
> > > where GET_MODE_NUNITS is equal to one.
> > >
> > > Tested on RISCV and x86_64-linux-gnu. Okay?
> >
> > OK.
> >
> > > 2023-03-09  Michael Collison  
> > >
> > > * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> > > Check that GET_MODE_NUNITS is a multiple of 2.
> > > ---
> > >  gcc/tree-vect-slp.cc | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > > index d73deaecce0..a64fe454e19 100644
> > > --- a/gcc/tree-vect-slp.cc
> > > +++ b/gcc/tree-vect-slp.cc
> > > @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > > unsigned int count,
> > > (GET_MODE_BITSIZE (int_mode), 1);
> > >   tree vector_type
> > > = get_vectype_for_scalar_type (vinfo, int_type, count);
> > > + poly_int64 half_nelts;
> > >   if (vector_type
> > >   && VECTOR_MODE_P (TYPE_MODE (vector_type))
> > >   && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> > > -  GET_MODE_SIZE (base_vector_mode)))
> > > +  GET_MODE_SIZE (base_vector_mode))
> > > + && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
> > > +2, _nelts))
> > > {
> > >   /* Try fusing consecutive sequences of COUNT / NVECTORS 
> > > elements
> > >  together into elements of type INT_TYPE and using the 
> > > result
> > > @@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > > unsigned int count,
> > >   poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE 
> > > (vector_type));
> > >   vec_perm_builder sel1 (nelts, 2, 3);
> > >   vec_perm_builder sel2 (nelts, 2, 3);
> > > - poly_int64 half_nelts = exact_div (nelts, 2);
> > > +
> > >   for (unsigned int i = 0; i < 3; ++i)
> > > {
> > >   sel1.quick_push (i);
> > > --
> > > 2.34.1
> > >
 

Re: [PATCH v3] vect: Verify that GET_MODE_UNITS is greater than one for vect_grouped_store_supported

[Jeff Law via Gcc-patches]

On 4/17/23 10:38, Kevin Lee wrote:

This patch properly guards gcc_assert (multiple_p (m_full_nelts,
m_npatterns)) in vec_perm_indices indices (sel, 2, nelt) for VNx1 vectors.

Based on the feedback from Richard Biener and Richard Sandiford,
multiple_p has been used instead of maybe_lt to compare nelt with the
minimum size 2.

Bootstrap and testing done on x86_64-pc-linux-gnu. Would this be ok for trunk?

Patch V1: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html
Patch V2: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614700.html
  
Kevin Lee 

gcc/ChangeLog:

* tree-vect-data-refs.cc (vect_grouped_store_supported): Add new
condition.

I fixed up the indentation and pushed this to the trunk.

jeff

Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Kito Cheng via Gcc-patches]

Few more background about RVV:

RISC-V has provide different VLEN configuration by different ISA
extension like `zve32x`, `zve64x` and `v`
zve32x just guarantee the minimal VLEN is 32 bits,
zve64x guarantee the minimal VLEN is 64 bits,
and v guarantee the minimal VLEN is 128 bits,

Current status (without that patch):

Zve32x: Mode for one vector register mode is VNx1SImode and VNx1DImode
is invalid mode
 - one vector register could hold 1 + 1x SImode where x is 0~n, so it
might hold just one SI

Zve64x: Mode for one vector register mode is VNx1DImode or VNx2SImode
 - one vector register could hold 1 + 1x DImode where x is 0~n, so it
might hold just one DI
 - one vector register could hold 2 + 2x SImode where x is 0~n, so it
might hold just two SI

So what I want to say here is VNx1DImode is really NOT safe to assume
to have more than two DI in theory.

However `v` extension guarantees the minimal VLEN is 128 bits.

We are trying to introduce another type/mode mapping for this configure:

v: Mode for one vector register mode is VNx2DImode or VNx4SImode
 - one vector register could hold 2 + 2x DImode where x is 0~n, so it
will hold at least two DI
 - one vector register could hold 4 + 4x SImode where x is 0~n, so it
will hold at least four DI

So GET_MODE_NUNITS for a single vector register with DI mode will
become 2 (VNx2DImode) if it is really possible, which is a more
precise way to model the vector extension for RISC-V .



On Tue, Apr 18, 2023 at 10:28 PM Kito Cheng  wrote:
>
> Wait, VNx1DImode can be really evaluate to just one element if
> -march=rv64g_zve64x,
>
> I thinks this should be just fixed on backend by this patch:
>
> https://patchwork.ozlabs.org/project/gcc/patch/20230414014518.15458-1-juzhe.zh...@rivai.ai/
>
> On Tue, Apr 18, 2023 at 2:12 PM Richard Biener via Gcc-patches
>  wrote:
> >
> > On Mon, Apr 17, 2023 at 8:42 PM Michael Collison  
> > wrote:
> > >
> > > While working on autovectorizing for the RISCV port I encountered an issue
> > > where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> > > evenly divisible by two. The RISC-V target has vector modes (e.g. 
> > > VNx1DImode),
> > > where GET_MODE_NUNITS is equal to one.
> > >
> > > Tested on RISCV and x86_64-linux-gnu. Okay?
> >
> > OK.
> >
> > > 2023-03-09  Michael Collison  
> > >
> > > * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> > > Check that GET_MODE_NUNITS is a multiple of 2.
> > > ---
> > >  gcc/tree-vect-slp.cc | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > > index d73deaecce0..a64fe454e19 100644
> > > --- a/gcc/tree-vect-slp.cc
> > > +++ b/gcc/tree-vect-slp.cc
> > > @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > > unsigned int count,
> > > (GET_MODE_BITSIZE (int_mode), 1);
> > >   tree vector_type
> > > = get_vectype_for_scalar_type (vinfo, int_type, count);
> > > + poly_int64 half_nelts;
> > >   if (vector_type
> > >   && VECTOR_MODE_P (TYPE_MODE (vector_type))
> > >   && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> > > -  GET_MODE_SIZE (base_vector_mode)))
> > > +  GET_MODE_SIZE (base_vector_mode))
> > > + && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
> > > +2, _nelts))
> > > {
> > >   /* Try fusing consecutive sequences of COUNT / NVECTORS 
> > > elements
> > >  together into elements of type INT_TYPE and using the 
> > > result
> > > @@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > > unsigned int count,
> > >   poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE 
> > > (vector_type));
> > >   vec_perm_builder sel1 (nelts, 2, 3);
> > >   vec_perm_builder sel2 (nelts, 2, 3);
> > > - poly_int64 half_nelts = exact_div (nelts, 2);
> > > +
> > >   for (unsigned int i = 0; i < 3; ++i)
> > > {
> > >   sel1.quick_push (i);
> > > --
> > > 2.34.1
> > >

Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Kito Cheng via Gcc-patches]

Wait, VNx1DImode can be really evaluate to just one element if
-march=rv64g_zve64x,

I thinks this should be just fixed on backend by this patch:

https://patchwork.ozlabs.org/project/gcc/patch/20230414014518.15458-1-juzhe.zh...@rivai.ai/

On Tue, Apr 18, 2023 at 2:12 PM Richard Biener via Gcc-patches
 wrote:
>
> On Mon, Apr 17, 2023 at 8:42 PM Michael Collison  
> wrote:
> >
> > While working on autovectorizing for the RISCV port I encountered an issue
> > where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> > evenly divisible by two. The RISC-V target has vector modes (e.g. 
> > VNx1DImode),
> > where GET_MODE_NUNITS is equal to one.
> >
> > Tested on RISCV and x86_64-linux-gnu. Okay?
>
> OK.
>
> > 2023-03-09  Michael Collison  
> >
> > * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> > Check that GET_MODE_NUNITS is a multiple of 2.
> > ---
> >  gcc/tree-vect-slp.cc | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > index d73deaecce0..a64fe454e19 100644
> > --- a/gcc/tree-vect-slp.cc
> > +++ b/gcc/tree-vect-slp.cc
> > @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > unsigned int count,
> > (GET_MODE_BITSIZE (int_mode), 1);
> >   tree vector_type
> > = get_vectype_for_scalar_type (vinfo, int_type, count);
> > + poly_int64 half_nelts;
> >   if (vector_type
> >   && VECTOR_MODE_P (TYPE_MODE (vector_type))
> >   && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> > -  GET_MODE_SIZE (base_vector_mode)))
> > +  GET_MODE_SIZE (base_vector_mode))
> > + && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
> > +2, _nelts))
> > {
> >   /* Try fusing consecutive sequences of COUNT / NVECTORS 
> > elements
> >  together into elements of type INT_TYPE and using the 
> > result
> > @@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> > unsigned int count,
> >   poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
> >   vec_perm_builder sel1 (nelts, 2, 3);
> >   vec_perm_builder sel2 (nelts, 2, 3);
> > - poly_int64 half_nelts = exact_div (nelts, 2);
> > +
> >   for (unsigned int i = 0; i < 3; ++i)
> > {
> >   sel1.quick_push (i);
> > --
> > 2.34.1
> >

Re: [PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Richard Biener via Gcc-patches]

On Mon, Apr 17, 2023 at 8:42 PM Michael Collison  wrote:
>
> While working on autovectorizing for the RISCV port I encountered an issue
> where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
> where GET_MODE_NUNITS is equal to one.
>
> Tested on RISCV and x86_64-linux-gnu. Okay?

OK.

> 2023-03-09  Michael Collison  
>
> * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> Check that GET_MODE_NUNITS is a multiple of 2.
> ---
>  gcc/tree-vect-slp.cc | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index d73deaecce0..a64fe454e19 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, 
> unsigned int count,
> (GET_MODE_BITSIZE (int_mode), 1);
>   tree vector_type
> = get_vectype_for_scalar_type (vinfo, int_type, count);
> + poly_int64 half_nelts;
>   if (vector_type
>   && VECTOR_MODE_P (TYPE_MODE (vector_type))
>   && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> -  GET_MODE_SIZE (base_vector_mode)))
> +  GET_MODE_SIZE (base_vector_mode))
> + && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
> +2, _nelts))
> {
>   /* Try fusing consecutive sequences of COUNT / NVECTORS elements
>  together into elements of type INT_TYPE and using the result
> @@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
> int count,
>   poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
>   vec_perm_builder sel1 (nelts, 2, 3);
>   vec_perm_builder sel2 (nelts, 2, 3);
> - poly_int64 half_nelts = exact_div (nelts, 2);
> +
>   for (unsigned int i = 0; i < 3; ++i)
> {
>   sel1.quick_push (i);
> --
> 2.34.1
>

Re: [PATCH v3] vect: Verify that GET_MODE_UNITS is greater than one for vect_grouped_store_supported

[Richard Biener via Gcc-patches]

On Mon, Apr 17, 2023 at 6:40 PM Kevin Lee  wrote:
>
> This patch properly guards gcc_assert (multiple_p (m_full_nelts,
> m_npatterns)) in vec_perm_indices indices (sel, 2, nelt) for VNx1 vectors.
>
> Based on the feedback from Richard Biener and Richard Sandiford,
> multiple_p has been used instead of maybe_lt to compare nelt with the
> minimum size 2.
>
> Bootstrap and testing done on x86_64-pc-linux-gnu. Would this be ok for trunk?

Yes, this is OK.

Thanks,
Richard.

> Patch V1: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html
> Patch V2: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614700.html
>
> Kevin Lee 
> gcc/ChangeLog:
>
> * tree-vect-data-refs.cc (vect_grouped_store_supported): Add new
> condition
> ---
>  gcc/tree-vect-data-refs.cc | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> index 8daf7bd7dd3..df393ba723d 100644
> --- a/gcc/tree-vect-data-refs.cc
> +++ b/gcc/tree-vect-data-refs.cc
> @@ -5399,6 +5399,8 @@ vect_grouped_store_supported (tree vectype, unsigned 
> HOST_WIDE_INT count)
>   poly_uint64 nelt = GET_MODE_NUNITS (mode);
>
>   /* The encoding has 2 interleaved stepped patterns.  */
> +if(!multiple_p (nelt, 2))
> +  return false;
>   vec_perm_builder sel (nelt, 2, 3);
>   sel.quick_grow (6);
>   for (i = 0; i < 3; i++)
> --
> 2.25.1
>

[PATCH v4 07/10] vect: Verify that GET_MODE_NUNITS is a multiple of 2.

[Michael Collison]

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is a multiple of 2.
---
 gcc/tree-vect-slp.cc | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index d73deaecce0..a64fe454e19 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -423,10 +423,13 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
(GET_MODE_BITSIZE (int_mode), 1);
  tree vector_type
= get_vectype_for_scalar_type (vinfo, int_type, count);
+ poly_int64 half_nelts;
  if (vector_type
  && VECTOR_MODE_P (TYPE_MODE (vector_type))
  && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
-  GET_MODE_SIZE (base_vector_mode)))
+  GET_MODE_SIZE (base_vector_mode))
+ && multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
+2, _nelts))
{
  /* Try fusing consecutive sequences of COUNT / NVECTORS elements
 together into elements of type INT_TYPE and using the result
@@ -434,7 +437,7 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
  poly_uint64 nelts = GET_MODE_NUNITS (TYPE_MODE (vector_type));
  vec_perm_builder sel1 (nelts, 2, 3);
  vec_perm_builder sel2 (nelts, 2, 3);
- poly_int64 half_nelts = exact_div (nelts, 2);
+
  for (unsigned int i = 0; i < 3; ++i)
{
  sel1.quick_push (i);
-- 
2.34.1

[PATCH v3] vect: Verify that GET_MODE_UNITS is greater than one for vect_grouped_store_supported

[Kevin Lee]

This patch properly guards gcc_assert (multiple_p (m_full_nelts, 
m_npatterns)) in vec_perm_indices indices (sel, 2, nelt) for VNx1 vectors.

Based on the feedback from Richard Biener and Richard Sandiford,
multiple_p has been used instead of maybe_lt to compare nelt with the
minimum size 2.

Bootstrap and testing done on x86_64-pc-linux-gnu. Would this be ok for trunk?

Patch V1: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html
Patch V2: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614700.html
 
Kevin Lee 
gcc/ChangeLog:

* tree-vect-data-refs.cc (vect_grouped_store_supported): Add new
condition
---
 gcc/tree-vect-data-refs.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index 8daf7bd7dd3..df393ba723d 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -5399,6 +5399,8 @@ vect_grouped_store_supported (tree vectype, unsigned 
HOST_WIDE_INT count)
  poly_uint64 nelt = GET_MODE_NUNITS (mode);
 
  /* The encoding has 2 interleaved stepped patterns.  */
+if(!multiple_p (nelt, 2))
+  return false;
  vec_perm_builder sel (nelt, 2, 3);
  sel.quick_grow (6);
  for (i = 0; i < 3; i++)
-- 
2.25.1

Re: [PATCH v2][RFC] vect: Verify that GET_MODE_NUNITS is greater than one for vect_grouped_store_supported

[Kevin Lee]

Thank you for the feedback Richard and Richard.

> Note the calls are guarded with
>
>   && ! known_eq (TYPE_VECTOR_SUBPARTS (vectype), 1U)

Yes, I believe nelt.is_constant() wouldn't be necessary. I didn't realize
the call was guarded by this condition.

> But I think the better check for location above is:
>
>if (!multiple_p (nelt, 2))
> return false;
>
> which then guards the assert in the later exact_div (nelt, 2).

I believe this check is better than using maybe_lt because it properly
guards exact_div(nelt, 2) and vec_perm_builder sel(nelt, 2, 3).
I'll modify the patch accordingly, build, test and submit the patch. Thank
you!!

Sincerely,

Re: [PATCH v2][RFC] vect: Verify that GET_MODE_NUNITS is greater than one for vect_grouped_store_supported

[Richard Sandiford via Gcc-patches]

Richard Biener via Gcc-patches  writes:
> On Mon, Mar 27, 2023 at 6:02 PM Kevin Lee  wrote:
>>
>> This patch is a proper fix to the previous patch
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html
>> vect_grouped_store_supported checks if the count is a power of 2, but
>> doesn't check the size of the GET_MODE_NUNITS.
>> This should handle the riscv case where the mode is VNx1DI since the
>> nelt would be {1, 1}.
>> It was tested on RISCV and x86_64-linux-gnu. Would this be correct
>> for the vectors with size smaller than 2?
>>
>> ---
>>  gcc/tree-vect-data-refs.cc | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
>> index 8daf7bd7dd3..04ad12f7d04 100644
>> --- a/gcc/tree-vect-data-refs.cc
>> +++ b/gcc/tree-vect-data-refs.cc
>> @@ -5399,6 +5399,8 @@ vect_grouped_store_supported (tree vectype, unsigned 
>> HOST_WIDE_INT count)
>>   poly_uint64 nelt = GET_MODE_NUNITS (mode);
>>
>>   /* The encoding has 2 interleaved stepped patterns.  */
>> +if(!nelt.is_constant() && maybe_lt(nelt, (unsigned int) 2))
>> +  return false;
>
> Indentation is off (or your MUA is broken).  I think the nelt.is_constant ()
> check is superfluous but with constant nelt we'd never end up with a
> grouped store.
>
> Note the calls are guarded with
>
>  && ! known_eq (TYPE_VECTOR_SUBPARTS (vectype), 1U)
>
> maybe the better fix is to change those to ! maybe_eq?

I think the point of those checks is that a grouped store of N 1-element
vectors is equivalent to a store of N scalars.  Nothing needs to happen
internally within the vectors.

For a grouped store of VNx1 vectors, some permutation would be needed.
But it's difficult to generate code for that case, because the minimum
size reduces to two scalars while larger sizes need normal interleaves.

But I think the better check for location above is:

   if (!multiple_p (nelt, 2))
 return false;

which then guards the assert in the later exact_div (nelt, 2).

Thanks,
Richard

Re: [PATCH v2][RFC] vect: Verify that GET_MODE_NUNITS is greater than one for vect_grouped_store_supported

[Richard Biener via Gcc-patches]

On Mon, Mar 27, 2023 at 6:02 PM Kevin Lee  wrote:
>
> This patch is a proper fix to the previous patch
> https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html
> vect_grouped_store_supported checks if the count is a power of 2, but
> doesn't check the size of the GET_MODE_NUNITS.
> This should handle the riscv case where the mode is VNx1DI since the
> nelt would be {1, 1}.
> It was tested on RISCV and x86_64-linux-gnu. Would this be correct
> for the vectors with size smaller than 2?
>
> ---
>  gcc/tree-vect-data-refs.cc | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> index 8daf7bd7dd3..04ad12f7d04 100644
> --- a/gcc/tree-vect-data-refs.cc
> +++ b/gcc/tree-vect-data-refs.cc
> @@ -5399,6 +5399,8 @@ vect_grouped_store_supported (tree vectype, unsigned 
> HOST_WIDE_INT count)
>   poly_uint64 nelt = GET_MODE_NUNITS (mode);
>
>   /* The encoding has 2 interleaved stepped patterns.  */
> +if(!nelt.is_constant() && maybe_lt(nelt, (unsigned int) 2))
> +  return false;

Indentation is off (or your MUA is broken).  I think the nelt.is_constant ()
check is superfluous but with constant nelt we'd never end up with a
grouped store.

Note the calls are guarded with

 && ! known_eq (TYPE_VECTOR_SUBPARTS (vectype), 1U)

maybe the better fix is to change those to ! maybe_eq?

Richard should know best here.

Richard.

>   vec_perm_builder sel (nelt, 2, 3);
>   sel.quick_grow (6);
>   for (i = 0; i < 3; i++)
> --
> 2.25.1
>

Ping: [PATCH v2][RFC] vect: Verify that GET_MODE_NUNITS is greater than one for vect_grouped_store_supported

[Kevin Lee]

May I ping this patch?
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614700.html
Any suggestions and comments would be appreciated. Thank you!

Sincerely,
Kevin Lee

Re: [PATCH] vect: Verify that GET_MODE_NUNITS is greater than one.

[Richard Sandiford via Gcc-patches]

Michael Collison  writes:
> While working on autovectorizing for the RISCV port I encountered an issue
> where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
> where GET_MODE_NUNITS is equal to one.
>
> Tested on RISCV and x86_64-linux-gnu. Okay?
>
> 2023-03-09  Michael Collison  
>
>   * tree-vect-slp.cc (can_duplicate_and_interleave_p):
>   Check that GET_MODE_NUNITS is greater than one.
> ---
>  gcc/tree-vect-slp.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 9a4e000925e..add58113fa8 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -426,7 +426,8 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
> int count,
> if (vector_type
> && VECTOR_MODE_P (TYPE_MODE (vector_type))
> && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> -GET_MODE_SIZE (base_vector_mode)))
> +GET_MODE_SIZE (base_vector_mode))
> +   && known_gt (GET_MODE_NUNITS (TYPE_MODE (vector_type)), 1))
>   {
> /* Try fusing consecutive sequences of COUNT / NVECTORS elements
>together into elements of type INT_TYPE and using the result

FWIW, I think it'd better to remove:

  poly_int64 half_nelts = exact_div (nelts, 2);

declare:

  poly_uint64 half_nelts;

before the if condition, and use:

&& multiple_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)),
   2, _nelts)

instead of the known_gt.  In other words, now that we can't assert
the exact_div, we should check it (using multiple_p) instead.

Thanks,
Richard

[PATCH v2][RFC] vect: Verify that GET_MODE_NUNITS is greater than one for vect_grouped_store_supported

[Kevin Lee]

This patch is a proper fix to the previous patch 
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614463.html 
vect_grouped_store_supported checks if the count is a power of 2, but
doesn't check the size of the GET_MODE_NUNITS.
This should handle the riscv case where the mode is VNx1DI since the
nelt would be {1, 1}. 
It was tested on RISCV and x86_64-linux-gnu. Would this be correct 
for the vectors with size smaller than 2?

---
 gcc/tree-vect-data-refs.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index 8daf7bd7dd3..04ad12f7d04 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -5399,6 +5399,8 @@ vect_grouped_store_supported (tree vectype, unsigned 
HOST_WIDE_INT count)
  poly_uint64 nelt = GET_MODE_NUNITS (mode);
 
  /* The encoding has 2 interleaved stepped patterns.  */
+if(!nelt.is_constant() && maybe_lt(nelt, (unsigned int) 2))
+  return false;
  vec_perm_builder sel (nelt, 2, 3);
  sel.quick_grow (6);
  for (i = 0; i < 3; i++)
-- 
2.25.1

[RFC][Patch] vect: verify that nelt is greater than one

[Kevin Lee]

This is a patch related to 
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613977.html, 
aiming for gcc14. Since the RISC-V target has vector modes (e.g. VNx1DImode)
with nelt smaller than 2, npat has to match with the nelt to create proper 
vec_perm_indices. 

I tested on x86_64-linux-gnu and didn't cause more failures, but wasn't sure if 
total_elem would be used in the rest of the function. Should there be additional
changes in the vect_grouped_store_supported? Thank you!

gcc/ChangeLog:
Kevin Lee 
* tree-vect-data-refs.cc (vect_grouped_store_supported): Check
if the nelt is greater than one.
---
 gcc/tree-vect-data-refs.cc | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index 8daf7bd7dd3..9c09cc973d0 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -5399,17 +5399,20 @@ vect_grouped_store_supported (tree vectype, unsigned 
HOST_WIDE_INT count)
  poly_uint64 nelt = GET_MODE_NUNITS (mode);
 
  /* The encoding has 2 interleaved stepped patterns.  */
- vec_perm_builder sel (nelt, 2, 3);
- sel.quick_grow (6);
+
+unsigned int npat = known_gt(nelt, (unsigned int) 1) ? 2 : 1;
+unsigned int total_elem = npat * 3;
+ vec_perm_builder sel (nelt, npat, 3);
+ sel.quick_grow (total_elem);
  for (i = 0; i < 3; i++)
{
- sel[i * 2] = i;
- sel[i * 2 + 1] = i + nelt;
+ sel[i * npat] = i;
+ sel[i * npat + 1] = i + nelt;
}
  vec_perm_indices indices (sel, 2, nelt);
  if (can_vec_perm_const_p (mode, mode, indices))
{
- for (i = 0; i < 6; i++)
+ for (i = 0; i < total_elem; i++)
sel[i] += exact_div (nelt, 2);
  indices.new_vector (sel, 2, nelt);
  if (can_vec_perm_const_p (mode, mode, indices))
-- 
2.25.1

Re: [PATCH] vect: Verify that GET_MODE_NUNITS is greater than one.

[Jeff Law via Gcc-patches]

On 3/14/23 15:52, Michael Collison wrote:

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is greater than one.
As far as I know this doesn't fix a regression so I would defer to 
gc-14.  As release managers, Richi, Jakub or Joseph can gate it in as an 
exception.


jeff

[PATCH] vect: Verify that GET_MODE_NUNITS is greater than one.

[Michael Collison]

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
evenly divisible by two. The RISC-V target has vector modes (e.g. VNx1DImode),
where GET_MODE_NUNITS is equal to one.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is greater than one.
---
 gcc/tree-vect-slp.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 9a4e000925e..add58113fa8 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -426,7 +426,8 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
  if (vector_type
  && VECTOR_MODE_P (TYPE_MODE (vector_type))
  && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
-  GET_MODE_SIZE (base_vector_mode)))
+  GET_MODE_SIZE (base_vector_mode))
+ && known_gt (GET_MODE_NUNITS (TYPE_MODE (vector_type)), 1))
{
  /* Try fusing consecutive sequences of COUNT / NVECTORS elements
 together into elements of type INT_TYPE and using the result
-- 
2.34.1

Re: [PATCH] vect: Verify that GET_MODE_NUNITS is power-of-2

[Richard Biener via Gcc-patches]

On Fri, Mar 10, 2023 at 9:16 PM Michael Collison  wrote:
>
> While working on autovectorizing for the RISCV port I encountered an issue
> where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
> power of two. The RISC-V target has vector modes (e.g. VNx1DImode) that
> are not a power of two.

We do not support vector types that do not have a power-of-two
element count, see TYPE_VECTOR_SUBPARTS.

Also your test below verifies that nunits is divisible by two, not that it
is power-of-two?  So maybe what you want to know is whether
known_gt (nunits, 1)?

> Tested on RISCV and x86_64-linux-gnu. Okay?
>
> 2023-03-09  Michael Collison  
>
> * poly-int.h (exact_div_p): New function to
> verify that argument is a power of 2 poly_int.
> * tree-vect-slp.cc (can_duplicate_and_interleave_p):
> Check that GET_MODE_NUNITS is a power of 2.
> ---
>  gcc/poly-int.h   | 17 +
>  gcc/tree-vect-slp.cc |  3 ++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/poly-int.h b/gcc/poly-int.h
> index 12571455081..d09632f341f 100644
> --- a/gcc/poly-int.h
> +++ b/gcc/poly-int.h
> @@ -2219,6 +2219,23 @@ multiple_p (const poly_int_pod , const 
> poly_int_pod ,
>return constant_multiple_p (a, b, multiple);
>  }
>
> +/* Return true, if A is known to be a multiple of B.  */
> +
> +template
> +inline bool
> +exact_div_p (const poly_int_pod , Cb b)
> +{
> +  typedef POLY_CONST_COEFF (Ca, Cb) C;
> +  poly_int r;
> +  for (unsigned int i = 0; i < N; i++)
> +{
> +  if ((a.coeffs[i] % b) != 0)
> +   return false;
> +
> +}
> +  return true;
> +}
> +
>  /* Return A / B, given that A is known to be a multiple of B.  */
>
>  template
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 9a4e000925e..6be2036a13a 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -426,7 +426,8 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
> int count,
>   if (vector_type
>   && VECTOR_MODE_P (TYPE_MODE (vector_type))
>   && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
> -  GET_MODE_SIZE (base_vector_mode)))
> +  GET_MODE_SIZE (base_vector_mode))
> + && exact_div_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)), 2))
> {
>   /* Try fusing consecutive sequences of COUNT / NVECTORS elements
>  together into elements of type INT_TYPE and using the result
> --
> 2.34.1
>

[PATCH] vect: Verify that GET_MODE_NUNITS is power-of-2

[Michael Collison]

While working on autovectorizing for the RISCV port I encountered an issue
where can_duplicate_and_interleave_p assumes that GET_MODE_NUNITS is a
power of two. The RISC-V target has vector modes (e.g. VNx1DImode) that
are not a power of two.

Tested on RISCV and x86_64-linux-gnu. Okay?

2023-03-09  Michael Collison  

* poly-int.h (exact_div_p): New function to
verify that argument is a power of 2 poly_int.
* tree-vect-slp.cc (can_duplicate_and_interleave_p):
Check that GET_MODE_NUNITS is a power of 2.
---
 gcc/poly-int.h   | 17 +
 gcc/tree-vect-slp.cc |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/gcc/poly-int.h b/gcc/poly-int.h
index 12571455081..d09632f341f 100644
--- a/gcc/poly-int.h
+++ b/gcc/poly-int.h
@@ -2219,6 +2219,23 @@ multiple_p (const poly_int_pod , const 
poly_int_pod ,
   return constant_multiple_p (a, b, multiple);
 }
 
+/* Return true, if A is known to be a multiple of B.  */
+
+template
+inline bool
+exact_div_p (const poly_int_pod , Cb b)
+{
+  typedef POLY_CONST_COEFF (Ca, Cb) C;
+  poly_int r;
+  for (unsigned int i = 0; i < N; i++)
+{
+  if ((a.coeffs[i] % b) != 0)
+   return false;
+
+}
+  return true;
+}
+
 /* Return A / B, given that A is known to be a multiple of B.  */
 
 template
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 9a4e000925e..6be2036a13a 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -426,7 +426,8 @@ can_duplicate_and_interleave_p (vec_info *vinfo, unsigned 
int count,
  if (vector_type
  && VECTOR_MODE_P (TYPE_MODE (vector_type))
  && known_eq (GET_MODE_SIZE (TYPE_MODE (vector_type)),
-  GET_MODE_SIZE (base_vector_mode)))
+  GET_MODE_SIZE (base_vector_mode))
+ && exact_div_p (GET_MODE_NUNITS (TYPE_MODE (vector_type)), 2))
{
  /* Try fusing consecutive sequences of COUNT / NVECTORS elements
 together into elements of type INT_TYPE and using the result
-- 
2.34.1

Document/verify another aspect of OpenACC 'async' semantics in 'libgomp.oacc-c-c++-common/data-3.c'

[Thomas Schwinge]

Hi!

In order to document/verify one aspect of OpenACC 'async' semantics, I've
pushed to master branch commit 442d51a20ef13a8e6c080ca30bc37fc93b6bfac4
"Document/verify another aspect of OpenACC 'async' semantics in 
'libgomp.oacc-c-c++-common/data-3.c'",
see attached.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 442d51a20ef13a8e6c080ca30bc37fc93b6bfac4 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Fri, 24 Feb 2023 16:21:31 +0100
Subject: [PATCH] Document/verify another aspect of OpenACC 'async' semantics
 in 'libgomp.oacc-c-c++-common/data-3.c'

... that I almost broke with later implementation changes.

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/data-3.c: Document/verify
	another aspect of OpenACC 'async' semantics.
---
 libgomp/testsuite/libgomp.oacc-c-c++-common/data-3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/data-3.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/data-3.c
index 5ec50b808a7..c422cbcd325 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/data-3.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/data-3.c
@@ -144,8 +144,8 @@ main (int argc, char **argv)
 
 #pragma acc exit data copyout (a[0:N]) copyout (b[0:N]) copyout (c[0:N]) \
   copyout (d[0:N]) copyout (e[0:N]) wait (1, 2, 3, 4) async (1)
-#pragma acc exit data delete (N)
-#pragma acc wait (1)
+#pragma acc exit data delete (N) wait(1) async(2)
+#pragma acc wait (2)
 
   for (i = 0; i < N; i++)
 {
-- 
2.25.1

[og12] Clarify/verify OpenMP 'omp_calloc' zero-initialization for pinned memory (was: [PATCH] libgomp, openmp: pinned memory)

[Thomas Schwinge]

Hi!

On 2022-01-13T13:53:03+, Andrew Stubbs  wrote:
> Pinned memory is allocated via mmap

> --- /dev/null
> +++ b/libgomp/config/linux/allocator.c

> +static void *
> +linux_memspace_calloc (omp_memspace_handle_t memspace, size_t size, int pin)
> +{
> +  if (pin)
> +return linux_memspace_alloc (memspace, size, pin);
> +[...]

This confused me for a moment, why we don't have to manually
zero-initialize here.  I've pushed to devel/omp/gcc-12 branch
commit 57b8f0600262566cd4f1ab12bf1bdafb29dbdc34
"Clarify/verify OpenMP 'omp_calloc' zero-initialization for pinned memory",
see attached.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 57b8f0600262566cd4f1ab12bf1bdafb29dbdc34 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 15 Feb 2023 10:23:03 +0100
Subject: [PATCH] Clarify/verify OpenMP 'omp_calloc' zero-initialization for
 pinned memory

Clarification for og12 commit ab7520b3b4cd9fdabfd63652badde478955bd3b5
"libgomp: pinned memory".  No functional change.

	libgomp/
	* config/linux/allocator.c (linux_memspace_alloc)
	(linux_memspace_calloc): Clarify zero-initialization for pinned
	memory.
	* testsuite/libgomp.c/alloc-pinned-1.c: Verify zero-initialization
	for pinned memory.
	* testsuite/libgomp.c/alloc-pinned-2.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-3.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-4.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-5.c: Likewise.
---
 libgomp/ChangeLog.omp| 10 ++
 libgomp/config/linux/allocator.c |  2 ++
 libgomp/testsuite/libgomp.c/alloc-pinned-1.c | 10 ++
 libgomp/testsuite/libgomp.c/alloc-pinned-2.c | 10 ++
 libgomp/testsuite/libgomp.c/alloc-pinned-3.c |  9 +
 libgomp/testsuite/libgomp.c/alloc-pinned-4.c |  9 +
 libgomp/testsuite/libgomp.c/alloc-pinned-5.c | 10 ++
 7 files changed, 60 insertions(+)

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 1c4b1833c0b..530f5c6acf6 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,5 +1,15 @@
 2023-02-16  Thomas Schwinge  
 
+	* config/linux/allocator.c (linux_memspace_alloc)
+	(linux_memspace_calloc): Clarify zero-initialization for pinned
+	memory.
+	* testsuite/libgomp.c/alloc-pinned-1.c: Verify zero-initialization
+	for pinned memory.
+	* testsuite/libgomp.c/alloc-pinned-2.c: Likewise.
+	* testsuite/libgomp.c/alloc-pinned-3.c: Likewise.
+	* testsuite/libgomp.c/alloc-pinned-4.c: Likewise.
+	* testsuite/libgomp.c/alloc-pinned-5.c: Likewise.
+
 	* config/linux/allocator.c (linux_memspace_calloc): Elide
 	(innocuous) duplicate 'if' condition.
 	* config/nvptx/allocator.c (nvptx_memspace_free): Explicitly
diff --git a/libgomp/config/linux/allocator.c b/libgomp/config/linux/allocator.c
index 8a9171c36df..f278e5cdf14 100644
--- a/libgomp/config/linux/allocator.c
+++ b/libgomp/config/linux/allocator.c
@@ -65,6 +65,7 @@ linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin)
 }
   else if (pin)
 {
+  /* 'mmap' zero-initializes, which 'linux_memspace_calloc' relies on.  */
   void *addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
 			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (addr == MAP_FAILED)
@@ -96,6 +97,7 @@ linux_memspace_calloc (omp_memspace_handle_t memspace, size_t size, int pin)
   return ret;
 }
   else if (pin)
+/* If PINned, 'linux_memspace_alloc' 'mmap's, which zero-initializes.  */
 return linux_memspace_alloc (memspace, size, pin);
   else
 return calloc (1, size);
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
index 79792b16d83..fb7ac8b0080 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
@@ -54,6 +54,14 @@ get_pinned_mem ()
 }
 #endif
 
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+if (p[i] != 0)
+  abort ();
+}
+
 #include 
 
 int
@@ -91,5 +99,7 @@ main ()
   if (get_pinned_mem () <= amount2)
 abort ();
 
+  verify0 (p, SIZE);
+
   return 0;
 }
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-2.c b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
index 228c656b715..651b89fb42f 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
@@ -54,6 +54,14 @@ get_pinned_mem ()
 }
 #endif
 
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+if (p[i] != 0)
+  abort ();
+}
+
 #include 
 
 int
@@ -97,5 +105,7 @@ main ()
   if (get_pinned_mem () <= amount2)
 abort ();
 
+  verify0 (p, SIZE);
+
   return 0;
 }
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-3.c b/

Re: PING^5 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

On 2022-11-18 09:14, Richard Biener wrote:

On Thu, Nov 17, 2022 at 6:09 PM Torbjorn SVENSSON via Gcc-patches
 wrote:


Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604895.html

Ok for trunk?


OK.


Pushed.




Kind regards,
Torbjörn

On 2022-11-02 19:13, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602844.html

Ok for trunk?

Kind regards,
Torbjörn

On 2022-10-25 16:24, Torbjorn SVENSSON via Gcc-patches wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603544.html

Kind regards,
Torbjörn

On 2022-10-14 09:42, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html

Kind regards,
Torbjörn

On 2022-10-05 11:17, Torbjorn SVENSSON wrote:

Hi,

Ping,
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html

Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

 * lib/target-supports.exp (check_is_prog_name_available):
 New.
 * lib/target-supports-dg.exp
 (dg-require-prog-name-available): New.
 * g++.dg/modules/modules.exp: Verify avilability of module
 mapper.

Signed-off-by: Torbjörn SVENSSON  
---
   gcc/testsuite/g++.dg/modules/modules.exp | 31

   gcc/testsuite/lib/target-supports-dg.exp | 15 
   gcc/testsuite/lib/target-supports.exp| 15 
   3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp
b/gcc/testsuite/g++.dg/modules/modules.exp
index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
   return $option_list
   }
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+foreach test $tests {
+set tmp [dg-get-options $test]
+foreach op $tmp {
+switch [lindex $op 0] {
+"dg-additional-options" {
+# Example strings to match:
+# -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\
-t\\ [srcdir]/inc-xlate-1.map
+# -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)}
[lindex $op 2] dummy dummy2 prog] {
+verbose "Checking that mapper exist: $prog"
+if { ![ check_is_prog_name_available $prog ] } {
+return 0
+}
+}
+}
+}
+}
+}
+return 1
+}
+
   # cleanup any detritus from previous run
   cleanup_module_files [find $DEFAULT_REPO *.gcm]
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir
{*_a.[CHX}]] {
   set tests [lsort [find [file dirname $src] \
 [regsub {_a.[CHX]$} [file tail $src]
{_[a-z].[CHX]}]]]
+if { ![module-check-requirements $tests] } {
+set testcase [regsub {_a.[CH]} $src {}]
+set testcase \
+[string range $testcase [string length "$srcdir/"] end]
+unsupported $testcase
+continue
+}
+
   set std_list [module-init $src]
   foreach std $std_list {
   set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp
b/gcc/testsuite/lib/target-supports-dg.exp
index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
   set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
   }
   }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+# The args are within another list; pull them out.
+set args [lindex $args 0]
+
+set prog [lindex $args 1]
+
+if { ![ check_is_prog_name_available $prog ] } {
+upvar dg-do-what dg-do-what
+set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+}
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
   .byte 0
 } ""]
   }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+global tool
+
+set options [list "additional_flags=-print-prog-name=$prog"]
+set output [lindex [${tool}_target_compile "" "" "none"
$options] 0]
+
+if { $output == $prog } {
+return 0
+}
+
+return 1
+}

Re: PING^5 [PATCH] testsuite: Verify that module-mapper is available

[Richard Biener via Gcc-patches]

On Thu, Nov 17, 2022 at 6:09 PM Torbjorn SVENSSON via Gcc-patches
 wrote:
>
> Hi,
>
> Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604895.html
>
> Ok for trunk?

OK.

> Kind regards,
> Torbjörn
>
> On 2022-11-02 19:13, Torbjorn SVENSSON wrote:
> > Hi,
> >
> > Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602844.html
> >
> > Ok for trunk?
> >
> > Kind regards,
> > Torbjörn
> >
> > On 2022-10-25 16:24, Torbjorn SVENSSON via Gcc-patches wrote:
> >> Hi,
> >>
> >> Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603544.html
> >>
> >> Kind regards,
> >> Torbjörn
> >>
> >> On 2022-10-14 09:42, Torbjorn SVENSSON wrote:
> >>> Hi,
> >>>
> >>> Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html
> >>>
> >>> Kind regards,
> >>> Torbjörn
> >>>
> >>> On 2022-10-05 11:17, Torbjorn SVENSSON wrote:
> >>>> Hi,
> >>>>
> >>>> Ping,
> >>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html
> >>>>
> >>>> Kind regards,
> >>>> Torbjörn
> >>>>
> >>>> On 2022-09-23 14:03, Torbjörn SVENSSON wrote:
> >>>>> For some test cases, it's required that the optional module mapper
> >>>>> "g++-mapper-server" is built. As the server is not required, the
> >>>>> test cases will fail if it can't be found.
> >>>>>
> >>>>> gcc/testsuite/ChangeLog:
> >>>>>
> >>>>> * lib/target-supports.exp (check_is_prog_name_available):
> >>>>> New.
> >>>>> * lib/target-supports-dg.exp
> >>>>> (dg-require-prog-name-available): New.
> >>>>> * g++.dg/modules/modules.exp: Verify avilability of module
> >>>>> mapper.
> >>>>>
> >>>>> Signed-off-by: Torbjörn SVENSSON  
> >>>>> ---
> >>>>>   gcc/testsuite/g++.dg/modules/modules.exp | 31
> >>>>> 
> >>>>>   gcc/testsuite/lib/target-supports-dg.exp | 15 
> >>>>>   gcc/testsuite/lib/target-supports.exp| 15 
> >>>>>   3 files changed, 61 insertions(+)
> >>>>>
> >>>>> diff --git a/gcc/testsuite/g++.dg/modules/modules.exp
> >>>>> b/gcc/testsuite/g++.dg/modules/modules.exp
> >>>>> index afb323d0efd..4784803742a 100644
> >>>>> --- a/gcc/testsuite/g++.dg/modules/modules.exp
> >>>>> +++ b/gcc/testsuite/g++.dg/modules/modules.exp
> >>>>> @@ -279,6 +279,29 @@ proc module-init { src } {
> >>>>>   return $option_list
> >>>>>   }
> >>>>> +# Return 1 if requirements are met
> >>>>> +proc module-check-requirements { tests } {
> >>>>> +foreach test $tests {
> >>>>> +set tmp [dg-get-options $test]
> >>>>> +foreach op $tmp {
> >>>>> +switch [lindex $op 0] {
> >>>>> +"dg-additional-options" {
> >>>>> +# Example strings to match:
> >>>>> +# -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\
> >>>>> -t\\ [srcdir]/inc-xlate-1.map
> >>>>> +# -fmodules-ts -fmodule-mapper=|@g++-mapper-server
> >>>>> +if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)}
> >>>>> [lindex $op 2] dummy dummy2 prog] {
> >>>>> +verbose "Checking that mapper exist: $prog"
> >>>>> +if { ![ check_is_prog_name_available $prog ] } {
> >>>>> +return 0
> >>>>> +}
> >>>>> +}
> >>>>> +}
> >>>>> +}
> >>>>> +}
> >>>>> +}
> >>>>> +return 1
> >>>>> +}
> >>>>> +
> >>>>>   # cleanup any detritus from previous run
> >>>>>   cleanup_module_files [find $DEFAULT_REPO *.gcm]
> >>>>> @@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir
> >>>>> {*_a.[CHX}]] {
> >>>>>   se

PING^5 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604895.html

Ok for trunk?

Kind regards,
Torbjörn

On 2022-11-02 19:13, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602844.html

Ok for trunk?

Kind regards,
Torbjörn

On 2022-10-25 16:24, Torbjorn SVENSSON via Gcc-patches wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603544.html

Kind regards,
Torbjörn

On 2022-10-14 09:42, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html

Kind regards,
Torbjörn

On 2022-10-05 11:17, Torbjorn SVENSSON wrote:

Hi,

Ping, 
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html


Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
  gcc/testsuite/g++.dg/modules/modules.exp | 31 


  gcc/testsuite/lib/target-supports-dg.exp | 15 
  gcc/testsuite/lib/target-supports.exp    | 15 
  3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp

index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
  return $option_list
  }
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+    foreach test $tests {
+    set tmp [dg-get-options $test]
+    foreach op $tmp {
+    switch [lindex $op 0] {
+    "dg-additional-options" {
+    # Example strings to match:
+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ 
-t\\ [srcdir]/inc-xlate-1.map

+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+    if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} 
[lindex $op 2] dummy dummy2 prog] {

+    verbose "Checking that mapper exist: $prog"
+    if { ![ check_is_prog_name_available $prog ] } {
+    return 0
+    }
+    }
+    }
+    }
+    }
+    }
+    return 1
+}
+
  # cleanup any detritus from previous run
  cleanup_module_files [find $DEFAULT_REPO *.gcm]
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir 
{*_a.[CHX}]] {

  set tests [lsort [find [file dirname $src] \
    [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]

+    if { ![module-check-requirements $tests] } {
+    set testcase [regsub {_a.[CH]} $src {}]
+    set testcase \
+    [string range $testcase [string length "$srcdir/"] end]
+    unsupported $testcase
+    continue
+    }
+
  set std_list [module-init $src]
  foreach std $std_list {
  set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp

index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
  set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
  }
  }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+    # The args are within another list; pull them out.
+    set args [lindex $args 0]
+
+    set prog [lindex $args 1]
+
+    if { ![ check_is_prog_name_available $prog ] } {
+    upvar dg-do-what dg-do-what
+    set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+    }
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp

index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
  .byte 0
    } ""]
  }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+    global tool
+
+    set options [list "additional_flags=-print-prog-name=$prog"]
+    set output [lindex [${tool}_target_compile "" "" "none" 
$options] 0]

+
+    if { $output == $prog } {
+    return 0
+    }
+
+    return 1
+}

Re: PING^4 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602844.html

Ok for trunk?

Kind regards,
Torbjörn

On 2022-10-25 16:24, Torbjorn SVENSSON via Gcc-patches wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603544.html

Kind regards,
Torbjörn

On 2022-10-14 09:42, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html

Kind regards,
Torbjörn

On 2022-10-05 11:17, Torbjorn SVENSSON wrote:

Hi,

Ping, 
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html


Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
  gcc/testsuite/g++.dg/modules/modules.exp | 31 


  gcc/testsuite/lib/target-supports-dg.exp | 15 
  gcc/testsuite/lib/target-supports.exp    | 15 
  3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp

index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
  return $option_list
  }
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+    foreach test $tests {
+    set tmp [dg-get-options $test]
+    foreach op $tmp {
+    switch [lindex $op 0] {
+    "dg-additional-options" {
+    # Example strings to match:
+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ 
-t\\ [srcdir]/inc-xlate-1.map

+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+    if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} 
[lindex $op 2] dummy dummy2 prog] {

+    verbose "Checking that mapper exist: $prog"
+    if { ![ check_is_prog_name_available $prog ] } {
+    return 0
+    }
+    }
+    }
+    }
+    }
+    }
+    return 1
+}
+
  # cleanup any detritus from previous run
  cleanup_module_files [find $DEFAULT_REPO *.gcm]
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir 
{*_a.[CHX}]] {

  set tests [lsort [find [file dirname $src] \
    [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]

+    if { ![module-check-requirements $tests] } {
+    set testcase [regsub {_a.[CH]} $src {}]
+    set testcase \
+    [string range $testcase [string length "$srcdir/"] end]
+    unsupported $testcase
+    continue
+    }
+
  set std_list [module-init $src]
  foreach std $std_list {
  set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp

index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
  set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
  }
  }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+    # The args are within another list; pull them out.
+    set args [lindex $args 0]
+
+    set prog [lindex $args 1]
+
+    if { ![ check_is_prog_name_available $prog ] } {
+    upvar dg-do-what dg-do-what
+    set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+    }
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp

index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
  .byte 0
    } ""]
  }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+    global tool
+
+    set options [list "additional_flags=-print-prog-name=$prog"]
+    set output [lindex [${tool}_target_compile "" "" "none" 
$options] 0]

+
+    if { $output == $prog } {
+    return 0
+    }
+
+    return 1
+}

PING^3 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603544.html

Kind regards,
Torbjörn

On 2022-10-14 09:42, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html

Kind regards,
Torbjörn

On 2022-10-05 11:17, Torbjorn SVENSSON wrote:

Hi,

Ping, 
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html


Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
  gcc/testsuite/g++.dg/modules/modules.exp | 31 
  gcc/testsuite/lib/target-supports-dg.exp | 15 
  gcc/testsuite/lib/target-supports.exp    | 15 
  3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp

index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
  return $option_list
  }
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+    foreach test $tests {
+    set tmp [dg-get-options $test]
+    foreach op $tmp {
+    switch [lindex $op 0] {
+    "dg-additional-options" {
+    # Example strings to match:
+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ 
-t\\ [srcdir]/inc-xlate-1.map

+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+    if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} 
[lindex $op 2] dummy dummy2 prog] {

+    verbose "Checking that mapper exist: $prog"
+    if { ![ check_is_prog_name_available $prog ] } {
+    return 0
+    }
+    }
+    }
+    }
+    }
+    }
+    return 1
+}
+
  # cleanup any detritus from previous run
  cleanup_module_files [find $DEFAULT_REPO *.gcm]
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir 
{*_a.[CHX}]] {

  set tests [lsort [find [file dirname $src] \
    [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]

+    if { ![module-check-requirements $tests] } {
+    set testcase [regsub {_a.[CH]} $src {}]
+    set testcase \
+    [string range $testcase [string length "$srcdir/"] end]
+    unsupported $testcase
+    continue
+    }
+
  set std_list [module-init $src]
  foreach std $std_list {
  set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp

index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
  set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
  }
  }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+    # The args are within another list; pull them out.
+    set args [lindex $args 0]
+
+    set prog [lindex $args 1]
+
+    if { ![ check_is_prog_name_available $prog ] } {
+    upvar dg-do-what dg-do-what
+    set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+    }
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp

index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
  .byte 0
    } ""]
  }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+    global tool
+
+    set options [list "additional_flags=-print-prog-name=$prog"]
+    set output [lindex [${tool}_target_compile "" "" "none" 
$options] 0]

+
+    if { $output == $prog } {
+    return 0
+    }
+
+    return 1
+}

PING^2 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602843.html

Kind regards,
Torbjörn

On 2022-10-05 11:17, Torbjorn SVENSSON wrote:

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html

Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
  gcc/testsuite/g++.dg/modules/modules.exp | 31 
  gcc/testsuite/lib/target-supports-dg.exp | 15 
  gcc/testsuite/lib/target-supports.exp    | 15 
  3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp

index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
  return $option_list
  }
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+    foreach test $tests {
+    set tmp [dg-get-options $test]
+    foreach op $tmp {
+    switch [lindex $op 0] {
+    "dg-additional-options" {
+    # Example strings to match:
+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ -t\\ 
[srcdir]/inc-xlate-1.map

+    # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+    if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} [lindex 
$op 2] dummy dummy2 prog] {

+    verbose "Checking that mapper exist: $prog"
+    if { ![ check_is_prog_name_available $prog ] } {
+    return 0
+    }
+    }
+    }
+    }
+    }
+    }
+    return 1
+}
+
  # cleanup any detritus from previous run
  cleanup_module_files [find $DEFAULT_REPO *.gcm]
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir 
{*_a.[CHX}]] {

  set tests [lsort [find [file dirname $src] \
    [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]

+    if { ![module-check-requirements $tests] } {
+    set testcase [regsub {_a.[CH]} $src {}]
+    set testcase \
+    [string range $testcase [string length "$srcdir/"] end]
+    unsupported $testcase
+    continue
+    }
+
  set std_list [module-init $src]
  foreach std $std_list {
  set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp

index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
  set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
  }
  }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+    # The args are within another list; pull them out.
+    set args [lindex $args 0]
+
+    set prog [lindex $args 1]
+
+    if { ![ check_is_prog_name_available $prog ] } {
+    upvar dg-do-what dg-do-what
+    set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+    }
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp

index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
  .byte 0
    } ""]
  }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+    global tool
+
+    set options [list "additional_flags=-print-prog-name=$prog"]
+    set output [lindex [${tool}_target_compile "" "" "none" $options] 0]
+
+    if { $output == $prog } {
+    return 0
+    }
+
+    return 1
+}

PING^1 [PATCH] testsuite: Verify that module-mapper is available

[Torbjorn SVENSSON via Gcc-patches]

Hi,

Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602111.html

Kind regards,
Torbjörn

On 2022-09-23 14:03, Torbjörn SVENSSON wrote:

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
  gcc/testsuite/g++.dg/modules/modules.exp | 31 
  gcc/testsuite/lib/target-supports-dg.exp | 15 
  gcc/testsuite/lib/target-supports.exp| 15 
  3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp
index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
  return $option_list
  }
  
+# Return 1 if requirements are met

+proc module-check-requirements { tests } {
+foreach test $tests {
+   set tmp [dg-get-options $test]
+   foreach op $tmp {
+   switch [lindex $op 0] {
+   "dg-additional-options" {
+   # Example strings to match:
+   # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ -t\\ 
[srcdir]/inc-xlate-1.map
+   # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+   if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} [lindex 
$op 2] dummy dummy2 prog] {
+   verbose "Checking that mapper exist: $prog"
+   if { ![ check_is_prog_name_available $prog ] } {
+   return 0
+   }
+   }
+   }
+   }
+   }
+}
+return 1
+}
+
  # cleanup any detritus from previous run
  cleanup_module_files [find $DEFAULT_REPO *.gcm]
  
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir {*_a.[CHX}]] {

set tests [lsort [find [file dirname $src] \
  [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]
  
+	if { ![module-check-requirements $tests] } {

+   set testcase [regsub {_a.[CH]} $src {}]
+   set testcase \
+   [string range $testcase [string length "$srcdir/"] end]
+   unsupported $testcase
+   continue
+   }
+
set std_list [module-init $src]
foreach std $std_list {
set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp
index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
  }
  }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+# The args are within another list; pull them out.
+set args [lindex $args 0]
+
+set prog [lindex $args 1]
+
+if { ![ check_is_prog_name_available $prog ] } {
+upvar dg-do-what dg-do-what
+set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+}
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
.byte 0
} ""]
  }
+
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+global tool
+
+set options [list "additional_flags=-print-prog-name=$prog"]
+set output [lindex [${tool}_target_compile "" "" "none" $options] 0]
+
+if { $output == $prog } {
+return 0
+}
+
+return 1
+}

[PATCH] testsuite: Verify that module-mapper is avialable

[Torbjörn SVENSSON via Gcc-patches]

For some test cases, it's required that the optional module mapper
"g++-mapper-server" is built. As the server is not required, the
test cases will fail if it can't be found.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_is_prog_name_available):
New.
* lib/target-supports-dg.exp
(dg-require-prog-name-available): New.
* g++.dg/modules/modules.exp: Verify avilability of module
mapper.

Signed-off-by: Torbjörn SVENSSON  
---
 gcc/testsuite/g++.dg/modules/modules.exp | 31 
 gcc/testsuite/lib/target-supports-dg.exp | 15 
 gcc/testsuite/lib/target-supports.exp| 15 
 3 files changed, 61 insertions(+)

diff --git a/gcc/testsuite/g++.dg/modules/modules.exp 
b/gcc/testsuite/g++.dg/modules/modules.exp
index afb323d0efd..4784803742a 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -279,6 +279,29 @@ proc module-init { src } {
 return $option_list
 }
 
+# Return 1 if requirements are met
+proc module-check-requirements { tests } {
+foreach test $tests {
+   set tmp [dg-get-options $test]
+   foreach op $tmp {
+   switch [lindex $op 0] {
+   "dg-additional-options" {
+   # Example strings to match:
+   # -fmodules-ts -fmodule-mapper=|@g++-mapper-server\\ -t\\ 
[srcdir]/inc-xlate-1.map
+   # -fmodules-ts -fmodule-mapper=|@g++-mapper-server
+   if [regexp -- {(^| )-fmodule-mapper=\|@([^\\ ]*)} [lindex 
$op 2] dummy dummy2 prog] {
+   verbose "Checking that mapper exist: $prog"
+   if { ![ check_is_prog_name_available $prog ] } {
+   return 0
+   }
+   }
+   }
+   }
+   }
+}
+return 1
+}
+
 # cleanup any detritus from previous run
 cleanup_module_files [find $DEFAULT_REPO *.gcm]
 
@@ -307,6 +330,14 @@ foreach src [lsort [find $srcdir/$subdir {*_a.[CHX}]] {
set tests [lsort [find [file dirname $src] \
  [regsub {_a.[CHX]$} [file tail $src] 
{_[a-z].[CHX]}]]]
 
+   if { ![module-check-requirements $tests] } {
+   set testcase [regsub {_a.[CH]} $src {}]
+   set testcase \
+   [string range $testcase [string length "$srcdir/"] end]
+   unsupported $testcase
+   continue
+   }
+
set std_list [module-init $src]
foreach std $std_list {
set mod_files {}
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp
index aa2164bc789..6ce3b2b1a1b 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -683,3 +683,18 @@ proc dg-require-symver { args } {
set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
 }
 }
+
+# If this target does not provide prog named "$args", skip this test.
+
+proc dg-require-prog-name-available { args } {
+# The args are within another list; pull them out.
+set args [lindex $args 0]
+
+set prog [lindex $args 1]
+
+if { ![ check_is_prog_name_available $prog ] } {
+upvar dg-do-what dg-do-what
+set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+}
+}
+
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index 703aba412a6..c3b7a6c17b3 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11928,3 +11928,18 @@ main:
.byte 0
   } ""]
 }
+ 
+# Return 1 if this target has prog named "$prog", 0 otherwise.
+
+proc check_is_prog_name_available { prog } {
+global tool
+
+set options [list "additional_flags=-print-prog-name=$prog"]
+set output [lindex [${tool}_target_compile "" "" "none" $options] 0]
+
+if { $output == $prog } {
+return 0
+}
+
+return 1
+}
-- 
2.25.1

Re: [PATCH] tree-optimization/106422 - verify block copying in forward threading

[Jeff Law via Gcc-patches]

On 7/29/2022 2:47 AM, Richard Biener via Gcc-patches wrote:

The forward threader failed to check whether it can actually duplicate
blocks.  The following adds this in a similar place the backwards threader
performs this check.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR tree-optimization/106422
* tree-ssa-threadupdate.cc (fwd_jt_path_registry::update_cfg):
Check whether we can copy thread blocks and cancel the thread if not.

* gcc.dg/torture/pr106422.c: New testcase.

I'm back from PTO.  This looks obviously correct to me.

jeff

[PATCH] tree-optimization/106422 - verify block copying in forward threading

[Richard Biener via Gcc-patches]

The forward threader failed to check whether it can actually duplicate
blocks.  The following adds this in a similar place the backwards threader
performs this check.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR tree-optimization/106422
* tree-ssa-threadupdate.cc (fwd_jt_path_registry::update_cfg):
Check whether we can copy thread blocks and cancel the thread if not.

* gcc.dg/torture/pr106422.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr106422.c | 14 ++
 gcc/tree-ssa-threadupdate.cc|  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr106422.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr106422.c 
b/gcc/testsuite/gcc.dg/torture/pr106422.c
new file mode 100644
index 000..a2cef1aecb6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr106422.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+
+void vfork() __attribute__((__leaf__));
+void semanage_reload_policy(char *arg, void cb(void))
+{
+  if (!arg)
+{
+  cb();
+  return;
+}
+  vfork();
+  if (arg)
+__builtin_free(arg);
+}
diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc
index f901c7759e3..0f2b319d44a 100644
--- a/gcc/tree-ssa-threadupdate.cc
+++ b/gcc/tree-ssa-threadupdate.cc
@@ -2678,7 +2678,9 @@ fwd_jt_path_registry::update_cfg (bool 
may_peel_loop_headers)
for (j = 0; j < path->length (); j++)
  {
edge e = (*path)[j]->e;
-   if (m_removed_edges->find_slot (e, NO_INSERT))
+   if (m_removed_edges->find_slot (e, NO_INSERT)
+   || ((*path)[j]->type == EDGE_COPY_SRC_BLOCK
+   && !can_duplicate_block_p (e->src)))
  break;
  }
 
-- 
2.35.3

Re: nvptx: forward '-v' command-line option to assembler, linker (was: [MentorEmbedded/nvptx-tools] Issue 30: Ignore not-supported sm_* error without --verify (PR #31))

[Tobias Burnus]

Hi Thomas,

On 29.05.22 22:49, Thomas Schwinge wrote:

Not sure if that's what you had in mind, but what do you think about the
attached "nvptx: forward '-v' command-line option to assembler, linker"?
OK to push to GCC master branch (after merging

"Put '-v' verbose output onto stderr instead of stdout")?


I was mainly thinking of some way to have it available — which
'-foffload-options=-Wa,-v' already permits on the GCC side. (Once the
nvptx-tools patch actually makes use of the '-v'.)

If I understand your patch correctly, this patch now causes 'gcc -v' to
imply 'gcc -v -Wa,-v'. I think that's okay, since 'gcc -v' already
outputs a lot of lines and those lines can be helpful to understand what
happens and what not.

Tom, your thoughts on this?

Tobias

-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955

nvptx: forward '-v' command-line option to assembler, linker (was: [MentorEmbedded/nvptx-tools] Issue 30: Ignore not-supported sm_* error without --verify (PR #31))

[Thomas Schwinge]

Hi!

One item from 
:

On 2022-04-11T04:25:01-0700, Tobias Burnus  wrote:
> we could consider invoking nvptx-as with `-v` from GCC to make this more 
> prominent – at least when building GCC itself. _(The user could do `-Wa,-v` 
> to force this warning.)_

Not sure if that's what you had in mind, but what do you think about the
attached "nvptx: forward '-v' command-line option to assembler, linker"?
OK to push to GCC master branch (after merging

"Put '-v' verbose output onto stderr instead of stdout")?


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 17c35607d4927299b0c4bd19dd6fd205c85c4a4b Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Sun, 29 May 2022 22:31:43 +0200
Subject: [PATCH] nvptx: forward '-v' command-line option to assembler, linker

For example, for offloading compilation with '-save-temps -v', before vs. after
word-diff then looks like:

[...]
 [...]/build-gcc-offload-nvptx-none/gcc/as {+-v -v+} -o ./a.xnvptx-none.mkoffload.o ./a.xnvptx-none.mkoffload.s
{+Verifying sm_30 code with sm_35 code generation.+}
{+ ptxas -c -o /dev/null ./a.xnvptx-none.mkoffload.o --gpu-name sm_35 -O0+}
[...]
 [...]/build-gcc-offload-nvptx-none/gcc/collect2 {+-v -v+} -o ./a.xnvptx-none.mkoffload [...] @./a.xnvptx-none.mkoffload.args.1 -lgomp -lgcc -lc -lgcc
{+collect2 version 12.0.1 20220428 (experimental)+}
{+[...]/build-gcc-offload-nvptx-none/gcc/collect-ld -v -v -o ./a.xnvptx-none.mkoffload [...] ./a.xnvptx-none.mkoffload.o -lgomp -lgcc -lc -lgcc+}
{+Linking ./a.xnvptx-none.mkoffload.o as 0+}
{+trying lib libc.a+}
{+trying lib libgcc.a+}
{+trying lib libgomp.a+}
{+Resolving abort+}
{+Resolving acc_on_device+}
{+Linking libgomp.a::oacc-init.o/ as 1+}
{+Linking libc.a::lib_a-abort.o/   as 2+}
[...]

(This depends on 
"Put '-v' verbose output onto stderr instead of stdout".)

	gcc/
	* config/nvptx/nvptx.h (ASM_SPEC, LINK_SPEC): Define.
---
 gcc/config/nvptx/nvptx.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/config/nvptx/nvptx.h b/gcc/config/nvptx/nvptx.h
index ed72c253191..b184f1d0150 100644
--- a/gcc/config/nvptx/nvptx.h
+++ b/gcc/config/nvptx/nvptx.h
@@ -27,6 +27,13 @@
 
 /* Run-time Target.  */
 
+/* Assembler supports '-v' option; handle similar to
+   '../../gcc.cc:asm_options', 'HAVE_GNU_AS'.  */
+#define ASM_SPEC "%{v}"
+
+/* Linker supports '-v' option.  */
+#define LINK_SPEC "%{v}"
+
 #define STARTFILE_SPEC "%{mmainkernel:crt0.o}"
 
 #define TARGET_CPU_CPP_BUILTINS() nvptx_cpu_cpp_builtins ()
-- 
2.25.1

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jan Hubicka via Gcc-patches]

> On Thu, Apr 28, 2022 at 01:54:51PM +0200, Jan Hubicka wrote:
> > > --- gcc/cgraph.cc.jj  2022-04-20 09:24:12.194579146 +0200
> > > +++ gcc/cgraph.cc 2022-04-27 11:53:52.102173154 +0200
> > > @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
> > >"returns a pointer");
> > >error_found = true;
> > >  }
> > > -  if (definition && externally_visible
> > > +  if (definition
> > > +  && externally_visible
> > > +  && (!alias || thunk || !in_lto_p)
> > 
> > The alias check seems OK to me.
> > Why we need thunk?  Here thunk may eventually gain a gimple body and we
> > will be interested in its opt_for_fn flags.  So if we do not set
> > DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
> > (such as blocked inlining).
> 
> I've added || thunk because free_lang_data sets 
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> for thunks, the guarding condition is:
> if (gimple_has_body_p (decl) || (node && node->thunk))
> I wasn't sure if node->alias and node->thunk can be both set or not.
> If they can't, I can take it out.

Aha, I misread is as !alias || !thunk.
But node can not be both alias and thunk, so I would leave it out.

Honza

> 
>   Jakub
> 

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 28, 2022 at 01:54:51PM +0200, Jan Hubicka wrote:
> > --- gcc/cgraph.cc.jj2022-04-20 09:24:12.194579146 +0200
> > +++ gcc/cgraph.cc   2022-04-27 11:53:52.102173154 +0200
> > @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
> >  "returns a pointer");
> >error_found = true;
> >  }
> > -  if (definition && externally_visible
> > +  if (definition
> > +  && externally_visible
> > +  && (!alias || thunk || !in_lto_p)
> 
> The alias check seems OK to me.
> Why we need thunk?  Here thunk may eventually gain a gimple body and we
> will be interested in its opt_for_fn flags.  So if we do not set
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
> (such as blocked inlining).

I've added || thunk because free_lang_data sets 
DECL_FUNCTION_SPECIFIC_OPTIMIZATION
for thunks, the guarding condition is:
if (gimple_has_body_p (decl) || (node && node->thunk))
I wasn't sure if node->alias and node->thunk can be both set or not.
If they can't, I can take it out.

Jakub

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jan Hubicka via Gcc-patches]

Hello,
> Hi!
> 
> The following testcase ICEs, because the ctors during cc1plus all have
> !opt_for_fn (decl, flag_semantic_interposition) - they have NULL
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
> is for -Ofast and so has flag_semantic_interposition cleared.
> During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> for the ctor which has body (or for thunks), but don't touch it for
> aliases.

Hmm, this is bit interesting.  We definitly want to preserve original
semantic interposition behaviour of aliases and it works here since we
move the flag to symbol table, but it is bit of accident (I added the
flag because variables does not have their ovn
DECL_VAR_SPECIFIC_OPTIMIZATION so it needs to be part of symbol table).

Looking for current uses of opt_for_fn we seems to care about properties
of the actual function bodies, not aliases, but it feels a bit fragile.
Next stage1 I should make opt_for_fn to bomb on all aliases so we know
we are not using it wrongly.

> During lto1 optimization_default_node reflects the lto1 flags which
> are -O2 rather than -Ofast and so has flag_semantic_interposition
> set, for the ctor which has body that makes no difference, but as the
> alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
> we now trigger this verification check.
> 
> The following patch just doesn't verify it for aliases during lto1.
> Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> during free lang data even for aliases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-04-28  Jakub Jelinek  
> 
>   PR lto/105399
>   * cgraph.cc (cgraph_node::verify_node): Don't verify
>   semantic_interposition flag against
>   opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.
> 
>   * g++.dg/lto/pr105399_0.C: New test.
> 
> --- gcc/cgraph.cc.jj  2022-04-20 09:24:12.194579146 +0200
> +++ gcc/cgraph.cc 2022-04-27 11:53:52.102173154 +0200
> @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
>"returns a pointer");
>error_found = true;
>  }
> -  if (definition && externally_visible
> +  if (definition
> +  && externally_visible
> +  && (!alias || thunk || !in_lto_p)

The alias check seems OK to me.
Why we need thunk?  Here thunk may eventually gain a gimple body and we
will be interested in its opt_for_fn flags.  So if we do not set
DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
(such as blocked inlining).
>&& semantic_interposition
>!= opt_for_fn (decl, flag_semantic_interposition))
>  {
> --- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj  2022-04-27 11:54:25.659703199 
> +0200
> +++ gcc/testsuite/g++.dg/lto/pr105399_0.C 2022-04-27 11:48:31.387664565 
> +0200
> @@ -0,0 +1,9 @@
> +// PR lto/105399
> +// { dg-lto-do link }
> +// { dg-lto-options { { -fPIC -flto -Ofast } } }
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> +// { dg-extra-ld-options "-shared -O2" }
> +
> +struct S { S (); };
> +S::S () {}
> 
>   Jakub
> 

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Richard Biener via Gcc-patches]

On Thu, 28 Apr 2022, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because the ctors during cc1plus all have
> !opt_for_fn (decl, flag_semantic_interposition) - they have NULL
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
> is for -Ofast and so has flag_semantic_interposition cleared.
> During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> for the ctor which has body (or for thunks), but don't touch it for
> aliases.
> During lto1 optimization_default_node reflects the lto1 flags which
> are -O2 rather than -Ofast and so has flag_semantic_interposition
> set, for the ctor which has body that makes no difference, but as the
> alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
> we now trigger this verification check.
> 
> The following patch just doesn't verify it for aliases during lto1.
> Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> during free lang data even for aliases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-04-28  Jakub Jelinek  
> 
>   PR lto/105399
>   * cgraph.cc (cgraph_node::verify_node): Don't verify
>   semantic_interposition flag against
>   opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.
> 
>   * g++.dg/lto/pr105399_0.C: New test.
> 
> --- gcc/cgraph.cc.jj  2022-04-20 09:24:12.194579146 +0200
> +++ gcc/cgraph.cc 2022-04-27 11:53:52.102173154 +0200
> @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
>"returns a pointer");
>error_found = true;
>  }
> -  if (definition && externally_visible
> +  if (definition
> +  && externally_visible
> +  && (!alias || thunk || !in_lto_p)

Can you add a comment above this?

OK with this if Honza doesn't have any suggestions today.

Richard.

>&& semantic_interposition
>!= opt_for_fn (decl, flag_semantic_interposition))
>  {
> --- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj  2022-04-27 11:54:25.659703199 
> +0200
> +++ gcc/testsuite/g++.dg/lto/pr105399_0.C 2022-04-27 11:48:31.387664565 
> +0200
> @@ -0,0 +1,9 @@
> +// PR lto/105399
> +// { dg-lto-do link }
> +// { dg-lto-options { { -fPIC -flto -Ofast } } }
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> +// { dg-extra-ld-options "-shared -O2" }
> +
> +struct S { S (); };
> +S::S () {}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

[PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jakub Jelinek via Gcc-patches]

Hi!

The following testcase ICEs, because the ctors during cc1plus all have
!opt_for_fn (decl, flag_semantic_interposition) - they have NULL
DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
is for -Ofast and so has flag_semantic_interposition cleared.
During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
for the ctor which has body (or for thunks), but don't touch it for
aliases.
During lto1 optimization_default_node reflects the lto1 flags which
are -O2 rather than -Ofast and so has flag_semantic_interposition
set, for the ctor which has body that makes no difference, but as the
alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
we now trigger this verification check.

The following patch just doesn't verify it for aliases during lto1.
Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
during free lang data even for aliases.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-04-28  Jakub Jelinek  

PR lto/105399
* cgraph.cc (cgraph_node::verify_node): Don't verify
semantic_interposition flag against
opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.

* g++.dg/lto/pr105399_0.C: New test.

--- gcc/cgraph.cc.jj2022-04-20 09:24:12.194579146 +0200
+++ gcc/cgraph.cc   2022-04-27 11:53:52.102173154 +0200
@@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
 "returns a pointer");
   error_found = true;
 }
-  if (definition && externally_visible
+  if (definition
+  && externally_visible
+  && (!alias || thunk || !in_lto_p)
   && semantic_interposition
 != opt_for_fn (decl, flag_semantic_interposition))
 {
--- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj2022-04-27 11:54:25.659703199 
+0200
+++ gcc/testsuite/g++.dg/lto/pr105399_0.C   2022-04-27 11:48:31.387664565 
+0200
@@ -0,0 +1,9 @@
+// PR lto/105399
+// { dg-lto-do link }
+// { dg-lto-options { { -fPIC -flto -Ofast } } }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
+// { dg-extra-ld-options "-shared -O2" }
+
+struct S { S (); };
+S::S () {}

Jakub

Enhance further testcases to verify Openacc 'kernels' decomposition

[Thomas Schwinge]

Hi!

Pushed to master branch commit c43cb355f25dd22133d15819bd6ec03d3d3939fd
"Enhance further testcases to verify Openacc 'kernels' decomposition",
see attached.


Kwok, this ought to resolve some testsuite noise from your upcoming og12
branch.  Please let me know if there are any more such issues.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From c43cb355f25dd22133d15819bd6ec03d3d3939fd Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 16 Mar 2022 14:19:41 +0100
Subject: [PATCH] Enhance further testcases to verify Openacc 'kernels'
 decomposition

	gcc/testsuite/
	* c-c++-common/goacc-gomp/nesting-1.c: Enhance.
	* c-c++-common/goacc/kernels-loop-g.c: Likewise.
	* c-c++-common/goacc/nesting-1.c: Likewise.
	* gcc.dg/goacc/nested-function-1.c: Likewise.
	* gfortran.dg/goacc/common-block-3.f90: Likewise.
	* gfortran.dg/goacc/nested-function-1.f90: Likewise.
	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c:
	Enhance.
	* testsuite/libgomp.oacc-c-c++-common/kernels-loop-g.c: Likewise.
	* testsuite/libgomp.oacc-fortran/if-1.f90: Likewise.
---
 .../c-c++-common/goacc-gomp/nesting-1.c   |  7 ++-
 .../c-c++-common/goacc/kernels-loop-g.c   |  3 ++
 gcc/testsuite/c-c++-common/goacc/nesting-1.c  | 18 +--
 .../gcc.dg/goacc/nested-function-1.c  | 22 
 .../gfortran.dg/goacc/common-block-3.f90  | 18 +--
 .../gfortran.dg/goacc/nested-function-1.f90   | 10 
 .../acc_prof-kernels-1.c  | 19 +--
 .../kernels-loop-g.c  |  3 ++
 .../testsuite/libgomp.oacc-fortran/if-1.f90   | 51 ++-
 9 files changed, 134 insertions(+), 17 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c b/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
index 39b92712b31..51c5e359f29 100644
--- a/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
+++ b/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
@@ -1,3 +1,5 @@
+/* { dg-additional-options "--param=openacc-kernels=decompose" }
+
 /* { dg-additional-options "-fopt-info-omp-note" } */
 
 /* { dg-additional-options "--param=openacc-privatization=noisy" }
@@ -21,10 +23,11 @@ void
 f_acc_kernels (void)
 {
 #pragma acc kernels
-  /* { dg-note {variable 'i' declared in block is candidate for adjusting OpenACC privatization level} "" { target *-*-* } .-1 }
- { dg-note {variable 'i' ought to be adjusted for OpenACC privatization level: 'gang'} "" { target *-*-* } .-2 } */
+  /* { dg-note {variable 'i' declared in block is candidate for adjusting OpenACC privatization level} "" { target *-*-* } .-1 } */
   {
 int i;
+/* { dg-note {beginning 'gang-single' part in OpenACC 'kernels' region} "" { target c } .+3 }
+   { dg-note {beginning 'gang-single' part in OpenACC 'kernels' region} "" { target c++ } .+1 } */
 #pragma omp atomic write
 i = 0;
   }
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c b/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c
index 73b469d7061..5bdaa40b02c 100644
--- a/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c
@@ -1,5 +1,8 @@
+/* { dg-additional-options "--param=openacc-kernels=decompose" } */
+
 /* { dg-additional-options "-O2" } */
 /* { dg-additional-options "-g" } */
+/*TODO PR100400 { dg-additional-options -fcompare-debug } */
 /* { dg-additional-options "-fdump-tree-parloops1-all" } */
 /* { dg-additional-options "-fdump-tree-optimized" } */
 
diff --git a/gcc/testsuite/c-c++-common/goacc/nesting-1.c b/gcc/testsuite/c-c++-common/goacc/nesting-1.c
index 83cbff767a4..8c3d1adc785 100644
--- a/gcc/testsuite/c-c++-common/goacc/nesting-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/nesting-1.c
@@ -1,3 +1,5 @@
+/* { dg-additional-options "--param=openacc-kernels=decompose" } */
+
 /* { dg-additional-options "-fopt-info-all-omp" } */
 
 /* { dg-additional-options "--param=openacc-privatization=noisy" } */
@@ -32,11 +34,13 @@ void
 f_acc_kernels (void)
 {
 #pragma acc kernels /* { dg-line l_compute[incr c_compute] } */
-  /* { dg-optimized {assigned OpenACC seq loop parallelism} {} { target *-*-* } l_compute$c_compute } */
+  /* { dg-note {variable 'i\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_compute$c_compute } */
   {
 #pragma acc loop /* { dg-line l_loop_i[incr c_loop_i] } */
+/* { dg-note {forwarded loop nest in OpenACC 'kernels' region to 'parloops' for analysis} {} { target *-*-* } l_loop_i$c_loop_i } */
 /* { dg-note {variable 'i\.[0-9]+' in 'priva

Enhance further testcases to verify handling of OpenACC privatization level [PR90115]

[Thomas Schwinge]

Hi!

On 2021-05-21T21:29:19+0200, I wrote:
> I've pushed "[OpenACC privatization] Largely extend diagnostics and
> corresponding testsuite coverage [PR90115]" to master branch in commit
> 11b8286a83289f5b54e813f14ff56d730c3f3185

Pushed to master branch commit 004fc4f2fc686d3366c9e1a2d8b9183796073866
"Enhance further testcases to verify handling of OpenACC privatization
level [PR90115]", see attached.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 004fc4f2fc686d3366c9e1a2d8b9183796073866 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 16 Mar 2022 12:15:01 +0100
Subject: [PATCH] Enhance further testcases to verify handling of OpenACC
 privatization level [PR90115]

As originally introduced in commit 11b8286a83289f5b54e813f14ff56d730c3f3185
"[OpenACC privatization] Largely extend diagnostics and corresponding testsuite
coverage [PR90115]".

	PR middle-end/90115
	gcc/testsuite/
	* c-c++-common/goacc-gomp/nesting-1.c: Enhance.
	* gfortran.dg/goacc/common-block-3.f90: Likewise.
	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c: Enhance.
	* testsuite/libgomp.oacc-fortran/if-1.f90: Likewise.
---
 .../c-c++-common/goacc-gomp/nesting-1.c   |  9 +-
 .../gfortran.dg/goacc/common-block-3.f90  | 13 ++-
 .../acc_prof-kernels-1.c  | 35 ++--
 .../testsuite/libgomp.oacc-fortran/if-1.f90   | 82 +--
 4 files changed, 68 insertions(+), 71 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c b/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
index b0b78374016..39b92712b31 100644
--- a/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
+++ b/gcc/testsuite/c-c++-common/goacc-gomp/nesting-1.c
@@ -1,14 +1,15 @@
 /* { dg-additional-options "-fopt-info-omp-note" } */
-/* { dg-additional-options "--param=openacc-privatization=noisy" } for
-   testing/documenting aspects of that functionality.  */
+
+/* { dg-additional-options "--param=openacc-privatization=noisy" }
+   Prune a few: uninteresting, and potentially varying depending on GCC configuration (data types):
+   { dg-prune-output {note: variable 'D\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} } */
 
 
 void
 f_acc_data (void)
 {
 #pragma acc data
-  /* { dg-note {variable 'D\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } .-1 } */
-  /* { dg-note {variable 'i' declared in block is candidate for adjusting OpenACC privatization level} "" { target *-*-* } .-2 } */
+  /* { dg-note {variable 'i' declared in block is candidate for adjusting OpenACC privatization level} "" { target *-*-* } .-1 } */
   {
 int i;
 #pragma omp atomic write
diff --git a/gcc/testsuite/gfortran.dg/goacc/common-block-3.f90 b/gcc/testsuite/gfortran.dg/goacc/common-block-3.f90
index 5defe2ea85d..9dbfa4cd2f0 100644
--- a/gcc/testsuite/gfortran.dg/goacc/common-block-3.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/common-block-3.f90
@@ -1,5 +1,11 @@
 ! { dg-options "-fopenacc -fdump-tree-omplower" }
 
+! { dg-additional-options "-fopt-info-omp-all" }
+
+! { dg-additional-options "--param=openacc-privatization=noisy" }
+! Prune a few: uninteresting, and potentially varying depending on GCC configuration (data types):
+! { dg-prune-output {note: variable 'D\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} }
+
 module consts
   integer, parameter :: n = 100
 end module consts
@@ -15,11 +21,14 @@ program main
   common /KERNELS_BLOCK/ x, y, z
 
   c = 1.0
-  !$acc parallel loop copy(/BLOCK/)
+  !$acc parallel loop copy(/BLOCK/) ! { dg-line l1 }
+  ! { dg-note {variable 'i' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l1 }
+  ! { dg-optimized {assigned OpenACC gang vector loop parallelism} {} { target *-*-* } l1 }
   do i = 1, n
  a(i) = b(i) + c
   end do
-  !$acc kernels
+  !$acc kernels ! { dg-line l2 }
+  ! { dg-optimized {assigned OpenACC seq loop parallelism} {} { target *-*-* } l2 }
   do i = 1, n
  x(i) = y(i) + c
   end do
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c
index d7f47627438..c82a7edbfa0 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-kernels-1.c
@@ -1,5 +1,21 @@
 /* Test dispatch of events to callbacks.  */
 
+/* { dg-additional-options "-fopt-info-omp-all" }
+   { dg-ad

Enhance further testcases to verify handling of OpenACC privatization level [PR90115]

[Thomas Schwinge]

Hi!

On 2021-05-21T21:29:19+0200, I wrote:
> I've pushed "[OpenACC privatization] Largely extend diagnostics and
> corresponding testsuite coverage [PR90115]" to master branch in commit
> 11b8286a83289f5b54e813f14ff56d730c3f3185

To demonstrate that later changes don't vs. how they do change things,
pushed to master branch commit 2e53fa7bb2ae9fe1152c27e423be9e261da82ddc
"Enhance further testcases to verify handling of OpenACC privatization
level [PR90115]", see attached.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 2e53fa7bb2ae9fe1152c27e423be9e261da82ddc Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Fri, 11 Mar 2022 15:10:59 +0100
Subject: [PATCH] Enhance further testcases to verify handling of OpenACC
 privatization level [PR90115]

As originally introduced in commit 11b8286a83289f5b54e813f14ff56d730c3f3185
"[OpenACC privatization] Largely extend diagnostics and corresponding testsuite
coverage [PR90115]".

	PR middle-end/90115
	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/default-1.c: Enhance.
	* testsuite/libgomp.oacc-c-c++-common/kernels-reduction-1.c: Likewise.
	* testsuite/libgomp.oacc-c-c++-common/parallel-dims.c: Likewise.
	* testsuite/libgomp.oacc-fortran/kernels-reduction-1.f90: Likewise.
---
 .../libgomp.oacc-c-c++-common/default-1.c |  32 ++-
 .../kernels-reduction-1.c |  14 +-
 .../libgomp.oacc-c-c++-common/parallel-dims.c | 261 +++---
 .../kernels-reduction-1.f90   |  14 +-
 4 files changed, 266 insertions(+), 55 deletions(-)

diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/default-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/default-1.c
index 1ac0b9587b9..0ac8d7132d4 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/default-1.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/default-1.c
@@ -1,4 +1,18 @@
-/* { dg-do run } */
+/* { dg-additional-options "-fopt-info-all-omp" }
+   { dg-additional-options "-foffload=-fopt-info-all-omp" } */
+
+/* { dg-additional-options "--param=openacc-privatization=noisy" }
+   { dg-additional-options "-foffload=--param=openacc-privatization=noisy" }
+   Prune a few: uninteresting, and potentially varying depending on GCC configuration (data types):
+   { dg-prune-output {note: variable 'D\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} } */
+
+/* It's only with Tcl 8.5 (released in 2007) that "the variable 'varName'
+   passed to 'incr' may be unset, and in that case, it will be set to [...]",
+   so to maintain compatibility with earlier Tcl releases, we manually
+   initialize counter variables:
+   { dg-line l_dummy[variable c_compute 0 c_loop_i 0] }
+   { dg-message dummy {} { target iN-VAl-Id } l_dummy } to avoid
+   "WARNING: dg-line var l_dummy defined, but not used".  */
 
 #include  
 
@@ -13,10 +27,15 @@ int test_parallel ()
 ary[i] = ~0;
 
   /* val defaults to firstprivate, ary defaults to copy.  */
-#pragma acc parallel num_gangs (32) copy (ok) copy(ondev)
+#pragma acc parallel num_gangs (32) copy (ok) copy(ondev) /* { dg-line l_compute[incr c_compute] } */
+  /* { dg-note {variable 'i' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_compute$c_compute } */
   {
 ondev = acc_on_device (acc_device_not_host);
-#pragma acc loop gang(static:1)
+/* { dg-optimized {assigned OpenACC seq loop parallelism} {} { target { c++ && { ! __OPTIMIZE__ } } } .-1 }
+   ..., as without optimizations, we're not inlining the C++ 'acc_on_device' wrapper.  */
+#pragma acc loop gang(static:1) /* { dg-line l_loop_i[incr c_loop_i] } */
+/* { dg-note {variable 'i' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_loop_i$c_loop_i } */
+/* { dg-optimized {assigned OpenACC gang loop parallelism} {} { target *-*-* } l_loop_i$c_loop_i } */
 for (unsigned i = 0; i < 32; i++)
   {
 	if (val != 2)
@@ -51,10 +70,13 @@ int test_kernels ()
 ary[i] = ~0;
 
   /* val defaults to copy, ary defaults to copy.  */
-#pragma acc kernels copy(ondev)
+#pragma acc kernels copy(ondev) /* { dg-line l_compute[incr c_compute] } */
+  /* { dg-note {variable 'i' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_compute$c_compute } */
+  /* { dg-optimized {assigned OpenACC seq loop parallelism} {} { target *-*-* } l_compute$c_compute } */
   {
 ondev = acc_on_device (acc_device_not_host);
-#pragma acc loop 
+#pragma acc loop /* { dg-line l_loop_i[incr c_loop_i] }

Enhance further testcases to verify handling of OpenACC privatization level [PR90115]

[Thomas Schwinge]

Hi!

On 2021-05-21T21:29:19+0200, I wrote:
> I've pushed "[OpenACC privatization] Largely extend diagnostics and
> corresponding testsuite coverage [PR90115]" to master branch in commit
> 11b8286a83289f5b54e813f14ff56d730c3f3185

To demonstrate that later changes don't vs. how they do change things,
pushed to master branch commit 1d9dc3dd74eddd192bec1ac6f4d6548a81deb9a5
"Enhance further testcases to verify handling of OpenACC privatization
level [PR90115]", see attached.


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 1d9dc3dd74eddd192bec1ac6f4d6548a81deb9a5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 8 Mar 2022 11:51:55 +0100
Subject: [PATCH] Enhance further testcases to verify handling of OpenACC
 privatization level [PR90115]

As originally introduced in commit 11b8286a83289f5b54e813f14ff56d730c3f3185
"[OpenACC privatization] Largely extend diagnostics and corresponding testsuite
coverage [PR90115]".

	PR middle-end/90115
	gcc/testsuite/
	* c-c++-common/goacc/nesting-1.c: Enhance.
	* gcc.dg/goacc/nested-function-1.c: Likewise.
	* gcc.dg/goacc/nested-function-2.c: Likewise.
	* gfortran.dg/goacc/nested-function-1.f90: Likewise.
	libgomp/
	* testsuite/libgomp.oacc-fortran/routine-1.f90: Enhance.
	* testsuite/libgomp.oacc-fortran/routine-2.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-3.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-9.f90: Likewise.
---
 gcc/testsuite/c-c++-common/goacc/nesting-1.c  | 57 +
 .../gcc.dg/goacc/nested-function-1.c  | 54 
 .../gcc.dg/goacc/nested-function-2.c  | 28 -
 .../gfortran.dg/goacc/nested-function-1.f90   | 62 +++
 .../libgomp.oacc-fortran/routine-1.f90| 19 +-
 .../libgomp.oacc-fortran/routine-2.f90| 19 +-
 .../libgomp.oacc-fortran/routine-3.f90| 19 +-
 .../libgomp.oacc-fortran/routine-9.f90| 19 +-
 8 files changed, 227 insertions(+), 50 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/goacc/nesting-1.c b/gcc/testsuite/c-c++-common/goacc/nesting-1.c
index cab4f98950d..83cbff767a4 100644
--- a/gcc/testsuite/c-c++-common/goacc/nesting-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/nesting-1.c
@@ -1,3 +1,15 @@
+/* { dg-additional-options "-fopt-info-all-omp" } */
+
+/* { dg-additional-options "--param=openacc-privatization=noisy" } */
+
+/* It's only with Tcl 8.5 (released in 2007) that "the variable 'varName'
+   passed to 'incr' may be unset, and in that case, it will be set to [...]",
+   so to maintain compatibility with earlier Tcl releases, we manually
+   initialize counter variables:
+   { dg-line l_dummy[variable c_compute 0 c_loop_i 0] }
+   { dg-message dummy {} { target iN-VAl-Id } l_dummy } to avoid
+   "WARNING: dg-line var l_dummy defined, but not used".  */
+
 extern int i;
 
 void
@@ -5,7 +17,11 @@ f_acc_parallel (void)
 {
 #pragma acc parallel
   {
-#pragma acc loop
+#pragma acc loop /* { dg-line l_loop_i[incr c_loop_i] } */
+/* { dg-note {variable 'i\.[0-9]+' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_loop_i$c_loop_i } */
+/* { dg-note {variable 'i' in 'private' clause is candidate for adjusting OpenACC privatization level} {} { target *-*-* } l_loop_i$c_loop_i }
+   { dg-note {variable 'i' ought to be adjusted for OpenACC privatization level: 'vector'} {} { target *-*-* } l_loop_i$c_loop_i } */
+/* { dg-optimized {assigned OpenACC gang vector loop parallelism} {} { target *-*-* } l_loop_i$c_loop_i } */
 for (i = 0; i < 2; ++i)
   ;
   }
@@ -15,9 +31,12 @@ f_acc_parallel (void)
 void
 f_acc_kernels (void)
 {
-#pragma acc kernels
+#pragma acc kernels /* { dg-line l_compute[incr c_compute] } */
+  /* { dg-optimized {assigned OpenACC seq loop parallelism} {} { target *-*-* } l_compute$c_compute } */
   {
-#pragma acc loop
+#pragma acc loop /* { dg-line l_loop_i[incr c_loop_i] } */
+/* { dg-note {variable 'i\.[0-9]+' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_loop_i$c_loop_i } */
+/* { dg-note {variable 'i' in 'private' clause is candidate for adjusting OpenACC privatization level} {} { target *-*-* } l_loop_i$c_loop_i } */
 for (i = 0; i < 2; ++i)
   ;
   }
@@ -34,17 +53,25 @@ f_acc_data (void)
 
 #pragma acc parallel
 {
-#pragma acc loop
+#pragma acc loop /* { dg-line l_loop_i[incr c_loop_i] } */
+  /* { dg-note {variable 'i\.[0-9]+' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { target *-*-* } l_loop_i$c_loop_i } */
+  /* { dg

[committed][nvptx] Use --no-verify for sm_30

[Tom de Vries via Gcc-patches]

Hi,

In PR97348, we ran into the problem that recent CUDA dropped support for
sm_30, which inhibited the build when building with CUDA bin in the path,
because the nvptx-tools assembler uses CUDA's ptxas to do ptx verification.

To fix this, in gcc-11 the default sm_xx was moved from sm_30 to sm_35.

This however broke support for sm_30 boards: an executable build for sm_30
might contain sm_35 code from the libraries, which are build with the default
sm_xx (PR104758).

We want to fix this by going back to having the libraries build with sm_30, as
was the case for gcc-5 to gcc-10.  That however reintroduces the problem from
PR97348.

Deal with PR97348 in the simplest way possible: when calling the assembler for
sm_30, specify --no-verify.

This has the unfortunate effect that after fixing PR104758 by building
libraries with sm_30, the libraries are no longer verified.  This can be
improved upon by:
- adding a configure test in gcc that tests if CUDA supports sm_30, and
  if so disabling this patch
- dealing with this in nvptx-tools somehow, either:
  - detect at ptxas execution time that it doesn't support sm_30, or
  - detect this at nvptx-tool configure time.

Committed to trunk.

Thanks,
- Tom

[nvptx] Use --no-verify for sm_30

gcc/ChangeLog:

2022-03-03  Tom de Vries  

* config/nvptx/nvptx.h (ASM_SPEC): Add %{misa=sm_30:--no-verify}.

---
 gcc/config/nvptx/nvptx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/nvptx/nvptx.h b/gcc/config/nvptx/nvptx.h
index 4ab412bc7d8..3ca22a595d2 100644
--- a/gcc/config/nvptx/nvptx.h
+++ b/gcc/config/nvptx/nvptx.h
@@ -32,7 +32,7 @@
 /* Default needs to be in sync with default for misa in nvptx.opt.
We add a default here to work around a hard-coded sm_30 default in
nvptx-as.  */
-#define ASM_SPEC "%{misa=*:-m %*; :-m sm_35}"
+#define ASM_SPEC "%{misa=*:-m %*; :-m sm_35}%{misa=sm_30:--no-verify}"
 
 #define TARGET_CPU_CPP_BUILTINS() nvptx_cpu_cpp_builtins ()
 

[committed] openmp: For min/max omp atomic compare forms verify arg types with build_binary_op [PR104531]

[Jakub Jelinek via Gcc-patches]

Hi!

The MIN_EXPR/MAX_EXPR handling in *build_binary_op is minimal (especially
for C FE), because min/max aren't expressions the languages contain directly.
I'm using those for the
  #pragma omp atomic
  x = x < y ? y : x;
forms, but e.g. for the attached testcase we normally reject _Complex int vs. 
int
comparisons, in C++ due to MIN/MAX_EXPR we were diagnosing it as invalid types
for  while in C we accept it and ICEd later on.

The following patch will try build_binary_op with LT_EXPR on the operands first
to get needed diagnostics and fail if it returns error_mark_node.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2022-02-16  Jakub Jelinek  

PR c/104531
* c-omp.cc (c_finish_omp_atomic): For MIN_EXPR/MAX_EXPR, try first
build_binary_op with LT_EXPR and only if that doesn't return
error_mark_node call build_modify_expr.

* c-c++-common/gomp/atomic-31.c: New test.

--- gcc/c-family/c-omp.cc.jj2022-02-11 00:19:22.107067674 +0100
+++ gcc/c-family/c-omp.cc   2022-02-15 16:06:24.173311609 +0100
@@ -353,8 +353,13 @@ c_finish_omp_atomic (location_t loc, enu
 }
   bool save = in_late_binary_op;
   in_late_binary_op = true;
-  x = build_modify_expr (loc, blhs ? blhs : lhs, NULL_TREE, opcode,
-loc, rhs, NULL_TREE);
+  if ((opcode == MIN_EXPR || opcode == MAX_EXPR)
+  && build_binary_op (loc, LT_EXPR, blhs ? blhs : lhs, rhs,
+ true) == error_mark_node)
+x = error_mark_node;
+  else
+x = build_modify_expr (loc, blhs ? blhs : lhs, NULL_TREE, opcode,
+  loc, rhs, NULL_TREE);
   in_late_binary_op = save;
   if (x == error_mark_node)
 return error_mark_node;
--- gcc/testsuite/c-c++-common/gomp/atomic-31.c.jj  2022-02-15 
17:02:17.938486108 +0100
+++ gcc/testsuite/c-c++-common/gomp/atomic-31.c 2022-02-15 17:04:23.811729201 
+0100
@@ -0,0 +1,11 @@
+/* c/104531 */
+/* { dg-do compile } */
+
+int x;
+
+void
+foo (_Complex int y)
+{
+  #pragma omp atomic compare   /* { dg-error "invalid operands" } */
+  x = x > y ? y : x;
+}

Jakub

[COMMITTED] test to verify -Wformat-overflow uses context-sensitive ranges

[Martin Sebor via Gcc-patches]

Converting the strlen/sprintf pass to Ranger has considerably
improved the accuracy of -Wformat-overflow warnings: they can avoid
triggering for safe input even at -O0 while at the same time detect
provable overflow.  The conversion didn't come with any tests so in
r12-6591 I committed one that verifies both of these improvements.

https://gcc.gnu.org/pipermail/gcc-cvs/2022-January/359299.html

Martin

[committed] analyzer: verify that -Wanalyzer-too-complex can be disabled via pragmas [PR100524]

[David Malcolm via Gcc-patches]

Successfully regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-5640-g03ea0ca1189a39e095188b0425c66446cc84a0a5.

gcc/testsuite/ChangeLog:
PR analyzer/100524
* gcc.dg/analyzer/pragma-2.c: New test.

Signed-off-by: David Malcolm 
---
 gcc/testsuite/gcc.dg/analyzer/pragma-2.c | 57 
 1 file changed, 57 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pragma-2.c

diff --git a/gcc/testsuite/gcc.dg/analyzer/pragma-2.c 
b/gcc/testsuite/gcc.dg/analyzer/pragma-2.c
new file mode 100644
index 000..58fcaab11df
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pragma-2.c
@@ -0,0 +1,57 @@
+/* Verify that we can disable -Wanalyzer-too-complex via pragmas.  */
+/* { dg-additional-options "-Wanalyzer-too-complex 
-Werror=analyzer-too-complex -fno-analyzer-state-merge -g" } */
+
+#include 
+
+extern int get (void);
+
+/* In theory each of p0...p4 can be in various malloc states,
+   independently, so the total combined number of states
+   at any program point within the loop is NUM_VARS * NUM_STATES.  */
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wanalyzer-too-complex"
+
+void test (void)
+{
+  void *p0 = NULL, *p1 = NULL, *p2 = NULL, *p3 = NULL, *p4 = NULL;
+  void **pp = NULL;
+  while (get ())
+{
+  switch (get ())
+   {
+   default:
+   case 0:
+ pp = 
+ break;
+   case 1:
+ pp = 
+ break;
+   case 2:
+ pp = 
+ break;
+   case 3:
+ pp = 
+ break;
+   case 4:
+ pp = 
+ break;
+   }
+
+  switch (get ())
+   {
+   default:
+   case 0:
+ *pp = malloc (16); /* { dg-warning "leak" } */
+ break;
+   case 1:
+ free (*pp);
+ break;
+   case 2:
+ /* no-op.  */
+ break;
+   }
+}
+}
+
+#pragma GCC diagnostic pop
-- 
2.26.3

[PATCH] Verify unallocated edge/BB flags are clear

[Richard Biener via Gcc-patches]

This adds verification that unused auto_{edge,bb}_flag are not
remaining set but correctly cleared by consumers.  The intent
is that those flags can be cheaply used on a smaller IL region
and thus afterwards clearing can be restricted to the same
small region as well.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2021-09-24  Richard Biener  

* cfghooks.c (verify_flow_info): Verify unallocated BB and
edge flags are not set.
---
 gcc/cfghooks.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/gcc/cfghooks.c b/gcc/cfghooks.c
index 50b9b177639..6446e16ca8c 100644
--- a/gcc/cfghooks.c
+++ b/gcc/cfghooks.c
@@ -161,6 +161,12 @@ verify_flow_info (void)
  err = 1;
}
 
+  if (bb->flags & ~cfun->cfg->bb_flags_allocated)
+   {
+ error ("verify_flow_info: unallocated flag set on BB %d", bb->index);
+ err = 1;
+   }
+
   FOR_EACH_EDGE (e, ei, bb->succs)
{
  if (last_visited [e->dest->index] == bb)
@@ -202,6 +208,13 @@ verify_flow_info (void)
  err = 1;
}
 
+ if (e->flags & ~cfun->cfg->edge_flags_allocated)
+   {
+ error ("verify_flow_info: unallocated edge flag set on %d -> %d",
+e->src->index, e->dest->index);
+ err = 1;
+   }
+
  edge_checksum[e->dest->index] += (size_t) e;
}
   if (n_fallthru > 1)
-- 
2.31.1

Re: fix typo in attr_fnspec::verify

[Alexandre Oliva]

On Jul 13, 2021, Alexandre Oliva  wrote:

> On Jul 13, 2021, Richard Biener  wrote:
>> oops - also worth backporting to affected branches.

> Thanks, I took that as explicit approval and put it in.

> attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
> trivial backport, just to be sure...  Will install in gcc-11 when done.

Here's what I've just installed.


fix typo in attr_fnspec::verify

Odd-numbered indices describing argument access sizes in the fnspec
string can only hold 't' or a digit, as tested in the beginning of the
case.  When checking that the size-supplying argument does not have
additional information associated with it, the test that excludes the
't' possibility looks for it at the even position in the fnspec
string.  Oops.

This might yield false positives and negatives if a function has a
fnspec in which an argument uses a 't' access-size, and ('t' - '1')
happens to be the index of an argument described in an fnspec string.
Assuming ASCII encoding, it would take a function with at least 68
arguments described in fnspec.  Still, probably worth fixing.


for  gcc/ChangeLog

* tree-ssa-alias.c (attr_fnspec::verify): Fix index in
non-'t'-sized arg check.

(cherry picked from commit a7098d6ef4e4e799dab8ef925c62b199d707694b)
---
 gcc/tree-ssa-alias.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index ebb3f49c86c66..3e578e5d05f49 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -3868,7 +3868,7 @@ attr_fnspec::verify ()
&& str[idx] != 'w' && str[idx] != 'W'
&& str[idx] != 'o' && str[idx] != 'O')
  err = true;
-   if (str[idx] != 't'
+   if (str[idx + 1] != 't'
/* Size specified is scalar, so it should be described
   by ". " if specified at all.  */
&& (arg_specified_p (str[idx + 1] - '1')


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: fix typo in attr_fnspec::verify

[Richard Biener via Gcc-patches]

On Wed, Jul 14, 2021 at 3:45 AM Alexandre Oliva  wrote:
>
> On Jul 13, 2021, Richard Biener  wrote:
>
> > oops - also worth backporting to affected branches.
>
> Thanks, I took that as explicit approval and put it in.
>
> attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
> trivial backport, just to be sure...  Will install in gcc-11 when done.

Thanks - yes, it was meant as approval ;)

Richard.

> >> * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
> >> non-'t'-sized arg check.
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: fix typo in attr_fnspec::verify

[Alexandre Oliva]

On Jul 13, 2021, Richard Biener  wrote:

> oops - also worth backporting to affected branches.

Thanks, I took that as explicit approval and put it in.

attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
trivial backport, just to be sure...  Will install in gcc-11 when done.

>> * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
>> non-'t'-sized arg check.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: fix typo in attr_fnspec::verify

[Richard Biener via Gcc-patches]

On Tue, Jul 13, 2021 at 5:15 AM Alexandre Oliva  wrote:
>
>
> Odd-numbered indices describing argument access sizes in the fnspec
> string can only hold 't' or a digit, as tested in the beginning of the
> case.  When checking that the size-supplying argument does not have
> additional information associated with it, the test that excludes the
> 't' possibility looks for it at the even position in the fnspec
> string.  Oops.
>
> This might yield false positives and negatives if a function has a
> fnspec in which an argument uses a 't' access-size, and ('t' - '1')
> happens to be the index of an argument described in an fnspec string.
> Assuming ASCII encoding, it would take a function with at least 68
> arguments described in fnspec.  Still, probably worth fixing.
>
> Regstrapped on x86_64-linux-gnu.  I'm checking this in as obvious unless
> there are objections within some 48 hours.

oops - also worth backporting to affected branches.

Richard.

>
> for  gcc/ChangeLog
>
> * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
> non-'t'-sized arg check.
> ---
>  gcc/tree-ssa-alias.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 0421bfac99869..742a95a549e20 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -3895,7 +3895,7 @@ attr_fnspec::verify ()
> && str[idx] != 'w' && str[idx] != 'W'
> && str[idx] != 'o' && str[idx] != 'O')
>   err = true;
> -   if (str[idx] != 't'
> +   if (str[idx + 1] != 't'
> /* Size specified is scalar, so it should be described
>by ". " if specified at all.  */
> && (arg_specified_p (str[idx + 1] - '1')
>
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>

fix typo in attr_fnspec::verify

[Alexandre Oliva]

Odd-numbered indices describing argument access sizes in the fnspec
string can only hold 't' or a digit, as tested in the beginning of the
case.  When checking that the size-supplying argument does not have
additional information associated with it, the test that excludes the
't' possibility looks for it at the even position in the fnspec
string.  Oops.

This might yield false positives and negatives if a function has a
fnspec in which an argument uses a 't' access-size, and ('t' - '1')
happens to be the index of an argument described in an fnspec string.
Assuming ASCII encoding, it would take a function with at least 68
arguments described in fnspec.  Still, probably worth fixing.

Regstrapped on x86_64-linux-gnu.  I'm checking this in as obvious unless
there are objections within some 48 hours.


for  gcc/ChangeLog

* tree-ssa-alias.c (attr_fnspec::verify): Fix index in
non-'t'-sized arg check.
---
 gcc/tree-ssa-alias.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 0421bfac99869..742a95a549e20 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -3895,7 +3895,7 @@ attr_fnspec::verify ()
&& str[idx] != 'w' && str[idx] != 'W'
&& str[idx] != 'o' && str[idx] != 'O')
  err = true;
-   if (str[idx] != 't'
+   if (str[idx + 1] != 't'
/* Size specified is scalar, so it should be described
   by ". " if specified at all.  */
&& (arg_specified_p (str[idx + 1] - '1')


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

[committed] openmp: Add testcases to verify OpenMP 5.0 2.14 and OpenMP 5.1 2.17 rules [PR99928]

[Jakub Jelinek via Gcc-patches]

Hi!

In preparation of PR99928 patch review, I've prepared testcases with clauses
that need more interesting handling on combined/composite constructs,
in particular firstprivate, lastprivate, firstprivate+lastprivate, linear
(explicit on non-iv, explicit on simd iv, implicit on simd iv, implicit on
simd iv declared in the construct), reduction (scalars, array sections of
array variables, array sections with pointer bases) and in_reduction.

OpenMP 5.0 had the wording broken for reduction, the intended rule to use
map(tofrom:) on target when combined with it was bound only on inscan modifier
presence which makes no sense, as then inscan may not be used, this has
been fixed in 5.1 and I'm just assuming 5.1 wording for that.

There are various cases where e.g. from historical or optimization reasons
GCC slightly deviates from the rules, but in most cases it is something
that shouldn't be really observable, e.g. whether
  #pragma omp parallel for firstprivate(x)
is handled as
  #pragma omp parallel shared(x)
  #pragma omp for firstprivate(x)
or
  #pragma omp parallel firstprivate(x)
  #pragma omp for
shouldn't be possible to distinguish in user code.  I've added FIXMEs
in the testcases about that, but maybe we just should keep it as is
(alternative would be to do it in standard compliant way and transform
into whatever we like after gimplification (e.g. early during omplower)).
Some cases we for historical reasons implement even with clauses on
constructs which in the standard don't accept them that way and then
handling those magically in omp lowering/expansion, in particular e.g.
  #pragma omp parallel for firstprivate(x) lastprivate(x)
we treat as
  #pragma omp parallel firstprivate(x) lastprivate(x)
  #pragma omp for
even when lastprivate is not valid on parallel.  Maybe one day we
could change that if we make sure we don't regress generated code quality.

I've also found a bug in OpenMP 5.0/5.1,
  #pragma omp parallel sections firstprivate(x) lastprivate(x)
incorrectly says that it should be handled as
  #pragma omp parallel firstprivate(x)
  #pragma omp sections lastprivate(x)
which when written that way results in error; filed as
https://github.com/OpenMP/spec/issues/2758
to be fixed in OpenMP 5.2.  GCC handles it the way it used to do
and users expect, so nothing to fix on the GCC side.

Also, we don't support yet in_reduction clause on target construct,
which means the -11.c testcase can't include any tests about in_reduction
handling on all the composite constructs that include target.

The work found two kinds of bugs on the GCC side, one is the known thing
that we implement still the 4.5 behavior and don't mark for
lastprivate/linear/reduction the list item as map(tofrom:) as mentioned
in PR99928.  These cases are xfailed in the tests.

And another one is with r21 and r28 in -{8,9,10}.c tests - we don't add
reduction clause on teams for
  #pragma omp {target ,}teams distribute simd reduction(+:r)
even when the spec says that teams shouldn't receive reduction only
when combined with loop construct.

In
make check-gcc check-g++ RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} 
gomp.exp=pr99928*'
testing this shows:

  # of expected passes  5648
  # of expected failures872

and with Tobias' patch applied:

  # of expected passes  5648
  # of unexpected successes 384
  # of expected failures488

Committed to trunk.

2021-05-13  Jakub Jelinek  

PR middle-end/99928
* c-c++-common/gomp/pr99928-1.c: New test.
* c-c++-common/gomp/pr99928-2.c: New test.
* c-c++-common/gomp/pr99928-3.c: New test.
* c-c++-common/gomp/pr99928-4.c: New test.
* c-c++-common/gomp/pr99928-5.c: New test.
* c-c++-common/gomp/pr99928-6.c: New test.
* c-c++-common/gomp/pr99928-7.c: New test.
* c-c++-common/gomp/pr99928-8.c: New test.
* c-c++-common/gomp/pr99928-9.c: New test.
* c-c++-common/gomp/pr99928-10.c: New test.
* c-c++-common/gomp/pr99928-11.c: New test.

--- gcc/testsuite/c-c++-common/gomp/pr99928-1.c.jj  2021-05-13 
12:32:49.240146205 +0200
+++ gcc/testsuite/c-c++-common/gomp/pr99928-1.c 2021-05-13 12:32:49.240146205 
+0200
@@ -0,0 +1,206 @@
+/* PR middle-end/99928 */
+/* { dg-do compile } */
+/* { dg-options "-fopenmp -fdump-tree-gimple" } */
+
+int f00, f01, f02, f03, f04, f05, f06, f07, f08, f09;
+int f12, f13, f14, f15, f16, f17, f18, f19;
+int f20, f21, f22, f23, f24, f25, f26, f27, f28, f29;
+
+void
+foo (void)
+{
+  /* { dg-final { scan-tree-dump "omp 
distribute\[^\n\r]*firstprivate\\(f00\\)" "gimple" } } */
+  /* { dg-final { scan-tree-dump "omp parallel\[^\n\r]*firstprivate\\(f00\\)" 
"gimple" } } *//* FIXME: This should be on for instead.  */
+  /* { dg-final { scan-tree-dump-not "omp for\[^\n\r]*firstprivate\\(f00\\)" 
"gimple" } } *//* FIXME.  */
+  #pragma omp distribute parallel for firstprivate (f00)
+  for (int i = 0; i < 64; i++)
+f00++;
+  /* { dg-final { 

Re: [PATCH] libgfortran: Verify the presence of all functions for POSIX 2008 locale

[Maciej W. Rozycki]

On Wed, 25 Nov 2020, Tobias Burnus wrote:

> Patch looks good to me.

 This has now been committed, thank you for your review.

> PS: Are there still remaining issues to be solved? Or does libgfortran
> now build?

 Sadly this is not enough for VAX/NetBSD, but as I gather from a comment 
in the other thread it is for IEEE-754-FP (and soft-float if any) NetBSD 
ports.

  Maciej

Re: [PATCH] libgfortran: Verify the presence of all functions for POSIX 2008 locale

[Tobias Burnus]

Patch looks good to me.

Thanks,

Tobias

PS: Are there still remaining issues to be solved? Or does libgfortran
now build?

On 25.11.20 19:14, Maciej W. Rozycki wrote:

While we have `configure' checks for the individual POSIX 2008 extended
locale functions we refer to and use to guard the respective call sites,
we only verify the presence of `newlocale' for our global feature enable
check.  Consequently compilation fails for targets like NetBSD that only
have partial support for POSIX 2008 locale features and in particular
lack the `uselocale' function:

.../libgfortran/io/transfer.c: In function 'data_transfer_init_worker':
.../libgfortran/io/transfer.c:3416:30: error:
'old_locale_lock' undeclared (first use in this function)
  3416 |   __gthread_mutex_lock (_locale_lock);
   |  ^~~
.../libgfortran/io/transfer.c:3416:30: note: each undeclared identifier is 
reported only once for each function it appears in
.../libgfortran/io/transfer.c:3417:12: error:
'old_locale_ctr' undeclared (first use in this function)
  3417 |   if (!old_locale_ctr++)
   |^~
.../libgfortran/io/transfer.c:3419:11: error:
'old_locale' undeclared (first use in this function); did you mean 'c_locale'?
  3419 |   old_locale = setlocale (LC_NUMERIC, NULL);
   |   ^~
   |   c_locale
.../libgfortran/io/transfer.c: In function 'finalize_transfer':
.../libgfortran/io/transfer.c:4253:26: error:
'old_locale_lock' undeclared (first use in this function)
  4253 |   __gthread_mutex_lock (_locale_lock);
   |  ^~~
.../libgfortran/io/transfer.c:4254:10: error:
'old_locale_ctr' undeclared (first use in this function)
  4254 |   if (!--old_locale_ctr)
   |  ^~
.../libgfortran/io/transfer.c:4256:30: error:
'old_locale' undeclared (first use in this function); did you mean 'c_locale'?
  4256 |   setlocale (LC_NUMERIC, old_locale);
   |  ^~
   |  c_locale
make[3]: *** [Makefile:6221: transfer.lo] Error 1

Only enable the use of POSIX 2008 extended locale features then when all
the three functions required are present, removing said build errors.

  libgfortran/
  * io/io.h [HAVE_NEWLOCALE]: Also check for HAVE_FREELOCALE and
  HAVE_USELOCALE.
  [HAVE_FREELOCALE && HAVE_NEWLOCALE && HAVE_USELOCALE]
  (HAVE_POSIX_2008_LOCALE): New macro.
  (st_parameter_dt) [HAVE_NEWLOCALE]: Check for
  HAVE_POSIX_2008_LOCALE instead.
  * io/transfer.c (data_transfer_init_worker, finalize_transfer)
  [HAVE_USELOCALE]: Check for HAVE_POSIX_2008_LOCALE instead.
  * io/unit.c [HAVE_NEWLOCALE]: Likewise.
  (init_units) [HAVE_NEWLOCALE]: Likewise.
  (close_units) [HAVE_FREELOCALE]: Likewise.
  * runtime/error.c (gf_strerror) [HAVE_USELOCALE]: Likewise.
---
  libgfortran/io/io.h |   10 +++---
  libgfortran/io/transfer.c   |4 ++--
  libgfortran/io/unit.c   |6 +++---
  libgfortran/runtime/error.c |2 +-
  4 files changed, 13 insertions(+), 9 deletions(-)

gcc-netbsd-libgfortran-locale.diff
Index: gcc/libgfortran/io/io.h
===
--- gcc.orig/libgfortran/io/io.h
+++ gcc/libgfortran/io/io.h
@@ -52,8 +52,12 @@ struct format_data;
  typedef struct fnode fnode;
  struct gfc_unit;

-#ifdef HAVE_NEWLOCALE
-/* We have POSIX 2008 extended locale stuff.  */
+#if defined (HAVE_FREELOCALE) && defined (HAVE_NEWLOCALE) \
+  && defined (HAVE_USELOCALE)
+/* We have POSIX 2008 extended locale stuff.  We only choose to use it
+   if all the functions required are present as some systems, e.g. NetBSD
+   do not have `uselocale'.  */
+#define HAVE_POSIX_2008_LOCALE
  extern locale_t c_locale;
  internal_proto(c_locale);
  #else
@@ -562,7 +566,7 @@ typedef struct st_parameter_dt
char *line_buffer;
struct format_data *fmt;
namelist_info *ionml;
-#ifdef HAVE_NEWLOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
locale_t old_locale;
  #endif
/* Current position within the look-ahead line buffer.  */
Index: gcc/libgfortran/io/transfer.c
===
--- gcc.orig/libgfortran/io/transfer.c
+++ gcc/libgfortran/io/transfer.c
@@ -3410,7 +3410,7 @@ data_transfer_init_worker (st_parameter_

if (dtp->u.p.current_unit->flags.form == FORM_FORMATTED)
  {
-#ifdef HAVE_USELOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
dtp->u.p.old_locale = uselocale (c_locale);
  #else
__gthread_mutex_lock (_locale_lock);
@@ -4243,7 +4243,7 @@ finalize_transfer (st_parameter_dt *dtp)
  }
  }

-#ifdef HAVE_USELOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
if (dtp->u.p.old_locale != (locale_t) 0)
  {
uselocale (dtp->u.

[PATCH] libgfortran: Verify the presence of all functions for POSIX 2008 locale

[Maciej W. Rozycki]

While we have `configure' checks for the individual POSIX 2008 extended 
locale functions we refer to and use to guard the respective call sites, 
we only verify the presence of `newlocale' for our global feature enable 
check.  Consequently compilation fails for targets like NetBSD that only 
have partial support for POSIX 2008 locale features and in particular 
lack the `uselocale' function:

.../libgfortran/io/transfer.c: In function 'data_transfer_init_worker':
.../libgfortran/io/transfer.c:3416:30: error:
'old_locale_lock' undeclared (first use in this function)
 3416 |   __gthread_mutex_lock (_locale_lock);
  |  ^~~
.../libgfortran/io/transfer.c:3416:30: note: each undeclared identifier is 
reported only once for each function it appears in
.../libgfortran/io/transfer.c:3417:12: error:
'old_locale_ctr' undeclared (first use in this function)
 3417 |   if (!old_locale_ctr++)
  |^~
.../libgfortran/io/transfer.c:3419:11: error:
'old_locale' undeclared (first use in this function); did you mean 'c_locale'?
 3419 |   old_locale = setlocale (LC_NUMERIC, NULL);
  |   ^~
  |   c_locale
.../libgfortran/io/transfer.c: In function 'finalize_transfer':
.../libgfortran/io/transfer.c:4253:26: error:
'old_locale_lock' undeclared (first use in this function)
 4253 |   __gthread_mutex_lock (_locale_lock);
  |  ^~~
.../libgfortran/io/transfer.c:4254:10: error:
'old_locale_ctr' undeclared (first use in this function)
 4254 |   if (!--old_locale_ctr)
  |  ^~
.../libgfortran/io/transfer.c:4256:30: error:
'old_locale' undeclared (first use in this function); did you mean 'c_locale'?
 4256 |   setlocale (LC_NUMERIC, old_locale);
  |  ^~
  |  c_locale
make[3]: *** [Makefile:6221: transfer.lo] Error 1

Only enable the use of POSIX 2008 extended locale features then when all 
the three functions required are present, removing said build errors.

libgfortran/
* io/io.h [HAVE_NEWLOCALE]: Also check for HAVE_FREELOCALE and
HAVE_USELOCALE.
[HAVE_FREELOCALE && HAVE_NEWLOCALE && HAVE_USELOCALE]
(HAVE_POSIX_2008_LOCALE): New macro.
(st_parameter_dt) [HAVE_NEWLOCALE]: Check for
HAVE_POSIX_2008_LOCALE instead.
* io/transfer.c (data_transfer_init_worker, finalize_transfer)
[HAVE_USELOCALE]: Check for HAVE_POSIX_2008_LOCALE instead.
* io/unit.c [HAVE_NEWLOCALE]: Likewise.
(init_units) [HAVE_NEWLOCALE]: Likewise.
(close_units) [HAVE_FREELOCALE]: Likewise.
* runtime/error.c (gf_strerror) [HAVE_USELOCALE]: Likewise.
---
 libgfortran/io/io.h |   10 +++---
 libgfortran/io/transfer.c   |4 ++--
 libgfortran/io/unit.c   |6 +++---
 libgfortran/runtime/error.c |2 +-
 4 files changed, 13 insertions(+), 9 deletions(-)

gcc-netbsd-libgfortran-locale.diff
Index: gcc/libgfortran/io/io.h
===
--- gcc.orig/libgfortran/io/io.h
+++ gcc/libgfortran/io/io.h
@@ -52,8 +52,12 @@ struct format_data;
 typedef struct fnode fnode;
 struct gfc_unit;
 
-#ifdef HAVE_NEWLOCALE
-/* We have POSIX 2008 extended locale stuff.  */
+#if defined (HAVE_FREELOCALE) && defined (HAVE_NEWLOCALE) \
+  && defined (HAVE_USELOCALE)
+/* We have POSIX 2008 extended locale stuff.  We only choose to use it
+   if all the functions required are present as some systems, e.g. NetBSD
+   do not have `uselocale'.  */
+#define HAVE_POSIX_2008_LOCALE
 extern locale_t c_locale;
 internal_proto(c_locale);
 #else
@@ -562,7 +566,7 @@ typedef struct st_parameter_dt
  char *line_buffer;
  struct format_data *fmt;
  namelist_info *ionml;
-#ifdef HAVE_NEWLOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
  locale_t old_locale;
 #endif
  /* Current position within the look-ahead line buffer.  */
Index: gcc/libgfortran/io/transfer.c
===
--- gcc.orig/libgfortran/io/transfer.c
+++ gcc/libgfortran/io/transfer.c
@@ -3410,7 +3410,7 @@ data_transfer_init_worker (st_parameter_
 
   if (dtp->u.p.current_unit->flags.form == FORM_FORMATTED)
 {
-#ifdef HAVE_USELOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
   dtp->u.p.old_locale = uselocale (c_locale);
 #else
   __gthread_mutex_lock (_locale_lock);
@@ -4243,7 +4243,7 @@ finalize_transfer (st_parameter_dt *dtp)
}
 }
 
-#ifdef HAVE_USELOCALE
+#ifdef HAVE_POSIX_2008_LOCALE
   if (dtp->u.p.old_locale != (locale_t) 0)
 {
   uselocale (dtp->u.p.old_locale);
Index: gcc/libgfortran/io/unit.c
===
--- gcc.orig/libgfortran/io/unit.c
+++ gcc/libgfortran/io/unit.c
@@ -114,7 +114,7 @@ static char std

Re: [PATCH 0/2] Add tests to verify OpenACC clause locations

[Thomas Schwinge]

Hi!

On 2020-11-03T09:17:49+0100, I wrote:
> I've just pushed "[OpenACC] More precise diagnostics for 'gang',
> 'worker', 'vector' clauses with arguments on 'loop' only allowed in
> 'kernels' regions" to master branch in commit
> beddd1762ad2bbe84dd776c54489153f83f21e56, and backported to
> releases/gcc-10 in commit 8d09f49006ce4c2f8d4018206c12e131c49ca6ce

> [...], and 'inform' at the location of the enclosing parent
> compute construct/[...].

Now really.  I've pushed "[OpenACC] Use proper location to 'inform' of
enclosing parent compute construct" to master branch in commit
fab72592d86d11b89a01f0f3c2c9c329d43466c1, and backported to
releases/gcc-10 branch in commit
a32d089dcf3edaa625e4871e78150b7d297cda5b, see attached.


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From fab72592d86d11b89a01f0f3c2c9c329d43466c1 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 3 Nov 2020 22:06:29 +0100
Subject: [PATCH] [OpenACC] Use proper location to 'inform' of enclosing parent
 compute construct

Bug fix for recent commit beddd1762ad2bbe84dd776c54489153f83f21e56 "[OpenACC]
More precise diagnostics for 'gang', 'worker', 'vector' clauses with arguments
on 'loop' only allowed in 'kernels' regions":

> [...], and 'inform' at the location of the enclosing parent
> compute construct/[...].

Now really.

	gcc/
	* omp-low.c (scan_omp_for) : Use proper location to
	'inform' of enclosing parent compute construct.
	gcc/testsuite/
	* c-c++-common/goacc/pr92793-1.c: Extend.
	* gfortran.dg/goacc/pr92793-1.f90: Likewise.
---
 gcc/omp-low.c |  2 +-
 gcc/testsuite/c-c++-common/goacc/pr92793-1.c  | 58 +--
 gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 55 --
 3 files changed, 104 insertions(+), 11 deletions(-)

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 2f1a544bd46e..ea9008b61c41 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2443,7 +2443,7 @@ scan_omp_for (gomp_for *stmt, omp_context *outer_ctx)
 			  "argument not permitted on %qs clause",
 			  omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
 		if (tgt)
-		  inform (gimple_location (outer_ctx->stmt),
+		  inform (gimple_location (tgt->stmt),
 			  "enclosing parent compute construct");
 		else if (oacc_get_fn_attrib (current_function_decl))
 		  inform (DECL_SOURCE_LOCATION (current_function_decl),
diff --git a/gcc/testsuite/c-c++-common/goacc/pr92793-1.c b/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
index 77ebb20265cf..71a556e27513 100644
--- a/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
@@ -57,7 +57,7 @@ reduction(-:sum  ) /* { dg-line sum2 } */ \
 
 
 void
-a_sl() {
+gwv_sl_1() {
 #pragma acc serial loop /* { dg-message "9: enclosing parent compute construct" } */ \
 gang(num:5) /* { dg-error "5: argument not permitted on 'gang' clause" } */ \
   worker(num:5) /* { dg-error "3: argument not permitted on 'worker' clause" } */ \
@@ -67,7 +67,25 @@ a_sl() {
 }
 
 void
-a_s_l() {
+gwv_sl_2() {
+#pragma acc serial loop /* { dg-message "9: enclosing parent compute construct" } */
+  for (int i = 0; i < 10; i++)
+{
+#pragma acc loop /* { dg-bogus "enclosing parent compute construct" } */
+  for (int j = 0; j < 10; j++)
+	{
+#pragma acc loop \
+gang(num:5) /* { dg-error "9: argument not permitted on 'gang' clause" } */ \
+worker(num:5) /* { dg-error "5: argument not permitted on 'worker' clause" } */ \
+  vector(length:5) /* { dg-error "3: argument not permitted on 'vector' clause" } */
+	  for (int k = 0; k < 10; k++)
+	;
+	}
+}
+}
+
+void
+gwv_s_l() {
 #pragma acc serial /* { dg-message "9: enclosing parent compute construct" } */
   {
 #pragma acc loop \
@@ -76,18 +94,48 @@ a_s_l() {
   vector(length:5) /* { dg-error "3: argument not permitted on 'vector' clause" } */
 for (int i = 0; i < 10; i++)
   ;
+
+#pragma acc loop
+for (int i = 0; i < 10; i++)
+  {
+#pragma acc loop /* { dg-bogus "enclosing parent compute construct" } */
+	for (int j = 0; j < 10; j++)
+	  {
+#pragma acc loop \
+ gang(num:5) /* { dg-error "10: argument not permitted on 'gang' clause" } */ \
+  worker(num:5) /* { dg-error "7: argument not permitted on 'worker' clause" } */ \
+vector(length:5) /* { dg-error "5: argument not permitted on 'vector' clause" } */
+	for (int k = 0; k < 10; k++)
+	  ;
+	  }
+  }
   }
 }
 
-void a_r();
-#pragma acc routine(a_r)
+void gwv_r();
+#pragma acc routine(gwv_r)
 
 void
-a_r() { /* { dg-message "1: enclosing routine" } */
+gwv_r() { /* { dg-message "1: enclosing routine" } */
 #pragma acc loop \
gang(num:5) /* { dg-error "4: argument not permitted on 'gang' clause" } */ \
 worker(num:5) /* { dg-error "5: argument not permitted on 'worker' clause" } */ \
   vector(length:5) /* { dg-error "3: argument not permitted 

Re: [PATCH 0/2] Add tests to verify OpenACC clause locations

[Thomas Schwinge]

Hi!

On 2019-12-10T15:23:01+0100, Frederik Harwath  wrote:
> On 09.12.19 16:58, Harwath, Frederik wrote:
>> [use] the location of clauses in warnings instead of the location of the 
>> loop to which the clause belongs.

> Frederik Harwath (2):
>   Use clause locations in OpenACC nested reduction warnings

Basically:

-warning_at (gimple_location (stmt), 0,
+warning_at (OMP_CLAUSE_LOCATION (clause), 0,

>   Add tests to verify OpenACC clause locations

Similar changes are desirable for other directives/clauses, too.

I've just pushed "[OpenACC] More precise diagnostics for 'gang',
'worker', 'vector' clauses with arguments on 'loop' only allowed in
'kernels' regions" to master branch in commit
beddd1762ad2bbe84dd776c54489153f83f21e56, and backported to
releases/gcc-10 in commit 8d09f49006ce4c2f8d4018206c12e131c49ca6ce, see
attached.


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From beddd1762ad2bbe84dd776c54489153f83f21e56 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 27 Oct 2020 17:13:16 +0100
Subject: [PATCH] [OpenACC] More precise diagnostics for 'gang', 'worker',
 'vector' clauses with arguments on 'loop' only allowed in 'kernels' regions

Instead of at the location of the 'loop' directive, 'error_at' the location of
the improper clause, and 'inform' at the location of the enclosing parent
compute construct/routine.

The Fortran testcases come with some XFAILing, to be resolved later.

	gcc/
	* omp-low.c (scan_omp_for) : More precise diagnostics for
	'gang', 'worker', 'vector' clauses with arguments only allowed in
	'kernels' regions.
	gcc/testsuite/
	* c-c++-common/goacc/pr92793-1.c: Extend.
	* gfortran.dg/goacc/pr92793-1.f90: Likewise.
---
 gcc/omp-low.c | 29 +++
 gcc/testsuite/c-c++-common/goacc/pr92793-1.c  | 37 +
 gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 52 +++
 3 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 5392fa7e3086..de5142f979b0 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2418,30 +2418,39 @@ scan_omp_for (gomp_for *stmt, omp_context *outer_ctx)
   if (!tgt || is_oacc_parallel_or_serial (tgt))
 	for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
 	  {
-	char const *check = NULL;
-
+	tree c_op0;
 	switch (OMP_CLAUSE_CODE (c))
 	  {
 	  case OMP_CLAUSE_GANG:
-		check = "gang";
+		c_op0 = OMP_CLAUSE_GANG_EXPR (c);
 		break;
 
 	  case OMP_CLAUSE_WORKER:
-		check = "worker";
+		c_op0 = OMP_CLAUSE_WORKER_EXPR (c);
 		break;
 
 	  case OMP_CLAUSE_VECTOR:
-		check = "vector";
+		c_op0 = OMP_CLAUSE_VECTOR_EXPR (c);
 		break;
 
 	  default:
-		break;
+		continue;
 	  }
 
-	if (check && OMP_CLAUSE_OPERAND (c, 0))
-	  error_at (gimple_location (stmt),
-			"argument not permitted on %qs clause in"
-			" OpenACC % or %", check);
+	if (c_op0)
+	  {
+		error_at (OMP_CLAUSE_LOCATION (c),
+			  "argument not permitted on %qs clause",
+			  omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+		if (tgt)
+		  inform (gimple_location (outer_ctx->stmt),
+			  "enclosing parent compute construct");
+		else if (oacc_get_fn_attrib (current_function_decl))
+		  inform (DECL_SOURCE_LOCATION (current_function_decl),
+			  "enclosing routine");
+		else
+		  gcc_unreachable ();
+	  }
 	  }
 
   if (tgt && is_oacc_kernels (tgt))
diff --git a/gcc/testsuite/c-c++-common/goacc/pr92793-1.c b/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
index d7a2ae487992..77ebb20265cf 100644
--- a/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/pr92793-1.c
@@ -54,3 +54,40 @@ reduction(-:sum  ) /* { dg-line sum2 } */ \
   }
   }
 }
+
+
+void
+a_sl() {
+#pragma acc serial loop /* { dg-message "9: enclosing parent compute construct" } */ \
+gang(num:5) /* { dg-error "5: argument not permitted on 'gang' clause" } */ \
+  worker(num:5) /* { dg-error "3: argument not permitted on 'worker' clause" } */ \
+   vector(length:5) /* { dg-error "4: argument not permitted on 'vector' clause" } */
+  for (int i = 0; i < 10; i++)
+;
+}
+
+void
+a_s_l() {
+#pragma acc serial /* { dg-message "9: enclosing parent compute construct" } */
+  {
+#pragma acc loop \
+   gang(num:5) /* { dg-error "8: argument not permitted on 'gang' clause" } */ \
+   worker(num:5) /* { dg-error "4: argument not permitted on 'worker' clause" } */ \
+  vector(length:5) /* { dg-error "3: argument not permitted on 'vector' clause" } */
+for (int i = 0; i < 10; i++)
+  ;
+  }
+}
+
+void a_r();
+#pragma acc routine(a_r)
+
+void
+a_r() { /*

Re: [PATCH v2] c++: Verify 'this' of NS member functions in constexpr [PR97230]

[Jason Merrill via Gcc-patches]

On 10/7/20 2:48 PM, Marek Polacek wrote:

On Tue, Oct 06, 2020 at 01:06:37AM -0400, Jason Merrill via Gcc-patches wrote:

On 10/1/20 1:08 PM, Marek Polacek wrote:

@@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
new_obj = NULL_TREE;
}
   }
+   /* Verify that the object we're invoking the function on is sane.  */
+  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
+  /* maybe_add_lambda_conv_op creates a null 'this' pointer.  */
+  && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))


Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; we
don't want to allow calling a lambda op() with a null object from other
contexts.


OK, fixed.


+{
+  tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
+  if (integer_zerop (thisarg))
+   {
+ if (!ctx->quiet)
+   error_at (loc, "member call on null pointer is not allowed "
+ "in a constant expression");
+ *non_constant_p = true;
+ result = error_mark_node;
+   }
+  else
+   {
+ STRIP_NOPS (thisarg);
+ if (TREE_CODE (thisarg) == ADDR_EXPR)
+   thisarg = TREE_OPERAND (thisarg, 0);
+ /* Detect out-of-bounds accesses.  */
+ if (TREE_CODE (thisarg) == ARRAY_REF)
+   {
+ eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
+ non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+   }
+ /* Detect using an inactive member of a union.  */
+ else if (TREE_CODE (thisarg) == COMPONENT_REF)
+   {
+ cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
+   non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+   }
+ /* Detect other invalid accesses like
-  tree result = NULL_TREE;
+  X x;
+  ()[1].foo();
+
+where we'll end up with  p+ 1.  */


Is there any way to combine this POINTER_PLUS_EXPR handling with
cxx_fold_pointer_plus_expression or cxx_fold_indirect_ref?


cxx_fold_pointer_plus_expression looked like a place where the new checking
could go but in the end it didn't work.  E.g., it STRIP_NOPS the lhs of the
POINTER_PLUS_EXPR so any casts would get lost.



+ else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
+   {
+ tree op0 = TREE_OPERAND (thisarg, 0);
+ /* This shouldn't trigger if we're accessing a base class of
+the object in question.  */
+ if (TREE_CODE (op0) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (op0, 0)))
+   {
+ if (!ctx->quiet)
+   {
+ tree op1 = TREE_OPERAND (thisarg, 1);
+ if (integer_onep (op1))
+   error_at (loc, "cannot dereference one-past-the-end "
+ "pointer in a constant expression");
+ else
+   error_at (loc, "cannot access element %qE of a "
+ "non-array object in a constant expression",
+ op1);
+   }
+ *non_constant_p = true;
+ result = error_mark_node;
+   }
+   }
+   }
+}


These checks seem like the requirement that "A reference shall be
initialized to refer to a valid object or function.", which matches with the
change to describe the object argument as a reference rather than a pointer.
Let's split this out into a separate function that expresses this, and
perhaps then also use it to check the initializer of a reference variable.


I've moved it to a new function called cxx_verify_object_argument.

But checking the initializers of reference variables is still not handled
in this patch, because I don't know how to do that.  For something like

   constexpr const int  = ()[1].m;

store_init_value calls cxx_constant_init for the init, which is

   (const int &) &( + 4)->m

It could also be something like

   (const int &) [3].m


That could also be an object argument for a member function, right?  I'm 
not sure what point you're making.



Maybe cxx_eval_constant_expression/NOP_EXPR should set some flag so that
we perform some checking when we encounter a POINTER_PLUS_EXPR or an
ARRAY_REF when evaluating op0 of the NOP_EXPR?


Hmm, I suppose it might make sense to check for a valid object when we 
see a conversion to reference rather than wait for initialization.



I've attached constexpr-ref13.C which is a testcase I played with; we
diagnose no errors there.


And let's call it from cxx_bind_parameters_in_call.


Done.

Boot

Re: [PATCH v2] c++: Verify 'this' of NS member functions in constexpr [PR97230]

[Marek Polacek via Gcc-patches]

On Tue, Oct 06, 2020 at 01:06:37AM -0400, Jason Merrill via Gcc-patches wrote:
> On 10/1/20 1:08 PM, Marek Polacek wrote:
> > @@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, 
> > tree t,
> > new_obj = NULL_TREE;
> >     }
> >   }
> > +   /* Verify that the object we're invoking the function on is sane.  */
> > +  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
> > +  /* maybe_add_lambda_conv_op creates a null 'this' pointer.  */
> > +  && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))
> 
> Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; we
> don't want to allow calling a lambda op() with a null object from other
> contexts.

OK, fixed.

> > +{
> > +  tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
> > +  if (integer_zerop (thisarg))
> > +   {
> > + if (!ctx->quiet)
> > +   error_at (loc, "member call on null pointer is not allowed "
> > + "in a constant expression");
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > +   }
> > +  else
> > +   {
> > + STRIP_NOPS (thisarg);
> > + if (TREE_CODE (thisarg) == ADDR_EXPR)
> > +   thisarg = TREE_OPERAND (thisarg, 0);
> > + /* Detect out-of-bounds accesses.  */
> > + if (TREE_CODE (thisarg) == ARRAY_REF)
> > +   {
> > + eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
> > + non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > +   result = error_mark_node;
> > +   }
> > + /* Detect using an inactive member of a union.  */
> > + else if (TREE_CODE (thisarg) == COMPONENT_REF)
> > +   {
> > + cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
> > +   non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > +   result = error_mark_node;
> > +   }
> > + /* Detect other invalid accesses like
> > -  tree result = NULL_TREE;
> > +  X x;
> > +  ()[1].foo();
> > +
> > +where we'll end up with  p+ 1.  */
> 
> Is there any way to combine this POINTER_PLUS_EXPR handling with
> cxx_fold_pointer_plus_expression or cxx_fold_indirect_ref?

cxx_fold_pointer_plus_expression looked like a place where the new checking
could go but in the end it didn't work.  E.g., it STRIP_NOPS the lhs of the
POINTER_PLUS_EXPR so any casts would get lost.

> > + else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
> > +   {
> > + tree op0 = TREE_OPERAND (thisarg, 0);
> > + /* This shouldn't trigger if we're accessing a base class of
> > +the object in question.  */
> > + if (TREE_CODE (op0) == ADDR_EXPR
> > + && DECL_P (TREE_OPERAND (op0, 0)))
> > +   {
> > + if (!ctx->quiet)
> > +   {
> > + tree op1 = TREE_OPERAND (thisarg, 1);
> > + if (integer_onep (op1))
> > +   error_at (loc, "cannot dereference one-past-the-end "
> > + "pointer in a constant expression");
> > + else
> > +   error_at (loc, "cannot access element %qE of a "
> > + "non-array object in a constant expression",
> > + op1);
> > +   }
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > +   }
> > +   }
> > +   }
> > +}
> 
> These checks seem like the requirement that "A reference shall be
> initialized to refer to a valid object or function.", which matches with the
> change to describe the object argument as a reference rather than a pointer.
> Let's split this out into a separate function that expresses this, and
> perhaps then also use it to check the initializer of a reference variable.

I've moved it to a new function called cxx_verify_object_argument.

But checking the initializers of reference variables is still not handled
in this patch, because I don't know how to do that.  For something like

  constexpr const int  = ()[1].m;

store_init_value calls cxx_constant_init for the init, which is

  (const int &) &( + 4)->m

It could also be something like

  (const int &) [3].m

Maybe cxx_eval_constant_expression/NOP_EXPR should set some flag so that
we perform some checking when we encounter a POINT

Re: [PATCH] c++: Verify 'this' of NS member functions in constexpr [PR97230]

[Jason Merrill via Gcc-patches]

On 10/1/20 1:08 PM, Marek Polacek wrote:

This PR points out that when we're invoking a non-static member function
on a null instance during constant evaluation, we should reject.
cxx_eval_call_expression calls cxx_bind_parameters_in_call which
evaluates function arguments, but it won't detect problems like these.

Well, ok, so use integer_zerop to detect a null 'this'.  This also
detects member calls on a variable whose lifetime has ended, because
check_return_expr creates an artificial nullptr:
10195   else if (!processing_template_decl
10196&& maybe_warn_about_returning_address_of_local (retval, 
loc)
10197&& INDIRECT_TYPE_P (valtype))
10198 retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
10199  build_zero_cst (TREE_TYPE (retval)));
It would be great if we could somehow distinguish between those two
cases, but experiments with setting TREE_THIS_VOLATILE on the zero
didn't work, so I left it be.

But by the same token, we should detect out-of-bounds accesses.  For
this I'm (ab)using eval_and_check_array_index so that I don't have
to reimplement bounds checking yet again.  But this only works for
ARRAY_REFs, so won't detect

   X x;
   ()[0].foo(); // ok
   ()[1].foo(); // bad

so I've added a special handling of POINTER_PLUS_EXPRs.

While here, we should also detect using an inactive union member.  For
that, I'm using cxx_eval_component_reference.

Does this approach seem sensible?

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

PR c++/97230
* constexpr.c (eval_and_check_array_index): Forward declare.
(cxx_eval_component_reference): Likewise.
(cxx_eval_call_expression): Verify the 'this' pointer for
non-static member functions.

gcc/testsuite/ChangeLog:

PR c++/97230
* g++.dg/cpp0x/constexpr-member-fn1.C: New test.
---
  gcc/cp/constexpr.c| 72 ++-
  .../g++.dg/cpp0x/constexpr-member-fn1.C   | 44 
  2 files changed, 115 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a118f8a810b..f62f37ce384 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2181,6 +2181,11 @@ cxx_eval_thunk_call (const constexpr_ctx *ctx, tree t, 
tree thunk_fndecl,
   non_constant_p, overflow_p);
  }
  
+static tree eval_and_check_array_index (const constexpr_ctx *, tree, bool,

+   bool *, bool *);
+static tree cxx_eval_component_reference (const constexpr_ctx *, tree,
+ bool, bool *, bool *);
+
  /* Subroutine of cxx_eval_constant_expression.
 Evaluate the call expression tree T in the context of OLD_CALL expression
 evaluation.  */
@@ -2467,6 +2472,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
if (*non_constant_p)
  return t;
  
+  tree result = NULL_TREE;

depth_ok = push_cx_call_context (t);
  
/* Remember the object we are constructing.  */

@@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
new_obj = NULL_TREE;
}
  }
+   /* Verify that the object we're invoking the function on is sane.  */
+  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
+  /* maybe_add_lambda_conv_op creates a null 'this' pointer.  */
+  && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))


Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; 
we don't want to allow calling a lambda op() with a null object from 
other contexts.



+{
+  tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
+  if (integer_zerop (thisarg))
+   {
+ if (!ctx->quiet)
+   error_at (loc, "member call on null pointer is not allowed "
+ "in a constant expression");
+ *non_constant_p = true;
+ result = error_mark_node;
+   }
+  else
+   {
+ STRIP_NOPS (thisarg);
+ if (TREE_CODE (thisarg) == ADDR_EXPR)
+   thisarg = TREE_OPERAND (thisarg, 0);
+ /* Detect out-of-bounds accesses.  */
+ if (TREE_CODE (thisarg) == ARRAY_REF)
+   {
+ eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
+ non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+   }
+ /* Detect using an inactive member of a union.  */
+ else if (TREE_CODE (thisarg) == COMPONENT_REF)
+   {
+ cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
+   non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+

[PATCH] c++: Verify 'this' of NS member functions in constexpr [PR97230]

[Marek Polacek via Gcc-patches]

This PR points out that when we're invoking a non-static member function
on a null instance during constant evaluation, we should reject.
cxx_eval_call_expression calls cxx_bind_parameters_in_call which
evaluates function arguments, but it won't detect problems like these.

Well, ok, so use integer_zerop to detect a null 'this'.  This also
detects member calls on a variable whose lifetime has ended, because
check_return_expr creates an artificial nullptr:
10195   else if (!processing_template_decl
10196&& maybe_warn_about_returning_address_of_local (retval, 
loc)
10197&& INDIRECT_TYPE_P (valtype))
10198 retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
10199  build_zero_cst (TREE_TYPE (retval)));
It would be great if we could somehow distinguish between those two
cases, but experiments with setting TREE_THIS_VOLATILE on the zero
didn't work, so I left it be.

But by the same token, we should detect out-of-bounds accesses.  For
this I'm (ab)using eval_and_check_array_index so that I don't have
to reimplement bounds checking yet again.  But this only works for
ARRAY_REFs, so won't detect

  X x;
  ()[0].foo(); // ok
  ()[1].foo(); // bad

so I've added a special handling of POINTER_PLUS_EXPRs.

While here, we should also detect using an inactive union member.  For
that, I'm using cxx_eval_component_reference.

Does this approach seem sensible?

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

PR c++/97230
* constexpr.c (eval_and_check_array_index): Forward declare.
(cxx_eval_component_reference): Likewise.
(cxx_eval_call_expression): Verify the 'this' pointer for
non-static member functions.

gcc/testsuite/ChangeLog:

PR c++/97230
* g++.dg/cpp0x/constexpr-member-fn1.C: New test.
---
 gcc/cp/constexpr.c| 72 ++-
 .../g++.dg/cpp0x/constexpr-member-fn1.C   | 44 
 2 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a118f8a810b..f62f37ce384 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2181,6 +2181,11 @@ cxx_eval_thunk_call (const constexpr_ctx *ctx, tree t, 
tree thunk_fndecl,
   non_constant_p, overflow_p);
 }
 
+static tree eval_and_check_array_index (const constexpr_ctx *, tree, bool,
+   bool *, bool *);
+static tree cxx_eval_component_reference (const constexpr_ctx *, tree,
+ bool, bool *, bool *);
+
 /* Subroutine of cxx_eval_constant_expression.
Evaluate the call expression tree T in the context of OLD_CALL expression
evaluation.  */
@@ -2467,6 +2472,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
   if (*non_constant_p)
 return t;
 
+  tree result = NULL_TREE;
   depth_ok = push_cx_call_context (t);
 
   /* Remember the object we are constructing.  */
@@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
new_obj = NULL_TREE;
}
 }
+   /* Verify that the object we're invoking the function on is sane.  */
+  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
+  /* maybe_add_lambda_conv_op creates a null 'this' pointer.  */
+  && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))
+{
+  tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
+  if (integer_zerop (thisarg))
+   {
+ if (!ctx->quiet)
+   error_at (loc, "member call on null pointer is not allowed "
+ "in a constant expression");
+ *non_constant_p = true;
+ result = error_mark_node;
+   }
+  else
+   {
+ STRIP_NOPS (thisarg);
+ if (TREE_CODE (thisarg) == ADDR_EXPR)
+   thisarg = TREE_OPERAND (thisarg, 0);
+ /* Detect out-of-bounds accesses.  */
+ if (TREE_CODE (thisarg) == ARRAY_REF)
+   {
+ eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
+ non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+   }
+ /* Detect using an inactive member of a union.  */
+ else if (TREE_CODE (thisarg) == COMPONENT_REF)
+   {
+ cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
+   non_constant_p, overflow_p);
+ if (*non_constant_p)
+   result = error_mark_node;
+   }
+ /* Detect other invalid accesses like
 
-  tree result = NULL_TREE;
+  X x;
+  ()[1].foo();
+
+where we'll end up with  p+ 1.  */
+ else if (TREE_CODE (thisarg) == POINTER_

[committed] libstdc++: Replace some VERIFY tests with static_assert

[Jonathan Wakely via Gcc-patches]

libstdc++-v3/ChangeLog:

* testsuite/18_support/comparisons/algorithms/partial_order.cc:
Replace VERIFY with static_assert where the compiler now
allows it.
* testsuite/18_support/comparisons/algorithms/weak_order.cc:
Likewise.

Tested x86_64-linux. Committed to trunk.

commit 6c3ae88d1e13b71665d1b27821159dcbea410267
Author: Jonathan Wakely 
Date:   Fri Aug 7 17:45:42 2020

libstdc++: Replace some VERIFY tests with static_assert

libstdc++-v3/ChangeLog:

* testsuite/18_support/comparisons/algorithms/partial_order.cc:
Replace VERIFY with static_assert where the compiler now
allows it.
* testsuite/18_support/comparisons/algorithms/weak_order.cc:
Likewise.

diff --git 
a/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc 
b/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
index 0806eabf74a..62b379a98cb 100644
--- a/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
+++ b/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
@@ -28,11 +28,11 @@ using std::partial_ordering;
 void
 test01()
 {
-  int one = 1, two = 2;
+  const int one = 1, two = 2;
 
-  VERIFY( partial_order(one, two) == partial_ordering::less );
-  VERIFY( partial_order(one, one) == partial_ordering::equivalent );
-  VERIFY( partial_order(two, one) == partial_ordering::greater );
+  static_assert( partial_order(one, two) == partial_ordering::less );
+  static_assert( partial_order(one, one) == partial_ordering::equivalent );
+  static_assert( partial_order(two, one) == partial_ordering::greater );
   static_assert( noexcept(partial_order(1, 1)) );
 }
 
@@ -44,44 +44,45 @@ constexpr partial_ordering different_cv_quals(int i, const 
int j)
 void
 test02()
 {
-  int fortytwo = 42, nines = 999, lots = 1000;
-  VERIFY( different_cv_quals(fortytwo, nines) == partial_ordering::less );
-  VERIFY( different_cv_quals(-nines, -nines) == partial_ordering::equivalent );
-  VERIFY( different_cv_quals(-nines, -lots) == partial_ordering::greater );
+  const int fortytwo = 42, nines = 999, lots = 1000;
+  static_assert( different_cv_quals(fortytwo, nines) == partial_ordering::less 
);
+  static_assert( different_cv_quals(-nines, -nines) == 
partial_ordering::equivalent );
+  static_assert( different_cv_quals(-nines, -lots) == 
partial_ordering::greater );
 }
 
 void
 test03()
 {
-  double zero = 0.0;
-  VERIFY( partial_order(zero, zero) == partial_ordering::equivalent );
-  VERIFY( partial_order(-zero, -zero) == partial_ordering::equivalent );
-  VERIFY( partial_order(-zero, zero) == partial_ordering::equivalent );
-  VERIFY( partial_order(zero, -zero) == partial_ordering::equivalent );
+  constexpr double zero = 0.0;
+  static_assert( partial_order(zero, zero) == partial_ordering::equivalent );
+  static_assert( partial_order(-zero, -zero) == partial_ordering::equivalent );
+  static_assert( partial_order(-zero, zero) == partial_ordering::equivalent );
+  static_assert( partial_order(zero, -zero) == partial_ordering::equivalent );
   static_assert( noexcept(partial_order(zero, 1.0)) );
   static_assert( partial_order(0.0, 1.0) == std::partial_ordering::less );
 
-  double min = std::numeric_limits::lowest();
-  double max = std::numeric_limits::max();
-  double nan = std::numeric_limits::quiet_NaN();
-  double inf = std::numeric_limits::infinity();
-  double denorm = std::numeric_limits::denorm_min();
-  double smallest = std::numeric_limits::min();
-  double epsilon = std::numeric_limits::epsilon();
-  VERIFY( partial_order(denorm, smallest) == partial_ordering::less );
-  VERIFY( partial_order(denorm, 0.0) == partial_ordering::greater );
+  constexpr double min = std::numeric_limits::lowest();
+  constexpr double max = std::numeric_limits::max();
+  constexpr double nan = std::numeric_limits::quiet_NaN();
+  constexpr double inf = std::numeric_limits::infinity();
+  constexpr double denorm = std::numeric_limits::denorm_min();
+  constexpr double smallest = std::numeric_limits::min();
+  constexpr double epsilon = std::numeric_limits::epsilon();
+  static_assert( partial_order(denorm, smallest) == partial_ordering::less );
+  static_assert( partial_order(denorm, 0.0) == partial_ordering::greater );
+  // FIXME: these should all use static_assert
   VERIFY( partial_order(0.0, nan) == partial_ordering::unordered );
   VERIFY( partial_order(nan, nan) == partial_ordering::unordered );
   VERIFY( partial_order(nan, 0.0) == partial_ordering::unordered );
   VERIFY( partial_order(-nan, 0.0) == partial_ordering::unordered );
   VERIFY( partial_order(-nan, min) == partial_ordering::unordered );
-  VERIFY( partial_order(-inf, min) == partial_ordering::less );
+  static_assert( partial_order(-inf, min) == partial_ordering::less );
   VERIFY( partial_order(-nan, -inf) == partial_ordering::unordered );
   VERIFY( partial_order(-inf, -nan) == partial_ordering

[PATCH] verify SCEV cache for stale entries

[Richard Biener]

This adds verification for the SCEV cache to avoid stale entries
that when picked up will lead to ICEs or other surprises.

Bootstrapped / tested on x86_64-unknown-linux-gnu with the two
previous fixes.

Posted for reference and archival purposes, I'm not going to push
this at this point.

2020-07-29  Richard Biener  

* passes.c (execute_function_todo): Call verify_scev_cache
if it is initialized.
* tree-scalar-evolution.h (verify_scev_cache): Declare.
* tree-scalar-evolution.c (verify_scev_cache_r): New.
(verify_scev_cache): Likewise.
---
 gcc/passes.c| 11 ---
 gcc/tree-scalar-evolution.c | 29 +
 gcc/tree-scalar-evolution.h |  1 +
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/gcc/passes.c b/gcc/passes.c
index a5da9a46f4e..c3c3a086236 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-core.h" /* for fnotice */
 #include "stringpool.h"
 #include "attribs.h"
+#include "tree-scalar-evolution.h" /* for verify_scev_cache.  */
 
 using namespace gcc;
 
@@ -1994,9 +1995,13 @@ execute_function_todo (function *fn, void *data)
verify_gimple_in_seq (gimple_body (cfun->decl));
}
  if (cfun->curr_properties & PROP_ssa)
-   /* IPA passes leave stmts to be fixed up, so make sure to
-  not verify SSA operands whose verifier will choke on that.  */
-   verify_ssa (true, !from_ipa_pass);
+   {
+ /* IPA passes leave stmts to be fixed up, so make sure to
+not verify SSA operands whose verifier will choke on that.  */
+ verify_ssa (true, !from_ipa_pass);
+ if (scev_initialized_p ())
+   verify_scev_cache ();
+   }
  /* IPA passes leave basic-blocks unsplit, so make sure to
 not trip on that.  */
  if ((cfun->curr_properties & PROP_cfg)
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index edab778277b..9645fed980c 100644
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -284,6 +284,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-into-ssa.h"
 #include "builtins.h"
 #include "case-cfn-macros.h"
+#include "diagnostic.h"
 
 static tree analyze_scalar_evolution_1 (class loop *, tree);
 static tree analyze_scalar_evolution_for_address_of (class loop *loop,
@@ -3025,6 +3026,34 @@ scev_reset (void)
 }
 }
 
+/* CHREC walker for verify_scev_cache.  */
+
+static tree
+verify_scev_cache_r (tree *tp, int *walk_subtrees, void *)
+{
+  if (TREE_CODE (*tp) == SSA_NAME
+  && SSA_NAME_IN_FREE_LIST (*tp))
+internal_error ("stale SCEV hash table entries");
+  if (!EXPR_P (*tp))
+*walk_subtrees = 0;
+  return NULL_TREE;
+}
+
+/* Verify the SCEV cache has no stale entries.  */
+
+void
+verify_scev_cache ()
+{
+  for (auto i = scalar_evolution_info->begin ();
+   i != scalar_evolution_info->end (); ++i)
+{
+  if ((*i)->name_version >= num_ssa_names
+ || ! ssa_name ((*i)->name_version))
+   internal_error ("stale SCEV hash table entries");
+  walk_tree (&(*i)->chrec, verify_scev_cache_r, NULL, NULL);
+}
+}
+
 /* Return true if the IV calculation in TYPE can overflow based on the 
knowledge
of the upper bound on the number of iterations of LOOP, the BASE and STEP
of IV.
diff --git a/gcc/tree-scalar-evolution.h b/gcc/tree-scalar-evolution.h
index e2fbfb55bd0..d0c578708ac 100644
--- a/gcc/tree-scalar-evolution.h
+++ b/gcc/tree-scalar-evolution.h
@@ -42,6 +42,7 @@ extern bool simple_iv (class loop *, class loop *, tree, 
struct affine_iv *,
   bool);
 extern bool iv_can_overflow_p (class loop *, tree, tree, tree);
 extern tree compute_overall_effect_of_inner_loop (class loop *, tree);
+extern void verify_scev_cache ();
 
 /* Returns the basic block preceding LOOP, or the CFG entry block when
the loop is function's body.  */
-- 
2.26.2