[PATCH] i386: Handle memory operand for direct call to cvtps2pd in unpack

2022-07-06 Thread Haochen Jiang via Gcc-patches
Hi all,

This patch aim to fix the ICE for vec unpack using for memory after the commit
r13-1418 on inproper insn of cvtps2pd.

Regtested on x86_64-pc-linux-gnu. Ok for trunk?

BRs,
Haochen

gcc/ChangeLog:

PR target/106180
* config/i386/sse.md (sse2_cvtps2pd_1):
Rename from *sse2_cvtps2pd_1.
(vec_unpacks_lo_v4sf): Add handler for memory operand.

gcc/testsuite/ChangeLog:

PR target/106180
* g++.target/i386/pr106180-1.C: New test.
---
 gcc/config/i386/sse.md | 12 +++--
 gcc/testsuite/g++.target/i386/pr106180-1.C | 31 ++
 2 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/i386/pr106180-1.C

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 3396ff748da..5b91c7be54e 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -9208,7 +9208,7 @@
(set_attr "prefix" "maybe_vex")
(set_attr "mode" "V2DF")])
 
-(define_insn "*sse2_cvtps2pd_1"
+(define_insn "sse2_cvtps2pd_1"
   [(set (match_operand:V2DF 0 "register_operand" "=v")
(float_extend:V2DF
  (match_operand:V2SF 1 "memory_operand" "m")))]
@@ -9270,7 +9270,15 @@
  (vec_select:V2SF
(match_operand:V4SF 1 "vector_operand")
(parallel [(const_int 0) (const_int 1)]]
-  "TARGET_SSE2")
+  "TARGET_SSE2"
+{
+  if (MEM_P (operands[1]))
+{
+  operands[1] = adjust_address_nv (operands[1], V2SFmode, 0);
+  emit_insn (gen_sse2_cvtps2pd_1 (operands[0], operands[1]));
+  DONE;
+}
+})
 
 (define_expand "vec_unpacks_lo_v8sf"
   [(set (match_operand:V4DF 0 "register_operand")
diff --git a/gcc/testsuite/g++.target/i386/pr106180-1.C 
b/gcc/testsuite/g++.target/i386/pr106180-1.C
new file mode 100644
index 000..7f734536001
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr106180-1.C
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -c -ffloat-store  -std=c++11" } */
+
+struct PointT 
+{
+  double x, y;
+};
+using PointF = PointT;
+
+template  struct __array_traits { typedef PointT _Type[_Nm]; };
+template  struct array
+{
+  typename __array_traits<_Nm>::_Type _M_elems;
+};
+
+float SampleGrid_low, SampleGrid_high;
+using QuadrilateralF = array<4>;
+struct PerspectiveTransform
+{
+  PerspectiveTransform (QuadrilateralF, QuadrilateralF);
+};
+
+void SampleGrid()
+{
+  PerspectiveTransform
+  {
+{ PointF {SampleGrid_high, SampleGrid_low},
+  SampleGrid_low, SampleGrid_high },
+{}
+  };
+}
-- 
2.18.2



Re: [PATCH] c++: generic targs and identity substitution [PR105956]

2022-07-06 Thread Jason Merrill via Gcc-patches

On 7/6/22 15:26, Patrick Palka wrote:

On Tue, 5 Jul 2022, Jason Merrill wrote:


On 7/5/22 10:06, Patrick Palka wrote:

On Fri, 1 Jul 2022, Jason Merrill wrote:


On 6/29/22 13:42, Patrick Palka wrote:

In r13-1045-gcb7fd1ea85feea I assumed that substitution into generic
DECL_TI_ARGS corresponds to an identity mapping of the given arguments,
and hence its safe to always elide such substitution.  But this PR
demonstrates that such a substitution isn't always the identity mapping,
in particular when there's an ARGUMENT_PACK_SELECT argument, which gets
handled specially during substitution:

 * when substituting an APS into a template parameter, we strip the
   APS to its underlying argument;
 * and when substituting an APS into a pack expansion, we strip the
   APS to its underlying argument pack.


Ah, right.  For instance, in variadic96.C we have

  10template < typename... T >
  11struct derived
  12  : public base< T, derived< T... > >...

so when substituting into the base-specifier, we're approaching it from
the
outside in, so when we get to the inner T... we need some way to find the
T
pack again.  It might be possible to remove the need for APS by
substituting
inner pack expansions before outer ones, which could improve worst-case
complexity, but I don't know how relevant that is in real code; I imagine
most
inner pack expansions are as simple as this one.


Aha, that makes sense.




In this testcase, when expanding the pack expansion pattern (idx +
Ns)...
with Ns={0,1}, we specialize idx twice, first with Ns=APS<0,{0,1}> and
then Ns=APS<1,{0,1}>.  The DECL_TI_ARGS of idx are the generic template
arguments of the enclosing class template impl, so before r13-1045,
we'd substitute into its DECL_TI_ARGS which gave Ns={0,1} as desired.
But after r13-1045, we elide this substitution and end up attempting to
hash the original Ns argument, an APS, which ICEs.

So this patch partially reverts this part of r13-1045.  I considered
using preserve_args in this case instead, but that'd break the
static_assert in the testcase because preserve_args always strips APS to
its underlying argument, but here we want to strip it to its underlying
argument pack, so we'd incorrectly end up forming the specializations
impl<0>::idx and impl<1>::idx instead of impl<0,1>::idx.

Although we can't elide the substitution into DECL_TI_ARGS in light of
ARGUMENT_PACK_SELECT, it should still be safe to elide template argument
coercion in the case of a non-template decl, which this patch preserves.

It's unfortunate that we need to remove this optimization just because
it doesn't hold for one special tree code.  So this patch implements a
heuristic in tsubst_template_args to avoid allocating a new TREE_VEC if
the substituted elements are identical to those of a level from ARGS.
It turns out that about 30% of all calls to tsubst_template_args benefit
from this optimization, and it reduces memory usage by about 1.5% for
e.g. stdc++.h (relative to r13-1045).  (This is the maybe_reuse stuff,
the rest of the changes to tsubst_template_args are just drive-by
cleanups.)

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Patch generated with -w to ignore noisy whitespace changes.

PR c++/105956

gcc/cp/ChangeLog:

* pt.cc (tsubst_template_args): Move variable declarations
closer to their first use.  Replace 'orig_t' with 'r'.  Rename
'need_new' to 'const_subst_p'.  Heuristically detect if the
substituted elements are identical to that of a level from
'args' and avoid allocating a new TREE_VEC if so.
(tsubst_decl) : Revert
r13-1045-gcb7fd1ea85feea change for avoiding substitution into
DECL_TI_ARGS, but still avoid coercion in this case.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/variadic183.C: New test.
---
gcc/cp/pt.cc | 113
++-
gcc/testsuite/g++.dg/cpp0x/variadic183.C |  14 +++
2 files changed, 85 insertions(+), 42 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic183.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 8672da123f4..7898834faa6 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
 Fixed by: C++20 modules.  */
  #include "config.h"
+#define INCLUDE_ALGORITHM // for std::equal
#include "system.h"
#include "coretypes.h"
#include "cp-tree.h"
@@ -13544,17 +13545,22 @@ tsubst_argument_pack (tree orig_arg, tree
args,
tsubst_flags_t complain,
tree
tsubst_template_args (tree t, tree args, tsubst_flags_t complain,
tree
in_decl)
{
-  tree orig_t = t;
-  int len, need_new = 0, i, expanded_len_adjust = 0, out;
-  tree *elts;
-
  if (t == error_mark_node)
return error_mark_node;
-  len = TREE_VEC_LENGTH (t);
-  elts = XALLOCAVEC (tree, len);
+  const int len = TREE_VEC_LENGTH (t);
+  tree 

[PATCH 2/2] loongarch: avoid unnecessary sign-extend after 32-bit division

2022-07-06 Thread Xi Ruoyao via Gcc-patches
Like add.w/sub.w/mul.w, div.w/mod.w/div.wu/mod.wu also sign-extend the
output on LA64.  But, LoongArch v1.00 mandates that the inputs of 32-bit
division to be sign-extended so we have to expand 32-bit division into
RTL sequences.

We defined div.w/mod.w/div.wu/mod.wu as a (DI, DI) -> SI instruction.
This definition does not indicate the fact that these instructions will
store the result as sign-extended value in a 64-bit GR.  Then the
compiler would emit unnecessary sign-extend operations.  For example:

int div(int a, int b) { return a / b; }

was compiled to:

div.w  $r4, $r4, $r5
slli.w $r4, $r4, 0# this is unnecessary
jr $r1

To remove this unnecessary operation, we change the division
instructions to (DI, DI) -> DI and describe the sign-extend behavior
explicitly in the RTL template.  In the expander for 32-bit division we
then use simplify_gen_subreg to extract the lower 32 bits.

gcc/ChangeLog:

* config/loongarch/loongarch.md (di3_fake): Describe
the sign-extend of result in the RTL template.
(3): Adjust for di3_fake change.

gcc/testsuite/ChangeLog:

* gcc.target/loongarch/div-4.c: New test.
---
 gcc/config/loongarch/loongarch.md  | 12 
 gcc/testsuite/gcc.target/loongarch/div-4.c |  9 +
 2 files changed, 17 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-4.c

diff --git a/gcc/config/loongarch/loongarch.md 
b/gcc/config/loongarch/loongarch.md
index b002eb2ac22..0202f73efae 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -752,6 +752,7 @@ (define_expand "3"
   {
 rtx reg1 = gen_reg_rtx (DImode);
 rtx reg2 = gen_reg_rtx (DImode);
+rtx rd = gen_reg_rtx (DImode);
 
 operands[1] = gen_rtx_SIGN_EXTEND (word_mode, operands[1]);
 operands[2] = gen_rtx_SIGN_EXTEND (word_mode, operands[2]);
@@ -759,7 +760,9 @@ (define_expand "3"
 emit_insn (gen_rtx_SET (reg1, operands[1]));
 emit_insn (gen_rtx_SET (reg2, operands[2]));
 
-emit_insn (gen_di3_fake (operands[0], reg1, reg2));
+emit_insn (gen_di3_fake (rd, reg1, reg2));
+emit_insn (gen_rtx_SET (operands[0],
+   simplify_gen_subreg (SImode, rd, DImode, 0)));
 DONE;
   }
 })
@@ -781,9 +784,10 @@ (define_insn "*3"
(const_string "no")))])
 
 (define_insn "di3_fake"
-  [(set (match_operand:SI 0 "register_operand" "=r,,")
-   (any_div:SI (match_operand:DI 1 "register_operand" "r,r,0")
-   (match_operand:DI 2 "register_operand" "r,r,r")))]
+  [(set (match_operand:DI 0 "register_operand" "=r,,")
+   (sign_extend:DI
+ (any_div:SI (match_operand:DI 1 "register_operand" "r,r,0")
+ (match_operand:DI 2 "register_operand" "r,r,r"]
   ""
 {
   return loongarch_output_division (".w\t%0,%1,%2", operands);
diff --git a/gcc/testsuite/gcc.target/loongarch/div-4.c 
b/gcc/testsuite/gcc.target/loongarch/div-4.c
new file mode 100644
index 000..a52f87d6caf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/div-4.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "slli" } } */
+
+int
+div(int a, int b)
+{
+  return a / b;
+}
-- 
2.37.0




[PATCH 1/2] loongarch: add alternatives for idiv insns to improve code generation

2022-07-06 Thread Xi Ruoyao via Gcc-patches
Currently in the description of LoongArch integer division instructions,
the output is marked as earlyclobbered ('&').  It's necessary when
loongarch_check_zero_div_p() because clobbering operand 2 (divisor) will
make the checking for zero divisor impossible.

But, for -mno-check-zero-division (the default of GCC >= 12.2 for
optimized code), the output is not earlyclobbered at all.  And, the
read of operand 1 only occurs before clobbering the output.  So we make
three alternatives for an idiv instruction:

* (=r,r,r): For -mno-check-zero-division.
* (=,r,r): For -mcheck-zero-division.
* (=,0,r): For -mcheck-zero-division, to explicitly allow patterns
  like "div.d $a0, $a0, $a1".

gcc/ChangeLog:

* config/loongarch/loongarch.cc (loongarch_check_zero_div_p):
Remove static, for use in the machine description file.
* config/loongarch/loongarch-protos.h:
(loongarch_check_zero_div_p): Add prototype.
* config/loongarch/loongarch.md (enabled): New attr.
(*3): Add (=r,r,r) and (=,0,r) alternatives for
idiv.  Conditionally enable the alternatives using
loongarch_check_zero_div_p.
(di3_fake): Likewise.

gcc/testsuite/ChangeLog:

* gcc.target/loongarch/div-1.c: New test.
* gcc.target/loongarch/div-2.c: New test.
* gcc.target/loongarch/div-3.c: New test.
---
 gcc/config/loongarch/loongarch-protos.h|  1 +
 gcc/config/loongarch/loongarch.cc  |  2 +-
 gcc/config/loongarch/loongarch.md  | 28 +++---
 gcc/testsuite/gcc.target/loongarch/div-1.c |  9 +++
 gcc/testsuite/gcc.target/loongarch/div-2.c |  9 +++
 gcc/testsuite/gcc.target/loongarch/div-3.c |  9 +++
 6 files changed, 49 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-1.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-2.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-3.c

diff --git a/gcc/config/loongarch/loongarch-protos.h 
b/gcc/config/loongarch/loongarch-protos.h
index 2144c2421ed..2287fd3763c 100644
--- a/gcc/config/loongarch/loongarch-protos.h
+++ b/gcc/config/loongarch/loongarch-protos.h
@@ -130,6 +130,7 @@ extern bool loongarch_symbol_binds_local_p (const_rtx);
 extern const char *current_section_name (void);
 extern unsigned int current_section_flags (void);
 extern bool loongarch_use_ins_ext_p (rtx, HOST_WIDE_INT, HOST_WIDE_INT);
+extern bool loongarch_check_zero_div_p (void);
 
 union loongarch_gen_fn_ptrs
 {
diff --git a/gcc/config/loongarch/loongarch.cc 
b/gcc/config/loongarch/loongarch.cc
index d72b256df51..bc56282c9a7 100644
--- a/gcc/config/loongarch/loongarch.cc
+++ b/gcc/config/loongarch/loongarch.cc
@@ -2104,7 +2104,7 @@ loongarch_load_store_insns (rtx mem, rtx_insn *insn)
 
 /* Return true if we need to trap on division by zero.  */
 
-static bool
+bool
 loongarch_check_zero_div_p (void)
 {
   /* if -m[no-]check-zero-division is given explicitly.  */
diff --git a/gcc/config/loongarch/loongarch.md 
b/gcc/config/loongarch/loongarch.md
index d3c809e25f3..b002eb2ac22 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -110,6 +110,8 @@ (define_constants
 ;;
 ;; 
 
+(define_attr "enabled" "no,yes" (const_string "yes"))
+
 (define_attr "got" "unset,load"
   (const_string "unset"))
 
@@ -763,26 +765,36 @@ (define_expand "3"
 })
 
 (define_insn "*3"
-  [(set (match_operand:GPR 0 "register_operand" "=")
-   (any_div:GPR (match_operand:GPR 1 "register_operand" "r")
-(match_operand:GPR 2 "register_operand" "r")))]
+  [(set (match_operand:GPR 0 "register_operand" "=r,,")
+   (any_div:GPR (match_operand:GPR 1 "register_operand" "r,r,0")
+(match_operand:GPR 2 "register_operand" "r,r,r")))]
   ""
 {
   return loongarch_output_division (".\t%0,%1,%2", operands);
 }
   [(set_attr "type" "idiv")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")
+   (set (attr "enabled")
+  (if_then_else
+   (match_test "!!which_alternative == loongarch_check_zero_div_p()")
+   (const_string "yes")
+   (const_string "no")))])
 
 (define_insn "di3_fake"
-  [(set (match_operand:SI 0 "register_operand" "=")
-   (any_div:SI (match_operand:DI 1 "register_operand" "r")
-   (match_operand:DI 2 "register_operand" "r")))]
+  [(set (match_operand:SI 0 "register_operand" "=r,,")
+   (any_div:SI (match_operand:DI 1 "register_operand" "r,r,0")
+   (match_operand:DI 2 "register_operand" "r,r,r")))]
   ""
 {
   return loongarch_output_division (".w\t%0,%1,%2", operands);
 }
   [(set_attr "type" "idiv")
-   (set_attr "mode" "SI")])
+   (set_attr "mode" "SI")
+   (set (attr "enabled")
+  (if_then_else
+   (match_test "!!which_alternative == loongarch_check_zero_div_p()")
+   (const_string "yes")
+   (const_string "no")))])
 
 ;; Floating point multiply accumulate instructions.
 
diff --git 

[PATCH 0/2] loongarch: improve code generation for integer division

2022-07-06 Thread Xi Ruoyao via Gcc-patches
We were generating some unnecessary instructions for integer division.
These two patches improve the code generation to compile

template  T div(T a, T b) { return a / b; }

into a single division instruction (along with a return instruction of
course) as we expected for T in {int32_t, uint32_t, int64_t}.

Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?

Xi Ruoyao (2):
  loongarch: add alternatives for idiv insns to improve code generation
  loongarch: avoid unnecessary sign-extend after 32-bit division

 gcc/config/loongarch/loongarch-protos.h|  1 +
 gcc/config/loongarch/loongarch.cc  |  2 +-
 gcc/config/loongarch/loongarch.md  | 34 --
 gcc/testsuite/gcc.target/loongarch/div-1.c |  9 ++
 gcc/testsuite/gcc.target/loongarch/div-2.c |  9 ++
 gcc/testsuite/gcc.target/loongarch/div-3.c |  9 ++
 gcc/testsuite/gcc.target/loongarch/div-4.c |  9 ++
 7 files changed, 63 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-1.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-2.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-3.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/div-4.c

-- 
2.37.0


Re: [PATCH] Mips: Resolve build issues for the n32 ABI

2022-07-06 Thread Xi Ruoyao via Gcc-patches
On Thu, 2022-07-07 at 02:11 +0800, Xi Ruoyao via Gcc-patches wrote:
> On Wed, 2022-07-06 at 11:34 +, Dimitrije Milosevic wrote:
> > Ping. :)
> > This change just landed on LLVM (see
> > https://reviews.llvm.org/rG5d8077565e4196efdd4ed525a64c11a96d5aa5dd)
> > .
> > Unfortunately, I do not have commit access, so if anyone can commit
> > it, that would be great!
> > Here is the patch with the updated commit message.
> 
> I will push this and
> https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596226.html (with
> ChangeLog format fixed) after a regression test.

Pushed r13-1548 and r13-1549.

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


Re: [PATCH 3/3] lto-plugin: implement LDPT_GET_API_VERSION

2022-07-06 Thread Rui Ueyama via Gcc-patches
On Mon, Jul 4, 2022 at 10:17 PM Martin Liška  wrote:

> On 7/1/22 08:36, Richard Biener wrote:
> > On Thu, Jun 30, 2022 at 10:42 AM Martin Liška  wrote:
> >>
> >> On 6/30/22 08:43, Rui Ueyama wrote:
> >>> Thanks Martin for creating this patch.
> >>
> >> You're welcome.
> >>
> >>>
> >>> Here is a preliminary change for the mold side:
> https://github.com/rui314/mold/commit/9ad49d1c556bc963d06cca8233535183490de605
> <
> https://github.com/rui314/mold/commit/9ad49d1c556bc963d06cca8233535183490de605
> >
> >>>
> >>> Overall the API is looking fine,
> >>
> >> Good then!
> >>
> >>> though it is not clear what kind of value is expected as a linker
> version. A linker version is not a single unsigned integer but something
> like "1.3.0". Something like "1.3.0-rc2" can also be a linker version. So I
> don't think we can represent a linker version as a single integer.
> >>
> >> Well, you can use the same what we use GCC_VERSION (plugin_version):
> >>
> >> 1000 * MAJOR + MINOR
> >>
> >> Let me adjust the documentation of the API.
> >
> > Hmm, but then why not go back to the original suggestion merging
> > linker_identifier and linker_version into
> > a single string.  That of course puts the burden of parsing to the
> > consumer - still that's probably better
> > than imposing the constraint of encoding the version in an unsigned
> > integer.  Alternatively easing
> > parsing by separating out the version in a string would be possible as
> > well (but then you'd have
> > to care for 1.3.0-rc2+gitab4316174 or so, not sure what the advantage
> > over putting everything in
> > the identifier would be).
>
> I'm fine with the suggested 2 strings (linker_identifier and
> linker_version).
>
> Does it work for you Rui?
>

Yes.


> Cheers,
> Martin
>
> >
> > You usually cannot rely on a version anyway since distributors usually
> > apply patches.
> >
> >> Richi: May I install the patch?
> >
> > Let's sort out the version thing and consider simplifying the API.
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >>>
> >>> On Mon, Jun 20, 2022 at 9:01 PM Martin Liška  mli...@suse.cz>> wrote:
> >>>
> >>> On 6/20/22 11:35, Richard Biener wrote:
> >>> > I think this is OK.  Can we get buy-in from mold people?
> >>>
> >>> Sure, I've just pinged Rui:
> >>> https://github.com/rui314/mold/issues/454#issuecomment-1160419030
> 
> >>>
> >>> Martin
> >>>
>
>


[PATCH] libcpp: Optimize #pragma once with a hash table [PR58770]

2022-07-06 Thread Paul Hollinsky via Gcc-patches
Rather than traversing the all_files linked list for every include,
this factors out the quick idempotency checks (modification time
and size) to be the keys in a hash table so we can find matching
files quickly.

The hash table value type is a linked list, in case more than one
file matches the quick check.

The table is only built if a once-only file is seen, so include
guard performance is not affected.

My laptop would previously complete Ricardo's benchmark from the
PR in ~1.1s using #pragma once, and ~0.35s using include guards.

After this change, both benchmarks now complete in ~0.35s. I did
have to randomize the modification dates on the benchmark headers
so the files did not all end up in the same hash table list, but
that would likely not come up outside of the contrived benchmark.

libcpp/ChangeLog:

PR preprocessor/58770
* internal.h: Add hash table for #pragma once
* files.cc: Optimize #pragma once with the hash table
---
 libcpp/files.cc   | 113 +++---
 libcpp/internal.h |   3 ++
 2 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/libcpp/files.cc b/libcpp/files.cc
index 24208f7b0f8..51901aa31dd 100644
--- a/libcpp/files.cc
+++ b/libcpp/files.cc
@@ -167,6 +167,33 @@ struct file_hash_entry_pool
   struct cpp_file_hash_entry pool[FILE_HASH_POOL_SIZE];
 };
 
+/* A set of attributes designed to quickly identify obviously different files
+   in a hashtable.  Just in case there are collisions, we still maintain a
+   list.  These sub-lists can then be checked for #pragma once rather than
+   interating through all_files.  */
+struct file_quick_idempotency_attrs
+{
+  file_quick_idempotency_attrs(const _cpp_file *f)
+: mtime(f->st.st_mtime), size(f->st.st_size) {}
+
+  time_t mtime;
+  off_t size;
+
+  static hashval_t hash (/* _cpp_file* */ const void *p);
+};
+
+/* Sub-list of very similar files kept in a hashtable to check for #pragma
+   once.  */
+struct file_sublist
+{
+  _cpp_file *f;
+  file_sublist *next;
+
+  static int eq (/* _cpp_file* */ const void *p,
+/* file_sublist* */ const void *q);
+  static void del (/* file_sublist* */ void *p);
+};
+
 static bool open_file (_cpp_file *file);
 static bool pch_open_file (cpp_reader *pfile, _cpp_file *file,
   bool *invalid_pch);
@@ -849,17 +876,18 @@ has_unique_contents (cpp_reader *pfile, _cpp_file *file, 
bool import,
   if (!pfile->seen_once_only)
 return true;
 
-  /* We may have read the file under a different name.  Look
- for likely candidates and compare file contents to be sure.  */
-  for (_cpp_file *f = pfile->all_files; f; f = f->next_file)
+  /* We may have read the file under a different name.  We've kept
+ similar looking files in this lists under this hash table, so
+ check those more thoroughly.  */
+  void* ent = htab_find(pfile->pragma_once_files, file);
+  for (file_sublist *e = reinterpret_cast (ent); e; e = e->next)
 {
+  _cpp_file *f = e->f;
   if (f == file)
continue; /* It'sa me!  */
 
   if ((import || f->once_only)
- && f->err_no == 0
- && f->st.st_mtime == file->st.st_mtime
- && f->st.st_size == file->st.st_size)
+ && f->err_no == 0)
{
  _cpp_file *ref_file;
 
@@ -895,6 +923,36 @@ has_unique_contents (cpp_reader *pfile, _cpp_file *file, 
bool import,
   return true;
 }
 
+/* Add the given file to the #pragma once table so it can be
+   quickly identified and excluded the next time it's seen.  */
+static void
+update_pragma_once_table (cpp_reader *pfile, _cpp_file *file)
+{
+  void **slot = htab_find_slot (pfile->pragma_once_files, file, INSERT);
+  if (slot)
+{
+  if (!*slot)
+   *slot = xcalloc(1, sizeof(file_sublist));
+
+  file_sublist *e = reinterpret_cast (*slot);
+  while (e->f)
+   {
+ if (!e->next)
+   e->next = reinterpret_cast (
+   xcalloc(1, sizeof(file_sublist)));
+ e = e->next;
+   }
+
+  e->f = file;
+}
+  else
+{
+  cpp_error (pfile, CPP_DL_ERROR,
+"Unable to create #pragma once table space for %s",
+_cpp_get_file_name(file));
+}
+}
+
 /* Place the file referenced by FILE into a new buffer on the buffer
stack if possible.  Returns true if a buffer is stacked.  Use LOC
for any diagnostics.  */
@@ -950,6 +1008,9 @@ _cpp_stack_file (cpp_reader *pfile, _cpp_file *file, 
include_type type,
   if (!has_unique_contents (pfile, file, type == IT_IMPORT, loc))
return false;
 
+  if (pfile->seen_once_only && file->once_only)
+   update_pragma_once_table(pfile, file);
+
   if (pfile->buffer && file->dir)
sysp = MAX (pfile->buffer->sysp, file->dir->sysp);
 
@@ -1434,6 +1495,41 @@ nonexistent_file_hash_eq (const void *p, const void *q)
   return filename_cmp ((const char *) p, (const char *) q) == 0;
 }
 
+/* Hasher for the #pragma once hash table. 

Re: [PATCH] testsuite: Fix incorrect -mfloat128-type option

2022-07-06 Thread Segher Boessenkool
On Wed, Jul 06, 2022 at 07:15:08PM +0100, Jonathan Wakely wrote:
> Tested powerpc64-linux, OK for trunk?

Okay, thanks!

> -! { dg-options "-mdejagnu-cpu=405 -mpower9-minmax -mfloat128-type" }
> +! { dg-options "-mdejagnu-cpu=405 -mpower9-minmax -mfloat128" }

We really shouldn't have -mpower9-minmax selectable independently of
-mcpu=power9, but that is a different thing.

Thanks,


Segher


Re: libstdc++: Minor codegen improvement for atomic wait spinloop

2022-07-06 Thread Thomas Rodgers via Gcc-patches
Ok for trunk? backport?

On Wed, Jul 6, 2022 at 1:56 PM Jonathan Wakely  wrote:

> On Wed, 6 Jul 2022 at 02:05, Thomas Rodgers via Libstdc++
>  wrote:
> >
> > This patch merges the spin loops in the atomic wait implementation which
> is
> > a
> > minor codegen improvement.
> >
> > libstdc++-v3/ChangeLog:
> >  * include/bits/atomic_wait.h (__atomic_spin): Merge spin loops.
>
> OK, thanks.
>
>


Re: Restore 'GOMP_offload_unregister_ver' functionality (was: [Patch][v5] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Thomas Schwinge
Hi Tobias!

On 2022-07-06T15:59:59+0200, Tobias Burnus  wrote:
> On 06.07.22 12:42, Thomas Schwinge wrote:
>> --- a/libgomp/target.c
>> +++ b/libgomp/target.c

>>   /* This function should be called from every offload image while unloading.

>>   GOMP_offload_unregister_ver (unsigned version, const void *host_table,

>> /* Remove image from array of pending images.  */
>> +  bool found = false;
>> for (i = 0; i < num_offload_images; i++)
>>   if (offload_images[i].target_data == target_data)
>> {
>>  offload_images[i] = offload_images[--num_offload_images];
>> +found = true;
>>  break;
>> }
>> +  assert (found);
>>
>> gomp_mutex_unlock (_lock);
>>   }
>
> ... I don't like that libgomp crashes without any helpful message in that 
> case.
>
> In my opinion:
> * Either we assume that it is unlikely to occur - ignore it.
>(Matches the current implementation: do nothing.)
> * Or we want to have some diagnostic in case it occurs. But in that case,
>it should be some explicit diagnostic printed by gomp_error or gomp_fatal.
>IMHO, gomp_error is better than gomp_fatal as libgomp then continues 
> cleaning
>up after this error, which IMHO makes more sense that just aborting.

I'd be fine to change this into a 'gomp_error', but I don't think it's
necessary.  Maybe that wasn't obvious (and I should add a source code
comment), but my point here is that this situation really should never
arise (hence, if it does: internal error, thus 'assert').  Or, in other
words, such a check should've been present in the original implementation
already -- and would then have flagged your patch as being incomplete in
that function.

Thinking about it again, shouldn't we also add a corresponding
sanity-check ('assert') to 'GOMP_offload_register_ver', such that the
newly registered offload image must not already be present in
'offload_images'?  (Isn't that understanding also supported by the
'break' in 'if (offload_images[i].target_data == target_data)' in
'GOMP_offload_unregister_ver', as cited above: that no duplicates are
expected?)

That's at least my understanding of the situation; happy to hear if I'm
wrong.  (It's a pity that we're totally devoid of test cases for dynamic
registration/unregistration of offload images...)

Anyway: it's totally fine to address (or not, if so desired) this
sanity-check aspect independently of the other changes, so I've backed
that out, and then pushed to master branch
commit 3f05e03d6cfdf723ca0556318b6a9aa37be438e7
"Restore 'GOMP_offload_unregister_ver' functionality", 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 3f05e03d6cfdf723ca0556318b6a9aa37be438e7 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 5 Jul 2022 18:23:15 +0200
Subject: [PATCH] Restore 'GOMP_offload_unregister_ver' functionality

The recent commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp" changed the
'GOMP_offload_register_ver' interface but didn't change
'GOMP_offload_unregister_ver' accordingly, so we're no longer
actually unregistering.

	gcc/
	* config/gcn/mkoffload.cc (process_obj): Clarify 'target_data' ->
	'[...]_data'.
	* config/nvptx/mkoffload.cc (process): Likewise.
	libgomp/
	* target.c (GOMP_offload_register_ver): Clarify 'target_data' ->
	'data'.
	(GOMP_offload_unregister_ver): Likewise.  Fix up 'target_data'.
---
 gcc/config/gcn/mkoffload.cc   |  8 
 gcc/config/nvptx/mkoffload.cc |  8 
 libgomp/target.c  | 30 +++---
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index b8b3fecfcb4..d2464332275 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -692,13 +692,13 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
 	   len);
 
   fprintf (cfile,
-	   "static const struct gcn_image_desc {\n"
+	   "static const struct gcn_data {\n"
 	   "  uintptr_t omp_requires_mask;\n"
 	   "  const struct gcn_image *gcn_image;\n"
 	   "  unsigned kernel_count;\n"
 	   "  const struct hsa_kernel_description *kernel_infos;\n"
 	   "  unsigned global_variable_count;\n"
-	   "} target_data = {\n"
+	   "} gcn_data = {\n"
 	   "  %d,\n"
 	   "  _image,\n"
 	   "  sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n"
@@ -723,7 +723,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
   fprintf (cfile, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*GCN*/, _data);\n"
+	   " %d/*GCN*/, _data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 	   GOMP_DEVICE_GCN);
@@ -731,7 +731,7 @@ process_obj 

Re: libstdc++: Minor codegen improvement for atomic wait spinloop

2022-07-06 Thread Jonathan Wakely via Gcc-patches
On Wed, 6 Jul 2022 at 02:05, Thomas Rodgers via Libstdc++
 wrote:
>
> This patch merges the spin loops in the atomic wait implementation which is
> a
> minor codegen improvement.
>
> libstdc++-v3/ChangeLog:
>  * include/bits/atomic_wait.h (__atomic_spin): Merge spin loops.

OK, thanks.



Re: Ping^2: [PATCH v2] diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431]

2022-07-06 Thread Lewis Hyatt via Gcc-patches
On Wed, Jul 6, 2022 at 2:17 PM Jason Merrill  wrote:
>> On 7/6/22 10:23, Lewis Hyatt wrote:
> > Here is an updated patch addressing all your comments, thanks again for the
> > review. Regarding the idea about rewinding tokens, it seems potentially
> > problematic to try to make use of libcpp's backup token feature. I think 
> > there
> > can be pragmas which allow macro expansion, and then libcpp is not able to
> > step back through the expansion AFAIK. It would work fine for `#pragma GCC
> > diagnostic' but would break if a similar approach was added for some other
> > pragma allowing expansion in the future, and I think potentially also for
> > _Pragma. But it seems pretty tenable to handle this case just by providing 
> > an
> > interface in c-ppoutput.c to ask it to stream a given token that you have
> > lexed external to the token streaming process. That's how I set it up here,
> > it's much better than my first way IMHO, hopefully it looks OK?
> >
> > I also attached a diff of this version vs the previous version in case that
> > makes it easier to review. The C++ parts were not touched other than adding
> > the comment you suggested, the changes in this version are mostly in
> > c-ppoutput.cc and c-pragma.cc.
>
>
> > +/* When preprocessing only, pragma_lex() is not available, so obtain the 
> > tokens
> > +   directly from libcpp.  We also need to inform the token streamer about 
> > all
> > +   tokens we lex ourselves here, so it outputs them too; this is done by 
> > calling
> > +   c_pp_stream_token () for each.  */
>
> Let's add to this comment
>
> ??? If we need to support more pragmas in the future, maybe initialize
> this_parser with the pragma tokens and call pragma_lex?
>
> OK with that change.
>

Thank you very much, done.


-Lewis


Re: [PATCH]middle-end simplify complex if expressions where comparisons are inverse of one another.

2022-07-06 Thread Andrew Pinski via Gcc-patches
On Wed, Jul 6, 2022 at 9:06 AM Tamar Christina  wrote:
>
> > -Original Message-
> > From: Andrew Pinski 
> > Sent: Wednesday, July 6, 2022 3:10 AM
> > To: Tamar Christina 
> > Cc: Richard Biener ; nd ; gcc-
> > patc...@gcc.gnu.org
> > Subject: Re: [PATCH]middle-end simplify complex if expressions where
> > comparisons are inverse of one another.
> >
> > On Tue, Jul 5, 2022 at 8:16 AM Tamar Christina via Gcc-patches  > patc...@gcc.gnu.org> wrote:
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Richard Biener 
> > > > Sent: Monday, June 20, 2022 9:57 AM
> > > > To: Tamar Christina 
> > > > Cc: gcc-patches@gcc.gnu.org; nd 
> > > > Subject: Re: [PATCH]middle-end simplify complex if expressions where
> > > > comparisons are inverse of one another.
> > > >
> > > > On Thu, 16 Jun 2022, Tamar Christina wrote:
> > > >
> > > > > Hi All,
> > > > >
> > > > > This optimizes the following sequence
> > > > >
> > > > >   ((a < b) & c) | ((a >= b) & d)
> > > > >
> > > > > into
> > > > >
> > > > >   (a < b ? c : d) & 1
> > > > >
> > > > > for scalar. On vector we can omit the & 1.
> > > > >
> > > > > This changes the code generation from
> > > > >
> > > > > zoo2:
> > > > > cmp w0, w1
> > > > > csetw0, lt
> > > > > csetw1, ge
> > > > > and w0, w0, w2
> > > > > and w1, w1, w3
> > > > > orr w0, w0, w1
> > > > > ret
> > > > >
> > > > > into
> > > > >
> > > > > cmp w0, w1
> > > > > cselw0, w2, w3, lt
> > > > > and w0, w0, 1
> > > > > ret
> > > > >
> > > > > and significantly reduces the number of selects we have to do in
> > > > > the vector code.
> > > > >
> > > > > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > > > x86_64-pc-linux-gnu and no issues.
> > > > >
> > > > > Ok for master?
> > > > >
> > > > > Thanks,
> > > > > Tamar
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * fold-const.cc (inverse_conditions_p): Traverse if SSA_NAME.
> > > > > * match.pd: Add new rule.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > * gcc.target/aarch64/if-compare_1.c: New test.
> > > > > * gcc.target/aarch64/if-compare_2.c: New test.
> > > > >
> > > > > --- inline copy of patch --
> > > > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index
> > > > >
> > > >
> > 39a5a52958d87497f301826e706886b290771a2d..f180599b90150acd3ed895a64
> > > > 280
> > > > > aa3255061256 100644
> > > > > --- a/gcc/fold-const.cc
> > > > > +++ b/gcc/fold-const.cc
> > > > > @@ -2833,15 +2833,38 @@ compcode_to_comparison (enum
> > > > comparison_code
> > > > > code)  bool  inverse_conditions_p (const_tree cond1, const_tree
> > > > > cond2) {
> > > > > -  return (COMPARISON_CLASS_P (cond1)
> > > > > - && COMPARISON_CLASS_P (cond2)
> > > > > - && (invert_tree_comparison
> > > > > - (TREE_CODE (cond1),
> > > > > -  HONOR_NANS (TREE_OPERAND (cond1, 0))) == TREE_CODE
> > > > (cond2))
> > > > > - && operand_equal_p (TREE_OPERAND (cond1, 0),
> > > > > - TREE_OPERAND (cond2, 0), 0)
> > > > > - && operand_equal_p (TREE_OPERAND (cond1, 1),
> > > > > - TREE_OPERAND (cond2, 1), 0));
> > > > > +  if (COMPARISON_CLASS_P (cond1)
> > > > > +  && COMPARISON_CLASS_P (cond2)
> > > > > +  && (invert_tree_comparison
> > > > > +  (TREE_CODE (cond1),
> > > > > +   HONOR_NANS (TREE_OPERAND (cond1, 0))) == TREE_CODE
> > > > (cond2))
> > > > > +  && operand_equal_p (TREE_OPERAND (cond1, 0),
> > > > > + TREE_OPERAND (cond2, 0), 0)
> > > > > +  && operand_equal_p (TREE_OPERAND (cond1, 1),
> > > > > + TREE_OPERAND (cond2, 1), 0))
> > > > > +return true;
> > > > > +
> > > > > +  if (TREE_CODE (cond1) == SSA_NAME
> > > > > +  && TREE_CODE (cond2) == SSA_NAME)
> > > > > +{
> > > > > +  gimple *gcond1 = SSA_NAME_DEF_STMT (cond1);
> > > > > +  gimple *gcond2 = SSA_NAME_DEF_STMT (cond2);
> > > > > +  if (!is_gimple_assign (gcond1) || !is_gimple_assign (gcond2))
> > > > > +   return false;
> > > > > +
> > > > > +  tree_code code1 = gimple_assign_rhs_code (gcond1);
> > > > > +  tree_code code2 = gimple_assign_rhs_code (gcond2);
> > > > > +  return TREE_CODE_CLASS (code1) == tcc_comparison
> > > > > +&& TREE_CODE_CLASS (code2) == tcc_comparison
> > > > > +&& invert_tree_comparison (code1,
> > > > > + HONOR_NANS (gimple_arg (gcond1, 0))) == code2
> > > > > +&& operand_equal_p (gimple_arg (gcond1, 0),
> > > > > +gimple_arg (gcond2, 0), 0)
> > > > > +&& operand_equal_p (gimple_arg (gcond1, 1),
> > > > > +gimple_arg (gcond2, 1), 0);
> > > > > +}
> > > > > +
> > > > > +  return false;
> > > >
> > > > if we do extend inverse_condition_p please add an overload like
> > >
> > > Done.
> > >
> > > >
> > > > bool
> > > > inverse_condition_p (enum tree_code, tree op00, tree op01,

Re: [PATCH] c++: generic targs and identity substitution [PR105956]

2022-07-06 Thread Patrick Palka via Gcc-patches
On Tue, 5 Jul 2022, Jason Merrill wrote:

> On 7/5/22 10:06, Patrick Palka wrote:
> > On Fri, 1 Jul 2022, Jason Merrill wrote:
> > 
> > > On 6/29/22 13:42, Patrick Palka wrote:
> > > > In r13-1045-gcb7fd1ea85feea I assumed that substitution into generic
> > > > DECL_TI_ARGS corresponds to an identity mapping of the given arguments,
> > > > and hence its safe to always elide such substitution.  But this PR
> > > > demonstrates that such a substitution isn't always the identity mapping,
> > > > in particular when there's an ARGUMENT_PACK_SELECT argument, which gets
> > > > handled specially during substitution:
> > > > 
> > > > * when substituting an APS into a template parameter, we strip the
> > > >   APS to its underlying argument;
> > > > * and when substituting an APS into a pack expansion, we strip the
> > > >   APS to its underlying argument pack.
> > > 
> > > Ah, right.  For instance, in variadic96.C we have
> > > 
> > >  10   template < typename... T >
> > >  11   struct derived
> > >  12 : public base< T, derived< T... > >...
> > > 
> > > so when substituting into the base-specifier, we're approaching it from
> > > the
> > > outside in, so when we get to the inner T... we need some way to find the
> > > T
> > > pack again.  It might be possible to remove the need for APS by
> > > substituting
> > > inner pack expansions before outer ones, which could improve worst-case
> > > complexity, but I don't know how relevant that is in real code; I imagine
> > > most
> > > inner pack expansions are as simple as this one.
> > 
> > Aha, that makes sense.
> > 
> > > 
> > > > In this testcase, when expanding the pack expansion pattern (idx +
> > > > Ns)...
> > > > with Ns={0,1}, we specialize idx twice, first with Ns=APS<0,{0,1}> and
> > > > then Ns=APS<1,{0,1}>.  The DECL_TI_ARGS of idx are the generic template
> > > > arguments of the enclosing class template impl, so before r13-1045,
> > > > we'd substitute into its DECL_TI_ARGS which gave Ns={0,1} as desired.
> > > > But after r13-1045, we elide this substitution and end up attempting to
> > > > hash the original Ns argument, an APS, which ICEs.
> > > > 
> > > > So this patch partially reverts this part of r13-1045.  I considered
> > > > using preserve_args in this case instead, but that'd break the
> > > > static_assert in the testcase because preserve_args always strips APS to
> > > > its underlying argument, but here we want to strip it to its underlying
> > > > argument pack, so we'd incorrectly end up forming the specializations
> > > > impl<0>::idx and impl<1>::idx instead of impl<0,1>::idx.
> > > > 
> > > > Although we can't elide the substitution into DECL_TI_ARGS in light of
> > > > ARGUMENT_PACK_SELECT, it should still be safe to elide template argument
> > > > coercion in the case of a non-template decl, which this patch preserves.
> > > > 
> > > > It's unfortunate that we need to remove this optimization just because
> > > > it doesn't hold for one special tree code.  So this patch implements a
> > > > heuristic in tsubst_template_args to avoid allocating a new TREE_VEC if
> > > > the substituted elements are identical to those of a level from ARGS.
> > > > It turns out that about 30% of all calls to tsubst_template_args benefit
> > > > from this optimization, and it reduces memory usage by about 1.5% for
> > > > e.g. stdc++.h (relative to r13-1045).  (This is the maybe_reuse stuff,
> > > > the rest of the changes to tsubst_template_args are just drive-by
> > > > cleanups.)
> > > > 
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > > trunk?  Patch generated with -w to ignore noisy whitespace changes.
> > > > 
> > > > PR c++/105956
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > * pt.cc (tsubst_template_args): Move variable declarations
> > > > closer to their first use.  Replace 'orig_t' with 'r'.  Rename
> > > > 'need_new' to 'const_subst_p'.  Heuristically detect if the
> > > > substituted elements are identical to that of a level from
> > > > 'args' and avoid allocating a new TREE_VEC if so.
> > > > (tsubst_decl) : Revert
> > > > r13-1045-gcb7fd1ea85feea change for avoiding substitution into
> > > > DECL_TI_ARGS, but still avoid coercion in this case.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > * g++.dg/cpp0x/variadic183.C: New test.
> > > > ---
> > > >gcc/cp/pt.cc | 113
> > > > ++-
> > > >gcc/testsuite/g++.dg/cpp0x/variadic183.C |  14 +++
> > > >2 files changed, 85 insertions(+), 42 deletions(-)
> > > >create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic183.C
> > > > 
> > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > index 8672da123f4..7898834faa6 100644
> > > > --- a/gcc/cp/pt.cc
> > > > +++ b/gcc/cp/pt.cc
> > > > @@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
> > > > Fixed 

[committed] d: Merge upstream dmd 56589f0f4, druntime 651389b5, phobos 1516ecad9.

2022-07-06 Thread Iain Buclaw via Gcc-patches
Hi,

This patch merges the D front-end with upstream dmd 56589f0f4, and
the standard library with druntime 651389b5 and phobos 1516ecad9.

D front-end changes:

- Import latest bug fixes to mainline.

D runtime changes:

- Import latest bug fixes to mainline.

Phobos changes:

- Import latest bug fixes to mainline.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32/-mx32, and
committed to mainline.

Regards,
Iain.

---
gcc/d/ChangeLog:

* dmd/MERGE: Merge upstream dmd 56589f0f4.

libphobos/ChangeLog:

* libdruntime/MERGE: Merge upstream druntime 651389b5.
* src/MERGE: Merge upstream phobos 1516ecad9.
---
 gcc/d/dmd/MERGE   |   2 +-
 gcc/d/dmd/cparse.d| 118 +++-
 gcc/d/dmd/dmodule.d   |   2 +-
 gcc/d/dmd/expressionsem.d |   8 +
 gcc/d/dmd/globals.d   |   2 +-
 gcc/d/dmd/globals.h   |   2 +-
 gcc/d/dmd/hdrgen.d|  17 +-
 gcc/d/dmd/mtype.d |  15 +-
 gcc/d/dmd/tokens.d|   5 +-
 gcc/d/dmd/tokens.h|   1 +
 gcc/d/dmd/typesem.d   |   9 +-
 gcc/testsuite/gdc.test/compilable/test3004.d  |   4 +-
 gcc/testsuite/gdc.test/compilable/vcg-ast.d   |   3 +
 .../gdc.test/fail_compilation/diag_in_array.d |  20 +
 libphobos/libdruntime/MERGE   |   2 +-
 .../libdruntime/core/internal/parseoptions.d  |  17 +
 libphobos/libdruntime/core/thread/osthread.d  |   9 +
 libphobos/libdruntime/rt/aApply.d | 108 ++-
 libphobos/libdruntime/rt/aApplyR.d|  71 +-
 libphobos/libdruntime/rt/aaA.d|  39 +-
 libphobos/libdruntime/rt/arrayassign.d|  83 ++-
 libphobos/libdruntime/rt/lifetime.d   | 378 +++---
 libphobos/src/MERGE   |   2 +-
 libphobos/src/std/complex.d   |   4 +-
 libphobos/src/std/file.d  |  35 +-
 libphobos/src/std/math/exponential.d  | 648 +++---
 26 files changed, 1115 insertions(+), 489 deletions(-)
 create mode 100644 gcc/testsuite/gdc.test/fail_compilation/diag_in_array.d

diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index f5c42f0ff00..8324c1cc88c 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-529110f66d7d301d62d943a4e4482edaddeb46ea
+56589f0f4d724c1c8022c57509a243f16a04228a
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/cparse.d b/gcc/d/dmd/cparse.d
index dff76345fa5..a3bebb7365a 100644
--- a/gcc/d/dmd/cparse.d
+++ b/gcc/d/dmd/cparse.d
@@ -1619,6 +1619,12 @@ final class CParser(AST) : Parser!AST
 return;
 }
 
+if (token.value == TOK.__pragma)
+{
+uupragmaDirective(scanloc);
+return;
+}
+
 if (token.value == TOK._import) // import declaration extension
 {
 auto a = parseImport();
@@ -2322,6 +2328,14 @@ final class CParser(AST) : Parser!AST
 break;
 }
 
+case TOK.__declspec:
+{
+/* Microsoft extension
+ */
+cparseDeclspec(specifier);
+break;
+}
+
 case TOK.typeof_:
 {
 nextToken();
@@ -3042,9 +3056,13 @@ final class CParser(AST) : Parser!AST
  * extended-decl-modifier:
  *dllimport
  *dllexport
+ *noreturn
+ * Params:
+ *  specifier = filled in with the attribute(s)
  */
-private void cparseDeclspec()
+private void cparseDeclspec(ref Specifier specifier)
 {
+//printf("cparseDeclspec()\n");
 /* Check for dllexport, dllimport
  * Ignore the rest
  */
@@ -3073,6 +3091,11 @@ final class CParser(AST) : Parser!AST
 dllexport = true;
 nextToken();
 }
+else if (token.ident == Id.noreturn)
+{
+specifier.noreturn = true;
+nextToken();
+}
 else
 {
 nextToken();
@@ -3083,8 +3106,8 @@ final class CParser(AST) : Parser!AST
 else
 {
 error("extended-decl-modifier expected");
+break;
 }
-break;
 }
 }
 
@@ -4789,6 +4812,8 @@ final class CParser(AST) : Parser!AST
 // type function itself.
 if (auto tf = t.isTypeFunction())
 tf.next = tf.next.addSTC(STC.const_);
+else if (auto tt = t.isTypeTag())
+tt.mod |= MODFlags.const_;
 else
 t = t.addSTC(STC.const_);
 return t;
@@ -4961,10 +4986,40 @@ final class 

[committed] d: Build the D sources in the front-end with -fno-exceptions

2022-07-06 Thread Iain Buclaw via Gcc-patches
Hi,

The D front-end does not use exceptions, but it still requires RTTI for
some lowerings of convenience language features.  This patch enforces
that by now building GDC with `-fno-exceptions'.

Bootstrapped with gcc-9, and committed to mainline.

Regards,
Iain.

---
gcc/d/ChangeLog:

* Make-lang.in (NOEXCEPTION_DFLAGS): Define.
(ALL_DFLAGS): Add NO_EXCEPTION_DFLAGS.
---
 gcc/d/Make-lang.in | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/d/Make-lang.in b/gcc/d/Make-lang.in
index 9f134370218..6f9b2e5c26a 100644
--- a/gcc/d/Make-lang.in
+++ b/gcc/d/Make-lang.in
@@ -57,8 +57,12 @@ CHECKING_DFLAGS =
 endif
 WARN_DFLAGS = -Wall -Wdeprecated $(NOCOMMON_FLAG)
 
+# D front-end doesn't use exceptions, but it does require RTTI.
+NOEXCEPTION_DFLAGS = $(filter-out -fno-rtti, $(NOEXCEPTION_FLAGS))
+
 ALL_DFLAGS = $(DFLAGS-$@) $(GDCFLAGS) -fversion=IN_GCC $(CHECKING_DFLAGS) \
-   $(PICFLAG) $(ALIASING_FLAGS) $(COVERAGE_FLAGS) $(WARN_DFLAGS)
+   $(PICFLAG) $(ALIASING_FLAGS) $(NOEXCEPTION_DFLAGS) $(COVERAGE_FLAGS) \
+   $(WARN_DFLAGS)
 
 DCOMPILE.base = $(GDC) $(NO_PIE_CFLAGS) -c $(ALL_DFLAGS) -o $@
 DCOMPILE = $(DCOMPILE.base) -MT $@ -MMD -MP -MF $(@D)/$(DEPDIR)/$(*F).TPo
-- 
2.34.1



Re: Ping^2: [PATCH v2] diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431]

2022-07-06 Thread Jason Merrill via Gcc-patches

On 7/6/22 10:23, Lewis Hyatt wrote:

On Fri, Jul 01, 2022 at 08:23:53PM -0400, Jason Merrill wrote:

On 7/1/22 18:05, Lewis Hyatt wrote:

On Fri, Jul 1, 2022 at 3:59 PM Jason Merrill  wrote:


On 6/29/22 12:59, Jason Merrill wrote:

On 6/23/22 13:03, Lewis Hyatt via Gcc-patches wrote:

Hello-

https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595556.html
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431#c49

Would a C++ maintainer have some time to take a look at this patch
please? I feel like the PR is still worth resolving. If this doesn't
seem like a good way, I am happy to try another -- would really
appreciate any feedback. Thanks!


Thanks for your persistence, I'll take a look now.

Incidentally, when pinging it's often useful to ping someone from
MAINTAINERS directly, as well as the list.  I think your last ping got
eaten by some trouble Red Hat email was having at the time.

The cp_token_is_module_directive cleanup is OK.


Thank you very much for the advice and for going through the patch, I
really appreciate it. I went ahead and pushed the small cleanup patch.
I have responses to your comments on the main patch below too.




+  bool skip_this_pragma;


This member seems to be equivalent to
in_pragma && !should_output_pragmas ()
Maybe it could be a member function instead of a data member?



Yeah, makes sense, although I hope that by implementing your
suggestion below regarding rewinding the tokens for preprocessing,
then this can be removed anyway.


More soon.


Looks good, just a few minor comments:


+  PD_KIND_INVALID,
+  PD_KIND_PUSH,
+  PD_KIND_POP,
+  PD_KIND_IGNORED_ATTRIBUTES,
+  PD_KIND_DIAGNOSTIC,


The PD_KIND_ prefix seems unnecessary for a scoped enum.



Sure, will shorten it to PK_ instead.


+/* When preprocessing only, pragma_lex() is not available, so obtain the tokens
+   directly from libcpp.  */
+static void
+pragma_diagnostic_lex_pp (pragma_diagnostic_data *result)


Hmm, we could make a temporary lexer, but I guess this is short enough
that the duplication is OK.


I see. It would look like a version of pragma_lex() (the one in
c-parser.cc) which took a c_parser* argument so it wouldn't need to
use the global the_parser.


Or creates the_parser itself and feeds it tokens somewhat like
cp_parser_handle_statement_omp_attributes.


I didn't consider this because I was
starting from Manuel's prototype patch on the PR
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431#c10), which was
doing the parsing in libcpp itself. Perhaps it would make sense to
move to this approach in the future, if it became necessary sometime
to lex some other pragmas during preprocessing?


Sure.


+/* Similar, for the portion of a diagnostic pragma that was parsed
+   internally and so not seen by our token streamer.  */


Can we rewind after parsing so that the token streamer sees it?



Oh that's an interesting idea. It would avoid some potential issues.
For instance, I have another patch submitted to fix PR55971
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55971#c8), which is that
you can't put raw strings containing newlines into a preprocessing
directive. It occurs to me now, once that's applied, then I think a
#pragma GCC diagnostic with such a raw string (useless though it would
be) would not get output correctly by gcc -E with this current patch's
approach. An edge case certainly, but would be nice to get it right
also, and your approach would automatically handle it. I'm going to
explore this now and then follow up with a new version of the patch.


+  if (early && arg)
+{
+  /* This warning is needed because of PR71603 - popping the diagnostic
+state does not currently reset changes to option arguments, only
+changes to the option dispositions.  */
+  warning_at (data.loc_option, OPT_Wpragmas,
+ "a diagnostic pragma attempting to modify a preprocessor"
+ " option argument may not work as expected");
+}


Maybe only warn if we subsequently see a pop?



Right, that makes sense. Changing the option does work just fine until
you try to pop it. But actually this warning was kinda an
afterthought, now I just checked and at the time I wrote it, there was
only one option it could possibly apply to, since it needs to be an
option that's both for libcpp, and taking an argument, which was
-Wnormalized=. In the meantime one more has been added, -Wbidi-chars=.
Rather than add more complicated logic to remember to warn on pop for
these 2 options only, feels like maybe it would be better to either
just fix PR71603 (which I can work on sometime), or add this warning
for all options, not just libcpp options, which I guess means it
should go inside the implementation of pop... So in either case feels
like it's not really relevant to this patch and I'd propose just to
remove it for now, and then address it subsequently?


Sounds good.


+/* Handle #pragma gcc diagnostic, which needs to be done during preprocessing

[PATCH] testsuite: Fix incorrect -mfloat128-type option

2022-07-06 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64-linux, OK for trunk?

-- >8 --

This test currently fails with an error about -mfloat128-type being an
invalid option, which is not what it's supposed to be testing:

XFAIL: gcc.target/powerpc/ppc-fortran/pr80108-1.f90   -O  (test for excess 
errors)
Excess errors:
xgcc: error: unrecognized command-line option '-mfloat128-type'; did you mean 
'-mfloat128'?

With this change we get the error that the comment says it expects:

XFAIL: gcc.target/powerpc/ppc-fortran/pr80108-1.f90   -O  (test for excess 
errors)
Excess errors:
f951: Error: power9 target option is incompatible with '-mcpu=' for  
less than power9
f951: Error: '-mfloat128' requires VSX support
f951: Error: '-m64' requires a PowerPC64 cpu

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/ppc-fortran/pr80108-1.f90: Change
-mfloat128-type to -mfloat128.
---
 gcc/testsuite/gcc.target/powerpc/ppc-fortran/pr80108-1.f90 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-fortran/pr80108-1.f90 
b/gcc/testsuite/gcc.target/powerpc/ppc-fortran/pr80108-1.f90
index 844166d55e3..dac5c361689 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-fortran/pr80108-1.f90
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-fortran/pr80108-1.f90
@@ -1,7 +1,7 @@
 ! Originally contributed by Tobias Burnas.
 ! { dg-do compile { target { powerpc*-*-* } } }
 ! { dg-require-effective-target powerpc_p9vector_ok }
-! { dg-options "-mdejagnu-cpu=405 -mpower9-minmax -mfloat128-type" }
+! { dg-options "-mdejagnu-cpu=405 -mpower9-minmax -mfloat128" }
 ! { dg-excess-errors "expect error due to conflicting target options" }
 ! Since the error message is not associated with a particular line
 ! number, we cannot use the dg-error directive and cannot specify a
-- 
2.36.1



Re: [PATCH] Mips: Resolve build issues for the n32 ABI

2022-07-06 Thread Xi Ruoyao via Gcc-patches
On Wed, 2022-07-06 at 11:34 +, Dimitrije Milosevic wrote:
> Ping. :)
> This change just landed on LLVM (see 
> https://reviews.llvm.org/rG5d8077565e4196efdd4ed525a64c11a96d5aa5dd).
> Unfortunately, I do not have commit access, so if anyone can commit it, that 
> would be great!
> Here is the patch with the updated commit message.

I will push this and
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596226.html (with
ChangeLog format fixed) after a regression test.

If you will submit more GCC patches, please note:

(1) You may need to sign a FSF copyright assignment or contribute
further patches under DCO.  See "Legal Prerequisites" in
https://gcc.gnu.org/contribute.html.  For these two patches I'd consider
them "small" enough and acceptable w/o copyright assignment or DCO.

(2) You may require a sourceware.org account with write access to
gcc.git if you can get the approve from Richard or another Global
Reviewer.


[PATCH] Implement global ranges for all vrange types (SSA_NAME_RANGE_INFO).

2022-07-06 Thread Aldy Hernandez via Gcc-patches
Currently SSA_NAME_RANGE_INFO only handles integer ranges, and loses
half the precision in the process because its use of legacy
value_range's.  This patch rewrites all the SSA_NAME_RANGE_INFO
(nonzero bits included) to use the recently contributed
vrange_storage.  With it, we'll be able to efficiently save any ranges
supported by ranger in GC memory.  Presently this will only be
irange's, but shortly we'll add floating ranges and others to the mix.

As per the discussion with the trailing_wide_ints adjustments and
vrange_storage, we'll be able to save integer ranges with a maximum of
5 sub-ranges.  This could be adjusted later if more sub-ranges are
needed (unlikely).

Since this is a behavior changing patch, I would like to take a few
days for discussion, and commit early next week if all goes well.

A few notes.

First, we get rid of the SSA_NAME_ANTI_RANGE_P bit in the SSA_NAME
since we store full resolution ranges.  Perhaps it could be re-used
for something else.

The range_info_def struct is gone in favor of an opaque type handled
by vrange_storage.  It currently supports irange, but will support
frange, prange, etc, in due time.

>From the looks of it, set_range_info was an update operation despite
its name, as we improved the nonzero bits with each call, even though
we clobbered the ranges.  Presumably this was because doing a proper
intersect of ranges lost information with the anti-range hack.  We no
longer have this limitation so now we formalize both set_range_info
and set_nonzero_bits to an update operation.  After all, we should
never be losing information, but enhancing it whenever possible.  This
means, that if folks' finger-memory is not offended, as a follow-up,
I'd like to rename set_nonzero_bits and set_range_info to update_*.

I have kept the same global API we had in tree-ssanames.h, with the
caveat that all set operations are now update as discussed above.

There is a 2% performance penalty for evrp and a 3% penalty for VRP
that is coincidentally in line with a previous improvement of the same
amount in the vrange abstraction patchset.  Interestingly, this
penalty is mostly due to the wide int to tree dance we keep doing with
irange and legacy.  In a first draft of this patch where I was
streaming trees directly, there was actually a small improvement
instead.  I hope to get some of the gain back when we move irange's to
wide-ints, though I'm not in a hurry ;-).

Tested and benchmarked on x86-64 Linux.  I will also test on ppc64le
before the final commit.

Comments welcome.

gcc/ChangeLog:

* gimple-range.cc (gimple_ranger::export_global_ranges): Remove
verification against legacy value_range.
* tree-core.h (struct range_info_def): Remove.
(struct irange_storage_slot): New.
(struct tree_base): Remove SSA_NAME_ANTI_RANGE_P documentation.
(struct tree_ssa_name): Add vrange_storage support.
* tree-ssanames.cc (range_info_p): New.
(range_info_fits_p): New.
(range_info_alloc): New.
(range_info_free): New.
(range_info_get_range): New.
(range_info_set_range): New.
(set_range_info_raw): Remove.
(set_range_info): Adjust to use vrange_storage.
(set_nonzero_bits): Same.
(get_nonzero_bits): Same.
(duplicate_ssa_name_range_info): Remove overload taking
value_range_kind.
Rewrite tree overload to use vrange_storage.
(duplicate_ssa_name_fn): Adjust to use vrange_storage.
* tree-ssanames.h (struct range_info_def): Remove.
(set_range_info): Adjust prototype to take vrange.
* tree-vrp.cc (vrp_asserts::remove_range_assertions): Call
duplicate_ssa_name_range_info.
* tree.h (SSA_NAME_ANTI_RANGE_P): Remove.
(SSA_NAME_RANGE_TYPE): Remove.
* value-query.cc (get_ssa_name_range_info): Adjust to use
vrange_storage.
(update_global_range): Use int_range_max.
(get_range_global): Remove as_a.
---
 gcc/gimple-range.cc  |  11 +--
 gcc/tree-core.h  |  13 +--
 gcc/tree-ssanames.cc | 231 +++
 gcc/tree-ssanames.h  |  12 +--
 gcc/tree-vrp.cc  |  22 +++--
 gcc/tree.h   |   8 --
 gcc/value-query.cc   |  21 ++--
 7 files changed, 135 insertions(+), 183 deletions(-)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 3a9f0b07e79..2a599ce870e 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -525,19 +525,10 @@ gimple_ranger::export_global_ranges ()
  if (!irange::supports_p (TREE_TYPE (name)))
continue;
 
- vrange  = r;
- value_range vr = as_a  (v);
  print_generic_expr (dump_file, name , TDF_SLIM);
  fprintf (dump_file, "  : ");
- vr.dump (dump_file);
+ r.dump (dump_file);
  fprintf (dump_file, "\n");
- int_range_max same = vr;
- if (same != as_a  (v))
-   {
- fprintf (dump_file, " irange : ");

RE: [PATCH]middle-end simplify complex if expressions where comparisons are inverse of one another.

2022-07-06 Thread Tamar Christina via Gcc-patches
> -Original Message-
> From: Andrew Pinski 
> Sent: Wednesday, July 6, 2022 3:10 AM
> To: Tamar Christina 
> Cc: Richard Biener ; nd ; gcc-
> patc...@gcc.gnu.org
> Subject: Re: [PATCH]middle-end simplify complex if expressions where
> comparisons are inverse of one another.
> 
> On Tue, Jul 5, 2022 at 8:16 AM Tamar Christina via Gcc-patches  patc...@gcc.gnu.org> wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Richard Biener 
> > > Sent: Monday, June 20, 2022 9:57 AM
> > > To: Tamar Christina 
> > > Cc: gcc-patches@gcc.gnu.org; nd 
> > > Subject: Re: [PATCH]middle-end simplify complex if expressions where
> > > comparisons are inverse of one another.
> > >
> > > On Thu, 16 Jun 2022, Tamar Christina wrote:
> > >
> > > > Hi All,
> > > >
> > > > This optimizes the following sequence
> > > >
> > > >   ((a < b) & c) | ((a >= b) & d)
> > > >
> > > > into
> > > >
> > > >   (a < b ? c : d) & 1
> > > >
> > > > for scalar. On vector we can omit the & 1.
> > > >
> > > > This changes the code generation from
> > > >
> > > > zoo2:
> > > > cmp w0, w1
> > > > csetw0, lt
> > > > csetw1, ge
> > > > and w0, w0, w2
> > > > and w1, w1, w3
> > > > orr w0, w0, w1
> > > > ret
> > > >
> > > > into
> > > >
> > > > cmp w0, w1
> > > > cselw0, w2, w3, lt
> > > > and w0, w0, 1
> > > > ret
> > > >
> > > > and significantly reduces the number of selects we have to do in
> > > > the vector code.
> > > >
> > > > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > > x86_64-pc-linux-gnu and no issues.
> > > >
> > > > Ok for master?
> > > >
> > > > Thanks,
> > > > Tamar
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > * fold-const.cc (inverse_conditions_p): Traverse if SSA_NAME.
> > > > * match.pd: Add new rule.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * gcc.target/aarch64/if-compare_1.c: New test.
> > > > * gcc.target/aarch64/if-compare_2.c: New test.
> > > >
> > > > --- inline copy of patch --
> > > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index
> > > >
> > >
> 39a5a52958d87497f301826e706886b290771a2d..f180599b90150acd3ed895a64
> > > 280
> > > > aa3255061256 100644
> > > > --- a/gcc/fold-const.cc
> > > > +++ b/gcc/fold-const.cc
> > > > @@ -2833,15 +2833,38 @@ compcode_to_comparison (enum
> > > comparison_code
> > > > code)  bool  inverse_conditions_p (const_tree cond1, const_tree
> > > > cond2) {
> > > > -  return (COMPARISON_CLASS_P (cond1)
> > > > - && COMPARISON_CLASS_P (cond2)
> > > > - && (invert_tree_comparison
> > > > - (TREE_CODE (cond1),
> > > > -  HONOR_NANS (TREE_OPERAND (cond1, 0))) == TREE_CODE
> > > (cond2))
> > > > - && operand_equal_p (TREE_OPERAND (cond1, 0),
> > > > - TREE_OPERAND (cond2, 0), 0)
> > > > - && operand_equal_p (TREE_OPERAND (cond1, 1),
> > > > - TREE_OPERAND (cond2, 1), 0));
> > > > +  if (COMPARISON_CLASS_P (cond1)
> > > > +  && COMPARISON_CLASS_P (cond2)
> > > > +  && (invert_tree_comparison
> > > > +  (TREE_CODE (cond1),
> > > > +   HONOR_NANS (TREE_OPERAND (cond1, 0))) == TREE_CODE
> > > (cond2))
> > > > +  && operand_equal_p (TREE_OPERAND (cond1, 0),
> > > > + TREE_OPERAND (cond2, 0), 0)
> > > > +  && operand_equal_p (TREE_OPERAND (cond1, 1),
> > > > + TREE_OPERAND (cond2, 1), 0))
> > > > +return true;
> > > > +
> > > > +  if (TREE_CODE (cond1) == SSA_NAME
> > > > +  && TREE_CODE (cond2) == SSA_NAME)
> > > > +{
> > > > +  gimple *gcond1 = SSA_NAME_DEF_STMT (cond1);
> > > > +  gimple *gcond2 = SSA_NAME_DEF_STMT (cond2);
> > > > +  if (!is_gimple_assign (gcond1) || !is_gimple_assign (gcond2))
> > > > +   return false;
> > > > +
> > > > +  tree_code code1 = gimple_assign_rhs_code (gcond1);
> > > > +  tree_code code2 = gimple_assign_rhs_code (gcond2);
> > > > +  return TREE_CODE_CLASS (code1) == tcc_comparison
> > > > +&& TREE_CODE_CLASS (code2) == tcc_comparison
> > > > +&& invert_tree_comparison (code1,
> > > > + HONOR_NANS (gimple_arg (gcond1, 0))) == code2
> > > > +&& operand_equal_p (gimple_arg (gcond1, 0),
> > > > +gimple_arg (gcond2, 0), 0)
> > > > +&& operand_equal_p (gimple_arg (gcond1, 1),
> > > > +gimple_arg (gcond2, 1), 0);
> > > > +}
> > > > +
> > > > +  return false;
> > >
> > > if we do extend inverse_condition_p please add an overload like
> >
> > Done.
> >
> > >
> > > bool
> > > inverse_condition_p (enum tree_code, tree op00, tree op01,
> > >  enum tree_code, tree op10, tree op11)
> > >
> > > so you can avoid some code duplication here.
> > >
> > > >  }
> > > >
> > > >  /* Return a tree for the comparison which is the combination of
> > > > diff --git a/gcc/match.pd b/gcc/match.pd index
> > > >
> > >
> 

Re: [PATCH 3/3] analyzer: add a new testcase to demonstrate passsing of a file descriptor to a function that does not emit any warning

2022-07-06 Thread David Malcolm via Gcc-patches
On Wed, 2022-07-06 at 19:55 +0530, Immad Mir wrote:
> gcc/testsuite/ChangeLog:
> * gcc.dg/analyzer/fd-4.c: Add a new testcase to demonstrate
> passsing
> of a file descriptor to a function that does not emit any
> warning.

The patch itself is OK for trunk, but the initial line of the commit
message is probably too long - this gets used as the title of the
commit in various git UIs.  I can't remember the recommended limit, it
might be 80 chars (which can be hard when the leading "analyzer: "
takes up 10 chars).

Please reduce the length before pushing, maybe to:
  analyzer: add testcase of using closed fd without warning
or somesuch

Thanks
Dave


> 
> Signed-off-by: Immad Mir 
> ---
>  gcc/testsuite/gcc.dg/analyzer/fd-4.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> index c992db619e7..fcfa6168efa 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> @@ -1,3 +1,5 @@
> +#include 
> +
>  int open(const char *, int mode);
>  void close(int fd);
>  int write (int fd, void *buf, int nbytes);
> @@ -60,3 +62,11 @@ test_4 (const char *path, void *buf)
>  /* {dg-message "\\(3\\) 'write' on closed file descriptor
> 'fd'; 'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
>  }
>  }
> +
> +void
> +test_5 (const char *path)
> +{
> +    int fd = open (path, O_RDWR);
> +    close(fd);
> +    printf("%d", fd); /* { dg-bogus "'printf' on a closed file
> descriptor 'fd'" } */
> +}
> \ No newline at end of file




Re: [PATCH 1/3] analyzer: show close event for use_after_close diagnostic

2022-07-06 Thread David Malcolm via Gcc-patches
On Wed, 2022-07-06 at 19:55 +0530, Immad Mir wrote:
> From: Immad Mir 
> 
> This patch saves the "close" event in use_after_close diagnostic
> and shows it where possible.

This patch is OK to push to trunk (assuming you've done the usual
testing).

Thanks
Dave

> 
> gcc/analyzer/ChangeLog:
> * sm-fd.cc (use_after_close): save the "close" event and
> show it where possible.
> 
> gcc/testsuite/ChangeLog:
> * gcc.dg/analyzer/fd-4.c (test_3): change the message note to
> conform to the
> changes in analyzer/sm-fd.cc
> (test_4): Likewise.
> 
> Signed-off-by: Immad Mir 
> ---
>  gcc/analyzer/sm-fd.cc    | 15 ---
>  gcc/testsuite/gcc.dg/analyzer/fd-4.c |  4 ++--
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> index 4058ac53308..8196d33223a 100644
> --- a/gcc/analyzer/sm-fd.cc
> +++ b/gcc/analyzer/sm-fd.cc
> @@ -454,7 +454,10 @@ public:
>    return label_text::borrow ("opened here");
>  
>  if (change.m_new_state == m_sm.m_closed)
> -  return change.formatted_print ("closed here");
> +  {
> +    m_first_close_event = change.m_event_id;
> +    return change.formatted_print ("closed here");
> +  }
>  
>  return fd_diagnostic::describe_state_change (change);
>    }
> @@ -462,11 +465,17 @@ public:
>    label_text
>    describe_final_event (const evdesc::final_event ) final
> override
>    {
> -    return ev.formatted_print ("%qE on closed file descriptor %qE
> here",
> -   m_callee_fndecl, m_arg);
> +    if (m_first_close_event.known_p ())
> +  return ev.formatted_print (
> +  "%qE on closed file descriptor %qE; %qs was at %@",
> m_callee_fndecl,
> +  m_arg, "close", _first_close_event);
> +    else
> +  return ev.formatted_print ("%qE on closed file descriptor
> %qE",
> + m_callee_fndecl, m_arg);
>    }
>  
>  private:
> +  diagnostic_event_id_t m_first_close_event;
>    const tree m_callee_fndecl;
>  };
>  
> diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> index a973704f403..c992db619e7 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
> @@ -45,7 +45,7 @@ test_3 (const char *path, void *buf)
>  {
>  close(fd); /* {dg-message "\\(2\\) closed here"} */
>  read(fd, buf, 1); /* { dg-warning "'read' on closed file
> descriptor 'fd'" }  */
> -    /* {dg-message "\\(3\\) 'read' on closed file descriptor
> 'fd' here" "" {target *-*-*} .-1 } */
> +    /* {dg-message "\\(3\\) 'read' on closed file descriptor
> 'fd'; 'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
>  }
>  }
>  
> @@ -57,6 +57,6 @@ test_4 (const char *path, void *buf)
>  {
>  close(fd); /* {dg-message "\\(2\\) closed here"} */
>  write(fd, buf, 1); /* { dg-warning "'write' on closed file
> descriptor 'fd'" }  */
> -    /* {dg-message "\\(3\\) 'write' on closed file descriptor
> 'fd' here" "" {target *-*-*} .-1 } */
> +    /* {dg-message "\\(3\\) 'write' on closed file descriptor
> 'fd'; 'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
>  }
>  }




Re: [PATCH 2/3] analyzer: reorder initialization of state m_invalid in sm-fd.cc [PR106184]

2022-07-06 Thread David Malcolm via Gcc-patches
On Wed, 2022-07-06 at 19:55 +0530, Immad Mir wrote:
> From: Immad Mir 
> 
> This patch reorders the initialization of state m_invalid in sm-fd.cc
> to conform with standard practice in C++.

Might want to reword to say that the order of the initializers is now
the same as the ordering of the fields in the class decl, or somesuch.


I think I already approved this; it's OK to push this to trunk.

Thanks
Dave

> 
> 
> gcc/analyzer/ChangeLog:
> PR analyzer/106184
> * sm-fd.cc (fd_state_machine): Change ordering of
> initialization
> of state m_invalid.
> 
> Signed-off-by: Immad Mir 
> ---
>  gcc/analyzer/sm-fd.cc | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> index 8196d33223a..8e4300b06e2 100644
> --- a/gcc/analyzer/sm-fd.cc
> +++ b/gcc/analyzer/sm-fd.cc
> @@ -551,11 +551,12 @@ fd_state_machine::fd_state_machine (logger
> *logger)
>    m_unchecked_read_write (add_state ("fd-unchecked-read-write")),
>    m_unchecked_read_only (add_state ("fd-unchecked-read-only")),
>    m_unchecked_write_only (add_state ("fd-unchecked-write-only")),
> -  m_invalid (add_state ("fd-invalid")),
>    m_valid_read_write (add_state ("fd-valid-read-write")),
>    m_valid_read_only (add_state ("fd-valid-read-only")),
>    m_valid_write_only (add_state ("fd-valid-write-only")),
> -  m_closed (add_state ("fd-closed")), m_stop (add_state ("fd-
> stop"))
> +  m_invalid (add_state ("fd-invalid")),
> +  m_closed (add_state ("fd-closed")),
> +  m_stop (add_state ("fd-stop"))
>  {
>  }
>  




[PATCH 2/3] analyzer: reorder initialization of state m_invalid in sm-fd.cc [PR106184]

2022-07-06 Thread Immad Mir via Gcc-patches
From: Immad Mir 

This patch reorders the initialization of state m_invalid in sm-fd.cc
to conform with standard practice in C++.

gcc/analyzer/ChangeLog:
PR analyzer/106184
* sm-fd.cc (fd_state_machine): Change ordering of initialization
of state m_invalid.

Signed-off-by: Immad Mir 
---
 gcc/analyzer/sm-fd.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 8196d33223a..8e4300b06e2 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -551,11 +551,12 @@ fd_state_machine::fd_state_machine (logger *logger)
   m_unchecked_read_write (add_state ("fd-unchecked-read-write")),
   m_unchecked_read_only (add_state ("fd-unchecked-read-only")),
   m_unchecked_write_only (add_state ("fd-unchecked-write-only")),
-  m_invalid (add_state ("fd-invalid")),
   m_valid_read_write (add_state ("fd-valid-read-write")),
   m_valid_read_only (add_state ("fd-valid-read-only")),
   m_valid_write_only (add_state ("fd-valid-write-only")),
-  m_closed (add_state ("fd-closed")), m_stop (add_state ("fd-stop"))
+  m_invalid (add_state ("fd-invalid")),
+  m_closed (add_state ("fd-closed")),
+  m_stop (add_state ("fd-stop"))
 {
 }
 
-- 
2.25.1



[PATCH 3/3] analyzer: add a new testcase to demonstrate passsing of a file descriptor to a function that does not emit any warning

2022-07-06 Thread Immad Mir via Gcc-patches
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/fd-4.c: Add a new testcase to demonstrate passsing
of a file descriptor to a function that does not emit any warning.

Signed-off-by: Immad Mir 
---
 gcc/testsuite/gcc.dg/analyzer/fd-4.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c 
b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
index c992db619e7..fcfa6168efa 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
@@ -1,3 +1,5 @@
+#include 
+
 int open(const char *, int mode);
 void close(int fd);
 int write (int fd, void *buf, int nbytes);
@@ -60,3 +62,11 @@ test_4 (const char *path, void *buf)
 /* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd'; 
'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
 }
 }
+
+void
+test_5 (const char *path)
+{
+int fd = open (path, O_RDWR);
+close(fd);
+printf("%d", fd); /* { dg-bogus "'printf' on a closed file descriptor 
'fd'" } */
+}
\ No newline at end of file
-- 
2.25.1



