Re: [PATCH] libstdc++: testsuite: fs rename to self may fail

2022-06-22 Thread Alexandre Oliva via Gcc-patches
Hello, Sebastian,

On Jun 22, 2022, Sebastian Huber  wrote:

> On 22/06/2022 08:24, Alexandre Oliva via Libstdc++ wrote:
>> rtems6's rename() implementation errors with EEXIST when the rename-to
>> filename exists, even when renaming a file to itself or when renaming
>> a nonexisting file.  Adjust expectations.
>> 
>> Regstrapped on x86_64-linux-gnu, also tested with a cross to
>> aarch64-rtems6.  Ok to install?
>> 
>> PS:https://devel.rtems.org/ticket/2169  doesn't seem to suggest plans to
>> change behavior so as to comply with POSIX.

> I would not adjust the test case to cope with systems which are not in
> line with POSIX.

My understanding is that the libstdc++ testsuite is not meant to test
for POSIX conformance, but for conformance with the C++ language
standards.

C++ inherits rename from C, and C says the behavior is implementation
defined if the new name already exists.

RTEMS is thus comformant with the requirements from C (and thus C++),
and it is therefore reasonable for libstdc++'s testsuite to accept
RTEMS' behavior as such.


That said, because libstdc++ tests are all-or-nothing, perhaps it would
make sense to have a separate test for strict POSIX conformance in
rename, XFAILed on RTEMS targets.  How about that?

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


Re: [PATCH] libgompd: Fix sizes in OMPD support and add local ICVs finctions.

2022-06-22 Thread Jakub Jelinek via Gcc-patches
On Wed, Jun 22, 2022 at 04:17:45AM +0200, Mohamed Atef wrote:
> I forgot the DCO line. And I edited the commit message, but I can't push,
> even forced push doesn't work.

No, once you push, it will remain as is.
Don't forget next time...

Jakub



Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642]

2022-06-22 Thread Jakub Jelinek via Gcc-patches
On Tue, Jun 21, 2022 at 11:59:56PM -0400, Jason Merrill wrote:
>   PR c++/104642
> 
> gcc/ChangeLog:
> 
>   * common.opt: Add -funreachable-traps.
>   * doc/invoke.texi (-funreachable-traps): Document it.
>   * opts.cc (finish_options): Enable at -O0 or -Og.
>   * tree.cc (build_common_builtin_nodes): Add __builtin_trap.
>   (builtin_decl_unreachable, build_builtin_unreachable): New.
>   * tree.h: Declare them.
>   * ubsan.cc (sanitize_unreachable_fn): Factor out.
>   (ubsan_instrument_unreachable): Use
>   gimple_build_builtin_unreachable.
>   * ubsan.h (sanitize_unreachable_fn): Declare.
>   * gimple.cc (gimple_build_builtin_unreachable): New.
>   * gimple.h: Declare it.
>   * builtins.cc (expand_builtin_unreachable): Add assert.
>   (fold_builtin_0): Call build_builtin_unreachable.
>   * sanopt.cc: Don't run for just SANITIZE_RETURN
>   or SANITIZE_UNREACHABLE when trapping.
>   * cgraphunit.cc (walk_polymorphic_call_targets): Use new
>   unreachable functions.
>   * gimple-fold.cc (gimple_fold_call)
>   (gimple_get_virt_method_for_vtable)
>   * ipa-fnsummary.cc (redirect_to_unreachable)
>   * ipa-prop.cc (ipa_make_edge_direct_to_target)
>   (ipa_impossible_devirt_target)
>   * ipa.cc (walk_polymorphic_call_targets)
>   * tree-cfg.cc (pass_warn_function_return::execute)
>   (execute_fixup_cfg)
>   * tree-ssa-loop-ivcanon.cc (remove_exits_and_undefined_stmts)
>   (unloop_loops)
>   * tree-ssa-sccvn.cc (eliminate_dom_walker::eliminate_stmt):
>   Likewise.
> 
> gcc/cp/ChangeLog:
> 
>   * constexpr.cc (cxx_eval_builtin_function_call): Handle
>   unreachable/trap earlier.
>   * cp-gimplify.cc (cp_maybe_instrument_return): Use
>   build_builtin_unreachable.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/ubsan/return-8a.C: New test.
>   * g++.dg/ubsan/return-8b.C: New test.
>   * g++.dg/ubsan/return-8d.C: New test.
>   * g++.dg/ubsan/return-8e.C: New test.

LGTM, thanks.

Jakub



[PATCH v2, rs6000] Use CC for BCD operations [PR100736]

2022-06-22 Thread HAO CHEN GUI via Gcc-patches
Hi,
  This patch uses CC instead of CCFP for all BCD operations. Thus, infinite
math flag has no impact on BCD operations. To support BCD overflow and
invalid coding, an UNSPEC is defined to move the bit to a general register.
The patterns of condition branch and return with overflow bit are defined as
the UNSPEC and branch/return can be combined to one jump insn. The split
pattern of overflow bit extension is define for optimization.

  This patch also replaces bcdadd with bcdsub for BCD invaliding coding
expand.

ChangeLog
2022-06-22 Haochen Gui 

gcc/
PR target/100736
* config/rs6000/altivec.md (BCD_TEST): Remove unordered.
(bcd_): Replace CCFP with CC.
(*bcd_test_): Replace CCFP with CC.  Generate
condition insn with CC mode.
(bcd_overflow_): New.
(*bcdoverflow_): New.
(*bcdinvalid_): Removed.
(bcdinvalid_): Implement by UNSPEC_BCDSUB and UNSPEC_BCD_OVERFLOW.
(nuun): New.
(*overflow_cbranch): New.
(*overflow_creturn): New.
(*overflow_extendsidi): New.
(bcdshift_v16qi): Replace CCFP with CC.
(bcdmul10_v16qi): Likewise.
(bcddiv10_v16qi): Likewise.
(peephole for bcd_add/sub): Likewise.
* config/rs6000/rs6000-builtins.def (__builtin_bcdadd_ov_v1ti): Set
pattern to bcdadd_overflow_v1ti.
(__builtin_bcdadd_ov_v16qi): Set pattern to bcdadd_overflow_v16qi.
(__builtin_bcdsub_ov_v1ti): Set pattern to bcdsub_overflow_v1ti.
(__builtin_bcdsub_ov_v16qi): Set pattern to bcdsub_overflow_v16qi.

gcc/testsuite/
PR target/100736
* gcc.target/powerpc/bcd-4.c: Adjust number of bcdadd and bcdsub.
Scan no cror insns.

patch.diff
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index efc8ae35c2e..26f131e61ea 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -4370,7 +4370,7 @@ (define_int_iterator UNSPEC_BCD_ADD_SUB [UNSPEC_BCDADD 
UNSPEC_BCDSUB])
 (define_int_attr bcd_add_sub [(UNSPEC_BCDADD "add")
  (UNSPEC_BCDSUB "sub")])

-(define_code_iterator BCD_TEST [eq lt le gt ge unordered])
+(define_code_iterator BCD_TEST [eq lt le gt ge])
 (define_mode_iterator VBCD [V1TI V16QI])

 (define_insn "bcd_"
@@ -4379,7 +4379,7 @@ (define_insn "bcd_"
  (match_operand:VBCD 2 "register_operand" "v")
  (match_operand:QI 3 "const_0_to_1_operand" "n")]
 UNSPEC_BCD_ADD_SUB))
-   (clobber (reg:CCFP CR6_REGNO))]
+   (clobber (reg:CC CR6_REGNO))]
   "TARGET_P8_VECTOR"
   "bcd. %0,%1,%2,%3"
   [(set_attr "type" "vecsimple")])
