Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Jason Merrill via Gcc-patches]

On 3/11/20 6:28 PM, Jakub Jelinek wrote:

On Wed, Mar 11, 2020 at 04:02:51PM -0400, Jason Merrill via Gcc-patches wrote:

We should certainly avoid copying if they're the same.  The code above for
only copying the bits that aren't going to be thrown away seems pretty
straightforward, might as well use it even if the savings aren't likely to
be large.


So like this if it passes bootstrap/regtest?
Calling vec_safe_truncate with the same number of elts the vector already
has is a nop, so IMHO we just should make sure we only unshare if it
changed.

2020-03-11  Jakub Jelinek  

PR c++/94124
* decl.c (reshape_init_array_1): Don't unshare constructor if there
aren't any trailing zero elts, otherwise only unshare the first
nelts.

--- gcc/cp/decl.c.jj2020-03-11 09:28:53.966213943 +0100
+++ gcc/cp/decl.c   2020-03-11 23:19:08.832780798 +0100
@@ -6066,10 +6066,22 @@ reshape_init_array_1 (tree elt_type, tre
 overload resolution.  E.g., initializing a class from
 {{0}} might be invalid while initializing the same class
 from {{}} might be valid.  */
-  if (reuse)
-   new_init = unshare_constructor (new_init);
-
-  vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
+  if (reuse && nelts < CONSTRUCTOR_NELTS (new_init))
+   {
+ vec *v = NULL;
+ if (nelts)


vec_alloc does nothing if nelts is 0, so this test seems unnecessary. 
OK either way.



+   vec_alloc (v, nelts);
+ for (unsigned int i = 0; i < nelts; i++)
+   {
+ constructor_elt elt = *CONSTRUCTOR_ELT (new_init, i);
+ if (TREE_CODE (elt.value) == CONSTRUCTOR)
+   elt.value = unshare_constructor (elt.value);
+ v->quick_push (elt);
+   }
+ new_init = build_constructor (TREE_TYPE (new_init), v);
+   }
+  else
+   vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
  }
  
return new_init;



Jakub

[PATCH] libstdc++: Fix the return type of __cxa_finalize according to the Itanium C++ ABI

[Fangrui Song via Gcc-patches]

Alternatively, we can delete it, because no user code should call it.
It may be weird that libc is expected to define this function.
This function is a language runtime interface that has nothing to do
with a libc.

---
 libstdc++-v3/libsupc++/cxxabi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h
index 50298205daa..000713ecdf8 100644
--- a/libstdc++-v3/libsupc++/cxxabi.h
+++ b/libstdc++-v3/libsupc++/cxxabi.h
@@ -127,7 +127,7 @@ namespace __cxxabiv1
   int
   __cxa_atexit(void (*)(void*), void*, void*) _GLIBCXX_NOTHROW;
 
-  int
+  void
   __cxa_finalize(void*);
 
   // TLS destruction.
-- 
2.25.1

Re: [testsuite] Fix PR93935 to guard case under vect_hw_misalign

[Jeff Law via Gcc-patches]

On Wed, 2020-03-11 at 14:22 +0800, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> Gentle ping this patch, also request to backport to gcc9 after some burn-in
> time.
> 
> BR,
> Kewen
> 
> on 2020/2/26 下午2:17, Kewen.Lin wrote:
> > Hi,
> > 
> > This patch is to apply the same fix as r267528 to another similar case
> > bb-slp-over-widen-2.c which requires misaligned vector access.
> > 
> > Verified it on ppc64-redhat-linux (Power7 BE).
> > 
> > Is it ok for trunk?
> > 
> > BR,
> > Kewen
> > ---
> > 
> > gcc/testsuite/ChangeLog
> > 
> > 2020-02-26  Kewen Lin  
> > 
> > PR testsuite/93935
> > * gcc.dg/vect/bb-slp-over-widen-2.c: Expect basic block vectorized
> > messages only on vect_hw_misalign targets.
OK for the trunk and gcc-9.

jeff
> > 

Re: [PATCH 6/6] i386: Use ix86_output_ssemov for MMX TYPE_SSEMOV

[Jeff Law via Gcc-patches]

On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> There is no need to set mode attribute to XImode since ix86_output_ssemov
> can properly encode xmm16-xmm31 registers with and without AVX512VL.
> 
> Remove ext_sse_reg_operand since it is no longer needed.
> 
>   PR target/89229
>   * config/i386/i386.c (ix86_output_ssemov): Handle MODE_V1DF and
>   MODE_V2SF.
>   * config/i386/mmx.md (MMXMODE:*mov_internal): Call
>   ix86_output_ssemov for TYPE_SSEMOV.  Remove ext_sse_reg_operand
>   check.
>   * config/i386/predicates.md (ext_sse_reg_operand): Removed.
This is OK.   I think once this is in, patch #2 becomes OK because this patch
adds V2SF handling in ix86_output_ssemov.

Similarly I think patch #4 is OK once this one goes in since it adds V1DF as
well.

So perhaps an integration plan would be to immediately install #6, followed 
24hrs
later by patch #4, then 24hrs after patch #2.

Then we can work on patch #5 and patch #3 where I think we go with patch #5 plus
the MODE_SI hunk from patch #3.  THen 24hrs after that the remaining bits of
patch #3.

I think that covers the whole series.

jeff

Re: [PATCH 5/6] i386: Use ix86_output_ssemov for SFmode TYPE_SSEMOV

[Jeff Law via Gcc-patches]

On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> There is no need to set mode attribute to V16SFmode since ix86_output_ssemov
> can properly encode xmm16-xmm31 registers with and without AVX512VL.
> 
> gcc/
> 
>   PR target/89229
>   * config/i386/i386.c (ix86_output_ssemov): Handle MODE_SF.
>   * config/i386/i386.md (*movdf_internal): Call ix86_output_ssemov
>   for TYPE_SSEMOV.  Remove TARGET_PREFER_AVX256, TARGET_AVX512VL
>   and ext_sse_reg_operand check.
> 
> gcc/testsuite/
> 
>   PR target/89229
>   * gcc.target/i386/pr89229-7a.c: New test.
>   * gcc.target/i386/pr89229-7b.c: Likewise.
>   * gcc.target/i386/pr89229-7c.c: Likewise.
I believe this as a dependency on patch #3.  It's OK once patch #3 is approved. 
Alternately, you could break out the MODE_SI hunk in ix86_output_ssemov from
patch #3, add it to this patch and that would be approved for immediate
integration.

Jeff

Re: [PATCH 4/6] i386: Use ix86_output_ssemov for DFmode TYPE_SSEMOV

[Jeff Law via Gcc-patches]

On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> There is no need to set mode attribute to XImode nor V8DFmode since
> ix86_output_ssemov can properly encode xmm16-xmm31 registers with and
> without AVX512VL.
> 
> gcc/
> 
>   PR target/89229
>   * config/i386/i386.c (ix86_output_ssemov): Handle MODE_DF.
>   * config/i386/i386.md (*movdf_internal): Call ix86_output_ssemov
>   for TYPE_SSEMOV.  Remove TARGET_AVX512F, TARGET_PREFER_AVX256,
>   TARGET_AVX512VL and ext_sse_reg_operand check.
And I worry about V1DF for alternatives 14,18 and SSE_SPLIT_REGS as well as
alternative 15,19 and !TARGET_SSE2 which has insn_mode of V2SF.

jeff
> 

Re: [PATCH 3/6] i386: Use ix86_output_ssemov for SImode TYPE_SSEMOV

[Jeff Law via Gcc-patches]

On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> There is no need to set mode attribute to XImode since ix86_output_ssemov
> can properly encode xmm16-xmm31 registers with and without AVX512VL.
> 
> gcc/
> 
>   PR target/89229
>   * config/i386/i386.c (ix86_output_ssemov): Handle MODE_SI.
>   * config/i386/i386.md (*movsi_internal): Call ix86_output_ssemov
>   for TYPE_SSEMOV.  Remove ext_sse_reg_operand and TARGET_AVX512VL
>   check.
> 
> gcc/testsuite/
> 
>   PR target/89229
>   * gcc.target/i386/pr89229-5a.c: New test.
>   * gcc.target/i386/pr89229-5b.c: Likewise.
>   * gcc.target/i386/pr89229-5c.c: Likewise.
Similar to #2, can't we get insn_mode to be SFmode for alternatives 10,11 and
!TARGET_SSE2?  Won't that cause us to hit the gcc_unreachable in
ix86_output_ssemov?

jeff
> 

Re: [PATCH 2/6] i386: Use ix86_output_ssemov for DImode TYPE_SSEMOV

[Jeff Law via Gcc-patches]

On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> There is no need to set mode attribute to XImode since ix86_output_ssemov
> can properly encode xmm16-xmm31 registers with and without AVX512VL.
> 
> gcc/
> 
>   PR target/89229
>   * config/i386/i386.c (ix86_output_ssemov): Handle MODE_DI.
>   * config/i386/i386.md (*movdi_internal): Call ix86_output_ssemov
>   for TYPE_SSEMOV.  Remove ext_sse_reg_operand and TARGET_AVX512VL
>   check.
> 
> gcc/testsuite/
> 
>   PR target/89229
>   * gcc.target/i386/pr89229-4a.c: New test.
>   * gcc.target/i386/pr89229-4b.c: Likewise.
>   * gcc.target/i386/pr89229-4c.c: Likewise.
So for alternatives 14, 15, 16 and !TARGET_SSE2 can't the insn_mode be V2SF? 
Isn't that going to trigger the gcc_unreachable in ix86_output_ssemov?



Jeff

Re: [PATCH] Add myself to MAINTAINERS

[binbin via Gcc-patches]

Hi,

On 2020/3/12 上午10:19, Bin Bin Lv wrote:

Hi,

This adds myself to MAINTAINERS in the Write After Approval section.

gcc/ChangeLog

2020-03-11  Bin Bin Lv  

* MAINTAINERS (Write After Approval): Add myself.
---
  MAINTAINERS | 1 +
  1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index be80166..56574fb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -497,6 +497,7 @@ Martin v. Löwis 

  H.J. Lu   
  Xiong Hu Luo  
  Christophe Lyon   
+Bin Bin Lv 
  Luis Machado  
  Ziga Mahkovec 
  Matthew Malcomson 



Oops, updated to the right place.  Thanks.

Re: Fix modulo-scheduler -fcompare-debug issues

[Jeff Law via Gcc-patches]

On Tue, 2020-03-10 at 19:39 +0300, Roman Zhuykov wrote:
> Hi!
> 
> Current modulo-sched implementation is a bit faulty from -fcompile-debug
> perspective.
> 
> I found that few years ago, the most simple example is pr42631.c which fails
> (with just -fmodulo-sched or together with -fmodulo-sched-allow-regmoves) on
> powerpc64le with at least any gcc-4.9 or newer compiler.
> I've investigated that difference about 3 years ago, it is mostly technical,
> dumps shows there are some "flying" NOTE_INSN_DELETED items.
> I understood that it is minor, and I planned to commit the fix only when my
> other modulo-sched stuff will be ready.
> 
> But right now I see that when I enable -fmodulo-sched by default, powerpc64le
> bootstrap give comparison failure as of r10-7056.
> 
> Comparing stages 2 and 3
> Bootstrap comparison failure!
> gcc/ggc-page.o differs
> 
> That doesn't happen on released branches, so it is a kind of "regression"
> (certainly, nobody runs bootstrap with -fmodulo-sched).
> 
> Is that a good reason to commit the patch right now in stage4?
> 
> Patch was successfully regstrapped (based on r10-7056) using x86_64 and
> powerpc64le, both with default options and with -fmodulo-sched enabled.
> 
> Roman
> 
> --
> modulo-sched: fix compare-debug issues
> 
> This patch fixes bootstrap comparison failure on powerpc64le when running it
> with -fmodulo-sched enabled.
> 
> When applying the schedule we have to move debug insns in the same
> way as we move note insns.  Also we have to discard adding debug insns
> to SCCs in DDG graph.
> 
> 20YY-MM-DD  Roman Zhuykov  
>
>   * ddg.c (create_ddg_dep_from_intra_loop_link): Adjust assertions.
>   (create_ddg_dep_no_link): Likewise.
>   (add_inter_loop_mem_dep): Do not create "debug --> non-debug" anti-deps.
>   (create_ddg): Adjust first_note field filling.
>   (check_sccs): Assert if any debug instruction is in SCC.
>   * modulo-sched.c (ps_first_note): Add an assertion if first_note
>   is empty.
> 
> testsuite:
> 
> 20YY-MM-DD  Roman Zhuykov  
> 
>   * gcc.dg/pr42631-sms1.c: New test.
>   * gcc.dg/pr42631-sms2.c: New test.
Even though we don't have a BZ, I think a bootstrap failure, even one with
modulo-scheduling enabled is severe enough that we should try to fix it.

OK for the trunk.

jeff
> 

Re: [testsuite] Add @ lines to check-function-bodies fluff

[Jeff Law via Gcc-patches]

