[PATCH 1/2] vect: Add costing_for_scalar parameter to init_cost hook

2021-05-08 Thread Kewen.Lin via Gcc-patches
Hi Richi,

Thanks for the comments!

on 2021/5/7 下午5:43, Richard Biener wrote:
> On Fri, May 7, 2021 at 5:30 AM Kewen.Lin via Gcc-patches
>  wrote:
>>
>> Hi,
>>
>> When I was investigating density_test heuristics, I noticed that
>> the current rs6000_density_test could be used for single scalar
>> iteration cost calculation, through the call trace:
>>   vect_compute_single_scalar_iteration_cost
>> -> rs6000_finish_cost
>>  -> rs6000_density_test
>>
>> It looks unexpected as its desriptive function comments and Bill
>> helped to confirm this needs to be fixed (thanks!).
>>
>> So this patch is to check the passed data, if it's the same as
>> the one in loop_vinfo, it indicates it's working on vector version
>> cost calculation, otherwise just early return.
>>
>> Bootstrapped/regtested on powerpc64le-linux-gnu P9.
>>
>> Nothing remarkable was observed with SPEC2017 Power9 full run.
>>
>> Is it ok for trunk?
> 
> +  /* Only care about cost of vector version, so exclude scalar
> version here.  */
> +  if (LOOP_VINFO_TARGET_COST_DATA (loop_vinfo) != (void *) data)
> +return;
> 
> Hmm, looks like a quite "random" test to me.  What about adding a
> parameter to finish_cost () (or init_cost?) indicating the cost kind?
> 

I originally wanted to change the hook interface, but noticed that
the finish_cost in function vect_estimate_min_profitable_iters is
the only invocation with LOOP_VINFO_TARGET_COST_DATA (loop_vinfo),
it looks enough to differentiate the scalar version costing or
vector version costing for loop.  Do you mean this observation/
assumption easy to be broken sometime later?

The attached patch to add one new parameter to indicate the
costing kind explicitly as you suggested.

Does it look better?

gcc/ChangeLog:

* doc/tm.texi: Regenerated.
* target.def (init_cost): Add new parameter costing_for_scalar.
* targhooks.c (default_init_cost): Adjust for new parameter.
* targhooks.h (default_init_cost): Likewise.
* tree-vect-loop.c (_loop_vec_info::_loop_vec_info): Likewise.
(vect_compute_single_scalar_iteration_cost): Likewise.
(vect_analyze_loop_2): Likewise.
* tree-vect-slp.c (_bb_vec_info::_bb_vec_info): Likewise.
(vect_bb_vectorization_profitable_p): Likewise.
* tree-vectorizer.h (init_cost): Likewise.
* config/aarch64/aarch64.c (aarch64_init_cost): Likewise.
* config/i386/i386.c (ix86_init_cost): Likewise.
* config/rs6000/rs6000.c (rs6000_init_cost): Likewise.  

> OTOH we already pass scalar_stmt to individual add_stmt_cost,
> so not sure whether the context really matters.  That said,
> the density test looks "interesting" ... the intent was that finish_cost
> might look at gathered data from add_stmt, not that it looks at
> the GIMPLE IL ... so why are you not counting vector_stmt vs.
> scalar_stmt entries in vect_body and using that for this metric?
> 

Good to know the intention behind finish_cost, thanks!

I'm afraid that the check on vector_stmt and scalar_stmt entries
from add_stmt_cost doesn't work for the density test here.  The
density test focuses on the vector version itself, there are some
stmts whose relevants are marked as vect_unused_in_scope, IIUC
they won't be passed down when costing for both versions.  But the
existing density check would like to know the cost for the
non-vectorized part.  The current implementation does:
 
 vec_cost = data->cost[vect_body]

  if (!STMT_VINFO_RELEVANT_P (stmt_info)
  && !STMT_VINFO_IN_PATTERN_P (stmt_info))
not_vec_cost++

 density_pct = (vec_cost * 100) / (vec_cost + not_vec_cost);

it takes those unrelevant stmts into account, and then has
both costs from the non-vectorized part (not_vec_cost)
and vectorized part (cost[vect_body]), it can calculate the
vectorization code density ratio.

BR,
Kewen
 gcc/config/aarch64/aarch64.c | 2 +-
 gcc/config/i386/i386.c   | 2 +-
 gcc/config/rs6000/rs6000.c   | 2 +-
 gcc/doc/tm.texi  | 4 ++--
 gcc/target.def   | 6 --
 gcc/targhooks.c  | 3 ++-
 gcc/targhooks.h  | 2 +-
 gcc/tree-vect-loop.c | 6 +++---
 gcc/tree-vect-slp.c  | 8 +---
 gcc/tree-vectorizer.h| 4 ++--
 10 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 12625a4bee3..de3c86d85fb 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -14390,7 +14390,7 @@ struct aarch64_vector_costs
 
 /* Implement TARGET_VECTORIZE_INIT_COST.  */
 void *
-aarch64_init_cost (class loop *)
+aarch64_init_cost (class loop *, bool)
 {
   return new aarch64_vector_costs;
 }
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7c41302c75b..5078d94970a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3,7 +3,7 @@ ix86_noce_conversion_profitable_p (rtx_insn *seq, 
struct noce_if_info *if_info)
 /* Implemen

[PATCH 2/2 v2] rs6000: Guard density_test only for vector version

2021-05-08 Thread Kewen.Lin via Gcc-patches
Hi,

v1: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569790.html

This is the updated version with one new parameter costing_for_scalar
passed by init_cost hook, instead of checking the passed data point
identity.

Bootstrapped/regtested on powerpc64le-linux-gnu P9.

Is it ok for trunk?

BR,
Kewen
-
gcc/ChangeLog:

* config/rs6000/rs6000.c (struct rs6000_cost_data): New member
costing_for_scalar.
(rs6000_density_test): Early return if costing_for_scalar is true.
(rs6000_init_cost): Init costing_for_scalar of rs6000_cost_data.
---
 gcc/config/rs6000/rs6000.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 88f16b909a3..4981597a10e 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5234,6 +5234,8 @@ typedef struct _rs6000_cost_data
   /* For each vectorized loop, this var holds TRUE iff a non-memory vector
  instruction is needed by the vectorization.  */
   bool vect_nonmem;