@@ -4389,9 +4389,9 @@ (define_insn "bcd_"
 ;; UNORDERED test on an integer type (like V1TImode) is not defined.  The type
 ;; probably should be one that can go in the VMX (Altivec) registers, so we
 ;; can't use DDmode or DFmode.
-(define_insn "*bcd_test_"
-  [(set (reg:CCFP CR6_REGNO)
-   (compare:CCFP
+(define_insn "bcd_test_"
+  [(set (reg:CC CR6_REGNO)
+   (compare:CC
 (unspec:V2DF [(match_operand:VBCD 1 "register_operand" "v")
   (match_operand:VBCD 2 "register_operand" "v")
   (match_operand:QI 3 "const_0_to_1_operand" "i")]
@@ -4408,8 +4408,8 @@ (define_insn "*bcd_test2_"
  (match_operand:VBCD 2 "register_operand" "v")
  (match_operand:QI 3 "const_0_to_1_operand" "i")]
 UNSPEC_BCD_ADD_SUB))
-   (set (reg:CCFP CR6_REGNO)
-   (compare:CCFP
+   (set (reg:CC CR6_REGNO)
+   (compare:CC
 (unspec:V2DF [(match_dup 1)
   (match_dup 2)
   (match_dup 3)]
@@ -4502,8 +4502,8 @@ (define_insn "vclrrb"
[(set_attr "type" "vecsimple")])

 (define_expand "bcd__"
-  [(parallel [(set (reg:CCFP CR6_REGNO)
-  (compare:CCFP
+  [(parallel [(set (reg:CC CR6_REGNO)
+  (compare:CC
(unspec:V2DF [(match_operand:VBCD 1 "register_operand")
  (match_operand:VBCD 2 "register_operand")
  (match_operand:QI 3 "const_0_to_1_operand")]
@@ -4511,46 +4511,138 @@ (define_expand "bcd__"
(match_dup 4)))
  (clobber (match_scratch:VBCD 5))])
(set (match_operand:SI 0 "register_operand")
-   (BCD_TEST:SI (reg:CCFP CR6_REGNO)
+   (BCD_TEST:SI (reg:CC CR6_REGNO)
 (const_int 0)))]
   "TARGET_P8_VECTOR"
 {
   operands[4] = CONST0_RTX (V2DFmode);
+  emit_insn (gen_bcd_test_ (operands[0], operands[1],
+  operands[2], operands[3],
+  operands[4]));
+
+  rtx cr6 = gen_rtx_REG (CCmode, CR6_REGNO);
+  rtx condition_rtx = gen_rtx_ (SImode, cr6, const0_rtx);
+
+  if ( == GE ||  == LE)
+{
+  rtx not_result = gen_reg_rtx (CCEQmode);
+  rtx not_op, rev_cond_rtx;
+  rev_cond_rtx = gen_rtx_fmt_ee (

Re: [PATCH] c: Extend the -Wpadded message with actual padding size

2022-06-22 Thread Vit Kabele
Hello,

On Mon, Jun 20, 2022 at 04:05:17PM -0700, Andrew Pinski wrote:
> > new file mode 100644
> > index 000..e8f1044a36b
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/Wpadded.c
> > @@ -0,0 +1,10 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Wpadded" } */
> > +
> > +/*
> > + * The struct is on single line, because C++ compiler emits the -Wpadded
> > + * warning at the first line of the struct, while the C compiler at the 
> > last
> > + * line of the struct definition. This way the test passes on both
> > + */
> > +struct S { __UINT32_TYPE__ i; char c; }; /* { dg-warning "padding struct 
> > size to alignment boundary with 3 bytes" } */
> > +
> Note the testcase will fail on some targets where alignment is 1 for 
> everything.
> You most likely want the dg-warning to be like it is in gcc.dg/Wpadded.c:
> /* { dg-warning "padding struct size to alignment boundary with 3
> bytes" ""  { target { ! default_packed } } } */
> 
> You might want the following from the same file too:
> /* -fpack-struct is necessary because the warning expected requires the 
> initial
>packing to be larger than 1, which cannot be guaranteed for all targets.
>We won't get a warning anyway if the target has "packed" structure
>layout.  */
> /* { dg-options "-Wpadded -fpack-struct=8" } */
> /* { dg-additional-options "-mno-ms-bitfields" { target *-*-mingw* } } */
I added the ! default_packed directive, but I am not sure whether the
-fpack-struct is needed. Could you please provide a name of the particular 
target
with such alignment constraints so I can test it?

-- 
Thank you,
Vit Kabele


Re: [PATCH] aarch64: testsuite: symbol-range compile only

2022-06-22 Thread Richard Sandiford via Gcc-patches
Alexandre Oliva  writes:
> On Jun 21, 2022, Richard Sandiford  wrote:
>
>> Could we instead have a new target selector for whether the memory
>> map includes xGB of RAM?
>
> How about this?  Testing on aarch64-rtems6.0.  Ok to install?
>
>
> aarch64: testsuite: symbol-range fallback to compile
>
> From: Alexandre Oliva 
>
> On some of our embedded aarch64 targets, RAM size is too small for
> this test to fit.  It doesn't look like this test requires linking,
> and if it does, the -tiny version may presumably get most of the
> coverage without going overboard in target system requirements.
>
> Still, linking may be useful, so introduce a TwoPlusGigs effective
> target, that checks for the ability to link a program with 2GB of
> sbss, and use that to select whether to link or just compile
> symbol-range.c.
>
>
> for  gcc/testsuite/ChangeLog
>
>   * lib/target-supports.exp
>   (check_effective_target_TwoPlusGigs): New.
>   * gcc.target/aarch64/symbol-range.c: Link only on
>   TwoPlusGigs targets, compile otherwise.

Other selectors don't use CamelCase, so I guess it should be
two_plus_gigs instead.  There also needs to be an entry in
sourcebuild.texi.  (Personally I'm not sure how useful those
entries are, since grepping for comments in the source is
usually easier, but still.)

OK with those changes, thanks.

Richard

> ---
>  gcc/testsuite/gcc.target/aarch64/symbol-range.c |3 ++-
>  gcc/testsuite/lib/target-supports.exp   |9 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c 
> b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index d8e82fa1b2829..f9a916c7ae2f0 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,4 +1,5 @@
> -/* { dg-do link } */
> +/* { dg-do link { target TwoPlusGigs } } */
> +/* { dg-do compile { target { ! TwoPlusGigs } } } */
>  /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>  
>  char fixed_regs[0x8000];
> diff --git a/gcc/testsuite/lib/target-supports.exp 
> b/gcc/testsuite/lib/target-supports.exp
> index d1f4eb7641fa7..0507d6e617fef 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -2906,6 +2906,15 @@ proc check_effective_target_le { } {
>  }]
>  }
>  
> +# Return 1 if we can link a program with 2+GB of data.
> +
> +proc check_effective_target_TwoPlusGigs { } {
> +return [check_no_compiler_messages TwoPlusGigs executable {
> + int dummy[0x8000];
> + int main () { return 0; }
> +}]
> +}
> +
>  # Return 1 if we're generating 32-bit code using default options, 0
>  # otherwise.


Re: [PATCH] c: Extend the -Wpadded message with actual padding size

2022-06-22 Thread Andrew Pinski via Gcc-patches
On Wed, Jun 22, 2022 at 1:34 AM Vit Kabele  wrote:
>
> Hello,
>
> On Mon, Jun 20, 2022 at 04:05:17PM -0700, Andrew Pinski wrote:
> > > new file mode 100644
> > > index 000..e8f1044a36b
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/Wpadded.c
> > > @@ -0,0 +1,10 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-Wpadded" } */
> > > +
> > > +/*
> > > + * The struct is on single line, because C++ compiler emits the -Wpadded
> > > + * warning at the first line of the struct, while the C compiler at the 
> > > last
> > > + * line of the struct definition. This way the test passes on both
> > > + */
> > > +struct S { __UINT32_TYPE__ i; char c; }; /* { dg-warning "padding struct 
> > > size to alignment boundary with 3 bytes" } */
> > > +
> > Note the testcase will fail on some targets where alignment is 1 for 
> > everything.
> > You most likely want the dg-warning to be like it is in gcc.dg/Wpadded.c:
> > /* { dg-warning "padding struct size to alignment boundary with 3
> > bytes" ""  { target { ! default_packed } } } */
> >
> > You might want the following from the same file too:
> > /* -fpack-struct is necessary because the warning expected requires the 
> > initial
> >packing to be larger than 1, which cannot be guaranteed for all targets.
> >We won't get a warning anyway if the target has "packed" structure
> >layout.  */
> > /* { dg-options "-Wpadded -fpack-struct=8" } */
> > /* { dg-additional-options "-mno-ms-bitfields" { target *-*-mingw* } } */
> I added the ! default_packed directive, but I am not sure whether the
> -fpack-struct is needed. Could you please provide a name of the particular 
> target
> with such alignment constraints so I can test it?

cris is one example. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=23304 (which is why
default_packed was added).

Thanks,
Andrew


>
> --
> Thank you,
> Vit Kabele


Re: [PATCH] libstdc++: 60241.cc: tolerate slightly shorter aggregate sleep

2022-06-22 Thread Sebastian Huber

On 22/06/2022 08:22, Sebastian Huber wrote:

On 22/06/2022 08:01, Alexandre Oliva via Gcc-patches wrote:


On rtems under qemu, the frequently-interrupted nanosleep ends up
sleeping shorter than expected, by a margin of less than 0,3%.

I figured failing the library test over a system (emulator?) bug is
undesirable, so I put in some tolerance for the drift.

Regstrapped on x86_64-linux-gnu, also tested with a cross to
aarch64-rtems6.  Ok to install?

PS: I see nothing wrong with the implementation of clock_nanosleep (used
by nanosleep) on rtems6 that could cause it to wake up too early.  I
suspect some artifact of the emulation environment.


for  libstdc++-v3/ChangeLog

* testsuite/30_threads/this_thread/60421.cc: Tolerate a
slightly early wakeup.
---
  .../testsuite/30_threads/this_thread/60421.cc  |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc 
b/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc

index 12dbeba1cc492..f3a5af453c4ad 100644
--- a/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc
+++ b/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc
@@ -51,9 +51,10 @@ test02()
    std::thread t([&result, &sleeping] {
  auto start = std::chrono::system_clock::now();
  auto time = std::chrono::seconds(3);
+    auto tolerance = std::chrono::milliseconds(10);
  sleeping = true;
  std::this_thread::sleep_for(time);
-    result = std::chrono::system_clock::now() >= (start + time);
+    result = std::chrono::system_clock::now() + tolerance >= (start + 
time);

  sleeping = false;
    });
    while (!sleeping)


This looks like a bug in RTEMS or the BSP for the test platform. I would 
first investigate this and then change the test which looks all right to 
me.


This is a problem in RTEMS. RTEMS uses the FreeBSD timecounters to 
maintain CLOCK_REALTIME and provides two methods to get the time in a 
coarse and fine resolution. The std::chrono::system_clock::now() uses 
the fine resolution (higher overhead). The clock_nanosleep() uses the 
coarse resolution which may give a time before now().


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


Re: [PATCH] libstdc++: testsuite: use -lbsd for net_ts on RTEMS

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 06:25, Alexandre Oliva via Libstdc++
 wrote:
>
>
> Networking functions that net_ts tests rely on are defined in libbsd
> on RTEMS, so link with it.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK, thanks.



Re: [PATCH] libstdc++: testsuite: tolerate non-cancelling sleep

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 06:26, Alexandre Oliva via Libstdc++
 wrote:
>
>
> Though sleep, nanosleep and clock_nanosleep are all POSIX cancellation
> points, not all target systems follow this POSIX requirement.
> 30_threads/thread/native_handle/cancel.cc will run until it times out
> on such systems.
>
> Rather than failing a C++ library test because of a limitation of the
> target system, this patch gives the test a chance to successfully
> exercise the features it intends to exercise, by introducing a
> cancellation point in a loop that would otherwise run indefinitely on
> systems exhibiting this limitation.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK, thanks.

This test is already explicitly pthread-specific, so there's no
problem using pthread_testcancel there. It already uses
pthread_cancel, and is gated by:
// { dg-require-effective-target pthread }



Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:05, Alexandre Oliva via Libstdc++
 wrote:
>
>
> This patch was originally meant to reduce the likelihood that
> nonexistent_path() returns the same pathname for from and to.
>
> It was prompted by a target system with a non-random implementation of
> mkstemp, that returns a predictable sequence of filenames and selects
> the first one that isn't already taken.
>
> That turned out not to be enough: nonexistent_path adds a suffix to
> the filename chosen by mkstemp and removes the file it created, so
> mkstemp may very well insist on the same basename, and the case that
> doesn't use mkstemp doesn't even check whether the file already
> exists.
>
> Anyway, by the time I realized this wasn't enough, I'd already
> implemented some of the changes, and I figured I might as well
> contribute them, even though they don't really solve any problem, and
> even if they did, they'd be just a partial solution.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK



Re: [PATCH] libstdc++: testsuite: test symlnks ifdef _GLIBCXX_HAVE_SYMLINK

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:14, Alexandre Oliva via Libstdc++
 wrote:
>
>
> Several filesystem tests expect to be able to create symlinks even
> when !defined (_GLIBCXX_HAVE_SYMLINK), and fail predictably, reducing
> the amount of testing of other filesystem features.
>
> They are already skipped for mingw targets.  I've extended the
> skipping to other targets in which _GLIBCXX_HAVE_SYMLINK is
> undefined.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK.

I'd like to clean this up so the tests don't rely on the "internal"
HAVE_SYMLINK macro. We could add something like this to
testsuite/util/testsuite_fs.h

#if defined(__MINGW32__) || defined(__MINGW64__) \
  || !defined (_GLIBCXX_HAVE_SYMLINK)
# define NO_SYMLINKS
#endif

and then use that in the tests. That way the private macro is only
checked in one place. We can do that later though.

>
> PS: Testing with trunk was somewhat impaired by various changes in the
> filesystem implementation and tests that cause new failures on rtems6

The only significant changes are for PR104161 but the directory
iterators did change fairly significantly.

Which tests are failing? I might be able to point you to the cause
much faster than you can debug it yourself.



Re: [PATCH] libstdc++: retry removal of dir entries if dir removal fails

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:20, Alexandre Oliva via Libstdc++
 wrote:
>
>
> On some target systems (e.g. rtems6.0), removing directory components
> while iterating over directory entries may cause some of the directory
> entries to be skipped, which prevents the removal of the parent
> directory from succeeding.

This probably comes as no surprise to you, but for the record I don't
think that's POSIX-confirming. The spec for readdir says:

"If a file is removed from or added to the directory after the most
recent call to opendir() or rewinddir(), whether a subsequent call to
readdir() returns an entry for that file is unspecified."

This implies to me that all existing directory entries should get
visited, even if you remove some.


>
> Advancing the iterator before removing a member proved not to be
> enough, so I've instead arranged for remove_all to retry the removal
> of components if the removal of the parent dir fails after removing at
> least one entry.  The fail will be permanent only if no components got
> removed in the current try.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

I haven't properly reviewed it yet, just commenting for now.

It looks like it would be possible for this to livelock. If another
process keeps writing to the directory tree you're trying to remove,
it will keep removing some entries (increasing the count) but also
keep failing to remove a non-empty directory. The current
implementation will fail with an error in that case, rather than
getting stuck forever in a loop.

We could add a max-retries limit, although that would presumably mean
the original problem remains for rtems if trying to remove a directory
with a huge number of entries.

>
> PS: The implementation of remove_all has changed completely, compared

That's PR104161, and in the r12-7062 commit I wrote:

It would be possible to add a __rewind member to directory iterators
too, to call rewinddir after each modification to the directory. That
would make it more likely for filesystem::remove_all to successfully
remove everything even if files are being written to the directory tree
while removing it. It's unclear if that is actually prefereable, or if
it's better to fail and report an error at the first opportunity.

I was thinking of cases where there are concurrent modifications to
the directory while we're trying to remove it, but the RTEMS case of
skipping subsequent entries is more motivating.

The rewind operation would reopen the current directory stream, but
without creating a new directory iterator and starting again from the
very top of the directory tree. For the non-recursive directory
iterator there's no real difference, but for the recursive one it
would only rewind at the current nesting depth, not restart from the
top of the tree. I also need to consider the interaction with symlink
races, and whether retrying can bring back the vulnerability that
PR104161 addressed (I think the way you've done it doesn't have the
same race condition, but would be susceptible to a different race
condition where the top of the directory tree to be removed is
different when you retry).



> with the gcc-11 environment in which the need for this patch came up.  I
> have reimplemented it for mainline, and I have attempted to test it in
> this environment, but new filesystem tests and subtests that fail on
> rtems6.0 have impaired testing and prevented the full pass rate I got
> for them with a similar patchset on gcc-11.
>
>
> for  libstdc++-v3/ChangeLog
>
> * src/c++17/fs_ops.cc (remove_all): Retry removal of
> directory entries.
> ---
>  libstdc++-v3/src/c++17/fs_ops.cc |   22 --
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/src/c++17/fs_ops.cc 
> b/libstdc++-v3/src/c++17/fs_ops.cc
> index 435368fa5c5ff..b3390310132b4 100644
> --- a/libstdc++-v3/src/c++17/fs_ops.cc
> +++ b/libstdc++-v3/src/c++17/fs_ops.cc
> @@ -1286,6 +1286,8 @@ fs::remove_all(const path& p)
>  {
>error_code ec;
>uintmax_t count = 0;
> + retry:
> +  uintmax_t init_count = count;
>recursive_directory_iterator dir(p, directory_options{64|128}, ec);
>switch (ec.value()) // N.B. assumes ec.category() == 
> std::generic_category()
>{
> @@ -1303,7 +1305,7 @@ fs::remove_all(const path& p)
>  break;
>case ENOENT:
>  // Our work here is done.
> -return 0;
> +return count;
>case ENOTDIR:
>case ELOOP:
>  // Not a directory, will remove below.
> @@ -1313,6 +1315,18 @@ fs::remove_all(const path& p)
>  _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
>}
>
> +  if (count > init_count)
> +{
> +  if (int last = fs::remove(p, ec); !ec)
> +   return count + last;
> +  else
> +   // Some systems seem to skip entries in the dir iteration if
> +   // you remove dir entries while iterating, so if we removed
> +   // anyt

Re: [PATCH] libstdc++: testsuite: skip fs space tests if not available

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:28, Alexandre Oliva via Libstdc++
 wrote:
>
>
> The do_space function is defined in ways that are useful, or that fail
> immediately, depending on various macros.  When it fails immediately,
> the filesystem space.cc tests fail noisily, but the fail is entirely
> expected.
>
> Define HAVE_SPACE in the space.cc tests, according to the macros that
> select implementations of do_space, and use it to skip tests that are
> expected to fail.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK.

This could be done once in testsuite_fs.h as well.



Re: [PATCH] libstdc++-v3: check for openat

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:43, Alexandre Oliva via Libstdc++
 wrote:
>
>
> rtems6.0 has fdopendir, and fcntl.h defines AT_FDCWD and declares
> openat, but there's no openat in libc.  Adjust dir-common.h to not
> assume ::openat just because of AT_FDCWD.
>
> Regstrapped on x86_64-linux-gnu (detects and still uses openat), also
> tested with a cross to aarch64-rtems6 (detects openat's absence and
> refrains from using it).  Ok to install?
>
> PS: This is the last patch in my rtems6.0 patchset, and the only patch
> for the filesystem-related patchset that was written specifically for a
> mainline gcc.  gcc-11 did not attempt to use openat.  This patch enabled
> filesystem tests to link when testing mainline on aarch64-rtems6.0.
> Alas, several filesystem tests still failed with it, in ways that AFAICT
> are not related with the use of openat, or with the other patches I've
> posted.  However, I'm not able to look into the remaining failures right
> now.


There are other interactions between AT_CDCWD and ::openat not covered
by this patch. I this this also needs to check HAVE_OPENAT:

  // Return a file descriptor for the directory and current entry's path.
  // If dirfd is available, use it and return only the filename.
  // Otherwise, return AT_FDCWD and return the full path.
  pair
  dir_and_pathname() const noexcept
  {
const fs::path& p = entry.path();
#if _GLIBCXX_HAVE_DIRFD
if (!p.empty())
  return {::dirfd(this->dirp), std::prev(p.end())->c_str()};
#endif
return {this->fdcwd(), p.c_str()};
  }

Otherwise, if rterms defines HAVE_DIRFD this function will return a
file descriptor and a filename (not a full path) but then
_Dir_base::openat doesn't use ::openat and so ignores the file
descriptor, and needs a full path. Or maybe instead of changing
dir_and_pathname() (because that's used with both openat and unlinkat)
we should change _Dir::open_subdir like so:

--- a/libstdc++-v3/src/c++17/fs_dir.cc
+++ b/libstdc++-v3/src/c++17/fs_dir.cc
@@ -136,7 +136,12 @@ struct fs::_Dir : _Dir_base
  open_subdir(bool skip_permission_denied, bool nofollow,
 error_code& ec) const noexcept
  {
+#if _GLIBCXX_HAVE_OPENAT
auto [dirfd, pathname] = dir_and_pathname();
+#else
+int dirfd = -1;
+const char* pathname = entry.path().c_str();
+#endif
_Dir_base d(dirfd, pathname, skip_permission_denied, nofollow, ec);
// If this->path is empty, the new _Dir should have an empty path too.
const fs::path& p = this->path.empty() ? this->path : this->entry.path();

This will ensure we pass a full path to _Dir_base::openat when it needs one.

I'm a bit sad that this breaks the abstraction, so that the derived
class needs to know how the base class is implemented, but that
coupling is actually already implicitly present (which is what you're
trying to fix). Maybe a cleaner solution is for the _Dir_base to take
both a filename *and* a full pathname, and then decide which to use in
_Dir_base::openat. So the derived class would provide both, and not
care how the base class chooses to use them.





>
>
> for  libstdc++-v3/ChangeLog
>
> * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Check for
> openat.
> * aclocal.m4, configure, config.h.in: Rebuilt.
> * src/filesystem/dir-common.h (openat): Use ::openat if
> _GLIBCXX_HAVE_OPENAT.
> ---
>  libstdc++-v3/acinclude.m4|   12 +++
>  libstdc++-v3/config.h.in |3 ++
>  libstdc++-v3/configure   |   55 
> ++
>  libstdc++-v3/src/filesystem/dir-common.h |2 +
>  4 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> index 138bd58d86cb9..e3cc3a8e867d3 100644
> --- a/libstdc++-v3/acinclude.m4
> +++ b/libstdc++-v3/acinclude.m4
> @@ -4772,6 +4772,18 @@ dnl
>if test $glibcxx_cv_dirfd = yes; then
>  AC_DEFINE(HAVE_DIRFD, 1, [Define if dirfd is available in .])
>fi
> +dnl
> +  AC_CACHE_CHECK([for openat],
> +glibcxx_cv_openat, [dnl
> +GCC_TRY_COMPILE_OR_LINK(
> +  [#include ],
> +  [int fd = ::openat(AT_FDCWD, "", 0);],
> +  [glibcxx_cv_openat=yes],
> +  [glibcxx_cv_openat=no])
> +  ])
> +  if test $glibcxx_cv_openat = yes; then
> +AC_DEFINE(HAVE_OPENAT, 1, [Define if openat is available in .])
> +  fi
>  dnl
>AC_CACHE_CHECK([for unlinkat],
>  glibcxx_cv_unlinkat, [dnl
> diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
> index f30a8c51c458c..2a3972eef5412 100644
> --- a/libstdc++-v3/config.h.in
> +++ b/libstdc++-v3/config.h.in
> @@ -292,6 +292,9 @@
>  /* Define if  defines obsolete isnan function. */
>  #undef HAVE_OBSOLETE_ISNAN
>
> +/* Define if openat is available in . */
> +#undef HAVE_OPENAT
> +
>  /* Define if poll is available in . */
>  #undef HAVE_POLL
>
> diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
> index 9b94fd71e4248..eac6039212168 100755
> --- a/libstdc++-v3/configure
> +++ b/l

Re: [PATCH] libstdc++: testsuite: fs rename to self may fail

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 08:02, Alexandre Oliva via Libstdc++
 wrote:
>
> Hello, Sebastian,
>
> On Jun 22, 2022, Sebastian Huber  wrote:
>
> > On 22/06/2022 08:24, Alexandre Oliva via Libstdc++ wrote:
> >> rtems6's rename() implementation errors with EEXIST when the rename-to
> >> filename exists, even when renaming a file to itself or when renaming
> >> a nonexisting file.  Adjust expectations.
> >>
> >> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> >> aarch64-rtems6.  Ok to install?
> >>
> >> PS:https://devel.rtems.org/ticket/2169  doesn't seem to suggest plans to
> >> change behavior so as to comply with POSIX.
>
> > I would not adjust the test case to cope with systems which are not in
> > line with POSIX.
>
> My understanding is that the libstdc++ testsuite is not meant to test
> for POSIX conformance, but for conformance with the C++ language
> standards.
>
> C++ inherits rename from C, and C says the behavior is implementation
> defined if the new name already exists.

std::filesystem::rename is explicitly specified in terms of POSIX
rename, not C rename. POSIX says:

"If the old argument and the new argument resolve to either the same
existing directory entry or different directory entries for the same
existing file, rename() shall return successfully and perform no other
action." and "If the link named by the new argument exists, it shall
be removed and old renamed to new."

So I agree with Sebastian, the tests are correct.

Instead, the implementation of std::filesystem::rename should have a
special-case for rtems (and maybe other targets) that implements the
POSIX rename semantics if calling ::rename isn't good enough.


>
> RTEMS is thus comformant with the requirements from C (and thus C++),
> and it is therefore reasonable for libstdc++'s testsuite to accept
> RTEMS' behavior as such.
>
>
> That said, because libstdc++ tests are all-or-nothing, perhaps it would
> make sense to have a separate test for strict POSIX conformance in
> rename, XFAILed on RTEMS targets.  How about that?
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 
>



Re: [PATCH] libstdc++: testsuite: test symlnks ifdef _GLIBCXX_HAVE_SYMLINK

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 10:25, Jonathan Wakely wrote:
>
> On Wed, 22 Jun 2022 at 07:14, Alexandre Oliva via Libstdc++
>  wrote:
> >
> >
> > Several filesystem tests expect to be able to create symlinks even
> > when !defined (_GLIBCXX_HAVE_SYMLINK), and fail predictably, reducing
> > the amount of testing of other filesystem features.
> >
> > They are already skipped for mingw targets.  I've extended the
> > skipping to other targets in which _GLIBCXX_HAVE_SYMLINK is
> > undefined.
> >
> > Regstrapped on x86_64-linux-gnu, also tested with a cross to
> > aarch64-rtems6.  Ok to install?
>
> OK.

P.S. there's a typo in the Subject: "symlnks" not "symlinks". I don't
know if you intend to use that as the Git commit summary line.


>
> I'd like to clean this up so the tests don't rely on the "internal"
> HAVE_SYMLINK macro. We could add something like this to
> testsuite/util/testsuite_fs.h
>
> #if defined(__MINGW32__) || defined(__MINGW64__) \
>   || !defined (_GLIBCXX_HAVE_SYMLINK)
> # define NO_SYMLINKS
> #endif
>
> and then use that in the tests. That way the private macro is only
> checked in one place. We can do that later though.
>
> >
> > PS: Testing with trunk was somewhat impaired by various changes in the
> > filesystem implementation and tests that cause new failures on rtems6
>
> The only significant changes are for PR104161 but the directory
> iterators did change fairly significantly.
>
> Which tests are failing? I might be able to point you to the cause
> much faster than you can debug it yourself.



Re: [PATCH] libstdc++: testsuite: skip fs last_write_time tests if not available

2022-06-22 Thread Jonathan Wakely via Gcc-patches
On Wed, 22 Jun 2022 at 07:35, Alexandre Oliva via Libstdc++
 wrote:
>
>
> The last_write_time functions are defined in ways that are useful, or
> that fail immediately, depending on various macros.  When they fail
> immediately, the filesystem last_write_time.cc tests fail noisily, but
> the fail is entirely expected.
>
> Define HAVE_LWT in the last_write_time.cc tests, according to the
> macros that select implementations of last_write_time, and use it to
> skip tests that are expected to fail.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK

> PS: I realize _GLIBCXX_HAVE_SYS_STAT_H is tested for in two different
> ways in the #if expressions added to the tests.  This mirrors the
> different uses in the do_stat template body, and in
> fs::last_write_time(const path&, file_time_type, error_code&).  Perhaps
> they should all be using either value or definedness, but I didn't want
> to go there, at least not at first, so I retained the apparent
> inconsistency.

Yes, they should be tested consistently in the libstdc++ sources.

And again, this could be a macro defined in testsuite_fs.h



Re: [PATCH] ipa-icf: skip variables with body_removed

2022-06-22 Thread Jan Hubicka via Gcc-patches
> @Honza: PING
> 
> On 5/20/22 09:46, Martin Liška wrote:
> > On 5/19/22 17:02, Jan Hubicka wrote:
> >>> Similarly to cgraph_nodes, it may happen that body_removed is set
> >>> during merging of symbols.
> >>>
> >>>   PR ipa/105600
> >>>
> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>>
> >>> Ready to be installed?
> >>> Thanks,
> >>> Martin
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>>   * ipa-icf.cc (sem_item_optimizer::filter_removed_items):
> >>>   Skip variables with body_removed.
> >>> ---
> >>>  gcc/ipa-icf.cc | 7 ---
> >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
> >>> index 765ae746745..6528a7a10b2 100644
> >>> --- a/gcc/ipa-icf.cc
> >>> +++ b/gcc/ipa-icf.cc
> >>> @@ -2411,10 +2411,11 @@ sem_item_optimizer::filter_removed_items (void)
> >>>   {
> >>> /* Filter out non-readonly variables.  */
> >>> tree decl = item->decl;
> >>> -   if (TREE_READONLY (decl))
> >>> - filtered.safe_push (item);
> >>> -   else
> >>> +   varpool_node *vnode = static_cast  >>> *>(item)->get_node ();
> >>> +   if (!TREE_READONLY (decl) || vnode->body_removed)
> >>>   remove_item (item);
> >>> +   else
> >>> + filtered.safe_push (item);
> >>
> >> So the situation here is that we merge symbols but keep syntactic alias
> >> because the declarations are not compatible (have different attributes
> >> perhaps because of fortify source)?
> > 
> > The test-case is more about a constexpr symbol or so, I'm not familiar 
> > enough
> > with these modern C++.
> > 
> > cgraph_node looks like:
> > 
> > (gdb) p item->node->debug()
> > _ZN8nlohmann6detail12static_constINS0_10to_json_fnEE5valueE/10 (value) 
> > @0x77fb3200
> >   Type: variable
> >   Body removed by symtab_remove_unreachable_nodes
> >   Visibility: externally_visible semantic_interposition preempted_reg 
> > external public weak comdat 
> > comdat_group:_ZN8nlohmann6detail12static_constINS0_10to_json_fnEE5valueE 
> > one_only
> >   References: 
> >   Referring: 
> > _Z7to_jsonRN8nlohmann10basic_jsonISt3mapSt6vectorNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEblmiSaNS_14adl_serializerES2_IhSaIhRK8Settings/1
> >  (addr) 
> > _Z7to_jsonRN8nlohmann10basic_jsonISt3mapSt6vectorNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEblmiSaNS_14adl_serializerES2_IhSaIhRK8Settings/1
> >  (addr) 
> >   Read from file: a.o
> >   Availability: not_available
> >   Varpool flags: initialized read-only

OK, I think I worked out what happens here.  The thing is that the
symbol has "external" flag set.  In my mental picture we should be
keeping their definitions around since they are used for constant
folding, but it seems it is not what happens. We special case inline
functions but we do not special case external symbols.

This is a missed optimization oppurtunity but not completely trivial to
fix, since external symbol may reffer to internal one (such as comdat)
but even if it does so we want to optimize out that comdat.  So we need
some bookkeeping for referneces from external symbols.  I will implement
that but I guess we still want your patch since this is optional
optimization.

So patch is OK.
Honza
> > 
> > 
> >>
> >> Will ICF still see through the aliases and do merging?
> > 
> > No.
> > 
> >> I think you can
> >> craft a testcase by triggering the attribute mismatch and see what
> >> happens.  At the time we implemented ICF these aliases did not exists,
> >> so maybe some TLC is needed here.
> > 
> > Please come up with such test case :)
> > 
> > Thanks,
> > Martin
> > 
> >>
> >> Honza
> > 
> 


[x86 PATCH] PR target/105930: Split *xordi3_doubleword after reload.

2022-06-22 Thread Roger Sayle

This patch addresses PR target/105930 which is an ia32 stack frame size
regression in high-register pressure XOR-rich cryptography functions
reported by Linus Torvalds.  The underlying problem is once the limited
number of registers on the x86 are exhausted, the register allocator
has to decide which to spill, where some eviction choices lead to much
poorer code, but these consequences are difficult to predict in advance.

The patch below, which splits xordi3_doubleword and iordi3_doubleword
after reload (instead of before), significantly reduces the amount of
spill code and stack frame size, in what might appear to be an arbitrary
choice.

My explanation of this behaviour is that the mixing of pre-reload split
SImode instructions and post-reload split DImode instructions is
confusing some of the heuristics used by reload.  One might think
that splitting early gives the register allocator more freedom to
use available registers, but in practice the constraint that double
word values occupy consecutive registers (when ultimately used as a
DImode value) is the greater constraint.  Instead, I believe in this
case, the pseudo registers used in memory addressing, appear to be
double counted for split SImode instructions compared to unsplit
DImode instructions.  For the reduced test case in comment #13, this
leads to %eax being used to hold the long-lived argument pointer "v",
blocking the use of the ax:dx pair for processing double word values.
The important lines are at the very top of the assembly output:

GCC 11  [use %ecx to address memory, require a 24-byte stack frame]
sub esp, 24
mov ecx, DWORD PTR [esp+40]

GCC 12 [use %eax to address memory, require a 44-byte stack frame]
sub esp, 44
mov eax, DWORD PTR [esp+64]

Jakub's alternative proposed patch in comment #17 is to improve
consistency by splitting more instructions (rotates) before reload,
which shows a small improvement, but not to GCC v11 levels.

I have some follow-up patches (based on other approaches I've tried),
including splitting rotations by 32 after reload, and supporting
TImode operations via , notice this same pattern is mentioned in
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596201.html
but this patch below is the core minimal fix that's hopefully
suitable for benchmarking and possibly backporting to the 12 branch.
I believe that changes to the register allocator itself, to tweak how
stack slots are assigned and which values can be cheaply materialized
are out-of-scope for the release branches.


I'm curious what folks (especially Uros and Jakub) think?
This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32},
with no new failures.  CSiBE also shows a (minor) code size reduction.
It would be great if someone could benchmark this patch, or alternatively
it can be baked on mainline to let the automatic benchmarking evaluate it,
then revert the patch if there are any observed performance issues.

Thoughts?


2022-06-22  Roger Sayle  

gcc/ChangeLog
PR target/105930
* config/i386/i386.md (*di3_doubleword): Split after
reload.  Use rtx_equal_p to avoid creating memory-to-memory moves,
and emit NOTE_INSN_DELETED if operand[2] is zero (i.e. with -O0).
(define_insn _1): Renamed from *mode_1
to provide gen_si_1 functions.

Roger
--

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 3093cb5..537fba21 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -10539,48 +10539,61 @@
   "ix86_expand_binary_operator (, mode, operands); DONE;")
 
 (define_insn_and_split "*di3_doubleword"
-  [(set (match_operand:DI 0 "nonimmediate_operand")
+  [(set (match_operand:DI 0 "nonimmediate_operand" "=ro,r")
(any_or:DI
-(match_operand:DI 1 "nonimmediate_operand")
-(match_operand:DI 2 "x86_64_szext_general_operand")))
+(match_operand:DI 1 "nonimmediate_operand" "0,0")
+(match_operand:DI 2 "x86_64_szext_general_operand" "re,o")))
(clobber (reg:CC FLAGS_REG))]
   "!TARGET_64BIT
-   && ix86_binary_operator_ok (, DImode, operands)
-   && ix86_pre_reload_split ()"
+   && ix86_binary_operator_ok (, DImode, operands)"
   "#"
-  "&& 1"
+  "&& reload_completed"
   [(const_int 0)]
 {
+  /* This insn may disappear completely when operands[2] == const0_rtx
+ and operands[0] == operands[1], which requires a NOTE_INSN_DELETED.  */
+  bool emit_insn_deleted_note_p = false;
+
   split_double_mode (DImode, &operands[0], 3, &operands[0], &operands[3]);
 
   if (operands[2] == const0_rtx)
-emit_move_insn (operands[0], operands[1]);
+{
+  if (!rtx_equal_p (operands[0], operands[1]))
+   emit_move_insn (operands[0], operands[1]);
+  else
+   emit_insn_deleted_note_p = true;
+}
   else if (operands[2] == constm1_rtx)
 {
   if ( == IOR)
emit_move_insn (operands[0], constm1_rtx);
   

Re: [PATCH] Inline memchr with a small constant string

2022-06-22 Thread Richard Biener via Gcc-patches
On Tue, Jun 21, 2022 at 11:03 PM H.J. Lu via Gcc-patches
 wrote:
>
> When memchr is applied on a constant string of no more than the bytes of
> a word, inline memchr by checking each byte in the constant string.
>
> int f (int a)
> {
>return  __builtin_memchr ("eE", a, 2) != 0;
> }
>
> is simplified to
>
> int f (int a)
> {
>   return (char) a == 'e' || (char) a == 'E';
> }
>
> gcc/
>
> PR tree-optimization/103798
> * match.pd (__builtin_memchr (const_str, a, N)): Inline memchr
> with constant strings of no more than the bytes of a word.

Please do this in strlenopt or so, with match.pd you will end up moving
the memchr loads across possible aliasing stores to the point of the
comparison.

Richard.

> gcc/testsuite/
>
> PR tree-optimization/103798
> * c-c++-common/pr103798-1.c: New test.
> * c-c++-common/pr103798-2.c: Likewise.
> * c-c++-common/pr103798-3.c: Likewise.
> * c-c++-common/pr103798-4.c: Likewise.
> * c-c++-common/pr103798-5.c: Likewise.
> * c-c++-common/pr103798-6.c: Likewise.
> * c-c++-common/pr103798-7.c: Likewise.
> * c-c++-common/pr103798-8.c: Likewise.
> ---
>  gcc/match.pd| 136 
>  gcc/testsuite/c-c++-common/pr103798-1.c |  28 +
>  gcc/testsuite/c-c++-common/pr103798-2.c |  30 ++
>  gcc/testsuite/c-c++-common/pr103798-3.c |  28 +
>  gcc/testsuite/c-c++-common/pr103798-4.c |  28 +
>  gcc/testsuite/c-c++-common/pr103798-5.c |  26 +
>  gcc/testsuite/c-c++-common/pr103798-6.c |  27 +
>  gcc/testsuite/c-c++-common/pr103798-7.c |  27 +
>  gcc/testsuite/c-c++-common/pr103798-8.c |  27 +
>  9 files changed, 357 insertions(+)
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-2.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-3.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-4.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-5.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-6.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-7.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr103798-8.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index a63b649841b..aa4766749af 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -7976,3 +7976,139 @@ and,
>  (match (bitwise_induction_p @0 @2 @3)
>   (bit_not
>(nop_convert1? (bit_xor@0 (convert2? (lshift integer_onep@1 @2)) @3
> +
> +#if GIMPLE
> +/* __builtin_memchr (const_str, a, N) != 0 ->
> +   a == const_str[0] .. || a == const_str[N-1]
> +   __builtin_memchr (const_str, a, N) == 0 ->
> +   a != const_str[0] .. && a != const_str[N-1]
> +   where N is less than the string size.  */
> +(for cmp (eq ne)
> + icmp (ne eq)
> + bit_op (bit_and bit_ior)
> + (simplify (cmp:c @0 (BUILT_IN_MEMCHR ADDR_EXPR@1 @2 INTEGER_CST@3))
> +  (if (UNITS_PER_WORD <= 8
> +   && CHAR_TYPE_SIZE == 8
> +   && BITS_PER_UNIT == 8
> +   && CHAR_BIT == 8
> +   && integer_zerop (@0)
> +   && !integer_zerop (@3)
> +   && TREE_CODE (TREE_OPERAND (@1, 0)) == STRING_CST
> +   && TREE_STRING_LENGTH (TREE_OPERAND (@1, 0)) >= 2
> +   && wi::leu_p (wi::to_wide (@3), UNITS_PER_WORD)
> +   && wi::ltu_p (wi::to_wide (@3),
> +TREE_STRING_LENGTH (TREE_OPERAND (@1, 0
> +   (with
> +{
> +  const char *p = TREE_STRING_POINTER (TREE_OPERAND (@1, 0));
> +  unsigned HOST_WIDE_INT size = TREE_INT_CST_LOW (@3);
> +}
> +(switch
> + (if (size == 1)
> +  (icmp (convert:char_type_node @2)
> +   { build_int_cst (char_type_node, p[0]); }))
> + (if (size == 2)
> +  (bit_op
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[0]); })
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[1]); })))
> + (if (size == 3)
> +  (bit_op
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[0]); })
> +   (bit_op
> +(icmp (convert:char_type_node @2)
> + { build_int_cst (char_type_node, p[1]); })
> +(icmp (convert:char_type_node @2)
> + { build_int_cst (char_type_node, p[2]); }
> + (if (size == 4)
> +  (bit_op
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[0]); })
> +   (bit_op
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[1]); })
> +   (bit_op
> +(icmp (convert:char_type_node @2)
> +  { build_int_cst (char_type_node, p[2]); })
> +(icmp (convert:char_type_node @2)
> +  { build_int_cst (char_type_node, p[3]); })
> + (if (size == 5)
> +  (bit_op
> +   (icmp (convert:char_type_node @2)
> +{ build_int_cst (char_type_node, p[0]); })
> +   (bit_op
> +  

[PATCH v2] c: Extend the -Wpadded message with actual padding size

2022-06-22 Thread Vit Kabele
Hello,
I added the ! default_packed directive, and now the test is properly
skipped on the targets with that property. I tested with cris-elf
target and the test behaves properly.

Best regards,
Vit Kabele

-- >8 --
Subject: [PATCH v2] c: Extend the -Wpadded message with actual padding size

When the compiler warns about padding struct to alignment boundary, it
now also informs the user about the size of the alignment that needs to
be added to get rid of the warning.

This removes the need of using pahole or similar tools, or manually
determining the padding size.

Tested for x86_64-pc-linux-gnu and cris-elf targets.

gcc/ChangeLog:

* stor-layout.cc (finalize_record_size): Extend warning message.

gcc/testsuite/ChangeLog:

* c-c++-common/Wpadded.c: New test.

Signed-off-by: Vit Kabele 
---
 gcc/stor-layout.cc   |  7 ++-
 gcc/testsuite/c-c++-common/Wpadded.c | 13 +
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wpadded.c

diff --git a/gcc/stor-layout.cc b/gcc/stor-layout.cc
index 765f22f68b9..88923c4136b 100644
--- a/gcc/stor-layout.cc
+++ b/gcc/stor-layout.cc
@@ -1781,7 +1781,12 @@ finalize_record_size (record_layout_info rli)
   && simple_cst_equal (unpadded_size, TYPE_SIZE (rli->t)) == 0
   && input_location != BUILTINS_LOCATION
   && !TYPE_ARTIFICIAL (rli->t))
-warning (OPT_Wpadded, "padding struct size to alignment boundary");
+  {
+   tree pad_size
+ = size_binop (MINUS_EXPR, TYPE_SIZE_UNIT (rli->t), 
unpadded_size_unit);
+ warning (OPT_Wpadded,
+   "padding struct size to alignment boundary with %E bytes", 
pad_size);
+  }
 
   if (warn_packed && TREE_CODE (rli->t) == RECORD_TYPE
   && TYPE_PACKED (rli->t) && ! rli->packed_maybe_necessary
diff --git a/gcc/testsuite/c-c++-common/Wpadded.c 
b/gcc/testsuite/c-c++-common/Wpadded.c
new file mode 100644
index 000..9e7e9f240c1
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wpadded.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wpadded" } */
+
+/*
+ * The struct is on single line, because C++ compiler emits the -Wpadded
+ * warning at the first line of the struct definition, while the C compiler at
+ * the last line. This way the test passes on both.
+ * 
+ * The test is skipped on targets where default behavior is to pack the
+ * structs because there is no warning triggered.
+ */
+struct S { __UINT32_TYPE__ i; char c; } foo; /* { dg-warning "padding struct 
size to alignment boundary with 3 bytes" "" { target { ! default_packed } } } */
+
-- 
2.30.2


c++: Remove ifdefed code

2022-06-22 Thread Nathan Sidwell via Gcc-patches

The only reason I chose to use DECL_UID on this hash table was to make
it stable against ASLR and perturbations due to other allocations.
It's not required for correctness, as the comment mentions the
equality fn uses pointer identity.

nathan

--
Nathan SidwellFrom d844478ab47a16c8ae65f253fd1cdc685c7951fc Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Wed, 22 Jun 2022 07:51:44 -0700
Subject: [PATCH] c++: Remove ifdefed code

The only reason I chose to use DECL_UID on this hash table was to make
it stable against ASLR and perturbations due to other allocations.
It's not required for correctness, as the comment mentions the
equality fn uses pointer identity.

	gcc/cp/
	* module.cc (struct duplicate_hash): Remove.
	(duplicate_hash_map): Adjust.
---
 gcc/cp/module.cc | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index b3fbd467ecb..d735d7e8b30 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2829,24 +2829,10 @@ struct merge_key {
   }
 };
 
-struct duplicate_hash : nodel_ptr_hash
-{
-#if 0
-  /* This breaks variadic bases in the xtreme_header tests.  Since ::equal is
- the default pointer_hash::equal, let's use the default hash as well.  */
-  inline static hashval_t hash (value_type decl)
-  {
-if (TREE_CODE (decl) == TREE_BINFO)
-  decl = TYPE_NAME (BINFO_TYPE (decl));
-return hashval_t (DECL_UID (decl));
-  }
-#endif
-};
-
 /* Hashmap of merged duplicates.  Usually decls, but can contain
BINFOs.  */
 typedef hash_map >
+		 simple_hashmap_traits,uintptr_t> >
 duplicate_hash_map;
 
 /* Tree stream reader.  Note that reading a stream doesn't mark the
-- 
2.30.2



[committed] d: Merge upstream dmd 6203135dc, druntime e150cca1, phobos a4a18d21c.

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

This patch merges the D front-end with upstream dmd 6203135dc, and the D
run-time library with upstream druntime e150cca1, and phobos a4a18d21c.

D front-end changes:

- Input parameters can now be applied on extern(C++) functions to
  bind to `const &' when the `-fpreview=in' flag is in effect.

D runtime changes:

- Run-time flag `--DRT-oncycle=deprecate' has been removed.

Phobos changes:

- Removed std.experimental.logger's capability to set the minimal
  LogLevel at compile time.

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 6203135dc.
* typeinfo.cc (TypeInfoVisitor::visit (TypeInfoStructDeclaration *)):
Update for new front-end interface.
(SpeculativeTypeVisitor::visit (TypeStruct *)): Likewise.

libphobos/ChangeLog:

* libdruntime/MERGE: Merge upstream druntime e150cca1.
* src/MERGE: Merge upstream phobos a4a18d21c.
* testsuite/libphobos.cycles/cycles.exp (cycle_test_list): Update
expected result of deprecate test.
---
 gcc/d/dmd/MERGE   |   2 +-
 gcc/d/dmd/aggregate.h |  36 +-
 gcc/d/dmd/clone.d |   9 +-
 gcc/d/dmd/denum.d |  14 +-
 gcc/d/dmd/dstruct.d   |  33 +-
 gcc/d/dmd/dsymbolsem.d|  29 +-
 gcc/d/dmd/enum.h  |  13 +-
 gcc/d/dmd/escape.d|  40 ++-
 gcc/d/dmd/expression.d|   2 +-
 gcc/d/dmd/func.d  |  21 +-
 gcc/d/dmd/parse.d |   4 +-
 gcc/d/dmd/statementsem.d  |  25 +-
 gcc/d/dmd/typesem.d   |  31 +-
 gcc/d/typeinfo.cc |   4 +-
 gcc/testsuite/gdc.test/compilable/b16360.d|  39 ---
 gcc/testsuite/gdc.test/compilable/inliner.d   |  21 ++
 gcc/testsuite/gdc.test/compilable/inliner2.d  |  27 ++
 .../gdc.test/fail_compilation/fail17927.d |   8 +-
 .../gdc.test/fail_compilation/fail20108.d |   2 +-
 .../gdc.test/fail_compilation/fail_scope.d|   8 +-
 .../gdc.test/fail_compilation/ice12574.d  |   2 +-
 .../gdc.test/fail_compilation/previewin.d |   2 +-
 .../gdc.test/fail_compilation/previewin2.d|  18 +
 .../gdc.test/fail_compilation/retscope.d  |  12 +-
 .../gdc.test/fail_compilation/retscope2.d |   4 +-
 .../gdc.test/fail_compilation/retscope6.d |  12 +-
 .../gdc.test/fail_compilation/test14238.d |   2 +-
 .../gdc.test/fail_compilation/test17423.d |   2 +-
 .../gdc.test/fail_compilation/test17450.d |   4 +-
 .../gdc.test/fail_compilation/test20245.d |   8 +-
 .../gdc.test/fail_compilation/test22818.d |   2 +-
 .../gdc.test/fail_compilation/typeerrors.d|   2 +-
 .../gdc.test/fail_compilation/udaparams.d |   4 +-
 .../gdc.test/fail_compilation/udatypes.d  |   8 +
 gcc/testsuite/gdc.test/runnable/ice10086b.d   |  50 +++
 gcc/testsuite/gdc.test/runnable/inline3.d |  44 +++
 .../gdc.test/runnable/staticforeach.d |  29 ++
 gcc/testsuite/gdc.test/runnable_cxx/cppa.d|  20 +-
 .../runnable_cxx/extra-files/cppb.cpp |  30 +-
 libphobos/libdruntime/MERGE   |   2 +-
 libphobos/libdruntime/core/stdc/config.d  |  31 ++
 libphobos/libdruntime/core/stdc/errno.d   | 137 
 libphobos/libdruntime/core/stdc/stdarg.d  |  21 ++
 libphobos/libdruntime/core/stdc/stddef.d  |   5 +
 libphobos/libdruntime/core/stdc/stdint.d  |  35 ++
 libphobos/libdruntime/core/stdc/stdio.d   |  72 
 libphobos/libdruntime/core/stdc/stdlib.d  |   1 +
 libphobos/libdruntime/core/sys/elf/package.d  |  63 
 libphobos/libdruntime/core/sys/posix/fcntl.d  |   6 +
 .../libdruntime/core/sys/windows/winsock2.d   |  14 +-
 libphobos/libdruntime/core/thread/osthread.d  |   4 +-
 .../libdruntime/core/thread/threadbase.d  |  12 +-
 libphobos/libdruntime/core/time.d |   4 +-
 libphobos/libdruntime/core/vararg.d   |  17 +
 libphobos/libdruntime/rt/critical_.d  |   4 +-
 libphobos/libdruntime/rt/dmain2.d |  16 +-
 libphobos/libdruntime/rt/lifetime.d   |   6 +-
 libphobos/libdruntime/rt/minfo.d  | 216 +---
 libphobos/libdruntime/rt/monitor_.d   |   6 +-
 libphobos/src/MERGE   |   2 +-
 libphobos/src/std/complex.d   |   2 +-
 libphobos/src/std/experimental/logger/core.d  | 329 +++---
 .../src/std/experimental/logger/package.d |  20 --
 libphobos/src/std/file.d  |  30 +-
 libphobos/src/std/math/algebraic.d|   6 +-
 libphobos/src/std/math/hardware.d |   6 +-
 libphobos/src/std/math/trigonometry.d |  46 +--
 libphobos/src/std/numeric.d   |  44 ++-
 libph

Re: [PATCH] Inline memchr with a small constant string

2022-06-22 Thread H.J. Lu via Gcc-patches
On Wed, Jun 22, 2022 at 4:39 AM Richard Biener
 wrote:
>
> On Tue, Jun 21, 2022 at 11:03 PM H.J. Lu via Gcc-patches
>  wrote:
> >
> > When memchr is applied on a constant string of no more than the bytes of
> > a word, inline memchr by checking each byte in the constant string.
> >
> > int f (int a)
> > {
> >return  __builtin_memchr ("eE", a, 2) != 0;
> > }
> >
> > is simplified to
> >
> > int f (int a)
> > {
> >   return (char) a == 'e' || (char) a == 'E';
> > }
> >
> > gcc/
> >
> > PR tree-optimization/103798
> > * match.pd (__builtin_memchr (const_str, a, N)): Inline memchr
> > with constant strings of no more than the bytes of a word.
>
> Please do this in strlenopt or so, with match.pd you will end up moving
> the memchr loads across possible aliasing stores to the point of the
> comparison.

strlenopt is run after many other passes.  The code won't be well optimized.
Since we are only optimizing

__builtin_memchr ("eE", a, 2) != 0;

I don't see any aliasing store issues here.

> Richard.
>
> > gcc/testsuite/
> >
> > PR tree-optimization/103798
> > * c-c++-common/pr103798-1.c: New test.
> > * c-c++-common/pr103798-2.c: Likewise.
> > * c-c++-common/pr103798-3.c: Likewise.
> > * c-c++-common/pr103798-4.c: Likewise.
> > * c-c++-common/pr103798-5.c: Likewise.
> > * c-c++-common/pr103798-6.c: Likewise.
> > * c-c++-common/pr103798-7.c: Likewise.
> > * c-c++-common/pr103798-8.c: Likewise.
> > ---
> >  gcc/match.pd| 136 
> >  gcc/testsuite/c-c++-common/pr103798-1.c |  28 +
> >  gcc/testsuite/c-c++-common/pr103798-2.c |  30 ++
> >  gcc/testsuite/c-c++-common/pr103798-3.c |  28 +
> >  gcc/testsuite/c-c++-common/pr103798-4.c |  28 +
> >  gcc/testsuite/c-c++-common/pr103798-5.c |  26 +
> >  gcc/testsuite/c-c++-common/pr103798-6.c |  27 +
> >  gcc/testsuite/c-c++-common/pr103798-7.c |  27 +
> >  gcc/testsuite/c-c++-common/pr103798-8.c |  27 +
> >  9 files changed, 357 insertions(+)
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-1.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-2.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-3.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-4.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-5.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-6.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-7.c
> >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-8.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index a63b649841b..aa4766749af 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -7976,3 +7976,139 @@ and,
> >  (match (bitwise_induction_p @0 @2 @3)
> >   (bit_not
> >(nop_convert1? (bit_xor@0 (convert2? (lshift integer_onep@1 @2)) @3
> > +
> > +#if GIMPLE
> > +/* __builtin_memchr (const_str, a, N) != 0 ->
> > +   a == const_str[0] .. || a == const_str[N-1]
> > +   __builtin_memchr (const_str, a, N) == 0 ->
> > +   a != const_str[0] .. && a != const_str[N-1]
> > +   where N is less than the string size.  */
> > +(for cmp (eq ne)
> > + icmp (ne eq)
> > + bit_op (bit_and bit_ior)
> > + (simplify (cmp:c @0 (BUILT_IN_MEMCHR ADDR_EXPR@1 @2 INTEGER_CST@3))
> > +  (if (UNITS_PER_WORD <= 8
> > +   && CHAR_TYPE_SIZE == 8
> > +   && BITS_PER_UNIT == 8
> > +   && CHAR_BIT == 8
> > +   && integer_zerop (@0)
> > +   && !integer_zerop (@3)
> > +   && TREE_CODE (TREE_OPERAND (@1, 0)) == STRING_CST
> > +   && TREE_STRING_LENGTH (TREE_OPERAND (@1, 0)) >= 2
> > +   && wi::leu_p (wi::to_wide (@3), UNITS_PER_WORD)
> > +   && wi::ltu_p (wi::to_wide (@3),
> > +TREE_STRING_LENGTH (TREE_OPERAND (@1, 0
> > +   (with
> > +{
> > +  const char *p = TREE_STRING_POINTER (TREE_OPERAND (@1, 0));
> > +  unsigned HOST_WIDE_INT size = TREE_INT_CST_LOW (@3);
> > +}
> > +(switch
> > + (if (size == 1)
> > +  (icmp (convert:char_type_node @2)
> > +   { build_int_cst (char_type_node, p[0]); }))
> > + (if (size == 2)
> > +  (bit_op
> > +   (icmp (convert:char_type_node @2)
> > +{ build_int_cst (char_type_node, p[0]); })
> > +   (icmp (convert:char_type_node @2)
> > +{ build_int_cst (char_type_node, p[1]); })))
> > + (if (size == 3)
> > +  (bit_op
> > +   (icmp (convert:char_type_node @2)
> > +{ build_int_cst (char_type_node, p[0]); })
> > +   (bit_op
> > +(icmp (convert:char_type_node @2)
> > + { build_int_cst (char_type_node, p[1]); })
> > +(icmp (convert:char_type_node @2)
> > + { build_int_cst (char_type_node, p[2]); }
> > + (if (size == 4)
> > +  (bit_op
> > +   (icmp (convert:char_type_node @2)
> > +{ build_int_cst (char_type_node, p[0]); })
> > +   (bit_op
> > +   (icmp (con

[PATCH] tilegx: Fix infinite loop in gen-mul-tables generator

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

Since around GCC 10, the condition `j < (INTMAX_MAX / 10)' will get
optimized into `j != 922337203685477580', which will result in an
infinite loop for certain inputs of `j'.

This patch just copies the condition already used by the -DTILEPRO
generator code, which doesn't fall into the same trap.

OK for mainline?  OK for backporting to all open release branches?

Regards,
Iain.

---
gcc/ChangeLog:

* config/tilepro/gen-mul-tables.cc (tilegx_emit): Adjust loop
condition to avoid overflow.
---
 gcc/config/tilepro/gen-mul-tables.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/tilepro/gen-mul-tables.cc 
b/gcc/config/tilepro/gen-mul-tables.cc
index 52183982f65..c125748a328 100644
--- a/gcc/config/tilepro/gen-mul-tables.cc
+++ b/gcc/config/tilepro/gen-mul-tables.cc
@@ -1192,11 +1192,11 @@ tilegx_emit (long long multiplier, int num_ops)
 long long next_pow10;
 
 while (((j * 10) < abs_multiplier)
-  && (j < (INTMAX_MAX / 10)))
+  && (j < (j * 10)))
   j = j * 10;
 
 prev_pow10 = j;
-next_pow10 = (j > (INTMAX_MAX / 10)) ? 0 : j * 10;
+next_pow10 = j * 10;
 
 if ((abs_multiplier - prev_pow10 <= 100)
|| (next_pow10
-- 
2.34.1



Re: [PATCH] tilegx: Fix infinite loop in gen-mul-tables generator

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




On 6/22/2022 11:30 AM, Iain Buclaw via Gcc-patches wrote:

Hi,

Since around GCC 10, the condition `j < (INTMAX_MAX / 10)' will get
optimized into `j != 922337203685477580', which will result in an
infinite loop for certain inputs of `j'.

This patch just copies the condition already used by the -DTILEPRO
generator code, which doesn't fall into the same trap.

OK for mainline?  OK for backporting to all open release branches?
Sure, but note the tile ports have been deprecated for a long time and 
I've got a TODO to finally remove them.


jeff



[r13-1203 Regression] FAIL: gcc.dg/tree-ssa/slsr-39.c scan-tree-dump-times slsr "Replacing reference: " 4 on Linux/x86_64

2022-06-22 Thread skpandey--- via Gcc-patches
On Linux/x86_64,

038b077689bb5310386b04d40a2cea234f01e6aa is the first bad commit
commit 038b077689bb5310386b04d40a2cea234f01e6aa
Author: Richard Sandiford 
Date:   Wed Jun 22 11:27:15 2022 +0100

data-ref: Improve non-loop disambiguation [PR106019]

caused

FAIL: gcc.dg/tree-ssa/slsr-39.c scan-tree-dump-times slsr "Replacing reference: 
" 4

with GCC configured with

../../gcc/configure 
--prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r13-1203/usr 
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
--enable-libmpx x86_64-linux --disable-bootstrap

To reproduce:

$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="tree-ssa.exp=gcc.dg/tree-ssa/slsr-39.c --target_board='unix{-m32\ 
-march=cascadelake}'"

(Please do not reply to this email, for question about this report, contact me 
at skpgkp2 at gmail dot com)


Re: [PATCH] Introduce -nolibstdc++ option

2022-06-22 Thread Iain Sandoe via Gcc-patches



> On 22 Jun 2022, at 01:36, Alexandre Oliva via Gcc-patches 
>  wrote:
> 
> On Jun 21, 2022, Fangrui Song  wrote:
> 
>> Is this similar to clang -nostdlib++ ?
>> When libstdc++ is selected, clang -nostdlib++ removes -lstdc++.
> 
> Sounds like they're the same indeed, but the clang option you mention
> makes little sense to me, so I'd rather to introduce the one that does.
> If someone feels offering this option with the same spelling as clang,
> it's easy enough to add a synonym.  Now, if others feel we'd be better
> off following clang's practices, I don't mind adjusting the patch to use
> the same spelling.  It's not like this option is going to have much use
> one way or another, aside from this testcase.

we have nostdlib, nodefaultlibs (which also omit the C++ libs)

It makes some sense to have the option named -nostdlib++ if a target
might add multiple libs (and/or make other changes) for linking C++.

(so, fo example, if libstdc++ were separate from libsupc++ I would
 expect your use-case to wish to exclude both, not just libstdc++)?

From the PoV of users and build systems, it’s also helpful to avoid
proliferating option names unless there’s some clear distinction in function
between compilers - if GCC already has an option spelling, usually
clang would follow that - it does not seem unreasonable to reciprocate.

0.02GBP only, of course,
Iain

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



Re: [PATCH] gcc/configure.ac: fix --enable-fixed-point enablement [PR34422]

2022-06-22 Thread Alexandre Oliva via Gcc-patches
Hello, Eric,

On Jun 21, 2022, Eric Gallager  wrote:

> https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596654.html
> (cc-ing the build machinery maintainers listed in MAINTAINERS this time)

Thanks

> On Tue, Jun 14, 2022 at 3:51 PM Eric Gallager  wrote:

>> So, in investigating PR target/34422, I discovered that the gcc
>> subdirectory's configure script had an instance of AC_ARG_ENABLE with
>> 3rd and 4th its arguments reversed: the one where it warns that the
>> --enable-fixed-point flag is being ignored is the one where that flag
>> hasn't even been passed in the first place.

I'm not sure it was reversed or meant to enable the feature by default
(if not given), but neither theory holds much water to me.

Alas, the proposed change needs a little more work.  Specifically, I
don't think we should warn that the feature is not available if it was
explicitly --disable'd.

Moreover, if it was intended for fixed-point to be enabled by default,
then the help string should probably mention the --disable flag instead,
otherwise, the per-target setting to yes should probably be dropped.


I dislike the duplication of the targets in two case statements.  I
think the best way to deal with this would be to only set
enable_fixed_point=default in ACTION-IF-NOT-GIVEN, leaving
ACTION-IF-GIVEN empty, and afterwards, if "x$enable_fixed_point" !=
"xno", check for target support, bumping default to yes on supported
targets, and decaying it to no if unsupported, with a warning only if it
was yes.

>> with an appropriate ChangeLog?

It's good practice to post the proposed ChangeLog entry along with the
patch, so that it can also be reviewed.

Thanks,

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


[pushed] c++: class scope function lookup [PR105908]

2022-06-22 Thread Jason Merrill via Gcc-patches
In r12-1273 for PR91706, I removed the code in get_class_binding that
stripped BASELINK.  This testcase demonstrates that we still need to strip
it in outer_binding before putting the overload set in IDENTIFIER_BINDING,
for compatibility with bindings added directly for declarations.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/105908

gcc/cp/ChangeLog:

* name-lookup.cc (outer_binding): Strip BASELINK.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/trailing16.C: New test.
---
 gcc/cp/name-lookup.cc   |  4 
 gcc/testsuite/g++.dg/cpp0x/trailing16.C | 17 +
 2 files changed, 21 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/trailing16.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 421bf2e4f7a..ce622761a1a 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -7629,6 +7629,10 @@ outer_binding (tree name,
/* Thread this new class-scope binding onto the
   IDENTIFIER_BINDING list so that future lookups
   find it quickly.  */
+   if (BASELINK_P (class_binding->value))
+ /* Don't put a BASELINK in IDENTIFIER_BINDING.  */
+ class_binding->value
+   = BASELINK_FUNCTIONS (class_binding->value);
class_binding->previous = outer;
if (binding)
  binding->previous = class_binding;
diff --git a/gcc/testsuite/g++.dg/cpp0x/trailing16.C 
b/gcc/testsuite/g++.dg/cpp0x/trailing16.C
new file mode 100644
index 000..4feb3f81c27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/trailing16.C
@@ -0,0 +1,17 @@
+// PR c++/105908
+// { dg-do compile { target c++11 } }
+
+struct test
+{
+  template 
+  int templated_func();
+
+  template 
+  auto call_templated_func() -> decltype(templated_func());
+};
+
+template 
+auto test::call_templated_func() -> decltype(templated_func())
+{
+  return templated_func();
+}

base-commit: d68d366425369649cb4e25a07752e25a4fff52cf
-- 
2.27.0



[PATCH 03/12] Add more emit_diagnostic overloads

2022-06-22 Thread David Malcolm via Gcc-patches
gcc/ChangeLog:
* diagnostic-core.h (emit_diagnostic): New overload.
(emit_diagnostic_valist): New overload.
* diagnostic.cc (emit_diagnostic): New overload.
(emit_diagnostic_valist): New overload.

Signed-off-by: David Malcolm 
---
 gcc/diagnostic-core.h |  7 +++
 gcc/diagnostic.cc | 25 +
 2 files changed, 32 insertions(+)

diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 286954ac2f8..53f39a110e4 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -114,8 +114,15 @@ extern bool emit_diagnostic (diagnostic_t, location_t, int,
 const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
 extern bool emit_diagnostic (diagnostic_t, rich_location *, int,
 const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
+extern bool emit_diagnostic (diagnostic_t, rich_location *,
+const diagnostic_metadata *, int,
+const char *, ...) ATTRIBUTE_GCC_DIAG(5,6);
 extern bool emit_diagnostic_valist (diagnostic_t, location_t, int, const char 
*,
va_list *) ATTRIBUTE_GCC_DIAG (4,0);
+extern bool emit_diagnostic_valist (diagnostic_t, rich_location *,
+   const diagnostic_metadata *, int,
+   const char *, va_list *)
+  ATTRIBUTE_GCC_DIAG (5,0);
 extern bool seen_error (void);
 
 #ifdef BUFSIZ
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 22f7b0b6d6e..8099f585bfb 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -1769,6 +1769,21 @@ emit_diagnostic (diagnostic_t kind, rich_location 
*richloc, int opt,
   return ret;
 }
 
+/* As above, but with optional metadata.  */
+
+bool
+emit_diagnostic (diagnostic_t kind, rich_location *richloc,
+const diagnostic_metadata *metadata, int opt,
+const char *gmsgid, ...)
+{
+  auto_diagnostic_group d;
+  va_list ap;
+  va_start (ap, gmsgid);
+  bool ret = diagnostic_impl (richloc, metadata, opt, gmsgid, &ap, kind);
+  va_end (ap);
+  return ret;
+}
+
 /* Wrapper around diagnostic_impl taking a va_list parameter.  */
 
 bool
@@ -1779,6 +1794,16 @@ emit_diagnostic_valist (diagnostic_t kind, location_t 
location, int opt,
   return diagnostic_impl (&richloc, NULL, opt, gmsgid, ap, kind);
 }
 
+/* As above, but with optional metadata.  */
+
+bool
+emit_diagnostic_valist (diagnostic_t kind, rich_location *richloc,
+   const diagnostic_metadata *metadata, int opt,
+   const char *gmsgid, va_list *ap)
+{
+  return diagnostic_impl (richloc, metadata, opt, gmsgid, ap, kind);
+}
+
 /* An informative note at LOCATION.  Use this for additional details on an 
error
message.  */
 void
-- 
2.26.3



[PATCH 05/12] Placeholder libcpp fixups

2022-06-22 Thread David Malcolm via Gcc-patches
Obviously this isn't quite ready for trunk yet.

libcpp/ChangeLog:
* include/line-map.h (rich_location::maybe_add_fixit): Make
public.
* line-map.cc (linemap_add): Hack away assertion about LC_RENAME
for now.

Signed-off-by: David Malcolm 
---
 libcpp/include/line-map.h | 7 ---
 libcpp/line-map.cc| 3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 80335721e03..c27d8a6fdcd 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1799,13 +1799,14 @@ class rich_location
   bool escape_on_output_p () const { return m_escape_on_output; }
   void set_escape_on_output (bool flag) { m_escape_on_output = flag; }
 
-private:
-  bool reject_impossible_fixit (location_t where);
-  void stop_supporting_fixits ();
   void maybe_add_fixit (location_t start,
location_t next_loc,
const char *new_content);
 
+private:
+  bool reject_impossible_fixit (location_t where);
+  void stop_supporting_fixits ();
+
 public:
   static const int STATICALLY_ALLOCATED_RANGES = 3;
 
diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
index 62077c3857c..82f27280ea7 100644
--- a/libcpp/line-map.cc
+++ b/libcpp/line-map.cc
@@ -496,10 +496,11 @@ linemap_add (line_maps *set, enum lc_reason reason,
   linemap_assert (!LINEMAPS_ORDINARY_USED (set)
  || (start_location
  >= MAP_START_LOCATION (LINEMAPS_LAST_ORDINARY_MAP 
(set;
-
+#if 0
   /* When we enter the file for the first time reason cannot be
  LC_RENAME.  */
   linemap_assert (!(set->depth == 0 && reason == LC_RENAME));
+#endif
 
   /* If we are leaving the main file, return a NULL map.  */
   if (reason == LC_LEAVE
-- 
2.26.3



[PATCH 01/12] diagnostics: add ability to associate diagnostics with rules from coding standards

2022-06-22 Thread David Malcolm via Gcc-patches
gcc/ChangeLog:
* common.opt (fdiagnostics-show-rules): New option.
* diagnostic-format-json.cc (diagnostic_output_format_init_json):
Fix up context->show_rules.
* diagnostic-format-sarif.cc
(diagnostic_output_format_init_sarif): Likewise.
* diagnostic-metadata.h (diagnostic_metadata::rule): New class.
(diagnostic_metadata::precanned_rule): New class.
(diagnostic_metadata::add_rule): New.
(diagnostic_metadata::get_num_rules): New.
(diagnostic_metadata::get_rule): New.
(diagnostic_metadata::m_rules): New field.
* diagnostic.cc (diagnostic_initialize): Initialize show_rules.
(print_any_rules): New.
(diagnostic_report_diagnostic): Call it.
* diagnostic.h (diagnostic_context::show_rules): New field.
* doc/invoke.texi (-fno-diagnostics-show-rules): New option.
* opts.cc (common_handle_option): Handle
OPT_fdiagnostics_show_rules.
* toplev.cc (general_init): Set up global_dc->show_rules.

gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-metadata.c: Expect " [STR34-C]" to
be emitted at the "gets" call.
* gcc.dg/plugin/diagnostic_plugin_test_metadata.c
(pass_test_metadata::execute): Associate the "gets" diagnostic
with a rule named "STR34-C".

Signed-off-by: David Malcolm 
---
 gcc/common.opt|  4 ++
 gcc/diagnostic-format-json.cc |  1 +
 gcc/diagnostic-format-sarif.cc|  1 +
 gcc/diagnostic-metadata.h | 47 +-
 gcc/diagnostic.cc | 48 +++
 gcc/diagnostic.h  |  3 ++
 gcc/doc/invoke.texi   | 10 
 gcc/opts.cc   |  4 ++
 .../gcc.dg/plugin/diagnostic-test-metadata.c  |  2 +-
 .../plugin/diagnostic_plugin_test_metadata.c  |  9 +++-
 gcc/toplev.cc |  2 +
 11 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 32917aafcae..3a842847a74 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1466,6 +1466,10 @@ fdiagnostics-show-cwe
 Common Var(flag_diagnostics_show_cwe) Init(1)
 Print CWE identifiers for diagnostic messages, where available.
 
+fdiagnostics-show-rules
+Common Var(flag_diagnostics_show_rules) Init(1)
+Print any rules associated with diagnostic messages.
+
 fdiagnostics-path-format=
 Common Joined RejectNegative Var(flag_diagnostics_path_format) 
Enum(diagnostic_path_format) Init(DPF_INLINE_EVENTS)
 Specify how to print any control-flow path associated with a diagnostic.
diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
index 051fa6c2e48..d1d8d3f2081 100644
--- a/gcc/diagnostic-format-json.cc
+++ b/gcc/diagnostic-format-json.cc
@@ -345,6 +345,7 @@ diagnostic_output_format_init_json (diagnostic_context 
*context)
 
   /* The metadata is handled in JSON format, rather than as text.  */
   context->show_cwe = false;
+  context->show_rules = false;
 
   /* The option is handled in JSON format, rather than as text.  */
   context->show_option_requested = false;
diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 0c33179e8cf..a7bb9fb639d 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -1556,6 +1556,7 @@ diagnostic_output_format_init_sarif (diagnostic_context 
*context)
 
   /* The metadata is handled in SARIF format, rather than as text.  */
   context->show_cwe = false;
+  context->show_rules = false;
 
   /* The option is handled in SARIF format, rather than as text.  */
   context->show_option_requested = false;
diff --git a/gcc/diagnostic-metadata.h b/gcc/diagnostic-metadata.h
index ae59942c65e..80017d35fa9 100644
--- a/gcc/diagnostic-metadata.h
+++ b/gcc/diagnostic-metadata.h
@@ -24,19 +24,62 @@ along with GCC; see the file COPYING3.  If not see
 /* A bundle of additional metadata that can be associated with a
diagnostic.
 
-   Currently this only supports associating a CWE identifier with a
-   diagnostic.  */
+   This supports an optional CWE identifier, and zero or more
+   "rules".  */
 
 class diagnostic_metadata
 {
  public:
+  /* Abstract base class for referencing a rule that has been violated,
+ such as within a coding standard, or within a specification.  */
+  class rule
+  {
+  public:
+virtual char *make_description () const = 0;
+virtual char *make_url () const = 0;
+  };
+
+  /* Concrete subclass.  */
+  class precanned_rule : public rule
+  {
+  public:
+precanned_rule (const char *desc, const char *url)
+: m_desc (desc), m_url (url)
+{}
+
+char *make_description () const final override
+{
+  return m_desc ? xstrdup (m_desc) : NULL;
+}
+
+char *make_url () const final override
+{
+  return m_url ? xstrdup (m_url) : NULL;
+}
+
+  private:
+const c

[PATCH 00/12] RFC: Replay of serialized diagnostics

2022-06-22 Thread David Malcolm via Gcc-patches
We currently have a couple of formats into which our diagnostics can
be serialized: (a) gcc's own json format and (b) SARIF, via:

  -fdiagnostics-format=json-stderr (and -fdiagnostics-format=json)
  -fdiagnostics-format=json-file
  -fdiagnostics-format=sarif-stderr
  -fdiagnostics-format=sarif-file

This experimental patch kit implements the ability for GCC to
replay these serialized diagnostics - and thus to use GCC's diagnostic
printing machinery to print the output of any tool that can write to
these formats.

It implements the replayers as gcc frontends, handling .json and .sarif
files, so that in theory you can write:

  gcc foo.sarif

and have gcc replay things.  Unfortunately that doesn't quite work: you
have to tell it -S, and if the file isn't found, it thinks it's a linker
script.

For example, here's the output of replaying some of the files from the
SARIF tutorial [https://github.com/microsoft/sarif-tutorials]:

e.g. the results of a javascript linter:

$ ./xgcc -B. -S 
../../../sarif-tutorials/samples/1-Introduction/simple-example.sarif
file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js:1:5:
 error: 'x' is assigned a value but never used. [no-unused-vars]

(the no-unused-vars is actually a hyperlink to 
https://eslint.org/docs/rules/no-unused-vars in my terminal)

and a Python analyzer:

$ ./xgcc -B. -S -xsarif 
../../../sarif-tutorials/samples/3-Beyond-basics/bad-eval-with-code-flow.sarif 
3-Beyond-basics/bad-eval-with-code-flow.py:8:1: warning: Use of tainted 
variable 'raw_input' in the insecure function 'eval'. [PY2335]
  events 1-2
|
|
+--> event 3
   |
   |

As you can see, it has some issues finding source code when replaying
things (there's an example in the final patch in the kit that manages
this).

Thoughts?

Would it be better to have an explicit tool for this
(e.g. "gcc-replay-sarif"), rather than have it as a frontend?

Doesn't yet bootstrap, and has FIXMEs and TODOs but posting now in the
hope of getting feedback.


David Malcolm (12):
  diagnostics: add ability to associate diagnostics with rules from
coding standards
  diagnostics: associate rules with plugins in SARIF output
  Add more emit_diagnostic overloads
  json: add json parsing support
  Placeholder libcpp fixups
  prune.exp: move multiline-handling to before other pruning
  Add deferred-locations.h/cc
  Add json-reader.h/cc
  Add json frontend
  Add sarif frontend
  Fixups to diagnostic-format-sarif.cc
  Work-in-progress of path remapping

 gcc/Makefile.in   |2 +-
 gcc/common.opt|4 +
 gcc/deferred-locations.cc |  231 ++
 gcc/deferred-locations.h  |   52 +
 gcc/diagnostic-client-data-hooks.h|   18 -
 gcc/diagnostic-client-plugin.h|   43 +
 gcc/diagnostic-core.h |7 +
 gcc/diagnostic-format-json.cc |1 +
 gcc/diagnostic-format-sarif.cc|  303 ++-
 gcc/diagnostic-metadata.h |   65 +-
 gcc/diagnostic.cc |   73 +
 gcc/diagnostic.h  |3 +
 gcc/doc/invoke.texi   |   10 +
 gcc/doc/plugins.texi  |   17 +-
 gcc/input.cc  |  107 +-
 gcc/input.h   |   18 +-
 gcc/json-parsing.cc   | 2391 +
 gcc/json-parsing.h|   94 +
 gcc/json-reader.cc|  122 +
 gcc/json-reader.h |  107 +
 gcc/json.cc   |2 +-
 gcc/json.h|   59 +-
 gcc/json/Make-lang.in |  131 +
 gcc/json/config-lang.in   |   34 +
 gcc/json/json-frontend.cc |  176 ++
 gcc/json/json-replay.cc   |  614 +
 gcc/json/json-replay.h|   26 +
 gcc/json/lang-specs.h |   26 +
 gcc/json/lang.opt |   31 +
 gcc/opts.cc   |4 +
 gcc/plugin.cc |   36 +-
 gcc/plugin.h  |   12 +-
 gcc/sarif/Make-lang.in|  132 +
 gcc/sarif/config-lang.in  |   34 +
 gcc/sarif/lang-specs.h|   26 +
 gcc/sarif/lang.opt|   31 +
 gcc/sarif/sarif-frontend.cc   |  191 ++
 gcc/sarif/sarif-replay.cc | 1528 +++
 gcc/sarif/sarif-replay.h  |   26 +
 gcc/selftest-run-tests.cc |1 +
 gcc/selftest.h|1 +
 .../plugin/diagnostic-test-metadata-sarif.c   |   39 +
 .../gcc.dg/plugin/diagnostic-test-metadata.c  |2 +-
 .../plugin/dia

[PATCH 02/12] diagnostics: associate rules with plugins in SARIF output

2022-06-22 Thread David Malcolm via Gcc-patches
gcc/ChangeLog:
* diagnostic-client-data-hooks.h
(class diagnostic_client_plugin_info): Move to...
* diagnostic-client-plugin.h: ...this new file.
* diagnostic-format-sarif.cc: Include "diagnostic-client-plugin.h".
(class sarif_tool_component): New class.
(sarif_builder::m_rule_id_set): Move field to sarif_tool_component.
(sarif_builder::m_rules_arr): Likewise.
(sarif_builder::m_driver_obj): New field.
(sarif_builder::m_extensions_arr): New field.
(sarif_builder::m_plugin_objs): New field.
(sarif_tool_component::sarif_tool_component): New.
(sarif_tool_component::lazily_add_rule): New.
(sarif_builder::sarif_builder): Update for changes to fields.
(sarif_builder::get_plugin_tool_component_object): New, adapted
from code within sarif_builder::make_tool_object.
(maybe_get_first_rule): New.
(sarif_builder::set_result_rule_id): New, adapted from code within
sarif_builder::make_result_object.
(sarif_builder::make_result_object): Move rule_id logic to
sarif_builder::set_result_rule_id.
(sarif_builder::make_reporting_descriptor_object_for_rule): New.
(sarif_builder::make_tool_object): Drop "const" qualifier.  Use
the m_driver_obj and m_extensions_arr created in the ctor.  Update
the visitor to call get_plugin_tool_component_object, rather than
duplicate the toolComponent creation logic.
(sarif_builder::make_driver_tool_component_object): Convert return
type to sarif_tool_component *.  Move setting of "rules" property
to sarif_tool_component ctor.
* diagnostic-metadata.h (class diagnostic_client_plugin_info): New
forward decl.
(diagnostic_metadata::diagnostic_metadata): Add overloaded ctor.
Initialize m_plugin.
(diagnostic_metadata::get_plugin): New.
(diagnostic_metadata::m_plugin): New field.
* doc/plugins.texi (Plugin initialization): Update.
* plugin.cc: Include "diagnostic-client-plugin.h".
(plugin_name_args::plugin_name_args): New ctor.
(plugin_name_args::get_short_name): New.
(plugin_name_args::get_full_name): New.
(plugin_name_args::get_version): New.
(add_new_plugin): Rewrite creation of "plugin" to use new with a
ctor.
* plugin.h: Include "diagnostic-client-plugin.h".
(struct plugin_name_args): Convert to...
(class plugin_name_args): ...this, deriving it from
diagnostic_client_plugin_info.
(plugin_name_args::plugin_name_args): New ctor decl.
(plugin_name_args::get_short_name): New decl.
(plugin_name_args::get_full_name): New decl.
(plugin_name_args::get_version): New decl.
* tree-diagnostic-client-data-hooks.cc: Include
"diagnostic-client-plugin.h".
(class compiler_diagnostic_client_plugin_info): Delete.
(compiler_version_info::on_plugin_cb): Pass plugin_name_args
to the visitor, now that the former is a
diagnostic_client_plugin_info.

gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-metadata-sarif.c: New test.
* gcc.dg/plugin/diagnostic_plugin_test_metadata.c
(diag_plugin_info): New global.
(pass_test_metadata::execute): Pass it to metadata.
(plugin_init): Initialize it.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
diagnostic-test-metadata-sarif.c to
diagnostic_plugin_test_metadata.c.

Signed-off-by: David Malcolm 
---
 gcc/diagnostic-client-data-hooks.h|  18 --
 gcc/diagnostic-client-plugin.h|  43 +++
 gcc/diagnostic-format-sarif.cc| 283 +-
 gcc/diagnostic-metadata.h |  22 +-
 gcc/doc/plugins.texi  |  17 +-
 gcc/plugin.cc |  36 ++-
 gcc/plugin.h  |  12 +-
 .../plugin/diagnostic-test-metadata-sarif.c   |  39 +++
 .../plugin/diagnostic_plugin_test_metadata.c  |   6 +-
 gcc/testsuite/gcc.dg/plugin/plugin.exp|   4 +-
 gcc/tree-diagnostic-client-data-hooks.cc  |  36 +--
 11 files changed, 382 insertions(+), 134 deletions(-)
 create mode 100644 gcc/diagnostic-client-plugin.h
 create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-metadata-sarif.c

diff --git a/gcc/diagnostic-client-data-hooks.h 
b/gcc/diagnostic-client-data-hooks.h
index ba78546abeb..03464202fe1 100644
--- a/gcc/diagnostic-client-data-hooks.h
+++ b/gcc/diagnostic-client-data-hooks.h
@@ -84,22 +84,4 @@ public:
   virtual void for_each_plugin (plugin_visitor &v) const = 0;
 };
 
-/* Abstract base class for a diagnostic_context to get at
-   information about a specific plugin within a client.  */
-
-class diagnostic_client_plugin_info
-{
-public:
-  /* For use e.g. by SARIF "name" property (SARIF v2.1.0 section 3.19.8).  */
-  virtual const char *

[PATCH 07/12] Add deferred-locations.h/cc

2022-06-22 Thread David Malcolm via Gcc-patches
libcpp requires locations to be created as if by a tokenizer, creating
them by filename, in ascending order of line/column.

This patch adds support classes that allow the creation of locations
in arbitrary orders, by deferring all location creation,
grouping things up by filename/line, and then creating the linemap
entries in a post-processing phase.

gcc/ChangeLog:
* deferred-locations.cc: New file, adapted from code in
jit/jit-playback.cc.
* deferred-locations.h: New file.

Signed-off-by: David Malcolm 
---
 gcc/deferred-locations.cc | 231 ++
 gcc/deferred-locations.h  |  52 +
 2 files changed, 283 insertions(+)
 create mode 100644 gcc/deferred-locations.cc
 create mode 100644 gcc/deferred-locations.h

diff --git a/gcc/deferred-locations.cc b/gcc/deferred-locations.cc
new file mode 100644
index 000..e78b29a4d58
--- /dev/null
+++ b/gcc/deferred-locations.cc
@@ -0,0 +1,231 @@
+/* Support for deferred creation of location_t values.
+   Copyright (C) 2013-2022 Free Software Foundation, Inc.
+   Contributed by David Malcolm .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "deferred-locations.h"
+
+/* Concrete implementation for use by deferred_locations.  */
+
+class deferred_locations_impl
+{
+public:
+  class source_file;
+  class source_line;
+  class source_location;
+
+  /* A specific location on a source line, with a saved location_t *
+ to write back to.  */
+  class source_location
+  {
+  public:
+source_location (int column_num, location_t *out_loc)
+: m_column_num (column_num), m_out_loc (out_loc)
+{
+}
+
+int get_column_num () const { return m_column_num; }
+
+/* qsort comparator for comparing pairs of source_location *,
+   ordering them by column number.  */
+
+static int
+comparator (const void *lhs, const void *rhs)
+{
+  const source_location &location_lhs
+   = *static_cast (lhs);
+  const source_location &location_rhs
+   = *static_cast (rhs);
+  return location_lhs.m_column_num - location_rhs.m_column_num;
+}
+
+void generate_location_t_value () const
+{
+  *m_out_loc = linemap_position_for_column (line_table, m_column_num);
+}
+
+  private:
+int m_column_num;
+location_t *m_out_loc;
+  };
+
+  /* A source line, with one or more locations of interest.  */
+  class source_line
+  {
+  public:
+source_line (int line_num) : m_line_num (line_num) {}
+
+source_location *
+get_location (int column_num, location_t *out_loc);
+
+int get_line_num () const { return m_line_num; }
+
+void add_location (const expanded_location &exploc,
+  location_t *out_loc)
+{
+  m_locations.safe_push (source_location (exploc.column, out_loc));
+}
+
+/* qsort comparator for comparing pairs source_line *,
+   ordering them by line number.  */
+
+static int
+comparator (const void *lhs, const void *rhs)
+{
+  const source_line *line_lhs
+   = *static_cast (lhs);
+  const source_line *line_rhs
+   = *static_cast (rhs);
+  return line_lhs->get_line_num () - line_rhs->get_line_num ();
+}
+
+void generate_location_t_values ()
+{
+  /* Determine maximum column within this line.  */
+  m_locations.qsort (source_location::comparator);
+  gcc_assert (m_locations.length () > 0);
+  source_location *final_column = &m_locations[m_locations.length () - 1];
+  int max_col = final_column->get_column_num ();
+
+  linemap_line_start (line_table, m_line_num, max_col);
+  for (auto loc_iter : m_locations)
+   loc_iter.generate_location_t_value ();
+}
+
+  private:
+int m_line_num;
+auto_vec m_locations;
+  };
+
+  /* A set of locations, all sharing a filename */
+  class source_file
+  {
+  public:
+source_file (const char *filename)
+: m_filename (xstrdup (filename))
+{
+}
+~source_file ()
+{
+  free (m_filename);
+}
+
+source_line *
+get_source_line (int line_num);
+
+const char*
+get_filename () const { return m_filename; }
+
+bool
+matches (const char *filename)
+{
+  return ((filename == NULL && m_filename == NULL)
+ || ((filename && m_filename)
+ && 0 == strcmp (filename,

[PATCH 08/12] Add json-reader.h/cc

2022-06-22 Thread David Malcolm via Gcc-patches
This patch adds classes that better integrate the JSON parser with
GCC's diagnostic subsystem (e.g. line_maps).

gcc/ChangeLog:
* json-reader.cc: New file.
* json-reader.h: New file.

Signed-off-by: David Malcolm 
---
 gcc/json-reader.cc | 122 +
 gcc/json-reader.h  | 107 +++
 2 files changed, 229 insertions(+)
 create mode 100644 gcc/json-reader.cc
 create mode 100644 gcc/json-reader.h

diff --git a/gcc/json-reader.cc b/gcc/json-reader.cc
new file mode 100644
index 000..e4fbd0db803
--- /dev/null
+++ b/gcc/json-reader.cc
@@ -0,0 +1,122 @@
+/* Integration of JSON parsing with GCC diagnostics.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "diagnostic.h"
+#include "json-reader.h"
+
+/* Read the contents of PATH into memory, returning a 0-terminated buffer
+   that must be freed by the caller.
+   Issue a fatal error if there are any problems.  */
+
+char *
+read_file (const char *path)
+{
+  FILE *f_in = fopen (path, "r");
+  if (!f_in)
+fatal_error (UNKNOWN_LOCATION, "unable to open file %qs: %s",
+path, xstrerror (errno));
+
+  /* Read content, allocating a buffer for it.  */
+  char *result = NULL;
+  size_t total_sz = 0;
+  size_t alloc_sz = 0;
+  char buf[4096];
+  size_t iter_sz_in;
+
+  while ( (iter_sz_in = fread (buf, 1, sizeof (buf), f_in)) )
+{
+  gcc_assert (alloc_sz >= total_sz);
+  size_t old_total_sz = total_sz;
+  total_sz += iter_sz_in;
+  /* Allow 1 extra byte for 0-termination.  */
+  if (alloc_sz < (total_sz + 1))
+   {
+ size_t new_alloc_sz = alloc_sz ? alloc_sz * 2: total_sz + 1;
+ result = (char *)xrealloc (result, new_alloc_sz);
+ alloc_sz = new_alloc_sz;
+   }
+  memcpy (result + old_total_sz, buf, iter_sz_in);
+}
+
+  if (!feof (f_in))
+fatal_error (UNKNOWN_LOCATION, "error reading from %qs: %s", path,
+xstrerror (errno));
+
+  fclose (f_in);
+
+  /* 0-terminate the buffer.  */
+  gcc_assert (total_sz < alloc_sz);
+  result[total_sz] = '\0';
+
+  return result;
+}
+
+/* json_reader's ctor.  */
+
+json_reader::json_reader (const char *filename)
+: m_filename (filename),
+  m_json_loc_map (filename, line_table)
+{
+}
+
+/* Parse UTF8, capturing source location information in m_json_loc_map.
+   If successful, return the top-level value.
+   Otherwise, return NULL and write to *ERR_OUT.  */
+
+json::value *
+json_reader::parse_utf8_string (const char *utf8, bool allow_comments,
+   json::error **err_out)
+{
+  json::value *result
+= json::parse_utf8_string (utf8, allow_comments, err_out, &m_json_loc_map);
+  return result;
+}
+
+/* Issue an error diagnostic for GMSGID at the location of JV, and exit.  */
+
+void
+json_reader::fatal_error (json::value *jv, const char *gmsgid, ...)
+{
+  location_t loc = m_json_loc_map.get_range_for_value (jv);
+
+  auto_diagnostic_group d;
+  va_list ap;
+  va_start (ap, gmsgid);
+  rich_location richloc (line_table, loc);
+  emit_diagnostic_valist (DK_ERROR, &richloc, NULL, 0, gmsgid, &ap);
+  va_end (ap);
+  exit (1);
+  /* Ideally we'd use ::fatal_error here, but we seem to need to use
+ DK_ERROR for it to be usable from DejaGnu.  */
+}
+
+/* Issue an error diagnostic for ERR, and exit.  */
+
+void
+json_reader::fatal_error (json::error *err)
+{
+  location_t loc = m_json_loc_map.make_location_for_range (err->get_range ());
+  ::error_at (loc, "%s", err->get_msg ());
+  exit (1);
+  /* Ideally we'd use ::fatal_error here, but we seem to need to use
+ DK_ERROR for it to be usable from DejaGnu.  */
+}
diff --git a/gcc/json-reader.h b/gcc/json-reader.h
new file mode 100644
index 000..4537e29cb6a
--- /dev/null
+++ b/gcc/json-reader.h
@@ -0,0 +1,107 @@
+/* Integration of JSON parsing with GCC diagnostics.
+   Copyright (C) 2022 David Malcolm .
+   Contributed by David Malcolm .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT A

[PATCH 11/12] Fixups to diagnostic-format-sarif.cc

2022-06-22 Thread David Malcolm via Gcc-patches
I believe these are needed by one of the rule-handling patches.

gcc/ChangeLog:
* diagnostic-format-sarif.cc (sarif_builder::sarif_builder):
Defer population of m_driver_obj until...
(sarif_builder::make_tool_object): ...here.
(sarif_builder::make_driver_tool_component_object): Replace
with...
(sarif_builder::populate_driver_object): ...this.

Signed-off-by: David Malcolm 
---
 gcc/diagnostic-format-sarif.cc | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index a409abf648b..9c304fd8e49 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -158,7 +158,7 @@ private:
   json::object *make_top_level_object (json::array *results);
   json::object *make_run_object (json::array *results);
   json::object *make_tool_object ();
-  sarif_tool_component *make_driver_tool_component_object () const;
+  void populate_driver_object () const;
   json::array *maybe_make_taxonomies_array () const;
   json::object *maybe_make_cwe_taxonomy_object () const;
   json::object *make_tool_component_reference_object_for_cwe () const;
@@ -289,7 +289,7 @@ sarif_builder::sarif_builder (diagnostic_context *context)
   m_extensions_arr (NULL),
   m_tabstop (context->tabstop)
 {
-  m_driver_obj = make_driver_tool_component_object ();
+  m_driver_obj = new sarif_tool_component ();
   m_extensions_arr = new json::array ();
 }
 
@@ -1256,6 +1256,7 @@ sarif_builder::make_tool_object ()
 
   /* "driver" property (SARIF v2.1.0 section 3.18.2).  */
   tool_obj->set ("driver", m_driver_obj);
+  populate_driver_object ();
 
   /* Report plugins via the "extensions" property
  (SARIF v2.1.0 section 3.18.3).  */
@@ -1288,42 +1289,40 @@ sarif_builder::make_tool_object ()
   return tool_obj;
 }
 
-/* Make a toolComponent object (SARIF v2.1.0 section 3.19) for what SARIF
-   calls the "driver" (see SARIF v2.1.0 section 3.18.1).  */
+/* Populate the toolComponent object (SARIF v2.1.0 section 3.19) for what SARIF
+   calls the "driver" (see SARIF v2.1.0 section 3.18.1).
+   We delay this to ensure that the m_client_data_hooks is set up (e.g. for
+   roundtripping from SARIF to SARIF).  */
 
-sarif_tool_component *
-sarif_builder::make_driver_tool_component_object () const
+void
+sarif_builder::populate_driver_object () const
 {
-  sarif_tool_component *driver_obj = new sarif_tool_component ();
-
   if (m_context->m_client_data_hooks)
 if (const client_version_info *vinfo
  = m_context->m_client_data_hooks->get_any_version_info ())
   {
/* "name" property (SARIF v2.1.0 section 3.19.8).  */
if (const char *name = vinfo->get_tool_name ())
- driver_obj->set ("name", new json::string (name));
+ m_driver_obj->set ("name", new json::string (name));
 
/* "fullName" property (SARIF v2.1.0 section 3.19.9).  */
if (char *full_name = vinfo->maybe_make_full_name ())
  {
-   driver_obj->set ("fullName", new json::string (full_name));
+   m_driver_obj->set ("fullName", new json::string (full_name));
free (full_name);
  }
 
/* "version" property (SARIF v2.1.0 section 3.19.13).  */
if (const char *version = vinfo->get_version_string ())
- driver_obj->set ("version", new json::string (version));
+ m_driver_obj->set ("version", new json::string (version));
 
/* "informationUri" property (SARIF v2.1.0 section 3.19.17).  */
if (char *version_url =  vinfo->maybe_make_version_url ())
  {
-   driver_obj->set ("informationUri", new json::string (version_url));
+   m_driver_obj->set ("informationUri", new json::string 
(version_url));
free (version_url);
  }
   }
-
-  return driver_obj;
 }
 
 /* If we've seen any CWE IDs, make an array for the "taxonomies" property
-- 
2.26.3



[PATCH 06/12] prune.exp: move multiline-handling to before other pruning

2022-06-22 Thread David Malcolm via Gcc-patches
Doing so allows for multiline directives to contain things like
"note: " which would otherwise already have been pruned.

gcc/testsuite/ChangeLog:
* lib/prune.exp (prune_gcc_output): Move multiline-handling to
before other pruning.

Signed-off-by: David Malcolm 
---
 gcc/testsuite/lib/prune.exp | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index 04c6a1dd7a1..fd6584d0e7d 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -40,6 +40,13 @@ proc prune_gcc_output { text } {
 # Remove Windows .exe suffix
 regsub -all "(as|cc1|cc1plus|collect2|f951|ld|lto-wrapper)\.exe?:" $text 
{\1:} text
 
+# If dg-enable-nn-line-numbers was provided, then obscure source-margin
+# line numbers by converting them to "NN" form.
+set text [maybe-handle-nn-line-numbers $text]
+
+# Call into multiline.exp to handle any multiline output directives.
+set text [handle-multiline-outputs $text]
+
 regsub -all "(^|\n)(\[^\n\]*: \[iI\]|I)n ((static member |lambda 
)?function|member|method|(copy 
)?constructor|destructor|instantiation|substitution|program|subroutine|block-data)\[^\n\]*"
 $text "" text
 regsub -all "(^|\n)\[^\n\]*(: )?At (top level|global scope):\[^\n\]*" 
$text "" text
 regsub -all "(^|\n)\[^\n\]*:   (recursively )?required \[^\n\]*" $text "" 
text
@@ -108,13 +115,6 @@ proc prune_gcc_output { text } {
 # Many tests that use visibility will still pass on platforms that don't 
support it.
 regsub -all "(^|\n)\[^\n\]*lto1: warning: visibility attribute not 
supported in this configuration; ignored\[^\n\]*" $text "" text
 
-# If dg-enable-nn-line-numbers was provided, then obscure source-margin
-# line numbers by converting them to "NN" form.
-set text [maybe-handle-nn-line-numbers $text]
-
-# Call into multiline.exp to handle any multiline output directives.
-set text [handle-multiline-outputs $text]
-
 #send_user "After:$text\n"
 
 return $text
-- 
2.26.3



[PATCH 12/12] Work-in-progress of path remapping

2022-06-22 Thread David Malcolm via Gcc-patches
This work-in-progress hacks up the file_cache code in input.cc so that
it can have an optional path remapper, which can map e.g. paths in a
.sarif file to paths relative to that .sarif file, so that the
sarif-replayer can locate and display sources.

gcc/ChangeLog:
* input.cc (file_cache::get_file_path): Replace with...
(file_cache::get_original_file_path): ...this, and...
(file_cache::get_remapped_file_path): ...this.
(file_cache::create): Split "file_path" param into
"original_file_path" and "remapped_file_path".
(file_cache::m_file_path): Replace with...
(file_cache::m_original_file_path): ...this, and...
(file_cache::m_remapped_file_path): ...this.
(total_lines_num): Rename file_path to original_file_path.
(file_cache::lookup_file): Rename file_path to remapped_file_path;
update cache lookup to use remapped file path.
(file_cache::remap_file): New.
(file_cache::set_path_remapper): New.
(diagnostics_file_cache_set_path_remapper): New.
(file_cache_slot::evict): Update for split into a pair of paths.
(file_cache::evicted_cache_tab_entry): Likewise.
(file_cache::add_file): Likewise.
(file_cache_slot::create): Likewise.
(file_cache::file_cache): Initialize m_remapper
(file_cache::~file_cache): Delete m_remapper.
(file_cache::lookup_or_add_file): Remap the file path.
(file_cache_slot::file_cache_slot): Update for changes to fields.
(file_cache_slot::~file_cache_slot): Free the m_remapped_file_path.
* input.h (class path_remapper): New.
(file_cache::set_path_remapper): New decl.
(file_cache::lookup_file): Update decl.
(file_cache::add_file): Update decl.
(file_cache::remap_file): New decl.
(file_cache::m_remapper): New field.
(diagnostics_file_cache_set_path_remapper): New decl.
* sarif/sarif-replay.cc (class sarif_path_remapper): New class.
(sarif_replayer::emit_sarif_as_diagnostics): Use it.

gcc/testsuite/ChangeLog:
* sarif/tutorial-example-foo.sarif: Fix the line numbers.
Update the expected multiline output to show the source code.

Signed-off-by: David Malcolm 
---
 gcc/input.cc  | 107 +-
 gcc/input.h   |  18 ++-
 gcc/sarif/sarif-replay.cc |  39 +++
 .../sarif/tutorial-example-foo.sarif  |  25 +++-
 4 files changed, 155 insertions(+), 34 deletions(-)

diff --git a/gcc/input.cc b/gcc/input.cc
index 2acbfdea4f8..a78b949faa5 100644
--- a/gcc/input.cc
+++ b/gcc/input.cc
@@ -55,7 +55,8 @@ public:
  char ** line, ssize_t *line_len);
 
   /* Accessors.  */
-  const char *get_file_path () const { return m_file_path; }
+  const char *get_original_file_path () const { return m_original_file_path; }
+  const char *get_remapped_file_path () const { return m_remapped_file_path; }
   unsigned get_use_count () const { return m_use_count; }
   bool missing_trailing_newline_p () const
   {
@@ -65,7 +66,10 @@ public:
   void inc_use_count () { m_use_count++; }
 
   bool create (const file_cache::input_context &in_context,
-  const char *file_path, FILE *fp, unsigned highest_use_count);
+  const char *original_file_path,
+  char *remapped_file_path,
+  FILE *fp,
+  unsigned highest_use_count);
   void evict ();
 
  private:
@@ -112,11 +116,14 @@ public:
  array.  */
   unsigned m_use_count;
 
-  /* The file_path is the key for identifying a particular file in
+  /* The m_original_file_path is the key for identifying a particular file in
  the cache.
  For libcpp-using code, the underlying buffer for this field is
  owned by the corresponding _cpp_file within the cpp_reader.  */
-  const char *m_file_path;
+  const char *m_original_file_path;
+
+  // FIXME:
+  char *m_remapped_file_path;
 
   FILE *m_fp;
 
@@ -310,11 +317,11 @@ diagnostic_file_cache_fini (void)
equals the actual number of lines of the file.  */
 
 static size_t
-total_lines_num (const char *file_path)
+total_lines_num (const char *original_file_path)
 {
   size_t r = 0;
   location_t l = 0;
-  if (linemap_get_file_highest_location (line_table, file_path, &l))
+  if (linemap_get_file_highest_location (line_table, original_file_path, &l))
 {
   gcc_assert (l >= RESERVED_LOCATION_COUNT);
   expanded_location xloc = expand_location (l);
@@ -328,16 +335,17 @@ total_lines_num (const char *file_path)
cached file was found.  */
 
 file_cache_slot *
-file_cache::lookup_file (const char *file_path)
+file_cache::lookup_file (const char *remapped_file_path)
 {
-  gcc_assert (file_path);
+  gcc_assert (remapped_file_path);
 
   /* This will contain the found cached file.  */
   file_cache_slot *r = NULL;
   for (unsigned i = 0; i < num_file_slots; ++i)
 {
   file_cache_s

[PATCH 04/12] json: add json parsing support

2022-06-22 Thread David Malcolm via Gcc-patches
This patch implements JSON parsing support.

It's based on the parsing parts of the patch I posted here:
  https://gcc.gnu.org/legacy-ml/gcc-patches/2017-08/msg00417.html
with the parsing moved to a separate source file and header, and heavily
rewritten to capture source location information for JSON values.  I also
added optional support for C and C++ style comments, which is extremely
useful in DejaGnu tests.

gcc/ChangeLog:
* Makefile.in (OBJS-libcommon): Add json-parsing.o.
* json-parsing.cc: New file.
* json-parsing.h: New file.
* json.cc (selftest::assert_print_eq): Remove "static".
* json.h (json::array::begin): New.
(json::array::end): New.
(json::array::length): New.
(json::array::get): New.
(is_a_helper ::test): New.
(is_a_helper ::test): New.
(is_a_helper ::test): New.
(is_a_helper ::test): New.
(is_a_helper ::test): New.
(selftest::assert_print_eq): New.
* selftest-run-tests.cc (selftest::run_tests): Call
selftest::json_parser_cc_tests.
* selftest.h (selftest::json_parser_cc_tests): New decl.

Signed-off-by: David Malcolm 
---
 gcc/Makefile.in   |2 +-
 gcc/json-parsing.cc   | 2391 +
 gcc/json-parsing.h|   94 ++
 gcc/json.cc   |2 +-
 gcc/json.h|   59 +-
 gcc/selftest-run-tests.cc |1 +
 gcc/selftest.h|1 +
 7 files changed, 2546 insertions(+), 4 deletions(-)
 create mode 100644 gcc/json-parsing.cc
 create mode 100644 gcc/json-parsing.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b6dcc45a58a..acdb608f393 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1735,7 +1735,7 @@ OBJS-libcommon = diagnostic-spec.o diagnostic.o 
diagnostic-color.o \
diagnostic-show-locus.o \
edit-context.o \
pretty-print.o intl.o \
-   json.o \
+   json.o json-parsing.o \
sbitmap.o \
vec.o input.o hash-table.o ggc-none.o memory-block.o \
selftest.o selftest-diagnostic.o sort.o
diff --git a/gcc/json-parsing.cc b/gcc/json-parsing.cc
new file mode 100644
index 000..a8b45bdca33
--- /dev/null
+++ b/gcc/json-parsing.cc
@@ -0,0 +1,2391 @@
+/* JSON parsing
+   Copyright (C) 2017-2022 Free Software Foundation, Inc.
+   Contributed by David Malcolm .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "json-parsing.h"
+#include "pretty-print.h"
+#include "math.h"
+#include "selftest.h"
+
+using namespace json;
+
+/* Declarations relating to parsing JSON, all within an
+   anonymous namespace.  */
+
+namespace {
+
+/* A typedef representing a single unicode character.  */
+
+typedef unsigned unichar;
+
+/* An enum for discriminating different kinds of JSON token.  */
+
+enum token_id
+{
+  TOK_ERROR,
+
+  TOK_EOF,
+
+  /* Punctuation.  */
+  TOK_OPEN_SQUARE,
+  TOK_OPEN_CURLY,
+  TOK_CLOSE_SQUARE,
+  TOK_CLOSE_CURLY,
+  TOK_COLON,
+  TOK_COMMA,
+
+  /* Literal names.  */
+  TOK_TRUE,
+  TOK_FALSE,
+  TOK_NULL,
+
+  TOK_STRING,
+  TOK_FLOAT_NUMBER,
+  TOK_INTEGER_NUMBER
+};
+
+/* Human-readable descriptions of enum token_id.  */
+
+static const char *token_id_name[] = {
+  "error",
+  "EOF",
+  "'['",
+  "'{'",
+  "']'",
+  "'}'",
+  "':'",
+  "','",
+  "'true'",
+  "'false'",
+  "'null'",
+  "string",
+  "number",
+  "number"
+};
+
+/* Tokens within the JSON lexer.  */
+
+struct token
+{
+  /* The kind of token.  */
+  enum token_id id;
+
+  /* The location of this token within the unicode
+ character stream.  */
+  location_map::range range;
+
+  union
+  {
+/* Value for TOK_ERROR and TOK_STRING.  */
+char *string;
+
+/* Value for TOK_FLOAT_NUMBER.  */
+double float_number;
+
+/* Value for TOK_INTEGER_NUMBER.  */
+long integer_number;
+  } u;
+};
+
+/* A class for lexing JSON.  */
+
+class lexer
+{
+ public:
+  lexer (bool support_comments);
+  ~lexer ();
+  bool add_utf8 (size_t length, const char *utf8_buf, error **err_out);
+
+  const token *peek ();
+  void consume ();
+
+ private:
+  bool get_char (unichar &out_char, location_map::point *out_point);
+  void unget_char ();
+  location_map::point get_next_point () const;
+  static void dump_token (FILE *outf, const token *tok);
+  void lex_token (token *out);
+  void lex_string (t

[PATCH 09/12] Add json frontend

2022-06-22 Thread David Malcolm via Gcc-patches
This patch adds a new json frontend: json-replayer, which the gcc driver
can invoke on .json files saved with -fdiagnostics-format=json-file.

gcc/ChangeLog:
* json/Make-lang.in: New file.
* json/config-lang.in: New file.
* json/json-frontend.cc: New file.
* json/json-replay.cc: New file.
* json/json-replay.h: New file.
* json/lang-specs.h: New file.
* json/lang.opt: New file.

gcc/testsuite/ChangeLog:
* json/invalid-json-array-missing-comma.json: New test.
* json/invalid-json-array-with-trailing-comma.json: New test.
* json/invalid-json-bad-token.json: New test.
* json/invalid-json-object-missing-comma.json: New test.
* json/invalid-json-object-with-trailing-comma.json: New test.
* json/invalid-jsondump-diag-not-an-object.json: New test.
* json/invalid-jsondump-kind-not-a-string.json: New test.
* json/invalid-jsondump-not-an-array.json: New test.
* json/json.exp: New test.
* json/signal-1.c.json: New test.
* lib/json-dg.exp: New test.
* lib/json.exp: New test.

Signed-off-by: David Malcolm 
---
 gcc/json/Make-lang.in | 131 
 gcc/json/config-lang.in   |  34 +
 gcc/json/json-frontend.cc | 176 +
 gcc/json/json-replay.cc   | 614 ++
 gcc/json/json-replay.h|  26 +
 gcc/json/lang-specs.h |  26 +
 gcc/json/lang.opt |  31 +
 .../invalid-json-array-missing-comma.json |   6 +
 ...nvalid-json-array-with-trailing-comma.json |   6 +
 .../json/invalid-json-bad-token.json  |   6 +
 .../invalid-json-object-missing-comma.json|   7 +
 ...valid-json-object-with-trailing-comma.json |   6 +
 .../invalid-jsondump-diag-not-an-object.json  |   6 +
 .../invalid-jsondump-kind-not-a-string.json   |  20 +
 .../json/invalid-jsondump-not-an-array.json   |   6 +
 gcc/testsuite/json/json.exp   |  50 ++
 gcc/testsuite/json/signal-1.c.json| 131 
 gcc/testsuite/lib/json-dg.exp | 233 +++
 gcc/testsuite/lib/json.exp|  36 +
 19 files changed, 1551 insertions(+)
 create mode 100644 gcc/json/Make-lang.in
 create mode 100644 gcc/json/config-lang.in
 create mode 100644 gcc/json/json-frontend.cc
 create mode 100644 gcc/json/json-replay.cc
 create mode 100644 gcc/json/json-replay.h
 create mode 100644 gcc/json/lang-specs.h
 create mode 100644 gcc/json/lang.opt
 create mode 100644 gcc/testsuite/json/invalid-json-array-missing-comma.json
 create mode 100644 
gcc/testsuite/json/invalid-json-array-with-trailing-comma.json
 create mode 100644 gcc/testsuite/json/invalid-json-bad-token.json
 create mode 100644 gcc/testsuite/json/invalid-json-object-missing-comma.json
 create mode 100644 
gcc/testsuite/json/invalid-json-object-with-trailing-comma.json
 create mode 100644 gcc/testsuite/json/invalid-jsondump-diag-not-an-object.json
 create mode 100644 gcc/testsuite/json/invalid-jsondump-kind-not-a-string.json
 create mode 100644 gcc/testsuite/json/invalid-jsondump-not-an-array.json
 create mode 100644 gcc/testsuite/json/json.exp
 create mode 100644 gcc/testsuite/json/signal-1.c.json
 create mode 100644 gcc/testsuite/lib/json-dg.exp
 create mode 100644 gcc/testsuite/lib/json.exp

diff --git a/gcc/json/Make-lang.in b/gcc/json/Make-lang.in
new file mode 100644
index 000..a62d028f41a
--- /dev/null
+++ b/gcc/json/Make-lang.in
@@ -0,0 +1,131 @@
+# Make-lang.in -- Top level -*- makefile -*- fragment for gcc JSON "frontend".
+
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# This file is part of GCC.
+
+# GCC is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+
+# GCC is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# This file provides the language dependent support in the main Makefile.
+
+# The name for selecting json in LANGUAGES.
+json: json-replay$(exeext)
+
+.PHONY: json
+
+JSON_OBJS = json/json-frontend.o json/json-replay.o \
+   attribs.o deferred-locations.o json-reader.o
+json_OBJS = $(JSON_OBJS)
+
+json-replay$(exeext): $(JSON_OBJS) $(BACKEND) $(LIBDEPS)
+   @$(call LINK_PROGRESS,$(INDEX.json),start)
+   +$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+ $(JSON_OBJS) $(BACKEND) $(LIBS) $(BACKENDLIBS)
+   @$(call LINK_PROGRESS,$(INDEX.json),end)
+
+# Build hooks.
+
+json.all.cross:
+json.sta

Re: [PATCH] Introduce -nolibstdc++ option

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Iain Sandoe  wrote:

> It makes some sense to have the option named -nostdlib++ if a target
> might add multiple libs (and/or make other changes) for linking C++.

if it was nostdlibc++, I'd agree.  lib++ is not something that brings
C++ to (my) mind.

> (so, fo example, if libstdc++ were separate from libsupc++ I would
>  expect your use-case to wish to exclude both, not just libstdc++)?

That's what the testcase requires, yes.  IIRC there's another that would
benefit from the ability to link with libsupc++, but not with libstdc++.

> if GCC already has an option spelling, usually clang would follow that
> - it does not seem unreasonable to reciprocate.

Yeah, I suppose that makes sense, it's beneficial for users to avoid the
cognitive overload of dealing with equivalent options with different
spellings.  I'll swallow my dislike for the spelling and change the
patch to use -nostdlib++.

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


Re: [PATCH] Introduce -nolibstdc++ option

2022-06-22 Thread Fangrui Song via Gcc-patches
On Wed, Jun 22, 2022 at 4:29 PM Alexandre Oliva  wrote:
>
> On Jun 22, 2022, Iain Sandoe  wrote:
>
> > It makes some sense to have the option named -nostdlib++ if a target
> > might add multiple libs (and/or make other changes) for linking C++.
>
> if it was nostdlibc++, I'd agree.  lib++ is not something that brings
> C++ to (my) mind.

Agree that clang --stdlib= and -nostdlib++ probably should be better
named. There are many
standard libraries and "stdlib" as a name isn't tied to C++ much.
That said, --stdlib= has a very long history and seems not so
necessary to change now.
For new Clang driver options (I subscribe to clang/lib/Driver files to
try catching up the change),
I try to keep an eye on and for something useful which may be matched by GCC,
I'll notify some GCC folks I know (e.g. Nathan, Martin).

> > (so, fo example, if libstdc++ were separate from libsupc++ I would
> >  expect your use-case to wish to exclude both, not just libstdc++)?
>
> That's what the testcase requires, yes.  IIRC there's another that would
> benefit from the ability to link with libsupc++, but not with libstdc++.
>
> > if GCC already has an option spelling, usually clang would follow that
> > - it does not seem unreasonable to reciprocate.

Thanks.

> Yeah, I suppose that makes sense, it's beneficial for users to avoid the
> cognitive overload of dealing with equivalent options with different
> spellings.  I'll swallow my dislike for the spelling and change the
> patch to use -nostdlib++.

Thanks:)

BTW: even if -static-libstdc++ is a bit of misnomer when a clang user
uses libc++,
since -static-libc++ or -static-stdlib does not exist, they are still
using -static-libstdc++.

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


Re: [PATCH] libstdc++: 60241.cc: tolerate slightly shorter aggregate sleep

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Sebastian Huber  wrote:

> The clock_nanosleep() uses the coarse resolution

Thanks for looking into this.  So, is it missing a rounding-up to ensure
the sleep time is >= the requested time, or is it even more elaborate
than that?

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


Re: [PATCH] libstdc++: 60241.cc: tolerate slightly shorter aggregate sleep

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Sebastian Huber  wrote:

> The clock_nanosleep() uses the coarse resolution which may give a time
> before now().

Uhh, sorry, hit send too early.

I also meant to ask whether you'd like me to file an RTEMS ticket about
this issue.

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


Re: [PATCH] libstdc++: retry removal of dir entries if dir removal fails

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Jonathan Wakely  wrote:

> It looks like it would be possible for this to livelock.

True

> The current
> implementation will fail with an error in that case, rather than
> getting stuck forever in a loop.

In the equivalent livelock scenario, newly-added dir entries are added
to the end of the directory, and get visited in the same readdir
iteration, so you never reach the end as long as the entry-creator runs
faster than the remover.

> It would be possible to add a __rewind member to directory iterators
> too, to call rewinddir after each modification to the directory.

That would likely lead to O(n^2) behavior in some implementations, in
which remove entries get rescanned over and over, whereas the approach I
proposed cuts that down to O(nlogn).  Unless you rewind once you hit the
end after successful removals, even before trying to remove the dir.
That's still a little wasteful on POSIX-compliant targets, because you
rewind and rescan a dir that should already be empty.  I aimed for
minimizing the overhead on compliant targets, but that was before
remove_all switched to recursive iterators.

With recursive iterators, rewinding might be better done in a custom
iterator, tuned for recursive removal, that knows to rewind a dir if
there were removals in it or something.  Rewinding the entire recursive
removal is not something I realized my rewritten patch did.  I still had
the recursive remove_all implementation in cache.  Oops ;-)

That said, I'm not sure it makes much of a difference in the end.  As
long as the recursion and removal doesn't treat symlinks as dirs (which
IIUC requires openat and unlinkat, so that's a big if to boot), the
rewinding seems to only change the nature of filesystem race conditions
that recursive removal enables.  E.g., consider that you start removing
the entries in a dir, and then the dir you're visiting is moved out of
the subtree you're removing, and other dirs are moved into it: the
recursive removal with openat and unlinkat will happily attempt to wipe
out everything moved in there, even if it wasn't within that subtree at
the time of the remove_all request, and even if the newly-moved dirs
were never part of the subtree whose removal was requested.  To make it
clearer:

  c++::std::filesystem::remove_all foo/bar &
  mv foo/bar/temp temp
  mv foo temp
  wait
  ls -d temp/foo

temp/foo might be removed if you happened to be iterating in temp when
the 'mv' commands run.  Is that another kind of race that needs to be
considered?  If a dir is moved while we're visiting it, should we stop
visiting it?  What about its parent?

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


RE: gcc-patches@gcc.gnu.org

2022-06-22 Thread thereisnotimeforsleep1--- via Gcc-patches
<>


PING^2 [PATCH] rs6000: Handle unresolved overloaded builtin [PR105485]

2022-06-22 Thread Kewen.Lin via Gcc-patches
Hi,

Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html

BR,
Kewen


> on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote:
>> Hi,
>>
>> PR105485 exposes that new builtin function framework doesn't handle
>> unresolved overloaded builtin function well.  With new builtin
>> function support, we don't have builtin info for any overloaded
>> rs6000_gen_builtins enum, since they are expected to be resolved to
>> one specific instance.  So when function rs6000_gimple_fold_builtin
>> faces one unresolved overloaded builtin, the access for builtin info
>> becomes out of bound and gets ICE then.
>>
>> We should not try to fold one unresolved overloaded builtin there
>> and as the previous support we should emit one error message during
>> expansion phase like "unresolved overload for builtin ...".
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
>> powerpc64le-linux-gnu P9 and P10.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -
>>  PR target/105485
>>
>> gcc/ChangeLog:
>>
>>  * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add
>>  the handling for unresolved overloaded builtin function.
>>  (rs6000_expand_builtin): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  * g++.target/powerpc/pr105485.C: New test.
>>
>> ---
>>  gcc/config/rs6000/rs6000-builtin.cc | 13 +
>>  gcc/testsuite/g++.target/powerpc/pr105485.C |  9 +
>>  2 files changed, 22 insertions(+)
>>  create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtin.cc 
>> b/gcc/config/rs6000/rs6000-builtin.cc
>> index e925ba9fad9..e102305c90c 100644
>> --- a/gcc/config/rs6000/rs6000-builtin.cc
>> +++ b/gcc/config/rs6000/rs6000-builtin.cc
>> @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
>>enum tree_code bcode;
>>gimple *g;
>>
>> +  /* For an unresolved overloaded builtin, return early here since there
>> + is no builtin info for it and we are unable to fold it.  */
>> +  if (fn_code > RS6000_OVLD_NONE)
>> +return false;
>> +
>>size_t uns_fncode = (size_t) fn_code;
>>enum insn_code icode = rs6000_builtin_info[uns_fncode].icode;
>>const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname;
>> @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* 
>> subtarget */,
>>tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
>>enum rs6000_gen_builtins fcode
>>  = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl);
>> +
>> +  /* Emit error message if it's an unresolved overloaded builtin.  */
>> +  if (fcode > RS6000_OVLD_NONE)
>> +{
>> +  error ("unresolved overload for builtin %qF", fndecl);
>> +  return const0_rtx;
>> +}
>> +
>>size_t uns_fcode = (size_t)fcode;
>>enum insn_code icode = rs6000_builtin_info[uns_fcode].icode;
>>
>> diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C 
>> b/gcc/testsuite/g++.target/powerpc/pr105485.C
>> new file mode 100644
>> index 000..a3b8290df8c
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C
>> @@ -0,0 +1,9 @@
>> +/* It's to verify no ICE here, ignore error/warning messages since
>> +   they are not test points here.  */
>> +/* { dg-excess-errors "pr105485" } */
>> +
>> +template  void __builtin_vec_vslv();
>> +typedef  __attribute__((altivec(vector__))) char T;
>> +T b (T c, T d) {
>> +return __builtin_vec_vslv(c, d);
>> +}


PING^2 [PATCH v3] rs6000: Fix the check of bif argument number [PR104482]

2022-06-22 Thread Kewen.Lin via Gcc-patches
Hi,

Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595208.html

BR,
Kewen

>> Hi,
>>
>> As PR104482 shown, it's one regression about the handlings when
>> the argument number is more than the one of built-in function
>> prototype.  The new bif support only catches the case that the
>> argument number is less than the one of function prototype, but
>> it misses the case that the argument number is more than the one
>> of function prototype.  Because it uses "n != expected_args",
>> n is updated in
>>
>>for (n = 0; !VOID_TYPE_P (TREE_VALUE (fnargs)) && n < nargs;
>> fnargs = TREE_CHAIN (fnargs), n++)
>>
>> , it's restricted to be less than or equal to expected_args with
>> the guard !VOID_TYPE_P (TREE_VALUE (fnargs)), so it's wrong.
>>
>> The fix is to use nargs instead, also move the checking hunk's
>> location ahead to avoid useless further scanning when the counts
>> mismatch.
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
>> powerpc64le-linux-gnu P9 and P10.
>>
>> v3: Update test case with dg-excess-errors.
>>
>> v2: Add one test case and refine commit logs.
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593155.html
>>
>> v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591768.html
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -
>>  PR target/104482
>>
>> gcc/ChangeLog:
>>
>>  * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): Fix
>>  the equality check for argument number, and move this hunk ahead.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  * gcc.target/powerpc/pr104482.c: New test.
>> ---
>>  gcc/config/rs6000/rs6000-c.cc   | 60 ++---
>>  gcc/testsuite/gcc.target/powerpc/pr104482.c | 16 ++
>>  2 files changed, 46 insertions(+), 30 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr104482.c
>>
>> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
>> index 9c8cbd7a66e..61881f29230 100644
>> --- a/gcc/config/rs6000/rs6000-c.cc
>> +++ b/gcc/config/rs6000/rs6000-c.cc
>> @@ -1756,6 +1756,36 @@ altivec_resolve_overloaded_builtin (location_t loc, 
>> tree fndecl,
>>vec *arglist = static_cast *> 
>> (passed_arglist);
>>unsigned int nargs = vec_safe_length (arglist);
>>
>> +  /* If the number of arguments did not match the prototype, return NULL
>> + and the generic code will issue the appropriate error message.  Skip
>> + this test for functions where we don't fully describe all the possible
>> + overload signatures in rs6000-overload.def (because they aren't 
>> relevant
>> + to the expansion here).  If we don't, we get confusing error messages. 
>>  */
>> +  /* As an example, for vec_splats we have:
>> +
>> +; There are no actual builtins for vec_splats.  There is special handling 
>> for
>> +; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call
>> +; is replaced by a constructor.  The single overload here causes
>> +; __builtin_vec_splats to be registered with the front end so that can 
>> happen.
>> +[VEC_SPLATS, vec_splats, __builtin_vec_splats]
>> +  vsi __builtin_vec_splats (vsi);
>> +ABS_V4SI SPLATS_FAKERY
>> +
>> +So even though __builtin_vec_splats accepts all vector types, the
>> +infrastructure cheats and just records one prototype.  We end up getting
>> +an error message that refers to this specific prototype even when we
>> +are handling a different argument type.  That is completely confusing
>> +to the user, so it's best to let these cases be handled individually
>> +in the resolve_vec_splats, etc., helper functions.  */
>> +
>> +  if (expected_args != nargs
>> +  && !(fcode == RS6000_OVLD_VEC_PROMOTE
>> +   || fcode == RS6000_OVLD_VEC_SPLATS
>> +   || fcode == RS6000_OVLD_VEC_EXTRACT
>> +   || fcode == RS6000_OVLD_VEC_INSERT
>> +   || fcode == RS6000_OVLD_VEC_STEP))
>> +return NULL;
>> +
>>for (n = 0;
>> !VOID_TYPE_P (TREE_VALUE (fnargs)) && n < nargs;
>> fnargs = TREE_CHAIN (fnargs), n++)
>> @@ -1816,36 +1846,6 @@ altivec_resolve_overloaded_builtin (location_t loc, 
>> tree fndecl,
>>types[n] = type;
>>  }
>>
>> -  /* If the number of arguments did not match the prototype, return NULL
>> - and the generic code will issue the appropriate error message.  Skip
>> - this test for functions where we don't fully describe all the possible
>> - overload signatures in rs6000-overload.def (because they aren't 
>> relevant
>> - to the expansion here).  If we don't, we get confusing error messages. 
>>  */
>> -  /* As an example, for vec_splats we have:
>> -
>> -; There are no actual builtins for vec_splats.  There is special handling 
>> for
>> -; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call
>> -; is replaced by a constructor.  The single overload here causes
>> -; __builtin_vec_splats to be registered with the front end so that can 
>> happen.
>> -[VEC_SPLATS, 

PING^2 [PATCH v3] rs6000: Adjust mov optabs for opaque modes [PR103353]

2022-06-22 Thread Kewen.Lin via Gcc-patches
Hi,

Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595209.html

BR,
Kewen

>> Hi,
>>
>> As PR103353 shows, we may want to continue to expand a MMA built-in
>> function like a normal function, even if we have already emitted
>> error messages about some missing required conditions.  As shown in
>> that PR, without one explicit mov optab on OOmode provided, it would
>> call emit_move_insn recursively.
>>
>> So this patch is to allow the mov pattern to be generated when we are
>> expanding to RTL and have seen errors even without MMA supported, it's
>> expected that the generated pattern would not cause further ICEs as the
>> compilation would stop soon after expanding.
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
>> powerpc64le-linux-gnu P9 and P10.
>>
>> v3: Update test case with dg-excess-errors.
>>
>> v2: Polish some comments and add one test case as Will and Peter suggested.
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592916.html
>>
>> v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591150.html
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -
>>  PR target/103353
>>
>> gcc/ChangeLog:
>>
>>  * config/rs6000/mma.md (define_expand movoo): Move TARGET_MMA condition
>>  check to preparation statements and add handlings for !TARGET_MMA.
>>  (define_expand movxo): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  * gcc.target/powerpc/pr103353.c: New test.
>> ---
>>  gcc/config/rs6000/mma.md| 42 ++---
>>  gcc/testsuite/gcc.target/powerpc/pr103353.c | 22 +++
>>  2 files changed, 58 insertions(+), 6 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103353.c
>>
>> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
>> index 907c9d6d516..746a77a0957 100644
>> --- a/gcc/config/rs6000/mma.md
>> +++ b/gcc/config/rs6000/mma.md
>> @@ -268,10 +268,25 @@ (define_int_attr avvi4i4i4 
>> [(UNSPEC_MMA_PMXVI8GER4PP   "pmxvi8ger4pp")
>>  (define_expand "movoo"
>>[(set (match_operand:OO 0 "nonimmediate_operand")
>>  (match_operand:OO 1 "input_operand"))]
>> -  "TARGET_MMA"
>> +  ""
>>  {
>> -  rs6000_emit_move (operands[0], operands[1], OOmode);
>> -  DONE;
>> +  if (TARGET_MMA) {
>> +rs6000_emit_move (operands[0], operands[1], OOmode);
>> +DONE;
>> +  }
>> +  /* Opaque modes are only expected to be available when MMA is supported,
>> + but PR103353 shows we may want to continue to expand a MMA built-in
>> + function, even if we have already emitted error messages about some
>> + missing required conditions.  As shown in that PR, without one
>> + explicit mov optab on OOmode provided, it would call emit_move_insn
>> + recursively.  So we allow this pattern to be generated when we are
>> + expanding to RTL and have seen errors, even though there is no MMA
>> + support.  It would not cause further ICEs as the compilation would
>> + stop soon after expanding.  */
>> +  else if (currently_expanding_to_rtl && seen_error ())
>> +;
>> +  else
>> +gcc_unreachable ();
>>  })
>>
>>  (define_insn_and_split "*movoo"
>> @@ -300,10 +315,25 @@ (define_insn_and_split "*movoo"
>>  (define_expand "movxo"
>>[(set (match_operand:XO 0 "nonimmediate_operand")
>>  (match_operand:XO 1 "input_operand"))]
>> -  "TARGET_MMA"
>> +  ""
>>  {
>> -  rs6000_emit_move (operands[0], operands[1], XOmode);
>> -  DONE;
>> +  if (TARGET_MMA) {
>> +rs6000_emit_move (operands[0], operands[1], XOmode);
>> +DONE;
>> +  }
>> +  /* Opaque modes are only expected to be available when MMA is supported,
>> + but PR103353 shows we may want to continue to expand a MMA built-in
>> + function, even if we have already emitted error messages about some
>> + missing required conditions.  As shown in that PR, without one
>> + explicit mov optab on XOmode provided, it would call emit_move_insn
>> + recursively.  So we allow this pattern to be generated when we are
>> + expanding to RTL and have seen errors, even though there is no MMA
>> + support.  It would not cause further ICEs as the compilation would
>> + stop soon after expanding.  */
>> +  else if (currently_expanding_to_rtl && seen_error ())
>> +;
>> +  else
>> +gcc_unreachable ();
>>  })
>>
>>  (define_insn_and_split "*movxo"
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103353.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr103353.c
>> new file mode 100644
>> index 000..5d519fb1b7b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr103353.c
>> @@ -0,0 +1,22 @@
>> +/* { dg-require-effective-target powerpc_altivec_ok } */
>> +/* If the default cpu type is power10 or later, MMA is enabled by default.
>> +   To keep the test point available all the time, this case specifies
>> +   -mdejagnu-cpu=power6 to make it be tested without MMA.  */
>> +/* { dg-options "-maltivec -mdejagnu-cpu=power6" } */
>> +
>> +/* Verify there is no ICE and d

PING^1 [PATCH] inline: Rebuild target option node for caller [PR105459]

2022-06-22 Thread Kewen.Lin via Gcc-patches
Hi,

Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596212.html

BR,
Kewen

on 2022/6/6 14:20, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> PR105459 exposes one issue in inline_call handling that when it
> decides to copy FP flags from callee to caller and rebuild the
> optimization node for caller fndecl, it's possible that the target
> option node is also necessary to be rebuilt.  Without updating
> target option node early, it can make nodes share the same target
> option node wrongly, later when we want to unshare it somewhere
> (like in target hook) it can get unexpected results, like ICE on
> uninitialized secondary member of target globals exposed in this PR.
> 
> Commit r12-3721 makes it get exact fp_expression info and causes
> more optimization chances then exposes this issue.  Commit r11-5855
> introduces two target options to shadow flag_excess_precision and
> flag_unsafe_math_optimizations and shows the need to rebuild target
> node in inline_call when optimization node changes.
> 
> As commented in PR105459, I tried to postpone init_function_start
> in cgraph_node::expand, but abandoned it since I thought it just
> concealed the issue.  And I also tried to adjust the target node
> when current function switching, but failed since we get the NULL
> cfun and fndecl in WPA phase.
> 
> Bootstrapped and regtested on x86_64-redhat-linux, powerpc64-linux-gnu
> P8 and powerpc64le-linux-gnu P9.
> 
> Any thoughts?  Is it OK for trunk?
> 
> BR,
> Kewen
> -
> 
>   PR tree-optimization/105459
> 
> gcc/ChangeLog:
> 
>   * ipa-inline-transform.cc (inline_call): Rebuild target option node
>   once optimization node gets rebuilt.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.dg/lto/pr105459_0.c: New test.
> ---
>  gcc/ipa-inline-transform.cc   | 50 +--
>  gcc/testsuite/gcc.dg/lto/pr105459_0.c | 35 +++
>  2 files changed, 83 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr105459_0.c
> 
> diff --git a/gcc/ipa-inline-transform.cc b/gcc/ipa-inline-transform.cc
> index 07288e57c73..edba58377f4 100644
> --- a/gcc/ipa-inline-transform.cc
> +++ b/gcc/ipa-inline-transform.cc
> @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-modref.h"
>  #include "symtab-thunks.h"
>  #include "symtab-clones.h"
> +#include "target.h"
> 
>  int ncalls_inlined;
>  int nfunctions_inlined;
> @@ -469,8 +470,53 @@ inline_call (struct cgraph_edge *e, bool update_original,
>  }
> 
>/* Reload global optimization flags.  */
> -  if (reload_optimization_node && DECL_STRUCT_FUNCTION (to->decl) == cfun)
> -set_cfun (cfun, true);
> +  if (reload_optimization_node)
> +{
> +  /* Only need to check and update target option node
> +  when target_option_default_node is not NULL.  */
> +  if (target_option_default_node)
> + {
> +   /* Save the current context for optimization and target option
> +  node.  */
> +   tree old_optimize
> + = build_optimization_node (&global_options, &global_options_set);
> +   tree old_target_opt
> + = build_target_option_node (&global_options, &global_options_set);
> +
> +   /* Restore optimization with new optimizatin node.  */
> +   tree new_optimize = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (to->decl);
> +   if (old_optimize != new_optimize)
> + cl_optimization_restore (&global_options, &global_options_set,
> +  TREE_OPTIMIZATION (new_optimize));
> +
> +   /* Restore target option with the one from caller fndecl.  */
> +   tree cur_target_opt = DECL_FUNCTION_SPECIFIC_TARGET (to->decl);
> +   if (!cur_target_opt)
> + cur_target_opt = target_option_default_node;
> +   cl_target_option_restore (&global_options, &global_options_set,
> + TREE_TARGET_OPTION (cur_target_opt));
> +
> +   /* Update target option as optimization changes.  */
> +   targetm.target_option.override ();
> +
> +   /* Rebuild target option node for caller fndecl and replace
> +  with it if the node changes.  */
> +   tree new_target_opt
> + = build_target_option_node (&global_options, &global_options_set);
> +   if (cur_target_opt != new_target_opt)
> + DECL_FUNCTION_SPECIFIC_TARGET (to->decl) = new_target_opt;
> +
> +   /* Restore the context with previous saved nodes.  */
> +   if (old_optimize != new_optimize)
> + cl_optimization_restore (&global_options, &global_options_set,
> +  TREE_OPTIMIZATION (old_optimize));
> +   cl_target_option_restore (&global_options, &global_options_set,
> + TREE_TARGET_OPTION (old_target_opt));
> + }
> +
> +  if (DECL_STRUCT_FUNCTION (to->decl) == cfun)
> + set_cfun (cfun, true);
> +}
> 
>/* If aliases are involved, redirect edge to the actual destination and
> 

[PATCH] Fix ICE when multiple speculative targets are expanded in different ltrans unit [PR93318]

2022-06-22 Thread Xionghu Luo via Gcc-patches
There is a corner case for speculative multiple targets, that if speculative
edges are streamed to different ltrans units, and then edges are expanded
in one ltrans unit but the speculative property is not cleared by
resolve_speculation in other ltrans unit finally cause ICE.  This patch
fixes this by adding checking to guard speculative edge without no
indirect edge binded to it.  No case is provided since this is from
large program with 128 LTO ltrans units not easy to reduce originated
from protobuf.

Bootstrap and regression tested pass on x86_64-linux-gnu, OK for master?

Signed-off-by: Xionghu Luo 

gcc/ChangeLog:

PR ipa/93318
* cgraph.cc (cgraph_edge::resolve_speculation): Remove
speculative info if no indirect edge found.
* cgraph.h: Return NULL if the edges are resloved in other
ltrans units.
* tree-inline.cc (copy_bb): Only clone current edge if the
speculative call is processed in other ltrans units.

Signed-off-by: Xionghu Luo 
---
 gcc/cgraph.cc  |  7 +++
 gcc/cgraph.h   |  3 ++-
 gcc/tree-inline.cc | 11 +++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index 7eeda53ca84..120c0ac7650 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -1217,6 +1217,13 @@ cgraph_edge::resolve_speculation (cgraph_edge *edge, 
tree callee_decl)
 e2 = edge;
   ref = e2->speculative_call_target_ref ();
   edge = edge->speculative_call_indirect_edge ();
+  if (!edge)
+  {
+e2->speculative = false;
+ref->remove_reference ();
+return e2;
+  }
+
   if (!callee_decl
   || !ref->referred->semantically_equivalent_p
   (symtab_node::get (callee_decl)))
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 4be67e3cea9..5404f023e31 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1773,11 +1773,12 @@ public:
 if (!callee)
   return this;
 for (cgraph_edge *e2 = caller->indirect_calls;
-true; e2 = e2->next_callee)
+e2; e2 = e2->next_callee)
   if (e2->speculative
  && call_stmt == e2->call_stmt
  && lto_stmt_uid == e2->lto_stmt_uid)
return e2;
+return NULL;
   }
 
   /* When called on any edge in speculative call and when given any target
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 043e1d5987a..777d9efdd70 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -2262,6 +2262,17 @@ copy_bb (copy_body_data *id, basic_block bb,
 
  cgraph_edge *indirect
 = old_edge->speculative_call_indirect_edge ();
+ if (indirect == NULL
+ && old_edge->num_speculative_call_targets_p ()
+  == 0)
+   {
+ cgraph_edge::resolve_speculation (old_edge);
+ edge = old_edge->clone (id->dst_node, call_stmt,
+ gimple_uid (stmt), num,
+ den, true);
+ edge->count = copy_basic_block->count;
+ break;
+   }
  profile_count indir_cnt = indirect->count;
 
  /* Next iterate all direct edges, clone it and its
-- 
2.27.0



[PATCH] Fix typo

2022-06-22 Thread Xionghu Luo via Gcc-patches
Fix typo and commit as obvious.

Signed-off-by: Xionghu Luo 

gcc/ChangeLog:

* cgraph.cc (cgraph_edge::redirect_call_stmt_to_callee): Fix
typo.
* tree-ssa-loop-ivopts.cc (struct iv_cand): Likewise.
* tree-switch-conversion.h: Likewise.
---
 gcc/cgraph.cc| 2 +-
 gcc/tree-ssa-loop-ivopts.cc  | 2 +-
 gcc/tree-switch-conversion.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index 7eeda53ca84..8d6ed38efa2 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -1423,7 +1423,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
   else
{
  /* Be sure we redirect all speculative targets before poking
-abou tindirect edge.  */
+about indirect edge.  */
  gcc_checking_assert (e->callee);
  cgraph_edge *indirect = e->speculative_call_indirect_edge ();
  gcall *new_stmt;
diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
index 549168aebd6..a6f926a68ef 100644
--- a/gcc/tree-ssa-loop-ivopts.cc
+++ b/gcc/tree-ssa-loop-ivopts.cc
@@ -469,7 +469,7 @@ struct iv_cand
   bitmap inv_vars; /* The list of invariant ssa_vars used in step of the
   iv_cand.  */
   bitmap inv_exprs;/* If step is more complicated than a single ssa_var,
-  hanlde it as a new invariant expression which will
+  handle it as a new invariant expression which will
   be hoisted out of loop.  */
   struct iv *orig_iv;  /* The original iv if this cand is added from biv with
   smaller type.  */
diff --git a/gcc/tree-switch-conversion.h b/gcc/tree-switch-conversion.h
index 2b677d9f7e9..4063a6c16a0 100644
--- a/gcc/tree-switch-conversion.h
+++ b/gcc/tree-switch-conversion.h
@@ -249,7 +249,7 @@ public:
 
 /* Concrete subclass of group_cluster representing a collection
of cases to be implemented as a jump table.
-   The "emit" vfunc gernerates a nested switch statement which
+   The "emit" vfunc generates a nested switch statement which
is later lowered to a jump table.  */
 
 class jump_table_cluster: public group_cluster
-- 
2.27.0



[pushed] c++: dependence of baselink [PR105964]

2022-06-22 Thread Jason Merrill via Gcc-patches
helper::c isn't dependent just because we haven't deduced its return
type yet.  type_dependent_expression_p already knows how to deal with that
for bare FUNCTION_DECL, but needs to learn to look through a BASELINK.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/105964

gcc/cp/ChangeLog:

* pt.cc (type_dependent_expression_p): Look through BASELINK.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/nontype-auto21.C: New test.
---
 gcc/cp/pt.cc|  9 +
 gcc/testsuite/g++.dg/cpp1z/nontype-auto21.C | 20 
 2 files changed, 29 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto21.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 28edc6ae988..4d1c325432d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27960,6 +27960,15 @@ type_dependent_expression_p (tree expression)
   && DECL_INITIAL (expression))
return true;
 
+  /* Pull a FUNCTION_DECL out of a BASELINK if we can.  */
+  if (BASELINK_P (expression))
+{
+  if (BASELINK_OPTYPE (expression)
+ && dependent_type_p (BASELINK_OPTYPE (expression)))
+   return true;
+  expression = BASELINK_FUNCTIONS (expression);
+}
+
   /* A function or variable template-id is type-dependent if it has any
  dependent template arguments.  */
   if (VAR_OR_FUNCTION_DECL_P (expression)
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto21.C 
b/gcc/testsuite/g++.dg/cpp1z/nontype-auto21.C
new file mode 100644
index 000..376d63269cb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto21.C
@@ -0,0 +1,20 @@
+// PR c++/105964
+// { dg-do compile { target c++17 } }
+
+struct token {};
+
+struct example {};
+
+template< typename >
+struct helper
+{
+static constexpr auto c() { return 42; }
+};
+
+struct impostor_c
+{
+template< typename T, auto= helper< T >::c >
+static example func();
+};
+
+example c= impostor_c::func< token >();

base-commit: 349a39f061407ce2339d2ba25da97005f2030c88
-- 
2.27.0



[pushed] c++: tweak deduction with auto template parms

2022-06-22 Thread Jason Merrill via Gcc-patches
While looking at PR105964 I noticed that we were unnecessarily repeating
the deduction loop because of seeing a non-type parameter with type 'auto'.
It is indeed dependent, but not on any other deductions.

Tested x86_64-pc-linux-gnu, applying to trunk.

gcc/cp/ChangeLog:

* pt.cc (type_unification_real): An auto tparm can't
be affected by other deductions.
---
 gcc/cp/pt.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 4d1c325432d..80d2bec2348 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -22982,6 +22982,7 @@ type_unification_real (tree tparms,
 deduced from a later argument than the one from which
 this parameter can be deduced.  */
  if (TREE_CODE (tparm) == PARM_DECL
+ && !is_auto (TREE_TYPE (tparm))
  && uses_template_parms (TREE_TYPE (tparm))
  && saw_undeduced < 2)
{
@@ -23042,6 +23043,7 @@ type_unification_real (tree tparms,
 
  if (saw_undeduced == 1
  && TREE_CODE (parm) == PARM_DECL
+ && !is_auto (TREE_TYPE (parm))
  && uses_template_parms (TREE_TYPE (parm)))
{
  /* The type of this non-type parameter depends on undeduced

base-commit: 349a39f061407ce2339d2ba25da97005f2030c88
prerequisite-patch-id: d5622f522428b601200c8c234ddb5e58c91dcb06
-- 
2.27.0



[PATCH] Improve reg_or_subregno to return INVALID_REGNUM when the subreg of memory is processed.

2022-06-22 Thread liuhongt via Gcc-patches
Hi:
  This is follow-up to [1], return INVALID_REGNUM instead of gcc_assert,
  also adjust some conditions guarded for reg_or_subregno.

>OK, but I think that reg_or_subregno should be improved to return
>INVALID_REGNUM when the subreg of memory is processed.
>
>Thanks,
>Uros.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596841.html

Bootstrapped and regtested on x86_64-pc-linux{-m32,}.
Ok for trunk?

gcc/ChangeLog:

* combine.cc (try_combine): Replace condition with regno !=
INVALID_REGNUM.
* jump.cc (reg_or_subregno): Remove condition for (REG_P (x) ||
(SUBREG_P (x) && REG_P (SUBREG_REG (x.
* reload.cc (push_reload): Ditto.
---
 gcc/combine.cc | 7 ++-
 gcc/jump.cc| 4 ++--
 gcc/reload.cc  | 4 
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 9a34ef847aa..3e6feb323f7 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -4245,12 +4245,9 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, 
rtx_insn *i0,
   if (GET_CODE (x) == PARALLEL)
x = XVECEXP (newi2pat, 0, 0);
 
-  if (REG_P (SET_DEST (x))
- || (GET_CODE (SET_DEST (x)) == SUBREG
- && REG_P (SUBREG_REG (SET_DEST (x)
+  unsigned int regno = reg_or_subregno (SET_DEST (x));
+  if (regno != INVALID_REGNUM)
{
- unsigned int regno = reg_or_subregno (SET_DEST (x));
-
  bool done = false;
  for (rtx_insn *insn = NEXT_INSN (i3);
   !done
diff --git a/gcc/jump.cc b/gcc/jump.cc
index 332f86878e1..dd8fb50e08e 100644
--- a/gcc/jump.cc
+++ b/gcc/jump.cc
@@ -1889,6 +1889,6 @@ reg_or_subregno (const_rtx reg)
 {
   if (GET_CODE (reg) == SUBREG)
 reg = SUBREG_REG (reg);
-  gcc_assert (REG_P (reg));
-  return REGNO (reg);
+
+  return REG_P (reg) ? REGNO (reg) : INVALID_REGNUM;
 }
diff --git a/gcc/reload.cc b/gcc/reload.cc
index 3ed901e3944..c7f4fc9d965 100644
--- a/gcc/reload.cc
+++ b/gcc/reload.cc
@@ -1369,8 +1369,6 @@ push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
 
   if (subreg_in_class == NO_REGS
  && in != 0
- && (REG_P (in)
- || (GET_CODE (in) == SUBREG && REG_P (SUBREG_REG (in
  && reg_or_subregno (in) < FIRST_PSEUDO_REGISTER)
subreg_in_class = REGNO_REG_CLASS (reg_or_subregno (in));
   /* If a memory location is needed for the copy, make one.  */
@@ -1401,8 +1399,6 @@ push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
   n_reloads++;
 
   if (out != 0
-  && (REG_P (out)
- || (GET_CODE (out) == SUBREG && REG_P (SUBREG_REG (out
  && reg_or_subregno (out) < FIRST_PSEUDO_REGISTER
  && (targetm.secondary_memory_needed
  (outmode, rclass, REGNO_REG_CLASS (reg_or_subregno (out)
-- 
2.18.1



Re: [PATCH] libstdc++: testsuite: fs rename to self may fail

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Jonathan Wakely  wrote:

> On Wed, 22 Jun 2022 at 08:02, Alexandre Oliva via Libstdc++
>  wrote:
>> 
>> Hello, Sebastian,
>> 
>> On Jun 22, 2022, Sebastian Huber  wrote:
>> 
>> > On 22/06/2022 08:24, Alexandre Oliva via Libstdc++ wrote:
>> >> rtems6's rename() implementation errors with EEXIST when the rename-to
>> >> filename exists, even when renaming a file to itself or when renaming
>> >> a nonexisting file.  Adjust expectations.
>> >>
>> >> Regstrapped on x86_64-linux-gnu, also tested with a cross to
>> >> aarch64-rtems6.  Ok to install?
>> >>
>> >> PS:https://devel.rtems.org/ticket/2169  doesn't seem to suggest plans to
>> >> change behavior so as to comply with POSIX.
>> 
>> > I would not adjust the test case to cope with systems which are not in
>> > line with POSIX.
>> 
>> My understanding is that the libstdc++ testsuite is not meant to test
>> for POSIX conformance, but for conformance with the C++ language
>> standards.
>> 
>> C++ inherits rename from C, and C says the behavior is implementation
>> defined if the new name already exists.

> std::filesystem::rename is explicitly specified in terms of POSIX
> rename, not C rename.

Oh, sorry, I stand corrected.  I meant ::rename only.  My mind seems to
still resist the notion that  is standard C++, rather than
an experimental extension proposal :-)

Anyway, I did not realize it deferred to POSIX semantics, even when
that's stricter than C standard.  Interesting...

> Instead, the implementation of std::filesystem::rename should have a
> special-case for rtems (and maybe other targets) that implements the
> POSIX rename semantics if calling ::rename isn't good enough.

Given what I've just learned, I agree.  Patch withdrawn.  I suppose the
other patch, on subdir renaming, must share the same fate.

/me longs for XFAILs for libstdc++ subtests ;-)

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


Re: [PATCH] libstdc++: testsuite: test symlnks ifdef _GLIBCXX_HAVE_SYMLINK

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Jonathan Wakely  wrote:

> P.S. there's a typo in the Subject: "symlnks" not "symlinks". I don't
> know if you intend to use that as the Git commit summary line.

Thanks, I would have, fixed.  I ended up introducing the feature
abstraction macros in testsuite_fs.h, so I'll shortly post new versions
of those patches for review.

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


Re: [PATCH] libstdc++-v3: check for openat

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Jonathan Wakely  wrote:

> Otherwise, if rterms defines HAVE_DIRFD this function will return a
> file descriptor and a filename (not a full path) but then
> _Dir_base::openat doesn't use ::openat and so ignores the file
> descriptor, and needs a full path.

Yuck.  It does.  This may explain some of the fails I've observed and
not looked into yet.


How about introducing an internal wrapper class that can hold a ref to
base path or an fd for a dir, or an iterator that could resolve to
either, and that can be passed around instead of a dirfd that assumes
openat, enabling uses of openat where available and falling back to
paths otherwise?  I don't have much of a sense of all the uses involved,
but given the AFAICT impossibility of turning a dirfd into a pathname
(even in non-pathological cases of removed or out-of-chroot dirfds),
ISTM this wrapper class could demand base paths or CWD in the
!HAVE_OPENAT case, and enable both uses otherwise, offering some
additional type safety that the current abstraction doesn't.

Does that make sense to you?

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


Re: [PATCH] Inline memchr with a small constant string

2022-06-22 Thread Richard Biener via Gcc-patches
On Wed, Jun 22, 2022 at 7:13 PM H.J. Lu  wrote:
>
> On Wed, Jun 22, 2022 at 4:39 AM Richard Biener
>  wrote:
> >
> > On Tue, Jun 21, 2022 at 11:03 PM H.J. Lu via Gcc-patches
> >  wrote:
> > >
> > > When memchr is applied on a constant string of no more than the bytes of
> > > a word, inline memchr by checking each byte in the constant string.
> > >
> > > int f (int a)
> > > {
> > >return  __builtin_memchr ("eE", a, 2) != 0;
> > > }
> > >
> > > is simplified to
> > >
> > > int f (int a)
> > > {
> > >   return (char) a == 'e' || (char) a == 'E';
> > > }
> > >
> > > gcc/
> > >
> > > PR tree-optimization/103798
> > > * match.pd (__builtin_memchr (const_str, a, N)): Inline memchr
> > > with constant strings of no more than the bytes of a word.
> >
> > Please do this in strlenopt or so, with match.pd you will end up moving
> > the memchr loads across possible aliasing stores to the point of the
> > comparison.
>
> strlenopt is run after many other passes.  The code won't be well optimized.

What followup optimizations do you expect?  That is, other builtins are only
expanded inline at RTL expansion time?

> Since we are only optimizing
>
> __builtin_memchr ("eE", a, 2) != 0;
>
> I don't see any aliasing store issues here.

Ah, I failed to see the STRING_CST restriction.  Note that when optimizing for
size this doesn't look very good.

I would expect a target might produce some vector code for
memchr ("aAbBcCdDeE...", c, 9) != 0 by splatting 'c', doing
a v16qimode compare, masking off excess elements beyond length
and then comparing against zero or for == 0 against all-ones.

The repetitive pattern result also suggests an implementation elsewhere,
if you think strlenopt is too late there would be forwprop as well.

Richard.



> > Richard.
> >
> > > gcc/testsuite/
> > >
> > > PR tree-optimization/103798
> > > * c-c++-common/pr103798-1.c: New test.
> > > * c-c++-common/pr103798-2.c: Likewise.
> > > * c-c++-common/pr103798-3.c: Likewise.
> > > * c-c++-common/pr103798-4.c: Likewise.
> > > * c-c++-common/pr103798-5.c: Likewise.
> > > * c-c++-common/pr103798-6.c: Likewise.
> > > * c-c++-common/pr103798-7.c: Likewise.
> > > * c-c++-common/pr103798-8.c: Likewise.
> > > ---
> > >  gcc/match.pd| 136 
> > >  gcc/testsuite/c-c++-common/pr103798-1.c |  28 +
> > >  gcc/testsuite/c-c++-common/pr103798-2.c |  30 ++
> > >  gcc/testsuite/c-c++-common/pr103798-3.c |  28 +
> > >  gcc/testsuite/c-c++-common/pr103798-4.c |  28 +
> > >  gcc/testsuite/c-c++-common/pr103798-5.c |  26 +
> > >  gcc/testsuite/c-c++-common/pr103798-6.c |  27 +
> > >  gcc/testsuite/c-c++-common/pr103798-7.c |  27 +
> > >  gcc/testsuite/c-c++-common/pr103798-8.c |  27 +
> > >  9 files changed, 357 insertions(+)
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-1.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-2.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-3.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-4.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-5.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-6.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-7.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/pr103798-8.c
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index a63b649841b..aa4766749af 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -7976,3 +7976,139 @@ and,
> > >  (match (bitwise_induction_p @0 @2 @3)
> > >   (bit_not
> > >(nop_convert1? (bit_xor@0 (convert2? (lshift integer_onep@1 @2)) @3
> > > +
> > > +#if GIMPLE
> > > +/* __builtin_memchr (const_str, a, N) != 0 ->
> > > +   a == const_str[0] .. || a == const_str[N-1]
> > > +   __builtin_memchr (const_str, a, N) == 0 ->
> > > +   a != const_str[0] .. && a != const_str[N-1]
> > > +   where N is less than the string size.  */
> > > +(for cmp (eq ne)
> > > + icmp (ne eq)
> > > + bit_op (bit_and bit_ior)
> > > + (simplify (cmp:c @0 (BUILT_IN_MEMCHR ADDR_EXPR@1 @2 INTEGER_CST@3))
> > > +  (if (UNITS_PER_WORD <= 8
> > > +   && CHAR_TYPE_SIZE == 8
> > > +   && BITS_PER_UNIT == 8
> > > +   && CHAR_BIT == 8
> > > +   && integer_zerop (@0)
> > > +   && !integer_zerop (@3)
> > > +   && TREE_CODE (TREE_OPERAND (@1, 0)) == STRING_CST
> > > +   && TREE_STRING_LENGTH (TREE_OPERAND (@1, 0)) >= 2
> > > +   && wi::leu_p (wi::to_wide (@3), UNITS_PER_WORD)
> > > +   && wi::ltu_p (wi::to_wide (@3),
> > > +TREE_STRING_LENGTH (TREE_OPERAND (@1, 0
> > > +   (with
> > > +{
> > > +  const char *p = TREE_STRING_POINTER (TREE_OPERAND (@1, 0));
> > > +  unsigned HOST_WIDE_INT size = TREE_INT_CST_LOW (@3);
> > > +}
> > > +(switch
> > > + (if (size == 1)
> > > +  (icmp (convert:char_type_node @2)
> > > + 

Re: [PATCH] libstdc++: testsuite: fs rename to self may fail

2022-06-22 Thread Alexandre Oliva via Gcc-patches
On Jun 22, 2022, Jonathan Wakely  wrote:

> "If the old argument and the new argument resolve to either the same
> existing directory entry or different directory entries for the same
> existing file, rename() shall return successfully and perform no other
> action." and "If the link named by the new argument exists, it shall
> be removed and old renamed to new."

> Instead, the implementation of std::filesystem::rename should have a
> special-case for rtems (and maybe other targets) that implements the
> POSIX rename semantics if calling ::rename isn't good enough.

Other POSIX requirements that make implementing them "interesting" when
::rename() doesn't are:

- leaving "new" alone when the operation fails: implies permissions,
  same filesystem, etc must all be checked first, atomically, before
  removing the file, and if renaming somehow fails after that, "new"
  must be somehow brought back to existence

- if "new" exists, it must keep on existing throughout the renaming
  process: implies temporary removal is not allowed, even if needed to
  to allow ::rename to succeed; they'd have to be part of the same
  filesystem transaction

- even if both of the above could somehow be satisfied, there are all
  sorts of race conditions that could enable pre-check to succeed, but
  that, even the checks were implemented fully and perfectly, would
  still enable the rename to fail after removal of a preexisting new.


Clearly something's gotta give.  Since ensuring the existence of "new"
throughout renaming is impossible if you can't rename to existing
filenames, I propose giving that up, though it's a very useful property.

I'd start by trying to rename old to new.  If that fails with EEXIST,
I'd rename new to a temporary name, and then try to rename old to new
and then remove the temporary name.

If renaming new to a temporary name fails, that means we can't remove it
either.

If renaming old to the now-vacated new still fails, that means we can't
rename old, so rename the temporary back to new.

If removing the temporary name for new fails, that means we couldn't
remove it to begin with, say because it's not empty, or because we lack
permission to remove it, so rename both back.


Knowing rename won't overwrite an existing pathname makes it easier to
pick a temporary name to use for the renaming: we know there's no risk
of overwriting files created concurrently, though we may have to try
other names if the rename-to-temp fails with EEXIST.

Creating a temporary directory sibling to new and picking a temporary
name in that directory enables us to detect early some cases of lack of
permission to remove new, that renaming it in the same directory
wouldn't hit.

Creating such a temporary directory also increases new/..'s link count
before moving it.  In a way, it checks for room for old/.., and it's no
worse than keeping new under the same parent, but picking a temporary
dir elsewhere would enable the renaming even if new/.. is already at the
limit.  Concurrent attempts to create directories in the same parent dir
could enable the operation to fail while also preventing the reversal of
the preparatory renames.  That would be a rarer occurrence if we rename
new to a temporary name under the same parent.


And all this doesn't even cover the case of moving old into itself,
which we'd also have to detect and prevent.


This feels more and more like a case for xfail until it gets fixed in
the kernel, where atomic filesystem operations belong :-(

Would a patch to add:

// { dg-xfail-if "::rename is not POSIX-compliant" { target *-*-rtems* } }

to rename.cc tests be acceptable?  I'm afraid I can't go further down
this rabbit hole, and my choices ATM seem to be limited to XFAIL
patches, whether accepted by the GCC community or carried internally.

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


Re: [PATCH] libstdc++: 60241.cc: tolerate slightly shorter aggregate sleep

2022-06-22 Thread Sebastian Huber

On 23/06/2022 02:19, Alexandre Oliva wrote:

On Jun 22, 2022, Sebastian Huber  wrote:


The clock_nanosleep() uses the coarse resolution which may give a time
before now().

Uhh, sorry, hit send too early.

I also meant to ask whether you'd like me to file an RTEMS ticket about
this issue.


I already created a ticket for this and work on it:

http://devel.rtems.org/ticket/4669

--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/