On Tue, 2020-03-10 at 17:22 +, Matthew Malcomson wrote:
> When using `check-function-bodies`, the subroutine `parse_function_bodies` 
> uses
> the `fluff` regexp to remove uninteresting assembly lines.
> 
> Arm targets generate assembly with some lines prefixed by `@`, these lines are
> left by this process.
> 
> As an example of some lines prefixed by `@': the assembly output from the
> `stacktest1` function in "bfloat16_simd_3_1.c" is:
> 
> .align  2
> .global stacktest1
> .arch armv8.2-a
> .syntax unified
> .arm
> .fpu neon-fp-armv8
> .type   stacktest1, %function
> stacktest1:
> @ args = 0, pretend = 0, frame = 8
> @ frame_needed = 0, uses_anonymous_args = 0
> @ link register save eliminated.
> sub sp, sp, #8
> add r3, sp, #6
> vst1.16 {d0[0]}, [r3]
> vld1.16 {d0[0]}, [r3]
> add sp, sp, #8
> @ sp needed
> bx  lr
> .size   stacktest1, .-stacktest1
> 
> 
> 
> It seems that previous uses of `check-function-bodies` in the arm backend have
> avoided problems with such lines since they use the `...` regexp in each place
> such fluff occurs.
> 
> I'm currently writing a patch that I'd like to match the entire function body,
> so I'd like to remove such `@` lines automatically.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-03-10  Matthew Malcomson  
> 
>   * lib/scanasm.exp (parse_function_bodies): Lines starting with '@' also
>   counted as fluff.
I could see wanting to look at those lines in an ARM specific test.  For example
if there was a test that required a frame, but it was eliminated for some reason
or vice-versa.  Looking at the @ lines might make writing that kind of test
easier.

But I don't mind ACKing this now.  We could come back to the issue in the 
future.

WRT matching a whole function, I'd be leery of such tests in general, but there
may be times when they're appropriate.  I'll defer to the ARM maintainers on
whether or not they want such tests.

Jeff
> 

Re: Remove redundant zero extend

[Jeff Law via Gcc-patches]

On Wed, 2020-03-11 at 13:04 +, Nidal Faour via Gcc-patches wrote:
> This patch is a code density oriented and attempt to remove redundant 
> sign/zero
> extension from assignment statement.
> The approach taken is to use VRP data while expanding the assignment to RTL to
> determine whether a sign/zero extension is necessary.
> Thought the motivation of the patch is code density but it also good for 
> speed.
> 
> for example:
> extern unsigned int func ();
> 
>   unsigned char
>   foo (unsigned int arg)
>   {
> if (arg == 2)
>   return 0;
> 
> return (func() == arg || arg == 7);
>   }
> 
> the result of the comparison in the return will yield a False or True, which
> will be converted to integer and then casting to unsigned char.
> this casting from integer to unsigned char is redundant because the value is
> either 0 or 1.
> 
> this patch is targeting the RISCV-32bit only.
> This patch has been tested on a real embedded project and saved about 0.2% of
> code size.
> 
> P.S. I have an FSF license under Western Digital incorporation.
> P.S. I've attached the patch as a file in case my email client corrupted the
> patch itself
Just an FYI.  We're at stage4 in our development cycle, as a result most
developers are focused on regression bugfixing until the gcc-10 release is 
made. 
I've put your patch into the queue of things to evaluate for gcc-11.

Thanks,
jeff
> 

Re: [PATCH] [rs6000] Fix a wrong GC issue

[binbin via Gcc-patches]

Hi,

On 2020/3/10 上午10:30, binbin wrote:

Hi,

On 2020/3/9 下午10:32, Segher Boessenkool wrote:

Hi!

On Mon, Mar 09, 2020 at 01:58:01PM +0800, binbin wrote:

2020-03-09  Bin Bin Lv  

* config/rs6000/rs6000-internal.h (altivec_builtin_mask_for_load,
builtin_mode_to_type[MAX_MACHINE_MODE][2]): Remove the declaration.


Just write "builtin_mode_to_type", nothing []?  Writing down the
dimensions here doesn't add anything, just is a bit noisy: you normally
put just the name here.


OK, modified.  Thanks.




* config/rs6000/rs6000.c (altivec_builtin_mask_for_load,
builtin_mode_to_type[MAX_MACHINE_MODE][2]): Remove the GTY(())
declaration and add the definition.


The definitions were already there, so lose the second part of this?


OK, removed.  Thanks.




diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 1697186..724085b 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -2490,6 +2490,8 @@ extern GTY(()) tree 
rs6000_builtin_types[RS6000_BTI_MAX];

  extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT];
  #ifndef USED_FOR_TARGET
+extern GTY(()) tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
+extern GTY(()) tree altivec_builtin_mask_for_load;


Add a newline here please?


OK, added a newline.  Thanks.




  /* A C structure for machine-specific, per-function data.
 This is added to the cfun structure.  */
  typedef struct GTY(()) machine_function


Okay for trunk with those tweaks.  Thanks!


Segher



Submitted the patch.

[RS6000] make PLT loads volatile

[Alan Modra via Gcc-patches]

With lazy PLT resolution the first load of a PLT entry may be a value
pointing at a resolver stub.  gcc's loop processing can result in the
PLT load in inline PLT calls being hoisted out of a loop in the
mistaken idea that this is an optimisation.  It isn't.  If the value
hoisted was that for a resolver stub then every call to that function
in the loop will go via the resolver, slowing things down quite
dramatically.

The PLT really is volatile, so teach gcc about that.

Bootstrapped and regression tested on powerpc64le-linux and tested
with a spec build using -mlongcalls.  OK for mainline?

* config/rs6000/rs6000.c (rs6000_longcall_ref): Use unspec_volatile
for PLT16_LO and PLT_PCREL.
* config/rs6000/rs6000.md (UNSPEC_PLT16_LO, UNSPEC_PLT_PCREL): Remove.
(UNSPECV_PLT16_LO, UNSPECV_PLT_PCREL): Define.
(pltseq_plt16_lo_, pltseq_plt_pcrel): Use unspec_volatile.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 46b7dec2abd..2d6790877fa 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -19264,8 +19264,9 @@ rs6000_longcall_ref (rtx call_ref, rtx arg)
   if (rs6000_pcrel_p (cfun))
{
  rtx reg = gen_rtx_REG (Pmode, regno);
- rtx u = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, base, call_ref, arg),
- UNSPEC_PLT_PCREL);
+ rtx u = gen_rtx_UNSPEC_VOLATILE (Pmode,
+  gen_rtvec (3, base, call_ref, arg),
+  UNSPECV_PLT_PCREL);
  emit_insn (gen_rtx_SET (reg, u));
  return reg;
}
@@ -19284,8 +19285,9 @@ rs6000_longcall_ref (rtx call_ref, rtx arg)
   rtx reg = gen_rtx_REG (Pmode, regno);
   rtx hi = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, base, call_ref, arg),
   UNSPEC_PLT16_HA);
-  rtx lo = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, reg, call_ref, arg),
-  UNSPEC_PLT16_LO);
+  rtx lo = gen_rtx_UNSPEC_VOLATILE (Pmode,
+   gen_rtvec (3, reg, call_ref, arg),
+   UNSPECV_PLT16_LO);
   emit_insn (gen_rtx_SET (reg, hi));
   emit_insn (gen_rtx_SET (reg, lo));
   return reg;
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index ad88b6783af..5a8e9de670b 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -148,8 +148,6 @@
UNSPEC_SI_FROM_SF
UNSPEC_PLTSEQ
UNSPEC_PLT16_HA
-   UNSPEC_PLT16_LO
-   UNSPEC_PLT_PCREL
   ])
 
 ;;
@@ -178,6 +176,8 @@
UNSPECV_MTFSB1  ; Set FPSCR Field bit to 1
UNSPECV_SPLIT_STACK_RETURN   ; A camouflaged return
UNSPECV_SPEC_BARRIER ; Speculation barrier
+   UNSPECV_PLT16_LO
+   UNSPECV_PLT_PCREL
   ])
 
 ; The three different kinds of epilogue.
@@ -10359,10 +10359,10 @@
 
 (define_insn "*pltseq_plt16_lo_"
   [(set (match_operand:P 0 "gpc_reg_operand" "=r")
-   (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b")
-  (match_operand:P 2 "symbol_ref_operand" "s")
-  (match_operand:P 3 "" "")]
- UNSPEC_PLT16_LO))]
+   (unspec_volatile:P [(match_operand:P 1 "gpc_reg_operand" "b")
+   (match_operand:P 2 "symbol_ref_operand" "s")
+   (match_operand:P 3 "" "")]
+  UNSPECV_PLT16_LO))]
   "TARGET_PLTSEQ"
 {
   return rs6000_pltseq_template (operands, RS6000_PLTSEQ_PLT16_LO);
@@ -10382,10 +10382,10 @@
 
 (define_insn "*pltseq_plt_pcrel"
   [(set (match_operand:P 0 "gpc_reg_operand" "=r")
-   (unspec:P [(match_operand:P 1 "" "")
-  (match_operand:P 2 "symbol_ref_operand" "s")
-  (match_operand:P 3 "" "")]
- UNSPEC_PLT_PCREL))]
+   (unspec_volatile:P [(match_operand:P 1 "" "")
+   (match_operand:P 2 "symbol_ref_operand" "s")
+   (match_operand:P 3 "" "")]
+  UNSPECV_PLT_PCREL))]
   "HAVE_AS_PLTSEQ && TARGET_ELF
&& rs6000_pcrel_p (cfun)"
 {

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH] Add myself to MAINTAINERS

[Bin Bin Lv via Gcc-patches]

Hi,

This adds myself to MAINTAINERS in the Write After Approval section.

gcc/ChangeLog

2020-03-11  Bin Bin Lv  

* MAINTAINERS (Write After Approval): Add myself.
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index be80166..56574fb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -497,6 +497,7 @@ Martin v. Löwis 

 H.J. Lu
 Xiong Hu Luo   
 Christophe Lyon
+Bin Bin Lv 
 Luis Machado   
 Ziga Mahkovec  
 Matthew Malcomson  
-- 
1.8.3.1

[PATCH v3 2/2] generate EH info for all asm statements (PR93981)

[J.W. Jagersma via Gcc-patches]

This patch extends the generation of exception handling information to
cover all asm statements, when -fnon-call-exceptions is given.  The
previous patch only enabled this for volatile asms.

The previously added test cases are adjusted to cover this change.

gcc/
2020-03-11  Jan W. Jagersma  

PR inline-asm/93981
* tree-eh.c (tree_could_trap_p): Return true for case ASM_EXPR.
(stmt_could_throw_p): Likewise, for GIMPLE_ASM.
* rtlanal.c (may_trap_p_1): Likewise, for ASM_OPERANDS.
* doc/extend.texi: Document this change in the appropriate
subsection.

gcc/testsuite/
2020-03-11  Jan W. Jagersma  

PR inline-asm/93981
* g++.dg/eh/pr93981.C: Cover non-volatile throwing asms.
* g++.target/i386/pr93981.C: Likewise.
---
 gcc/doc/extend.texi | 10 +++
 gcc/rtlanal.c   |  4 +--
 gcc/testsuite/g++.dg/eh/pr93981.C   | 22 ---
 gcc/testsuite/g++.target/i386/pr93981.C | 36 ++---
 gcc/tree-eh.c   |  4 +--
 5 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index b51e34c617a..43929d9b129 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9454,6 +9454,11 @@ printf("%d\n", dst);
 
 This code copies @code{src} to @code{dst} and add 1 to @code{dst}.
 
+When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, an
+@code{asm} statement is also allowed to throw exceptions.  If it does, then its
+register output operands are assumed to be clobbered and will not be used.
+Memory operands, however, are always considered valid.
+
 @anchor{Volatile}
 @subsubsection Volatile
 @cindex volatile @code{asm}
@@ -9577,11 +9582,6 @@ errors during compilation if your @code{asm} code 
defines symbols or labels.
 Using @samp{%=} 
 (@pxref{AssemblerTemplate}) may help resolve this problem.
 
-When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, a
-@code{volatile asm} statement is also allowed to throw exceptions.  If it does,
-then its register output operands are assumed to be clobbered and will not be
-used.  Memory operands, however, are always considered valid.
-
 @anchor{AssemblerTemplate}
 @subsubsection Assembler Template
 @cindex @code{asm} assembler template
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index c7ab86e228b..e7eb2dad11b 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -2831,13 +2831,11 @@ may_trap_p_1 (const_rtx x, unsigned flags)
   return targetm.unspec_may_trap_p (x, flags);
 
 case UNSPEC_VOLATILE:
+case ASM_OPERANDS:
 case ASM_INPUT:
 case TRAP_IF:
   return 1;
 
-case ASM_OPERANDS:
-  return MEM_VOLATILE_P (x);
-
   /* Memory ref can trap unless it's a static var or a stack slot.  */
 case MEM:
   /* Recognize specific pattern of stack checking probes.  */
diff --git a/gcc/testsuite/g++.dg/eh/pr93981.C 
b/gcc/testsuite/g++.dg/eh/pr93981.C
index a9adb5c069e..2e0d1cb881c 100644
--- a/gcc/testsuite/g++.dg/eh/pr93981.C
+++ b/gcc/testsuite/g++.dg/eh/pr93981.C
@@ -3,7 +3,7 @@
 // { dg-options "-fnon-call-exceptions" }
 
 void
-f ()
+foo ()
 {
   try
 {
@@ -11,8 +11,24 @@ f ()
 }
   catch (...)
 {
-  asm ("#catch");
+  asm ("#catch-v");
 }
 }
 
-// { dg-final { scan-assembler "#catch" } }
+int
+bar ()
+{
+  int i;
+  try
+{
+  asm ("#try" : "=rm" (i));
+}
+  catch (...)
+{
+  asm ("#catch-nv");
+}
+  return i;
+}
+
+// { dg-final { scan-assembler "#catch-v" } }
+// { dg-final { scan-assembler "#catch-nv" } }
diff --git a/gcc/testsuite/g++.target/i386/pr93981.C 
b/gcc/testsuite/g++.target/i386/pr93981.C
index 7a3117901f9..e4b5d7cd90f 100644
--- a/gcc/testsuite/g++.target/i386/pr93981.C
+++ b/gcc/testsuite/g++.target/i386/pr93981.C
@@ -15,7 +15,7 @@ sigill (int)
 }
 
 int
-test_mem ()
+test_mem_v ()
 {
   int i = 2;
   try
@@ -30,7 +30,7 @@ test_mem ()
 }
 
 int
-test_reg ()
+test_reg_v ()
 {
   int i = 8;
   try
@@ -44,6 +44,36 @@ test_reg ()
   return i;
 }
 
+int
+test_mem_nv ()
+{
+  int i = 32;
+  try
+{
+  asm ("mov%z0 $16, %0; ud2" : "=m" (i));
+}
+  catch (const illegal_opcode&)
+{
+  if (i == 16) return 0;
+}
+  return i;
+}
+
+int
+test_reg_nv ()
+{
+  int i = 128;
+  try
+{
+  asm ("mov%z0 $64, %0; ud2" : "=r" (i));
+}
+  catch (const illegal_opcode&)
+{
+  if (i == 128) return 0;
+}
+  return i;
+}
+
 int
 main ()
 {
@@ -51,5 +81,5 @@ main ()
   sa.sa_handler = sigill;
   sa.sa_flags = SA_NODEFER;
   sigaction (SIGILL, , 0);
-  return test_mem () | test_reg ();
+  return test_mem_v () | test_reg_v () | test_mem_nv () | test_reg_nv ();
 }
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 58b16aa763a..b2e17099320 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2760,7 +2760,7 @@ tree_could_trap_p (tree expr)
   return !TREE_THIS_NOTRAP (expr);
 
 case ASM_EXPR:
-  return TREE_THIS_VOLATILE 

[PATCH v3 1/2] generate EH info for volatile asm statements (PR93981)

[J.W. Jagersma via Gcc-patches]

The following patch extends the generation of exception handling
information, so that it is possible to catch exceptions thrown from
volatile asm statements, when -fnon-call-exceptions is enabled.  Parts
of the gcc code already suggested this should be possible, but it was
never fully implemented.

Two new test cases are added.  The target-dependent test should pass on
platforms where throwing from a signal handler is allowed.  The only
platform I am aware of where that is the case is *-linux-gnu, so it is
set to XFAIL on all others.

gcc/
2020-03-11  Jan W. Jagersma  

PR inline-asm/93981
* tree-cfg.c (make_edges_bb): Make EH edges for GIMPLE_ASM.
* tree-eh.c (lower_eh_constructs_2): Add case for GIMPLE_ASM.
Assign register output operands to temporaries.
* doc/extend.texi: Document that volatile asms can now throw.

gcc/testsuite/
2020-03-11  Jan W. Jagersma  

PR inline-asm/93981
* g++.target/i386/pr93981.C: New test.
* g++.dg/eh/pr93981.C: New test.
---
 gcc/doc/extend.texi |  5 +++
 gcc/testsuite/g++.dg/eh/pr93981.C   | 18 
 gcc/testsuite/g++.target/i386/pr93981.C | 55 +
 gcc/tree-cfg.c  |  2 +
 gcc/tree-eh.c   | 32 ++
 5 files changed, 112 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/eh/pr93981.C
 create mode 100644 gcc/testsuite/g++.target/i386/pr93981.C

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e0e7f540c21..b51e34c617a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9577,6 +9577,11 @@ errors during compilation if your @code{asm} code 
defines symbols or labels.
 Using @samp{%=} 
 (@pxref{AssemblerTemplate}) may help resolve this problem.
 
+When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, a
+@code{volatile asm} statement is also allowed to throw exceptions.  If it does,
+then its register output operands are assumed to be clobbered and will not be
+used.  Memory operands, however, are always considered valid.
+
 @anchor{AssemblerTemplate}
 @subsubsection Assembler Template
 @cindex @code{asm} assembler template
diff --git a/gcc/testsuite/g++.dg/eh/pr93981.C 
b/gcc/testsuite/g++.dg/eh/pr93981.C
new file mode 100644
index 000..a9adb5c069e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/pr93981.C
@@ -0,0 +1,18 @@
+// PR inline-asm/93981
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions" }
+
+void
+f ()
+{
+  try
+{
+  asm ("#try");
+}
+  catch (...)
+{
+  asm ("#catch");
+}
+}
+
+// { dg-final { scan-assembler "#catch" } }
diff --git a/gcc/testsuite/g++.target/i386/pr93981.C 
b/gcc/testsuite/g++.target/i386/pr93981.C
new file mode 100644
index 000..7a3117901f9
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr93981.C
@@ -0,0 +1,55 @@
+// PR inline-asm/93981
+// { dg-do run }
+// { dg-options "-fnon-call-exceptions -O3" }
+// { dg-xfail-if "" { ! *-linux-gnu } }
+// { dg-xfail-run-if "" { ! *-linux-gnu } }
+
+#include 
+
+struct illegal_opcode { };
+
+extern "C" void
+sigill (int)
+{
+  throw illegal_opcode ( );
+}
+
+int
+test_mem ()
+{
+  int i = 2;
+  try
+{
+  asm volatile ("mov%z0 $1, %0; ud2" : "=m" (i));
+}
+  catch (const illegal_opcode&)
+{
+  if (i == 1) return 0;
+}
+  return i;
+}
+
+int
+test_reg ()
+{
+  int i = 8;
+  try
+{
+  asm volatile ("mov%z0 $4, %0; ud2" : "=r" (i));
+}
+  catch (const illegal_opcode&)
+{
+  if (i == 8) return 0;
+}
+  return i;
+}
+
+int
+main ()
+{
+  struct sigaction sa = { };
+  sa.sa_handler = sigill;
+  sa.sa_flags = SA_NODEFER;
+  sigaction (SIGILL, , 0);
+  return test_mem () | test_reg ();
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index f7b817d94e6..c21a7978493 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -913,6 +913,8 @@ make_edges_bb (basic_block bb, struct omp_region 
**pcur_region, int *pomp_index)
   break;
 
 case GIMPLE_ASM:
+  if (stmt_can_throw_internal (cfun, last))
+   make_eh_edges (last);
   make_gimple_asm_edges (bb);
   fallthru = true;
   break;
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 2a409dcaffe..58b16aa763a 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2077,6 +2077,8 @@ lower_eh_constructs_2 (struct leh_state *state, 
gimple_stmt_iterator *gsi)
DECL_GIMPLE_REG_P (tmp) = 1;
  gsi_insert_after (gsi, s, GSI_SAME_STMT);
}
+
+record_throwing_stmt:
   /* Look for things that can throw exceptions, and record them.  */
   if (state->cur_region && stmt_could_throw_p (cfun, stmt))
{
@@ -2085,6 +2087,36 @@ lower_eh_constructs_2 (struct leh_state *state, 
gimple_stmt_iterator *gsi)
}
   break;
 
+case GIMPLE_ASM:
+  {
+   /* As above with GIMPLE_ASSIGN.  Change each register output operand
+  to a temporary and insert a new stmt to assign this to the original
+  operand.  

[PATCH v3 0/2] generate EH info for asm statements (PR93981)

[J.W. Jagersma via Gcc-patches]

As there were no more comments in the previous thread, I am submitting
a new revision of this patch.

The patch is split up in two parts now.  The first covers only volatile
asms and is mostly identical to the last revision.  There are no code
changes, only the test cases and documentation are expanded.

The second part extends this to cover also non-volatile asms in the
generated EH tables.  I have separated this since I do not know if this
change might affect optimization.  I'm hoping someone here can clarify
this.  If it does not disable (m)any optimizations, then I expect there
will be no objections to also include this second part.

[committed] testsuite: Fix concepts-using2.C failure on 32-bit targets [PR93907]

[Jakub Jelinek via Gcc-patches]

Hi!

The test FAILs on 32-bit targets that don't have __int128 type.

Fixed thusly, regtested on x86_64-linux and i686-linux, committed to trunk
as obvious.

2020-03-12  Jakub Jelinek  

PR c++/93907
* g++.dg/cpp2a/concepts-using2.C (cc): Use long long instead of
__int128 if __SIZEOF_INT128__ isn't defined.

--- gcc/testsuite/g++.dg/cpp2a/concepts-using2.C.jj 2020-03-11 
23:31:45.818595378 +0100
+++ gcc/testsuite/g++.dg/cpp2a/concepts-using2.C2020-03-12 
01:25:44.011097711 +0100
@@ -15,7 +15,11 @@ template  using an = typenam
 template  constexpr bool ao = c::d;
 template  constexpr bool i = c<1>::d;
 template  concept bb = i;
+#ifdef __SIZEOF_INT128__
 using cc = __int128;
+#else
+using cc = long long;
+#endif
 template  concept cd = bb;
 template  concept ce = requires { requires cd; };
 template  concept h = ce;

Jakub

Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Marek Polacek via Gcc-patches]

On Wed, Mar 11, 2020 at 11:28:01PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Wed, Mar 11, 2020 at 04:02:51PM -0400, Jason Merrill via Gcc-patches wrote:
> > We should certainly avoid copying if they're the same.  The code above for
> > only copying the bits that aren't going to be thrown away seems pretty
> > straightforward, might as well use it even if the savings aren't likely to
> > be large.
> 
> So like this if it passes bootstrap/regtest?
> Calling vec_safe_truncate with the same number of elts the vector already
> has is a nop, so IMHO we just should make sure we only unshare if it
> changed.
> 
> 2020-03-11  Jakub Jelinek  
> 
>   PR c++/94124
>   * decl.c (reshape_init_array_1): Don't unshare constructor if there
>   aren't any trailing zero elts, otherwise only unshare the first
>   nelts.
> 
> --- gcc/cp/decl.c.jj  2020-03-11 09:28:53.966213943 +0100
> +++ gcc/cp/decl.c 2020-03-11 23:19:08.832780798 +0100
> @@ -6066,10 +6066,22 @@ reshape_init_array_1 (tree elt_type, tre
>overload resolution.  E.g., initializing a class from
>{{0}} might be invalid while initializing the same class
>from {{}} might be valid.  */
> -  if (reuse)
> - new_init = unshare_constructor (new_init);
> -
> -  vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
> +  if (reuse && nelts < CONSTRUCTOR_NELTS (new_init))
> + {
> +   vec *v = NULL;
> +   if (nelts)
> + vec_alloc (v, nelts);
> +   for (unsigned int i = 0; i < nelts; i++)
> + {
> +   constructor_elt elt = *CONSTRUCTOR_ELT (new_init, i);
> +   if (TREE_CODE (elt.value) == CONSTRUCTOR)
> + elt.value = unshare_constructor (elt.value);
> +   v->quick_push (elt);
> + }
> +   new_init = build_constructor (TREE_TYPE (new_init), v);
> + }
> +  else
> + vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
>  }
>  
>return new_init;

LGTM.

Marek

Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Jakub Jelinek via Gcc-patches]

On Wed, Mar 11, 2020 at 04:02:51PM -0400, Jason Merrill via Gcc-patches wrote:
> We should certainly avoid copying if they're the same.  The code above for
> only copying the bits that aren't going to be thrown away seems pretty
> straightforward, might as well use it even if the savings aren't likely to
> be large.

So like this if it passes bootstrap/regtest?
Calling vec_safe_truncate with the same number of elts the vector already
has is a nop, so IMHO we just should make sure we only unshare if it
changed.

2020-03-11  Jakub Jelinek  

PR c++/94124
* decl.c (reshape_init_array_1): Don't unshare constructor if there
aren't any trailing zero elts, otherwise only unshare the first
nelts.

--- gcc/cp/decl.c.jj2020-03-11 09:28:53.966213943 +0100
+++ gcc/cp/decl.c   2020-03-11 23:19:08.832780798 +0100
@@ -6066,10 +6066,22 @@ reshape_init_array_1 (tree elt_type, tre
 overload resolution.  E.g., initializing a class from
 {{0}} might be invalid while initializing the same class
 from {{}} might be valid.  */
-  if (reuse)
-   new_init = unshare_constructor (new_init);
-
-  vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
+  if (reuse && nelts < CONSTRUCTOR_NELTS (new_init))
+   {
+ vec *v = NULL;
+ if (nelts)
+   vec_alloc (v, nelts);
+ for (unsigned int i = 0; i < nelts; i++)
+   {
+ constructor_elt elt = *CONSTRUCTOR_ELT (new_init, i);
+ if (TREE_CODE (elt.value) == CONSTRUCTOR)
+   elt.value = unshare_constructor (elt.value);
+ v->quick_push (elt);
+   }
+ new_init = build_constructor (TREE_TYPE (new_init), v);
+   }
+  else
+   vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
 }
 
   return new_init;


Jakub

Re: [PATCH 1/2] c++: Replay errors during diagnosis of constraint satisfaction failures

[Patrick Palka via Gcc-patches]

On Wed, 11 Mar 2020, Jason Merrill wrote:

> On 3/9/20 6:23 PM, Patrick Palka wrote:
> > This patch adds a new flag -fconcepts-diagnostics-depth to the C++ frontend
> > which controls how deeply we replay errors when diagnosing a constraint
> > satisfaction failure.  The default is -fconcepts-diagnostics-depth=1 which
> > diagnoses only the topmost constraint satisfaction failure and is consistent
> > with our behavior before this patch.  By increasing this flag's value, the
> > user
> > can control how deeply they want the compiler to explain a constraint
> > satisfaction error.
> > 
> > For example, if the unsatisfied constraint is a disjunction, then the
> > default
> > behavior is to just say "no branch in the disjunction is satisfied", but
> > with
> > -fconcepts-diagnostics-depth=2 we will additionally replay and diagnose the
> > error in each branch of the disjunction.  And if the unsatisfied constraint
> > is a
> > requires expression, then we will replay the error in the requires
> > expression,
> > etc.  This proceeds recursively until there is nothing more to replay or we
> > reached the exceeded the maximum depth specified by the flag.
> > 
> > Implementation wise, this patch essentially just uncomments the existing
> > commented-out code that performs the error-replaying, adding logic to keep
> > track
> > of the current replay depth along the way.  Besides that, there is a new
> > routine
> > collect_operands_of_disjunction which flattens a disjunction and collects
> > all of
> > its operands into a vector.
> > 
> > Here are some examples of diagnostics with the two patches in this series.
> > 
> > For the simple test case in which we call ranges::begin() on something
> > that's
> > not a range:
> > 
> >  #include 
> > 
> >  struct S { } s;
> >  auto x = std::ranges::begin(s);
> > 
> > we get the following diagnostics with -fconcepts-diagnostics-depth={1,2,3}
> > respectively:
> > 
> >   https://pppalka.github.io/ranges-begin-depth-1.html
> >   https://pppalka.github.io/ranges-begin-depth-2.html
> >   https://pppalka.github.io/ranges-begin-depth-3.html
> > 
> > And for the new test g++.dg/concepts/diagnostic5.C, we get:
> > 
> >  https://pppalka.github.io/diagnostic5-depth-1.html
> >  https://pppalka.github.io/diagnostic5-depth-2.html
> >  https://pppalka.github.io/diagnostic5-depth-3.html
> >  https://pppalka.github.io/diagnostic5-depth-4.html
> > 
> > The extra diagnostics enabled by this flag are at times longer than they
> > need to
> > be (e.g.  "the operand is_array_v<...> is unsatisfied because \n the
> > expression
> > is_array_v<...> [with ...] evaluated to false") and not immediately easy to
> > follow (especially when there are nested disjunctions), but the transparency
> > provided by these optional diagnostics seems to be pretty helpful in
> > practice.
> > 
> > Does this seem like a sensible approach?  Thoughts and ideas for improvement
> > welcome.  Wording and naming suggestions would be much appreciated.
> 
> This does seem like a good approach, thanks.
> 
> > gcc/c-family/ChangeLog:
> > 
> > * c.opt: Add -fconcepts-diagnostics-depth.
> > 
> > gcc/cp/ChangeLog:
> > 
> > * constraint.cc (finish_constraint_binary_op): Set the location of
> > EXPR
> > as well as its range, because build_x_binary_op doesn't always do so.
> > (current_constraint_diagnosis_depth): New.
> > (concepts_diagnostics_max_depth_exceeded_p): New.
> > (collect_operands_of_disjunction): New.
> > (satisfy_disjunction): When diagnosing a satisfaction failure, maybe
> > replay each branch of the disjunction, subject to the current
> > diagnosis
> > depth.
> > (diagnose_valid_expression): When diagnosing a satisfaction failure,
> > maybe replay the substitution error, subject to the current diagnosis
> > recursion.
> > (diagnose_valid_type): Likewise.
> > (diagnose_nested_requiremnet): Likewise.
> > (diagnosing_failed_constraint::diagnosing_failed_constraint):
> > Increment
> > current_constraint_diagnosis_depth when diagnosing.
> > (diagnosing_failed_constraint::~diagnosing_failed_constraint):
> > Decrement
> > current_constraint_diagnosis_depth when diagnosing.
> > (diagnose_constraints): Don't diagnose if
> > concepts_diagnostics_max_depth
> > is 0.  Emit a one-off note to increase -fconcepts-diagnostics-depth if
> > the limit was exceeded.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of
> > "neither operand".
> > * g++.dg/concepts/diagnostic5.C: New test.
> > ---
> >   gcc/c-family/c.opt  |   4 +
> >   gcc/cp/constraint.cc| 146 +---
> >   gcc/testsuite/g++.dg/concepts/diagnostic2.C |   2 +-
> >   gcc/testsuite/g++.dg/concepts/diagnostic5.C |  46 ++
> >   4 files changed, 177 insertions(+), 21 deletions(-)
> >   create mode 100644 

Re: [PATCH,rs6000] Add command line and builtin compatibility

[Segher Boessenkool]

On Wed, Mar 11, 2020 at 02:12:59PM -0500, Bill Schmidt wrote:
> >+  error ("%s is incompatible with mno-fprnd option",
> 
> I believe you need %qs here.  Also replace mno-fprnd with %qs and put 
> "-mno-fprnd" as the associated parameter.
> 
> Example from nearby code:   error ("%qs requires %qs", "-mdirect-move", 
> "-mvsx");

Yes, that makes the translators' job easier (and forces more consistent
output quoting, etc.)


Segher

Re: [PATCH,rs6000] Add command line and builtin compatibility

[Segher Boessenkool]

Hi!

On Wed, Mar 11, 2020 at 12:00:12PM -0700, Carl Love wrote:
> The following patch add a check to make sure the user did not specify 
> -mno_fprnd with the builtins  __builtin_vsx_xsrdpim and
> __builtin_vsx_xsrdpip.  These builtins are incompatible with the
> -mno_fprnd command line.  The check prevents GCC crashing under these
> conditions.

(-mno-fprnd, a dash, not an underscore)

-mfprnd means all new insns in ISA 2.04; we should never allow this
option together with a -mcpu= that implies 2.04 or higher.

xsrdpi* require ISA 2.06 (Power7), so this testcase should work *anyway*,
even if fri* would be disabled for some strange reason.

> I read thru the source code looking for other builtins with the same
> issue.

>From the GCC manual:

  -mmfcrfp4  2.01
  -mpopcntb  p5  2.02
  -mfprndp5+ 2.04  ("info gcc" says 2.03, that's wrong?  But the ISA
says this is 2.02 even?  Now what!)
  -mcmpb p6  2.05
  -mpopcntd  p7  2.06

(and there are more, in fact).

> 2020-03-10  Carl Love  
> 
>   * gcc/config/rs6000/rs6000-c.c
> (altivec_resolve_overloaded_builtin):
>   Add check for TARGET_FRND and VSX_BUILTIN_XSRDPIM,
> VSX_BUILTIN_XSRDPIP
>   compatibility.

FPRND

> --- a/gcc/config/rs6000/rs6000-c.c
> +++ b/gcc/config/rs6000/rs6000-c.c
> @@ -915,6 +915,14 @@ altivec_resolve_overloaded_builtin (location_t
> loc, tree fndecl,
>const struct altivec_builtin_types *desc;
>unsigned int n;
>  
> +  /* Check builtin for command line argument conflicts.  */
> +  if (!TARGET_FPRND &&
> +  (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))

Lines never end in binary operators: that goes to the start of the next
line, instead, so

  if (!TARGET_FPRND
  && (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))

I'd write that the other way around, it reads nicer:

  if ((fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP)
  && !TARGET_FPRND)

but maybe that is just taste :-)

> {
> +  error ("%s is incompatible with mno-fprnd option",
> +  IDENTIFIER_POINTER (DECL_NAME (fndecl)));
> +  return error_mark_node;
> +  }

-mno-fprnd (options start with a dash, except in the first field in
rs6000.opt).

It would be better if you disallowed this option combination?  Don't
allow an options that says ISA X insns are allowed, but ISA Y insns are
not, with Y < X.  In this case X is 2.06 and Y is 2.02 or 2.03 or 2.04.


Segher

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Martin Sebor via Gcc-patches]

On 3/11/20 2:10 PM, Jason Merrill wrote:

On 3/11/20 12:57 PM, Martin Sebor wrote:

On 3/9/20 6:08 PM, Jason Merrill wrote:

On 3/9/20 5:39 PM, Martin Sebor wrote:

On 3/9/20 1:40 PM, Jason Merrill wrote:

On 3/9/20 12:31 PM, Martin Sebor wrote:

On 2/28/20 1:24 PM, Jason Merrill wrote:

On 2/28/20 12:45 PM, Martin Sebor wrote:

On 2/28/20 9:58 AM, Jason Merrill wrote:

On 2/24/20 6:58 PM, Martin Sebor wrote:

-Wredundant-tags doesn't consider type declarations that are also
the first uses of the type, such as in 'void f (struct S);' and
issues false positives for those.  According to the reported 
that's

making it harder to use the warning to clean up LibreOffice.

The attached patch extends -Wredundant-tags to avoid these false
positives by relying on the same class_decl_loc_t::class2loc 
mapping
as -Wmismatched-tags.  The patch also somewhat improves the 
detection
of both issues in template declarations (though more work is 
still

needed there).



+ a new entry for it and return unless it's a declaration
+ involving a template that may need to be diagnosed by
+ -Wredundant-tags.  */
   *rdl = class_decl_loc_t (class_key, false, def_p);
-  return;
+  if (TREE_CODE (decl) != TEMPLATE_DECL)
+    return;


How can the first appearance of a class template be redundant?


I'm not sure I correctly understand the question.  The comment says
"involving a template" (i.e., not one of the first declaration of
a template).  The test case that corresponds to this test is:

   template  struct S7 { };
   struct S7 s7v;  // { dg-warning "\\\[-Wredundant-tags" }

where DECL is the TEPLATE_DECL of S7.

As I mentioned, more work is still needed to handle templates right
because some redundant tags are still not diagnosed.  For example:

   template  struct S7 { };
   template 
   using U = struct S7;   // missing warning


When we get here for an instance of a template, it doesn't make 
sense to treat it as a new type.


If decl is a template and type_decl is an instance of that 
template, do we want to (before the lookup) change type_decl to 
the template or the corresponding generic TYPE_DECL, which should 
already be in the table?


I'm struggling with how to do this.  Given type (a RECORD_TYPE) and
type_decl (a TEMPLATE_DECL) representing the use of a template, how
do I get the corresponding template (or its explicit or partial
specialization) in the three cases below?

   1) Instance of the primary:
  template  class A;
  struct A a;

   2) Instance of an explicit specialization:
  template  class B;
  template <> struct B;
  class B b;

   3) Instance of a partial specialization:
  template  class C;
  template  struct C;
  class C c;

By trial and (lots of) error I figured out that in both (1) and (2),
but not in (3), TYPE_MAIN_DECL (TYPE_TI_TEMPLATE (type)) returns
the template's type_decl.

Is there some function to call to get it in (3), or even better,
in all three cases?


I think you're looking for most_general_template.


I don't think that's quite what I'm looking for.  At least it doesn't
return the template or its specialization in all three cases above.


Ah, true, that function stops at specializations.  Oddly, I don't 
think there's currently a similar function that looks through them. 
You could create one that does a simple loop through DECL_TI_TEMPLATE 
like is_specialization_of.


Thanks for the tip.  Even with that I'm having trouble with partial
specializations.  For example in:

   template    struct S;
   template  class S;
   extern class  S s1;
   extern struct S s2;  // expect -Wmismatched-tags

how do I find the declaration of the partial specialization when given
the type in the extern declaration?  A loop in my find_template_for()
function (similar to is_specialization_of) only visits the implicit
specialization S (i.e., its own type) and the primary.


Is that a problem?  The name is from the primary template, so does it 
matter for this warning whether there's an explicit specialization 
involved?


I don't understand the question.  S is an instance of
the partial specialization.  To diagnose the right mismatch the warning
needs to know how to find the template (i.e., either the primary, or
the explicit or partial specialization) the instance corresponds to and
the class-key it was declared with.  As it is, while GCC does diagnose
the right declaration (that of s2), it does that thanks to a bug:
because it finds and uses the type and class-key used to declare s1.
If we get rid of s1 it doesn't diagnose anything.

I tried using DECL_TEMPLATE_SPECIALIZATIONS() to get the list of
the partial specializations but it doesn't like any of the arguments
I've given it (it ICEs).

Martin

PS As an aside, both Clang and VC++ have trouble with templates as
well, just slightly different kinds of trouble.   Clang diagnoses
the declaration of the partial specialization while VC++ diagnoses
s1.  In other similar cases like the 

[pushed] c++: Fix ICE with concepts and aliases [PR93907].

[Jason Merrill via Gcc-patches]

The problem here was that we were checking satisfaction once with 'e', a
typedef of 'void', and another time with 'void' directly, and treated them
as different for hashing based on the assumption that
canonicalize_type_argument would have already removed a typedef that wasn't
a complex dependent alias.  But that wasn't happening here, so let's add a
call.

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

gcc/cp/ChangeLog
2020-03-11  Jason Merrill  

PR c++/93907
* constraint.cc (tsubst_parameter_mapping): Canonicalize type
argument.
---
 gcc/cp/cp-tree.h |  1 +
 gcc/cp/constraint.cc |  6 ++-
 gcc/cp/pt.c  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-using2.C | 45 
 4 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-using2.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 0a7381cee3f..757cdd8168a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7016,6 +7016,7 @@ extern tree resolve_nondeduced_context_or_error   (tree, 
tsubst_flags_t);
 extern hashval_t iterative_hash_template_arg (tree arg, hashval_t val);
 extern tree coerce_template_parms   (tree, tree, tree);
 extern tree coerce_template_parms   (tree, tree, tree, 
tsubst_flags_t);
+extern tree canonicalize_type_argument (tree, tsubst_flags_t);
 extern void register_local_specialization   (tree, tree);
 extern tree retrieve_local_specialization   (tree);
 extern tree extract_fnparm_pack (tree, tree *);
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 4bb4a3f7252..697ed6726b8 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2232,7 +2232,11 @@ tsubst_parameter_mapping (tree map, tree args, 
subst_info info)
   else if (ARGUMENT_PACK_P (arg))
new_arg = tsubst_argument_pack (arg, args, complain, in_decl);
   if (!new_arg)
-   new_arg = tsubst_template_arg (arg, args, complain, in_decl);
+   {
+ new_arg = tsubst_template_arg (arg, args, complain, in_decl);
+ if (TYPE_P (new_arg))
+   new_arg = canonicalize_type_argument (new_arg, complain);
+   }
   if (new_arg == error_mark_node)
return error_mark_node;
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index cb237ba0d9d..789ccdbbbd1 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -7943,7 +7943,7 @@ template_template_parm_bindings_ok_p (tree tparms, tree 
targs)
 /* Since type attributes aren't mangled, we need to strip them from
template type arguments.  */
 
-static tree
+tree
 canonicalize_type_argument (tree arg, tsubst_flags_t complain)
 {
   if (!arg || arg == error_mark_node || arg == TYPE_CANONICAL (arg))
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-using2.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-using2.C
new file mode 100644
index 000..b1a45d5e595
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-using2.C
@@ -0,0 +1,45 @@
+// PR c++/93907
+// { dg-options -std=gnu++20 }
+
+template  struct c {
+  static constexpr int d = a;
+  typedef c e;
+};
+template  struct f;
+template  using g = typename f::e;
+struct b;
+template  struct f { using e = b; };
+template  struct m { typedef g aj; };
+template  class n { typedef typename m::aj e; };
+template  using an = typename n::e;
+template  constexpr bool ao = c::d;
+template  constexpr bool i = c<1>::d;
+template  concept bb = i;
+using cc = __int128;
+template  concept cd = bb;
+template  concept ce = requires { requires cd; };
+template  concept h = ce;
+template  concept l = h;
+template  concept cl = ao;
+template  concept cp = requires(b j) {
+  requires h>;
+};
+struct o {
+  template  requires cp auto operator()(b) {}
+};
+template  using cm = decltype(o{}(b()));
+template  concept ct = l;
+template  concept dd = ct>;
+template  concept de = dd;
+struct {
+  template  void operator()(da, b);
+} di;
+class p {
+  void begin();
+};
+template  using df = p;
+template  void q() {
+  df k;
+  int d;
+  di(k, d);
+}

base-commit: 60342fdbfb0630243d2b85d2ca45204ded990b17
-- 
2.18.1

Re: [PATCH 1/2] c++: Replay errors during diagnosis of constraint satisfaction failures

[Jason Merrill via Gcc-patches]

On 3/9/20 6:23 PM, Patrick Palka wrote:

This patch adds a new flag -fconcepts-diagnostics-depth to the C++ frontend
which controls how deeply we replay errors when diagnosing a constraint
satisfaction failure.  The default is -fconcepts-diagnostics-depth=1 which
diagnoses only the topmost constraint satisfaction failure and is consistent
with our behavior before this patch.  By increasing this flag's value, the user
can control how deeply they want the compiler to explain a constraint
satisfaction error.

For example, if the unsatisfied constraint is a disjunction, then the default
behavior is to just say "no branch in the disjunction is satisfied", but with
-fconcepts-diagnostics-depth=2 we will additionally replay and diagnose the
error in each branch of the disjunction.  And if the unsatisfied constraint is a
requires expression, then we will replay the error in the requires expression,
etc.  This proceeds recursively until there is nothing more to replay or we
reached the exceeded the maximum depth specified by the flag.

Implementation wise, this patch essentially just uncomments the existing
commented-out code that performs the error-replaying, adding logic to keep track
of the current replay depth along the way.  Besides that, there is a new routine
collect_operands_of_disjunction which flattens a disjunction and collects all of
its operands into a vector.

Here are some examples of diagnostics with the two patches in this series.

For the simple test case in which we call ranges::begin() on something that's
not a range:

 #include 

 struct S { } s;
 auto x = std::ranges::begin(s);

we get the following diagnostics with -fconcepts-diagnostics-depth={1,2,3}
respectively:

  https://pppalka.github.io/ranges-begin-depth-1.html
  https://pppalka.github.io/ranges-begin-depth-2.html
  https://pppalka.github.io/ranges-begin-depth-3.html

And for the new test g++.dg/concepts/diagnostic5.C, we get:

 https://pppalka.github.io/diagnostic5-depth-1.html
 https://pppalka.github.io/diagnostic5-depth-2.html
 https://pppalka.github.io/diagnostic5-depth-3.html
 https://pppalka.github.io/diagnostic5-depth-4.html

The extra diagnostics enabled by this flag are at times longer than they need to
be (e.g.  "the operand is_array_v<...> is unsatisfied because \n the expression
is_array_v<...> [with ...] evaluated to false") and not immediately easy to
follow (especially when there are nested disjunctions), but the transparency
provided by these optional diagnostics seems to be pretty helpful in practice.

Does this seem like a sensible approach?  Thoughts and ideas for improvement
welcome.  Wording and naming suggestions would be much appreciated.


This does seem like a good approach, thanks.


gcc/c-family/ChangeLog:

* c.opt: Add -fconcepts-diagnostics-depth.

gcc/cp/ChangeLog:

* constraint.cc (finish_constraint_binary_op): Set the location of EXPR
as well as its range, because build_x_binary_op doesn't always do so.
(current_constraint_diagnosis_depth): New.
(concepts_diagnostics_max_depth_exceeded_p): New.
(collect_operands_of_disjunction): New.
(satisfy_disjunction): When diagnosing a satisfaction failure, maybe
replay each branch of the disjunction, subject to the current diagnosis
depth.
(diagnose_valid_expression): When diagnosing a satisfaction failure,
maybe replay the substitution error, subject to the current diagnosis
recursion.
(diagnose_valid_type): Likewise.
(diagnose_nested_requiremnet): Likewise.
(diagnosing_failed_constraint::diagnosing_failed_constraint): Increment
current_constraint_diagnosis_depth when diagnosing.
(diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement
current_constraint_diagnosis_depth when diagnosing.
(diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth
is 0.  Emit a one-off note to increase -fconcepts-diagnostics-depth if
the limit was exceeded.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of
"neither operand".
* g++.dg/concepts/diagnostic5.C: New test.
---
  gcc/c-family/c.opt  |   4 +
  gcc/cp/constraint.cc| 146 +---
  gcc/testsuite/g++.dg/concepts/diagnostic2.C |   2 +-
  gcc/testsuite/g++.dg/concepts/diagnostic5.C |  46 ++
  4 files changed, 177 insertions(+), 21 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 1cd585fa71d..97ef488931d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1453,6 +1453,10 @@ fconcepts-ts
  C++ ObjC++ Var(flag_concepts_ts) Init(0)
  Enable certain features present in the Concepts TS.
  
+fconcepts-diagnostics-depth=

+C++ ObjC++ Joined RejectNegative UInteger 

Re: [PATCH v2] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Marek Polacek via Gcc-patches]

On Wed, Mar 11, 2020 at 04:17:02PM -0400, Jason Merrill via Gcc-patches wrote:
> On 3/11/20 1:59 PM, Marek Polacek wrote:
> > On Tue, Mar 10, 2020 at 03:46:03PM -0400, Jason Merrill wrote:
> > > On 3/9/20 4:34 PM, Marek Polacek wrote:
> > > > On Mon, Mar 09, 2020 at 04:25:00PM -0400, Marek Polacek wrote:
> > > > > On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:
> > > > > > On 3/9/20 9:40 AM, Marek Polacek wrote:
> > > > > > > On Mon, Mar 09, 2020 at 09:19:30AM -0400, Jason Merrill wrote:
> > > > > > > > On 3/9/20 8:58 AM, Jakub Jelinek wrote:
> > > > > > > > > On Fri, Mar 06, 2020 at 07:43:43PM -0500, Jason Merrill wrote:
> > > > > > > > > > On 3/6/20 6:54 PM, Marek Polacek wrote:
> > > > > > > > > > > I got a report that building Chromium fails with the 
> > > > > > > > > > > "modifying a const
> > > > > > > > > > > object" error.  After some poking I realized it's a bug 
> > > > > > > > > > > in GCC, not in
> > > > > > > > > > > their codebase.
> > > > > > > > > > > 
> > > > > > > > > > > Much like with ARRAY_REFs, which can be const even though 
> > > > > > > > > > > the array
> > > > > > > > > > > itself isn't, COMPONENT_REFs can be const although 
> > > > > > > > > > > neither the object
> > > > > > > > > > > nor the field were declared const.  So let's dial down 
> > > > > > > > > > > the checking.
> > > > > > > > > > > Here the COMPONENT_REF was const because of the 
> > > > > > > > > > > "const_cast(m)"
> > > > > > > > > > > thing -- cxx_eval_component_reference then builds a 
> > > > > > > > > > > COMPONENT_REF with
> > > > > > > > > > > TREE_TYPE (t).
> > > > > > > > > > 
> > > > > > > > > > What is folding the const into the COMPONENT_REF?
> > > > > > > > > 
> > > > > > > > > cxx_eval_component_reference when it is called on
> > > > > > > > > ((const struct array *) this)->elems
> > > > > > > > > with /*lval=*/true and lval is true because we are evaluating
> > > > > > > > >  = (const int &) &((const struct array *) 
> > > > > > > > > this)->elems[VIEW_CONVERT_EXPR(n)];
> > > > > > > > 
> > > > > > > > Ah, sure.  We're pretty loose with cv-quals in the constexpr 
> > > > > > > > code in
> > > > > > > > general, so it's probably not worth trying to change that here. 
> > > > > > > >  Getting
> > > > > > > > back to the patch:
> > > > > > > 
> > > > > > > Yes, here the additional const was caused by a const_cast adding 
> > > > > > > a const.
> > > > > > > 
> > > > > > > But this could also happen with wrapper functions like this one 
> > > > > > > from
> > > > > > > __array_traits in std::array:
> > > > > > > 
> > > > > > >  static constexpr _Tp&
> > > > > > >  _S_ref(const _Type& __t, std::size_t __n) noexcept
> > > > > > >  { return const_cast<_Tp&>(__t[__n]); }
> > > > > > > 
> > > > > > > where the ref-to-const parameter added the const.
> > > > > > > 
> > > > > > > > > +  if (TREE_CODE (obj) == COMPONENT_REF)
> > > > > > > > > + {
> > > > > > > > > +   tree op1 = TREE_OPERAND (obj, 1);
> > > > > > > > > +   if (CP_TYPE_CONST_P (TREE_TYPE (op1)))
> > > > > > > > > + return true;
> > > > > > > > > +   else
> > > > > > > > > + {
> > > > > > > > > +   tree op0 = TREE_OPERAND (obj, 0);
> > > > > > > > > +   /* The LHS of . or -> might itself be a 
> > > > > > > > > COMPONENT_REF.  */
> > > > > > > > > +   if (TREE_CODE (op0) == COMPONENT_REF)
> > > > > > > > > + op0 = TREE_OPERAND (op0, 1);
> > > > > > > > > +   return CP_TYPE_CONST_P (TREE_TYPE (op0));
> > > > > > > > > + }
> > > > > > > > > + }
> > > > > > > > 
> > > > > > > > Shouldn't this be a loop?
> > > > > > > 
> > > > > > > I don't think so, though my earlier patch had a call to
> > > > > > > 
> > > > > > > +static bool
> > > > > > > +cref_has_const_field (tree ref)
> > > > > > > +{
> > > > > > > +  while (TREE_CODE (ref) == COMPONENT_REF)
> > > > > > > +{
> > > > > > > +  if (CP_TYPE_CONST_P (TREE_TYPE (TREE_OPERAND (ref, 1
> > > > > > > +   return true;
> > > > > > > +  ref = TREE_OPERAND (ref, 0);
> > > > > > > +}
> > > > > > > +  return false;
> > > > > > > +}
> > > > > > 
> > > > > > > here.  A problem arised when I checked even the outermost 
> > > > > > > expression (which is not a
> > > > > > > field_decl), then I saw another problematical error.
> > > > > > > 
> > > > > > > The more outer fields are expected to be checked in subsequent 
> > > > > > > calls to
> > > > > > > modifying_const_object_p in next iterations of the
> > > > > > > 
> > > > > > > 4459   for (tree probe = target; object == NULL_TREE; )
> > > > > > > 
> > > > > > > loop in cxx_eval_store_expression.
> > > > > > 
> > > > > > OK, but then why do you want to check two levels here rather than 
> > > > > > just one?
> > > > > 
> > > > > It's a hack to keep constexpr-tracking-const7.C working.  There we 
> > > > > have
> > > > > 
> > > > > b.a.c.d.n
> > > > > 
> > > > > wherein 'd' is const struct D, but 'n' 

Re: [PATCH v2] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jason Merrill via Gcc-patches]

On 3/11/20 1:59 PM, Marek Polacek wrote:

On Tue, Mar 10, 2020 at 03:46:03PM -0400, Jason Merrill wrote:

On 3/9/20 4:34 PM, Marek Polacek wrote:

On Mon, Mar 09, 2020 at 04:25:00PM -0400, Marek Polacek wrote:

On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:

On 3/9/20 9:40 AM, Marek Polacek wrote:

On Mon, Mar 09, 2020 at 09:19:30AM -0400, Jason Merrill wrote:

On 3/9/20 8:58 AM, Jakub Jelinek wrote:

On Fri, Mar 06, 2020 at 07:43:43PM -0500, Jason Merrill wrote:

On 3/6/20 6:54 PM, Marek Polacek wrote:

I got a report that building Chromium fails with the "modifying a const
object" error.  After some poking I realized it's a bug in GCC, not in
their codebase.

Much like with ARRAY_REFs, which can be const even though the array
itself isn't, COMPONENT_REFs can be const although neither the object
nor the field were declared const.  So let's dial down the checking.
Here the COMPONENT_REF was const because of the "const_cast(m)"
thing -- cxx_eval_component_reference then builds a COMPONENT_REF with
TREE_TYPE (t).


What is folding the const into the COMPONENT_REF?


cxx_eval_component_reference when it is called on
((const struct array *) this)->elems
with /*lval=*/true and lval is true because we are evaluating
 = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR(n)];


Ah, sure.  We're pretty loose with cv-quals in the constexpr code in
general, so it's probably not worth trying to change that here.  Getting
back to the patch:


Yes, here the additional const was caused by a const_cast adding a const.

But this could also happen with wrapper functions like this one from
__array_traits in std::array:

 static constexpr _Tp&
 _S_ref(const _Type& __t, std::size_t __n) noexcept
 { return const_cast<_Tp&>(__t[__n]); }

where the ref-to-const parameter added the const.


+  if (TREE_CODE (obj) == COMPONENT_REF)
+   {
+ tree op1 = TREE_OPERAND (obj, 1);
+ if (CP_TYPE_CONST_P (TREE_TYPE (op1)))
+   return true;
+ else
+   {
+ tree op0 = TREE_OPERAND (obj, 0);
+ /* The LHS of . or -> might itself be a COMPONENT_REF.  */
+ if (TREE_CODE (op0) == COMPONENT_REF)
+   op0 = TREE_OPERAND (op0, 1);
+ return CP_TYPE_CONST_P (TREE_TYPE (op0));
+   }
+   }


Shouldn't this be a loop?


I don't think so, though my earlier patch had a call to

+static bool
+cref_has_const_field (tree ref)
+{
+  while (TREE_CODE (ref) == COMPONENT_REF)
+{
+  if (CP_TYPE_CONST_P (TREE_TYPE (TREE_OPERAND (ref, 1
+   return true;
+  ref = TREE_OPERAND (ref, 0);
+}
+  return false;
+}



here.  A problem arised when I checked even the outermost expression (which is 
not a
field_decl), then I saw another problematical error.

The more outer fields are expected to be checked in subsequent calls to
modifying_const_object_p in next iterations of the

4459   for (tree probe = target; object == NULL_TREE; )

loop in cxx_eval_store_expression.


OK, but then why do you want to check two levels here rather than just one?


It's a hack to keep constexpr-tracking-const7.C working.  There we have

b.a.c.d.n

wherein 'd' is const struct D, but 'n' isn't const.  Without the hack
const_object_being_modified would be 'b.a.c.d', but due to the problem I
desribed in the original mail[1] the constructor for D wouldn't have
TREE_READONLY set.  With the hack const_object_being_modified will be
'b.a.c.d.n', which is of non-class type so we error:

4710   if (!CLASS_TYPE_P (const_objtype))
4711 fail = true;

I could remove the hack and maybe XFAIL constexpr-tracking-const7.C if you
want.  Unfortunately I wasn't aware of [1] when I added that feature and
checking if the whole COMPONENT_REF is const seemed to be enough.


So if D was a wrapper around another class with the int field, this hack
looking one level out wouldn't help?


Correct ;(.  I went back to my version using cref_has_const_field to keep
constexpr-tracking-const7.C and its derivates working.


It's probably not a good idea to make this checking more strict at this
stage.

[1] "While looking into this I noticed that we don't detect modifying a const
object in certain cases like in
.  That's because
we never evaluate an X::X() CALL_EXPR -- there's none.  So there's no
CONSTRUCTOR to set TREE_READONLY on.  No idea how to fix this, but it's
likely something for GCC 11 anyway."

How about this?




diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 76af0d710c4..b3d3499b9ac 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4759,6 +4759,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, 
tree t,
else
  *valp = init;
  
+  /* After initialization, 'const' semantics apply to the value of the

+ object. Make a note of this fact by marking the CONSTRUCTOR
+ TREE_READONLY.  */
+  if (TREE_CODE (t) == 

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Jason Merrill via Gcc-patches]

On 3/11/20 12:57 PM, Martin Sebor wrote:

On 3/9/20 6:08 PM, Jason Merrill wrote:

On 3/9/20 5:39 PM, Martin Sebor wrote:

On 3/9/20 1:40 PM, Jason Merrill wrote:

On 3/9/20 12:31 PM, Martin Sebor wrote:

On 2/28/20 1:24 PM, Jason Merrill wrote:

On 2/28/20 12:45 PM, Martin Sebor wrote:

On 2/28/20 9:58 AM, Jason Merrill wrote:

On 2/24/20 6:58 PM, Martin Sebor wrote:

-Wredundant-tags doesn't consider type declarations that are also
the first uses of the type, such as in 'void f (struct S);' and
issues false positives for those.  According to the reported 
that's

making it harder to use the warning to clean up LibreOffice.

The attached patch extends -Wredundant-tags to avoid these false
positives by relying on the same class_decl_loc_t::class2loc 
mapping
as -Wmismatched-tags.  The patch also somewhat improves the 
detection

of both issues in template declarations (though more work is still
needed there).



+ a new entry for it and return unless it's a declaration
+ involving a template that may need to be diagnosed by
+ -Wredundant-tags.  */
   *rdl = class_decl_loc_t (class_key, false, def_p);
-  return;
+  if (TREE_CODE (decl) != TEMPLATE_DECL)
+    return;


How can the first appearance of a class template be redundant?


I'm not sure I correctly understand the question.  The comment says
"involving a template" (i.e., not one of the first declaration of
a template).  The test case that corresponds to this test is:

   template  struct S7 { };
   struct S7 s7v;  // { dg-warning "\\\[-Wredundant-tags" }

where DECL is the TEPLATE_DECL of S7.

As I mentioned, more work is still needed to handle templates right
because some redundant tags are still not diagnosed.  For example:

   template  struct S7 { };
   template 
   using U = struct S7;   // missing warning


When we get here for an instance of a template, it doesn't make 
sense to treat it as a new type.


If decl is a template and type_decl is an instance of that 
template, do we want to (before the lookup) change type_decl to 
the template or the corresponding generic TYPE_DECL, which should 
already be in the table?


I'm struggling with how to do this.  Given type (a RECORD_TYPE) and
type_decl (a TEMPLATE_DECL) representing the use of a template, how
do I get the corresponding template (or its explicit or partial
specialization) in the three cases below?

   1) Instance of the primary:
  template  class A;
  struct A a;

   2) Instance of an explicit specialization:
  template  class B;
  template <> struct B;
  class B b;

   3) Instance of a partial specialization:
  template  class C;
  template  struct C;
  class C c;

By trial and (lots of) error I figured out that in both (1) and (2),
but not in (3), TYPE_MAIN_DECL (TYPE_TI_TEMPLATE (type)) returns
the template's type_decl.

Is there some function to call to get it in (3), or even better,
in all three cases?


I think you're looking for most_general_template.


I don't think that's quite what I'm looking for.  At least it doesn't
return the template or its specialization in all three cases above.


Ah, true, that function stops at specializations.  Oddly, I don't 
think there's currently a similar function that looks through them.  
You could create one that does a simple loop through DECL_TI_TEMPLATE 
like is_specialization_of.


Thanks for the tip.  Even with that I'm having trouble with partial
specializations.  For example in:

   template    struct S;
   template  class S;
   extern class  S s1;
   extern struct S s2;  // expect -Wmismatched-tags

how do I find the declaration of the partial specialization when given
the type in the extern declaration?  A loop in my find_template_for()
function (similar to is_specialization_of) only visits the implicit
specialization S (i.e., its own type) and the primary.


Is that a problem?  The name is from the primary template, so does it 
matter for this warning whether there's an explicit specialization involved?



Martin




In (2) and (3) it won't distinguish between specializations of B or
C on different types.  In (2), the function returns the same result
for both:

   template <> struct B;
   template <> struct B;

In (3), it similarly returns the same result for both of

   template  struct C;
   template  struct C;

even though they are declarations of distinct types.



Jason

Re: [PATCH,rs6000] Add command line and builtin compatibility

[Carl Love via Gcc-patches]

Bill:

On Wed, 2020-03-11 at 14:12 -0500, Bill Schmidt wrote:
> I believe you need %qs here.  Also replace mno-fprnd with %qs and
> put 
> "-mno-fprnd" as the associated parameter.
> 
> Example from nearby code:   error ("%qs requires %qs", "-mdirect-
> move", 
> "-mvsx");

Yes.  I had originally tried to put in -mno-fprnd in the message and
got compilation errors.  Had to drop the leading dash to get it to
compile.  The %qs fixes that!

I re-did the patch, retested on Power 8 and Power 9.  The test results
are now:

   gcc -g -mno-fprnd vsx-builtin-3.c -o  vsx-builtin-3
   vsx-builtin-3.c: In function ‘do_math’:
   vsx-builtin-3.c:145:3: error: ‘__builtin_vsx_xsrdpim’ is incompatible with 
‘-mno-fprnd’ option
 145 |   z[i][0] = __builtin_vsx_xsrdpim (z[i][1]); i++;
 |   ^
   vsx-builtin-3.c:146:3: error: ‘__builtin_vsx_xsrdpip’ is incompatible with 
‘-mno-fprnd’ option
 146 |   z[i][0] = __builtin_vsx_xsrdpip (z[i][1]); i++;
 |   ^

The updated patch is below.  Please let me know if there are any
additional things needing fixing for mainline.  Thanks.

   Carl Love

-
rs6000: Add command line and builtin compatibility check

PR/target 87583

gcc/ChangeLog

2020-03-11  Carl Love  

* gcc/config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
Add check for TARGET_FRND and VSX_BUILTIN_XSRDPIM,
VSX_BUILTIN_XSRDPIP compatibility.
---
 gcc/config/rs6000/rs6000-c.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index 8c1fbbf..558e829 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -915,6 +915,14 @@ altivec_resolve_overloaded_builtin (location_t loc, tree 
fndecl,
   const struct altivec_builtin_types *desc;
   unsigned int n;
 
+  /* Check builtin for command line argument conflicts.  */
+  if (!TARGET_FPRND &&
+  (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP)) {
+  error ("%qs is incompatible with %qs option",
+IDENTIFIER_POINTER (DECL_NAME (fndecl)), 
"-mno-fprnd");
+  return error_mark_node;
+  }
+
   if (!rs6000_overloaded_builtin_p (fcode))
 return NULL_TREE;
 
-- 
2.7.4

Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Jason Merrill via Gcc-patches]

On 3/11/20 11:44 AM, Marek Polacek wrote:

On Wed, Mar 11, 2020 at 07:52:15AM +0100, Jakub Jelinek via Gcc-patches wrote:

On Tue, Mar 10, 2020 at 07:38:17PM -0400, Marek Polacek via Gcc-patches wrote:

--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6062,6 +6062,13 @@ reshape_init_array_1 (tree elt_type, tree max_index, 
reshape_iter *d,
else if (last_nonzero < nelts - 1)
nelts = last_nonzero + 1;
  
+  /* Sharing a stripped constructor can get in the way of

+overload resolution.  E.g., initializing a class from
+{{0}} might be invalid while initializing the same class
+from {{}} might be valid.  */
+  if (reuse)
+   new_init = unshare_constructor (new_init);
+
vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);


Isn't it wasteful to first copy perhaps a large constructor (recursively)
and then truncate it to very few elts (zero in this case)?
So, perhaps doing instead:
   if (reuse)
{
  vec *v = NULL;
  if (nelts)
vec_alloc (v, nelts);
  for (unsigned int i = 0; i < nelts; i++)
{
  constructor_elt elt = CONSTRUCTOR_ELT (new_init, i);
  if (TREE_CODE (elt.value) == CONSTRUCTOR)
elt.value = unshare_constructor (elt.value);
  v->quick_push (elt);
}
  new_init = build_constructor (TREE_TYPE (new_init), v);
}
   else
vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
?


Large constructors aren't expected here.  From bootstrap + testsuite
measurements I can see that we have 8035 actual truncations, and 7884
of those are of constructors shorter than 10 elements.  There's one case
where we have a ctor of 567 elements and truncate it to 559 elements but
that seems pathological.  Is it worth it (genuine quiestion)?

Note there are 37077 attempts to truncate, of which 29042 truncate to
the same length.  Our ->truncate merely sets m_num, so it's cheap, but
I'm still a bit surprised we don't check

   if (CONSTRUCTOR_NELTS (new_init) != nelts)

before calling vec_safe_truncate.


We should certainly avoid copying if they're the same.  The code above 
for only copying the bits that aren't going to be thrown away seems 
pretty straightforward, might as well use it even if the savings aren't 
likely to be large.


Jason

Re: [PATCH,rs6000] Add command line and builtin compatibility

[Bill Schmidt via Gcc-patches]

On 3/11/20 2:00 PM, Carl Love wrote:

GCC maintianers:

The following patch add a check to make sure the user did not specify
-mno_fprnd with the builtins  __builtin_vsx_xsrdpim and
__builtin_vsx_xsrdpip.  These builtins are incompatible with the
-mno_fprnd command line.  The check prevents GCC crashing under these
conditions.

Manually tested the patch on

   powerpc64le-unknown-linux-gnu (Power 8 LE)
   powerpc64le-unknown-linux-gnu (Power 9 LE)

as follows:

gcc -mno-fprnd -g -c vsx-builtin-3.c
vsx-builtin-3.c: In function ‘do_math’:
vsx-builtin-3.c:145:3: error: __builtin_vsx_xsrdpim is incompatible
with mno-fprnd option
  145 |   z[i][0] = __builtin_vsx_xsrdpim (z[i][1]); i++;
  |   ^
vsx-builtin-3.c:146:3: error: __builtin_vsx_xsrdpip is incompatible
with mno-fprnd option
  146 |   z[i][0] = __builtin_vsx_xsrdpip (z[i][1]); i++;
  |   ^

I read thru the source code looking for other builtins with the same
issue.  I also created a script to compile all of the tests in
gcc/testsuite/gcc.target/powerpc with the -mno-fprnd option to check
for additional builtins that are incompatible with the -mno-fprnd
option.  These were the only two builtins that were identified as being
incompatible with the -mno-fprnd option.

Please let me know if the patch looks OK for mainline.  Thanks.

  Carl Love

---
rs6000: Add command line and builtin compatibility check

PR/target 87583

gcc/ChangeLog

2020-03-10  Carl Love  

* gcc/config/rs6000/rs6000-c.c
(altivec_resolve_overloaded_builtin):
Add check for TARGET_FRND and VSX_BUILTIN_XSRDPIM,
VSX_BUILTIN_XSRDPIP
compatibility.
---
  gcc/config/rs6000/rs6000-c.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-
c.c
index 8c1fbbf..6820782 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -915,6 +915,14 @@ altivec_resolve_overloaded_builtin (location_t
loc, tree fndecl,
const struct altivec_builtin_types *desc;
unsigned int n;

+  /* Check builtin for command line argument conflicts.  */
+  if (!TARGET_FPRND &&
+  (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))
{
+  error ("%s is incompatible with mno-fprnd option",



I believe you need %qs here.  Also replace mno-fprnd with %qs and put 
"-mno-fprnd" as the associated parameter.


Example from nearby code:   error ("%qs requires %qs", "-mdirect-move", 
"-mvsx");


Thanks,
Bill


+IDENTIFIER_POINTER (DECL_NAME (fndecl)));
+  return error_mark_node;
+  }
+
if (!rs6000_overloaded_builtin_p (fcode))
  return NULL_TREE;

[PATCH,rs6000] Add command line and builtin compatibility

[Carl Love via Gcc-patches]

GCC maintianers:

The following patch add a check to make sure the user did not specify 
-mno_fprnd with the builtins  __builtin_vsx_xsrdpim and
__builtin_vsx_xsrdpip.  These builtins are incompatible with the
-mno_fprnd command line.  The check prevents GCC crashing under these
conditions.

Manually tested the patch on 

  powerpc64le-unknown-linux-gnu (Power 8 LE)
  powerpc64le-unknown-linux-gnu (Power 9 LE)

as follows:

   gcc -mno-fprnd -g -c vsx-builtin-3.c
   vsx-builtin-3.c: In function ‘do_math’:
   vsx-builtin-3.c:145:3: error: __builtin_vsx_xsrdpim is incompatible
   with mno-fprnd option
 145 |   z[i][0] = __builtin_vsx_xsrdpim (z[i][1]); i++;
 |   ^
   vsx-builtin-3.c:146:3: error: __builtin_vsx_xsrdpip is incompatible
   with mno-fprnd option
 146 |   z[i][0] = __builtin_vsx_xsrdpip (z[i][1]); i++;
 |   ^

I read thru the source code looking for other builtins with the same
issue.  I also created a script to compile all of the tests in
gcc/testsuite/gcc.target/powerpc with the -mno-fprnd option to check
for additional builtins that are incompatible with the -mno-fprnd
option.  These were the only two builtins that were identified as being
incompatible with the -mno-fprnd option.

Please let me know if the patch looks OK for mainline.  Thanks.

 Carl Love

---
rs6000: Add command line and builtin compatibility check

PR/target 87583

gcc/ChangeLog

2020-03-10  Carl Love  

* gcc/config/rs6000/rs6000-c.c
(altivec_resolve_overloaded_builtin):
Add check for TARGET_FRND and VSX_BUILTIN_XSRDPIM,
VSX_BUILTIN_XSRDPIP
compatibility.
---
 gcc/config/rs6000/rs6000-c.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-
c.c
index 8c1fbbf..6820782 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -915,6 +915,14 @@ altivec_resolve_overloaded_builtin (location_t
loc, tree fndecl,
   const struct altivec_builtin_types *desc;
   unsigned int n;
 
+  /* Check builtin for command line argument conflicts.  */
+  if (!TARGET_FPRND &&
+  (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))
{
+  error ("%s is incompatible with mno-fprnd option",
+IDENTIFIER_POINTER (DECL_NAME (fndecl)));
+  return error_mark_node;
+  }
+
   if (!rs6000_overloaded_builtin_p (fcode))
 return NULL_TREE;
 
-- 
2.7.4

Re: [GCC][Patch]Bug fix: cannot convert 'const short int*' to 'const __bf16*'

[Kyrill Tkachov]

On 3/11/20 5:59 PM, Kyrill Tkachov wrote:

Hi Delia,

On 3/11/20 5:49 PM, Delia Burduv wrote:

This patch fixes a bug introduced by my earlier patch (
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/541680.html ).
It introduces a new scalar builtin type that was missing in the original
patch.

Bootstrapped cleanly on arm-none-linux-gnueabihf.
Tested for regression on arm-none-linux-gnueabihf. No regression from
before the original patch.
Tests that failed or became unsupported because of the original tests
now work as they did before it.

gcc/ChangeLog:

2020-03-11  Delia Burduv  

    * config/arm/arm-builtins.c
 (arm_init_simd_builtin_scalar_types): New
    * config/arm/arm_neon.h (vld2_bf16): Used new builtin type
    (vld2q_bf16): Used new builtin type
    (vld3_bf16): Used new builtin type
    (vld3q_bf16): Used new builtin type
    (vld4_bf16): Used new builtin type
    (vld4q_bf16): Used new builtin type
    (vld2_dup_bf16): Used new builtin type
    (vld2q_dup_bf16): Used new builtin type
    (vld3_dup_bf16): Used new builtin type
    (vld3q_dup_bf16): Used new builtin type
    (vld4_dup_bf16): Used new builtin type
    (vld4q_dup_bf16): Used new builtin type


ChangeLog entries should have a full stop after each entry.

The patch is ok.

Thanks for the quick fix,



To be clear, I've pushed it to master with a fixed ChangeLog as 
1c43ee69f4f6148fff4b5ace80d709d7f8b250d7


Kyrill




Kyrill

Re: [GCC][Patch]Bug fix: cannot convert 'const short int*' to 'const __bf16*'

[Kyrill Tkachov]

Hi Delia,

On 3/11/20 5:49 PM, Delia Burduv wrote:

This patch fixes a bug introduced by my earlier patch (
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/541680.html ).
It introduces a new scalar builtin type that was missing in the original
patch.

Bootstrapped cleanly on arm-none-linux-gnueabihf.
Tested for regression on arm-none-linux-gnueabihf. No regression from
before the original patch.
Tests that failed or became unsupported because of the original tests
now work as they did before it.

gcc/ChangeLog:

2020-03-11  Delia Burduv  

    * config/arm/arm-builtins.c
 (arm_init_simd_builtin_scalar_types): New
    * config/arm/arm_neon.h (vld2_bf16): Used new builtin type
    (vld2q_bf16): Used new builtin type
    (vld3_bf16): Used new builtin type
    (vld3q_bf16): Used new builtin type
    (vld4_bf16): Used new builtin type
    (vld4q_bf16): Used new builtin type
    (vld2_dup_bf16): Used new builtin type
    (vld2q_dup_bf16): Used new builtin type
    (vld3_dup_bf16): Used new builtin type
    (vld3q_dup_bf16): Used new builtin type
    (vld4_dup_bf16): Used new builtin type
    (vld4q_dup_bf16): Used new builtin type


ChangeLog entries should have a full stop after each entry.

The patch is ok.

Thanks for the quick fix,

Kyrill

Re: [PATCH v2] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Marek Polacek via Gcc-patches]

On Tue, Mar 10, 2020 at 03:46:03PM -0400, Jason Merrill wrote:
> On 3/9/20 4:34 PM, Marek Polacek wrote:
> > On Mon, Mar 09, 2020 at 04:25:00PM -0400, Marek Polacek wrote:
> > > On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:
> > > > On 3/9/20 9:40 AM, Marek Polacek wrote:
> > > > > On Mon, Mar 09, 2020 at 09:19:30AM -0400, Jason Merrill wrote:
> > > > > > On 3/9/20 8:58 AM, Jakub Jelinek wrote:
> > > > > > > On Fri, Mar 06, 2020 at 07:43:43PM -0500, Jason Merrill wrote:
> > > > > > > > On 3/6/20 6:54 PM, Marek Polacek wrote:
> > > > > > > > > I got a report that building Chromium fails with the 
> > > > > > > > > "modifying a const
> > > > > > > > > object" error.  After some poking I realized it's a bug in 
> > > > > > > > > GCC, not in
> > > > > > > > > their codebase.
> > > > > > > > > 
> > > > > > > > > Much like with ARRAY_REFs, which can be const even though the 
> > > > > > > > > array
> > > > > > > > > itself isn't, COMPONENT_REFs can be const although neither 
> > > > > > > > > the object
> > > > > > > > > nor the field were declared const.  So let's dial down the 
> > > > > > > > > checking.
> > > > > > > > > Here the COMPONENT_REF was const because of the 
> > > > > > > > > "const_cast(m)"
> > > > > > > > > thing -- cxx_eval_component_reference then builds a 
> > > > > > > > > COMPONENT_REF with
> > > > > > > > > TREE_TYPE (t).
> > > > > > > > 
> > > > > > > > What is folding the const into the COMPONENT_REF?
> > > > > > > 
> > > > > > > cxx_eval_component_reference when it is called on
> > > > > > > ((const struct array *) this)->elems
> > > > > > > with /*lval=*/true and lval is true because we are evaluating
> > > > > > >  = (const int &) &((const struct array *) 
> > > > > > > this)->elems[VIEW_CONVERT_EXPR(n)];
> > > > > > 
> > > > > > Ah, sure.  We're pretty loose with cv-quals in the constexpr code in
> > > > > > general, so it's probably not worth trying to change that here.  
> > > > > > Getting
> > > > > > back to the patch:
> > > > > 
> > > > > Yes, here the additional const was caused by a const_cast adding a 
> > > > > const.
> > > > > 
> > > > > But this could also happen with wrapper functions like this one from
> > > > > __array_traits in std::array:
> > > > > 
> > > > > static constexpr _Tp&
> > > > > _S_ref(const _Type& __t, std::size_t __n) noexcept
> > > > > { return const_cast<_Tp&>(__t[__n]); }
> > > > > 
> > > > > where the ref-to-const parameter added the const.
> > > > > 
> > > > > > > +  if (TREE_CODE (obj) == COMPONENT_REF)
> > > > > > > + {
> > > > > > > +   tree op1 = TREE_OPERAND (obj, 1);
> > > > > > > +   if (CP_TYPE_CONST_P (TREE_TYPE (op1)))
> > > > > > > + return true;
> > > > > > > +   else
> > > > > > > + {
> > > > > > > +   tree op0 = TREE_OPERAND (obj, 0);
> > > > > > > +   /* The LHS of . or -> might itself be a COMPONENT_REF.  */
> > > > > > > +   if (TREE_CODE (op0) == COMPONENT_REF)
> > > > > > > + op0 = TREE_OPERAND (op0, 1);
> > > > > > > +   return CP_TYPE_CONST_P (TREE_TYPE (op0));
> > > > > > > + }
> > > > > > > + }
> > > > > > 
> > > > > > Shouldn't this be a loop?
> > > > > 
> > > > > I don't think so, though my earlier patch had a call to
> > > > > 
> > > > > +static bool
> > > > > +cref_has_const_field (tree ref)
> > > > > +{
> > > > > +  while (TREE_CODE (ref) == COMPONENT_REF)
> > > > > +{
> > > > > +  if (CP_TYPE_CONST_P (TREE_TYPE (TREE_OPERAND (ref, 1
> > > > > +   return true;
> > > > > +  ref = TREE_OPERAND (ref, 0);
> > > > > +}
> > > > > +  return false;
> > > > > +}
> > > > 
> > > > > here.  A problem arised when I checked even the outermost expression 
> > > > > (which is not a
> > > > > field_decl), then I saw another problematical error.
> > > > > 
> > > > > The more outer fields are expected to be checked in subsequent calls 
> > > > > to
> > > > > modifying_const_object_p in next iterations of the
> > > > > 
> > > > > 4459   for (tree probe = target; object == NULL_TREE; )
> > > > > 
> > > > > loop in cxx_eval_store_expression.
> > > > 
> > > > OK, but then why do you want to check two levels here rather than just 
> > > > one?
> > > 
> > > It's a hack to keep constexpr-tracking-const7.C working.  There we have
> > > 
> > >b.a.c.d.n
> > > 
> > > wherein 'd' is const struct D, but 'n' isn't const.  Without the hack
> > > const_object_being_modified would be 'b.a.c.d', but due to the problem I
> > > desribed in the original mail[1] the constructor for D wouldn't have
> > > TREE_READONLY set.  With the hack const_object_being_modified will be
> > > 'b.a.c.d.n', which is of non-class type so we error:
> > > 
> > > 4710   if (!CLASS_TYPE_P (const_objtype))
> > > 4711 fail = true;
> > > 
> > > I could remove the hack and maybe XFAIL constexpr-tracking-const7.C if you
> > > want.  Unfortunately I wasn't aware of [1] when I added that feature and
> > > checking if the whole COMPONENT_REF is const seemed to be enough.
> 

[GCC][Patch]Bug fix: cannot convert 'const short int*' to 'const __bf16*'

[Delia Burduv]

This patch fixes a bug introduced by my earlier patch ( 
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/541680.html ).
It introduces a new scalar builtin type that was missing in the original 
patch.


Bootstrapped cleanly on arm-none-linux-gnueabihf.
Tested for regression on arm-none-linux-gnueabihf. No regression from 
before the original patch.
Tests that failed or became unsupported because of the original tests 
now work as they did before it.


gcc/ChangeLog:

2020-03-11  Delia Burduv  

* config/arm/arm-builtins.c
(arm_init_simd_builtin_scalar_types): New
* config/arm/arm_neon.h (vld2_bf16): Used new builtin type
(vld2q_bf16): Used new builtin type
(vld3_bf16): Used new builtin type
(vld3q_bf16): Used new builtin type
(vld4_bf16): Used new builtin type
(vld4q_bf16): Used new builtin type
(vld2_dup_bf16): Used new builtin type
(vld2q_dup_bf16): Used new builtin type
(vld3_dup_bf16): Used new builtin type
(vld3q_dup_bf16): Used new builtin type
(vld4_dup_bf16): Used new builtin type
(vld4q_dup_bf16): Used new builtin type
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
index e0561c5..1f55898 100644
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -784,6 +784,7 @@ const char *arm_scalar_builtin_types[] = {
   "__builtin_neon_oi",
   "__builtin_neon_ci",
   "__builtin_neon_xi",
+  "__builtin_neon_bf",
   NULL
 };
 
@@ -1101,7 +1102,8 @@ arm_init_simd_builtin_scalar_types (void)
 	 "__builtin_neon_df");
   (*lang_hooks.types.register_builtin_type) (intTI_type_node,
 	 "__builtin_neon_ti");
-
+  (*lang_hooks.types.register_builtin_type) (arm_bf16_type_node,
+ "__builtin_neon_bf");
   /* Unsigned integer types for various mode sizes.  */
   (*lang_hooks.types.register_builtin_type) (unsigned_intQI_type_node,
 	 "__builtin_neon_uqi");
diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h
index f5ccf18..aa21730 100644
--- a/gcc/config/arm/arm_neon.h
+++ b/gcc/config/arm/arm_neon.h
@@ -19562,7 +19562,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld2_bf16 (bfloat16_t const * __ptr)
 {
   union { bfloat16x4x2_t __i; __builtin_neon_ti __o; } __rv;
-  __rv.__o = __builtin_neon_vld2v4bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld2v4bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19571,7 +19571,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld2q_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x8x2_t __i; __builtin_neon_oi __o; } __rv;
-  __rv.__o = __builtin_neon_vld2v8bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld2v8bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19580,7 +19580,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld3_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x4x3_t __i; __builtin_neon_ei __o; } __rv;
-  __rv.__o = __builtin_neon_vld3v4bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld3v4bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19589,7 +19589,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld3q_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x8x3_t __i; __builtin_neon_ci __o; } __rv;
-  __rv.__o = __builtin_neon_vld3v8bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld3v8bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19598,7 +19598,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld4_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x4x4_t __i; __builtin_neon_oi __o; } __rv;
-  __rv.__o = __builtin_neon_vld4v4bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld4v4bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19607,7 +19607,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld4q_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x8x4_t __i; __builtin_neon_xi __o; } __rv;
-  __rv.__o = __builtin_neon_vld4v8bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld4v8bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19616,7 +19616,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld2_dup_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x4x2_t __i; __builtin_neon_ti __o; } __rv;
-  __rv.__o = __builtin_neon_vld2_dupv4bf ((const __builtin_neon_hi *) __ptr);
+  __rv.__o = __builtin_neon_vld2_dupv4bf ((const __builtin_neon_bf *) __ptr);
   return __rv.__i;
 }
 
@@ -19625,7 +19625,7 @@ __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
 vld2q_dup_bf16 (const bfloat16_t * __ptr)
 {
   union { bfloat16x8x2_t __i; __builtin_neon_oi __o; } __rv;
-  __rv.__o = 

Re: [PATCH] pdp11: Fix handling of common (local and global) vars [PR94134]

[Paul Koning via Gcc-patches]

Ok, thanks.  

paul

> On Mar 11, 2020, at 1:12 PM, Jakub Jelinek  wrote:
> 
> Hi!
> 
> As mentioned in the PR, the generic code decides to put the a variable into
> lcomm_section, which is a NOSWITCH section and thus the generic code doesn't
> switch into a particular section before using
> ASM_OUTPUT{_ALIGNED{,_DECL}_}_LOCAL, on many targets that results just in
> .lcomm (or for non-local .comm) directives which don't need a switch to some
> section, other targets put switch_to_section (bss_section) at the start of
> that macro.
> pdp11 doesn't do that (and doesn't have bss_section), and so emits the
> lcomm/comm variables in whatever section is current (it has only .text/.data
> and for DEC assembler rodata).
> 
> The following patch fixes that by putting it always into data section, and
> additionally avoids emitting an empty line in the assembly for the lcomm
> vars.
> 
> Tested on the testcase, I'm afraid I have no other way to test this target.
> Ok for trunk?
> 
> 2020-03-11  Jakub Jelinek  
> 
>   PR target/94134
>   * config/pdp11/pdp11.c (pdp11_asm_output_var): Call switch_to_section
>   at the start to switch to data section.  Don't print extra newline if
>   .globl directive has not been emitted.
> 
>   * gcc.c-torture/execute/pr94134.c: New test.
> 
> --- gcc/config/pdp11/pdp11.c.jj   2020-01-12 11:54:36.382413876 +0100
> +++ gcc/config/pdp11/pdp11.c  2020-03-11 15:44:43.37397 +0100
> @@ -743,6 +743,7 @@ void
> pdp11_asm_output_var (FILE *file, const char *name, int size,
> int align, bool global)
> {
> +  switch_to_section (data_section);
>   if (align > 8)
> fprintf (file, "\t.even\n");
>   if (TARGET_DEC_ASM)
> @@ -763,8 +764,8 @@ pdp11_asm_output_var (FILE *file, const
>   {
> fprintf (file, ".globl ");
> assemble_name (file, name);
> +   fprintf (file, "\n");
>   }
> -  fprintf (file, "\n");
>   assemble_name (file, name);
>   fputs (":", file);
>   ASM_OUTPUT_SKIP (file, size);
> --- gcc/testsuite/gcc.c-torture/execute/pr94134.c.jj  2020-03-11 
> 15:46:09.540710642 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr94134.c 2020-03-11 
> 15:40:18.538840663 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/94134 */
> +
> +static volatile int a = 0;
> +static volatile int b = 1;
> +
> +int
> +main ()
> +{
> +  a++;
> +  b++;
> +  if (a != 1 || b != 2)
> +__builtin_abort ();
> +  return 0;
> +}
> 
>   Jakub
> 

[PATCH] pdp11: Fix handling of common (local and global) vars [PR94134]

[Jakub Jelinek via Gcc-patches]

Hi!

As mentioned in the PR, the generic code decides to put the a variable into
lcomm_section, which is a NOSWITCH section and thus the generic code doesn't
switch into a particular section before using
ASM_OUTPUT{_ALIGNED{,_DECL}_}_LOCAL, on many targets that results just in
.lcomm (or for non-local .comm) directives which don't need a switch to some
section, other targets put switch_to_section (bss_section) at the start of
that macro.
pdp11 doesn't do that (and doesn't have bss_section), and so emits the
lcomm/comm variables in whatever section is current (it has only .text/.data
and for DEC assembler rodata).

The following patch fixes that by putting it always into data section, and
additionally avoids emitting an empty line in the assembly for the lcomm
vars.

Tested on the testcase, I'm afraid I have no other way to test this target.
Ok for trunk?

2020-03-11  Jakub Jelinek  

PR target/94134
* config/pdp11/pdp11.c (pdp11_asm_output_var): Call switch_to_section
at the start to switch to data section.  Don't print extra newline if
.globl directive has not been emitted.

* gcc.c-torture/execute/pr94134.c: New test.

--- gcc/config/pdp11/pdp11.c.jj 2020-01-12 11:54:36.382413876 +0100
+++ gcc/config/pdp11/pdp11.c2020-03-11 15:44:43.37397 +0100
@@ -743,6 +743,7 @@ void
 pdp11_asm_output_var (FILE *file, const char *name, int size,
  int align, bool global)
 {
+  switch_to_section (data_section);
   if (align > 8)
 fprintf (file, "\t.even\n");
   if (TARGET_DEC_ASM)
@@ -763,8 +764,8 @@ pdp11_asm_output_var (FILE *file, const
{
  fprintf (file, ".globl ");
  assemble_name (file, name);
+ fprintf (file, "\n");
}
-  fprintf (file, "\n");
   assemble_name (file, name);
   fputs (":", file);
   ASM_OUTPUT_SKIP (file, size);
--- gcc/testsuite/gcc.c-torture/execute/pr94134.c.jj2020-03-11 
15:46:09.540710642 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr94134.c   2020-03-11 
15:40:18.538840663 +0100
@@ -0,0 +1,14 @@
+/* PR target/94134 */
+
+static volatile int a = 0;
+static volatile int b = 1;
+
+int
+main ()
+{
+  a++;
+  b++;
+  if (a != 1 || b != 2)
+__builtin_abort ();
+  return 0;
+}

Jakub

[committed] RISC-V: Fix testsuite regression due to recent IRA changes.

[Kito Cheng]

After IRA changes, atomic version will use one more register, but
non-atomic still use 2 registers, however this testcase isn't testing for
atomic feature, so I decide change the testcase to always use COUNT++
to test.

ChangeLog

gcc/testsuite/

Kito Cheng  

* gcc.target/riscv/interrupt-2.c: Update testcase and expected output.
---
 gcc/testsuite/ChangeLog  | 4 
 gcc/testsuite/gcc.target/riscv/interrupt-2.c | 4 
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 11061adaf18..e2442fba35a 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-03-11  Kito Cheng  
+
+   * gcc.target/riscv/interrupt-2.c: Update testcase and expected output.
+
 2020-03-11  Richard Biener  
 
* gcc.dg/torture/20200311-1.c: New testcase.
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-2.c 
b/gcc/testsuite/gcc.target/riscv/interrupt-2.c
index 9559007e4ae..82e3fb24e81 100644
--- a/gcc/testsuite/gcc.target/riscv/interrupt-2.c
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-2.c
@@ -8,10 +8,6 @@ foo2 (void)
   INTERRUPT_FLAG = 0;
 
   extern volatile int COUNTER;
-#ifdef __riscv_atomic
-  __atomic_fetch_add (, 1, __ATOMIC_RELAXED);
-#else
   COUNTER++;
-#endif
 }
 /* { dg-final { scan-assembler-times "s\[wd\]\ta\[0-7\],\[0-9\]+\\(sp\\)" 2 } 
} */
-- 
2.25.1

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Martin Sebor via Gcc-patches]

On 3/9/20 6:08 PM, Jason Merrill wrote:

On 3/9/20 5:39 PM, Martin Sebor wrote:

On 3/9/20 1:40 PM, Jason Merrill wrote:

On 3/9/20 12:31 PM, Martin Sebor wrote:

On 2/28/20 1:24 PM, Jason Merrill wrote:

On 2/28/20 12:45 PM, Martin Sebor wrote:

On 2/28/20 9:58 AM, Jason Merrill wrote:

On 2/24/20 6:58 PM, Martin Sebor wrote:

-Wredundant-tags doesn't consider type declarations that are also
the first uses of the type, such as in 'void f (struct S);' and
issues false positives for those.  According to the reported that's
making it harder to use the warning to clean up LibreOffice.

The attached patch extends -Wredundant-tags to avoid these false
positives by relying on the same class_decl_loc_t::class2loc 
mapping
as -Wmismatched-tags.  The patch also somewhat improves the 
detection

of both issues in template declarations (though more work is still
needed there).



+ a new entry for it and return unless it's a declaration
+ involving a template that may need to be diagnosed by
+ -Wredundant-tags.  */
   *rdl = class_decl_loc_t (class_key, false, def_p);
-  return;
+  if (TREE_CODE (decl) != TEMPLATE_DECL)
+    return;


How can the first appearance of a class template be redundant?


I'm not sure I correctly understand the question.  The comment says
"involving a template" (i.e., not one of the first declaration of
a template).  The test case that corresponds to this test is:

   template  struct S7 { };
   struct S7 s7v;  // { dg-warning "\\\[-Wredundant-tags" }

where DECL is the TEPLATE_DECL of S7.

As I mentioned, more work is still needed to handle templates right
because some redundant tags are still not diagnosed.  For example:

   template  struct S7 { };
   template 
   using U = struct S7;   // missing warning


When we get here for an instance of a template, it doesn't make 
sense to treat it as a new type.


If decl is a template and type_decl is an instance of that 
template, do we want to (before the lookup) change type_decl to the 
template or the corresponding generic TYPE_DECL, which should 
already be in the table?


I'm struggling with how to do this.  Given type (a RECORD_TYPE) and
type_decl (a TEMPLATE_DECL) representing the use of a template, how
do I get the corresponding template (or its explicit or partial
specialization) in the three cases below?

   1) Instance of the primary:
  template  class A;
  struct A a;

   2) Instance of an explicit specialization:
  template  class B;
  template <> struct B;
  class B b;

   3) Instance of a partial specialization:
  template  class C;
  template  struct C;
  class C c;

By trial and (lots of) error I figured out that in both (1) and (2),
but not in (3), TYPE_MAIN_DECL (TYPE_TI_TEMPLATE (type)) returns
the template's type_decl.

Is there some function to call to get it in (3), or even better,
in all three cases?


I think you're looking for most_general_template.


I don't think that's quite what I'm looking for.  At least it doesn't
return the template or its specialization in all three cases above.


Ah, true, that function stops at specializations.  Oddly, I don't think 
there's currently a similar function that looks through them.  You could 
create one that does a simple loop through DECL_TI_TEMPLATE like 
is_specialization_of.


Thanks for the tip.  Even with that I'm having trouble with partial
specializations.  For example in:

  templatestruct S;
  template  class S;
  extern class  S s1;
  extern struct S s2;  // expect -Wmismatched-tags

how do I find the declaration of the partial specialization when given
the type in the extern declaration?  A loop in my find_template_for()
function (similar to is_specialization_of) only visits the implicit
specialization S (i.e., its own type) and the primary.

Martin




In (2) and (3) it won't distinguish between specializations of B or
C on different types.  In (2), the function returns the same result
for both:

   template <> struct B;
   template <> struct B;

In (3), it similarly returns the same result for both of

   template  struct C;
   template  struct C;

even though they are declarations of distinct types.



Jason

Re: [GCC][PATCH][AArch64] ACLE intrinsics for BFCVTN, BFCVTN2 (AArch64 AdvSIMD) and BFCVT (AArch64 FP)

[Richard Sandiford]

Vasee Vinayagamoorthy  writes:
> Hi,
>
> I think this commit causes a failure on aarch64-none-elf due to a DejaGnu
> typo in gcc.target/aarch64/advsimd-intrinsics/bfcvt-nosimd.c.
>
> { dg-final { check-function-bodies "**" "" "-O[^0]" } }
>
> I think the square brackets need to be escaped or use {-O[^0]}.

Gah, this is my fault.  I'd noticed that the line previously used
"-DCHECK_ASM", which meant that the test would never be performed
(because nothing ever adds -DCHECK_ASM for this test harness).
I should have noticed that during previous review cycles and
didn't want to force another review round over it, so I made
the change locally.  But then I compounded the mistake by messing
up the testing somehow, and also forgetting about the change when
doing the commit. :-(

Really sorry about that, and sorry especially to Delia for messing
up her patch.  It sounds from off-list discussion like you have
a fix in the works, so I'll defer to that.

Thanks,
Richard

>
> Regards
> Vasee
>
> Fri, Mar 06, 2020 at 09:54:07AM +, Richard Sandiford wrote:
>> Delia Burduv  writes:
>> > Hi,
>> >
>> > Here is the latest version of the  patch. That test should now work.
>> 
>> Thanks, pushed.
>> 
>> Richard

Re: [PATCH 2/2] [aarch64] Rework fpcr fpsr getter/setter builtins

[Szabolcs Nagy]

On 11/03/2020 13:53, Richard Earnshaw (lists) wrote:
> On 11/03/2020 13:50, Szabolcs Nagy wrote:
>> On 10/03/2020 09:55, Andrea Corallo wrote:
>>> Hi all,
>>>
>>> second and last patch of the two reworking FPCR and FPSR builtins.
>>>
>>> This rework __builtin_aarch64_set_fpcr (unsigned) and
>>> __builtin_aarch64_set_fpsr (unsigned) to emit a read-modify-sequences
>>> as:
>>>
>>>   mrs x1, fpsr
>>>   bfi x1, x0, 0, 32
>>>   msr fpsr, x1
>>>
>>> This in order to preserve the original high 32 bits of the system
>>> register.  Both FPSR and FPCR became 64bit regs with armv8.1.
>>
>> so the architectural fpsr and fpcr registers got
>> extended to 64bit from 32bit and currently the top
>> 32 bits are reserved 0.
>>
>> the floating-point environment has to fit in fenv_t
>> which is currently two unsigned ints on aarch64.
>>
>> so glibc cannot support 64bit registers without breaking
>> abi which means in c programs the fpsr, fpcr top bits
>> must be held 0 at all times when extern calls are made
>> that involves dynamic linking or calls into the libc
>> (otherwise fegetenv and fesetenv are not able to
>> reliably restore the default fp environment).
> 
> I disagree.  The top bits must be preserved, even if they are not zero. This 
> does potentially limit what those top bits might be used for, since
> they are beyond the ability to represent state in C-like programming 
> environments, but it doesn't mean they should be reset by a call to change
> the register.


ok preserving works for fesetenv.
(which internally uses the builtin)

it means the top bits are not part of fenv.
(we will have to change asm in glibc to deal with this)

since the top and bottom bits cannot be set
independently there are implications about
atomic updates, but they are not relevant
on linux where signal handlers save/restore
the fenv (not required by the standard so
theoretically treating the top and bottom bits
separately may cause issues on some systems).

> 
> 
> R.
> 
>>
>> i think trying to preserve the top bits is wrong
>> (it may be acceptable depending on what those bits
>> do, but it seems to me better to only allow 0 top bits).
>>
>> if the user changes the top bits the behaviour is
>> undefined if an extern c call is made with such setting.
>>
>> i think we have to change the builtins to ensure the
>> top bits of the used x reg is 0. we may or may not need
>> a new builtin to allow setting 64 bits of fpsr and fpcr:
>> currently non-zero top bits are unsupportable (but once
>> there is new meaning for the top 32bits then it may make
>> sense to break abi).
>>
>>
>>>
>>> Bootstrapped on aarch64-linux-gnu, does not introduce regressions.
>>>
>>> Regards
>>>
>>>    Andrea
>>>
>>> gcc/ChangeLog:
>>>
>>> 2020-??-??  Andrea Corallo  
>>>
>>> * config/aarch64/aarch64.md (insv_reg): Pattern renamed.
>>> (set_fpcr, set_fpsr): Pattern modified for read-modify-write
>>> sequence respecting high 32bit register content.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2020-??-??  Andrea Corallo  
>>>
>>> * gcc.target/aarch64/set_fpcr.c: New test.
>>> * gcc.target/aarch64/get_fpcr.c: New test.
>>> * gcc.target/aarch64/set_fpsr.c: New test.
>>> * gcc.target/aarch64/get_fpsr.c: New test.
>>>
>>
> 

Re: [PATCH] libstdc++: Add a test that takes the split_view of a non-forward range

[Jonathan Wakely via Gcc-patches]

On 11/03/20 11:46 -0400, Patrick Palka wrote:

This adds a tests that verifies taking the split_view of a non-forward range
works correctly.  Doing so revealed a typo in one of _OuterIter's constructors.


Oops!


It also revealed that the default constructor of
__gnu_test::test_range::iterator misbehaves, because by delegating to
Iter(nullptr, nullptr) we perform a null-pointer deref at runtime in
input_iterator_wrapper's constructor due to the ITERATOR_VERIFY check therein.


Oops for me this time.


Instead of delegating to this constructor it seems we can just inherit the
protected default constructor, which does not contain this ITERATOR_VERIFY
check.


Yep, that makes much more sense.

OK for master, thanks.

Re: [PATCH] Prevent IPA-SRA from creating calls to local comdats (PR 92676)

[Martin Jambor]

Hello,

ping.

Thanks,

Martin


On Mon, Dec 16 2019, Martin Jambor wrote:
> Hi,
>
> since r278669 (fix for PR ipa/91956), IPA-SRA makes sure that the clone
> it creates is put into the same same_comdat as the original cgraph_node,
> so that it can call private comdats (such as the ipa-split bits of a
> comdat that is private).
>
> However, that means that if there is non-comdat caller of a public
> comdat that is modified by IPA-SRA, it now finds itself calling a
> private comdat, which call graph verifier does not like (and for a
> reason, in theory it can disappear and since it is private it would not
> be available from other CUs).
>
> The patch fixes this by performing the fix for PR 91956 only when the
> node in question actually calls a local comdat and when it does, also
> making sure that no callers come from a different same_comdat (disabling
> IPA-SRA if both conditions are true), so that it plays by the rules in
> both modes, does not violate the private comdat calling rule and at the
> same time does not disable the transformation unnecessarily.
>
> The patch also fixes up the calls_comdat_local of callers of the
> modified node, despite that not triggering any known issues.  It has
> passed LTO-bootstrap and testing.  What do you think?
>
> Thanks,
>
> Martin
>
>
> 2019-12-16  Martin Jambor  
>
>   PR ipa/92676
>   * ipa-sra.c (struct caller_issues): New fields candidate and
>   call_from_outside_comdat.
>   (check_for_caller_issues): Check for calls from outsied of
>   candidate's same_comdat_group.
>   (check_all_callers_for_issues): Set up issues.candidate, check result
>   of the new check.
>   (process_isra_node_results): Set calls_comdat_local of callers if
>   appropriate.
> ---
>  gcc/ipa-sra.c | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
> index 421c0899e11..8612c8fc920 100644
> --- a/gcc/ipa-sra.c
> +++ b/gcc/ipa-sra.c
> @@ -2895,10 +2895,14 @@ ipa_sra_ipa_function_checks (cgraph_node *node)
>  
>  struct caller_issues
>  {
> +  /* The candidate being considered.  */
> +  cgraph_node *candidate;
>/* There is a thunk among callers.  */
>bool thunk;
>/* Call site with no available information.  */
>bool unknown_callsite;
> +  /* Call from outside the the candidate's comdat group.  */
> +  bool call_from_outside_comdat;
>/* There is a bit-aligned load into one of non-gimple-typed arguments. */
>bool bit_aligned_aggregate_argument;
>  };
> @@ -2920,6 +2924,13 @@ check_for_caller_issues (struct cgraph_node *node, 
> void *data)
>thunks.  */
> return true;
>   }
> +  if (issues->candidate->calls_comdat_local
> +   && issues->candidate->same_comdat_group
> +   && !issues->candidate->in_same_comdat_group_p (cs->caller))
> + {
> +   issues->call_from_outside_comdat = true;
> +   return true;
> + }
>  
>isra_call_summary *csum = call_sums->get (cs);
>if (!csum)
> @@ -2942,6 +2953,7 @@ check_all_callers_for_issues (cgraph_node *node)
>  {
>struct caller_issues issues;
>memset (, 0, sizeof (issues));
> +  issues.candidate = node;
>  
>node->call_for_symbol_and_aliases (check_for_caller_issues, , true);
>if (issues.unknown_callsite)
> @@ -2960,6 +2972,13 @@ check_all_callers_for_issues (cgraph_node *node)
>node->dump_name ());
>return true;
>  }
> +  if (issues.call_from_outside_comdat)
> +{
> +  if (dump_file)
> + fprintf (dump_file, "Function would become private comdat called "
> +  "outside of its comdat group.\n");
> +  return true;
> +}
>  
>if (issues.bit_aligned_aggregate_argument)
>  {
> @@ -3759,8 +3778,12 @@ process_isra_node_results (cgraph_node *node,
>  = node->create_virtual_clone (callers, NULL, new_adjustments, "isra",
> suffix_counter);
>suffix_counter++;
> -  if (node->same_comdat_group)
> -new_node->add_to_same_comdat_group (node);
> +  if (node->calls_comdat_local && node->same_comdat_group)
> +{
> +  new_node->add_to_same_comdat_group (node);
> +  for (cgraph_edge *cs = new_node->callers; cs; cs = cs->next_caller)
> + cs->caller->calls_comdat_local = true;
> +}
>new_node->calls_comdat_local = node->calls_comdat_local;
>  
>if (dump_file)
> -- 
> 2.24.0

[PATCH] libstdc++: Add a test that takes the split_view of a non-forward range

[Patrick Palka via Gcc-patches]

This adds a tests that verifies taking the split_view of a non-forward range
works correctly.  Doing so revealed a typo in one of _OuterIter's constructors.

It also revealed that the default constructor of
__gnu_test::test_range::iterator misbehaves, because by delegating to
Iter(nullptr, nullptr) we perform a null-pointer deref at runtime in
input_iterator_wrapper's constructor due to the ITERATOR_VERIFY check therein.
Instead of delegating to this constructor it seems we can just inherit the
protected default constructor, which does not contain this ITERATOR_VERIFY
check.

libstdc++-v3/ChangeLog:

* include/std/ranges (split_view::_OuterIter::_OuterIter): Typo fix,
'address' -> 'std::__addressof'.
* testsuite/std/ranges/adaptors/split.cc: Test taking the split_view of
a non-forward input_range.
* testsuite/util/testsuite_iterators.h (output_iterator_wrapper): Make
default constructor protected instead of deleted, like with
input_iterator_wrapper.
(test_range::iterator): Add comment explaining that this type is used
only when the underlying wrapper is input_iterator_wrapper or
output_iterator_wrapper.  Remove delegating defaulted constructor so
that the inherited default constructor is used instead.
---
 libstdc++-v3/include/std/ranges   |  2 +-
 .../testsuite/std/ranges/adaptors/split.cc| 20 +++
 .../testsuite/util/testsuite_iterators.h  | 14 -
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 23396947ac3..f4faec463e8 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -2786,7 +2786,7 @@ namespace views
 
  constexpr explicit
  _OuterIter(_Parent& __parent) requires (!forward_range<_Base>)
-   : _M_parent(address(__parent))
+   : _M_parent(std::__addressof(__parent))
  { }
 
  constexpr
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc 
b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
index abdbfb41d8b..fe895827fc5 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
@@ -27,6 +27,7 @@
 
 using __gnu_test::test_range;
 using __gnu_test::forward_iterator_wrapper;
+using __gnu_test::input_iterator_wrapper;
 
 namespace ranges = std::ranges;
 namespace views = std::ranges::views;
@@ -133,6 +134,24 @@ test07()
   static_assert( noexcept(iter_swap(b, b2)) );
 }
 
+void
+test08()
+{
+  char x[] = "the quick brown fox";
+  test_range rx(x, x+sizeof(x)-1);
+  auto v = rx | views::split(' ');
+  auto i = v.begin();
+  VERIFY( ranges::equal(*i, "the"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "quick"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "brown"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "fox"sv) );
+  ++i;
+  VERIFY( i == v.end() );
+}
+
 int
 main()
 {
@@ -143,4 +162,5 @@ main()
   test05();
   test06();
   test07();
+  test08();
 }
diff --git a/libstdc++-v3/testsuite/util/testsuite_iterators.h 
b/libstdc++-v3/testsuite/util/testsuite_iterators.h
index a915c02248b..5be47f47915 100644
--- a/libstdc++-v3/testsuite/util/testsuite_iterators.h
+++ b/libstdc++-v3/testsuite/util/testsuite_iterators.h
@@ -124,6 +124,11 @@ namespace __gnu_test
   struct output_iterator_wrapper
   : public std::iterator
   {
+  protected:
+output_iterator_wrapper() : ptr(0), SharedInfo(0)
+{ }
+
+  public:
 typedef OutputContainer ContainerType;
 T* ptr;
 ContainerType* SharedInfo;
@@ -135,8 +140,6 @@ namespace __gnu_test
 }
 
 #if __cplusplus >= 201103L
-output_iterator_wrapper() = delete;
-
 output_iterator_wrapper(const output_iterator_wrapper&) = default;
 
 output_iterator_wrapper&
@@ -706,13 +709,14 @@ namespace __gnu_test
   template class Iter>
 class test_range
 {
-  // Adds default constructor to Iter if needed
+  // Exposes the protected default constructor of Iter if needed.  This
+  // is needed only when Iter is input_iterator_wrapper or
+  // output_iterator_wrapper, because legacy forward iterators and beyond
+  // are already default constructible.
   struct iterator : Iter
   {
using Iter::Iter;
 
-   iterator() : Iter(nullptr, nullptr) { }
-
using Iter::operator++;
 
iterator& operator++() { Iter::operator++(); return *this; }
-- 
2.26.0.rc1

Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Marek Polacek via Gcc-patches]

On Wed, Mar 11, 2020 at 07:52:15AM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Mar 10, 2020 at 07:38:17PM -0400, Marek Polacek via Gcc-patches wrote:
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -6062,6 +6062,13 @@ reshape_init_array_1 (tree elt_type, tree max_index, 
> > reshape_iter *d,
> >else if (last_nonzero < nelts - 1)
> > nelts = last_nonzero + 1;
> >  
> > +  /* Sharing a stripped constructor can get in the way of
> > +overload resolution.  E.g., initializing a class from
> > +{{0}} might be invalid while initializing the same class
> > +from {{}} might be valid.  */
> > +  if (reuse)
> > +   new_init = unshare_constructor (new_init);
> > +
> >vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
> 
> Isn't it wasteful to first copy perhaps a large constructor (recursively)
> and then truncate it to very few elts (zero in this case)?
> So, perhaps doing instead:
>   if (reuse)
>   {
> vec *v = NULL;
> if (nelts)
>   vec_alloc (v, nelts);
> for (unsigned int i = 0; i < nelts; i++)
>   {
> constructor_elt elt = CONSTRUCTOR_ELT (new_init, i);
> if (TREE_CODE (elt.value) == CONSTRUCTOR)
>   elt.value = unshare_constructor (elt.value);
> v->quick_push (elt);
>   }
> new_init = build_constructor (TREE_TYPE (new_init), v);
>   }
>   else
>   vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
> ?

Large constructors aren't expected here.  From bootstrap + testsuite
measurements I can see that we have 8035 actual truncations, and 7884
of those are of constructors shorter than 10 elements.  There's one case
where we have a ctor of 567 elements and truncate it to 559 elements but
that seems pathological.  Is it worth it (genuine quiestion)?

Note there are 37077 attempts to truncate, of which 29042 truncate to
the same length.  Our ->truncate merely sets m_num, so it's cheap, but
I'm still a bit surprised we don't check 

  if (CONSTRUCTOR_NELTS (new_init) != nelts)

before calling vec_safe_truncate.

Marek

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT via Gcc-patches]

‐‐‐ Original Message ‐‐‐
On Sunday, February 16, 2020 7:06 PM, Segher Boessenkool 
 wrote:

> On Sat, Feb 15, 2020 at 05:22:09PM +, GT wrote:

> > I have not been able to configure protonmail for either git imap-send or 
> > send-email.
>
> Do you use git format-patch? You should, as the manual pages for both
> git imap-send and git send-email explain.
>
> > Will try pasting the .patch inline as plain text and see if that works.
>
> That doesn't work, unless you take extreme care. It is fine for showing
> small snippets, but it does not result in patches people can apply.
>

I have more patches I'd like to post and I have been unable to find an 
alternative to
sending them as binary attachments.

1. The only system I have access to is a Windows 10 machine. I am not allowed 
to modify
it in any way. So I can't install cygwin (or any other software), if having 
that would
have solved my issues.

2. I am creating the patches on a VM in one of the clouds IBM makes available 
for POWER
software development. To access the VM the only option I have thus far is to 
login using
Openstack from a browser. Then open a console in the interface. To get patches 
out of the
VM, I push them onto Github. Then download them onto the Windows PC to send as 
email
attachments.

3. On the VM git imap-send and send-email are not usable for me at the moment. 
Gmail now requires
2-factor authentication for them. And that in turn requires a cell phone. I 
can't say when
I will have a new phone.

Anyone have ideas on how to avoid the binary attachements?

Bert.

Re: ODR violation in ranges

[Nathan Sidwell]

On 3/11/20 11:23 AM, Jason Merrill wrote:
On Wed, Mar 11, 2020 at 11:05 AM Nathan Sidwell > wrote:


On 3/11/20 6:56 AM, Tam S. B. wrote:
 > IIUC using lambda in inline variable initializer is not ODR
violation. This is covered in CWG 2300 (
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1510r0.html#2300
).

ah, thanks for the pointer.  For lambdas in function scope we do
achieve
this (the mangling of the lambda uses the function as scope and a
function-specific index).

We do not do so for lambdas in the initializer of namespace-scope
inline
variables, which is I think the missing bit.  That's an ABI thing, I'll
ask over there.


Looks like the condition on the call to start_lambda_scope in 
cp_parser_init_declarator just needs to be adjusted to handle 
non-template inline variables.


thanks!

The ABI does describe how to mangle these (it focusses on data-members, 
but AFAICT works for namespace-scope vars too).  We don't do any of that 
though :(




The ranges header we'll die horribly if we ever emit non-inline code to
evaluate the lambda at runtime.  We'll give it a name that depends on
the order of (namespace-scope?) lambdas. :(

inline constexpr auto foo = [] (int i) {return i * 2;};
int bob (int i)
{ return foo (i); }

compile with -fno-inline and observe '_ZNKUliE_clEi'

sigh,

nathan
 >
 > 
 > From: Libstdc++ mailto:libstdc%2b%2b-boun...@gcc.gnu.org>> on behalf of Jonathan
Wakely via Libstdc++ mailto:libstdc%2b...@gcc.gnu.org>>
 > Sent: Wednesday, March 11, 2020 10:26
 > To: Nathan Sidwell
 > Cc: libstd...@gcc.gnu.org ; GCC
Patches
 > Subject: Re: ODR violation in ranges
 >
 > On 11/03/20 06:08 -0400, Nathan Sidwell wrote:
 >> Jonathan,
 >> the ranges header contains code like:
 >>     inline constexpr __adaptor::_RangeAdaptorClosure all
 >>       = []  (_Range&& __r)
 >>       {
 >> if constexpr (view>)
 >>    return std::forward<_Range>(__r);
 >> else if constexpr (requires {
ref_view{std::forward<_Range>(__r)}; })
 >>    return ref_view{std::forward<_Range>(__r)};
 >> else
 >>    return subrange{std::forward<_Range>(__r)};
 >>       };
 >>
 >> (line 1236)
 >>
 >> When you strip away all the templateyness, you have:
 >>
 >>
 >> inline constexpr auto all = [] () {};
 >>
 >>
 >> That's an ODR violation -- the initializers in different TUs are not
 >> the same!
 >>
 >> As you can guess, I can't turn this into a header unit (well, I can,
 >> but merging duplicates complains at you)
 >
 > CC libstdc++@ and Patrick.
 >
 > I did wonder if using lambdas for those global variables would be OK.
 >
 > I think we'll need a new class template for each view adaptor, rather
 > than using the _RangeAdaptorClosure to hold a closure.
 >
 > Patrick, can you look into that please?
 >


-- 
Nathan Sidwell





--
Nathan Sidwell

Re: ODR violation in ranges

[Jason Merrill via Gcc-patches]

On Wed, Mar 11, 2020 at 11:05 AM Nathan Sidwell  wrote:

> On 3/11/20 6:56 AM, Tam S. B. wrote:
> > IIUC using lambda in inline variable initializer is not ODR violation.
> This is covered in CWG 2300 (
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1510r0.html#2300
> ).
>
> ah, thanks for the pointer.  For lambdas in function scope we do achieve
> this (the mangling of the lambda uses the function as scope and a
> function-specific index).
>
> We do not do so for lambdas in the initializer of namespace-scope inline
> variables, which is I think the missing bit.  That's an ABI thing, I'll
> ask over there.
>

Looks like the condition on the call to start_lambda_scope in
cp_parser_init_declarator just needs to be adjusted to handle non-template
inline variables.


> The ranges header we'll die horribly if we ever emit non-inline code to
> evaluate the lambda at runtime.  We'll give it a name that depends on
> the order of (namespace-scope?) lambdas. :(
>
> inline constexpr auto foo = [] (int i) {return i * 2;};
> int bob (int i)
> { return foo (i); }
>
> compile with -fno-inline and observe '_ZNKUliE_clEi'
>
> sigh,
>
> nathan
> >
> > 
> > From: Libstdc++  on behalf of Jonathan
> Wakely via Libstdc++ 
> > Sent: Wednesday, March 11, 2020 10:26
> > To: Nathan Sidwell
> > Cc: libstd...@gcc.gnu.org; GCC Patches
> > Subject: Re: ODR violation in ranges
> >
> > On 11/03/20 06:08 -0400, Nathan Sidwell wrote:
> >> Jonathan,
> >> the ranges header contains code like:
> >> inline constexpr __adaptor::_RangeAdaptorClosure all
> >>   = []  (_Range&& __r)
> >>   {
> >> if constexpr (view>)
> >>return std::forward<_Range>(__r);
> >> else if constexpr (requires { ref_view{std::forward<_Range>(__r)}; })
> >>return ref_view{std::forward<_Range>(__r)};
> >> else
> >>return subrange{std::forward<_Range>(__r)};
> >>   };
> >>
> >> (line 1236)
> >>
> >> When you strip away all the templateyness, you have:
> >>
> >>
> >> inline constexpr auto all = [] () {};
> >>
> >>
> >> That's an ODR violation -- the initializers in different TUs are not
> >> the same!
> >>
> >> As you can guess, I can't turn this into a header unit (well, I can,
> >> but merging duplicates complains at you)
> >
> > CC libstdc++@ and Patrick.
> >
> > I did wonder if using lambdas for those global variables would be OK.
> >
> > I think we'll need a new class template for each view adaptor, rather
> > than using the _RangeAdaptorClosure to hold a closure.
> >
> > Patrick, can you look into that please?
> >
>
>
> --
> Nathan Sidwell
>
>

Re: ODR violation in ranges

[Nathan Sidwell]

On 3/11/20 6:56 AM, Tam S. B. wrote:

IIUC using lambda in inline variable initializer is not ODR violation. This is 
covered in CWG 2300 ( 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1510r0.html#2300 ).


ah, thanks for the pointer.  For lambdas in function scope we do achieve 
this (the mangling of the lambda uses the function as scope and a 
function-specific index).


We do not do so for lambdas in the initializer of namespace-scope inline 
variables, which is I think the missing bit.  That's an ABI thing, I'll 
ask over there.


The ranges header we'll die horribly if we ever emit non-inline code to 
evaluate the lambda at runtime.  We'll give it a name that depends on 
the order of (namespace-scope?) lambdas. :(


inline constexpr auto foo = [] (int i) {return i * 2;};
int bob (int i)
{ return foo (i); }

compile with -fno-inline and observe '_ZNKUliE_clEi'

sigh,

nathan



From: Libstdc++  on behalf of Jonathan Wakely via 
Libstdc++ 
Sent: Wednesday, March 11, 2020 10:26
To: Nathan Sidwell
Cc: libstd...@gcc.gnu.org; GCC Patches
Subject: Re: ODR violation in ranges

On 11/03/20 06:08 -0400, Nathan Sidwell wrote:

Jonathan,
the ranges header contains code like:
inline constexpr __adaptor::_RangeAdaptorClosure all
  = []  (_Range&& __r)
  {
if constexpr (view>)
   return std::forward<_Range>(__r);
else if constexpr (requires { ref_view{std::forward<_Range>(__r)}; })
   return ref_view{std::forward<_Range>(__r)};
else
   return subrange{std::forward<_Range>(__r)};
  };

(line 1236)

When you strip away all the templateyness, you have:


inline constexpr auto all = [] () {};


That's an ODR violation -- the initializers in different TUs are not
the same!

As you can guess, I can't turn this into a header unit (well, I can,
but merging duplicates complains at you)


CC libstdc++@ and Patrick.

I did wonder if using lambdas for those global variables would be OK.

I think we'll need a new class template for each view adaptor, rather
than using the _RangeAdaptorClosure to hold a closure.

Patrick, can you look into that please?




--
Nathan Sidwell

[PATCH] fold-undefined-pointer-offsetting

[Richard Biener]

This avoids breaking the old broken pointer offsetting via
(T)(ptr - ((T)0)->x) which should have used offsetof.  Breakage
was exposed by the introduction of POINTER_DIFF_EXPR and making
PTA not considering that producing a pointer.  The mitigation
for simple cases is to canonicalize

  _2 = _1 - 8B;
  o_9 = (struct obj *) _2;

to

  o_9 = [_1 + -8B];

eliding one statement and the offending pointer subtraction.

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

Richard.

2020-03-11  Richard Biener  

* match.pd ((T *)(ptr - ptr-cst) -> [ptr + -ptr-cst]):
New pattern.

* gcc.dg/torture/20200311-1.c: New testcase.
---
 gcc/match.pd  |  9 +
 gcc/testsuite/gcc.dg/torture/20200311-1.c | 26 ++
 2 files changed, 35 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/torture/20200311-1.c

diff --git a/gcc/match.pd b/gcc/match.pd
index 19df0c404a4..9cb37740f1e 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1864,6 +1864,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (ptr_difference_const (@0, @1, ))
 { build_int_cst_type (type, diff); }
 
+/* Canonicalize (T *)(ptr - ptr-cst) to [ptr + -ptr-cst].  */
+(simplify
+ (convert (pointer_diff @0 INTEGER_CST@1))
+ (if (POINTER_TYPE_P (type))
+  { build_fold_addr_expr_with_type
+  (build2 (MEM_REF, char_type_node, @0,
+  wide_int_to_tree (ptr_type_node, wi::neg (wi::to_wide (@1,
+  type); }))
+
 /* If arg0 is derived from the address of an object or function, we may
be able to fold this expression using the object or function's
alignment.  */
diff --git a/gcc/testsuite/gcc.dg/torture/20200311-1.c 
b/gcc/testsuite/gcc.dg/torture/20200311-1.c
new file mode 100644
index 000..ac82b6b7a0b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/20200311-1.c
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+
+struct list { struct list *n; };
+
+struct obj {
+int n;
+struct list l;
+} _o;
+
+struct list _l = { .n = &_o.l };
+
+int main(int argc, char *argv[])
+{
+  struct obj *o = &_o;
+  _o.l.n = &_l;
+  while (>l != &_l)
+/* Note the following is invoking undefined behavior but in
+   this kind of "obvious" cases we don't want to break things
+   unnecessarily and thus we avoid analyzing o as pointing
+   to nothing via the undefined pointer subtraction.  Instead
+   we canonicalize the pointer subtraction followed by the
+   pointer conversion to pointer offsetting.  */
+o = ((struct obj *)((const char *)(o->l.n)
+   - (const char *)&((struct obj *)0)->l));
+  return 0;
+}
-- 
2.16.4

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Alexander Monakov via Gcc-patches]

On Wed, 11 Mar 2020, Martin Liška wrote:

> > Is there a comprehensive list of plugins out in the wild using the LD
> > plugin API?
> 
> I know only about:
> $ ls /usr/lib/bfd-plugins
> liblto_plugin.so.0.0.0  LLVMgold.so
> 
> and I know about Alexander Monakov (some dead code elimination plug-in).

I don't recall seeing any other plugin when working on our "debloating"
stuff, but of course I suppose people could have used the plugin API
for experimentation.  I don't think a comprehensive list could exist.

> > Note we also have to bring in gold folks (not sure if lld also
> > implements the same plugin API)
> 
> I don't see how can they be affected?

As discussed downthread the other linkers could freely ignore the
new callback, but fwiw LLD has no plans to implement the plugin-api.h
interface: https://bugs.llvm.org/show_bug.cgi?id=42446

Alexander

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Martin Liška]

On 3/11/20 1:51 PM, Richard Biener wrote:

Splitting an existing field isn't hackish IMHO.  I guess even
explicitely changing it
to one short and two char fields would be OK.


Good, I'm doing that in the updated version of that patch.
H.J. is right now working on the corresponding binutils counterpart.

Thoughts?
Martin
>From 8a2e73edb53f26a1b6a543e2e2027b2661a48f94 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Fri, 6 Mar 2020 18:09:35 +0100
Subject: [PATCH] API extension for binutils (type of symbols).

---
 gcc/lto-streamer-out.c  | 12 +
 include/lto-symtab.h| 12 +
 include/plugin-api.h| 30 -
 lto-plugin/lto-plugin.c | 99 +
 4 files changed, 142 insertions(+), 11 deletions(-)

diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index cea5e71cffb..b212be35b3d 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "print-tree.h"
 #include "tree-dfa.h"
 #include "file-prefix-map.h" /* remap_debug_filename()  */
+#include "output.h"
 
 
 static void lto_write_tree (struct output_block*, tree, bool);
@@ -2773,6 +2774,17 @@ write_symbol (struct streamer_tree_cache_d *cache,
   lto_write_data (, 1);
   c = (unsigned char) visibility;
   lto_write_data (, 1);
+  c = ((unsigned char) TREE_CODE (t) == VAR_DECL
+   ? GCCST_VARIABLE : GCCST_FUNCTION);
+  lto_write_data (, 1);
+  unsigned char section_flags = 0;
+  if (TREE_CODE (t) == VAR_DECL)
+{
+  section *s = get_variable_section (t, false);
+  if (s->common.flags & SECTION_BSS)
+	section_flags |= GCCSSS_BSS;
+}
+  lto_write_data (_flags, 1);
   lto_write_data (, 8);
   lto_write_data (_num, 4);
 }
diff --git a/include/lto-symtab.h b/include/lto-symtab.h
index 0ce0de10121..47f0ff27df8 100644
--- a/include/lto-symtab.h
+++ b/include/lto-symtab.h
@@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
 GCCPV_HIDDEN
   };
 
+enum gcc_plugin_symbol_type
+{
+  GCCST_UNKNOWN,
+  GCCST_FUNCTION,
+  GCCST_VARIABLE,
+};
+
+enum gcc_plugin_symbol_section_flags
+{
+  GCCSSS_BSS = 1
+};
+
 #endif /* GCC_LTO_SYMTAB_H  */
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 09e1202df07..804684449cf 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -87,7 +87,17 @@ struct ld_plugin_symbol
 {
   char *name;
   char *version;
-  int def;
+#ifdef __BIG_ENDIAN__
+  char unused;
+  char section_flags;
+  char symbol_type;
+  char def;
+#else
+  char def;
+  char symbol_type;
+  char section_flags;
+  char unused;
+#endif
   int visibility;
   uint64_t size;
   char *comdat_key;
@@ -123,6 +133,20 @@ enum ld_plugin_symbol_visibility
   LDPV_HIDDEN
 };
 
+/* The type of the symbol.  */
+
+enum ld_plugin_symbol_type
+{
+  LDST_UNKNOWN,
+  LDST_FUNCTION,
+  LDST_VARIABLE,
+};
+
+enum ld_plugin_symbol_section_flags
+{
+  LDSSS_BSS = 1
+};
+
 /* How a symbol is resolved.  */
 
 enum ld_plugin_symbol_resolution
@@ -431,7 +455,9 @@ enum ld_plugin_tag
   LDPT_GET_INPUT_SECTION_ALIGNMENT = 29,
   LDPT_GET_INPUT_SECTION_SIZE = 30,
   LDPT_REGISTER_NEW_INPUT_HOOK = 31,
-  LDPT_GET_WRAP_SYMBOLS = 32
+  LDPT_GET_WRAP_SYMBOLS = 32,
+  LDPT_ADD_SYMBOLS_V2 = 33,
+  LDPT_GET_SYMBOLS_V4 = 34,
 };
 
 /* The plugin transfer vector.  */
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index c307fc871bf..bce9f183049 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -90,6 +90,8 @@ along with this program; see the file COPYING3.  If not see
 
 #define LTO_SECTION_PREFIX	".gnu.lto_.symtab"
 #define LTO_SECTION_PREFIX_LEN	(sizeof (LTO_SECTION_PREFIX) - 1)
+#define LTO_LTO_PREFIX		".gnu.lto_.lto"
+#define LTO_LTO_PREFIX_LEN	(sizeof (LTO_LTO_PREFIX) - 1)
 #define OFFLOAD_SECTION		".gnu.offload_lto_.opts"
 #define OFFLOAD_SECTION_LEN	(sizeof (OFFLOAD_SECTION) - 1)
 
@@ -154,12 +156,12 @@ enum symbol_style
 static char *arguments_file_name;
 static ld_plugin_register_claim_file register_claim_file;
 static ld_plugin_register_all_symbols_read register_all_symbols_read;
-static ld_plugin_get_symbols get_symbols, get_symbols_v2;
+static ld_plugin_get_symbols get_symbols, get_symbols_v2, get_symbols_v4;
 static ld_plugin_register_cleanup register_cleanup;
 static ld_plugin_add_input_file add_input_file;
 static ld_plugin_add_input_library add_input_library;
 static ld_plugin_message message;
-static ld_plugin_add_symbols add_symbols;
+static ld_plugin_add_symbols add_symbols, add_symbols_v2;
 
 static struct plugin_file_info *claimed_files = NULL;
 static unsigned int num_claimed_files = 0;
@@ -221,6 +223,20 @@ check_1 (int gate, enum ld_plugin_level level, const char *text)
 }
 }
 
+/* Structure that represents LTO ELF section with information
+   about the format.  */
+
+struct lto_section
+ {
+   int16_t major_version;
+   int16_t minor_version;
+   unsigned char slim_object: 1;
+   unsigned char compression: 4;
+   int32_t reserved0: 27;
+};
+
+struct 

Re: [PATCH 2/2] [aarch64] Rework fpcr fpsr getter/setter builtins

[Richard Earnshaw (lists)]

On 11/03/2020 13:50, Szabolcs Nagy wrote:

On 10/03/2020 09:55, Andrea Corallo wrote:

Hi all,

second and last patch of the two reworking FPCR and FPSR builtins.

This rework __builtin_aarch64_set_fpcr (unsigned) and
__builtin_aarch64_set_fpsr (unsigned) to emit a read-modify-sequences
as:

  mrs x1, fpsr
  bfi x1, x0, 0, 32
  msr fpsr, x1

This in order to preserve the original high 32 bits of the system
register.  Both FPSR and FPCR became 64bit regs with armv8.1.


so the architectural fpsr and fpcr registers got
extended to 64bit from 32bit and currently the top
32 bits are reserved 0.

the floating-point environment has to fit in fenv_t
which is currently two unsigned ints on aarch64.

so glibc cannot support 64bit registers without breaking
abi which means in c programs the fpsr, fpcr top bits
must be held 0 at all times when extern calls are made
that involves dynamic linking or calls into the libc
(otherwise fegetenv and fesetenv are not able to
reliably restore the default fp environment).


I disagree.  The top bits must be preserved, even if they are not zero. 
This does potentially limit what those top bits might be used for, since 
they are beyond the ability to represent state in C-like programming 
environments, but it doesn't mean they should be reset by a call to 
change the register.



R.



i think trying to preserve the top bits is wrong
(it may be acceptable depending on what those bits
do, but it seems to me better to only allow 0 top bits).

if the user changes the top bits the behaviour is
undefined if an extern c call is made with such setting.

i think we have to change the builtins to ensure the
top bits of the used x reg is 0. we may or may not need
a new builtin to allow setting 64 bits of fpsr and fpcr:
currently non-zero top bits are unsupportable (but once
there is new meaning for the top 32bits then it may make
sense to break abi).




Bootstrapped on aarch64-linux-gnu, does not introduce regressions.

Regards

   Andrea

gcc/ChangeLog:

2020-??-??  Andrea Corallo  

* config/aarch64/aarch64.md (insv_reg): Pattern renamed.
(set_fpcr, set_fpsr): Pattern modified for read-modify-write
sequence respecting high 32bit register content.

gcc/testsuite/ChangeLog:

2020-??-??  Andrea Corallo  

* gcc.target/aarch64/set_fpcr.c: New test.
* gcc.target/aarch64/get_fpcr.c: New test.
* gcc.target/aarch64/set_fpsr.c: New test.
* gcc.target/aarch64/get_fpsr.c: New test.

Re: [PATCH 2/2] [aarch64] Rework fpcr fpsr getter/setter builtins

[Szabolcs Nagy]

On 10/03/2020 09:55, Andrea Corallo wrote:
> Hi all,
> 
> second and last patch of the two reworking FPCR and FPSR builtins.
> 
> This rework __builtin_aarch64_set_fpcr (unsigned) and
> __builtin_aarch64_set_fpsr (unsigned) to emit a read-modify-sequences
> as:
> 
>  mrs x1, fpsr
>  bfi x1, x0, 0, 32
>  msr fpsr, x1
> 
> This in order to preserve the original high 32 bits of the system
> register.  Both FPSR and FPCR became 64bit regs with armv8.1.

so the architectural fpsr and fpcr registers got
extended to 64bit from 32bit and currently the top
32 bits are reserved 0.

the floating-point environment has to fit in fenv_t
which is currently two unsigned ints on aarch64.

so glibc cannot support 64bit registers without breaking
abi which means in c programs the fpsr, fpcr top bits
must be held 0 at all times when extern calls are made
that involves dynamic linking or calls into the libc
(otherwise fegetenv and fesetenv are not able to
reliably restore the default fp environment).

i think trying to preserve the top bits is wrong
(it may be acceptable depending on what those bits
do, but it seems to me better to only allow 0 top bits).

if the user changes the top bits the behaviour is
undefined if an extern c call is made with such setting.

i think we have to change the builtins to ensure the
top bits of the used x reg is 0. we may or may not need
a new builtin to allow setting 64 bits of fpsr and fpcr:
currently non-zero top bits are unsupportable (but once
there is new meaning for the top 32bits then it may make
sense to break abi).


> 
> Bootstrapped on aarch64-linux-gnu, does not introduce regressions.
> 
> Regards
> 
>   Andrea
> 
> gcc/ChangeLog:
> 
> 2020-??-??  Andrea Corallo  
> 
>   * config/aarch64/aarch64.md (insv_reg): Pattern renamed.
>   (set_fpcr, set_fpsr): Pattern modified for read-modify-write
>   sequence respecting high 32bit register content.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-??-??  Andrea Corallo  
> 
>   * gcc.target/aarch64/set_fpcr.c: New test.
>   * gcc.target/aarch64/get_fpcr.c: New test.
>   * gcc.target/aarch64/set_fpsr.c: New test.
>   * gcc.target/aarch64/get_fpsr.c: New test.
> 

RE: Remove redundant zero extend

[Nidal Faour via Gcc-patches]

Attaching the correct patch. 

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 9864e4344d2..c3739b2f331 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public 
License
 along with GCC; see the file COPYING3.  If not see
 .  */
 
+#include 
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
@@ -73,6 +74,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-address.h"
 #include "output.h"
 #include "builtins.h"
+#include "tree-vrp.h"
+
 
 /* Some systems use __main in a way incompatible with its use in gcc, in these
cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
@@ -3640,6 +3643,107 @@ expand_clobber (tree lhs)
 }
 }
 
+/* Print the ranges of the operand and save it to the dump file
+   used for debug purposes only.  */
+
+void
+print_range (tree operand, wide_int min, wide_int max)
+{
+  pretty_printer buffer;
+  pp_needs_newline () = false;
+  buffer.buffer->stream = dump_file;
+  fprintf (dump_file, "Range for lhs: [");
+  pp_wide_int (, min, TYPE_SIGN (TREE_TYPE (operand)));
+  pp_printf (, ", ");
+  pp_wide_int (, max, TYPE_SIGN (TREE_TYPE (operand)));
+  pp_printf (, "]");
+  pp_flush ();
+  fprintf (dump_file, "\n");
+}
+
+
+
+/* Check that REG and SUBREG modes match between src and dest,
+   and that there is a cast to be removed.  */
+
+bool
+cast_to_remove_p (rtx src, rtx dest, machine_mode from
+   , machine_mode to)
+{
+  if (GET_CODE (src) != SUBREG
+  || GET_CODE (dest) != SUBREG
+  || GET_CODE (SUBREG_REG (src)) != REG
+  || GET_CODE (SUBREG_REG (dest)) != REG
+  || GET_MODE (src) != GET_MODE (dest)
+  || GET_MODE (SUBREG_REG (src)) != GET_MODE (SUBREG_REG (dest))
+  || GET_MODE (src) != to
+  || GET_MODE (SUBREG_REG (src)) != from)
+return false;
+  return true;
+}
+
+bool
+remove_cast_p (rtx temp, rtx target, gassign *assign_stmt)
+{
+  enum gimple_code code;
+  value_range_kind lhs_range, rhs_range;
+  unsigned int ops_num;
+  wide_int lhs_min, lhs_max, rhs_min, rhs_max;
+  if (gimple_assign_cast_p (assign_stmt))
+  {
+code = gimple_code (assign_stmt);
+ops_num = gimple_num_ops (assign_stmt);
+if (code == GIMPLE_ASSIGN && ops_num < 3)
+{
+  tree lhs = gimple_assign_lhs (assign_stmt);
+  if (POINTER_TYPE_P (TREE_TYPE (lhs)))
+   return false;
+  lhs_range = get_range_info (lhs, _min, _max);
+  if (dump_file && lhs_range == VR_RANGE)
+   print_range (lhs, lhs_min, lhs_max);
+
+  tree rhs = gimple_assign_rhs1 (assign_stmt);
+  if (POINTER_TYPE_P (TREE_TYPE (rhs)))
+   return false;
+  rhs_range = get_range_info (rhs, _min, _max);
+  if (rhs_range == VR_RANGE)
+  {
+   unsigned int rhs_precision
+= std::max (wi::min_precision (rhs_max, TYPE_SIGN (TREE_TYPE (rhs))),
+   wi::min_precision (rhs_min, TYPE_SIGN (TREE_TYPE (rhs;
+   unsigned int lhs_precision = TYPE_PRECISION (TREE_TYPE (lhs));
+   if (lhs_precision > rhs_precision)
+   {
+
+if (dump_file)
+{
+  print_range (rhs, rhs_min, rhs_max);
+  if (lhs_precision > rhs_precision)
+   fprintf (dump_file,
+   "EXPAND-CAST: This casting can be optimized\n");
+}
+
+return cast_to_remove_p (temp,
+target,
+TYPE_MODE (TREE_TYPE (rhs)),
+TYPE_MODE (TREE_TYPE (lhs)));
+   }
+   else
+if (dump_file)
+  fprintf (dump_file, "EXPAND-CAST: lhs_precision < rhs_precision\n");
+  }
+  else
+   if (dump_file)
+ fprintf (dump_file, "EXPAND-CAST: Range is not VR_RANGE\n");
+}
+else
+  if (dump_file)
+   fprintf (dump_file, "EXPAND-CAST: Got more than 2 ops \n");
+  }
+  return false;
+}
+
+
 /* A subroutine of expand_gimple_stmt, expanding one gimple statement
STMT that doesn't require special handling for outgoing edges.  That
is no tailcalls and no GIMPLE_COND.  */
@@ -3801,6 +3905,11 @@ expand_gimple_stmt_1 (gimple *stmt)
temp = convert_modes (GET_MODE (SUBREG_REG (target)),
  GET_MODE (target), temp, unsignedp);
  }
+   if (targetm.remove_redundant_cast)
+ if (remove_cast_p (temp,
+   target,
+   assign_stmt))
+   temp = SUBREG_REG (temp);
 
convert_move (SUBREG_REG (target), temp, unsignedp);
  }
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 94b5ac01762..504b1abf85e 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -5226,6 +5226,9 @@ riscv_hard_regno_rename_ok (unsigned from_regno 
ATTRIBUTE_UNUSED,
 #undef TARGET_MACHINE_DEPENDENT_REORG
 

[C/C++, OpenACC] Reject vars of different scope in acc declare (PR94120)

[Tobias Burnus]

Fortran patch: https://gcc.gnu.org/pipermail/gcc-patches/current/541774.html

Like for Fortran, it also fixes some other issues – here for C++
related to namespaces. (For class, see PR c++/94140.)

Test case of the PR yields an ICE in the middle end and the
namespace tests an ICE in cc1plus. Additionally, invalid code
is not diagnosed.

The OpenACC spec has under "Declare Directive" has the following restriction:

"A declare directive must be in the same scope
 as the declaration of any var that appears in
 the data clauses of the directive."

("A declare directive is used […] following a variable
  declaration in C or C++".)

NOTE for C++: This patch assumes that variables in a namespace
are handled in the same way as those which are at
global (namespace) scope; however, the OpenACC specification's
wording currently is "In C or C++ global scope, only …".
Hence, one can argue about this part of the patch; but as
it fixes an ICE and is a very sensible extension – the other
option is to reject it – I believe it is fine.
(On the OpenACC side, this is now Issue 288.)

OK for the trunk?

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
[C/C++, OpenACC] Reject vars of different scope in acc declare (PR94120)

2020-10-11  Tobias Burnus  

	gcc/c/
	PR middle-end/94120
	* c-decl.c (c_check_oacc_same_scope): New function.
	* c-tree.h (c_check_oacc_same_scope): Declare it.
	* c-parser.c (c_parser_oacc_declare): Add check that variables
	are declared in the same scope as the directive. Fix handling
	of namespace vars.

	gcc/c/
	PR middle-end/94120
	* paser.c (cp_parser_oacc_declare): Add check that variables
are declared in the same scope as the directive.

	gcc/testsuite/
	PR middle-end/94120
	* c-c++-common/goacc/declare-pr94120.c: New.
	* g++.dg/declare-pr94120.C: New.

	libgomp/testsuite/
	PR middle-end/94120
	* libgomp.oacc-c++/declare-pr94120.C: New.
	

 gcc/c/c-decl.c |  8 +++
 gcc/c/c-parser.c   |  9 
 gcc/c/c-tree.h |  1 +
 gcc/cp/parser.c| 21 +++-
 gcc/testsuite/c-c++-common/goacc/declare-pr94120.c | 23 +
 gcc/testsuite/g++.dg/declare-pr94120.C | 30 
 .../testsuite/libgomp.oacc-c++/declare-pr94120.C   | 57 ++
 7 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index c819fd0d0d5..eda95d664de 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -12016,4 +12016,12 @@ c_check_omp_declare_reduction_r (tree *tp, int *, void *data)
   return NULL_TREE;
 }
 
+
+bool
+c_check_oacc_same_scope (tree decl)
+{
+  struct c_binding *b = I_SYMBOL_BINDING (DECL_NAME (decl));
+  return b != NULL && B_IN_CURRENT_SCOPE (b);
+}
+
 #include "gt-c-c-decl.h"
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1e8f2f7108d..63e8ab0ad17 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -16573,6 +16573,15 @@ c_parser_oacc_declare (c_parser *parser)
 	  break;
 	}
 
+  if (!c_check_oacc_same_scope (decl))
+	{
+	  error_at (loc,
+		"%qD must be a variable declared in the same scope as "
+		"%<#pragma acc declare%>", decl);
+	  error = true;
+	  continue;
+	}
+
   if (lookup_attribute ("omp declare target", DECL_ATTRIBUTES (decl))
 	  || lookup_attribute ("omp declare target link",
 			   DECL_ATTRIBUTES (decl)))
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 71229927cb6..6d578705d77 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -789,6 +789,7 @@ extern tree c_omp_reduction_id (enum tree_code, tree);
 extern tree c_omp_reduction_decl (tree);
 extern tree c_omp_reduction_lookup (tree, tree);
 extern tree c_check_omp_declare_reduction_r (tree *, int *, void *);
+extern bool c_check_oacc_same_scope (tree);
 extern void c_pushtag (location_t, tree, tree);
 extern void c_bind (location_t, tree, bool);
 extern bool tag_exists_p (enum tree_code, tree);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 24f71671469..8f09eb0d375 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -40722,6 +40722,7 @@ cp_parser_oacc_declare (cp_parser *parser, cp_token *pragma_tok)
 {
   tree clauses, stmt;
   bool error = false;
+  bool found_in_scope = global_bindings_p ();
 
   clauses = cp_parser_oacc_all_clauses (parser, OACC_DECLARE_CLAUSE_MASK,
 	"#pragma acc declare", pragma_tok, true);
@@ -40794,6 +40795,22 @@ cp_parser_oacc_declare (cp_parser *parser, cp_token *pragma_tok)
 	  break;
 	}
 
+  if (!found_in_scope)
+	for (tree d = current_binding_level->names; d; d = TREE_CHAIN (d))
+	  if (d == decl)
+	{
+	  found_in_scope = true;
+	  break;
+	}
+  if (!found_in_scope)
+	{
+	  error_at (loc,
+		"%qD must be a variable declared in the same scope as "
+		"%<#pragma acc declare%>", decl);
+	  

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Martin Liška]

On 3/11/20 2:24 PM, H.J. Lu wrote:

ld-new don't have to use the new interface since it isn't needed.


Yeah. We talk about nm that should utilize the new API, right?

Martin

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[H.J. Lu via Gcc-patches]

On Wed, Mar 11, 2020 at 6:09 AM Martin Liška  wrote:
>
> On 3/11/20 1:51 PM, Richard Biener wrote:
> > I'm not sure I understand the versioning, we should aim at something where
> > an updated plugin can talk to old and new ld and where a new ld can also 
> > talk
> > to an old plugin.  That requires an arbitration which I don't see 
> > implemented?
>
> So ld-new will set new callbacks tv_add_symbols_v2 and tv_get_symbols_v4, 
> these contain

ld-new don't have to use the new interface since it isn't needed.


-- 
H.J.

Re: ODR violation in ranges

[Patrick Palka via Gcc-patches]

On Wed, 11 Mar 2020, Tam S. B. via Gcc-patches wrote:

> IIUC using lambda in inline variable initializer is not ODR violation. This 
> is covered in CWG 2300 ( 
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1510r0.html#2300 ).
> 
> 
> From: Libstdc++  on behalf of Jonathan Wakely 
> via Libstdc++ 
> Sent: Wednesday, March 11, 2020 10:26
> To: Nathan Sidwell
> Cc: libstd...@gcc.gnu.org; GCC Patches
> Subject: Re: ODR violation in ranges
> 
> On 11/03/20 06:08 -0400, Nathan Sidwell wrote:
> >Jonathan,
> >the ranges header contains code like:
> >inline constexpr __adaptor::_RangeAdaptorClosure all
> >  = []  (_Range&& __r)
> >  {
> > if constexpr (view>)
> >   return std::forward<_Range>(__r);
> > else if constexpr (requires { ref_view{std::forward<_Range>(__r)}; })
> >   return ref_view{std::forward<_Range>(__r)};
> > else
> >   return subrange{std::forward<_Range>(__r)};
> >  };
> >
> >(line 1236)
> >
> >When you strip away all the templateyness, you have:
> >
> >
> >inline constexpr auto all = [] () {};
> >
> >
> >That's an ODR violation -- the initializers in different TUs are not
> >the same!
> >
> >As you can guess, I can't turn this into a header unit (well, I can,
> >but merging duplicates complains at you)
> 
> CC libstdc++@ and Patrick.
> 
> I did wonder if using lambdas for those global variables would be OK.
> 
> I think we'll need a new class template for each view adaptor, rather
> than using the _RangeAdaptorClosure to hold a closure.
> 
> Patrick, can you look into that please?

IIUC, it should suffice to replace the lambda in the initializer with a
function object, maybe something like:

  struct _All
  {
constexpr auto
operator()(...)
{
};
  };

  inline constexpr __adaptor::_RangeAdaptorClosure<_All> all;

Do the lambdas in the bodies of _RangeAdaptor::operator() and
_RangeAdaptorClosure::operator|() pose a problem too?

Re: Ping: [PATCH] wwwdocs: Document support for extended identifiers added to GCC 10

[Lewis Hyatt via Gcc-patches]

Great, thank you very much for taking a look at it. Sorry if this
email is unnecessary noise, but it wasn't quite clear to me whether
this should also still be approved from a content perspective? I
didn't want to assume. Thanks!

-Lewis

On Sat, Mar 7, 2020 at 6:23 PM Gerald Pfeifer  wrote:
>
> On Tue, 25 Feb 2020, Lewis Hyatt wrote:
> > Just checking whether the below is OK for gcc 10 changes.html please. 
> > Thanks!
> > https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01667.html
>
> Yes, this looks fine to me from a web perspective.
>
> Thank you,
> Gerald

Re: Remove redundant zero extend

[David Malcolm via Gcc-patches]

On Wed, 2020-03-11 at 13:04 +, Nidal Faour via Gcc-patches wrote:

[...]

> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index f91af78a302..c5a701e08af 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-02-19  Nidal Faour  
> +   * gcc.target/riscv/masking-to-byte-1.c: New test
> +   * gcc.target/riscv/masking-to-byte-2.c: New test
> +
> 2020-03-09  Marek Polacek  
> PR c++/92031 - bogus taking address of rvalue error.

If I'm reading it right, the patch is missing these new testcases.

Dave

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Martin Liška]

On 3/11/20 1:51 PM, Richard Biener wrote:

I'm not sure I understand the versioning, we should aim at something where
an updated plugin can talk to old and new ld and where a new ld can also talk
to an old plugin.  That requires an arbitration which I don't see implemented?


So ld-new will set new callbacks tv_add_symbols_v2 and tv_get_symbols_v4, these 
contain
ld_plugin_symbol_v2 with filled ::symbol_type. ld-new will check for he new API 
with
LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2 and will tv_add_symbols and 
tv_get_symbols callbacks.

ld-old will use the old tv_add_symbols and tv_get_symbols callback.

One another level of compatibility is the LTO bytecode, where new plugin should
handle also old LTO bytecode. That's done with parsing of LTO bytecode version
(lto_section).



Splitting an existing field isn't hackish IMHO.  I guess even
explicitely changing it
to one short and two char fields would be OK.


Then I'm fine with that as well. Am I right that the split order
will depend of the endianess?



Is there a comprehensive list of plugins out in the wild using the LD
plugin API?


I know only about:
$ ls /usr/lib/bfd-plugins
liblto_plugin.so.0.0.0  LLVMgold.so

and I know about Alexander Monakov (some dead code elimination plug-in).



Note we also have to bring in gold folks (not sure if lld also
implements the same
plugin API)


I don't see how can they be affected?

Martin

Remove redundant zero extend

[Nidal Faour via Gcc-patches]

This patch is a code density oriented and attempt to remove redundant sign/zero 
extension from assignment statement.
The approach taken is to use VRP data while expanding the assignment to RTL to 
determine whether a sign/zero extension is necessary.
Thought the motivation of the patch is code density but it also good for speed.

for example:
extern unsigned int func ();

  unsigned char
  foo (unsigned int arg)
  {
if (arg == 2)
  return 0;

return (func() == arg || arg == 7);
  }

the result of the comparison in the return will yield a False or True, which 
will be converted to integer and then casting to unsigned char.
this casting from integer to unsigned char is redundant because the value is 
either 0 or 1.

this patch is targeting the RISCV-32bit only.
This patch has been tested on a real embedded project and saved about 0.2% of 
code size.

P.S. I have an FSF license under Western Digital incorporation.
P.S. I've attached the patch as a file in case my email client corrupted the 
patch itself

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 9864e4344d2..c3739b2f331 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public 
License
along with GCC; see the file COPYING3.  If not see
.  */
+#include 
#include "config.h"
#include "system.h"
#include "coretypes.h"
@@ -73,6 +74,8 @@ along with GCC; see the file COPYING3.  If not see
#include "tree-ssa-address.h"
#include "output.h"
#include "builtins.h"
+#include "tree-vrp.h"
+
 /* Some systems use __main in a way incompatible with its use in gcc, in these
cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
@@ -3640,6 +3643,107 @@ expand_clobber (tree lhs)
 }
}
+/* Print the ranges of the operand and save it to the dump file
+   used for debug purposes only.  */
+
+void
+print_range (tree operand, wide_int min, wide_int max)
+{
+  pretty_printer buffer;
+  pp_needs_newline () = false;
+  buffer.buffer->stream = dump_file;
+  fprintf (dump_file, "Range for lhs: [");
+  pp_wide_int (, min, TYPE_SIGN (TREE_TYPE (operand)));
+  pp_printf (, ", ");
+  pp_wide_int (, max, TYPE_SIGN (TREE_TYPE (operand)));
+  pp_printf (, "]");
+  pp_flush ();
+  fprintf (dump_file, "\n");
+}
+
+
+
+/* Check that REG and SUBREG modes match between src and dest,
+   and that there is a cast to be removed.  */
+
+bool
+cast_to_remove_p (rtx src, rtx dest, machine_mode from
+ , machine_mode to)
+{
+  if (GET_CODE (src) != SUBREG
+  || GET_CODE (dest) != SUBREG
+  || GET_CODE (SUBREG_REG (src)) != REG
+  || GET_CODE (SUBREG_REG (dest)) != REG
+  || GET_MODE (src) != GET_MODE (dest)
+  || GET_MODE (SUBREG_REG (src)) != GET_MODE (SUBREG_REG (dest))
+  || GET_MODE (src) != to
+  || GET_MODE (SUBREG_REG (src)) != from)
+return false;
+  return true;
+}
+
+bool
+remove_cast_p (rtx temp, rtx target, gassign *assign_stmt)
+{
+  enum gimple_code code;
+  value_range_kind lhs_range, rhs_range;
+  unsigned int ops_num;
+  wide_int lhs_min, lhs_max, rhs_min, rhs_max;
+  if (gimple_assign_cast_p (assign_stmt))
+  {
+code = gimple_code (assign_stmt);
+ops_num = gimple_num_ops (assign_stmt);
+if (code == GIMPLE_ASSIGN && ops_num < 3)
+{
+  tree lhs = gimple_assign_lhs (assign_stmt);
+  if (POINTER_TYPE_P (TREE_TYPE (lhs)))
+   return false;
+  lhs_range = get_range_info (lhs, _min, _max);
+  if (dump_file && lhs_range == VR_RANGE)
+   print_range (lhs, lhs_min, lhs_max);
+
+  tree rhs = gimple_assign_rhs1 (assign_stmt);
+  if (POINTER_TYPE_P (TREE_TYPE (rhs)))
+   return false;
+  rhs_range = get_range_info (rhs, _min, _max);
+  if (rhs_range == VR_RANGE)
+  {
+   unsigned int rhs_precision
+ = std::max (wi::min_precision (rhs_max, TYPE_SIGN (TREE_TYPE 
(rhs))),
+ wi::min_precision (rhs_min, TYPE_SIGN (TREE_TYPE 
(rhs;
+   unsigned int lhs_precision = TYPE_PRECISION (TREE_TYPE (lhs));
+   if (lhs_precision > rhs_precision)
+   {
+
+ if (dump_file)
+ {
+print_range (rhs, rhs_min, rhs_max);
+if (lhs_precision > rhs_precision)
+ fprintf (dump_file,
+ "EXPAND-CAST: This casting can be 
optimized\n");
+ }
+
+ return cast_to_remove_p (temp,
+ target,
+ TYPE_MODE (TREE_TYPE (rhs)),
+ TYPE_MODE (TREE_TYPE (lhs)));
+   }
+   else
+ if (dump_file)
+fprintf (dump_file, "EXPAND-CAST: lhs_precision < 
rhs_precision\n");
+  }
+  else
+ if (dump_file)
+   fprintf (dump_file, "EXPAND-CAST: Range is not VR_RANGE\n");
+  

Re: [PATCH] Generalize -fuse-ld= to support absolute path or arbitrary ld.linker

[Martin Liška]

On 2/10/20 1:02 AM, Fangrui Song via gcc-patches wrote:

Hello.

Thank you for the patch. You haven't received a review because we are right now
in stage4 of the development cycle:
https://gcc.gnu.org/develop.html#stage4

Anyway, I'm going to provide a review (even though I'm not maintainer of that).

Can you please describe your test-case why you need such option? When using
a different ld, I typically export PATH=/path/to/binutils and then run configure
and make.

I noticed not ideal error message:

$ gcc -fuse-ld=pes /tmp/foo.c
collect2: fatal error: cannot find ‘ld’
compilation terminated.

while clang prints:

$clang -fuse-ld=pes /tmp/foo.c
clang-9.0: error: invalid linker name in argument '-fuse-ld=pes'

The rest of the patch is comment inline...


PR driver/93645
* common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
* opts.c (common_handle_option): Handle OPT_fuse_ld_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
---
  gcc/ChangeLog   |  8 ++
  gcc/collect2.c  | 67 -
  gcc/common.opt  | 14 ++
  gcc/doc/invoke.texi | 15 +++---
  gcc/gcc.c   | 14 --
  gcc/opts.c  |  4 +--
  6 files changed, 57 insertions(+), 65 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index feb2d066d0b..6bcec12d841 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-09  Fangrui Song  
+
+   PR driver/93645
+   * common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
+   * opts.c (common_handle_option): Handle OPT_fuse_ld_.
+   * gcc.c (driver_handle_option): Likewise.
+   * collect2.c (main): Likewise.
+
  2020-02-09  Uroš Bizjak  
  
  	* recog.c: Move pass_split_before_sched2 code in front of

diff --git a/gcc/collect2.c b/gcc/collect2.c
index 502d629141c..a3ef525a93b 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -859,18 +859,12 @@ main (int argc, char **argv)
  {
USE_DEFAULT_LD,
USE_PLUGIN_LD,
-  USE_GOLD_LD,
-  USE_BFD_LD,
-  USE_LLD_LD,
-  USE_LD_MAX
+  USE_LD
  } selected_linker = USE_DEFAULT_LD;
-  static const char *const ld_suffixes[USE_LD_MAX] =
+  static const char *const ld_suffixes[USE_LD] =
  {
"ld",
-  PLUGIN_LD_SUFFIX,
-  "ld.gold",
-  "ld.bfd",
-  "ld.lld"
+  PLUGIN_LD_SUFFIX
  };
static const char *const real_ld_suffix = "real-ld";
static const char *const collect_ld_suffix = "collect-ld";
@@ -882,7 +876,7 @@ main (int argc, char **argv)
static const char *const strip_suffix = "strip";
static const char *const gstrip_suffix = "gstrip";
  
-  const char *full_ld_suffixes[USE_LD_MAX];

+  const char *full_ld_suffixes[USE_LD];
  #ifdef CROSS_DIRECTORY_STRUCTURE
/* If we look for a program in the compiler directories, we just use
   the short name, since these directories are already system-specific.
@@ -924,6 +918,7 @@ main (int argc, char **argv)
const char **ld1;
bool use_plugin = false;
bool use_collect_ld = false;
+  const char *use_ld = NULL;
  
/* The kinds of symbols we will have to consider when scanning the

   outcome of a first pass link.  This is ALL to start with, then might
@@ -948,7 +943,7 @@ main (int argc, char **argv)
  #endif
int i;
  
-  for (i = 0; i < USE_LD_MAX; i++)

+  for (i = 0; i < USE_LD; i++)
  full_ld_suffixes[i]
  #ifdef CROSS_DIRECTORY_STRUCTURE
= concat (target_machine, "-", ld_suffixes[i], NULL);
@@ -1041,12 +1036,11 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
  selected_linker = USE_PLUGIN_LD;
  }
-   else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+   else if (!strncmp (argv[i], "-fuse-ld=", 9))
+ {
+   use_ld = argv[i] + 9;
+   selected_linker = USE_LD;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
  {
/* Parse the output filename if it's given so that we can make
@@ -1152,8 +1146,7 @@ main (int argc, char **argv)
/* Maybe we know the right file to use (if not cross).  */
ld_file_name = 0;
  #ifdef DEFAULT_LINKER
-  if (selected_linker == USE_BFD_LD || selected_linker == USE_GOLD_LD ||
-  selected_linker == USE_LLD_LD)
+  if (!ld_file_name && selected_linker == USE_LD)
  {
char *linker_name;
  # ifdef HOST_EXECUTABLE_SUFFIX
@@ -1168,15 +1161,13 @@ main (int argc, char **argv)
  if (! strcmp (_linker[len], HOST_EXECUTABLE_SUFFIX))
{
  default_linker[len] = '\0';
- linker_name = concat (default_linker,
-   _suffixes[selected_linker][2],

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Richard Biener via Gcc-patches]

On Wed, Mar 11, 2020 at 1:22 PM Martin Liška  wrote:
>
> On 3/11/20 11:22 AM, Richard Biener wrote:
> > On Wed, Mar 11, 2020 at 10:19 AM Martin Liška  wrote:
> >>
> >> On 3/10/20 1:07 PM, Martin Liška wrote:
> >>> On 3/10/20 12:24 PM, Richard Biener wrote:
>  Not sure how symtab is encoded right now but we also could have
> >>>
> >>> Ok, right now I don't see symtab entry much extensible.
> >>>
> >>> But what am I suggesting is to parse LTO bytecode version and then
> >>> process conditional parsing of lto_symtab section.
> >>>
> >>> Thoughts?
> >>> Martin
> >>
> >> So as H.J. correctly pointed I can't add new symbol_type into 
> >> ld_plugin_symbol struct.
> >> It would make ABI change as correctly identified by abidiff:
> >>
> >> abidiff /tmp/before.o /tmp/after.o
> >> Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> >> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> >> Function symbols changes summary: 0 Removed, 0 Added function symbol not 
> >> referenced by debug info
> >> Variable symbols changes summary: 0 Removed, 1 Added variable symbol not 
> >> referenced by debug info
> >>
> >> 1 function with some indirect sub-type change:
> >>
> >> [C]'function ld_plugin_status onload(ld_plugin_tv*)' at 
> >> lto-plugin.c:1275:1 has some indirect sub-type changes:
> >>   parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
> >> in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
> >>   type size hasn't changed
> >>   1 data member changes (1 filtered):
> >>type of 'union {int tv_val; const char* tv_string; 
> >> ld_plugin_register_claim_file tv_register_claim_file; 
> >> ld_plugin_register_all_symbols_read tv_register_all_symbols_read; 
> >> ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols 
> >> tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; 
> >> ld_plugin_add_input_file tv_add_input_file; ld_plugin_message tv_message; 
> >> ld_plugin_get_input_file tv_get_input_file; ld_plugin_get_view 
> >> tv_get_view; ld_plugin_release_input_file tv_release_input_file; 
> >> ld_plugin_add_input_library tv_add_input_library; 
> >> ld_plugin_set_extra_library_path tv_set_extra_library_path; 
> >> ld_plugin_get_input_section_count tv_get_input_section_count; 
> >> ld_plugin_get_input_section_type tv_get_input_section_type; 
> >> ld_plugin_get_input_section_name tv_get_input_section_name; 
> >> ld_plugin_get_input_section_contents tv_get_input_section_contents; 
> >> ld_plugin_update_section_order tv_update_section_order; 
> >> ld_plugin_allow_section_ordering tv_allow_section_ordering; 
> >> ld_plugin_allow_unique_segment_for_sections 
> >> tv_allow_unique_segment_for_sections; 
> >> ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections; 
> >> ld_plugin_get_input_section_alignment tv_get_input_section_alignment; 
> >> ld_plugin_get_input_section_size tv_get_input_section_size; 
> >> ld_plugin_register_new_input tv_register_new_input; 
> >> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} ld_plugin_tv::tv_u' 
> >> changed:
> >>  type size hasn't changed
> >>  1 data member changes (1 filtered):
> >>   type of 'ld_plugin_add_symbols tv_add_symbols' changed:
> >> underlying type 'enum ld_plugin_status (void*, int, const 
> >> ld_plugin_symbol*)*' changed:
> >>   in pointed to type 'function type enum ld_plugin_status 
> >> (void*, int, const ld_plugin_symbol*)':
> >> parameter 3 of type 'const ld_plugin_symbol*' has 
> >> sub-type changes:
> >>   in pointed to type 'const ld_plugin_symbol':
> >> in unqualified underlying type 'struct 
> >> ld_plugin_symbol' at plugin-api.h:86:1:
> >>   type size changed from 256 to 288 (in bits)
> >>   1 data member insertion:
> >> 'int ld_plugin_symbol::symbol_type', at offset 
> >> 256 (in bits) at plugin-api.h:95:1
> >>
> >> So that I need to come up with ld_plugin_symbol_v2. It brings more 
> >> challenges: one has 2 parallel symbol
> >> tables:
> >>
> >> struct plugin_symtab
> >> {
> >> ...
> >> struct ld_plugin_symbol_v2 *syms;
> >> struct ld_plugin_symbol *syms_v1;
> >> ...
> >> };
> >>
> >> and the information of these should by aligned.
> >>
> >> The patch can survive lto.exp and I would like to ask H.J. to write 
> >> bintuils counterpart that will
> >> utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.
> >>
> >> Thoughts?
> >
> > Can't we simply have _V4/V2 use the upper half of
> > ld_plugin_symbol::def?  If the linker
> > then requests _V4 but the plugin cannot cope it could still "use" the
> > data but get
> > LDST_UNKNOWN (zero) there.
>
> Can be possible, but it's hack a bit. The plugin has a mechanisms for 
> versioning
> and this change does not align with the idea.

I'm not sure I understand the versioning, we 

Re: ODR violation in ranges

[Jonathan Wakely via Gcc-patches]

On 11/03/20 10:56 +, Tam S. B. via Libstdc++ wrote:

IIUC using lambda in inline variable initializer is not ODR violation. This is 
covered in CWG 2300 ( 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1510r0.html#2300 ).


Ah yes, I think somebody (probably Patrick) has pointed out that
wording before, and I'd forgotten about it.

So that would make it an unsupported G++ feature then?

RE: [PATCHv2] Ada: gcc-interface: fixed assertion for aliased entities

[Richard Wai]

> -Original Message-
> From: Eric Botcazou 
> Sent: March 11, 2020 6:16 AM
> To: Richard Wai 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCHv2] Ada: gcc-interface: fixed assertion for aliased
entities
> 

> 
> Thanks for the analysis and the fix.  This has indeed apparently never
worked
> and could be the origin of a few Ada PRs in Bugzilla, although I didn't
really
> check that.  The patch is obviously very safe so I have installed it on
the
> mainline and the 9 branch (after slightly trimming down the comment) with
> the following ChangeLogs (I forgot to say that we require them too):
> 
> 2020-03-11  Richard Wai  
> 
>   * gcc-interface/decl.c (gnat_to_gnu_entity): Also test Is_Public on
>   the Alias of the entitiy, if it is present, in the main assertion.
> 
> 2020-03-11  Richard Wai  
> 
>   * gnat.dg/subpools1.adb: New test.
> 
> 
> [The next time please send the patch and the testcase as attachments
> instead, they apparently have been mangled by your mailer.]
> 

Thanks for this; all points noted!


Richard Wai
ANNEXI-STRAYLINE

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Martin Liška]

On 3/11/20 11:22 AM, Richard Biener wrote:

On Wed, Mar 11, 2020 at 10:19 AM Martin Liška  wrote:


On 3/10/20 1:07 PM, Martin Liška wrote:

On 3/10/20 12:24 PM, Richard Biener wrote:

Not sure how symtab is encoded right now but we also could have


Ok, right now I don't see symtab entry much extensible.

But what am I suggesting is to parse LTO bytecode version and then
process conditional parsing of lto_symtab section.

Thoughts?
Martin


So as H.J. correctly pointed I can't add new symbol_type into ld_plugin_symbol 
struct.
It would make ABI change as correctly identified by abidiff:

abidiff /tmp/before.o /tmp/after.o
Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 0 Removed, 0 Added function symbol not 
referenced by debug info
Variable symbols changes summary: 0 Removed, 1 Added variable symbol not 
referenced by debug info

1 function with some indirect sub-type change:

[C]'function ld_plugin_status onload(ld_plugin_tv*)' at lto-plugin.c:1275:1 
has some indirect sub-type changes:
  parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
  type size hasn't changed
  1 data member changes (1 filtered):
   type of 'union {int tv_val; const char* tv_string; 
ld_plugin_register_claim_file tv_register_claim_file; 
ld_plugin_register_all_symbols_read tv_register_all_symbols_read; 
ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols 
tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; ld_plugin_add_input_file 
tv_add_input_file; ld_plugin_message tv_message; ld_plugin_get_input_file 
tv_get_input_file; ld_plugin_get_view tv_get_view; ld_plugin_release_input_file 
tv_release_input_file; ld_plugin_add_input_library tv_add_input_library; 
ld_plugin_set_extra_library_path tv_set_extra_library_path; 
ld_plugin_get_input_section_count tv_get_input_section_count; 
ld_plugin_get_input_section_type tv_get_input_section_type; 
ld_plugin_get_input_section_name tv_get_input_section_name; 
ld_plugin_get_input_section_contents tv_get_input_section_contents; 
ld_plugin_update_section_order tv_update_section_order; 
ld_plugin_allow_section_ordering tv_allow_section_ordering; 
ld_plugin_allow_unique_segment_for_sections 
tv_allow_unique_segment_for_sections; ld_plugin_unique_segment_for_sections 
tv_unique_segment_for_sections; ld_plugin_get_input_section_alignment 
tv_get_input_section_alignment; ld_plugin_get_input_section_size 
tv_get_input_section_size; ld_plugin_register_new_input tv_register_new_input; 
ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} ld_plugin_tv::tv_u' changed:
 type size hasn't changed
 1 data member changes (1 filtered):
  type of 'ld_plugin_add_symbols tv_add_symbols' changed:
underlying type 'enum ld_plugin_status (void*, int, const 
ld_plugin_symbol*)*' changed:
  in pointed to type 'function type enum ld_plugin_status 
(void*, int, const ld_plugin_symbol*)':
parameter 3 of type 'const ld_plugin_symbol*' has sub-type 
changes:
  in pointed to type 'const ld_plugin_symbol':
in unqualified underlying type 'struct 
ld_plugin_symbol' at plugin-api.h:86:1:
  type size changed from 256 to 288 (in bits)
  1 data member insertion:
'int ld_plugin_symbol::symbol_type', at offset 256 
(in bits) at plugin-api.h:95:1

So that I need to come up with ld_plugin_symbol_v2. It brings more challenges: 
one has 2 parallel symbol
tables:

struct plugin_symtab
{
...
struct ld_plugin_symbol_v2 *syms;
struct ld_plugin_symbol *syms_v1;
...
};

and the information of these should by aligned.

The patch can survive lto.exp and I would like to ask H.J. to write bintuils 
counterpart that will
utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.

Thoughts?


Can't we simply have _V4/V2 use the upper half of
ld_plugin_symbol::def?  If the linker
then requests _V4 but the plugin cannot cope it could still "use" the
data but get
LDST_UNKNOWN (zero) there.


Can be possible, but it's hack a bit. The plugin has a mechanisms for versioning
and this change does not align with the idea.



IMHO LDST_VARIABLE_BSS is "misplaced"?  "BSS" is the section of the variable.


Yes.


If we want to encode more of ELF it should be LDST_OBJECT and LDST_FUNC.
Note there's also rodata vs data info that would be missing in case
we'd want tools
like readelf -s dump the symbol table of the IL part of an object.  It
looks like
nm can also distinguish rodata from data ("R", "r" vs "d") and "small object"
data sections (not sure what's that about).  It seems nm cannot distinguish
symbols in mergeable string sections (it dumps "R" for me there).  So intead
of mangling everything into enum 

Re: [testsuite] Add @ lines to check-function-bodies fluff

[Richard Sandiford]

Matthew Malcomson  writes:
> When using `check-function-bodies`, the subroutine `parse_function_bodies` 
> uses
> the `fluff` regexp to remove uninteresting assembly lines.
>
> Arm targets generate assembly with some lines prefixed by `@`, these lines are
> left by this process.
>
> As an example of some lines prefixed by `@': the assembly output from the
> `stacktest1` function in "bfloat16_simd_3_1.c" is:
>
> .align  2
> .global stacktest1
> .arch armv8.2-a
> .syntax unified
> .arm
> .fpu neon-fp-armv8
> .type   stacktest1, %function
> stacktest1:
> @ args = 0, pretend = 0, frame = 8
> @ frame_needed = 0, uses_anonymous_args = 0
> @ link register save eliminated.
> sub sp, sp, #8
> add r3, sp, #6
> vst1.16 {d0[0]}, [r3]
> vld1.16 {d0[0]}, [r3]
> add sp, sp, #8
> @ sp needed
> bx  lr
> .size   stacktest1, .-stacktest1
>
>
>
> It seems that previous uses of `check-function-bodies` in the arm backend have
> avoided problems with such lines since they use the `...` regexp in each place
> such fluff occurs.
>
> I'm currently writing a patch that I'd like to match the entire function body,
> so I'd like to remove such `@` lines automatically.
>
> gcc/testsuite/ChangeLog:
>
> 2020-03-10  Matthew Malcomson  
>
>   * lib/scanasm.exp (parse_function_bodies): Lines starting with '@' also
>   counted as fluff.