+  /* Indicates costing for the scalar version of a loop or block.  */
+  bool costing_for_scalar;
 } rs6000_cost_data;
 
 /* Test for likely overcommitment of vector hardware resources.  If a
@@ -5255,6 +5257,12 @@ rs6000_density_test (rs6000_cost_data *data)
   int vec_cost = data->cost[vect_body], not_vec_cost = 0;
   int i, density_pct;
 
+  /* This density test only cares about the cost of vector version of the
+ loop, early return if it's costing for the scalar version (namely
+ computing single scalar iteration cost).  */
+  if (data->costing_for_scalar)
+return;
+
   for (i = 0; i < nbbs; i++)
 {
   basic_block bb = bbs[i];
@@ -5292,7 +5300,7 @@ rs6000_density_test (rs6000_cost_data *data)
 /* Implement targetm.vectorize.init_cost.  */
 
 static void *
-rs6000_init_cost (struct loop *loop_info, bool)
+rs6000_init_cost (struct loop *loop_info, bool costing_for_scalar)
 {
   rs6000_cost_data *data = XNEW (struct _rs6000_cost_data);
   data->loop_info = loop_info;
@@ -5300,6 +5308,7 @@ rs6000_init_cost (struct loop *loop_info, bool)
   data->cost[vect_body] = 0;
   data->cost[vect_epilogue] = 0;
   data->vect_nonmem = false;
+  data->costing_for_scalar = costing_for_scalar;
   return data;
 }
 
-- 
2.17.1



[PATCH] rs6000: Move rs6000_vect_nonmem into target cost_data

2021-05-08 Thread Kewen.Lin via Gcc-patches
Hi,

This patch is to move rs6000_vect_nonmem (target cost_data
related information) into target cost_data struct.
Following Richi's comments in the thread[1], we can gather
data from add_stmt_cost invocations.  This is one pre-step
to centralize target cost_data related stuffs. 

Is it ok for trunk?

BR,
Kewen

[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569836.html
-
gcc/ChangeLog:

* config/rs6000/rs6000.c (rs6000_vect_nonmem): Renamed to
vect_nonmem and moved into...
(struct rs6000_cost_data): ...here.
(rs6000_init_cost): Use vect_nonmem of cost_data instead.
(rs6000_add_stmt_cost): Likewise.
(rs6000_finish_cost): Likewise.
 gcc/config/rs6000/rs6000.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 48b8efd732b..e2ff00145c5 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5231,6 +5231,9 @@ typedef struct _rs6000_cost_data
 {
   struct loop *loop_info;
   unsigned cost[3];
+  /* For each vectorized loop, this var holds TRUE iff a non-memory vector
+ instruction is needed by the vectorization.  */
+  bool vect_nonmem;
 } rs6000_cost_data;
 
 /* Test for likely overcommitment of vector hardware resources.  If a
@@ -5288,10 +5291,6 @@ rs6000_density_test (rs6000_cost_data *data)
 
 /* Implement targetm.vectorize.init_cost.  */
 
-/* For each vectorized loop, this var holds TRUE iff a non-memory vector
-   instruction is needed by the vectorization.  */
-static bool rs6000_vect_nonmem;
-
 static void *
 rs6000_init_cost (struct loop *loop_info)
 {
@@ -5300,7 +5299,7 @@ rs6000_init_cost (struct loop *loop_info)
   data->cost[vect_prologue] = 0;
   data->cost[vect_body] = 0;
   data->cost[vect_epilogue] = 0;
-  rs6000_vect_nonmem = false;
+  data->vect_nonmem = false;
   return data;
 }
 
@@ -5360,7 +5359,7 @@ rs6000_add_stmt_cost (class vec_info *vinfo, void *data, 
int count,
   || kind == vec_promote_demote || kind == vec_construct
   || kind == scalar_to_vec)
  || (where == vect_body && kind == vector_stmt))
-   rs6000_vect_nonmem = true;
+   cost_data->vect_nonmem = true;
 }
 
   return retval;