[PATCH 1/3] analyzer: show close event for use_after_close diagnostic

2022-07-06 Thread Immad Mir via Gcc-patches
From: Immad Mir 

This patch saves the "close" event in use_after_close diagnostic
and shows it where possible.

gcc/analyzer/ChangeLog:
* sm-fd.cc (use_after_close): save the "close" event and
show it where possible.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/fd-4.c (test_3): change the message note to conform 
to the
changes in analyzer/sm-fd.cc
(test_4): Likewise.

Signed-off-by: Immad Mir 
---
 gcc/analyzer/sm-fd.cc| 15 ---
 gcc/testsuite/gcc.dg/analyzer/fd-4.c |  4 ++--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 4058ac53308..8196d33223a 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -454,7 +454,10 @@ public:
   return label_text::borrow ("opened here");
 
 if (change.m_new_state == m_sm.m_closed)
-  return change.formatted_print ("closed here");
+  {
+m_first_close_event = change.m_event_id;
+return change.formatted_print ("closed here");
+  }
 
 return fd_diagnostic::describe_state_change (change);
   }
@@ -462,11 +465,17 @@ public:
   label_text
   describe_final_event (const evdesc::final_event ) final override
   {
-return ev.formatted_print ("%qE on closed file descriptor %qE here",
-   m_callee_fndecl, m_arg);
+if (m_first_close_event.known_p ())
+  return ev.formatted_print (
+  "%qE on closed file descriptor %qE; %qs was at %@", m_callee_fndecl,
+  m_arg, "close", _first_close_event);
+else
+  return ev.formatted_print ("%qE on closed file descriptor %qE",
+ m_callee_fndecl, m_arg);
   }
 
 private:
+  diagnostic_event_id_t m_first_close_event;
   const tree m_callee_fndecl;
 };
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c 
b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
index a973704f403..c992db619e7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
@@ -45,7 +45,7 @@ test_3 (const char *path, void *buf)
 {
 close(fd); /* {dg-message "\\(2\\) closed here"} */
 read(fd, buf, 1); /* { dg-warning "'read' on closed file descriptor 
'fd'" }  */
-/* {dg-message "\\(3\\) 'read' on closed file descriptor 'fd' here" "" 
{target *-*-*} .-1 } */
+/* {dg-message "\\(3\\) 'read' on closed file descriptor 'fd'; 'close' 
was at \\(2\\)" "" {target *-*-*} .-1 } */
 }
 }
 
@@ -57,6 +57,6 @@ test_4 (const char *path, void *buf)
 {
 close(fd); /* {dg-message "\\(2\\) closed here"} */
 write(fd, buf, 1); /* { dg-warning "'write' on closed file descriptor 
'fd'" }  */
-/* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd' here" 
"" {target *-*-*} .-1 } */
+/* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd'; 
'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
 }
 }
-- 
2.25.1



Re: Ping^2: [PATCH v2] diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431]

2022-07-06 Thread Lewis Hyatt via Gcc-patches
On Fri, Jul 01, 2022 at 08:23:53PM -0400, Jason Merrill wrote:
> On 7/1/22 18:05, Lewis Hyatt wrote:
> > On Fri, Jul 1, 2022 at 3:59 PM Jason Merrill  wrote:
> > > 
> > > On 6/29/22 12:59, Jason Merrill wrote:
> > > > On 6/23/22 13:03, Lewis Hyatt via Gcc-patches wrote:
> > > > > Hello-
> > > > > 
> > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595556.html
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431#c49
> > > > > 
> > > > > Would a C++ maintainer have some time to take a look at this patch
> > > > > please? I feel like the PR is still worth resolving. If this doesn't
> > > > > seem like a good way, I am happy to try another -- would really
> > > > > appreciate any feedback. Thanks!
> > > > 
> > > > Thanks for your persistence, I'll take a look now.
> > > > 
> > > > Incidentally, when pinging it's often useful to ping someone from
> > > > MAINTAINERS directly, as well as the list.  I think your last ping got
> > > > eaten by some trouble Red Hat email was having at the time.
> > > > 
> > > > The cp_token_is_module_directive cleanup is OK.
> > 
> > Thank you very much for the advice and for going through the patch, I
> > really appreciate it. I went ahead and pushed the small cleanup patch.
> > I have responses to your comments on the main patch below too.
> > 
> > > > 
> > > > > +  bool skip_this_pragma;
> > > > 
> > > > This member seems to be equivalent to
> > > >in_pragma && !should_output_pragmas ()
> > > > Maybe it could be a member function instead of a data member?
> > > > 
> > 
> > Yeah, makes sense, although I hope that by implementing your
> > suggestion below regarding rewinding the tokens for preprocessing,
> > then this can be removed anyway.
> > 
> > > > More soon.
> > > 
> > > Looks good, just a few minor comments:
> > > 
> > > > +  PD_KIND_INVALID,
> > > > +  PD_KIND_PUSH,
> > > > +  PD_KIND_POP,
> > > > +  PD_KIND_IGNORED_ATTRIBUTES,
> > > > +  PD_KIND_DIAGNOSTIC,
> > > 
> > > The PD_KIND_ prefix seems unnecessary for a scoped enum.
> > > 
> > 
> > Sure, will shorten it to PK_ instead.
> > 
> > > > +/* When preprocessing only, pragma_lex() is not available, so obtain 
> > > > the tokens
> > > > +   directly from libcpp.  */
> > > > +static void
> > > > +pragma_diagnostic_lex_pp (pragma_diagnostic_data *result)
> > > 
> > > Hmm, we could make a temporary lexer, but I guess this is short enough
> > > that the duplication is OK.
> > 
> > I see. It would look like a version of pragma_lex() (the one in
> > c-parser.cc) which took a c_parser* argument so it wouldn't need to
> > use the global the_parser.
> 
> Or creates the_parser itself and feeds it tokens somewhat like
> cp_parser_handle_statement_omp_attributes.
> 
> > I didn't consider this because I was
> > starting from Manuel's prototype patch on the PR
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431#c10), which was
> > doing the parsing in libcpp itself. Perhaps it would make sense to
> > move to this approach in the future, if it became necessary sometime
> > to lex some other pragmas during preprocessing?
> 
> Sure.
> 
> > > > +/* Similar, for the portion of a diagnostic pragma that was parsed
> > > > +   internally and so not seen by our token streamer.  */
> > > 
> > > Can we rewind after parsing so that the token streamer sees it?
> > > 
> > 
> > Oh that's an interesting idea. It would avoid some potential issues.
> > For instance, I have another patch submitted to fix PR55971
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55971#c8), which is that
> > you can't put raw strings containing newlines into a preprocessing
> > directive. It occurs to me now, once that's applied, then I think a
> > #pragma GCC diagnostic with such a raw string (useless though it would
> > be) would not get output correctly by gcc -E with this current patch's
> > approach. An edge case certainly, but would be nice to get it right
> > also, and your approach would automatically handle it. I'm going to
> > explore this now and then follow up with a new version of the patch.
> > 
> > > > +  if (early && arg)
> > > > +{
> > > > +  /* This warning is needed because of PR71603 - popping the 
> > > > diagnostic
> > > > +state does not currently reset changes to option arguments, 
> > > > only
> > > > +changes to the option dispositions.  */
> > > > +  warning_at (data.loc_option, OPT_Wpragmas,
> > > > + "a diagnostic pragma attempting to modify a 
> > > > preprocessor"
> > > > + " option argument may not work as expected");
> > > > +}
> > > 
> > > Maybe only warn if we subsequently see a pop?
> > > 
> > 
> > Right, that makes sense. Changing the option does work just fine until
> > you try to pop it. But actually this warning was kinda an
> > afterthought, now I just checked and at the time I wrote it, there was
> > only one option it could possibly apply to, since it needs to be an
> > option that's both for libcpp, and taking an 