OK, thanks.

Richard

[PATCH v4][ARM][GCC][2/x]: MVE ACLE intrinsics framework patch.

[Srinath Parvathaneni]

Hello Kyrill,

This patch addresses all the comments in patch version v2.
(version v2) https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540416.html
Please ignore version v3.
(version v3) https://gcc.gnu.org/pipermail/gcc-patches/2020-March/541783.html

###
Hello,

This patch is part of MVE ACLE intrinsics framework.
This patches add support to update (read/write) the APSR (Application Program 
Status Register)
register and FPSCR (Floating-point Status and Control Register) register for 
MVE.
This patch also enables thumb2 mov RTL patterns for MVE.

A new feature bit vfp_base is added. This bit is enabled for all VFP, MVE and 
MVE with floating point
extensions. This bit is used to enable the macro TARGET_VFP_BASE. For all the 
VFP instructions, RTL patterns,
status and control registers are guarded by TARGET_HAVE_FLOAT. But this patch 
modifies that and the
common instructions, RTL patterns, status and control registers bewteen MVE and 
VFP are guarded by
TARGET_VFP_BASE macro.

The RTL pattern set_fpscr and get_fpscr are updated to use VFPCC_REGNUM because 
few MVE intrinsics
set/get carry bit of FPSCR register.