@@ -5415,7 +5414,7 @@ rs6000_finish_cost (void *data, unsigned *prologue_cost,
   if (cost_data->loop_info)
 {
   loop_vec_info vec_info = loop_vec_info_for_loop (cost_data->loop_info);
-  if (!rs6000_vect_nonmem
+  if (!cost_data->vect_nonmem
  && LOOP_VINFO_VECT_FACTOR (vec_info) == 2
  && LOOP_REQUIRES_VERSIONING (vec_info))
cost_data->cost[vect_body] += 1;
-- 
2.17.1



Re: [PATCH] rs6000: Make density_test only for vector version

2021-05-08 Thread Kewen.Lin via Gcc-patches
Hi Will,

Thanks for the comments!

on 2021/5/7 下午7:43, will schmidt wrote:
> On Fri, 2021-05-07 at 10:28 +0800, Kewen.Lin via Gcc-patches wrote:
>> Hi,
>>
>> When I was investigating density_test heuristics, I noticed that
>> the current rs6000_density_test could be used for single scalar
>> iteration cost calculation, through the call trace:
>>   vect_compute_single_scalar_iteration_cost
>> -> rs6000_finish_cost
>>  -> rs6000_density_test
>>
>> It looks unexpected as its desriptive function comments and Bill
>> helped to confirm this needs to be fixed (thanks!).
>>
>> So this patch is to check the passed data, if it's the same as
>> the one in loop_vinfo, it indicates it's working on vector version
>> cost calculation, otherwise just early return.
>>
>> Bootstrapped/regtested on powerpc64le-linux-gnu P9.
>>
>> Nothing remarkable was observed with SPEC2017 Power9 full run.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> --
>> gcc/ChangeLog:
>>
>>  * config/rs6000/rs6000.c (rs6000_density_test): Early return if
>>  calculating single scalar iteration cost.
> 
> 
> 
> Ok, so data is passed in.. 
>   static void
>   rs6000_density_test (rs6000_cost_data *data)
>   {
>   ...
> and loop_vinfo is calculated via...
>   loop_vec_info loop_vinfo = loop_vec_info_for_loop (data->loop_info);
> which is
>   static inline loop_vec_info
>   loop_vec_info_for_loop (class loop *loop)
>   {
> return (loop_vec_info) loop->aux;
>   }
> 
> 
>> +  /* Only care about cost of vector version, so exclude scalar
>> version here.  */
>>
>> +  if (LOOP_VINFO_TARGET_COST_DATA (loop_vinfo) != (void *) data)
>>
>> +return;
> 
> Can the loop contain both vector and scalar parts?  Comments in the
> function now mention a percentage of vector instructions within the
> loop.  So..  this is meant to return early if there are no(?) vector
> instructions?

Sorry for the confusion.  There are two call sites for finish_cost
in loop vectorization code, one is in function
vect_compute_single_scalar_iteration_cost, which is calculating the
cost of one scalar iteration of the loop, since it's costing the scalar
version of the loop, I called it as scalar version.  The other is in
function vect_estimate_min_profitable_iters, which is to finalize the
cost of the vector version of the loop, I called it as vector version.

Since the density test aims at the cost calculation of vector version
of the loop, we have to avoid the possible penalization when we are
actually calculating the cost for the scalar version of the loop.

As you pointed out, the scalar version looks easily confusing with the
scalar part of the vectorized loop.  I updated the comments in v2 with
the explicit words saying it's computing single scalar iteration cost.

Does it look good to you?

v2: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569896.html

BR,
Kewen


[PATCH v5 1/2] MIPS: Not trigger error for pre-R6 and -mcompact-branches=always

2021-05-08 Thread YunQiang Su
For MIPSr6, we may wish to use compact-branches only.
Currently, we have to use `always' option, while it is mark as conflict
with pre-R6.
  cc1: error: unsupported combination: ‘mips32r2’ -mcompact-branches=always
Just ignore -mcompact-branches=always for pre-R6.

This patch also defines
__mips_compact_branches_never
__mips_compact_branches_always
__mips_compact_branches_optimal
predefined macros

gcc/ChangeLog:
* config/mips/mips.c (mips_option_override):
* config/mips/mips.h (TARGET_RTP_PIC): not trigger error for
compact-branches=always for pre-R6.
(TARGET_CB_NEVER): Likewise.
(TARGET_CB_ALWAYS): Likewise.
(struct mips_cpu_info): define macros for compact branch policy.
* doc/invoke.texi: Document "always" with pre-R6.

gcc/testsuite/ChangeLog:
* gcc.target/mips/compact-branches-1.c: add isa_rev>=6.
* gcc.target/mips/mips.exp: don't add -mipsXXr6 option for
-mcompact-branches=always. It is usable for pre-R6 now.
* gcc.target/mips/compact-branches-8.c: New test.
* gcc.target/mips/compact-branches-9.c: New test.
---
 gcc/config/mips/mips.c|  8 +--
 gcc/config/mips/mips.h| 22 ---
 gcc/doc/invoke.texi   | 10 +
 .../gcc.target/mips/compact-branches-1.c  |  2 +-
 .../gcc.target/mips/compact-branches-8.c  | 10 +
 .../gcc.target/mips/compact-branches-9.c  | 10 +
 gcc/testsuite/gcc.target/mips/mips.exp|  4 +---
 7 files changed, 43 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-8.c
 create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-9.c

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 315545966f8..805ba8240e0 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -20107,13 +20107,7 @@ mips_option_override (void)
   target_flags |= MASK_ODD_SPREG;
 }
 
-  if (!ISA_HAS_COMPACT_BRANCHES && mips_cb == MIPS_CB_ALWAYS)
-{
-  error ("unsupported combination: %qs%s %s",
- mips_arch_info->name, TARGET_MICROMIPS ? " -mmicromips" : "",
- "-mcompact-branches=always");
-}
-  else if (!ISA_HAS_DELAY_SLOTS && mips_cb == MIPS_CB_NEVER)
+  if (!ISA_HAS_DELAY_SLOTS && mips_cb == MIPS_CB_NEVER)
 {
   error ("unsupported combination: %qs%s %s",
  mips_arch_info->name, TARGET_MICROMIPS ? " -mmicromips" : "",
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 47aac9d3d61..d845813f102 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -103,11 +103,9 @@ struct mips_cpu_info {
 #define TARGET_RTP_PIC (TARGET_VXWORKS_RTP && flag_pic)
 
 /* Compact branches must not be used if the user either selects the
-   'never' policy or the 'optimal' policy on a core that lacks
+   'never' policy or the 'optimal' / 'always' policy on a core that lacks
compact branch instructions.  */
-#define TARGET_CB_NEVER (mips_cb == MIPS_CB_NEVER  \
-|| (mips_cb == MIPS_CB_OPTIMAL \
-&& !ISA_HAS_COMPACT_BRANCHES))
+#define TARGET_CB_NEVER (mips_cb == MIPS_CB_NEVER || !ISA_HAS_COMPACT_BRANCHES)
 
 /* Compact branches may be used if the user either selects the
'always' policy or the 'optimal' policy on a core that supports
@@ -117,10 +115,11 @@ struct mips_cpu_info {
 && ISA_HAS_COMPACT_BRANCHES))
 
 /* Compact branches must always be generated if the user selects
-   the 'always' policy or the 'optimal' policy om a core that
-   lacks delay slot branch instructions.  */
-#define TARGET_CB_ALWAYS (mips_cb == MIPS_CB_ALWAYS\
-|| (mips_cb == MIPS_CB_OPTIMAL \
+   the 'always' policy on a core support compact branches,
+   or the 'optimal' policy on a core that lacks delay slot branch 
instructions.  */
+#define TARGET_CB_ALWAYS ((mips_cb == MIPS_CB_ALWAYS \
+&& ISA_HAS_COMPACT_BRANCHES) \
+|| (mips_cb == MIPS_CB_OPTIMAL   \
 && !ISA_HAS_DELAY_SLOTS))
 
 /* Special handling for JRC that exists in microMIPSR3 as well as R6
@@ -655,6 +654,13 @@ struct mips_cpu_info {
builtin_define ("__mips_no_lxc1_sxc1"); \
   if (!ISA_HAS_UNFUSED_MADD4 && !ISA_HAS_FUSED_MADD4)  \
builtin_define ("__mips_no_madd4"); \
+   \
+  if (TARGET_CB_NEVER) \
+   builtin_define ("__mips_compact_branches_never");   \
+  else if (TARGET_CB_ALWAYS)   \
+   builtin_define ("__mips_compact_branches_always");  \
+  else \
+   builtin_d

[PATCH v5 2/2] MIPS: add builtime option for -mcompact-branches

2021-05-08 Thread YunQiang Su
For R6+ target, it allows to configure gcc to use compact branches only
if avaiable.

gcc/ChangeLog:
* config.gcc: add -with-compact-branches=policy build option.
* doc/install.texi: Likewise.
* config/mips/mips.h: Likewise.
---
 gcc/config.gcc | 13 +++--
 gcc/config/mips/mips.h |  3 ++-
 gcc/doc/install.texi   | 19 +++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 92fad8e20ca..ac411f66de2 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4724,7 +4724,7 @@ case "${target}" in
;;
 
mips*-*-*)
-   supported_defaults="abi arch arch_32 arch_64 float fpu nan 
fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 
madd4"
+   supported_defaults="abi arch arch_32 arch_64 float fpu nan 
fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 
madd4 compact-branches"
 
case ${with_float} in
"" | soft | hard)
@@ -4877,6 +4877,15 @@ case "${target}" in
exit 1
;;
esac
+
+   case ${with_compact_branches} in
+   "" | never | always | optimal)
+   ;;
+   *)
+   echo "Unknown compact-branches policy used in 
--with-compact-branches" 1>&2
+   exit 1
+   ;;
+   esac
;;
 
nds32*-*-*)
@@ -5389,7 +5398,7 @@ case ${target} in
 esac
 
 t=
-all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 
schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls 
lxc1-sxc1 madd4"
+all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 
schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls 
lxc1-sxc1 madd4 compact-branches"
 for option in $all_defaults
 do
eval "val=\$with_"`echo $option | sed s/-/_/g`
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index d845813f102..64d7a66f8b6 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -897,7 +897,8 @@ struct mips_cpu_info {
   {"mips-plt", "%{!mplt:%{!mno-plt:-m%(VALUE)}}" }, \
   {"synci", "%{!msynci:%{!mno-synci:-m%(VALUE)}}" },   \
   {"lxc1-sxc1", "%{!mlxc1-sxc1:%{!mno-lxc1-sxc1:-m%(VALUE)}}" }, \
-  {"madd4", "%{!mmadd4:%{!mno-madd4:-m%(VALUE)}}" } \
+  {"madd4", "%{!mmadd4:%{!mno-madd4:-m%(VALUE)}}" }, \
+  {"compact-branches", "%{!mcompact-branches=*:-mcompact-branches=%(VALUE)}" } 
\
 
 /* A spec that infers the:
-mnan=2008 setting from a -mips argument,
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index d2cab15d9fd..b9980186ed3 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1464,6 +1464,25 @@ systems that support conditional traps).
 Division by zero checks use the break instruction.
 @end table
 
+@item --with-compact-branches=@var{policy}
+Specify how the compiler should generate branch instructions.
+This option is only supported on the MIPS target.
+The possibilities for @var{type} are:
+@table @code
+@item optimal
+Cause a delay slot branch to be used if one is available in the
+current ISA and the delay slot is successfully filled. If the delay slot
+is not filled, a compact branch will be chosen if one is available.
+@item never
+Ensures that compact branch instructions will never be generated.
+@item always
+Ensures that a compact branch instruction will be generated if available.
+If a compact branch instruction is not available,
+a delay slot form of the branch will be used instead.
+This option is supported from MIPS Release 6 onwards.
+For pre-R6/microMIPS/MIPS16, this option is just same as never/optimal.
+@end table
+
 @c If you make --with-llsc the default for additional targets,
 @c update the --with-llsc description in the MIPS section below.
 
-- 
2.30.2



[PATCH v2] MIPS: R6: load/store can process unaligned address

2021-05-08 Thread YunQiang Su
MIPS release 6 requires the lw/ld/sw/sd can work with
unaligned address, while it can be implemented by
full hardware or trap&emulate.

Since it is may be fully done by hardware, we add an
option -m(no-)unaligned-access, the kernel may need it.

gcc/ChangeLog:

* config/mips/mips.h (ISA_HAS_UNALIGNED_ACCESS):
(STRICT_ALIGNMENT): R6 can unaligned access.
* config/mips/mips.md (movmisalign): Likewise.
* config/mips/mips.opt: add -m(no-)unaligned-access
* doc/invoke.texi: Likewise.

gcc/testsuite/ChangeLog:

* gcc.target/mips/mips.exp: add unaligned-access
* gcc.target/mips/unaligned-2.c: New test.
* gcc.target/mips/unaligned-3.c: New test.
---
 gcc/config/mips/mips.h  |  6 ++-
 gcc/config/mips/mips.md | 10 
 gcc/config/mips/mips.opt|  4 ++
 gcc/doc/invoke.texi | 10 
 gcc/testsuite/gcc.target/mips/mips.exp  |  1 +
 gcc/testsuite/gcc.target/mips/unaligned-2.c | 53 +
 gcc/testsuite/gcc.target/mips/unaligned-3.c | 53 +
 7 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/mips/unaligned-2.c
 create mode 100644 gcc/testsuite/gcc.target/mips/unaligned-3.c

diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 47aac9d3d61..22a0d449aab 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -226,6 +226,10 @@ struct mips_cpu_info {
 && (mips_isa_rev >= 6 \
 || ISA_HAS_MSA))
 
+/* ISA load/store instructions can handle unaligned address */
+#define ISA_HAS_UNALIGNED_ACCESS (TARGET_UNALIGNED_ACCESS \
+&& (mips_isa_rev >= 6))
+
 /* The ISA compression flags that are currently in effect.  */
 #define TARGET_COMPRESSION (target_flags & (MASK_MIPS16 | MASK_MICROMIPS))
 
@@ -1666,7 +1670,7 @@ FP_ASM_SPEC "\
   (ISA_HAS_MSA ? BITS_PER_MSA_REG : LONG_DOUBLE_TYPE_SIZE)
 
 /* All accesses must be aligned.  */
-#define STRICT_ALIGNMENT 1
+#define STRICT_ALIGNMENT (!ISA_HAS_UNALIGNED_ACCESS)
 
 /* Define this if you wish to imitate the way many other C compilers
handle alignment of bitfields and the structures that contain
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index eef3cfd50a8..40e29c60432 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -4459,6 +4459,16 @@ (define_insn "mov_r"
   [(set_attr "move_type" "store")
(set_attr "mode" "")])
 
+;; Unaligned direct access
+(define_expand "movmisalign"
+  [(set (match_operand:JOIN_MODE 0)
+   (match_operand:JOIN_MODE 1))]
+  "ISA_HAS_UNALIGNED_ACCESS"
+{
+  if (mips_legitimize_move (mode, operands[0], operands[1]))
+DONE;
+})
+
 ;; An instruction to calculate the high part of a 64-bit SYMBOL_ABSOLUTE.
 ;; The required value is:
 ;;
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 6af8037e9bd..ebb4c616401 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -404,6 +404,10 @@ mtune=
 Target RejectNegative Joined Var(mips_tune_option) ToLower 
Enum(mips_arch_opt_value)
 -mtune=PROCESSOR   Optimize the output for PROCESSOR.
 
+munaligned-access
+Target Var(TARGET_UNALIGNED_ACCESS) Init(1)
+Generate code with unaligned load store, valid for MIPS R6.
+
 muninit-const-in-rodata
 Target Var(TARGET_UNINIT_CONST_IN_RODATA)
 Put uninitialized constants in ROM (needs -membedded-data).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 40cacc6f8e7..211a709fff8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1062,6 +1062,7 @@ Objective-C and Objective-C++ Dialects}.
 -mcheck-zero-division  -mno-check-zero-division @gol
 -mdivide-traps  -mdivide-breaks @gol
 -mload-store-pairs  -mno-load-store-pairs @gol
+-munaligned-access  -mno-unaligned-access @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mimadd  -mno-imadd  -mfused-madd  -mno-fused-madd  -nocpp 
@gol
 -mfix-24k  -mno-fix-24k @gol
@@ -24999,6 +25000,15 @@ instructions to enable load/store bonding.  This 
option is enabled by
 default but only takes effect when the selected architecture is known
 to support bonding.
 
+@item -munaligned-access
+@itemx -mno-unaligned-access
+@opindex munaligned-access
+@opindex mno-unaligned-access
+Enable (disable) direct unaligned access for MIPS Release 6.
+MIPSr6 requires load/store unaligned-access support,
+by hardware or trap&emulate. 
+So @option{-mno-unaligned-access} may be needed by kernel.
+
 @item -mmemcpy
 @itemx -mno-memcpy
 @opindex mmemcpy
diff --git a/gcc/testsuite/gcc.target/mips/mips.exp 
b/gcc/testsuite/gcc.target/mips/mips.exp
index 01292316944..cb1ee7d71b5 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -264,6 +264,7 @@ set mips_option_groups {
 frame-header "-mframe-header-opt|-mno-frame-header-opt"
 stack-pro

Re: [PATCH] c++: argument pack expansion inside constraint [PR100138]

2021-05-08 Thread Jason Merrill via Gcc-patches

On 5/7/21 12:33 PM, Patrick Palka wrote:

This PR is about CTAD but the underlying problems are more general;
CTAD is a good trigger for them because of the necessary substitution
into constraints that deduction guide generation entails.

In the testcase below, when generating the implicit deduction guide for
the constrained constructor template for A, we substitute the generic
flattening map 'tsubst_args' into the constructor's constraints.  During
this substitution, tsubst_pack_expansion returns a rebuilt pack
expansion for sizeof...(xs), but it's neglecting to carry over the
PACK_EXPANSION_LOCAL_P (and PACK_EXPANSION_SIZEOF_P) flag from the
original tree to the rebuilt one.  The flag is otherwise unset on the
original tree[1] but set for the rebuilt tree from make_pack_expansion
only because we're doing the CTAD at function scope (inside main).  This
leads us to crash when substituting into the pack expansion during
satisfaction because we don't have local_specializations set up (it'd be
set up for us if PACK_EXPANSION_LOCAL_P is unset)

Similarly, when substituting into a constraint we need to set
cp_unevaluated since constraints are unevaluated operands.  This avoids
a crash during CTAD for C below.

[1]: Although the original pack expansion is in a function context, I
guess it makes sense that PACK_EXPANSION_LOCAL_P is unset for it because
we can't rely on local specializations (which are formed when
substituting into the function declaration) during satisfaction.

Bootstrapped and regtested on x86_64-pc-linux-gnu, also tested on
cmcstl2 and range-v3, does this look OK for trunk?


OK.


gcc/cp/ChangeLog:

PR c++/100138
* constraint.cc (tsubst_constraint): Set up cp_unevaluated.
(satisfy_atom): Set up iloc_sentinel before calling
cxx_constant_value.
* pt.c (tsubst_pack_expansion): When returning a rebuilt pack
expansion, carry over PACK_EXPANSION_LOCAL_P and
PACK_EXPANSION_SIZEOF_P from the original pack expansion.

gcc/testsuite/ChangeLog:

PR c++/100138
* g++.dg/cpp2a/concepts-ctad4.C: New test.
---
  gcc/cp/constraint.cc|  6 -
  gcc/cp/pt.c |  2 ++
  gcc/testsuite/g++.dg/cpp2a/concepts-ctad4.C | 25 +
  3 files changed, 32 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad4.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 0709695fd08..30fccc46678 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2747,6 +2747,7 @@ tsubst_constraint (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
/* We also don't want to evaluate concept-checks when substituting the
   constraint-expressions of a declaration.  */
processing_constraint_expression_sentinel s;
+  cp_unevaluated u;
tree expr = tsubst_expr (t, args, complain, in_decl, false);
return expr;
  }
@@ -3005,7 +3006,10 @@ satisfy_atom (tree t, tree args, sat_info info)
  
/* Compute the value of the constraint.  */

if (info.noisy ())
-result = cxx_constant_value (result);
+{
+  iloc_sentinel ils (EXPR_LOCATION (result));
+  result = cxx_constant_value (result);
+}
else
  {
result = maybe_constant_value (result, NULL_TREE,
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 36a8cb5df5d..0d27dd1af65 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13203,6 +13203,8 @@ tsubst_pack_expansion (tree t, tree args, 
tsubst_flags_t complain,
else
result = tsubst (pattern, args, complain, in_decl);
result = make_pack_expansion (result, complain);
+  PACK_EXPANSION_LOCAL_P (result) = PACK_EXPANSION_LOCAL_P (t);
+  PACK_EXPANSION_SIZEOF_P (result) = PACK_EXPANSION_SIZEOF_P (t);
if (PACK_EXPANSION_AUTO_P (t))
{
  /* This is a fake auto... pack expansion created in add_capture with
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad4.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad4.C
new file mode 100644
index 000..95a3a22dd04
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad4.C
@@ -0,0 +1,25 @@
+// PR c++/100138
+// { dg-do compile { target c++20 } }
+
+template 
+struct A {
+  A(T, auto... xs) requires (sizeof...(xs) != 0) { }
+};
+
+constexpr bool f(...) { return true; }
+
+template 
+struct B {
+  B(T, auto... xs) requires (f(xs...)); // { dg-error "constant expression" }
+};
+
+template 
+struct C {
+  C(T, auto x) requires (f(x)); // { dg-error "constant expression" }
+};
+
+int main() {
+  A x{1, 2}; // { dg-bogus "" }
+  B y{1, 2}; // { dg-error "deduction|no match" }
+  C z{1, 2}; // { dg-error "deduction|no match" }
+}





[PATCH] x86: Add a test for PR tree-optimization/42587

2021-05-08 Thread H.J. Lu via Gcc-patches
PR tree-optimization/42587
* gcc.target/i386/pr42587.c: New test.
---
 gcc/testsuite/gcc.target/i386/pr42587.c | 35 +
 1 file changed, 35 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr42587.c

diff --git a/gcc/testsuite/gcc.target/i386/pr42587.c 
b/gcc/testsuite/gcc.target/i386/pr42587.c
new file mode 100644
index 000..d4b5143eb6d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr42587.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-store-merging" } */
+
+typedef unsigned char u8;
+typedef unsigned int u32;
+union __anonunion_out_195
+{
+  u32 value;
+  u8 bytes[4];
+};
+union __anonunion_in_196
+{
+  u32 value;
+  u8 bytes[4];
+};
+extern void acpi_ut_track_stack_ptr (void);
+u32 acpi_ut_dword_byte_swap (u32 value);
+u32
+acpi_ut_dword_byte_swap (u32 value)
+{
+  union __anonunion_out_195 out;
+  union __anonunion_in_196 in;
+
+  {
+acpi_ut_track_stack_ptr ();
+in.value = value;
+out.bytes[0] = in.bytes[3];
+out.bytes[1] = in.bytes[2];
+out.bytes[2] = in.bytes[1];
+out.bytes[3] = in.bytes[0];
+return (out.value);
+  }
+}
+
+/* { dg-final { scan-tree-dump "32 bit bswap implementation found at" 
"store-merging" } } */
-- 
2.31.1



[PATCH v2] Add a test for PR tree-optimization/42587

2021-05-08 Thread H.J. Lu via Gcc-patches
On Sat, May 8, 2021 at 7:18 AM H.J. Lu  wrote:
>
> PR tree-optimization/42587
> * gcc.target/i386/pr42587.c: New test.
> ---
>  gcc/testsuite/gcc.target/i386/pr42587.c | 35 +
>  1 file changed, 35 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr42587.c
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr42587.c 
> b/gcc/testsuite/gcc.target/i386/pr42587.c
> new file mode 100644
> index 000..d4b5143eb6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr42587.c
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-store-merging" } */
> +
> +typedef unsigned char u8;
> +typedef unsigned int u32;
> +union __anonunion_out_195
> +{
> +  u32 value;
> +  u8 bytes[4];
> +};
> +union __anonunion_in_196
> +{
> +  u32 value;
> +  u8 bytes[4];
> +};
> +extern void acpi_ut_track_stack_ptr (void);
> +u32 acpi_ut_dword_byte_swap (u32 value);
> +u32
> +acpi_ut_dword_byte_swap (u32 value)
> +{
> +  union __anonunion_out_195 out;
> +  union __anonunion_in_196 in;
> +
> +  {
> +acpi_ut_track_stack_ptr ();
> +in.value = value;
> +out.bytes[0] = in.bytes[3];
> +out.bytes[1] = in.bytes[2];
> +out.bytes[2] = in.bytes[1];
> +out.bytes[3] = in.bytes[0];
> +return (out.value);
> +  }
> +}
> +
> +/* { dg-final { scan-tree-dump "32 bit bswap implementation found at" 
> "store-merging" } } */
> --
> 2.31.1
>

The v2 patch to add gcc.dg/optimize-bswapsi-6.c.  Which one is
preferred?

-- 
H.J.
From 33804e2bf14605a4676de3b3784ada406fdbaa21 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Sat, 8 May 2021 07:23:25 -0700
Subject: [PATCH v2] Add a test for PR tree-optimization/42587

	PR tree-optimization/42587
	* gcc.dg/optimize-bswapsi-6.c: New test.
---
 gcc/testsuite/gcc.dg/optimize-bswapsi-6.c | 38 +++
 1 file changed, 38 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/optimize-bswapsi-6.c

diff --git a/gcc/testsuite/gcc.dg/optimize-bswapsi-6.c b/gcc/testsuite/gcc.dg/optimize-bswapsi-6.c
new file mode 100644
index 000..3c089b36fd8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/optimize-bswapsi-6.c
@@ -0,0 +1,38 @@
+/* PR tree-optimization/42587 */
+/* { dg-do compile } */
+/* { dg-require-effective-target bswap } */
+/* { dg-options "-O2 -fdump-tree-store-merging" } */
+/* { dg-additional-options "-march=z900" { target s390-*-* } } */
+
+typedef unsigned char u8;
+typedef unsigned int u32;
+union __anonunion_out_195
+{
+  u32 value;
+  u8 bytes[4];
+};
+union __anonunion_in_196
+{
+  u32 value;
+  u8 bytes[4];
+};
+extern void acpi_ut_track_stack_ptr (void);
+u32 acpi_ut_dword_byte_swap (u32 value);
+u32
+acpi_ut_dword_byte_swap (u32 value)
+{
+  union __anonunion_out_195 out;
+  union __anonunion_in_196 in;
+
+  {
+acpi_ut_track_stack_ptr ();
+in.value = value;
+out.bytes[0] = in.bytes[3];
+out.bytes[1] = in.bytes[2];
+out.bytes[2] = in.bytes[1];
+out.bytes[3] = in.bytes[0];
+return (out.value);
+  }
+}
+
+/* { dg-final { scan-tree-dump "32 bit bswap implementation found at" "store-merging" } } */
-- 
2.31.1



Re: [PATCH] tweak range-on-exit.

2021-05-08 Thread Aldy Hernandez via Gcc-patches




On 5/7/21 9:03 PM, Andrew MacLeod wrote:


+  else
+{
+  range_on_entry (r, bb, name);
+  // See if there was a deref in this block, if applicable
+  if (!cfun->can_throw_non_call_exceptions && r.varying_p () &&
+ m_cache.m_non_null.non_null_deref_p (name, bb))
+   r = range_nonzero (TREE_TYPE (name));
+}


Nit.  The final && must go on the following line.  And it probably looks 
better if you put each "&& conditional" on its own line.


Aldy



[GOVERNANCE] Where to file complaints re project-maintainers?

2021-05-08 Thread abebeos via Gcc-patches
(failed to join gcc, so posting here)

Is there any private email where one can file complaints re
project-maintainers (or "those who are supervising the maintainers") ?

Is there any information about the process for such complaints?

Related Issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100480

(please note that this complaint will most possibly escalate up to the
person(s) who are responsible for policies/rules)


[patch] Remove call to gcc_unreachable in range-op.cc

2021-05-08 Thread Eric Botcazou
Hi,

the attached Ada testcase happens to stumble on the call to gcc_unreachable in 
operator_bitwise_xor::op1_range.  My understanding is that there is nothing 
wrong going on and that it's safe to let it go through.

Tested on x86-64/Linux, OK for mainline and 11 branch?


2021-05-08  Eric Botcazou  

* range-op.cc (get_bool_state): Adjust head comment.
(operator_not_equal::op1_range): Fix comment.
(operator_bitwise_xor::op1_range): Remove call to gcc_unreachable.


2021-05-08  Eric Botcazou  

* gnat.dg/specs/opt5.ads: New test.
* gnat.dg/specs/opt5_pkg.ads: New helper.

-- 
Eric Botcazoudiff --git a/gcc/range-op.cc b/gcc/range-op.cc
index ab8f4e211ac..742e54686b4 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -358,10 +358,8 @@ range_true_and_false (tree type)
 
 enum bool_range_state { BRS_FALSE, BRS_TRUE, BRS_EMPTY, BRS_FULL };
 
-// Return the summary information about boolean range LHS.  Return an
-// "interesting" range in R.  For EMPTY or FULL, return the equivalent
-// range for TYPE, for BRS_TRUE and BRS false, return the negation of
-// the bool range.
+// Return the summary information about boolean range LHS.  If EMPTY/FULL,
+// return the equivalent range for TYPE in R; if FALSE/TRUE, do nothing.
 
 static bool_range_state
 get_bool_state (irange &r, const irange &lhs, tree val_type)
@@ -383,6 +381,7 @@ get_bool_state (irange &r, const irange &lhs, tree val_type)
   r.set_varying (val_type);
   return BRS_FULL;
 }
+
   return BRS_TRUE;
 }
 
@@ -538,7 +537,7 @@ operator_not_equal::op1_range (irange &r, tree type,
   break;
 
 case BRS_FALSE:
-  // If its true, the result is the same as OP2.
+  // If it's false, the result is the same as OP2.
   r = op2;
   break;
 
@@ -2643,7 +2642,7 @@ operator_bitwise_xor::op1_range (irange &r, tree type,
 	  r = op2;
 	  break;
 	default:
-	  gcc_unreachable ();
+	  break;
 	}
   return true;
 }
-- { dg-do compile }
-- { dg-options "-O2 -gnata -gnatVa" }

with Opt5_Pkg;

package Opt5 is

  type Object is new Opt5_Pkg.Object with private;

  Undefined : constant Object;

  overriding function Is_Defined (Self : Object) return Boolean;

  function Create (Sloc : Opt5_Pkg.Object) return Integer is (0)
with Pre  => Sloc.Is_Defined;

private

  type Object is new Opt5_Pkg.Object with null record;

  Undefined : constant Object := (Opt5_Pkg.Undefined with others => <>);

  overriding function Is_Defined (Self : Object) return Boolean is (Self /= Undefined);

end Opt5;
package Opt5_Pkg is

  type Object is tagged private;

  Undefined : constant Object;

  function Is_Defined (Self : Object) return Boolean;

private

  type Object is tagged null record;

  Undefined : constant Object := (others => <>);

  function Is_Defined (Self : Object) return Boolean is (Self /= Undefined);

end Opt5_Pkg;


Re: [PATCH v2] Add a test for PR tree-optimization/42587

2021-05-08 Thread H.J. Lu via Gcc-patches
On Sat, May 8, 2021 at 7:25 AM H.J. Lu  wrote:
>
> On Sat, May 8, 2021 at 7:18 AM H.J. Lu  wrote:
> >
> > PR tree-optimization/42587
> > * gcc.target/i386/pr42587.c: New test.
> > ---
> >  gcc/testsuite/gcc.target/i386/pr42587.c | 35 +
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr42587.c
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr42587.c 
> > b/gcc/testsuite/gcc.target/i386/pr42587.c
> > new file mode 100644
> > index 000..d4b5143eb6d
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr42587.c
> > @@ -0,0 +1,35 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-store-merging" } */
> > +
> > +typedef unsigned char u8;
> > +typedef unsigned int u32;
> > +union __anonunion_out_195
> > +{
> > +  u32 value;
> > +  u8 bytes[4];
> > +};
> > +union __anonunion_in_196
> > +{
> > +  u32 value;
> > +  u8 bytes[4];
> > +};
> > +extern void acpi_ut_track_stack_ptr (void);
> > +u32 acpi_ut_dword_byte_swap (u32 value);
> > +u32
> > +acpi_ut_dword_byte_swap (u32 value)
> > +{
> > +  union __anonunion_out_195 out;
> > +  union __anonunion_in_196 in;
> > +
> > +  {
> > +acpi_ut_track_stack_ptr ();
> > +in.value = value;
> > +out.bytes[0] = in.bytes[3];
> > +out.bytes[1] = in.bytes[2];
> > +out.bytes[2] = in.bytes[1];
> > +out.bytes[3] = in.bytes[0];
> > +return (out.value);
> > +  }
> > +}
> > +
> > +/* { dg-final { scan-tree-dump "32 bit bswap implementation found at" 
> > "store-merging" } } */
> > --
> > 2.31.1
> >
>
> The v2 patch to add gcc.dg/optimize-bswapsi-6.c.  Which one is
> preferred?
>

If there are no objections, I will check in the v2 testcase and close

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42587

-- 
H.J.


Re: [GOVERNANCE] Where to file complaints re project-maintainers?

2021-05-08 Thread Ian Lance Taylor via Gcc-patches
On Sat, May 8, 2021 at 8:49 AM abebeos via Gcc-patches
 wrote:
>
> Is there any private email where one can file complaints re
> project-maintainers (or "those who are supervising the maintainers") ?
>
> Is there any information about the process for such complaints?
>
> Related Issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100480
>
> (please note that this complaint will most possibly escalate up to the
> person(s) who are responsible for policies/rules)

The GCC project does not have a code of conduct, and it does not have
a managing hierarchy.  Nobody supervises the maintainers.  GCC has a
minimum of non-technical policies/rules, and it has no enforcement
mechanism.  There really isn't any place to file a complaint.  You can
complain to the GCC steering committee if you like
(https://gcc.gnu.org/steering.html; I am a member), but there are a
very limited number of actions that the steering committee could take,
and it is very unlikely that the committee would actually do anything.

Basically, GCC is a cooperative project.  It's not a company.  People
don't work for GCC.

I'm sorry you are having trouble, but I don't think there is much that
the GCC project can do to help.

Ian


Re: [PATCH] middle-end/100464 - avoid spurious TREE_ADDRESSABLE in folding debug stmts

2021-05-08 Thread Jason Merrill via Gcc-patches

On 5/7/21 6:21 AM, Richard Biener wrote:

On Fri, May 7, 2021 at 12:17 PM Richard Biener  wrote:


canonicalize_constructor_val was setting TREE_ADDRESSABLE on bases
of ADDR_EXPRs but that's futile when we're dealing with CTOR values
in debug stmts.  This rips out the code which was added for Java
and should have been an assertion when we didn't have debug stmts.

Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages
which revealed PR100468 for which I added the cp/class.c hunk below.
Re-testing with that in progress.

OK for trunk and branch?  It looks like this C++ code is new in GCC 11.


I mislooked, the code is old.

This hunk also breaks (or fixes) g++.dg/tree-ssa/array-temp1.C where
the gimplifier previously passes the

&& (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object))

check guarding it against unifying addresses of different instances
of variables.  Clearly in the case of the testcase there are addresses to
this variable as part of the initializer list construction.  So the hunk fixes
wrong-code, but it breaks the testcase.

Any comments?  I can of course change the testcase accordingly.


Hmm, I suppose if the optimization is wrong for PR38615, it's also wrong 
for the initializer_list variant:


extern "C" void abort (void);
#include 

int f(int t, const int *a)
{
  std::initializer_list b = { 1, 2, 3 };
  const int *p = b.begin();
  if (!t)
return f (1, p);
 return p == a;
}

int main(void)
{
 if (f(0, 0))
   abort ();
 return 0;
}

so adjusting the array-temp testcase seems like the right answer.


Thanks,
Richard.

2021-05-07  Richard Biener  

 PR middle-end/100464
 PR c++/100468
gcc/
 * gimple-fold.c (canonicalize_constructor_val): Do not set
 TREE_ADDRESSABLE.

gcc/cp/
 * call.c (set_up_extended_ref_temp): Mark the temporary
 addressable if the TARGET_EXPR was.

gcc/testsuite/
 * gcc.dg/pr100464.c: New testcase.
---
  gcc/cp/call.c   |  2 ++
  gcc/gimple-fold.c   |  4 +++-
  gcc/testsuite/gcc.dg/pr100464.c | 16 
  3 files changed, 21 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr100464.c

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 57bac05fe70..ea97be22f07 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -12478,6 +12478,8 @@ set_up_extended_ref_temp (tree decl, tree expr, vec **cleanups,
   VAR.  */
if (TREE_CODE (expr) != TARGET_EXPR)
  expr = get_target_expr (expr);
+  else if (TREE_ADDRESSABLE (expr))
+TREE_ADDRESSABLE (var) = 1;

if (TREE_CODE (decl) == FIELD_DECL
&& extra_warnings && !TREE_NO_WARNING (decl))
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index aa33779b753..768ef89d876 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -245,7 +245,9 @@ canonicalize_constructor_val (tree cval, tree from_decl)
if (TREE_TYPE (base) == error_mark_node)
 return NULL_TREE;
if (VAR_P (base))
-   TREE_ADDRESSABLE (base) = 1;
+   /* ???  We should be able to assert that TREE_ADDRESSABLE is set,
+  but since the use can be in a debug stmt we can't.  */
+   ;
else if (TREE_CODE (base) == FUNCTION_DECL)
 {
   /* Make sure we create a cgraph node for functions we'll reference.
diff --git a/gcc/testsuite/gcc.dg/pr100464.c b/gcc/testsuite/gcc.dg/pr100464.c
new file mode 100644
index 000..46cc37dff54
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr100464.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcompare-debug" } */
+
+int *a;
+static int b, c, d, e, g, h;
+int f;
+void i() {
+  int *j[] = {&e, &b, &b, &d, &b, &b, &g, &e, &g, &b, &b,
+  &b, &b, &g, &e, &e, &b, &b, &d, &b, &b, &e,
+  &e, &g, &b, &b, &b, &b, &g, &e, &g, &c, &e};
+  int **k = &j[5];
+  for (; f;)
+b |= *a;
+  *k = &h;
+}
+int main() {}
--
2.26.2