Re: [GCC 13][PATCH] PR101836: Add a new option -fstrict-flex-array[=n] and use it in __builtin_object_size

2022-07-06 Thread Qing Zhao via Gcc-patches
(Sorry for the late reply, just came back from a short vacation.)

> On Jul 4, 2022, at 2:49 AM, Richard Biener  wrote:
> 
> On Fri, Jul 1, 2022 at 5:32 PM Martin Sebor  wrote:
>> 
>> On 7/1/22 08:01, Qing Zhao wrote:
>>> 
>>> 
 On Jul 1, 2022, at 8:59 AM, Jakub Jelinek  wrote:
 
 On Fri, Jul 01, 2022 at 12:55:08PM +, Qing Zhao wrote:
> If so, comparing to the current implemenation to have all the checking in 
> middle-end, what’s the
> major benefit of moving part of the checking into FE, and leaving the 
> other part in middle-end?
 
 The point is recording early what FIELD_DECLs could be vs. can't possibly 
 be
 treated like flexible array members and just use that flag in the decisions
 in the current routines in addition to what it is doing.
>>> 
>>> Okay.
>>> 
>>> Based on the discussion so far, I will do the following:
>>> 
>>> 1. Add a new flag “DECL_NOT_FLEXARRAY” to FIELD_DECL;
>>> 2. In C/C++ FE, set the new flag “DECL_NOT_FLEXARRAY” for a FIELD_DECL 
>>> based on [0], [1],
>>> [] and the option -fstrict-flex-array, and whether it’s the last field 
>>> of the DECL_CONTEXT.
>>> 3. In Middle end,  Add a new utility routine is_flexible_array_member_p, 
>>> which bases on
>>> DECL_NOT_FLEXARRAY + array_at_struct_end_p to decide whether the array
>>> reference is a real flexible array member reference.
> 
> I would just update all existing users, not introduce another wrapper
> that takes DECL_NOT_FLEXARRAY
> into account additionally.

Okay. 
> 
>>> 
>>> 
>>> Middle end currently is quite mess, array_at_struct_end_p, 
>>> component_ref_size, and all the phases that
>>> use these routines need to be updated, + new testing cases for each of the 
>>> phases.
>>> 
>>> 
>>> So, I still plan to separate the patch set into 2 parts:
>>> 
>>>   Part A:the above 1 + 2 + 3,  and use these new utilities in 
>>> tree-object-size.cc to resolve PR101836 first.
>>>  Then kernel can use __FORTIFY_SOURCE correctly;
>>> 
>>>   Part B:update all other phases with the new utilities + new testing 
>>> cases + resolving regressions.
>>> 
>>> Let me know if you have any comment and suggestion.
>> 
>> It might be worth considering whether it should be possible to control
>> the "flexible array" property separately for each trailing array member
>> via either a #pragma or an attribute in headers that can't change
>> the struct layout but that need to be usable in programs compiled with
>> stricter -fstrict-flex-array=N settings.
> 
> Or an decl attribute.

Yes, it might be necessary to add a corresponding decl attribute 

strict_flex_array (N)

Which is attached to a trailing structure array member to provide the user a 
finer control when -fstrict-flex-array=N is specified. 

So, I will do the following:


*User interface:

1. command line option:
 -fstrict-flex-array=N   (N=0, 1, 2, 3)
2.  decl attribute:
 strict_flex_array (N)  (N=0, 1, 2, 3)


*Implementation:

1. Add a new flag “DECL_NOT_FLEXARRAY” to FIELD_DECL;
2. In C/C++ FE, set the new flag “DECL_NOT_FLEXARRAY” for a FIELD_DECL based on 
[0], [1],
 [], the option -fstrict-flex-array, the attribute strict_flex_array,  and 
whether it’s the last field 
 of the DECL_CONTEXT.
3. In Middle end,   update all users of “array_at_struct_end_p" or 
“component_ref_size”, or any place that treats
Trailing array as flexible array member with the new flag  
DECL_NOT_FLEXARRAY. 
(Still think we need a new consistent utility routine here). 


I still plan to separate the patch set into 2 parts:

Part A:the above 1 + 2 + 3,  and use these new utilities in 
tree-object-size.cc to resolve PR101836 first.
   Then kernel can use __FORTIFY_SOURCE correctly.
Part B:update all other phases with the new utilities + new testing cases + 
resolving regressions.


Let me know any more comment or suggestion.

Thanks a lot.

Qing




Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Tobias Burnus

Hi Thomas, hello all,

On 06.07.22 13:04, Thomas Schwinge wrote:

On 2022-06-08T05:56:02+0200, Tobias Burnus 
wrote:

PS: I have not fully tested the intelmic version.

...

Subject: [PATCH] Fix Intel MIC 'mkoffload' for OpenMP 'requires'

...

This also means finally switching Intel MIC 'mkoffload' to
'GOMP_offload_register_ver', 'GOMP_offload_unregister_ver',
making 'GOMP_offload_register', 'GOMP_offload_unregister'
legacy entry points.

...

  gcc/
  * config/i386/intelmic-mkoffload.cc (generate_host_descr_file)
  (prepare_target_image, main): Handle OpenMP 'requires'.
  (generate_host_descr_file): Switch to 'GOMP_offload_register_ver',
  'GOMP_offload_unregister_ver'.
  libgomp/
  * target.c (GOMP_offload_register, GOMP_offload_unregister):
  Denote as legacy entry points.
  * testsuite/libgomp.c-c++-common/requires-1.c: Enable for all
  'target offloading_enabled'.
  * testsuite/libgomp.c-c++-common/requires-5.c: Likewise.
  * testsuite/libgomp.c-c++-common/requires-7.c: Likewise.
  * testsuite/libgomp.fortran/requires-1.f90: Likewise.

...

diff --git a/gcc/config/i386/intelmic-mkoffload.cc 
b/gcc/config/i386/intelmic-mkoffload.cc
index c683d6f473e..596f6f107b8 100644
--- a/gcc/config/i386/intelmic-mkoffload.cc
+++ b/gcc/config/i386/intelmic-mkoffload.cc
@@ -370,7 +370,7 @@ generate_target_offloadend_file (const char 
*target_compiler)

  /* Generates object file with the host side descriptor.  */
  static const char *
-generate_host_descr_file (const char *host_compiler)
+generate_host_descr_file (const char *host_compiler, uint32_t omp_requires)
  {
char *dump_filename = concat (dumppfx, "_host_descr.c", NULL);
const char *src_filename = save_temps
@@ -386,39 +386,50 @@ generate_host_descr_file (const char *host_compiler)
if (!src_file)
  fatal_error (input_location, "cannot open '%s'", src_filename);

+  fprintf (src_file, "#include \n\n");
+
fprintf (src_file,
 "extern const void *const __OFFLOAD_TABLE__;\n"
 "extern const void *const __offload_image_intelmic_start;\n"
 "extern const void *const __offload_image_intelmic_end;\n\n"

-"static const void *const __offload_target_data[] = {\n"
+"static const struct intelmic_data {\n"
+"  uintptr_t omp_requires_mask;\n"
+"  const void *const image_start;\n"
+"  const void *const image_end;\n"
+"} intelmic_data = {\n"
+"  %d,\n"
 "  &__offload_image_intelmic_start, &__offload_image_intelmic_end\n"
-"};\n\n");
+"};\n\n", omp_requires);

fprintf (src_file,
 "#ifdef __cplusplus\n"
 "extern \"C\"\n"
 "#endif\n"
-"void GOMP_offload_register (const void *, int, const void *);\n"
+"void GOMP_offload_register_ver (unsigned, const void *, int, const void 
*);\n"

This line now ends in column 90, I think you want to wrap it.

 "#ifdef __cplusplus\n"
 "extern \"C\"\n"
 "#endif\n"
-"void GOMP_offload_unregister (const void *, int, const void *);\n\n"
+"void GOMP_offload_unregister_ver (unsigned, const void *, int, const void 
*);\n\n"

Likewise - before col 80, now col 94.


 "__attribute__((constructor))\n"
 "static void\n"
 "init (void)\n"
 "{\n"
-"  GOMP_offload_register (&__OFFLOAD_TABLE__, %d, 
__offload_target_data);\n"
-"}\n\n", GOMP_DEVICE_INTEL_MIC);
+"  GOMP_offload_register_ver (%#x, &__OFFLOAD_TABLE__, %d, 
_data);\n"

Likewise - albeit before already col 87, now col 89.

+"}\n\n",
+GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_INTEL_MIC),
+GOMP_DEVICE_INTEL_MIC);

fprintf (src_file,
 "__attribute__((destructor))\n"
 "static void\n"
 "fini (void)\n"
 "{\n"
-"  GOMP_offload_unregister (&__OFFLOAD_TABLE__, %d, 
__offload_target_data);\n"
-"}\n", GOMP_DEVICE_INTEL_MIC);