Please refer to Arm reference manual [1] for more details.
[1] https://developer.arm.com/docs/ddi0553/latest

Regression tested on target arm-none-eabi and armeb-none-eabi and found no 
regressions.

Ok for trunk?

Thanks,
Srinath
gcc/ChangeLog:

2020-03-06  Andre Vieira  
Mihail Ionescu  
Srinath Parvathaneni  

* common/config/arm/arm-common.c (arm_asm_auto_mfpu): When vfp_base
feature bit is on and -mfpu=auto is passed as compiler option, do not
generate error on not finding any matching fpu. Because in this case
fpu is not required.
* config/arm/arm-cpus.in (vfp_base): Define feature bit, this bit is
enabled for MVE and also for all VFP extensions.
(VFPv2): Modify fgroup to enable vfp_base feature bit when ever VFPv2
is enabled.
(MVE): Define fgroup to enable feature bits mve, vfp_base and armv7em.
(MVE_FP): Define fgroup to enable feature bits is fgroup MVE and FPv5
along with feature bits mve_float.
(mve): Modify add options in armv8.1-m.main arch for MVE.
(mve.fp): Modify add options in armv8.1-m.main arch for MVE with
floating point.
* config/arm/arm.c (use_return_insn): Replace the
check with TARGET_VFP_BASE.
(thumb2_legitimate_index_p): Replace TARGET_HARD_FLOAT with
TARGET_VFP_BASE.
(arm_rtx_costs_internal): Replace "TARGET_HARD_FLOAT || TARGET_HAVE_MVE"
with TARGET_VFP_BASE, to allow cost calculations for copies in MVE as
well.
(arm_get_vfp_saved_size): Replace TARGET_HARD_FLOAT with
TARGET_VFP_BASE, to allow space calculation for VFP registers in MVE
as well.
(arm_compute_frame_layout): Likewise.
(arm_save_coproc_regs): Likewise.
(arm_fixed_condition_code_regs): Modify to enable using VFPCC_REGNUM
in MVE as well.
(arm_hard_regno_mode_ok): Replace "TARGET_HARD_FLOAT || TARGET_HAVE_MVE"
with equivalent macro TARGET_VFP_BASE.
(arm_expand_epilogue_apcs_frame): Likewise.
(arm_expand_epilogue): Likewise.
(arm_conditional_register_usage): Likewise.
(arm_declare_function_name): Add check to skip printing .fpu directive
in assembly file when TARGET_VFP_BASE is enabled and fpu_to_print is
"softvfp".
* config/arm/arm.h (TARGET_VFP_BASE): Define.
* config/arm/arm.md (arch): Add "mve" to arch.
(eq_attr "arch" "mve"): Enable on TARGET_HAVE_MVE is true.
(vfp_pop_multiple_with_writeback): Replace "TARGET_HARD_FLOAT
|| TARGET_HAVE_MVE" with equivalent macro TARGET_VFP_BASE.
* config/arm/constraints.md (Uf): Define to allow modification to FPCCR
in MVE.
* config/arm/thumb2.md (thumb2_movsfcc_soft_insn): Modify target guard
to not allow for MVE.
* config/arm/unspecs.md (UNSPEC_GET_FPSCR): Move to volatile unspecs
enum.
(VUNSPEC_GET_FPSCR): Define.
* config/arm/vfp.md (thumb2_movhi_vfp): Add support for VMSR and VMRS
instructions which move to general-purpose Register from Floating-point
Special register and vice-versa.
(thumb2_movhi_fp16): Likewise.
(thumb2_movsi_vfp): Add support for VMSR and VMRS instructions along
with MCR and MRC instructions which set and get Floating-point Status
and Control Register (FPSCR).
(movdi_vfp): Modify pattern to enable Single-precision scalar float move
in MVE.
(thumb2_movdf_vfp): Modify pattern to enable Double-precision scalar
float move patterns in MVE.
(thumb2_movsfcc_vfp): Modify pattern to enable single float conditional
code move patterns of VFP also in MVE by adding TARGET_VFP_BASE check.
(thumb2_movdfcc_vfp): Modify pattern to enable double 

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Richard Biener via Gcc-patches]

On Wed, Mar 11, 2020 at 11:22 AM Richard Biener
 wrote:
>
> On Wed, Mar 11, 2020 at 10:19 AM Martin Liška  wrote:
> >
> > On 3/10/20 1:07 PM, Martin Liška wrote:
> > > On 3/10/20 12:24 PM, Richard Biener wrote:
> > >> Not sure how symtab is encoded right now but we also could have
> > >
> > > Ok, right now I don't see symtab entry much extensible.
> > >
> > > But what am I suggesting is to parse LTO bytecode version and then
> > > process conditional parsing of lto_symtab section.
> > >
> > > Thoughts?
> > > Martin
> >
> > So as H.J. correctly pointed I can't add new symbol_type into 
> > ld_plugin_symbol struct.
> > It would make ABI change as correctly identified by abidiff:
> >
> > abidiff /tmp/before.o /tmp/after.o
> > Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> > Function symbols changes summary: 0 Removed, 0 Added function symbol not 
> > referenced by debug info
> > Variable symbols changes summary: 0 Removed, 1 Added variable symbol not 
> > referenced by debug info
> >
> > 1 function with some indirect sub-type change:
> >
> >[C]'function ld_plugin_status onload(ld_plugin_tv*)' at 
> > lto-plugin.c:1275:1 has some indirect sub-type changes:
> >  parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
> >in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
> >  type size hasn't changed
> >  1 data member changes (1 filtered):
> >   type of 'union {int tv_val; const char* tv_string; 
> > ld_plugin_register_claim_file tv_register_claim_file; 
> > ld_plugin_register_all_symbols_read tv_register_all_symbols_read; 
> > ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols 
> > tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; 
> > ld_plugin_add_input_file tv_add_input_file; ld_plugin_message tv_message; 
> > ld_plugin_get_input_file tv_get_input_file; ld_plugin_get_view tv_get_view; 
> > ld_plugin_release_input_file tv_release_input_file; 
> > ld_plugin_add_input_library tv_add_input_library; 
> > ld_plugin_set_extra_library_path tv_set_extra_library_path; 
> > ld_plugin_get_input_section_count tv_get_input_section_count; 
> > ld_plugin_get_input_section_type tv_get_input_section_type; 
> > ld_plugin_get_input_section_name tv_get_input_section_name; 
> > ld_plugin_get_input_section_contents tv_get_input_section_contents; 
> > ld_plugin_update_section_order tv_update_section_order; 
> > ld_plugin_allow_section_ordering tv_allow_section_ordering; 
> > ld_plugin_allow_unique_segment_for_sections 
> > tv_allow_unique_segment_for_sections; ld_plugin_unique_segment_for_sections 
> > tv_unique_segment_for_sections; ld_plugin_get_input_section_alignment 
> > tv_get_input_section_alignment; ld_plugin_get_input_section_size 
> > tv_get_input_section_size; ld_plugin_register_new_input 
> > tv_register_new_input; ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} 
> > ld_plugin_tv::tv_u' changed:
> > type size hasn't changed
> > 1 data member changes (1 filtered):
> >  type of 'ld_plugin_add_symbols tv_add_symbols' changed:
> >underlying type 'enum ld_plugin_status (void*, int, const 
> > ld_plugin_symbol*)*' changed:
> >  in pointed to type 'function type enum ld_plugin_status 
> > (void*, int, const ld_plugin_symbol*)':
> >parameter 3 of type 'const ld_plugin_symbol*' has 
> > sub-type changes:
> >  in pointed to type 'const ld_plugin_symbol':
> >in unqualified underlying type 'struct 
> > ld_plugin_symbol' at plugin-api.h:86:1:
> >  type size changed from 256 to 288 (in bits)
> >  1 data member insertion:
> >'int ld_plugin_symbol::symbol_type', at offset 
> > 256 (in bits) at plugin-api.h:95:1
> >
> > So that I need to come up with ld_plugin_symbol_v2. It brings more 
> > challenges: one has 2 parallel symbol
> > tables:
> >
> > struct plugin_symtab
> > {
> > ...
> >struct ld_plugin_symbol_v2 *syms;
> >struct ld_plugin_symbol *syms_v1;
> > ...
> > };
> >
> > and the information of these should by aligned.
> >
> > The patch can survive lto.exp and I would like to ask H.J. to write 
> > bintuils counterpart that will
> > utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.
> >
> > Thoughts?
>
> Can't we simply have _V4/V2 use the upper half of
> ld_plugin_symbol::def?  If the linker
> then requests _V4 but the plugin cannot cope it could still "use" the
> data but get
> LDST_UNKNOWN (zero) there.
>
> IMHO LDST_VARIABLE_BSS is "misplaced"?  "BSS" is the section of the variable.
> If we want to encode more of ELF it should be LDST_OBJECT and LDST_FUNC.
> Note there's also rodata vs data info that would be missing in case
> we'd want tools
> like readelf -s dump the symbol table of the IL part of an object.  It
> looks like
> nm can also 