+"}\n",
+GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_INTEL_MIC),
+GOMP_DEVICE_INTEL_MIC);

fclose (src_file);

@@ -462,7 +473,7 @@ generate_host_descr_file (const char *host_compiler)
  }

  static const char *
-prepare_target_image (const char *target_compiler, int argc, char **argv)
+prepare_target_image (const char *target_compiler, int argc, char **argv, 
uint32_t *omp_requires)

Likewise now too long.

  {
const char *target_descr_filename
  = generate_target_descr_file (target_compiler);
@@ -509,8 +520,26 @@ prepare_target_image (const char *target_compiler, int 
argc, char **argv)
obstack_ptr_grow (_obstack, "");
obstack_ptr_grow (_obstack, "-o");
obstack_ptr_grow (_obstack, target_so_filename);
+
+  char *omp_requires_file;
+  if (save_temps)
+omp_requires_file = concat (dumppfx, ".mkoffload.omp_requires", NULL);
+  else
+omp_requires_file = make_temp_file (".mkoffload.omp_requires");
+  xputenv (concat ("GCC_OFFLOAD_OMP_REQUIRES_FILE=", 

Re: Restore 'GOMP_offload_unregister_ver' functionality (was: [Patch][v5] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Tobias Burnus

Hi Thomas, hello all,

On 06.07.22 12:42, Thomas Schwinge wrote:

On 2022-07-01T15:06:05+0200, Tobias Burnus 
wrote:

Attached is the updated patch. Main changes: [...]

This is now a great implementation of cross-component communication
(host/offloading compilers, runtime), thanks!  I'm sure this will be
usable (or at least instructing) for further purposes, too.

I also see potential use for other tools.

- Uses GOMP_register_var to pass the mask to libgomp

Like 'GOMP_offload_register_ver', also 'GOMP_offload_unregister_ver'
needs to be adjusted correspondingly.

Ups! Thanks for catching it and the patch.

OK to push the attached


Fixing the data handling in GOMP_offload_unregister_ver,
i.e. the 'target_data = &((void **) data)[1];' bit,
is obvious and a good bug fix.

Regarding the renaming of (..._)'target_data' to (..._)'data',
I do not have a real opinion as I also regard the requires part
as being part of the target(-related) data. Thus, I don't think
it harms but I also do not think that there is a large benefit.

Regarding the assert ... (continued below)


  gcc/
  * config/gcn/mkoffload.cc (process_obj): Clarify 'target_data' ->
  '[...]_data'.
  * config/nvptx/mkoffload.cc (process): Likewise.
  libgomp/
  * target.c (GOMP_offload_register_ver): Clarify 'target_data' ->
  'data'.
  (GOMP_offload_unregister_ver): Likewise.  Fix up 'target_data',
  and add 'assert'.

...

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index b8b3fecfcb4..d2464332275 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -692,13 +692,13 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
 len);

fprintf (cfile,
-"static const struct gcn_image_desc {\n"
+"static const struct gcn_data {\n"
 "  uintptr_t omp_requires_mask;\n"
 "  const struct gcn_image *gcn_image;\n"
 "  unsigned kernel_count;\n"
 "  const struct hsa_kernel_description *kernel_infos;\n"
 "  unsigned global_variable_count;\n"
-"} target_data = {\n"
+"} gcn_data = {\n"
 "  %d,\n"
 "  _image,\n"
 "  sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n"
@@ -723,7 +723,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
fprintf (cfile, "static __attribute__((constructor)) void init (void)\n"
 "{\n"
 "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-" %d/*GCN*/, _data);\n"
+" %d/*GCN*/, _data);\n"
 "};\n",
 GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 GOMP_DEVICE_GCN);
@@ -731,7 +731,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
fprintf (cfile, "static __attribute__((destructor)) void fini (void)\n"
 "{\n"
 "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-" %d/*GCN*/, _data);\n"
+" %d/*GCN*/, _data);\n"
 "};\n",
 GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 GOMP_DEVICE_GCN);
diff --git a/gcc/config/nvptx/mkoffload.cc b/gcc/config/nvptx/mkoffload.cc
index d8c81eb0547..0fa5f4423bf 100644
--- a/gcc/config/nvptx/mkoffload.cc
+++ b/gcc/config/nvptx/mkoffload.cc
@@ -310,7 +310,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
fprintf (out, "\n};\n\n");

fprintf (out,
-"static const struct nvptx_tdata {\n"
+"static const struct nvptx_data {\n"
 "  uintptr_t omp_requires_mask;\n"
 "  const struct ptx_obj *ptx_objs;\n"
 "  unsigned ptx_num;\n"
@@ -318,7 +318,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
 "  unsigned var_num;\n"
 "  const struct nvptx_fn *fn_names;\n"
 "  unsigned fn_num;\n"
-"} target_data = {\n"
+"} nvptx_data = {\n"
 "  %d, ptx_objs, sizeof (ptx_objs) / sizeof (ptx_objs[0]),\n"
 "  var_mappings,"
 "  sizeof (var_mappings) / sizeof (var_mappings[0]),\n"
@@ -344,7 +344,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 "{\n"
 "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-" %d/*NVIDIA_PTX*/, _data);\n"
+" %d/*NVIDIA_PTX*/, _data);\n"
 "};\n",
 GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 GOMP_DEVICE_NVIDIA_PTX);
@@ -352,7 +352,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 "{\n"
 "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-" %d/*NVIDIA_PTX*/, _data);\n"
+" %d/*NVIDIA_PTX*/, _data);\n"
 "};\n",
 GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 GOMP_DEVICE_NVIDIA_PTX);
diff --git a/libgomp/target.c b/libgomp/target.c
index 4dac81862d7..288b748b9e8 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -2334,23 +2334,29 @@ 

Re: Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a single place (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Tobias Burnus

Hi Thomas,

On 06.07.22 12:30, Thomas Schwinge wrote:

On 2022-06-08T05:56:02+0200, Tobias Burnus 
wrote:

[..]

To make things more failure proof, OK to push the attached
"Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a single place"?


I concur that it makes sense to avoid having multiple definitions,
especially as I can imagine that we might want to use this flag to
pass other options to the library - e.g. some of the command-line
pinning flags, proposed (and used in the OG11/OG12 branch).

Thus, the following is ok/makes sense from my point of view.


diff --git a/gcc/omp-general.h b/gcc/omp-general.h
index 7a94831e8f5..74e90e1a71a 100644
--- a/gcc/omp-general.h
+++ b/gcc/omp-general.h
@@ -126,12 +126,12 @@ extern int oacc_get_ifn_dim_arg (const gimple *stmt);

  enum omp_requires {
OMP_REQUIRES_ATOMIC_DEFAULT_MEM_ORDER = 0xf,
-  OMP_REQUIRES_UNIFIED_ADDRESS = 0x10,
-  OMP_REQUIRES_UNIFIED_SHARED_MEMORY = 0x20,
+  OMP_REQUIRES_UNIFIED_ADDRESS = GOMP_REQUIRES_UNIFIED_ADDRESS,
+  OMP_REQUIRES_UNIFIED_SHARED_MEMORY = GOMP_REQUIRES_UNIFIED_SHARED_MEMORY,
OMP_REQUIRES_DYNAMIC_ALLOCATORS = 0x40,
-  OMP_REQUIRES_REVERSE_OFFLOAD = 0x80,
+  OMP_REQUIRES_REVERSE_OFFLOAD = GOMP_REQUIRES_REVERSE_OFFLOAD,
OMP_REQUIRES_ATOMIC_DEFAULT_MEM_ORDER_USED = 0x100,
-  OMP_REQUIRES_TARGET_USED = 0x200
+  OMP_REQUIRES_TARGET_USED = GOMP_REQUIRES_TARGET_USED,
  };

  extern GTY(()) enum omp_requires omp_requires_mask;
diff --git a/include/gomp-constants.h b/include/gomp-constants.h
index 3e3078f082e..84316f953d0 100644
--- a/include/gomp-constants.h
+++ b/include/gomp-constants.h
@@ -341,8 +341,7 @@ enum gomp_map_kind
  #define GOMP_DEPEND_MUTEXINOUTSET   4
  #define GOMP_DEPEND_INOUTSET5

-/* Flag values for requires-directive features, must match corresponding
-   OMP_REQUIRES_* values in gcc/omp-general.h.  */
+/* Flag values for OpenMP 'requires' directive features.  */
  #define GOMP_REQUIRES_UNIFIED_ADDRESS   0x10
  #define GOMP_REQUIRES_UNIFIED_SHARED_MEMORY 0x20
  #define GOMP_REQUIRES_REVERSE_OFFLOAD   0x80

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


[Ada] Support ghost generic formal parameters

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
This adds support in GNAT for ghost generic formal parameters, as
included in SPARK RM 6.9.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* ghost.adb (Check_Ghost_Context): Delay checking for generic
associations.
(Check_Ghost_Context_In_Generic_Association): Perform ghost
checking in analyzed generic associations.
(Check_Ghost_Formal_Procedure_Or_Package): Check SPARK RM
6.9(13-14) for formal procedures and packages.
(Check_Ghost_Formal_Variable): Check SPARK RM 6.9(13-14) for
variables.
* ghost.ads: Declarations for the above.
* sem_ch12.adb (Analyze_Associations): Apply delayed checking
for generic associations.
(Analyze_Formal_Object_Declaration): Same.
(Analyze_Formal_Subprogram_Declaration): Same.
(Instantiate_Formal_Package): Same.
(Instantiate_Formal_Subprogram): Same.
(Instantiate_Object): Same.  Copy ghost aspect to newly declared
object for actual for IN formal object. Use new function
Get_Enclosing_Deep_Object to retrieve root object.
(Instantiate_Type): Copy ghost aspect to declared subtype for
actual for formal type.
* sem_prag.adb (Analyze_Pragma): Recognize new allowed
declarations.
* sem_util.adb (Copy_Ghost_Aspect): Copy the ghost aspect
between nodes.
(Get_Enclosing_Deep_Object): New function to return enclosing
deep object (or root for reachable part).
* sem_util.ads (Copy_Ghost_Aspect): Same.
(Get_Enclosing_Deep_Object): Same.
* libgnat/s-imageu.ads: Declare formal subprograms as ghost.
* libgnat/s-valuei.ads: Same.
* libgnat/s-valuti.ads: Same.diff --git a/gcc/ada/ghost.adb b/gcc/ada/ghost.adb
--- a/gcc/ada/ghost.adb
+++ b/gcc/ada/ghost.adb
@@ -472,6 +472,13 @@ package body Ghost is
if Is_Ignored_Ghost_Node (Par) then
   return True;
 
+   --  It is not possible to check correct use of Ghost entities
+   --  in generic instantiations until after the generic has been
+   --  resolved. Postpone that verification to after resolution.
+
+   elsif Nkind (Par) = N_Generic_Association then
+  return True;
+
--  A reference to a Ghost entity can appear within an aspect
--  specification (SPARK RM 6.9(10)). The precise checking will
--  occur when analyzing the corresponding pragma. We make an
@@ -521,19 +528,6 @@ package body Ghost is
then
   return True;
 
-   --  In the context of an instantiation, accept currently Ghost
-   --  arguments for formal subprograms, as SPARK does not provide
-   --  a way to distinguish Ghost formal parameters from non-Ghost
-   --  ones. Illegal use of such arguments in a non-Ghost context
-   --  will lead to errors inside the instantiation.
-
-   elsif Nkind (Parent (Par)) = N_Generic_Association
- and then (Nkind (Par) in N_Has_Entity
-and then Present (Entity (Par))
-and then Is_Subprogram (Entity (Par)))
-   then
-  return True;
-
elsif Is_OK_Declaration (Par) then
   return True;
 
@@ -680,6 +674,128 @@ package body Ghost is
   end if;
end Check_Ghost_Context;
 
+   
+   -- Check_Ghost_Context_In_Generic_Association --
+   
+
+   procedure Check_Ghost_Context_In_Generic_Association
+ (Actual : Node_Id;
+  Formal : Entity_Id)
+   is
+  function Emit_Error_On_Ghost_Reference
+(N : Node_Id)
+ return Traverse_Result;
+  --  Determine wether N denotes a reference to a ghost entity, and if so
+  --  issue an error.
+
+  ---
+  -- Emit_Error_On_Ghost_Reference --
+  ---
+
+  function Emit_Error_On_Ghost_Reference
+(N : Node_Id)
+ return Traverse_Result
+  is
+  begin
+ if Is_Entity_Name (N)
+   and then Present (Entity (N))
+   and then Is_Ghost_Entity (Entity (N))
+ then
+Error_Msg_N ("ghost entity cannot appear in this context", N);
+Error_Msg_Sloc := Sloc (Formal);
+Error_Msg_NE ("\formal & was not declared as ghost #", N, Formal);
+return Abandon;
+ end if;
+
+ return OK;
+  end Emit_Error_On_Ghost_Reference;
+
+  procedure Check_Ghost_References is
+new Traverse_Proc (Emit_Error_On_Ghost_Reference);
+
+   --  Start of processing for Check_Ghost_Context_In_Generic_Association
+
+   begin
+  --  The context is ghost when it appears within a Ghost package or
+  --  

[Ada] Simplify regular expression that matches 8 consecutive digits

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
Makefile cleanup; behaviour is unaffected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* gcc-interface/Make-lang.in (ada/generated/gnatvsn.ads):
Simplify regular expression. The "interval expression",
i.e. \{8\} is part of the POSIX regular expressions, so it
should not be a problem for modern implementations of sed.diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in
--- a/gcc/ada/gcc-interface/Make-lang.in
+++ b/gcc/ada/gcc-interface/Make-lang.in
@@ -1158,7 +1158,7 @@ ada/generated/gnatvsn.ads: ada/gnatvsn.ads BASE-VER ada/GNAT_DATE
 	s=`cat $(srcdir)/BASE-VER | sed -e "s/\([0-9]*\)\.\([0-9]*\)\..*/-\1\2/g"`; \
 	d=`if test -f $(srcdir)/ada/GNAT_DATE; then \
cat $(srcdir)/ada/GNAT_DATE; else date +%Y%m%d; fi`; \
-	cat $< | sed -e "/Version/s/(\([0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]\).*)/($$d$$s)/g" >$@
+	cat $< | sed -e "/Version/s/(\([0-9]\{8\}\).*)/($$d$$s)/g" >$@
 
 ada/gnatvsn.o : ada/gnatvsn.adb ada/generated/gnatvsn.ads
 	$(CC) -c $(ALL_ADAFLAGS) $(ADA_INCLUDES) $< $(ADA_OUTPUT_OPTION)




[Ada] Deferred constant considered as not preelaborable

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
Fix detection of non-preelaborable constructs for checking SPARK
elaboration rules, which was tagging deferred constant declarations as
not preelaborable.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_util.adb (Is_Non_Preelaborable_Construct): Fix for
deferred constants.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -18952,8 +18952,9 @@ package body Sem_Util is
if Has_Init_Expression (Nod) then
   Visit (Expression (Nod));
 
-   elsif not Has_Preelaborable_Initialization
-   (Etype (Defining_Entity (Nod)))
+   elsif not Constant_Present (Nod)
+ and then not Has_Preelaborable_Initialization
+(Etype (Defining_Entity (Nod)))
then
   raise Non_Preelaborable;
end if;




[Ada] Indexing error when calling GNAT.Regpat.Match

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an error in the compiler whereby a buffer sizing
error fails to get raised when compiling a regex expression with an
insufficiently sized Pattern_Matcher as the documentation indicated.
This, in turn, could lead to indexing errors when attempting to call
Match with the malformed regex program buffer.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/s-regpat.adb, libgnat/s-regpat.ads (Compile): Add a
new defaulted parameter Error_When_Too_Small to trigger an
error, if specified true, when Matcher is too small to hold the
compiled regex program.diff --git a/gcc/ada/libgnat/s-regpat.adb b/gcc/ada/libgnat/s-regpat.adb
--- a/gcc/ada/libgnat/s-regpat.adb
+++ b/gcc/ada/libgnat/s-regpat.adb
@@ -359,10 +359,11 @@ package body System.Regpat is
-
 
procedure Compile
- (Matcher : out Pattern_Matcher;
-  Expression  : String;
-  Final_Code_Size : out Program_Size;
-  Flags   : Regexp_Flags := No_Flags)
+ (Matcher  : out Pattern_Matcher;
+  Expression   : String;
+  Final_Code_Size  : out Program_Size;
+  Flags: Regexp_Flags := No_Flags;
+  Error_When_Too_Small : Boolean := True)
is
   --  We can't allocate space until we know how big the compiled form
   --  will be, but we can't compile it (and thus know how big it is)
@@ -1994,6 +1995,12 @@ package body System.Regpat is
   end if;
 
   PM.Flags := Flags;
+
+  --  Raise the appropriate error when Matcher does not have enough space
+
+  if Error_When_Too_Small and then Matcher.Size < Final_Code_Size then
+ raise Expression_Error with "Pattern_Matcher is too small";
+  end if;
end Compile;
 
function Compile
@@ -2009,7 +2016,7 @@ package body System.Regpat is
   Size  : Program_Size;
 
begin
-  Compile (Dummy, Expression, Size, Flags);
+  Compile (Dummy, Expression, Size, Flags, Error_When_Too_Small => False);
 
   if Size <= Dummy.Size then
  return Pattern_Matcher'
@@ -2023,17 +2030,13 @@ package body System.Regpat is
 Program  =>
   Dummy.Program
 (Dummy.Program'First .. Dummy.Program'First + Size - 1));
-  else
- --  We have to recompile now that we know the size
- --  ??? Can we use Ada 2005's return construct ?
-
- declare
-Result : Pattern_Matcher (Size);
- begin
-Compile (Result, Expression, Size, Flags);
-return Result;
- end;
   end if;
+
+  return
+ Result : Pattern_Matcher (Size)
+  do
+ Compile (Result, Expression, Size, Flags);
+  end return;
end Compile;
 
procedure Compile


diff --git a/gcc/ada/libgnat/s-regpat.ads b/gcc/ada/libgnat/s-regpat.ads
--- a/gcc/ada/libgnat/s-regpat.ads
+++ b/gcc/ada/libgnat/s-regpat.ads
@@ -403,10 +403,11 @@ package System.Regpat is
--  (e.g. case sensitivity,...).
 
procedure Compile
- (Matcher : out Pattern_Matcher;
-  Expression  : String;
-  Final_Code_Size : out Program_Size;
-  Flags   : Regexp_Flags := No_Flags);
+ (Matcher  : out Pattern_Matcher;
+  Expression   : String;
+  Final_Code_Size  : out Program_Size;
+  Flags: Regexp_Flags := No_Flags;
+  Error_When_Too_Small : Boolean := True);
--  Compile a regular expression into internal code
 
--  This procedure is significantly faster than the Compile function since
@@ -426,7 +427,25 @@ package System.Regpat is
--  expression.
--
--  This function raises Storage_Error if Matcher is too small to hold
-   --  the resulting code (i.e. Matcher.Size has too small a value).
+   --  the resulting code (i.e. Matcher.Size has too small a value) only when
+   --  the paramter Error_When_Too_Small is set to True. Otherwise, no error
+   --  will be raised and the required size will be placed in the
+   --  Final_Code_Size parameter.
+   --
+   --  Thus when Error_When_Too_Small is specified as false a check will need
+   --  to be made to ensure successful compilation - as in:
+   --
+   -- ...
+   -- Compile
+   --   (Matcher, Expr, Code_Size, Flags, Error_When_Too_Small => False);
+   --
+   -- if Matcher.Size < Code_Size then
+   --declare
+   --   New_Matcher : Pattern_Matcher (1..Code_Size);
+   --begin
+   --   Compile (New_Matcher, Expr, Code_Size, Flags);
+   --end;
+   -- end if;
--
--  Expression_Error is raised if the string Expression does not contain
--  a valid regular expression.




[Ada] Spurious non-callable warning on prefixed call in class condition

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an error in the compiler whereby a function call in
prefix notation within a class condition causes a spurious error
claiming the name in the call is a non-callable entity when there exists
a type extension in the same unit extended with a component featuring
the same name as the function in question.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_ch4.adb (Analyze_Selected_Component): Add condition to
avoid interpreting derived type components as candidates for
selected components in preanalysis of inherited class
conditions.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb
--- a/gcc/ada/sem_ch4.adb
+++ b/gcc/ada/sem_ch4.adb
@@ -5158,11 +5158,26 @@ package body Sem_Ch4 is
 
   elsif Is_Record_Type (Prefix_Type) then
 
- --  Find component with given name. In an instance, if the node is
- --  known as a prefixed call, do not examine components whose
- --  visibility may be accidental.
+ --  Find a component with the given name. If the node is a prefixed
+ --  call, do not examine components whose visibility may be
+ --  accidental.
 
- while Present (Comp) and then not Is_Prefixed_Call (N) loop
+ while Present (Comp)
+   and then not Is_Prefixed_Call (N)
+
+   --  When the selector has been resolved to a function then we may be
+   --  looking at a prefixed call which has been preanalyzed already as
+   --  part of a class condition. In such cases it is possible for a
+   --  derived type to declare a component which has the same name as
+   --  a primitive used in a parent's class condition.
+
+   --  Avoid seeing components as possible interpretations of the
+   --  selected component when this is true.
+
+   and then not (Inside_Class_Condition_Preanalysis
+  and then Present (Entity (Sel))
+  and then Ekind (Entity (Sel)) = E_Function)
+ loop
 if Chars (Comp) = Chars (Sel)
   and then Is_Visible_Component (Comp, N)
 then