Re: [patch] Fix middle-end/93961

[Eric Botcazou]

> OK.  Note I wondered for some time whether we can pre-compute (and LTO
> stream) whether a type is variably modified during type layout?

During type layout seems a little early, the end of gimplification would seem 
more natural since we have a workaround in RETURN_TRUE_IF_VAR:

/* Test if T is either variable (if FN is zero) or an expression containing
   a variable in FN.  If TYPE isn't gimplified, return true also if
   gimplify_one_sizepos would gimplify the expression into a local
   variable.  */

-- 
Eric Botcazou

Re: ODR violation in ranges

[Jonathan Wakely via Gcc-patches]

On 11/03/20 06:08 -0400, Nathan Sidwell wrote:

Jonathan,
the ranges header contains code like:
   inline constexpr __adaptor::_RangeAdaptorClosure all
 = []  (_Range&& __r)
 {
if constexpr (view>)
  return std::forward<_Range>(__r);
else if constexpr (requires { ref_view{std::forward<_Range>(__r)}; })
  return ref_view{std::forward<_Range>(__r)};
else
  return subrange{std::forward<_Range>(__r)};
 };

(line 1236)

When you strip away all the templateyness, you have:


inline constexpr auto all = [] () {};


That's an ODR violation -- the initializers in different TUs are not 
the same!


As you can guess, I can't turn this into a header unit (well, I can, 
but merging duplicates complains at you)


CC libstdc++@ and Patrick.

I did wonder if using lambdas for those global variables would be OK.

I think we'll need a new class template for each view adaptor, rather
than using the _RangeAdaptorClosure to hold a closure.

Patrick, can you look into that please?

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Richard Biener via Gcc-patches]

On Wed, Mar 11, 2020 at 10:19 AM Martin Liška  wrote:
>
> On 3/10/20 1:07 PM, Martin Liška wrote:
> > On 3/10/20 12:24 PM, Richard Biener wrote:
> >> Not sure how symtab is encoded right now but we also could have
> >
> > Ok, right now I don't see symtab entry much extensible.
> >
> > But what am I suggesting is to parse LTO bytecode version and then
> > process conditional parsing of lto_symtab section.
> >
> > Thoughts?
> > Martin
>
> So as H.J. correctly pointed I can't add new symbol_type into 
> ld_plugin_symbol struct.
> It would make ABI change as correctly identified by abidiff:
>
> abidiff /tmp/before.o /tmp/after.o
> Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> Function symbols changes summary: 0 Removed, 0 Added function symbol not 
> referenced by debug info
> Variable symbols changes summary: 0 Removed, 1 Added variable symbol not 
> referenced by debug info
>
> 1 function with some indirect sub-type change:
>
>[C]'function ld_plugin_status onload(ld_plugin_tv*)' at 
> lto-plugin.c:1275:1 has some indirect sub-type changes:
>  parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
>in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
>  type size hasn't changed
>  1 data member changes (1 filtered):
>   type of 'union {int tv_val; const char* tv_string; 
> ld_plugin_register_claim_file tv_register_claim_file; 
> ld_plugin_register_all_symbols_read tv_register_all_symbols_read; 
> ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols 
> tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; 
> ld_plugin_add_input_file tv_add_input_file; ld_plugin_message tv_message; 
> ld_plugin_get_input_file tv_get_input_file; ld_plugin_get_view tv_get_view; 
> ld_plugin_release_input_file tv_release_input_file; 
> ld_plugin_add_input_library tv_add_input_library; 
> ld_plugin_set_extra_library_path tv_set_extra_library_path; 
> ld_plugin_get_input_section_count tv_get_input_section_count; 
> ld_plugin_get_input_section_type tv_get_input_section_type; 
> ld_plugin_get_input_section_name tv_get_input_section_name; 
> ld_plugin_get_input_section_contents tv_get_input_section_contents; 
> ld_plugin_update_section_order tv_update_section_order; 
> ld_plugin_allow_section_ordering tv_allow_section_ordering; 
> ld_plugin_allow_unique_segment_for_sections 
> tv_allow_unique_segment_for_sections; ld_plugin_unique_segment_for_sections 
> tv_unique_segment_for_sections; ld_plugin_get_input_section_alignment 
> tv_get_input_section_alignment; ld_plugin_get_input_section_size 
> tv_get_input_section_size; ld_plugin_register_new_input 
> tv_register_new_input; ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} 
> ld_plugin_tv::tv_u' changed:
> type size hasn't changed
> 1 data member changes (1 filtered):
>  type of 'ld_plugin_add_symbols tv_add_symbols' changed:
>underlying type 'enum ld_plugin_status (void*, int, const 
> ld_plugin_symbol*)*' changed:
>  in pointed to type 'function type enum ld_plugin_status 
> (void*, int, const ld_plugin_symbol*)':
>parameter 3 of type 'const ld_plugin_symbol*' has sub-type 
> changes:
>  in pointed to type 'const ld_plugin_symbol':
>in unqualified underlying type 'struct 
> ld_plugin_symbol' at plugin-api.h:86:1:
>  type size changed from 256 to 288 (in bits)
>  1 data member insertion:
>'int ld_plugin_symbol::symbol_type', at offset 256 
> (in bits) at plugin-api.h:95:1
>
> So that I need to come up with ld_plugin_symbol_v2. It brings more 
> challenges: one has 2 parallel symbol
> tables:
>
> struct plugin_symtab
> {
> ...
>struct ld_plugin_symbol_v2 *syms;
>struct ld_plugin_symbol *syms_v1;
> ...
> };
>
> and the information of these should by aligned.
>
> The patch can survive lto.exp and I would like to ask H.J. to write bintuils 
> counterpart that will
> utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.
>
> Thoughts?

Can't we simply have _V4/V2 use the upper half of
ld_plugin_symbol::def?  If the linker
then requests _V4 but the plugin cannot cope it could still "use" the
data but get
LDST_UNKNOWN (zero) there.

IMHO LDST_VARIABLE_BSS is "misplaced"?  "BSS" is the section of the variable.
If we want to encode more of ELF it should be LDST_OBJECT and LDST_FUNC.
Note there's also rodata vs data info that would be missing in case
we'd want tools
like readelf -s dump the symbol table of the IL part of an object.  It
looks like
nm can also distinguish rodata from data ("R", "r" vs "d") and "small object"
data sections (not sure what's that about).  It seems nm cannot distinguish
symbols in mergeable string sections (it dumps "R" for me there).  So intead
of mangling everything into enum 

Re: [PATCHv2] Ada: gcc-interface: fixed assertion for aliased entities

[Eric Botcazou]

> This is the second go at this patch, and now with a testcase!
> 
> In summary:
> 
> If the type is derived in the current compilation unit, and Allocate is not
> overridden on derivation (as is typically the case with
> Root_Storage_Pool_With_Subpools), the entity for Allocate for the derived
> type is then an alias to System.Storage_Pools.Subpools.Allocate. When the
> allocator is built, gnat_to_gnu_entity is called with definition == false
> for the derived storage pool's allocate operation. An assertion is
> gnat_to_gnu_entity fails in this case, since it is not a definition, and
> Is_Public is false (since the entity is nested in the same compilation
> unit).
> 
> This patch adds an extra check in the assertion (decl.c: gnat_to_gnu_entity)
> that the entity has the Aliased property, and that the Alias is also
> Public.

Thanks for the analysis and the fix.  This has indeed apparently never worked 
and could be the origin of a few Ada PRs in Bugzilla, although I didn't really 
check that.  The patch is obviously very safe so I have installed it on the 
mainline and the 9 branch (after slightly trimming down the comment) with the 
following ChangeLogs (I forgot to say that we require them too):

2020-03-11  Richard Wai  

* gcc-interface/decl.c (gnat_to_gnu_entity): Also test Is_Public on
the Alias of the entitiy, if it is present, in the main assertion.

2020-03-11  Richard Wai  

* gnat.dg/subpools1.adb: New test.


[The next time please send the patch and the testcase as attachments instead,
they apparently have been mangled by your mailer.]

-- 
Eric Botcazou

Re: [AArch64] Backporting -moutline-atomics to gcc 9.x and 8.x

[Kyrill Tkachov]

Hi Sebastian,

On 3/9/20 9:47 PM, Pop, Sebastian wrote:

Hi,

Please see attached the patches to add -moutline-atomics to the gcc-9 branch.
Tested on graviton2 aarch64-linux with bootstrap and
`make check` passes with no new fails.
Tested `make check` on glibc built with gcc-9 with and without 
"-moutline-atomics"
and CFLAGS=" -O2 -g -fno-stack-protector -U_FORTIFY_SOURCE".

Ok to commit to gcc-9 branch?


Since this feature enables backwards-compatible deployment of LSE 
atomics, I'd support that.


That is okay with me in principle after GCC 9.3 is released (the branch 
is currently frozen).


However, there have been a few follow-up patches to fix some bugs 
revealed by testing.


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

and

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

come to mind.

Can you please make sure the fixes for those are included as well?




Does this mechanical `git am *.patch` require a copyright assignment?
I am still working with my employer on getting the FSF assignment signed.

Thanks,
Sebastian

PS: For gcc-8 backports there are 5 cleanup and improvement patches
that are needed for -moutline-atomics patches to apply cleanly.
Should these patches be back-ported in the same time as the flag patches,
or should I update the patches to apply to the older code base?


Hmm... normally I'd be for them. In this case I'd want to make sure that 
there aren't any fallout fixes that we're missing.


Did these patches have any bug reports against them?

Thanks,

Kyrill



Here is the list of the extra patches:

 From 77f33f44baf24c22848197aa80962c003dd7b3e2 Mon Sep 17 00:00:00 2001
From: Richard Henderson 
Date: Wed, 31 Oct 2018 09:29:29 +
Subject: [PATCH] aarch64: Simplify LSE cas generation

The cas insn is a single insn, and if expanded properly need not
be split after reload.  Use the proper inputs for the insn.

 * config/aarch64/aarch64.c (aarch64_expand_compare_and_swap):
 Force oldval into the rval register for TARGET_LSE; emit the compare
 during initial expansion so that it may be deleted if unused.
 (aarch64_gen_atomic_cas): Remove.
 * config/aarch64/atomics.md (@aarch64_compare_and_swap_lse):
 Change = to +r for operand 0; use match_dup for operand 2;
 remove is_weak and mod_f operands as unused.  Drop the split
 and merge with...
 (@aarch64_atomic_cas): ... this pattern's output; remove.
 (@aarch64_compare_and_swap_lse): Similarly.
 (@aarch64_atomic_cas): Similarly.

From-SVN: r265656

 From d400fda3a8c3330f77eb9d51874f5482d3819a9f Mon Sep 17 00:00:00 2001
From: Richard Henderson 
Date: Wed, 31 Oct 2018 09:42:39 +
Subject: [PATCH] aarch64: Improve cas generation

Do not zero-extend the input to the cas for subword operations;
instead, use the appropriate zero-extending compare insns.
Correct the predicates and constraints for immediate expected operand.

 * config/aarch64/aarch64.c (aarch64_gen_compare_reg_maybe_ze): New.
 (aarch64_split_compare_and_swap): Use it.
 (aarch64_expand_compare_and_swap): Likewise.  Remove convert_modes;
 test oldval against the proper predicate.
 * config/aarch64/atomics.md (@atomic_compare_and_swap):
 Use nonmemory_operand for expected.
 (cas_short_expected_pred): New.
 (@aarch64_compare_and_swap): Use it; use "rn" not "rI" to match.
 (@aarch64_compare_and_swap): Use "rn" not "rI" for expected.
 * config/aarch64/predicates.md (aarch64_plushi_immediate): New.
 (aarch64_plushi_operand): New.

From-SVN: r265657

 From 8f5603d363a4e0453d2c38c7103aeb0bdca85c4e Mon Sep 17 00:00:00 2001
From: Richard Henderson 
Date: Wed, 31 Oct 2018 09:47:21 +
Subject: [PATCH] aarch64: Improve swp generation

Allow zero as an input; fix constraints; avoid unnecessary split.

 * config/aarch64/aarch64.c (aarch64_emit_atomic_swap): Remove.
 (aarch64_gen_atomic_ldop): Don't call it.
 * config/aarch64/atomics.md (atomic_exchange):
 Use aarch64_reg_or_zero.
 (aarch64_atomic_exchange): Likewise.
 (aarch64_atomic_exchange_lse): Remove split; remove & from
 operand 0; use aarch64_reg_or_zero for input; merge ...
 (@aarch64_atomic_swp): ... this and remove.

From-SVN: r265659

 From 7803ec5ee2a547043fb6708a08ddb1361ba91202 Mon Sep 17 00:00:00 2001
From: Richard Henderson 
Date: Wed, 31 Oct 2018 09:58:48 +
Subject: [PATCH] aarch64: Improve atomic-op lse generation

Fix constraints; avoid unnecessary split.  Drop the use of the atomic_op
iterator in favor of the ATOMIC_LDOP iterator; this is simplier and more
logical for ldclr aka bic.

 * config/aarch64/aarch64.c (aarch64_emit_bic): Remove.
 (aarch64_atomic_ldop_supported_p): Remove.
 (aarch64_gen_atomic_ldop): Remove.
 * config/aarch64/atomic.md (atomic_):
 Fully expand LSE operations here.
 (atomic_fetch_): Likewise.
 

ODR violation in ranges

[Nathan Sidwell]

Jonathan,
the ranges header contains code like:
inline constexpr __adaptor::_RangeAdaptorClosure all
  = []  (_Range&& __r)
  {
 if constexpr (view>)
   return std::forward<_Range>(__r);
 else if constexpr (requires { ref_view{std::forward<_Range>(__r)}; })
   return ref_view{std::forward<_Range>(__r)};
 else
   return subrange{std::forward<_Range>(__r)};
  };

(line 1236)

When you strip away all the templateyness, you have:


inline constexpr auto all = [] () {};


That's an ODR violation -- the initializers in different TUs are not the 
same!


As you can guess, I can't turn this into a header unit (well, I can, but 
merging duplicates complains at you)


nathan
--
Nathan Sidwell

Re: [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121]

[Kyrill Tkachov]

Hi Jakub,

On 3/11/20 7:22 AM, Jakub Jelinek wrote:

Hi!

abs_hwi asserts that the argument is not HOST_WIDE_INT_MIN and as the
(invalid) testcase shows, the function can be called with such an offset.
The following patch is IMHO minimal fix, absu_hwi unlike abs_hwi 
allows even

that value and will return (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN
in that case.  The function then uses moffset in two spots which wouldn't
care if the value is (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN or
HOST_WIDE_INT_MIN and wouldn't accept it (!moffset and
aarch64_uimm12_shift (moffset)), then in one spot where the signedness of
moffset does matter and using unsigned is the right thing -
moffset < 0x100 - and finally has code which will handle even this
value right; the assembler doesn't really care for DImode immediates if
    mov x1, -9223372036854775808
or
    mov x1, 9223372036854775808
is used and similarly it doesn't matter if we add or sub it in DImode.

Bootstrapped/regtested on aarch64-linux, ok for trunk?


Ok.

Thanks,

Kyrill



2020-03-10  Jakub Jelinek  

    PR target/94121
    * config/aarch64/aarch64.c (aarch64_add_offset_1): Use absu_hwi
    instead of abs_hwi, change moffset type to unsigned HOST_WIDE_INT.

    * gcc.dg/pr94121.c: New test.

--- gcc/config/aarch64/aarch64.c.jj 2020-02-28 17:33:03.414258503 
+0100
+++ gcc/config/aarch64/aarch64.c    2020-03-10 17:01:39.435302124 
+0100

@@ -3713,7 +3713,7 @@ aarch64_add_offset_1 (scalar_int_mode mo
   gcc_assert (emit_move_imm || temp1 != NULL_RTX);
   gcc_assert (temp1 == NULL_RTX || !reg_overlap_mentioned_p (temp1, 
src));


-  HOST_WIDE_INT moffset = abs_hwi (offset);
+  unsigned HOST_WIDE_INT moffset = absu_hwi (offset);
   rtx_insn *insn;

   if (!moffset)
--- gcc/testsuite/gcc.dg/pr94121.c.jj   2020-03-10 16:58:40.246974306 
+0100
+++ gcc/testsuite/gcc.dg/pr94121.c  2020-03-10 16:58:40.246974306 
+0100

@@ -0,0 +1,16 @@
+/* PR target/94121 */
+/* { dg-do compile { target pie } } */
+/* { dg-options "-O2 -fpie -w" } */
+
+#define DIFF_MAX __PTRDIFF_MAX__
+#define DIFF_MIN (-DIFF_MAX - 1)
+
+extern void foo (char *);
+extern char v[];
+
+void
+bar (void)
+{
+  char *p = v;
+  foo ([DIFF_MIN]);
+}

    Jakub

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Martin Liška]

On 3/10/20 1:07 PM, Martin Liška wrote:

On 3/10/20 12:24 PM, Richard Biener wrote:

Not sure how symtab is encoded right now but we also could have


Ok, right now I don't see symtab entry much extensible.

But what am I suggesting is to parse LTO bytecode version and then
process conditional parsing of lto_symtab section.

Thoughts?
Martin


So as H.J. correctly pointed I can't add new symbol_type into ld_plugin_symbol 
struct.
It would make ABI change as correctly identified by abidiff:

abidiff /tmp/before.o /tmp/after.o
Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 0 Removed, 0 Added function symbol not 
referenced by debug info
Variable symbols changes summary: 0 Removed, 1 Added variable symbol not 
referenced by debug info

1 function with some indirect sub-type change:

  [C]'function ld_plugin_status onload(ld_plugin_tv*)' at lto-plugin.c:1275:1 
has some indirect sub-type changes:
parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
  in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
type size hasn't changed
1 data member changes (1 filtered):
 type of 'union {int tv_val; const char* tv_string; 
ld_plugin_register_claim_file tv_register_claim_file; 
ld_plugin_register_all_symbols_read tv_register_all_symbols_read; 
ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols 
tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; ld_plugin_add_input_file 
tv_add_input_file; ld_plugin_message tv_message; ld_plugin_get_input_file 
tv_get_input_file; ld_plugin_get_view tv_get_view; ld_plugin_release_input_file 
tv_release_input_file; ld_plugin_add_input_library tv_add_input_library; 
ld_plugin_set_extra_library_path tv_set_extra_library_path; 
ld_plugin_get_input_section_count tv_get_input_section_count; 
ld_plugin_get_input_section_type tv_get_input_section_type; 
ld_plugin_get_input_section_name tv_get_input_section_name; 
ld_plugin_get_input_section_contents tv_get_input_section_contents; 
ld_plugin_update_section_order tv_update_section_order; 
ld_plugin_allow_section_ordering tv_allow_section_ordering; 
ld_plugin_allow_unique_segment_for_sections 
tv_allow_unique_segment_for_sections; ld_plugin_unique_segment_for_sections 
tv_unique_segment_for_sections; ld_plugin_get_input_section_alignment 
tv_get_input_section_alignment; ld_plugin_get_input_section_size 
tv_get_input_section_size; ld_plugin_register_new_input tv_register_new_input; 
ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} ld_plugin_tv::tv_u' changed:
   type size hasn't changed
   1 data member changes (1 filtered):
type of 'ld_plugin_add_symbols tv_add_symbols' changed:
  underlying type 'enum ld_plugin_status (void*, int, const 
ld_plugin_symbol*)*' changed:
in pointed to type 'function type enum ld_plugin_status (void*, 
int, const ld_plugin_symbol*)':
  parameter 3 of type 'const ld_plugin_symbol*' has sub-type 
changes:
in pointed to type 'const ld_plugin_symbol':
  in unqualified underlying type 'struct ld_plugin_symbol' 
at plugin-api.h:86:1:
type size changed from 256 to 288 (in bits)
1 data member insertion:
  'int ld_plugin_symbol::symbol_type', at offset 256 
(in bits) at plugin-api.h:95:1

So that I need to come up with ld_plugin_symbol_v2. It brings more challenges: 
one has 2 parallel symbol
tables:

struct plugin_symtab
{
...
  struct ld_plugin_symbol_v2 *syms;
  struct ld_plugin_symbol *syms_v1;
...
};

and the information of these should by aligned.

The patch can survive lto.exp and I would like to ask H.J. to write bintuils 
counterpart that will
utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.

Thoughts?
Martin


>From 1271e9cc487847c1aa07e5bc0470e8c5b71900e9 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Fri, 6 Mar 2020 18:09:35 +0100
Subject: [PATCH] API extension for binutils (type of symbols).

gcc/ChangeLog:

2020-03-11  Martin Liska  

	* lto-streamer-out.c (write_symbol): Stream type
	of symbol (function, variable - bss/data).

include/ChangeLog:

2020-03-11  Martin Liska  

	* lto-symtab.h (enum gcc_plugin_symbol_type):
	New.
	* plugin-api.h (struct ld_plugin_symbol_v2): New
	ld_plugin_symbol structure with added symbol_type.
	(enum ld_plugin_symbol_type): New.
	(ld_plugin_add_symbols_v2): New.
	(ld_plugin_get_symbols_v2): New.
	(ld_plugin_tag): Add LDPT_GET_SYMBOLS_V4
	and LDPT_ADD_SYMBOLS_V2.
	(struct ld_plugin_tv): Add tv_add_symbols_v2
	and tv_get_symbols_v4.

lto-plugin/ChangeLog:

2020-03-11  Martin Liska  

	* lto-plugin.c (LTO_LTO_PREFIX): New.
	(LTO_LTO_PREFIX_LEN): Likewise.
	(struct plugin_symtab): Make syms of new
	type ld_plugin_symbol_v2 and add syms_v1
	for older API hooks.
	(struct lto_section): New.
	

Re: [PING PATCH coroutines] Do not strip cleanup_point when promote temporaries out of current stmt

[Iain Sandoe]

Bin.Cheng  wrote:

> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe  wrote:

>> With current trunk + Bin’s two approved patches.
>> 
>> I see no change in the testcase (lambda-09-capture-object.C) before / after
>> the patch
>>  (it fails for me at -O0 only - in both cases).
> 

> I tried exactly what you did, however, the result is different.

I am a bit concerned that we get different results - yesterday I retested this 
on:
  Linux : x86_64-linux (cfarm gcc122) 
  Darwin (x86_64-darwin16), 
 with the same results on both platforms.

1) I applied the two testcases (note I have renamed lambda-09-capture-object.C 
=> lambda-11-capture-object.C so that the test numbers are sequential).  
However, I have not changed the test in any other way.

Native configuration is x86_64-pc-linux-gnu

=== g++ tests ===

Schedule of variations:
unix/-m32
unix/-m64

Running target unix/-m32
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as 
tool-and-target-specific interface file.
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
 ...
FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler 
error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
errors)
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
excess errors)

Running target unix/-m64
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as 
tool-and-target-specific interface file.
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
 ...
FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler 
error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
errors)
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
excess errors)

^^ so, this shows that both tests fail (co-await-syntax-10.C is expected)

2) I apply Bin’s patch (Pickup more CO_AWAIT_EXPR expanding cases) (which is 
approved)

Native configuration is x86_64-pc-linux-gnu

=== g++ tests ===

Schedule of variations:
unix/-m32
unix/-m64

Running target unix/-m32
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as 
tool-and-target-specific interface file.
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler 
error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
errors)
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
excess errors)

Running target unix/-m64
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as 
tool-and-target-specific interface file.
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler 
error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
errors)
Running 
/home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
 ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
excess errors)