[Ada] Improve code generated for aggregates of VFA type

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
This avoids using a full access for constants internally generated from
assignments of aggregates with a Volatile_Full_Access type.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* gcc-interface/gigi.h (simple_constant_p): Declare.
* gcc-interface/decl.cc (gnat_to_gnu_entity) : Strip
the qualifiers from the type of a simple constant.
(simple_constant_p): New predicate.
* gcc-interface/trans.cc (node_is_atomic): Return true for objects
with atomic type except for simple constants.
(node_is_volatile_full_access): Return false for simple constants
with VFA type.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -660,8 +660,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	 like variables.  */
   if (definition
 	  && !gnu_expr
-	  && No (Address_Clause (gnat_entity))
 	  && !No_Initialization (gnat_decl)
+	  && No (Address_Clause (gnat_entity))
 	  && No (gnat_renamed_obj))
 	{
 	  gnu_decl = error_mark_node;
@@ -781,6 +781,11 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	if (kind == E_Loop_Parameter)
 	  gnu_type = get_base_type (gnu_type);
 
+	/* If this is a simple constant, strip the qualifiers from its type,
+	   since the constant represents only its value.  */
+	else if (simple_constant_p (gnat_entity))
+	  gnu_type = TYPE_MAIN_VARIANT (gnu_type);
+
 	/* Reject non-renamed objects whose type is an unconstrained array or
 	   any object whose type is a dummy type or void.  */
 	if ((TREE_CODE (gnu_type) == UNCONSTRAINED_ARRAY_TYPE
@@ -9541,6 +9546,19 @@ promote_object_alignment (tree gnu_type, tree gnu_size, Entity_Id gnat_entity)
   return align;
 }
 
+/* Return whether GNAT_ENTITY is a simple constant, i.e. it represents only
+   its value and reading it has no side effects.  */
+
+bool
+simple_constant_p (Entity_Id gnat_entity)
+{
+  return Ekind (gnat_entity) == E_Constant
+	 && Present (Constant_Value (gnat_entity))
+	 && !No_Initialization (gnat_entity)
+	 && No (Address_Clause (gnat_entity))
+	 && No (Renamed_Object (gnat_entity));
+}
+
 /* Verify that TYPE is something we can implement atomically.  If not, issue
an error for GNAT_ENTITY.  COMPONENT_P is true if we are being called to
process a component type.  */


diff --git a/gcc/ada/gcc-interface/gigi.h b/gcc/ada/gcc-interface/gigi.h
--- a/gcc/ada/gcc-interface/gigi.h
+++ b/gcc/ada/gcc-interface/gigi.h
@@ -998,6 +998,10 @@ extern Entity_Id get_debug_scope (Node_Id gnat_node, bool *is_subprogram);
should be synchronized with Exp_Dbug.Debug_Renaming_Declaration.  */
 extern bool can_materialize_object_renaming_p (Node_Id expr);
 
+/* Return whether GNAT_ENTITY is a simple constant, i.e. it represents only
+   its value and reading it has no side effects.  */
+extern bool simple_constant_p (Entity_Id gnat_entity);
+
 /* Return the size of TYPE, which must be a positive power of 2.  */
 extern unsigned int resolve_atomic_size (tree type);
 


diff --git a/gcc/ada/gcc-interface/trans.cc b/gcc/ada/gcc-interface/trans.cc
--- a/gcc/ada/gcc-interface/trans.cc
+++ b/gcc/ada/gcc-interface/trans.cc
@@ -4111,9 +4111,11 @@ node_is_atomic (Node_Id gnat_node)
 case N_Identifier:
 case N_Expanded_Name:
   gnat_entity = Entity (gnat_node);
-  if (Ekind (gnat_entity) != E_Variable)
+  if (!Is_Object (gnat_entity))
 	break;
-  return Is_Atomic (gnat_entity) || Is_Atomic (Etype (gnat_entity));
+  return Is_Atomic (gnat_entity)
+	 || (Is_Atomic (Etype (gnat_entity))
+		 && !simple_constant_p (gnat_entity));
 
 case N_Selected_Component:
   return Is_Atomic (Etype (gnat_node))
@@ -4152,7 +4154,8 @@ node_is_volatile_full_access (Node_Id gnat_node)
   if (!Is_Object (gnat_entity))
 	break;
   return Is_Volatile_Full_Access (gnat_entity)
-	 || Is_Volatile_Full_Access (Etype (gnat_entity));
+	 || (Is_Volatile_Full_Access (Etype (gnat_entity))
+		 && !simple_constant_p (gnat_entity));
 
 case N_Selected_Component:
   return Is_Volatile_Full_Access (Etype (gnat_node))




[Ada] Small tweak to gnat_to_gnu_subprog_type

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
No functional changes.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* gcc-interface/decl.cc (gnat_to_gnu_subprog_type): Constify a
local variable and move a couple of others around.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -5777,10 +5777,9 @@ gnat_to_gnu_subprog_type (Entity_Id gnat_subprog, bool definition,
 			  bool debug_info_p, tree *param_list)
 {
   const Entity_Kind kind = Ekind (gnat_subprog);
+  const Entity_Id gnat_return_type = Etype (gnat_subprog);
   const bool method_p = is_cplusplus_method (gnat_subprog);
   const bool variadic = IN (Convention (gnat_subprog), Convention_C_Variadic);
-  Entity_Id gnat_return_type = Etype (gnat_subprog);
-  Entity_Id gnat_param;
   tree gnu_type = present_gnu_tree (gnat_subprog)
 		  ? TREE_TYPE (get_gnu_tree (gnat_subprog)) : NULL_TREE;
   tree gnu_return_type;
@@ -5810,7 +5809,6 @@ gnat_to_gnu_subprog_type (Entity_Id gnat_subprog, bool definition,
   bool return_by_direct_ref_p = false;
   bool return_by_invisi_ref_p = false;
   bool incomplete_profile_p = false;
-  int num;
 
   /* Look into the return type and get its associated GCC tree if it is not
  void, and then compute various flags for the subprogram type.  But make
@@ -5944,6 +5942,8 @@ gnat_to_gnu_subprog_type (Entity_Id gnat_subprog, bool definition,
 
   /* Loop over the parameters and get their associated GCC tree.  While doing
  this, build a copy-in copy-out structure if we need one.  */
+  Entity_Id gnat_param;
+  int num;
   for (gnat_param = First_Formal_With_Extras (gnat_subprog), num = 0;
Present (gnat_param);
gnat_param = Next_Formal_With_Extras (gnat_param), num++)




[Ada] Handle secondary stack memory allocations alignment

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
To accomodate cases where objects allocated on the secondary stack
needed a more constrained alignement than Standard'Maximum_Alignement,
the alignment for all allocations in the full runtime were forced on to
be aligned on Standard'Maximum_Alignement*2. This changes removes this
workaround and correctly handles the over-alignment in all runtimes.

This change modifies the SS_Allocate procedure to accept a new Alignment
parameter and to dynamically realign the pointer returned by the memory
allocation (Allocate_* functions or dedicated stack allocations for
zfp/cert).

It also simplifies the 0-sized allocations by not allocating any memory
if pointer is already correctly aligned (already the case in cert and
zfp runtimes).

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/s-secsta.ads (SS_Allocate): Add new Alignment
parameter.
(Memory_Alignment): Remove.
* libgnat/s-secsta.adb (Align_Addr): New.
(SS_Allocate): Add new Alignment parameter. Realign pointer if
needed. Don't allocate anything for 0-sized allocations.
* gcc-interface/utils2.cc (build_call_alloc_dealloc_proc): Add
allocated object's alignment as last parameter to allocation
invocation.diff --git a/gcc/ada/gcc-interface/utils2.cc b/gcc/ada/gcc-interface/utils2.cc
--- a/gcc/ada/gcc-interface/utils2.cc
+++ b/gcc/ada/gcc-interface/utils2.cc
@@ -2139,6 +2139,8 @@ build_call_alloc_dealloc_proc (tree gnu_obj, tree gnu_size, tree gnu_type,
 			   Entity_Id gnat_proc, Entity_Id gnat_pool)
 {
   tree gnu_proc = gnat_to_gnu (gnat_proc);
+  tree gnu_align = size_int (TYPE_ALIGN (gnu_type) / BITS_PER_UNIT);
+
   tree gnu_call;
 
   /* A storage pool's underlying type is a record type for both predefined
@@ -2154,7 +2156,6 @@ build_call_alloc_dealloc_proc (tree gnu_obj, tree gnu_size, tree gnu_type,
 
   tree gnu_pool = gnat_to_gnu (gnat_pool);
   tree gnu_pool_addr = build_unary_op (ADDR_EXPR, NULL_TREE, gnu_pool);
-  tree gnu_align = size_int (TYPE_ALIGN (gnu_type) / BITS_PER_UNIT);
 
   gnu_size = convert (gnu_size_type, gnu_size);
   gnu_align = convert (gnu_size_type, gnu_align);
@@ -2178,6 +2179,7 @@ build_call_alloc_dealloc_proc (tree gnu_obj, tree gnu_size, tree gnu_type,
   tree gnu_size_type = gnat_to_gnu_type (gnat_size_type);
 
   gnu_size = convert (gnu_size_type, gnu_size);
+  gnu_align = convert (gnu_size_type, gnu_align);
 
   if (DECL_BUILT_IN_CLASS (gnu_proc) == BUILT_IN_FRONTEND
 	  && DECL_FE_FUNCTION_CODE (gnu_proc) == BUILT_IN_RETURN_SLOT)
@@ -2191,7 +2193,7 @@ build_call_alloc_dealloc_proc (tree gnu_obj, tree gnu_size, tree gnu_type,
 
 	  gnu_call = DECL_RESULT (current_function_decl);
 
-	  /* The allocation has alreay been done by the caller so we check that
+	  /* The allocation has already been done by the caller so we check that
 	 we are not going to overflow the return slot.  */
 	  if (TYPE_CI_CO_LIST (TREE_TYPE (current_function_decl)))
 	gnu_ret_size
@@ -2216,7 +2218,7 @@ build_call_alloc_dealloc_proc (tree gnu_obj, tree gnu_size, tree gnu_type,
 	gnu_call = build_call_n_expr (gnu_proc, 2, gnu_obj, gnu_size);
 
   else
-	gnu_call = build_call_n_expr (gnu_proc, 1, gnu_size);
+	gnu_call = build_call_n_expr (gnu_proc, 2, gnu_size, gnu_align);
 }
 
   return gnu_call;
@@ -2334,7 +2336,7 @@ maybe_wrap_free (tree data_ptr, tree data_type)
 
 /* Build a GCC tree to call an allocation or deallocation function.
If GNU_OBJ is nonzero, it is an object to deallocate.  Otherwise,
-   generate an allocator.
+   generate an allocation.
 
GNU_SIZE is the number of bytes to allocate and GNU_TYPE is the contained
object type, used to determine the to-be-honored address alignment.


diff --git a/gcc/ada/libgnat/s-secsta.adb b/gcc/ada/libgnat/s-secsta.adb
--- a/gcc/ada/libgnat/s-secsta.adb
+++ b/gcc/ada/libgnat/s-secsta.adb
@@ -550,22 +550,52 @@ package body System.Secondary_Stack is
 
procedure SS_Allocate
  (Addr : out Address;
-  Storage_Size : Storage_Count)
+  Storage_Size : Storage_Count;
+  Alignment: SSE.Storage_Count := Standard'Maximum_Alignment)
is
+
   function Round_Up (Size : Storage_Count) return Memory_Size;
   pragma Inline (Round_Up);
   --  Round Size up to the nearest multiple of the maximum alignment
 
+  function Align_Addr (Addr : Address) return Address;
+  pragma Inline (Align_Addr);
+  --  Align Addr to the next multiple of Alignment
+
+  
+  -- Align_Addr --
+  
+
+  function Align_Addr (Addr : Address) return Address is
+ Int_Algn : constant Integer_Address := Integer_Address (Alignment);
+ Int_Addr : constant Integer_Address := To_Integer (Addr);
+  begin
+
+ --  L : Alignment
+ --  A : Standard'Maximum_Alignment
+
+ --   Addr
+ --  L | L   L
+ --  

[Ada] Missing error on tagged type conversion

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
The compiler does not report an error on a type conversion to/from a
tagged type whose parent type is an interface type and there is no
relationship between the source and target types. This bug has been
dormant since January/2016.

This patch also improves the text of errors reported on interface type
conversions suggesting how to fix these errors.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_res.adb (Resolve_Type_Conversion): Code cleanup since the
previous static check has been moved to Valid_Tagged_Conversion.
(Valid_Tagged_Conversion): Fix the code checking conversion
to/from interface types since incorrectly returns True when the
parent type of the operand type (or the target type) is an
interface type; add missing static checks on interface type
conversions.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -31,6 +31,7 @@ with Debug_A;use Debug_A;
 with Einfo;  use Einfo;
 with Einfo.Entities; use Einfo.Entities;
 with Einfo.Utils;use Einfo.Utils;
+with Elists; use Elists;
 with Errout; use Errout;
 with Expander;   use Expander;
 with Exp_Ch6;use Exp_Ch6;
@@ -12308,26 +12309,7 @@ package body Sem_Res is
 --  Conversion to interface type
 
 elsif Is_Interface (Target) then
-
-   --  Handle subtypes
-
-   if Ekind (Opnd) in E_Protected_Subtype | E_Task_Subtype then
-  Opnd := Etype (Opnd);
-   end if;
-
-   if Is_Class_Wide_Type (Opnd)
- or else Interface_Present_In_Ancestor
-   (Typ   => Opnd,
-Iface => Target)
-   then
-  Expand_Interface_Conversion (N);
-   else
-  Error_Msg_Name_1 := Chars (Etype (Target));
-  Error_Msg_Name_2 := Chars (Opnd);
-  Error_Msg_N
-("wrong interface conversion (% is not a progenitor "
- & "of %)", N);
-   end if;
+   Expand_Interface_Conversion (N);
 end if;
  end;
   end if;
@@ -13621,29 +13603,115 @@ package body Sem_Res is
   Conversion_Check (False,
 "downward conversion of tagged objects not allowed");
 
- --  Ada 2005 (AI-251): The conversion to/from interface types is
- --  always valid. The types involved may be class-wide (sub)types.
+ --  Ada 2005 (AI-251): A conversion is valid if the operand and target
+ --  types are both class-wide types and the specific type associated
+ --  with at least one of them is an interface type (RM 4.6 (23.1/2));
+ --  at run-time a check will verify the validity of this interface
+ --  type conversion.
 
- elsif Is_Interface (Etype (Base_Type (Target_Type)))
-   or else Is_Interface (Etype (Base_Type (Opnd_Type)))
+ elsif Is_Class_Wide_Type (Target_Type)
+and then Is_Class_Wide_Type (Opnd_Type)
+and then (Is_Interface (Target_Type)
+or else Is_Interface (Opnd_Type))
  then
 return True;
 
- --  If the operand is a class-wide type obtained through a limited_
- --  with clause, and the context includes the nonlimited view, use
- --  it to determine whether the conversion is legal.
+ --  Report errors
+
+ elsif Is_Class_Wide_Type (Target_Type)
+   and then Is_Interface (Target_Type)
+   and then not Is_Interface (Opnd_Type)
+   and then not Interface_Present_In_Ancestor
+  (Typ   => Opnd_Type,
+   Iface => Target_Type)
+ then
+Error_Msg_Name_1 := Chars (Etype (Target_Type));
+Error_Msg_Name_2 := Chars (Opnd_Type);
+Conversion_Error_N
+  ("wrong interface conversion (% is not a progenitor "
+   & "of %)", N);
+return False;
 
  elsif Is_Class_Wide_Type (Opnd_Type)
-   and then From_Limited_With (Opnd_Type)
-   and then Present (Non_Limited_View (Etype (Opnd_Type)))
-   and then Is_Interface (Non_Limited_View (Etype (Opnd_Type)))
+   and then Is_Interface (Opnd_Type)
+   and then not Is_Interface (Target_Type)
+   and then not Interface_Present_In_Ancestor
+  (Typ   => Target_Type,
+   Iface => Opnd_Type)
  then
-return True;
+Error_Msg_Name_1 := Chars (Etype (Opnd_Type));
+Error_Msg_Name_2 := Chars (Target_Type);
+Conversion_Error_N
+  ("wrong interface conversion (% is not a progenitor "
+   & "of %)", N);
 
- elsif Is_Access_Type (Opnd_Type)
-   and then 

[Ada] Fix spurious error for aggregate with box component choice

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
It comes from the Volatile_Full_Access (or Atomic) aspect: the aggregate is
effectively analyzed/resolved twice and this does not work.  It is fixed by
calling Is_Full_Access_Aggregate before resolution.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_aggr.adb (Expand_Record_Aggregate): Do not call
Is_Full_Access_Aggregate here.
* freeze.ads (Is_Full_Access_Aggregate): Delete.
* freeze.adb (Is_Full_Access_Aggregate): Move to...
(Freeze_Entity): Do not call Is_Full_Access_Aggregate here.
* sem_aggr.adb (Is_Full_Access_Aggregate): ...here
(Resolve_Aggregate): Call Is_Full_Access_Aggregate here.diff --git a/gcc/ada/exp_aggr.adb b/gcc/ada/exp_aggr.adb
--- a/gcc/ada/exp_aggr.adb
+++ b/gcc/ada/exp_aggr.adb
@@ -8779,19 +8779,10 @@ package body Exp_Aggr is
--  Start of processing for Expand_Record_Aggregate
 
begin
-  --  If the aggregate is to be assigned to a full access variable, we have
-  --  to prevent a piecemeal assignment even if the aggregate is to be
-  --  expanded. We create a temporary for the aggregate, and assign the
-  --  temporary instead, so that the back end can generate an atomic move
-  --  for it.
-
-  if Is_Full_Access_Aggregate (N) then
- return;
-
   --  No special management required for aggregates used to initialize
   --  statically allocated dispatch tables
 
-  elsif Is_Static_Dispatch_Table_Aggregate (N) then
+  if Is_Static_Dispatch_Table_Aggregate (N) then
  return;
 
   --  Case pattern aggregates need to remain as aggregates


diff --git a/gcc/ada/freeze.adb b/gcc/ada/freeze.adb
--- a/gcc/ada/freeze.adb
+++ b/gcc/ada/freeze.adb
@@ -2309,67 +2309,6 @@ package body Freeze is
   end loop;
end Check_Unsigned_Type;
 
-   --
-   -- Is_Full_Access_Aggregate --
-   --
-
-   function Is_Full_Access_Aggregate (N : Node_Id) return Boolean is
-  Loc   : constant Source_Ptr := Sloc (N);
-  New_N : Node_Id;
-  Par   : Node_Id;
-  Temp  : Entity_Id;
-  Typ   : Entity_Id;
-
-   begin
-  Par := Parent (N);
-
-  --  Array may be qualified, so find outer context
-
-  if Nkind (Par) = N_Qualified_Expression then
- Par := Parent (Par);
-  end if;
-
-  if not Comes_From_Source (Par) then
- return False;
-  end if;
-
-  case Nkind (Par) is
- when N_Assignment_Statement =>
-Typ := Etype (Name (Par));
-
-if not Is_Full_Access (Typ)
-  and then not Is_Full_Access_Object (Name (Par))
-then
-   return False;
-end if;
-
- when N_Object_Declaration =>
-Typ := Etype (Defining_Identifier (Par));
-
-if not Is_Full_Access (Typ)
-  and then not Is_Full_Access (Defining_Identifier (Par))
-then
-   return False;
-end if;
-
- when others =>
-return False;
-  end case;
-
-  Temp := Make_Temporary (Loc, 'T', N);
-  New_N :=
-Make_Object_Declaration (Loc,
-  Defining_Identifier => Temp,
-  Constant_Present=> True,
-  Object_Definition   => New_Occurrence_Of (Typ, Loc),
-  Expression  => Relocate_Node (N));
-  Insert_Before (Par, New_N);
-  Analyze (New_N);
-
-  Set_Expression (Par, New_Occurrence_Of (Temp, Loc));
-  return True;
-   end Is_Full_Access_Aggregate;
-
---
-- Explode_Initialization_Compound_Statement --
---
@@ -6447,20 +6386,6 @@ package body Freeze is
  then
 Set_Encoded_Interface_Name
   (E, Get_Default_External_Name (E));
-
- --  If entity is an atomic object appearing in a declaration and
- --  the expression is an aggregate, assign it to a temporary to
- --  ensure that the actual assignment is done atomically rather
- --  than component-wise (the assignment to the temp may be done
- --  component-wise, but that is harmless).
-
- elsif Is_Full_Access (E)
-   and then Nkind (Parent (E)) = N_Object_Declaration
-   and then Present (Expression (Parent (E)))
-   and then Nkind (Expression (Parent (E))) = N_Aggregate
-   and then Is_Full_Access_Aggregate (Expression (Parent (E)))
- then
-null;
  end if;
 
  --  Subprogram case


diff --git a/gcc/ada/freeze.ads b/gcc/ada/freeze.ads
--- a/gcc/ada/freeze.ads
+++ b/gcc/ada/freeze.ads
@@ -177,15 +177,6 @@ package Freeze is
--  True when we are processing the body of a primitive with no previous
--  spec defined after R is frozen (see Check_Dispatching_Operation).
 
-   function Is_Full_Access_Aggregate (N : Node_Id) return Boolean;
-   --  If a full access object is initialized with an 

[Ada] Remove old vxworks from Makefile.rtl - e500 port.

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
The powerpc e500 port has been LTS'd

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/system-vxworks7-e500-kernel.ads: Remove.
* libgnat/system-vxworks7-e500-rtp-smp.ads: Likewise.
* libgnat/system-vxworks7-e500-rtp.ads: Likewise.diff --git a/gcc/ada/libgnat/system-vxworks7-e500-kernel.ads /dev/null
deleted file mode 100644
--- a/gcc/ada/libgnat/system-vxworks7-e500-kernel.ads
+++ /dev/null
@@ -1,160 +0,0 @@
---
---  --
---GNAT RUN-TIME COMPONENTS  --
---  --
---   S Y S T E M--
---  --
--- S p e c  --
---  (VxWorks 7 Kernel Version E500) --
---  --
---  Copyright (C) 1992-2022, Free Software Foundation, Inc. --
---  --
--- This specification is derived from the Ada Reference Manual for use with --
--- GNAT. The copyright notice above, and the license provisions that follow --
--- apply solely to the  contents of the part following the private keyword. --
---  --
--- GNAT is free software;  you can  redistribute it  and/or modify it under --
--- terms of the  GNU General Public License as published  by the Free Soft- --
--- ware  Foundation;  either version 3,  or (at your option) any later ver- --
--- sion.  GNAT is distributed in the hope that it will be useful, but WITH- --
--- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
--- or FITNESS FOR A PARTICULAR PURPOSE. --
---  --
--- As a special exception under Section 7 of GPL version 3, you are granted --
--- additional permissions described in the GCC Runtime Library Exception,   --
--- version 3.1, as published by the Free Software Foundation.   --
---  --
--- You should have received a copy of the GNU General Public License and--
--- a copy of the GCC Runtime Library Exception along with this program; --
--- see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see--
--- .  --
---  --
--- GNAT was originally developed  by the GNAT team at  New York University. --
--- Extensive contributions were provided by Ada Core Technologies Inc.  --
---  --
---
-
-package System is
-   pragma Pure;
-   --  Note that we take advantage of the implementation permission to make
-   --  this unit Pure instead of Preelaborable; see RM 13.7.1(15). In Ada
-   --  2005, this is Pure in any case (AI-362).
-
-   pragma No_Elaboration_Code_All;
-   --  Allow the use of that restriction in units that WITH this unit
-
-   type Name is (SYSTEM_NAME_GNAT);
-   System_Name : constant Name := SYSTEM_NAME_GNAT;
-
-   --  System-Dependent Named Numbers
-
-   Min_Int : constant := -2 ** (Standard'Max_Integer_Size - 1);
-   Max_Int : constant :=  2 ** (Standard'Max_Integer_Size - 1) - 1;
-
-   Max_Binary_Modulus: constant := 2 ** Standard'Max_Integer_Size;
-   Max_Nonbinary_Modulus : constant := 2 ** Integer'Size - 1;
-
-   Max_Base_Digits   : constant := Long_Long_Float'Digits;
-   Max_Digits: constant := Long_Long_Float'Digits;
-
-   Max_Mantissa  : constant := Standard'Max_Integer_Size - 1;
-   Fine_Delta: constant := 2.0 ** (-Max_Mantissa);
-
-   Tick  : constant := 1.0 / 60.0;
-
-   --  Storage-related Declarations
-
-   type Address is private;
-   pragma Preelaborable_Initialization (Address);
-   Null_Address : constant Address;
-
-   Storage_Unit : constant := 8;
-   Word_Size: constant := Standard'Word_Size;
-   Memory_Size  : constant := 2 ** Word_Size;
-
-   --  Address comparison
-
-   function "<"  (Left, Right : Address) return Boolean;
-   function "<=" (Left, Right : Address) return Boolean;
-   function ">"  (Left, Right : Address) return Boolean;
-   function ">=" (Left, Right : Address) return Boolean;
-   function "="  (Left, Right : Address) return Boolean;
-
-   pragma Import 

[Ada] Cleanup use of local scalars in GNAT.Socket.Get_Address_Info

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
A cleanup opportunity spotted while working on improved detection of
uninitialised local scalar objects.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/g-socket.adb (Get_Address_Info): Reduce scope of the
Found variable; avoid repeated assignment inside the loop.diff --git a/gcc/ada/libgnat/g-socket.adb b/gcc/ada/libgnat/g-socket.adb
--- a/gcc/ada/libgnat/g-socket.adb
+++ b/gcc/ada/libgnat/g-socket.adb
@@ -1036,7 +1036,6 @@ package body GNAT.Sockets is
 
   R : C.int;
   Iter  : Addrinfo_Access;
-  Found : Boolean;
 
   function To_Array return Address_Info_Array;
   --  Convert taken from OS addrinfo list A into Address_Info_Array
@@ -1046,8 +1045,6 @@ package body GNAT.Sockets is
   --
 
   function To_Array return Address_Info_Array is
- Result : Address_Info_Array (1 .. 8);
-
  procedure Unsupported;
  --  Calls Unknown callback if defiend
 
@@ -1066,6 +1063,9 @@ package body GNAT.Sockets is
 end if;
  end Unsupported;
 
+ Found  : Boolean;
+ Result : Address_Info_Array (1 .. 8);
+
   --  Start of processing for To_Array
 
   begin
@@ -1087,8 +1087,8 @@ package body GNAT.Sockets is
if Result (J).Addr.Family = Family_Unspec then
   Unsupported;
else
+  Found := False;
   for M in Modes'Range loop
- Found := False;
  if Modes (M) = Iter.ai_socktype then
 Result (J).Mode := M;
 Found := True;




[Ada] Vxworks7* - Makefile.rtl rtp vs rtp-smp cleanup

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
Only smp runtimes are built for vxworks7*, even though the -smp suffix
is removed during install. Therefore, in general, the build macros for
the non-smp runtimes are superfluous except on the legacy ppc-vxworks6
target where both the smp and non-smp runtime are built.  Lastly, an
error message is added if a runtime build is commanded that doesn't
exist, rather then letting the build mysteriously fail.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* Makefile.rtl [arm,aarch64 vxworks7]: Remove rtp and kernel
build macros and set an error variable if needed.
[x86,x86_vxworks7]: Likewise.
[ppc,ppc64]: Set an error variable if needed.
(rts-err): New phony Makefile target.
(setup-rts): Depend on rts-err.diff --git a/gcc/ada/Makefile.rtl b/gcc/ada/Makefile.rtl
--- a/gcc/ada/Makefile.rtl
+++ b/gcc/ada/Makefile.rtl
@@ -1124,6 +1124,7 @@ ifeq ($(strip $(filter-out powerpc% wrs vxworks vxworks7%, $(target_cpu) $(targe
 
   EH_MECHANISM=-gcc
 
+  # The rtp and kernel sections must be retained for the sake of ppc-vx6
   ifeq ($(strip $(filter-out rtp,$(THREAD_KIND))),)
 LIBGNAT_TARGET_PAIRS += \
 s-vxwext.ads

[Ada] Restore accidentally removed part of a comment about unset references

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
Fix an unintentionally removed comment.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_res.adb (Resolve_Actuals): Restore first sentence of a
comment.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -4620,6 +4620,7 @@ package body Sem_Res is
  ("invalid use of untagged formal incomplete type", A);
 end if;
 
+--  For mode IN, if actual is an entity, and the type of the formal
 --  has warnings suppressed, then we reset Never_Set_In_Source for
 --  the calling entity. The reason for this is to catch cases like
 --  GNAT.Spitbol.Patterns.Vstring_Var where the called subprogram




[Ada] Fix incorrect itype sharing for case expression in limited type return

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
The compiler aborts with an internal error in gigi, but the problem is an
itype incorrectly shared between several branches of an if_statement that
has been created for a Build-In-Place return.

Three branches of this if_statement contain an allocator statement and
the latter two have been obtained as the result of calling New_Copy_Tree
on the first; now the initialization expression of the first had also been
obtained as the result of calling New_Copy_Tree on the original tree, and
these chained calls to New_Copy_Tree run afoul of an issue with the copy
of itypes after the rewrite of an aggregate as an expression with actions.

Fixing this issue looks quite delicate, so this fixes the incorrect sharing
by replacing the chained calls to New_Copy_Tree with repeated calls on the
original expression, which is more elegant in any case.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_ch3.adb (Make_Allocator_For_BIP_Return): New local function.
(Expand_N_Object_Declaration): Use it to build the three allocators
for a Build-In-Place return with an unconstrained type.  Update the
head comment after other recent changes.diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -7980,16 +7980,11 @@ package body Exp_Ch3 is
 --  the value one, then the caller has passed access to an
 --  existing object for use as the return object. If the value
 --  is two, then the return object must be allocated on the
---  secondary stack. Otherwise, the object must be allocated in
---  a storage pool. We generate an if statement to test the
---  implicit allocation formal and initialize a local access
---  value appropriately, creating allocators in the secondary
---  stack and global heap cases. The special formal also exists
---  and must be tested when the function has a tagged result,
---  even when the result subtype is constrained, because in
---  general such functions can be called in dispatching contexts
---  and must be handled similarly to functions with a class-wide
---  result.
+--  secondary stack. If the value is three, then the return
+--  object must be allocated on the heap. Otherwise, the object
+--  must be allocated in a storage pool. We generate an if
+--  statement to test the BIP_Alloc_Form formal and initialize
+--  a local access value appropriately.
 
 if Needs_BIP_Alloc_Form (Func_Id) then
declare
@@ -8005,6 +8000,73 @@ package body Exp_Ch3 is
   Pool_Id  : constant Entity_Id :=
 Make_Temporary (Loc, 'P');
 
+  function Make_Allocator_For_BIP_Return return Node_Id;
+  --  Make an allocator for the BIP return being processed
+
+  ---
+  -- Make_Allocator_For_BIP_Return --
+  ---
+
+  function Make_Allocator_For_BIP_Return return Node_Id is
+ Alloc : Node_Id;
+
+  begin
+ if Present (Expr_Q)
+   and then not Is_Delayed_Aggregate (Expr_Q)
+   and then not No_Initialization (N)
+ then
+--  Always use the type of the expression for the
+--  qualified expression, rather than the result type.
+--  In general we cannot always use the result type
+--  for the allocator, because the expression might be
+--  of a specific type, such as in the case of an
+--  aggregate or even a nonlimited object when the
+--  result type is a limited class-wide interface type.
+
+Alloc :=
+  Make_Allocator (Loc,
+Expression =>
+  Make_Qualified_Expression (Loc,
+Subtype_Mark =>
+  New_Occurrence_Of (Etype (Expr_Q), Loc),
+Expression   => New_Copy_Tree (Expr_Q)));
+
+ else
+--  If the function returns a class-wide type we cannot
+--  use the return type for the allocator. Instead we
+--  use the type of the expression, which must be an
+--  aggregate of a definite type.
+
+if Is_Class_Wide_Type (Ret_Obj_Typ) then
+   Alloc :=
+ Make_Allocator (Loc,
+   Expression =>
+

[Ada] Incorrect emptying of CUDA global subprograms

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an error in the compiler whereby no
Corresponding_Spec was set for emptied CUDA global subprograms - leading
to a malformed tree.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* gnat_cuda.adb (Empty_CUDA_Global_Subprogram): Set
Specification and Corresponding_Spec to match the original
Kernel_Body.diff --git a/gcc/ada/gnat_cuda.adb b/gcc/ada/gnat_cuda.adb
--- a/gcc/ada/gnat_cuda.adb
+++ b/gcc/ada/gnat_cuda.adb
@@ -165,17 +165,20 @@ package body GNAT_CUDA is
 
   Kernel_Elm := First_Elmt (Kernels);
   while Present (Kernel_Elm) loop
- Kernel := Node (Kernel_Elm);
+ Kernel  := Node (Kernel_Elm);
  Kernel_Body := Subprogram_Body (Kernel);
- Loc := Sloc (Kernel_Body);
+ Loc := Sloc (Kernel_Body);
 
  Null_Body := Make_Subprogram_Body (Loc,
-   Specification  => Subprogram_Specification (Kernel),
+   Specification  => Specification (Kernel_Body),
Declarations   => New_List,
Handled_Statement_Sequence =>
  Make_Handled_Sequence_Of_Statements (Loc,
Statements => New_List (Make_Null_Statement (Loc;
 
+ Set_Corresponding_Spec (Null_Body,
+   Corresponding_Spec (Kernel_Body));
+
  Rewrite (Kernel_Body, Null_Body);
 
  Next_Elmt (Kernel_Elm);




[Ada] Remove explicit call to Make_Unchecked_Type_Conversion

2022-07-06 Thread Pierre-Marie de Rodat via Gcc-patches
Respect a comment in sinfo.ads, which says: "Unchecked type conversion
nodes should be created by calling Tbuild.Unchecked_Convert_To, rather
than by directly calling Nmake.Make_Unchecked_Type_Conversion."

No test appears to be affected by this change, so this is just a
cleanup.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_ch6.adb (Build_Static_Check_Helper_Call): Replace explicit
call to Make_Unchecked_Type_Conversion with a call to
Unchecked_Convert_To.
* tbuild.adb (Unchecked_Convert_To): Fix whitespace.diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb
--- a/gcc/ada/exp_ch6.adb
+++ b/gcc/ada/exp_ch6.adb
@@ -7578,9 +7578,7 @@ package body Exp_Ch6 is
   and then Etype (F) /= Etype (A)
 then
Append_To (Actuals,
- Make_Unchecked_Type_Conversion (Loc,
-   New_Occurrence_Of (Etype (F), Loc),
-   New_Copy_Tree (A)));
+ Unchecked_Convert_To (Etype (F), New_Copy_Tree (A)));
 else
Append_To (Actuals, New_Copy_Tree (A));
 end if;


diff --git a/gcc/ada/tbuild.adb b/gcc/ada/tbuild.adb
--- a/gcc/ada/tbuild.adb
+++ b/gcc/ada/tbuild.adb
@@ -882,8 +882,8 @@ package body Tbuild is
   --  We don't really want to allow E_Void here, but existing code passes
   --  it.
 
-  Loc : constant Source_Ptr := Sloc (Expr);
-  Result  : Node_Id;
+  Loc: constant Source_Ptr := Sloc (Expr);
+  Result : Node_Id;
 
begin
   --  If the expression is already of the correct type, then nothing




Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Tobias Burnus

Hi Thomas,

On 06.07.22 14:38, Thomas Schwinge wrote:

:-) Haha, that's actually *exactly* what I had implemented first!  But
then I realized that 'target offloading_enabled' is doing exactly that:
check that offloading compilation is configured -- not that "there is an
offloading device available or not" as you seem to understand?  Or am I
confused there?


I think as you mentioned below – there is a difference. And that difference,
I explicitly maked use of:
 - libgomp.c-c++-common/requires-{1,5,7}.c test the device lto1 compiler,
   which requires that it is actually available. Thus, I used:
   /* { dg-do link { target { offload_target_nvptx || offload_target_amdgcn } } 
} */
while
 - libgomp.c-c++-common/requires-2.c checks that ENABLE_OFFLOADING == true, i.e.
   the host lto1 compiler is configured to create the offload tables and, thus,
   can diagnose the omp-requires mismatch. Hence, the testcase has:
   /* { dg-do link { target offloading_enabled } } */
   /* { dg-additional-options "-foffload=disable -flto" } */

Granted, as the other files do not use -foffload=..., it should not
make a difference - but, still, replacing it unconditionally
with 'target offloading_enabled' feels wrong.

I was about to write again about --enable-offload-defaulted and
having no offloading compilers installed. But that comes too late.
gcc.cc (the driver) will set
  OFFLOAD_TARGET_NAMES=
to the configured offload-target compilers (after -foffload= filtering),
but the is-available check is only done in lto-wrapper.cc, which comes
too late. But I admit it is unlikely that someone configures + builds
offload compilers and, then, for testing have no offload compilers
available.


I do however agree that (generally) replacing 'target offloading_enabled'
with a new 'target offload_target_any' would seem appropriate (as a
separate patch), because that would also do the right thing when running
libgomp testing with non-default '-foffload=[...]', including
'-foffload=disable'.


I concur – but I would prefer to have that lib/libgomp.exp patch committed
before the testsuite-part of this patch is committed. Albeit, I do not feel
very strong about 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


Re: [PATCH]middle-end: don't lower past veclower [PR106063]

2022-07-06 Thread Jeff Law via Gcc-patches




On 7/5/2022 9:00 AM, Tamar Christina via Gcc-patches wrote:

Hi All,

My previous patch can cause a problem if the pattern matches after veclower
as it may replace the construct with a vector sequence which the target may not
directly support.

As such don't perform the rewriting if after veclower.

Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
and no issues.

Ok for master? and backport to GCC 12?

Thanks,
Tamar


gcc/ChangeLog:

PR tree-optimization/106063
* match.pd: Do not apply pattern after veclower.

gcc/testsuite/ChangeLog:

PR tree-optimization/106063
* gcc.dg/pr106063.c: New test.

OK for both the trunk and gcc-12.
jeff



Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Thomas Schwinge
Hi Tobias!

On 2022-07-06T13:29:14+0200, Tobias Burnus  wrote:
> On 06.07.22 13:04, Thomas Schwinge wrote:
>> On 2022-06-08T05:56:02+0200, Tobias Burnus  wrote:
>>> PS: I have not fully tested the intelmic version.
>> As part of my standard testing, I'm reporting that it got completely
>> broken.  ;'-)
>
> Interesting. Because intelmic-mkoffload.cc calls GOMP_offload_register
> and not GOMP_offload_register_ver - and that call path should be unchanged.

True indeed for that code path...

> However, I missed that I had an assert that GCC_OFFLOAD_OMP_REQUIRES_FILE is
> set.

..., but not for that one.

> Thus, an alternative is to change that into an 'if'.
> But I concur that updating intelmic-mkoffload.cc is nicer! Thanks!

ACK.


> Regarding:
>> -! { dg-do link { target { offload_target_nvptx || offload_target_amdgcn } } 
>> }
>> +! { dg-do link { target offloading_enabled } }
> This patch looks wrong. We are not interested whether there is an offloading 
> device
> available or not - but whether the offloading compiler is running.
>
> Those are completely independent. Obviously, offloading can be configured but 
> not
> being present. (That's the usual case for testing distro builds but also can
> occur elsewhere.)
> And also the reverse if possible - usually because of -foffload=... but when 
> GCC is
> configured with --enable-offload-defaulted, also other combinations are 
> possible.
>
>
> I think the proper check would be write and use an 'offload_target_any',
> i.e. OFFLOAD_TARGET_NAMES= being present and nonempty.
>
> Cf. check_effective_target_offload_target_nvptx / ..._amdgcn and
> libgomp_check_effective_target_offload_target
> in libgomp/testsuite/lib/libgomp.exp
>
> Possible patch (untested):
>
> # Return 1 if compiling for some offload target(s)
> proc check_effective_target_offload_target_any { } {
>  return [libgomp_check_effective_target_offload_target ""]
> }
>
> At least if I understand the following correctly, "" should work:
>  return [string match "*:$target_name*:*" ":$gcc_offload_targets:"]

:-) Haha, that's actually *exactly* what I had implemented first!  But
then I realized that 'target offloading_enabled' is doing exactly that:
check that offloading compilation is configured -- not that "there is an
offloading device available or not" as you seem to understand?  Or am I
confused there?

I do however agree that (generally) replacing 'target offloading_enabled'
with a new 'target offload_target_any' would seem appropriate (as a
separate patch), because that would also do the right thing when running
libgomp testing with non-default '-foffload=[...]', including
'-foffload=disable'.

For checking "offloading device available" we'd use
'check_effective_target_offload_device[...]'.


Grüße
 Thomas


> Thanks for taking care of the patch fallout!
>
> 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


[committed] analyzer: fix uninit false positive with -ftrivial-auto-var-init= [PR106204]

2022-07-06 Thread David Malcolm via Gcc-patches
-fanalyzer handles -ftrivial-auto-var-init= by special-casing
IFN_DEFERRED_INIT to be a no-op, so that e.g.:

  len_2 = .DEFERRED_INIT (4, 2, &"len"[0]);

is treated as a no-op, so that len_2 is still uninitialized after the
stmt.

PR analyzer/106204 reports that -fanalyzer gives false positives from
-Wanalyzer-use-of-uninitialized-value on locals that have their address
taken, due to e.g.:

  _1 = .DEFERRED_INIT (4, 2, &"len"[0]);
  len = _1;

where -fanalyzer leaves _1 uninitialized, and then complains about
the assignment to "len".

Fixed thusly by suppressing the warning when assigning from such SSA
names.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-1517-gb33dd7874523af.

gcc/analyzer/ChangeLog:
PR analyzer/106204
* region-model.cc (within_short_circuited_stmt_p): Move extraction
of assign_stmt to caller.
(due_to_ifn_deferred_init_p): New.
(region_model::check_for_poison): Move extraction of assign_stmt
from within_short_circuited_stmt_p to here.  Share logic with
call to due_to_ifn_deferred_init_p.

gcc/testsuite/ChangeLog:
PR analyzer/106204
* gcc.dg/analyzer/torture/uninit-pr106204.c: New test.
* gcc.dg/analyzer/uninit-pr106204.c: New test.

Signed-off-by: David Malcolm 
---
 gcc/analyzer/region-model.cc  | 69 +++
 .../gcc.dg/analyzer/torture/uninit-pr106204.c | 13 
 .../gcc.dg/analyzer/uninit-pr106204.c | 17 +
 3 files changed, 86 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/uninit-pr106204.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/uninit-pr106204.c

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 5d939327e01..8b7b4e1f697 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -896,17 +896,9 @@ region_model::get_gassign_result (const gassign *assign,
 
 static bool
 within_short_circuited_stmt_p (const region_model *model,
-  region_model_context *ctxt)
+  const gassign *assign_stmt)
 {
-  gcc_assert (ctxt);
-  const gimple *curr_stmt = ctxt->get_stmt ();
-  if (curr_stmt == NULL)
-return false;
-
   /* We must have an assignment to a temporary of _Bool type.  */
-  const gassign *assign_stmt = dyn_cast  (curr_stmt);
-  if (!assign_stmt)
-return false;
   tree lhs = gimple_assign_lhs (assign_stmt);
   if (TREE_TYPE (lhs) != boolean_type_node)
 return false;
@@ -959,6 +951,47 @@ within_short_circuited_stmt_p (const region_model *model,
   return true;
 }
 
+/* Workaround for discarding certain false positives from
+   -Wanalyzer-use-of-uninitialized-value
+   seen with -ftrivial-auto-var-init=.
+
+   -ftrivial-auto-var-init= will generate calls to IFN_DEFERRED_INIT.
+
+   If the address of the var is taken, gimplification will give us
+   something like:
+
+ _1 = .DEFERRED_INIT (4, 2, &"len"[0]);
+ len = _1;
+
+   The result of DEFERRED_INIT will be an uninit value; we don't
+   want to emit a false positive for "len = _1;"
+
+   Return true if ASSIGN_STMT is such a stmt.  */
+
+static bool
+due_to_ifn_deferred_init_p (const gassign *assign_stmt)
+
+{
+  /* We must have an assignment to a decl from an SSA name that's the
+ result of a IFN_DEFERRED_INIT call.  */
+  if (gimple_assign_rhs_code (assign_stmt) != SSA_NAME)
+return false;
+  tree lhs = gimple_assign_lhs (assign_stmt);
+  if (TREE_CODE (lhs) != VAR_DECL)
+return false;
+  tree rhs = gimple_assign_rhs1 (assign_stmt);
+  if (TREE_CODE (rhs) != SSA_NAME)
+return false;
+  const gimple *def_stmt = SSA_NAME_DEF_STMT (rhs);
+  const gcall *call = dyn_cast  (def_stmt);
+  if (!call)
+return false;
+  if (gimple_call_internal_p (call)
+  && gimple_call_internal_fn (call) == IFN_DEFERRED_INIT)
+return true;
+  return false;
+}
+
 /* Check for SVAL being poisoned, adding a warning to CTXT.
Return SVAL, or, if a warning is added, another value, to avoid
repeatedly complaining about the same poisoned value in followup code.  */
@@ -982,10 +1015,20 @@ region_model::check_for_poison (const svalue *sval,
  && is_empty_type (sval->get_type ()))
return sval;
 
-  /* Special case to avoid certain false positives.  */
-  if (pkind == POISON_KIND_UNINIT
- && within_short_circuited_stmt_p (this, ctxt))
- return sval;
+  if (pkind == POISON_KIND_UNINIT)
+   if (const gimple *curr_stmt = ctxt->get_stmt ())
+ if (const gassign *assign_stmt
+   = dyn_cast  (curr_stmt))
+   {
+ /* Special case to avoid certain false positives.  */
+ if (within_short_circuited_stmt_p (this, assign_stmt))
+   return sval;
+
+ /* Special case to avoid false positive on
+-ftrivial-auto-var-init=.  */
+ if (due_to_ifn_deferred_init_p (assign_stmt))
+   

Re: [PATCH] Mips: Resolve build issues for the n32 ABI

2022-07-06 Thread Dimitrije Milosevic
Ping. :)
This change just landed on LLVM (see 
https://reviews.llvm.org/rG5d8077565e4196efdd4ed525a64c11a96d5aa5dd).
Unfortunately, I do not have commit access, so if anyone can commit it, that 
would be great!
Here is the patch with the updated commit message.

libsanitizer: Cherry-pick 5d8077565e41 from upstream

Building the ASAN for the n32 MIPS ABI currently fails, due to a few reasons:
- defined(__mips64), which is set solely based on the architecture type 
(32-bit/64-bit),
was still used in some places. Therefore, defined(__mips64) is swapped with 
SANITIZER_MIPS64,
which takes the ABI into account as well - defined(__mips64) &&
_MIPS_SIM == ABI64.
- The n32 ABI still uses 64-bit *Linux* system calls, even though the word size 
is 32 bits.
- After the transition to canonical system calls
(https://reviews.llvm.org/D124212), the n32 ABI still didn't use them,
even though they are supported,
as per 
https://github.com/torvalds/linux/blob/master/arch/mips/kernel/syscalls/syscall_n32.tbl.

See https://reviews.llvm.org/D127098.

libsanitizer/ChangeLog:

* sanitizer_common/sanitizer_linux.cpp (defined): Resolve
ASAN build issues for the Mips n32 ABI.
* sanitizer_common/sanitizer_platform.h (defined): Likewise.
---
 .../sanitizer_common/sanitizer_linux.cpp| 17 ++---
 .../sanitizer_common/sanitizer_platform.h   |  2 +-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/libsanitizer/sanitizer_common/sanitizer_linux.cpp 
b/libsanitizer/sanitizer_common/sanitizer_linux.cpp
index e2c32d679ad..5ba033492e7 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_linux.cpp
@@ -34,7 +34,7 @@
 // format. Struct kernel_stat is defined as 'struct stat' in asm/stat.h. To
 // access stat from asm/stat.h, without conflicting with definition in
 // sys/stat.h, we use this trick.
-#if defined(__mips64)
+#if SANITIZER_MIPS64
 #include 
 #include 
 #define stat kernel_stat
@@ -124,8 +124,9 @@ const int FUTEX_WAKE_PRIVATE = FUTEX_WAKE | 
FUTEX_PRIVATE_FLAG;
 // Are we using 32-bit or 64-bit Linux syscalls?
 // x32 (which defines __x86_64__) has SANITIZER_WORDSIZE == 32
 // but it still needs to use 64-bit syscalls.
-#if SANITIZER_LINUX && (defined(__x86_64__) || defined(__powerpc64__) ||   
\
-SANITIZER_WORDSIZE == 64)
+#if SANITIZER_LINUX && (defined(__x86_64__) || defined(__powerpc64__) || \
+SANITIZER_WORDSIZE == 64 ||  \
+(defined(__mips__) && _MIPS_SIM == _ABIN32))
 # define SANITIZER_LINUX_USES_64BIT_SYSCALLS 1
 #else
 # define SANITIZER_LINUX_USES_64BIT_SYSCALLS 0
@@ -289,7 +290,7 @@ static void stat64_to_stat(struct stat64 *in, struct stat 
*out) {
 }
 #endif
 
-#if defined(__mips64)
+#if SANITIZER_MIPS64
 // Undefine compatibility macros from 
 // so that they would not clash with the kernel_stat
 // st_[a|m|c]time fields
@@ -343,7 +344,8 @@ uptr internal_stat(const char *path, void *buf) {
 #if SANITIZER_FREEBSD
   return internal_syscall(SYSCALL(fstatat), AT_FDCWD, (uptr)path, (uptr)buf, 
0);
 #elif SANITIZER_LINUX
-#  if SANITIZER_WORDSIZE == 64 || SANITIZER_X32
+#  if SANITIZER_WORDSIZE == 64 || SANITIZER_X32 || \
+  (defined(__mips__) && _MIPS_SIM == _ABIN32)
   return internal_syscall(SYSCALL(newfstatat), AT_FDCWD, (uptr)path, (uptr)buf,
   0);
 #  else
@@ -366,7 +368,8 @@ uptr internal_lstat(const char *path, void *buf) {
   return internal_syscall(SYSCALL(fstatat), AT_FDCWD, (uptr)path, (uptr)buf,
   AT_SYMLINK_NOFOLLOW);
 #elif SANITIZER_LINUX
-#  if defined(_LP64) || SANITIZER_X32
+#  if defined(_LP64) || SANITIZER_X32 || \
+  (defined(__mips__) && _MIPS_SIM == _ABIN32)
   return internal_syscall(SYSCALL(newfstatat), AT_FDCWD, (uptr)path, (uptr)buf,
   AT_SYMLINK_NOFOLLOW);
 #  else
@@ -1053,7 +1056,7 @@ uptr GetMaxVirtualAddress() {
   return (1ULL << (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1)) - 1;
 #elif SANITIZER_RISCV64
   return (1ULL << 38) - 1;
-# elif defined(__mips64)
+# elif SANITIZER_MIPS64
   return (1ULL << 40) - 1;  // 0x00ffUL;
 # elif defined(__s390x__)
   return (1ULL << 53) - 1;  // 0x001fUL;
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform.h 
b/libsanitizer/sanitizer_common/sanitizer_platform.h
index 8fe0d831431..8bd9a327623 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform.h
@@ -159,7 +159,7 @@
 
 #if defined(__mips__)
 #  define SANITIZER_MIPS 1
-#  if defined(__mips64)
+#  if defined(__mips64) && _MIPS_SIM == _ABI64
 #define SANITIZER_MIPS32 0
 #define SANITIZER_MIPS64 1
 #  else
-- 
2.25.1


From: Richard Sandiford 
Sent: Monday, July 4, 2022 1:23 PM
To: Xi Ruoyao via Gcc-patches 
Cc: Dimitrije Milosevic ; Xi Ruoyao 
; Djordje 

Re: Mips: Fix kernel_stat structure size

2022-07-06 Thread Dimitrije Milosevic
Ping. :)
I think this is good to go. Unfortunately, I do not have commit access, so if 
anyone
can commit it, that would be great!


From: Dimitrije Milosevic 
Sent: Friday, July 1, 2022 4:25 PM
To: Xi Ruoyao ; gcc-patches@gcc.gnu.org 

Cc: Djordje Todorovic ; Richard Sandiford 

Subject: Re: Mips: Fix kernel_stat structure size 
 
Thanks Xi. Forgive me as I'm not that familiar with the coding standards 
when submitting patches for a review.
Here is the updated version of the patch.

Fix kernel_stat structure size for non-Android 32-bit Mips.
LLVM currently has this value for the kernel_stat structure size,
as per compiler-rt/lib/sanitizer-common/sanitizer_platform_limits_posix.h.
This also resolves one of the build issues for non-Android 32-bit Mips.

libsanitizer/ChangeLog:

    * sanitizer_common/sanitizer_platform_limits_posix.h: Fix
    kernel_stat structure size for non-Android 32-bit Mips.

---

 libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h 
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
index 89772a7e5c0..62a99035db3 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -83,7 +83,7 @@ const unsigned struct_kernel_stat64_sz = 104;
 #elif defined(__mips__)
 const unsigned struct_kernel_stat_sz = SANITIZER_ANDROID
    ? FIRST_32_SECOND_64(104, 128)
-   : FIRST_32_SECOND_64(144, 216);
+   : FIRST_32_SECOND_64(160, 216);
 const unsigned struct_kernel_stat64_sz = 104;
 #elif defined(__s390__) && !defined(__s390x__)
 const unsigned struct_kernel_stat_sz = 64;

---

Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Tobias Burnus

Hi Thomas,

On 06.07.22 13:04, Thomas Schwinge wrote:

On 2022-06-08T05:56:02+0200, Tobias Burnus  wrote:

PS: I have not fully tested the intelmic version.

As part of my standard testing, I'm reporting that it got completely
broken.  ;'-)


Interesting. Because intelmic-mkoffload.cc calls GOMP_offload_register
and not GOMP_offload_register_ver - and that call path should be unchanged.

However, I missed that I had an assert that GCC_OFFLOAD_OMP_REQUIRES_FILE is
set. - Thus, an alternative is to change that into an 'if'.

But I concur that updating intelmic-mkoffload.cc is nicer! Thanks!


Regarding:

-! { dg-do link { target { offload_target_nvptx || offload_target_amdgcn } } }
+! { dg-do link { target offloading_enabled } }

This patch looks wrong. We are not interested whether there is an offloading 
device
available or not - but whether the offloading compiler is running.

Those are completely independent. Obviously, offloading can be configured but 
not
being present. (That's the usual case for testing distro builds but also can
occur elsewhere.)
And also the reverse if possible - usually because of -foffload=... but when 
GCC is
configured with --enable-offload-defaulted, also other combinations are 
possible.


I think the proper check would be write and use an 'offload_target_any',
i.e. OFFLOAD_TARGET_NAMES= being present and nonempty.

Cf. check_effective_target_offload_target_nvptx / ..._amdgcn and
libgomp_check_effective_target_offload_target
in libgomp/testsuite/lib/libgomp.exp

Possible patch (untested):

# Return 1 if compiling for some offload target(s)
proc check_effective_target_offload_target_any { } {
return [libgomp_check_effective_target_offload_target ""]
}

At least if I understand the following correctly, "" should work:
return [string match "*:$target_name*:*" ":$gcc_offload_targets:"]


Thanks for taking care of the patch fallout!

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


Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Thomas Schwinge
Hi!

On 2022-06-08T05:56:02+0200, Tobias Burnus  wrote:
> This is based on Chung-Lin's patch at 
> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563393.html

> PS: I have not fully tested the intelmic version.

As part of my standard testing, I'm reporting that it got completely
broken.  ;'-) Your commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp" states:
"When the device lto1 runs, it extracts the data for mkoffload. The
latter than passes the value on to GOMP_offload_register_ver."
That's not implemented for Intel MIC 'mkoffload', so we always run into
'gcc/lto-cgraph.cc:input_offload_tables':

+#ifdef ACCEL_COMPILER
+  char *omp_requires_file = getenv ("GCC_OFFLOAD_OMP_REQUIRES_FILE");
+  if (omp_requires_file == NULL || omp_requires_file[0] == '\0')
+fatal_error (input_location, "GCC_OFFLOAD_OMP_REQUIRES_FILE unset");

..., and all offloading compilation fail with that 'fatal_error'.  OK to
push the attached "Fix Intel MIC 'mkoffload' for OpenMP 'requires'"?
(Currently testing.)


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 afd77646d7ced9f58fb49667e37ee4e21dd6fc53 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 5 Jul 2022 12:21:33 +0200
Subject: [PATCH] Fix Intel MIC 'mkoffload' for OpenMP 'requires'

Similar to how the other 'mkoffload's got changed in
recent commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp".

This also means finally switching Intel MIC 'mkoffload' to
'GOMP_offload_register_ver', 'GOMP_offload_unregister_ver',
making 'GOMP_offload_register', 'GOMP_offload_unregister'
legacy entry points.

	gcc/
	* config/i386/intelmic-mkoffload.cc (generate_host_descr_file)
	(prepare_target_image, main): Handle OpenMP 'requires'.
	(generate_host_descr_file): Switch to 'GOMP_offload_register_ver',
	'GOMP_offload_unregister_ver'.
	libgomp/
	* target.c (GOMP_offload_register, GOMP_offload_unregister):
	Denote as legacy entry points.
	* testsuite/libgomp.c-c++-common/requires-1.c: Enable for all
	'target offloading_enabled'.
	* testsuite/libgomp.c-c++-common/requires-5.c: Likewise.
	* testsuite/libgomp.c-c++-common/requires-7.c: Likewise.
	* testsuite/libgomp.fortran/requires-1.f90: Likewise.
---
 gcc/config/i386/intelmic-mkoffload.cc | 56 +++
 libgomp/target.c  |  4 ++
 .../libgomp.c-c++-common/requires-1.c |  2 +-
 .../libgomp.c-c++-common/requires-5.c |  2 +-
 .../libgomp.c-c++-common/requires-7.c |  2 +-
 .../testsuite/libgomp.fortran/requires-1.f90  |  2 +-
 6 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/gcc/config/i386/intelmic-mkoffload.cc b/gcc/config/i386/intelmic-mkoffload.cc
index c683d6f473e..596f6f107b8 100644
--- a/gcc/config/i386/intelmic-mkoffload.cc
+++ b/gcc/config/i386/intelmic-mkoffload.cc
@@ -370,7 +370,7 @@ generate_target_offloadend_file (const char *target_compiler)
 
 /* Generates object file with the host side descriptor.  */
 static const char *
-generate_host_descr_file (const char *host_compiler)
+generate_host_descr_file (const char *host_compiler, uint32_t omp_requires)
 {
   char *dump_filename = concat (dumppfx, "_host_descr.c", NULL);
   const char *src_filename = save_temps
@@ -386,39 +386,50 @@ generate_host_descr_file (const char *host_compiler)
   if (!src_file)
 fatal_error (input_location, "cannot open '%s'", src_filename);
 
+  fprintf (src_file, "#include \n\n");
+
   fprintf (src_file,
 	   "extern const void *const __OFFLOAD_TABLE__;\n"
 	   "extern const void *const __offload_image_intelmic_start;\n"
 	   "extern const void *const __offload_image_intelmic_end;\n\n"
 
-	   "static const void *const __offload_target_data[] = {\n"
+	   "static const struct intelmic_data {\n"
+	   "  uintptr_t omp_requires_mask;\n"
+	   "  const void *const image_start;\n"
+	   "  const void *const image_end;\n"
+	   "} intelmic_data = {\n"
+	   "  %d,\n"
 	   "  &__offload_image_intelmic_start, &__offload_image_intelmic_end\n"
-	   "};\n\n");
+	   "};\n\n", omp_requires);
 
   fprintf (src_file,
 	   "#ifdef __cplusplus\n"
 	   "extern \"C\"\n"
 	   "#endif\n"
-	   "void GOMP_offload_register (const void *, int, const void *);\n"
+	   "void GOMP_offload_register_ver (unsigned, const void *, int, const void *);\n"
 	   "#ifdef __cplusplus\n"
 	   "extern \"C\"\n"
 	   "#endif\n"
-	   "void GOMP_offload_unregister (const void *, int, const void *);\n\n"
+	   "void GOMP_offload_unregister_ver (unsigned, const void *, int, const void *);\n\n"
 
 	   "__attribute__((constructor))\n"
 	   "static void\n"
 	   "init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (&__OFFLOAD_TABLE__, %d, __offload_target_data);\n"
-	   

[PATCH] Add a heuristic for eliminate redundant load and store in inline pass.

2022-07-06 Thread Cui,Lili via Gcc-patches
From: Lili 


Hi Hubicka,

This patch is to add a heuristic inline hint to eliminate redundant load and 
store.

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
OK for trunk?

Thanks,
Lili.

Add a INLINE_HINT_eliminate_load_and_store hint in to inline pass.
We accumulate the insn number of redundant load and store that can be
reduced by these three cases, when the count size is greater than the
threshold, we will enable the hint. with the hint, inlining_insns_auto
will enlarge the bound.

1. Caller's store is same with callee's load
2. Caller's load is same with callee's load
3. Callee's load is same with caller's local memory access

With the patch applied
Icelake server: 538.imagic_r get 14.10% improvement for multicopy and 38.90%
improvement for single copy with no measurable changes for other benchmarks.

Casecadelake: 538.imagic_r get 12.5% improvement for multicopy with and code
size increased by 0.2%. With no measurable changes for other benchmarks.

Znver3 server: 538.imagic_r get 14.20% improvement for multicopy with and codei
size increased by 0.3%. With no measurable changes for other benchmarks.

CPU2017 single copy performance data for Icelake server
BenchMarks   Score   Build time  Code size
500.perlbench_r  1.50%   -0.20%  0.00%
502.gcc_r0.10%   -0.10%  0.00%
505.mcf_r0.00%   1.70%   0.00%
520.omnetpp_r-0.60%  -0.30%  0.00%
523.xalancbmk_r  0.60%   0.00%   0.00%
525.x264_r   0.00%   -0.20%  0.00%
531.deepsjeng_r  0.40%   -1.10%  -0.10%
541.leela_r  0.00%   0.00%   0.00%
548.exchange2_r  0.00%   -0.90%  0.00%
557.xz_r 0.00%   0.00%   0.00%
503.bwaves_r 0.00%   1.40%   0.00%
507.cactuBSSN_r  0.00%   1.00%   0.00%
508.namd_r   0.00%   0.30%   0.00%
510.parest_r 0.00%   -0.40%  0.00%
511.povray_r 0.70%   -0.60%  0.00%
519.lbm_r0.00%   0.00%   0.00%
521.wrf_r0.00%   0.60%   0.00%
526.blender_r0.00%   0.00%   0.00%
527.cam4_r   -0.30%  -0.50%  0.00%
538.imagick_r38.90%  0.50%   0.20%
544.nab_r0.00%   1.10%   0.00%
549.fotonik3d_r  0.00%   0.90%   0.00%
554.roms_r   2.30%   -0.10%  0.00%
Geomean-int  0.00%   -0.30%  0.00%
Geomean-fp   3.80%   0.30%   0.00%

gcc/ChangeLog:

* ipa-fnsummary.cc (ipa_dump_hints): Add print for hint 
"eliminate_load_and_store"
* ipa-fnsummary.h (enum ipa_hints_vals): Add 
INLINE_HINT_eliminate_load_and_store.
* ipa-inline-analysis.cc (do_estimate_edge_time): Add judgment for 
INLINE_HINT_eliminate_load_and_store.
* ipa-inline.cc (want_inline_small_function_p): Add 
"INLINE_HINT_eliminate_load_and_store" for hints flag.
* ipa-modref-tree.h (struct modref_access_node): Move function contains 
to public..
(struct modref_tree): Add new function "same" and 
"local_vector_memory_accesse"
* ipa-modref.cc (eliminate_load_and_store): New.
(ipa_merge_modref_summary_after_inlining): Change the input value of 
useful_p.
* ipa-modref.h (eliminate_load_and_store): New.
* opts.cc: Add param "min_inline_hint_eliminate_loads_num"
* params.opt: Ditto.

gcc/testsuite/ChangeLog:

* gcc.dg/ipa/inlinehint-6.c: New test.
---
 gcc/ipa-fnsummary.cc|   5 ++
 gcc/ipa-fnsummary.h |   4 +-
 gcc/ipa-inline-analysis.cc  |   7 ++
 gcc/ipa-inline.cc   |   3 +-
 gcc/ipa-modref-tree.h   | 109 +++-
 gcc/ipa-modref.cc   |  46 +-
 gcc/ipa-modref.h|   1 +
 gcc/opts.cc |   1 +
 gcc/params.opt  |   4 +
 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c |  54 
 10 files changed, 229 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c

diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc
index e2a86680a21..0a962f62490 100644
--- a/gcc/ipa-fnsummary.cc
+++ b/gcc/ipa-fnsummary.cc
@@ -150,6 +150,11 @@ ipa_dump_hints (FILE *f, ipa_hints hints)
   hints &= ~INLINE_HINT_builtin_constant_p;
   fprintf (f, " builtin_constant_p");
 }
+  if (hints & INLINE_HINT_eliminate_load_and_store)
+{
+  hints &= ~INLINE_HINT_eliminate_load_and_store;
+  fprintf (f, " eliminate_load_and_store");
+}
   gcc_assert (!hints);
 }
 
diff --git a/gcc/ipa-fnsummary.h b/gcc/ipa-fnsummary.h
index 941fea6de0d..5f589e5ea0d 100644
--- a/gcc/ipa-fnsummary.h
+++ b/gcc/ipa-fnsummary.h
@@ -52,7 +52,9 @@ enum ipa_hints_vals {
   INLINE_HINT_known_hot = 128,
   /* There is builtin_constant_p dependent on parameter which is usually
  a strong hint to inline.  */
-  INLINE_HINT_builtin_constant_p = 256
+  INLINE_HINT_builtin_constant_p = 256,
+  /* Inlining can 

Restore 'GOMP_offload_unregister_ver' functionality (was: [Patch][v5] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Thomas Schwinge
Hi!

On 2022-07-01T15:06:05+0200, Tobias Burnus  wrote:
> Attached is the updated patch. Main changes: [...]

This is now a great implementation of cross-component communication
(host/offloading compilers, runtime), thanks!  I'm sure this will be
usable (or at least instructing) for further purposes, too.

> - Uses GOMP_register_var to pass the mask to libgomp

Like 'GOMP_offload_register_ver', also 'GOMP_offload_unregister_ver'
needs to be adjusted correspondingly.  OK to push the attached
"Restore 'GOMP_offload_unregister_ver' functionality"?  (Currently
testing.)

> (and no longer a weak variable)

... which actually removed my "contribution" (hack!) to this patch.  ;-)


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 9a49a3e1e4d3def7b48beccdde6fa9f218719244 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 5 Jul 2022 18:23:15 +0200
Subject: [PATCH] Restore 'GOMP_offload_unregister_ver' functionality

The recent commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp" changed the
'GOMP_offload_register_ver' interface but didn't change
'GOMP_offload_unregister_ver' accordingly, so we're no longer
actually unregistering.

	gcc/
	* config/gcn/mkoffload.cc (process_obj): Clarify 'target_data' ->
	'[...]_data'.
	* config/nvptx/mkoffload.cc (process): Likewise.
	libgomp/
	* target.c (GOMP_offload_register_ver): Clarify 'target_data' ->
	'data'.
	(GOMP_offload_unregister_ver): Likewise.  Fix up 'target_data',
	and add 'assert'.
---
 gcc/config/gcn/mkoffload.cc   |  8 
 gcc/config/nvptx/mkoffload.cc |  8 
 libgomp/target.c  | 33 ++---
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index b8b3fecfcb4..d2464332275 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -692,13 +692,13 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
 	   len);
 
   fprintf (cfile,
-	   "static const struct gcn_image_desc {\n"
+	   "static const struct gcn_data {\n"
 	   "  uintptr_t omp_requires_mask;\n"
 	   "  const struct gcn_image *gcn_image;\n"
 	   "  unsigned kernel_count;\n"
 	   "  const struct hsa_kernel_description *kernel_infos;\n"
 	   "  unsigned global_variable_count;\n"
-	   "} target_data = {\n"
+	   "} gcn_data = {\n"
 	   "  %d,\n"
 	   "  _image,\n"
 	   "  sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n"
@@ -723,7 +723,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
   fprintf (cfile, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*GCN*/, _data);\n"
+	   " %d/*GCN*/, _data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 	   GOMP_DEVICE_GCN);
@@ -731,7 +731,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
   fprintf (cfile, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
 	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*GCN*/, _data);\n"
+	   " %d/*GCN*/, _data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 	   GOMP_DEVICE_GCN);
diff --git a/gcc/config/nvptx/mkoffload.cc b/gcc/config/nvptx/mkoffload.cc
index d8c81eb0547..0fa5f4423bf 100644
--- a/gcc/config/nvptx/mkoffload.cc
+++ b/gcc/config/nvptx/mkoffload.cc
@@ -310,7 +310,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "\n};\n\n");
 
   fprintf (out,
-	   "static const struct nvptx_tdata {\n"
+	   "static const struct nvptx_data {\n"
 	   "  uintptr_t omp_requires_mask;\n"
 	   "  const struct ptx_obj *ptx_objs;\n"
 	   "  unsigned ptx_num;\n"
@@ -318,7 +318,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
 	   "  unsigned var_num;\n"
 	   "  const struct nvptx_fn *fn_names;\n"
 	   "  unsigned fn_num;\n"
-	   "} target_data = {\n"
+	   "} nvptx_data = {\n"
 	   "  %d, ptx_objs, sizeof (ptx_objs) / sizeof (ptx_objs[0]),\n"
 	   "  var_mappings,"
 	   "  sizeof (var_mappings) / sizeof (var_mappings[0]),\n"
@@ -344,7 +344,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*NVIDIA_PTX*/, _data);\n"
+	   " %d/*NVIDIA_PTX*/, _data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 	   GOMP_DEVICE_NVIDIA_PTX);
@@ -352,7 +352,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
 	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*NVIDIA_PTX*/, _data);\n"
+	   " 

Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a single place (was: [Patch] OpenMP: Move omp requires checks to libgomp)

2022-07-06 Thread Thomas Schwinge
Hi!

On 2022-06-08T05:56:02+0200, Tobias Burnus  wrote:
> This is based on Chung-Lin's patch at 
> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563393.html

> --- a/include/gomp-constants.h
> +++ b/include/gomp-constants.h

> +/* Flag values for requires-directive features, must match corresponding
> +   OMP_REQUIRES_* values in gcc/omp-general.h.  */
> +#define GOMP_REQUIRES_UNIFIED_ADDRESS   0x10
> +#define GOMP_REQUIRES_UNIFIED_SHARED_MEMORY 0x20
> +#define GOMP_REQUIRES_REVERSE_OFFLOAD   0x80

To make things more failure proof, OK to push the attached
"Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a single place"?


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 bd1aa5bc96e141a85bb53d61a5c7531e09ea3cf6 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 5 Jul 2022 11:04:46 +0200
Subject: [PATCH] Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a
 single place

Clean up for recent commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp".

	gcc/
	* omp-general.h (enum omp_requires): Use 'GOMP_REQUIRES_[...]'.
	include/
	* gomp-constants.h (OMP_REQUIRES_[...]): Update comment.
---
 gcc/omp-general.h| 8 
 include/gomp-constants.h | 3 +--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/omp-general.h b/gcc/omp-general.h
index 7a94831e8f5..74e90e1a71a 100644
--- a/gcc/omp-general.h
+++ b/gcc/omp-general.h
@@ -126,12 +126,12 @@ extern int oacc_get_ifn_dim_arg (const gimple *stmt);
 
 enum omp_requires {
   OMP_REQUIRES_ATOMIC_DEFAULT_MEM_ORDER = 0xf,
-  OMP_REQUIRES_UNIFIED_ADDRESS = 0x10,
-  OMP_REQUIRES_UNIFIED_SHARED_MEMORY = 0x20,
+  OMP_REQUIRES_UNIFIED_ADDRESS = GOMP_REQUIRES_UNIFIED_ADDRESS,
+  OMP_REQUIRES_UNIFIED_SHARED_MEMORY = GOMP_REQUIRES_UNIFIED_SHARED_MEMORY,
   OMP_REQUIRES_DYNAMIC_ALLOCATORS = 0x40,
-  OMP_REQUIRES_REVERSE_OFFLOAD = 0x80,
+  OMP_REQUIRES_REVERSE_OFFLOAD = GOMP_REQUIRES_REVERSE_OFFLOAD,
   OMP_REQUIRES_ATOMIC_DEFAULT_MEM_ORDER_USED = 0x100,
-  OMP_REQUIRES_TARGET_USED = 0x200
+  OMP_REQUIRES_TARGET_USED = GOMP_REQUIRES_TARGET_USED,
 };
 
 extern GTY(()) enum omp_requires omp_requires_mask;
diff --git a/include/gomp-constants.h b/include/gomp-constants.h
index 3e3078f082e..84316f953d0 100644
--- a/include/gomp-constants.h
+++ b/include/gomp-constants.h
@@ -341,8 +341,7 @@ enum gomp_map_kind
 #define GOMP_DEPEND_MUTEXINOUTSET	4
 #define GOMP_DEPEND_INOUTSET		5
 
-/* Flag values for requires-directive features, must match corresponding
-   OMP_REQUIRES_* values in gcc/omp-general.h.  */
+/* Flag values for OpenMP 'requires' directive features.  */
 #define GOMP_REQUIRES_UNIFIED_ADDRESS   0x10
 #define GOMP_REQUIRES_UNIFIED_SHARED_MEMORY 0x20
 #define GOMP_REQUIRES_REVERSE_OFFLOAD   0x80
-- 
2.35.1