^^^ this shows that co-await-syntax-10.C is now OK (which is why I was 

Re: [patch] Fix middle-end/93961

[Richard Biener via Gcc-patches]

On Wed, Mar 11, 2020 at 8:56 AM Eric Botcazou  wrote:
>
> Hi,
>
> this is a GIMPLE verification failure in LTO mode on Ada code:
>
> /vol/gcc/src/hg/master/local/gcc/testsuite/gnat.dg/lto24_pkg1.ads:9:8: error:
> type mismatch in 'component_ref'
> lto24_pkg1__rec___b___XVN
>
> lto24_pkg1__rec___b___XVN
>
> The __XVN fields are the fields with QUAL_UNION_TYPE in Ada, which is the only
> language using this type.  The issue is that tree_is_indexable doesn't return
> the same result for a FIELD_DECL with QUAL_UNION_TYPE and the QUAL_UNION_TYPE,
> resulting in two instances of the QUAL_UNION_TYPE in the bytecode.  The result
> for the type is the correct one (false, since it is variably modified) while
> the result for the field is falsely true because:
>
>   else if (TREE_CODE (t) == FIELD_DECL
>&& lto_variably_modified_type_p (DECL_CONTEXT (t)))
> return false;
>
> is not satisfied.  The reason for this is that the DECL_QUALIFIER of fields of
> a QUAL_UNION_TYPE depends on a discriminant in Ada, which means that the size
> of the type does too (CONTAINS_PLACEHOLDER_P), which in turn means that it is
> reset to a mere PLACEHOLDER_EXPR by free_lang_data, which finally means that
> the size of DECL_CONTEXT is too, so RETURN_TRUE_IF_VAR is false.
>
> In other words, the CONTAINS_PLACEHOLDER_P property of the DECL_QUALIFIER of
> fields of a QUAL_UNION_TYPE hides the variably_modified_type_p property of
> these fields, if you look from the outside.
>
> This was clearly overlooked (by me) when the handling of PLACEHOLDER_EXPR was
> added to LTO a decade ago, but I think that it's now too late to fundamentally
> change how this is done, so I propose the attached fix.
>
> Tested on SPARC/Solaris and x86-64/Linux, OK for the mainline?  It's not a
> regression, but the fix is a no-op except for Ada and we have been using it
> internally for some time without any issue so far.

OK.  Note I wondered for some time whether we can pre-compute (and LTO stream)
whether a type is variably modified during type layout?

Thanks,
Richard.

>
> 2020-03-11  Eric Botcazou  
>
> PR middle-end/93961
> * tree.c (variably_modified_type_p) : Recurse into fields
> whose type is a qualified union.
>
> --
> Eric Botcazou

Re: [PATCH] value-prof: Fix abs uses in value-prof.c [PR93962]

[Richard Biener]

On Wed, 11 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> Jeff has recently fixed dump_histogram_value to use std::abs instead of abs,
> because on FreeBSD apparently the ::abs isn't overloaded and only has
> int abs (int);
> Seems on Solaris /usr/include/iso/stdlib_iso.h abs has:
> int abs (int);
> long abs (long);
> overloads but already not
> long long abs (long long);
> and there is another abs use in get_nth_most_common_value, also on int64_t.
> The long long std::abs (long long); overload is there only in C++11 and we
> in GCC10 still support C++98.
> 
> Martin has said that a counter should never be INT64_MIN, so IMHO it is
> better to use abs_hwi which will assert that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK but using absu_hwi and an unsigned format would have been safer I 
guess.

Thanks,
Richard.

> 2020-03-11  Jakub Jelinek  
> 
>   PR bootstrap/93962
>   * value-prof.c (dump_histogram_value): Use abs_hwi instead of
>   std::abs.
>   (get_nth_most_common_value): Use abs_hwi instead of abs.
> 
> --- gcc/value-prof.c.jj   2020-03-05 07:58:02.693135980 +0100
> +++ gcc/value-prof.c  2020-03-10 15:51:14.094854715 +0100
> @@ -266,7 +266,7 @@ dump_histogram_value (FILE *dump_file, h
> if (hist->hvalue.counters)
>   {
> fprintf (dump_file, " all: %" PRId64 "%s, values: ",
> -std::abs ((int64_t) hist->hvalue.counters[0]),
> +(int64_t) abs_hwi (hist->hvalue.counters[0]),
>  hist->hvalue.counters[0] < 0
>  ? " (values missing)": "");
> for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
> @@ -743,7 +743,7 @@ get_nth_most_common_value (gimple *stmt,
>*count = 0;
>*value = 0;
>  
> -  gcov_type read_all = abs (hist->hvalue.counters[0]);
> +  gcov_type read_all = abs_hwi (hist->hvalue.counters[0]);
>  
>gcov_type v = hist->hvalue.counters[2 * n + 1];
>gcov_type c = hist->hvalue.counters[2 * n + 2];
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH] dfp: Fix decimal_to_binary [PR94111]

[Richard Biener]

On Wed, 11 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> As e.g. decimal_from_decnumber shows, the REAL_VALUE_TYPE representation
> contains a decimal128 embedded in ->sig only if it is rvc_normal, for
> other kinds like rvc_inf or rvc_nan, ->sig is ignored and everything is
> contained in the REAL_VALUE_TYPE flags (cl, sign, signalling and decimal).
> decimal_to_binary which is used when folding a decimal{32,64,128} constant
> to a binary floating point type ignores this and thus folds infinities and
> NaNs into +0.0.
> The following patch fixes that by only doing that for rvc_normal.
> Similarly to the binary to decimal folding, it goes through a string, in
> order to e.g. deal with canonical NaN mantissas, or binary float formats
> that don't support infinities and/or NaNs.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2020-03-11  Jakub Jelinek  
> 
>   PR middle-end/94111
>   * dfp.c (decimal_to_binary): Only use decimal128ToString if from->cl
>   is rvc_normal, otherwise use real_to_decimal to print the number to
>   string.
> 
>   * gcc.dg/dfp/pr94111.c: New test.
> 
> --- gcc/dfp.c.jj  2020-01-12 11:54:36.530411644 +0100
> +++ gcc/dfp.c 2020-03-10 12:32:07.246961100 +0100
> @@ -342,9 +342,13 @@ decimal_to_binary (REAL_VALUE_TYPE *to,
>  const real_format *fmt)
>  {
>char string[256];
> -  const decimal128 *const d128 = (const decimal128 *) from->sig;
> -
> -  decimal128ToString (d128, string);
> +  if (from->cl == rvc_normal)
> +{
> +  const decimal128 *const d128 = (const decimal128 *) from->sig;
> +  decimal128ToString (d128, string);
> +}
> +  else
> +real_to_decimal (string, from, sizeof (string), 0, 1);
>real_from_string3 (to, string, fmt);
>  }
>  
> --- gcc/testsuite/gcc.dg/dfp/pr94111.c.jj 2020-03-10 12:38:20.175451924 
> +0100
> +++ gcc/testsuite/gcc.dg/dfp/pr94111.c2020-03-10 12:38:12.832560341 
> +0100
> @@ -0,0 +1,12 @@
> +/* PR middle-end/94111 */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +int
> +main ()
> +{
> +  _Decimal32 d = (_Decimal32) __builtin_inff ();
> +  if (!__builtin_isinf ((double) d))
> +__builtin_abort ();
> +  return 0;
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH] ldist: Further fixes for -ftrapv [PR94114]

[Richard Biener]

On Wed, 11 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> As the testcase shows, arithmetics that for -ftrapv would need multiple
> basic blocks can show up not just in nb_bytes expressions where we
> are calling rewrite_to_non_trapping_overflow for a while already,
> but also in the pointer expression to the start of the region.
> While the testcase covers just the first hunk and I've failed to create
> a testcase for the latter, it is at least in theory possible too, so I've
> adjusted that hunk too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2020-03-11  Jakub Jelinek  
> 
>   PR tree-optimization/94114
>   * tree-loop-distribution.c (generate_memset_builtin): Call
>   rewrite_to_non_trapping_overflow even on mem.
>   (generate_memcpy_builtin): Call rewrite_to_non_trapping_overflow even
>   on dest and src.
> 
>   * gcc.dg/pr94114.c: New test.
> 
> --- gcc/tree-loop-distribution.c.jj   2020-01-12 11:54:38.497381967 +0100
> +++ gcc/tree-loop-distribution.c  2020-03-10 08:20:42.830265273 +0100
> @@ -1151,7 +1151,7 @@ generate_memset_builtin (class loop *loo
>nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
>nb_bytes = force_gimple_operand_gsi (, nb_bytes, true, NULL_TREE,
>  false, GSI_CONTINUE_LINKING);
> -  mem = builtin->dst_base;
> +  mem = rewrite_to_non_trapping_overflow (builtin->dst_base);
>mem = force_gimple_operand_gsi (, mem, true, NULL_TREE,
> false, GSI_CONTINUE_LINKING);
>  
> @@ -1205,8 +1205,8 @@ generate_memcpy_builtin (class loop *loo
>nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
>nb_bytes = force_gimple_operand_gsi (, nb_bytes, true, NULL_TREE,
>  false, GSI_CONTINUE_LINKING);
> -  dest = builtin->dst_base;
> -  src = builtin->src_base;
> +  dest = rewrite_to_non_trapping_overflow (builtin->dst_base);
> +  src = rewrite_to_non_trapping_overflow (builtin->src_base);
>if (partition->kind == PKIND_MEMCPY
>|| ! ptr_derefs_may_alias_p (dest, src))
>  kind = BUILT_IN_MEMCPY;
> --- gcc/testsuite/gcc.dg/pr94114.c.jj 2020-03-10 08:21:07.369900379 +0100
> +++ gcc/testsuite/gcc.dg/pr94114.c2020-03-10 08:19:06.691694811 +0100
> @@ -0,0 +1,13 @@
> +/* PR tree-optimization/94114 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-loop-distribute-patterns -ftrapv" } */
> +
> +void
> +foo (int *x, int *y, int *z, long int w)
> +{
> +  while (y + w > z)
> +{
> +  x[w] = 0;
> +  --w;
> +}
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: Fix modulo-scheduler -fcompare-debug issues

[Richard Biener]

On Tue, 10 Mar 2020, Roman Zhuykov wrote:

> Hi!
> 
> Current modulo-sched implementation is a bit faulty from -fcompile-debug 
> perspective.
> 
> I found that few years ago, the most simple example is pr42631.c which fails 
> (with just -fmodulo-sched or together with -fmodulo-sched-allow-regmoves) on 
> powerpc64le with at least any gcc-4.9 or newer compiler.
> I've investigated that difference about 3 years ago, it is mostly technical, 
> dumps shows there are some "flying" NOTE_INSN_DELETED items.
> I understood that it is minor, and I planned to commit the fix only when my 
> other modulo-sched stuff will be ready.
> 
> But right now I see that when I enable -fmodulo-sched by default, powerpc64le 
> bootstrap give comparison failure as of r10-7056.
> 
> Comparing stages 2 and 3
> Bootstrap comparison failure!
> gcc/ggc-page.o differs
> 
> That doesn't happen on released branches, so it is a kind of "regression" 
> (certainly, nobody runs bootstrap with -fmodulo-sched).
> 
> Is that a good reason to commit the patch right now in stage4?

Yes from a RM perspective, no comments about the technical details of
the patch.

Richard.

> Patch was successfully regstrapped (based on r10-7056) using x86_64 and 
> powerpc64le, both with default options and with -fmodulo-sched enabled.
> 
> Roman
> 
> --
> modulo-sched: fix compare-debug issues
> 
> This patch fixes bootstrap comparison failure on powerpc64le when running it
> with -fmodulo-sched enabled.
> 
> When applying the schedule we have to move debug insns in the same
> way as we move note insns.  Also we have to discard adding debug insns
> to SCCs in DDG graph.
> 
> 20YY-MM-DD  Roman Zhuykov  
>
>   * ddg.c (create_ddg_dep_from_intra_loop_link): Adjust assertions.
>   (create_ddg_dep_no_link): Likewise.
>   (add_inter_loop_mem_dep): Do not create "debug --> non-debug" anti-deps.
>   (create_ddg): Adjust first_note field filling.
>   (check_sccs): Assert if any debug instruction is in SCC.
>   * modulo-sched.c (ps_first_note): Add an assertion if first_note
>   is empty.
> 
> testsuite:
> 
> 20YY-MM-DD  Roman Zhuykov  
> 
>   * gcc.dg/pr42631-sms1.c: New test.
>   * gcc.dg/pr42631-sms2.c: New test.
> 
> diff --git a/gcc/ddg.c b/gcc/ddg.c
> index ca8cb74823..048207a354 100644
> --- a/gcc/ddg.c
> +++ b/gcc/ddg.c
> @@ -185,8 +185,8 @@ create_ddg_dep_from_intra_loop_link (ddg_ptr g, 
> ddg_node_ptr src_node,
>else if (DEP_TYPE (link) == REG_DEP_OUTPUT)
>  t = OUTPUT_DEP;
>  
> -  gcc_assert (!DEBUG_INSN_P (dest_node->insn) || t == ANTI_DEP);
> -  gcc_assert (!DEBUG_INSN_P (src_node->insn) || t == ANTI_DEP);
> +  gcc_assert (NONDEBUG_INSN_P (dest_node->insn) || t == ANTI_DEP);
> +  gcc_assert (NONDEBUG_INSN_P (src_node->insn) || DEBUG_INSN_P 
> (dest_node->insn));
>  
>/* We currently choose not to create certain anti-deps edges and
>   compensate for that by generating reg-moves based on the life-range
> @@ -222,9 +222,9 @@ create_ddg_dep_from_intra_loop_link (ddg_ptr g, 
> ddg_node_ptr src_node,
>  }
>  }
>  
> -   latency = dep_cost (link);
> -   e = create_ddg_edge (src_node, dest_node, t, dt, latency, distance);
> -   add_edge_to_ddg (g, e);
> +  latency = dep_cost (link);
> +  e = create_ddg_edge (src_node, dest_node, t, dt, latency, distance);
> +  add_edge_to_ddg (g, e);
>  }
>  
>  /* The same as the above function, but it doesn't require a link parameter.  
> */
> @@ -237,8 +237,8 @@ create_ddg_dep_no_link (ddg_ptr g, ddg_node_ptr from, 
> ddg_node_ptr to,
>enum reg_note dep_kind;
>struct _dep _dep, *dep = &_dep;
>  
> -  gcc_assert (!DEBUG_INSN_P (to->insn) || d_t == ANTI_DEP);
> -  gcc_assert (!DEBUG_INSN_P (from->insn) || d_t == ANTI_DEP);
> +  gcc_assert (NONDEBUG_INSN_P (to->insn) || d_t == ANTI_DEP);
> +  gcc_assert (NONDEBUG_INSN_P (from->insn) || DEBUG_INSN_P (to->insn));
>  
>if (d_t == ANTI_DEP)
>  dep_kind = REG_DEP_ANTI;
> @@ -455,8 +455,12 @@ add_inter_loop_mem_dep (ddg_ptr g, ddg_node_ptr from, 
> ddg_node_ptr to)
>   return;
>else if (from->cuid != to->cuid)
>   {
> -   create_ddg_dep_no_link (g, from, to, ANTI_DEP, MEM_DEP, 1);
> -   if (DEBUG_INSN_P (from->insn) || DEBUG_INSN_P (to->insn))
> +   gcc_assert (NONDEBUG_INSN_P (to->insn));
> +
> +   if (NONDEBUG_INSN_P (from->insn))
> + create_ddg_dep_no_link (g, from, to, ANTI_DEP, MEM_DEP, 1);
> +
> +   if (DEBUG_INSN_P (from->insn))
>   create_ddg_dep_no_link (g, to, from, ANTI_DEP, MEM_DEP, 1);
> else
>   create_ddg_dep_no_link (g, to, from, TRUE_DEP, MEM_DEP, 1);
> @@ -607,28 +611,34 @@ create_ddg (basic_block bb, int closing_branch_deps)
>if (! INSN_P (insn))
>   {
> if (! first_note && NOTE_P (insn)
> -   && NOTE_KIND (insn) !=  NOTE_INSN_BASIC_BLOCK)
> +   && NOTE_KIND (insn) != NOTE_INSN_BASIC_BLOCK)
>   first_note = insn;
> continue;
>   }
> +
>

[PATCH] value-prof: Fix abs uses in value-prof.c [PR93962]

[Jakub Jelinek via Gcc-patches]

Hi!

Jeff has recently fixed dump_histogram_value to use std::abs instead of abs,
because on FreeBSD apparently the ::abs isn't overloaded and only has
int abs (int);
Seems on Solaris /usr/include/iso/stdlib_iso.h abs has:
int abs (int);
long abs (long);
overloads but already not
long long abs (long long);
and there is another abs use in get_nth_most_common_value, also on int64_t.
The long long std::abs (long long); overload is there only in C++11 and we
in GCC10 still support C++98.

Martin has said that a counter should never be INT64_MIN, so IMHO it is
better to use abs_hwi which will assert that.

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

2020-03-11  Jakub Jelinek  

PR bootstrap/93962
* value-prof.c (dump_histogram_value): Use abs_hwi instead of
std::abs.
(get_nth_most_common_value): Use abs_hwi instead of abs.

--- gcc/value-prof.c.jj 2020-03-05 07:58:02.693135980 +0100
+++ gcc/value-prof.c2020-03-10 15:51:14.094854715 +0100
@@ -266,7 +266,7 @@ dump_histogram_value (FILE *dump_file, h
  if (hist->hvalue.counters)
{
  fprintf (dump_file, " all: %" PRId64 "%s, values: ",
-  std::abs ((int64_t) hist->hvalue.counters[0]),
+  (int64_t) abs_hwi (hist->hvalue.counters[0]),
   hist->hvalue.counters[0] < 0
   ? " (values missing)": "");
  for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
@@ -743,7 +743,7 @@ get_nth_most_common_value (gimple *stmt,
   *count = 0;
   *value = 0;
 
-  gcov_type read_all = abs (hist->hvalue.counters[0]);
+  gcov_type read_all = abs_hwi (hist->hvalue.counters[0]);
 
   gcov_type v = hist->hvalue.counters[2 * n + 1];
   gcov_type c = hist->hvalue.counters[2 * n + 2];

Jakub

[PATCH] dfp: Fix decimal_to_binary [PR94111]

[Jakub Jelinek via Gcc-patches]

Hi!

As e.g. decimal_from_decnumber shows, the REAL_VALUE_TYPE representation
contains a decimal128 embedded in ->sig only if it is rvc_normal, for
other kinds like rvc_inf or rvc_nan, ->sig is ignored and everything is
contained in the REAL_VALUE_TYPE flags (cl, sign, signalling and decimal).
decimal_to_binary which is used when folding a decimal{32,64,128} constant
to a binary floating point type ignores this and thus folds infinities and
NaNs into +0.0.
The following patch fixes that by only doing that for rvc_normal.
Similarly to the binary to decimal folding, it goes through a string, in
order to e.g. deal with canonical NaN mantissas, or binary float formats
that don't support infinities and/or NaNs.

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

2020-03-11  Jakub Jelinek  

PR middle-end/94111
* dfp.c (decimal_to_binary): Only use decimal128ToString if from->cl
is rvc_normal, otherwise use real_to_decimal to print the number to
string.

* gcc.dg/dfp/pr94111.c: New test.

--- gcc/dfp.c.jj2020-01-12 11:54:36.530411644 +0100
+++ gcc/dfp.c   2020-03-10 12:32:07.246961100 +0100
@@ -342,9 +342,13 @@ decimal_to_binary (REAL_VALUE_TYPE *to,
   const real_format *fmt)
 {
   char string[256];
-  const decimal128 *const d128 = (const decimal128 *) from->sig;
-
-  decimal128ToString (d128, string);
+  if (from->cl == rvc_normal)
+{
+  const decimal128 *const d128 = (const decimal128 *) from->sig;
+  decimal128ToString (d128, string);
+}
+  else
+real_to_decimal (string, from, sizeof (string), 0, 1);
   real_from_string3 (to, string, fmt);
 }
 
--- gcc/testsuite/gcc.dg/dfp/pr94111.c.jj   2020-03-10 12:38:20.175451924 
+0100
+++ gcc/testsuite/gcc.dg/dfp/pr94111.c  2020-03-10 12:38:12.832560341 +0100
@@ -0,0 +1,12 @@
+/* PR middle-end/94111 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int
+main ()
+{
+  _Decimal32 d = (_Decimal32) __builtin_inff ();
+  if (!__builtin_isinf ((double) d))
+__builtin_abort ();
+  return 0;
+}

Jakub

[patch] Fix middle-end/93961

[Eric Botcazou]

Hi,

this is a GIMPLE verification failure in LTO mode on Ada code:

/vol/gcc/src/hg/master/local/gcc/testsuite/gnat.dg/lto24_pkg1.ads:9:8: error: 
type mismatch in 'component_ref'
lto24_pkg1__rec___b___XVN

lto24_pkg1__rec___b___XVN

The __XVN fields are the fields with QUAL_UNION_TYPE in Ada, which is the only 
language using this type.  The issue is that tree_is_indexable doesn't return 
the same result for a FIELD_DECL with QUAL_UNION_TYPE and the QUAL_UNION_TYPE, 
resulting in two instances of the QUAL_UNION_TYPE in the bytecode.  The result
for the type is the correct one (false, since it is variably modified) while 
the result for the field is falsely true because:

  else if (TREE_CODE (t) == FIELD_DECL
   && lto_variably_modified_type_p (DECL_CONTEXT (t)))
return false;

is not satisfied.  The reason for this is that the DECL_QUALIFIER of fields of 
a QUAL_UNION_TYPE depends on a discriminant in Ada, which means that the size 
of the type does too (CONTAINS_PLACEHOLDER_P), which in turn means that it is 
reset to a mere PLACEHOLDER_EXPR by free_lang_data, which finally means that 
the size of DECL_CONTEXT is too, so RETURN_TRUE_IF_VAR is false.

In other words, the CONTAINS_PLACEHOLDER_P property of the DECL_QUALIFIER of 
fields of a QUAL_UNION_TYPE hides the variably_modified_type_p property of 
these fields, if you look from the outside.

This was clearly overlooked (by me) when the handling of PLACEHOLDER_EXPR was 
added to LTO a decade ago, but I think that it's now too late to fundamentally 
change how this is done, so I propose the attached fix.

Tested on SPARC/Solaris and x86-64/Linux, OK for the mainline?  It's not a 
regression, but the fix is a no-op except for Ada and we have been using it 
internally for some time without any issue so far.


2020-03-11  Eric Botcazou  

PR middle-end/93961
* tree.c (variably_modified_type_p) : Recurse into fields
whose type is a qualified union.

-- 
Eric Botcazoudiff --git a/gcc/tree.c b/gcc/tree.c
index 66d52c71c99..905563fa4be 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -9206,8 +9206,18 @@ variably_modified_type_p (tree type, tree fn)
 	RETURN_TRUE_IF_VAR (DECL_SIZE (t));
 	RETURN_TRUE_IF_VAR (DECL_SIZE_UNIT (t));
 
+	/* If the type is a qualified union, then the DECL_QUALIFIER
+	   of fields can also be an expression containing a variable.  */
 	if (TREE_CODE (type) == QUAL_UNION_TYPE)
 	  RETURN_TRUE_IF_VAR (DECL_QUALIFIER (t));
+
+	/* If the field is a qualified union, then it's only a container
+	   for what's inside so we look into it.  That's necessary in LTO
+	   mode because the sizes of the field tested above have been set
+	   to PLACEHOLDER_EXPRs by free_lang_data.  */
+	if (TREE_CODE (TREE_TYPE (t)) == QUAL_UNION_TYPE
+		&& variably_modified_type_p (TREE_TYPE (t), fn))
+	  return true;
 	  }
   break;
 

[PATCH] ldist: Further fixes for -ftrapv [PR94114]

[Jakub Jelinek via Gcc-patches]

Hi!

As the testcase shows, arithmetics that for -ftrapv would need multiple
basic blocks can show up not just in nb_bytes expressions where we
are calling rewrite_to_non_trapping_overflow for a while already,
but also in the pointer expression to the start of the region.
While the testcase covers just the first hunk and I've failed to create
a testcase for the latter, it is at least in theory possible too, so I've
adjusted that hunk too.

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

2020-03-11  Jakub Jelinek  

PR tree-optimization/94114
* tree-loop-distribution.c (generate_memset_builtin): Call
rewrite_to_non_trapping_overflow even on mem.
(generate_memcpy_builtin): Call rewrite_to_non_trapping_overflow even
on dest and src.

* gcc.dg/pr94114.c: New test.

--- gcc/tree-loop-distribution.c.jj 2020-01-12 11:54:38.497381967 +0100
+++ gcc/tree-loop-distribution.c2020-03-10 08:20:42.830265273 +0100
@@ -1151,7 +1151,7 @@ generate_memset_builtin (class loop *loo
   nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
   nb_bytes = force_gimple_operand_gsi (, nb_bytes, true, NULL_TREE,
   false, GSI_CONTINUE_LINKING);
-  mem = builtin->dst_base;
+  mem = rewrite_to_non_trapping_overflow (builtin->dst_base);
   mem = force_gimple_operand_gsi (, mem, true, NULL_TREE,
  false, GSI_CONTINUE_LINKING);
 
@@ -1205,8 +1205,8 @@ generate_memcpy_builtin (class loop *loo
   nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
   nb_bytes = force_gimple_operand_gsi (, nb_bytes, true, NULL_TREE,
   false, GSI_CONTINUE_LINKING);
-  dest = builtin->dst_base;
-  src = builtin->src_base;
+  dest = rewrite_to_non_trapping_overflow (builtin->dst_base);
+  src = rewrite_to_non_trapping_overflow (builtin->src_base);
   if (partition->kind == PKIND_MEMCPY
   || ! ptr_derefs_may_alias_p (dest, src))
 kind = BUILT_IN_MEMCPY;
--- gcc/testsuite/gcc.dg/pr94114.c.jj   2020-03-10 08:21:07.369900379 +0100
+++ gcc/testsuite/gcc.dg/pr94114.c  2020-03-10 08:19:06.691694811 +0100
@@ -0,0 +1,13 @@
+/* PR tree-optimization/94114 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-loop-distribute-patterns -ftrapv" } */
+
+void
+foo (int *x, int *y, int *z, long int w)
+{
+  while (y + w > z)
+{
+  x[w] = 0;
+  --w;
+}
+}

Jakub

[PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121]

[Jakub Jelinek via Gcc-patches]

Hi!

abs_hwi asserts that the argument is not HOST_WIDE_INT_MIN and as the
(invalid) testcase shows, the function can be called with such an offset.
The following patch is IMHO minimal fix, absu_hwi unlike abs_hwi allows even
that value and will return (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN
in that case.  The function then uses moffset in two spots which wouldn't
care if the value is (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN or
HOST_WIDE_INT_MIN and wouldn't accept it (!moffset and
aarch64_uimm12_shift (moffset)), then in one spot where the signedness of
moffset does matter and using unsigned is the right thing -
moffset < 0x100 - and finally has code which will handle even this
value right; the assembler doesn't really care for DImode immediates if
mov x1, -9223372036854775808
or
mov x1, 9223372036854775808
is used and similarly it doesn't matter if we add or sub it in DImode.

Bootstrapped/regtested on aarch64-linux, ok for trunk?

2020-03-10  Jakub Jelinek  

PR target/94121
* config/aarch64/aarch64.c (aarch64_add_offset_1): Use absu_hwi
instead of abs_hwi, change moffset type to unsigned HOST_WIDE_INT.

* gcc.dg/pr94121.c: New test.

--- gcc/config/aarch64/aarch64.c.jj 2020-02-28 17:33:03.414258503 +0100
+++ gcc/config/aarch64/aarch64.c2020-03-10 17:01:39.435302124 +0100
@@ -3713,7 +3713,7 @@ aarch64_add_offset_1 (scalar_int_mode mo
   gcc_assert (emit_move_imm || temp1 != NULL_RTX);
   gcc_assert (temp1 == NULL_RTX || !reg_overlap_mentioned_p (temp1, src));
 
-  HOST_WIDE_INT moffset = abs_hwi (offset);
+  unsigned HOST_WIDE_INT moffset = absu_hwi (offset);
   rtx_insn *insn;
 
   if (!moffset)
--- gcc/testsuite/gcc.dg/pr94121.c.jj   2020-03-10 16:58:40.246974306 +0100
+++ gcc/testsuite/gcc.dg/pr94121.c  2020-03-10 16:58:40.246974306 +0100
@@ -0,0 +1,16 @@
+/* PR target/94121 */
+/* { dg-do compile { target pie } } */
+/* { dg-options "-O2 -fpie -w" } */
+
+#define DIFF_MAX __PTRDIFF_MAX__
+#define DIFF_MIN (-DIFF_MAX - 1)
+
+extern void foo (char *);
+extern char v[];
+
+void
+bar (void)
+{
+  char *p = v;
+  foo ([DIFF_MIN]);
+}

Jakub

Re: [PATCH] c++: Fix wrong conversion error with non-viable overload [PR94124]

[Jakub Jelinek via Gcc-patches]

On Tue, Mar 10, 2020 at 07:38:17PM -0400, Marek Polacek via Gcc-patches wrote:
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6062,6 +6062,13 @@ reshape_init_array_1 (tree elt_type, tree max_index, 
> reshape_iter *d,
>else if (last_nonzero < nelts - 1)
>   nelts = last_nonzero + 1;
>  
> +  /* Sharing a stripped constructor can get in the way of
> +  overload resolution.  E.g., initializing a class from
> +  {{0}} might be invalid while initializing the same class
> +  from {{}} might be valid.  */
> +  if (reuse)
> + new_init = unshare_constructor (new_init);
> +
>vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);

Isn't it wasteful to first copy perhaps a large constructor (recursively)
and then truncate it to very few elts (zero in this case)?
So, perhaps doing instead:
  if (reuse)
{
  vec *v = NULL;
  if (nelts)
vec_alloc (v, nelts);
  for (unsigned int i = 0; i < nelts; i++)
{
  constructor_elt elt = CONSTRUCTOR_ELT (new_init, i);
  if (TREE_CODE (elt.value) == CONSTRUCTOR)
elt.value = unshare_constructor (elt.value);
  v->quick_push (elt);
}
  new_init = build_constructor (TREE_TYPE (new_init), v);
}
  else
vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
?

Jakub

Re: [testsuite] Fix PR93935 to guard case under vect_hw_misalign

[Kewen.Lin via Gcc-patches]

Hi,

Gentle ping this patch, also request to backport to gcc9 after some burn-in 
time.

BR,
Kewen

on 2020/2/26 下午2:17, Kewen.Lin wrote:
> Hi,
> 
> This patch is to apply the same fix as r267528 to another similar case
> bb-slp-over-widen-2.c which requires misaligned vector access.
> 
> Verified it on ppc64-redhat-linux (Power7 BE).
> 
> Is it ok for trunk?
> 
> BR,
> Kewen
> ---
> 
> gcc/testsuite/ChangeLog
> 
> 2020-02-26  Kewen Lin  
> 
>   PR testsuite/93935
>   * gcc.dg/vect/bb-slp-over-widen-2.c: Expect basic block vectorized
>   messages only on vect_hw_misalign targets.
> 
> * patch *:
> 
> diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-over-widen-2.c 
> b/gcc/testsuite/gcc.dg/vect/bb-slp-over-widen-2.c
> index 3750fb7..042b7e9 100644
> --- a/gcc/testsuite/gcc.dg/vect/bb-slp-over-widen-2.c
> +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-over-widen-2.c
> @@ -63,4 +63,4 @@ main (void)
>  /* { dg-final { scan-tree-dump "demoting int to signed short" "slp2" { 
> target { ! vect_widen_shift } } } } */
>  /* { dg-final { scan-tree-dump "demoting int to unsigned short" "slp2" { 
> target { ! vect_widen_shift } } } } */
>  /* { dg-final { scan-tree-dump {\.AVG_FLOOR} "slp2" { target vect_avg_qi } } 
> } */
> -/* { dg-final { scan-tree-dump-times "basic block vectorized" 2 "slp2" } } */
> +/* { dg-final { scan-tree-dump-times "basic block vectorized" 2 "slp2" { 
> target vect_hw_misalign } } } */
>