Re: [PATCH] Modify combine pattern by a pseudo AND with its nonzero bits [PR93453]

2021-12-02 Thread HAO CHEN GUI via Gcc-patches
Kewen,
Many thanks for your comments.

On 2/12/2021 上午 10:21, Kewen.Lin wrote:
> Hi Haochen,
> 
> on 2021/12/1 下午5:01, HAO CHEN GUI via Gcc-patches wrote:
>> Hi,
>>   This patch modifies the combine pattern with a helper - 
>> change_pseudo_and_mask when recog fails.
>> The helper converts a single pseudo to the pseudo AND with a mask if the 
>> outer operator is IOR/XOR/PLUS
>> and the inner operator is ASHIFT/LSHIFTRT/AND. The conversion helps match 
>> shift + ior pattern.
>>
> 
> Thanks for improving this!
> 
> I would expect this patch can make us remove the below existing splitting,
> which was added for a similar purpose but only works for more insns
> combination.
> 
> (define_split
>   [(set (match_operand:GPR 0 "gpc_reg_operand")
>   (plus_ior_xor:GPR (ashift:GPR (match_operand:GPR 1 "gpc_reg_operand")
> (match_operand:SI 2 "const_int_operand"))
> (match_operand:GPR 3 "gpc_reg_operand")))]
>   "nonzero_bits (operands[3], mode)
>< HOST_WIDE_INT_1U << INTVAL (operands[2])"
>   [(set (match_dup 0)
>   (ior:GPR (and:GPR (match_dup 3)
> (match_dup 4))
>(ashift:GPR (match_dup 1)
>(match_dup 2]
> {
>   operands[4] = GEN_INT ((HOST_WIDE_INT_1U << INTVAL (operands[2])) - 1);
> })
> 
> 
Just tested it. The split pattern should be removed.

> As the description, this seems to check some special operations and mainly
> for those targets having Power style rotate* operations?  If the understanding
> is correct, not sure it's a good idea to make this under a target hook/macro?
> 
I am not sure if it benefits others. Waiting for Segher's comment.
> 
>>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. 
>> Is this okay for trunk?
>> Any recommendations? Thanks a lot.
>>
>> ChangeLog
>> 2021-12-01 Haochen Gui 
>>
>> gcc/
>>  * combine.c (change_pseudo_and_mask): New.
>>  (recog_for_combine): If recog fails, try again with the pattern
>>  modified by change_pseudo_and_mask.
>>
>> gcc/testsuite/
>>  * gcc.target/powerpc/20050603-3.c: Modify dump check conditions.
>>  * gcc.target/powerpc/rlwimi-2.c: Likewise.
>>
>>
>> patch.diff
>> diff --git a/gcc/combine.c b/gcc/combine.c
>> index 03e9a780919..82ee3b2e9db 100644
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -11539,6 +11539,37 @@ change_zero_ext (rtx pat)
>>return changed;
>>  }
>>
>> +/* When the outer code of set_src is IOR/XOR/PLUS and the inner code is
>> +   ASHIFT/LSHIFTRT/AND, convert a pseudo to psuedo AND with a mask if its
>> +   nonzero_bits is less than its mode mask. The nonzero_bits in other pass
>> +   doesn't return the same value as it does in combine pass.  */
>> +static bool
>> +change_pseudo_and_mask (rtx pat)
>> +{
>> +  bool changed = false;
> 
> Maybe we can remove this variable ...
> 
>> +
>> +  rtx src = SET_SRC (pat);
>> +  if ((GET_CODE (src) == IOR
>> +   || GET_CODE (src) == XOR
>> +   || GET_CODE (src) == PLUS)
>> +  && (((GET_CODE (XEXP (src, 0)) == ASHIFT
>> +|| GET_CODE (XEXP (src, 0)) == LSHIFTRT
>> +|| GET_CODE (XEXP (src, 0)) == AND)
>> +   && REG_P (XEXP (src, 1)
>> +{
>> +  rtx *reg = &XEXP (SET_SRC (pat), 1);
>> +  machine_mode mode = GET_MODE (*reg);
>> +  unsigned HOST_WIDE_INT nonzero = nonzero_bits (*reg, mode);
>> +  if (nonzero < GET_MODE_MASK (mode))
>> +{
>> +  rtx x = gen_rtx_AND (mode, *reg, GEN_INT (nonzero));
>> +  SUBST (*reg, x);
>> +  changed = true;
> 
> ..., directly return true here ...
> 
>> +}
>> + }
>> +  return changed;
> 
> and return false here.

Yeah, it saves a variable. I changed it.
> 
>> +}
>> +
>>  /* Like recog, but we receive the address of a pointer to a new pattern.
>> We try to match the rtx that the pointer points to.
>> If that fails, we may try to modify or replace the pattern,
>> @@ -11586,7 +11617,14 @@ recog_for_combine (rtx *pnewpat, rtx_insn *insn, 
>> rtx *pnotes)
>>  }
>>  }
>>else
>> -changed = change_zero_ext (pat);
>> +{
>> +  if (change_pseudo_and_mask (pat))
>> +{
>> +  maybe_swap_commutative_operands (SET_SRC (pat));
> 
> maybe we can call maybe_swap_commutative_operands directly
> in function change_pseudo_and_mask and make the code here compact.

Yes, I put the method into the helper. It looks good.
> 
> BR,
> Kewen
> 
>> +  changed = true;
>> +}
>> +  changed |= change_zero_ext (pat);
>> +}
>>  }
>>else if (GET_CODE (pat) == PARALLEL)
>>  {
>> diff --git a/gcc/testsuite/gcc.target/powerpc/20050603-3.c 
>> b/gcc/testsuite/gcc.target/powerpc/20050603-3.c
>> index 4017d34f429..e628be11532 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/20050603-3.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/20050603-3.c
>> @@ -12,7 +12,7 @@ void rotins (unsigned int x)
>>b.y = (x<<12) | (x>>20);
>>  }
>>
>> -/* 

[PATCH] [i386] Prefer INT_SSE_REGS for SSE_FLOAT_MODE_P in preferred_reload_class.

2021-12-02 Thread liuhongt via Gcc-patches
  The patch helps reload to choose GENENRAL_REGS alternatives for
SSE_FLOAT_MODE and enabled optimization like

-   vmovd   %xmm0, -4(%rsp)
-   movl$1, %eax
-   addl-4(%rsp), %eax
+   movd%xmm0, %eax
+   addl$1, %eax

Bootstrapped anf regtested on x86_64-pc-linux-gnu{-m32,} and
x86_64-pc-linux-gnu{-m32\ march=cascadelake,\ -march=cadcadelake}.

No big performace impact is abserved for SPEC2017 on ICX/CLX with both
Ofast -march=native -flto -funroll-loops and -O2 -mtune=generic options.

Ok for trunk?

gcc/ChangeLog:

PR target/95740
* config/i386/i386.c (ix86_preferred_reload_class): Prefer
INT_SSE_REGS for SSE_FLOAT_MODE_P.
* config/i386/i386.h (INT_SSE_CLASS_P): New.

gcc/testsuite/ChangeLog:

* gcc.target/i386/pr95740.c: New test.
---
 gcc/config/i386/i386.c  |  5 +++--
 gcc/config/i386/i386.h  |  2 ++
 gcc/testsuite/gcc.target/i386/pr95740.c | 26 +
 3 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr95740.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 80fee627358..977af1c31a7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -19194,9 +19194,10 @@ ix86_preferred_reload_class (rtx x, reg_class_t 
regclass)
   return NO_REGS;
 }
 
-  /* Prefer SSE regs only, if we can use them for math.  */
+  /* Prefer INT_SSE_REGS, enable reload from SSE register to GENERAL_REGS,
+ refer to PR95740.  */
   if (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH)
-return SSE_CLASS_P (regclass) ? regclass : NO_REGS;
+return INT_SSE_CLASS_P (regclass) ? regclass : NO_REGS;
 
   /* Generally when we see PLUS here, it's the function invariant
  (plus soft-fp const_int).  Which can only be computed into general
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 2fda1e0686e..ec90e47904b 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1283,6 +1283,8 @@ enum reg_class
   reg_class_subset_p ((CLASS), FLOAT_REGS)
 #define SSE_CLASS_P(CLASS) \
   reg_class_subset_p ((CLASS), ALL_SSE_REGS)
+#define INT_SSE_CLASS_P(CLASS) \
+  reg_class_subset_p ((CLASS), INT_SSE_REGS)
 #define MMX_CLASS_P(CLASS) \
   ((CLASS) == MMX_REGS)
 #define MASK_CLASS_P(CLASS) \
diff --git a/gcc/testsuite/gcc.target/i386/pr95740.c 
b/gcc/testsuite/gcc.target/i386/pr95740.c
new file mode 100644
index 000..9bc7b862787
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95740.c
@@ -0,0 +1,26 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-msse2 -O2 -mtune-ctrl=use_incdec -masm=att -mfpmath=sse" } */
+/* { dg-final { scan-assembler-times {(?n)movd[\t ]*%xmm0.*%eax} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)incl[\t ]*%eax} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)movq[\t ]*%xmm0.*%rax} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)incq[\t ]*%rax} 1 } } */
+
+int
+foo (float a)
+{
+  union{
+int b;
+float a;}u;
+  u.a = a;
+  return u.b + 1;
+}
+
+long long
+foo1 (double a)
+{
+  union{
+long long b;
+double a;}u;
+  u.a = a;
+  return u.b + 1;
+}
-- 
2.18.1



Re: [PATCH v2 1/2] c++: Print function template parms when relevant

2021-12-02 Thread Matthias Kretz
On Friday, 26 November 2021 16:23:43 CET Matthias Kretz wrote:
> > Hmm, since it walks DECL_TEMPLATE_RESULT, I wouldn't expect it to find
> > template parms that aren't in the function signature.
> 
> You were right, walking TREE_TYPE (DECL_TEMPLATE_RESULT (t)) does what I
> need.

... for FUNCTION_TYPE, but not for METHOD_TYPE. Also, even though I did 
regression testing, I found a few minor issues while looking for new 
regressions in the diagnose_as patch. Therefore, the revised patch (part 1):

--

The choice when to print a function template parameter was still
suboptimal. That's because sometimes the function template parameter
list only adds noise, while in other situations the lack of a function
template parameter list makes diagnostic messages hard to understand.

The general idea of this change is to print template parms wherever they
would appear in the source code as well. Thus, the diagnostics code
needs to know whether any template parameter was given explicitly.

DWARF names of functions use dump_function_name to produce a name
without function arguments (function arguments are printed from
dump_function_decl). However, FLAGS should still state the intent of
printing a name without function arguments (TFF_NO_FUNCTION_ARGUMENTS so
that dump_function_name can decide correctly whether to call
dump_template_parms.

Based on an initial patch from Jason Merrill .

Signed-off-by: Matthias Kretz 

gcc/testsuite/ChangeLog:

* g++.dg/debug/dwarf2/template-params-12n.C: Optionally, allow
DW_AT_default_value.
* g++.dg/diagnostic/default-template-args-1.C: New test.
* g++.dg/diagnostic/default-template-args-2.C: New test.
* g++.dg/diagnostic/default-template-args-3.C: New test.
* g++.dg/diagnostic/default-template-args-4.C: New test.
* g++.dg/diagnostic/default-template-args-5.C: New test.
* g++.dg/diagnostic/param-type-mismatch-2.C: Expect template
parms in diagnostic.
* g++.dg/ext/pretty1.C: Expect function template specialization
to not pretty-print template parms.
* g++.old-deja/g++.ext/pretty3.C: Ditto.
* g++.old-deja/g++.pt/memtemp77.C: Ditto.
* g++.dg/goacc/template.C: Expect function template parms for
explicit arguments.
* g++.dg/template/error40.C: Expect only non-default template
arguments in diagnostic.

gcc/cp/ChangeLog:

* constraint.cc (get_mapped_args): Remove incorrect non-default
args count on multi-level template args; instead set the
non-default args count on each inner TREE_VEC.
* cp-tree.h: Rewrite NON_DEFAULT_TEMPLATE_ARGS_COUNT
implementation to store the number explicitly specified
arguments in a TREE_LIST.
(EXPLICIT_TEMPLATE_ARGS_P): New.
(SET_EXPLICIT_TEMPLATE_ARGS_P): New.
(set_non_default_template_args_count): New declaration.
(get_non_default_template_args_count): New declaration.
(set_explicit_template_args_count): New declaration.
(get_explicit_template_args_count): New declaration.
(TFF_AS_PRIMARY): New constant.
* decl.c (grokfndecl): Mark all template arguments in a friend
declaration as explicitly specified.
* error.c (args_or_non_default_template_args_count): Renamed
from get_non_default_template_args_count (name-clash). Make
independent of flag_pretty_templates.
(dump_template_bindings): Add flags parameter to be passed to
get_non_default_template_args_count. Print only non-default
template arguments. Add used_parms parameter: print defaulted
template bindings if the parameter name is part of used_parms.
(dump_substitution): Walk the function_decl to find all used
template parameters. Non-static member functions need a special
case to not find all template parameters of enclosing scopes.
(dump_function_decl): Call dump_function_name and dump_type of
the DECL_CONTEXT with specialized template and set
TFF_AS_PRIMARY for their flags. Don't print template arguments.
dump_function_name already does so.
(dump_function_name): Add and document conditions for calling
dump_template_parms. Move DECL_USE_TEMPLATE to PRIMARY parameter
of dump_template_parms.
(dump_template_parms): Print only non-default template
parameters.
(lang_decl_name): Add TFF_NO_FUNCTION_ARGUMENTS to
dump_function_name flags.
* module.cc (trees_out::core_vals): Accept TREE_LISTs in
TREE_CHAIN of TREE_VEC.
* pt.c (set_non_default_template_args_count): New function.
(get_non_default_template_args_count): New function.
(set_explicit_template_args_count): New function.
(get_explicit_template_args_count): New function.
(expand_template_argument_pack): Always adjust and set
the adjusted non-default args count.

Re: [PATCH] [i386] Prefer INT_SSE_REGS for SSE_FLOAT_MODE_P in preferred_reload_class.

2021-12-02 Thread Hongtao Liu via Gcc-patches
On Thu, Dec 2, 2021 at 4:27 PM liuhongt  wrote:
>
>   The patch helps reload to choose GENENRAL_REGS alternatives for
> SSE_FLOAT_MODE and enabled optimization like
>
> -   vmovd   %xmm0, -4(%rsp)
> -   movl$1, %eax
> -   addl-4(%rsp), %eax
> +   movd%xmm0, %eax
> +   addl$1, %eax
>
> Bootstrapped anf regtested on x86_64-pc-linux-gnu{-m32,} and
> x86_64-pc-linux-gnu{-m32\ march=cascadelake,\ -march=cadcadelake}.
>
> No big performace impact is abserved for SPEC2017 on ICX/CLX with both
> Ofast -march=native -flto -funroll-loops and -O2 -mtune=generic options.
>
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/95740
> * config/i386/i386.c (ix86_preferred_reload_class): Prefer
> INT_SSE_REGS for SSE_FLOAT_MODE_P.
> * config/i386/i386.h (INT_SSE_CLASS_P): New.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr95740.c: New test.
> ---
>  gcc/config/i386/i386.c  |  5 +++--
>  gcc/config/i386/i386.h  |  2 ++
>  gcc/testsuite/gcc.target/i386/pr95740.c | 26 +
>  3 files changed, 31 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr95740.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 80fee627358..977af1c31a7 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -19194,9 +19194,10 @@ ix86_preferred_reload_class (rtx x, reg_class_t 
> regclass)
>return NO_REGS;
>  }
>
> -  /* Prefer SSE regs only, if we can use them for math.  */
> +  /* Prefer INT_SSE_REGS, enable reload from SSE register to GENERAL_REGS,
> + refer to PR95740.  */
>if (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH)
> -return SSE_CLASS_P (regclass) ? regclass : NO_REGS;
> +return INT_SSE_CLASS_P (regclass) ? regclass : NO_REGS;
>
>/* Generally when we see PLUS here, it's the function invariant
>   (plus soft-fp const_int).  Which can only be computed into general
> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> index 2fda1e0686e..ec90e47904b 100644
> --- a/gcc/config/i386/i386.h
> +++ b/gcc/config/i386/i386.h
> @@ -1283,6 +1283,8 @@ enum reg_class
>reg_class_subset_p ((CLASS), FLOAT_REGS)
>  #define SSE_CLASS_P(CLASS) \
>reg_class_subset_p ((CLASS), ALL_SSE_REGS)
> +#define INT_SSE_CLASS_P(CLASS) \
> +  reg_class_subset_p ((CLASS), INT_SSE_REGS)
>  #define MMX_CLASS_P(CLASS) \
>((CLASS) == MMX_REGS)
>  #define MASK_CLASS_P(CLASS) \
> diff --git a/gcc/testsuite/gcc.target/i386/pr95740.c 
> b/gcc/testsuite/gcc.target/i386/pr95740.c
> new file mode 100644
> index 000..9bc7b862787
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95740.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-msse2 -O2 -mtune-ctrl=use_incdec -masm=att -mfpmath=sse" } 
> */
> +/* { dg-final { scan-assembler-times {(?n)movd[\t ]*%xmm0.*%eax} 1 } } */
> +/* { dg-final { scan-assembler-times {(?n)incl[\t ]*%eax} 1 } } */
> +/* { dg-final { scan-assembler-times {(?n)movq[\t ]*%xmm0.*%rax} 1 } } */
> +/* { dg-final { scan-assembler-times {(?n)incq[\t ]*%rax} 1 } } */
> +
> +int
> +foo (float a)
> +{
> +  union{
> +int b;
> +float a;}u;
> +  u.a = a;
> +  return u.b + 1;
> +}
> +
> +long long
> +foo1 (double a)
> +{
> +  union{
> +long long b;
> +double a;}u;
> +  u.a = a;
> +  return u.b + 1;
> +}
> --
> 2.18.1
>


-- 
BR,
Hongtao


Re: [PATCH] rs6000: Builtins test changes for pr80315-*.c, pr88100.c

2021-12-02 Thread Segher Boessenkool
On Wed, Dec 01, 2021 at 04:42:19PM -0600, Bill Schmidt wrote:
> On 12/1/21 4:29 PM, Segher Boessenkool wrote:
> > On Thu, Nov 18, 2021 at 10:15:21AM -0600, Bill Schmidt wrote:
> >> All error messages are now one of the following:
> >>   "argument %d must be a %d-bit unsigned literal"
> >>   "argument %d must be a literal between %d and %d, inclusive"
> >>   "argument %d must be a variable or a literal between %d and %d, 
> >> inclusive"
> >>   "argument %d must be either a literal %d or a literal %d"
> >>
> >> These messages were chosen to require the fewest changes from previous
> >> messages while still introducing uniformity.  This patch adjusts error
> >> messages for some cases where this produces changed messages.
> >>
> >> Tested on powerpc64le-linux-gnu and powerpc64-linux-gnu (-m32/-m64) with
> >> no regressions.  is this okay for trunk?
> > We should have opnly the middle two of those messages.  But, okay for
> > trunk if you put this on some to-do list.  Thanks!
> 
> The last one is actually needed also, because we have at least one case 
> where the two supported values aren't contiguous.

Ah okay.  Nasty :-)

> We can do without the
> first one, but that will affect quite a number of test cases, so agree
> that this should be done later.  (We already had a whole lot of tests
> of this form.)

We never say "must be a 5-bit signed literal", although we have
instructions like that (vspltis[bhw] for example).  We also don't say
"unsigned 2-bit literal" or "unsigned 3-bit literal" because those are
just silly (almost as bad as "unsigned 1-bit literal" :-) )  And saying
"unsigned" keeps promulgating the confusion about what "unsigned"
*means* in C.


Segher


Re: [PR103149] introduce asmnesia internal function

2021-12-02 Thread Richard Biener via Gcc-patches
On Wed, 1 Dec 2021, Alexandre Oliva wrote:

> 
> This is now used by -fharden-conditional-branches and
> -fharden-compares to detach the values used in redundant hardening
> compares from their original locations.  It eventually still expands
> to an asm statement, as before, but the temporary is only introduced
> at expand time, preventing gimple optimizations from diverging input
> and output, which may prevent their unification in asm match operands.
> 
> Additional logic to check asm operands matches, and to keep them
> matchable, which may improve some cases of "+m" and "+X" in asm that
> are currently rejected because the single operand, split into input
> and output, ends up diverging in ways that can't be merged back by
> reload.
> 
> While investigating regressions hit by an earlier iteration of this
> patch, I came across pr93027.c, and noticed that, with optimization
> (unlike the test), reload overwrites one of the inputs with another.
> This preexisted this patch, and I have not attempted to fix it.
> 
> 
> The changes above paved the way to fix PR103149: a vectorized loop
> ends up using vector booleans on aarch64, and the hardening pass
> attempts to "copy-and-forget" them with an asm statement (thus
> ASMNESIA) for the redundant hardening compare.
> 
> The problem there is that vector booleans, on the affected platform,
> can't go in GENERAL_REGS, but the asm statement we used to emit could
> only use those registers or memory.
> 
> The work-around introduced in the newly-introduced ASMNESIA expander
> is to check whether the mode is suitable for GENERAL_REGS, and force
> the asm operands to use MEM otherwise.  This is not ideal, but
> functionality to map a mode to a constraint string that selects a
> suitable register class doesn't seem readily available.  Maybe in a
> future patch...
> 
> ASMNESIA currently works only with gimple registers of types with
> scalar modes.  I have not attempted to make it more general than that,
> though I envision future uses that might end up requiring it.  I'll
> cross that bridge if/when I get to it.
> 
> 
> Regstrapped on x86_64-linux-gnu.  Also bootstrapped along with a patch
> that enables both compare hardening passes.  Built crosses to aarch64-
> and riscv64-, and verified that the mentioned aarch64 PR is fixed.  Ok
> to install?

While adding ASMNESIA looks OK at this point, I'm not sure about the
asm handling stuff.  You mention 'reload' above, do you mean LRA?

Thanks,
Richard.

> 
> (Manual inspection of the asm output for riscv PR103302 suggests that
> the reported problem may be gone with the given testcase, but I have not
> built enough of the target environment to enable me to run that.)
> 
> 
> 
> for  gcc/ChangeLog
> 
>   PR middle-end/103149
>   * cfgexpand.c (expand_asm_stmt): Pass original input
>   constraint, and constraint array, to asm_operand_ok.
>   * gimple-hander-conditionals.cc (detach_value): Call
>   ASMNESIA internal function.
>   * internal-fn.c (expand_ASMNESIA): New.
>   * internal-fn.def (ASMNESIA): New.
>   * recog.c (asm_operand_ok): Turn into wrapper of...
>   (asm_operand_ok_match): ... renamed.  Skip non-constraint
>   characters faster.  Don't require register_operand for matched
>   operands.  Combine results with...
>   (check_match): New.
>   (check_asm_operands): Enable match checking.
> 
> for  gcc/testsuite/ChangeLog
> 
>   PR middle-end/103149
>   * gcc.target/aarch64/pr103149.c: New.
>   * gcc.target/i386/pr85030.c: Also accept impossible constraint
>   error.
> ---
>  gcc/cfgexpand.c |6 +
>  gcc/gimple-harden-conditionals.cc   |   17 
>  gcc/internal-fn.c   |   81 +++
>  gcc/internal-fn.def |4 +
>  gcc/recog.c |  114 
> +++
>  gcc/testsuite/gcc.target/aarch64/pr103149.c |   13 +++
>  gcc/testsuite/gcc.target/i386/pr85030.c |2 
>  7 files changed, 200 insertions(+), 37 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr103149.c
> 
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index fb84d469f1e77..f0272ba1ac9ba 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -3374,10 +3374,10 @@ expand_asm_stmt (gasm *stmt)
>tree val = input_tvec[i];
>tree type = TREE_TYPE (val);
>bool allows_reg, allows_mem, ok;
> -  const char *constraint;
> +  const char *constraint, *orig_constraint;
>rtx op;
>  
> -  constraint = constraints[i + noutputs];
> +  orig_constraint = constraint = constraints[i + noutputs];
>ok = parse_input_constraint (&constraint, i, ninputs, noutputs, 0,
>  constraints.address (),
>  &allows_mem, &allows_reg);
> @@ -3397,7 +3397,7 @@ expand_asm_stmt (gasm *stmt)
>else if (MEM_P (op))
>   op = validize_

[PATCH] PR target/43892: Some carry flag (CA) optimizations on PowerPC.

2021-12-02 Thread Roger Sayle
This patch resolves PR target/43892 (suboptimal add with carry) by
adding four new define_insn_and_split to the rs6000 backend, that all
recognize pairs of instructions where the first instruction sets the
carry flag and the second one consumes it.  It also adds a commutative
variant of add3_carry_in_0 (aka "addze") to catch cases, not
caught by recog's insn canonicalization, where CA_REG appears first.

For the add32carry function in the original PR:

unsigned int add32carry(unsigned int sum, unsigned int x)
{
  unsigned int z = sum + x;
  if (sum + x < x)
z++;
  return z;
}

previously "-O2 -m32" would generate:

add32carry:
add 3,3,4
subfc 4,4,3
subfe 9,9,9
subf 3,9,3
blr

with this patch we now generate:

add32carry:
addc 3,3,4
addze 3,3
blr

And for the related examples in the new test case,

unsigned long add_leu(unsigned long a, unsigned long b, unsigned long c) {
  return a + (b <= c);
}

unsigned long add_geu(unsigned long a, unsigned long b, unsigned long c) {
  return a + (b >= c);
}

On powerpc64 with -O2 we'd previously generate:

add_leu:
subfc 4,4,5
subfe 9,9,9
addi 9,9,1
add 3,9,3
blr
add_geu:
subfc 5,5,4
subfe 9,9,9
addi 9,9,1
add 3,9,3
blr

but with this patch we now generate:

add_leu:
subfc 4,4,5
addze 3,3
blr
add_geu:
subfc 5,5,4
addze 3,3
blr

This patch has been tested on powerpc64-unknown-linux-gnu (many thanks
to gcc203.fsffrance.org on the GCC compile farm) with a make bootstrap
and make -k check with now new failures.

Ok for mainline?


2021-12-02  Roger Sayle  

gcc/ChangeLog
PR target/43892
* config/rs6000/rs6000.md (*add3_carry_in_0_2): New
define_insn to recognize commutative form of add3_carry_in_0.
(*add3_geu, *add3_leu, *subf3_carry_in_xx_subf,
*add3_carry_in_addc): New define_insn_and_split patterns.

gcc/testsuite/ChangeLog
PR target/43892
* gcc.target/powerpc/addcmp.c: New test case.
* gcc.target/powerpc/pr43892.c: New test case.


Many thanks in advance.
Roger
--




[ping][PATCH] DWARF: Match behaviour of .cfi_xxx when doing manual frame output.

2021-12-02 Thread Iain Sandoe
Hi,

I’d like to ping this patch at least for opinion on whether the approach is
reasonable (as noted below it could be put behind a target hook if necessary)

thanks
Iain

> On 17 Nov 2021, at 17:17, Iain Sandoe via Gcc-patches 
>  wrote:
> 
> At present, for several reasons, it is not possible to switch
> Darwin to use .cfi instructions for frame output.
> 
> When GCC uses .cfi_ instructions, the behaviour w.r.t frame
> sections (for a target with unwind frames by defaults):
> 
> (no options ) .eh_frame
> (-g ) .eh_frame
> (-g -fno-unwind-tables -fno-asynchronous-unwind-tables) .debug_frame
> (   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---
> 
> However, for a target which outputs the FDEs "manually" (using
> output_call_frame_info()) we have:
> 
> (no options ) __eh_frame
> (-g ) __eh_frame *and* __debug_frame
> (-g -fno-unwind-tables -fno-asynchronous-unwind-tables) __debug_frame
> (   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---
> 
> The first two cases are, of course, the most common and the extra
> frame table is (a) a waste of space and (b) actually triggers a bug
> when used with the LLVM assembler [with assertions enabled] for
> Mach-O when we have hot/cold partitioning on, since that emits
> Letext{.cold}0 labels *after* the __DWARF,__debug_frame and the
> assembler is set up reject switches to non-debug sections after the
> first __DWARF debug one has been seen.
> 
> The following patch makes the manual output of frame data follow the
> same pattern as the .cfi instructions.
> 
> (a) From testing on Darwin which uses the 'manual frame output' I see
> around 200Mb saving on gcc/ for master (5%).
> (b) Since Darwin defaults to unwind frames for all languages, we see
> only eh_frame sections before the "real debug" is emitted, so that
> the LLVM constraint is avoided.
> 
> On testing on x86_64 and powerpc64le Linux, I see only a single test
> that would need amendment (it counts the number of references to the
> start/end local labels).
> 
> Since the majority of targets are using .cfi instructions, it is hard
> to get wider testing.
> 
> It would be possible, of course, to wrap the change in a target hook
> but it's not clear that we need to.
> 
> Is there some case that I've missed?
> or - OK for master (the testcase amendments are not attached here)
> but are simple.
> 
> thanks,
> Iain
> 
> Signed-off-by: Iain Sandoe 
> 
> gcc/ChangeLog:
> 
>   * dwarf2out.c (output_call_frame_info): Output the FDEs when
>   either EH or debug support is needed.
>   (dwarf2out_frame_finish): When either EH or debug support is
>   needed, call output_call_frame_info().
> ---
> gcc/dwarf2out.c | 15 ++-
> 1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index e1d6a79ecd7..96307d6747a 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -283,7 +283,7 @@ static GTY(()) dw_die_ref decltype_auto_die;
> 
> /* Forward declarations for functions defined in this file.  */
> 
> -static void output_call_frame_info (int);
> +static void output_call_frame_info (bool, bool);
> 
> /* Personality decl of current unit.  Used only when assembler does not 
> support
>personality CFI.  */
> @@ -750,7 +750,7 @@ fde_needed_for_eh_p (dw_fde_ref fde)
>location of saved registers.  */
> 
> static void
> -output_call_frame_info (int for_eh)
> +output_call_frame_info (bool for_eh, bool for_debug)
> {
>   unsigned int i;
>   dw_fde_ref fde;
> @@ -795,7 +795,7 @@ output_call_frame_info (int for_eh)
>   targetm.asm_out.emit_unwind_label (asm_out_file, fde->decl, 1, 1);
>   }
> 
> -  if (!any_eh_needed)
> +  if (!any_eh_needed && !for_debug)
>   return;
> }
> 
> @@ -1271,12 +1271,9 @@ void
> dwarf2out_frame_finish (void)
> {
>   /* Output call frame information.  */
> -  if (targetm.debug_unwind_info () == UI_DWARF2)
> -output_call_frame_info (0);
> -
> -  /* Output another copy for the unwinder.  */
> -  if (do_eh_frame)
> -output_call_frame_info (1);
> +  if (targetm.debug_unwind_info () == UI_DWARF2 || do_eh_frame)
> +output_call_frame_info (do_eh_frame,
> + targetm.debug_unwind_info () == UI_DWARF2);
> }
> 
> static void var_location_switch_text_section (void);
> -- 
> 2.24.3 (Apple Git-128)
> 



Re: [PATCH] [i386] Prefer INT_SSE_REGS for SSE_FLOAT_MODE_P in preferred_reload_class.

2021-12-02 Thread Uros Bizjak via Gcc-patches
On Thu, Dec 2, 2021 at 9:36 AM Hongtao Liu  wrote:
>
> On Thu, Dec 2, 2021 at 4:27 PM liuhongt  wrote:
> >
> >   The patch helps reload to choose GENENRAL_REGS alternatives for
> > SSE_FLOAT_MODE and enabled optimization like
> >
> > -   vmovd   %xmm0, -4(%rsp)
> > -   movl$1, %eax
> > -   addl-4(%rsp), %eax
> > +   movd%xmm0, %eax
> > +   addl$1, %eax
> >
> > Bootstrapped anf regtested on x86_64-pc-linux-gnu{-m32,} and
> > x86_64-pc-linux-gnu{-m32\ march=cascadelake,\ -march=cadcadelake}.
> >
> > No big performace impact is abserved for SPEC2017 on ICX/CLX with both
> > Ofast -march=native -flto -funroll-loops and -O2 -mtune=generic options.
> >
> > Ok for trunk?

Please also consider TARGET_INTER_UNIT_MOVES_TO_VEC and
TARGET_INTER_UNIT_MOVES_FROM_VEC.

Uros.

> >
> > gcc/ChangeLog:
> >
> > PR target/95740
> > * config/i386/i386.c (ix86_preferred_reload_class): Prefer
> > INT_SSE_REGS for SSE_FLOAT_MODE_P.
> > * config/i386/i386.h (INT_SSE_CLASS_P): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/i386/pr95740.c: New test.
> > ---
> >  gcc/config/i386/i386.c  |  5 +++--
> >  gcc/config/i386/i386.h  |  2 ++
> >  gcc/testsuite/gcc.target/i386/pr95740.c | 26 +
> >  3 files changed, 31 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr95740.c
> >
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index 80fee627358..977af1c31a7 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -19194,9 +19194,10 @@ ix86_preferred_reload_class (rtx x, reg_class_t 
> > regclass)
> >return NO_REGS;
> >  }
> >
> > -  /* Prefer SSE regs only, if we can use them for math.  */
> > +  /* Prefer INT_SSE_REGS, enable reload from SSE register to GENERAL_REGS,
> > + refer to PR95740.  */
> >if (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH)
> > -return SSE_CLASS_P (regclass) ? regclass : NO_REGS;
> > +return INT_SSE_CLASS_P (regclass) ? regclass : NO_REGS;
> >
> >/* Generally when we see PLUS here, it's the function invariant
> >   (plus soft-fp const_int).  Which can only be computed into general
> > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> > index 2fda1e0686e..ec90e47904b 100644
> > --- a/gcc/config/i386/i386.h
> > +++ b/gcc/config/i386/i386.h
> > @@ -1283,6 +1283,8 @@ enum reg_class
> >reg_class_subset_p ((CLASS), FLOAT_REGS)
> >  #define SSE_CLASS_P(CLASS) \
> >reg_class_subset_p ((CLASS), ALL_SSE_REGS)
> > +#define INT_SSE_CLASS_P(CLASS) \
> > +  reg_class_subset_p ((CLASS), INT_SSE_REGS)
> >  #define MMX_CLASS_P(CLASS) \
> >((CLASS) == MMX_REGS)
> >  #define MASK_CLASS_P(CLASS) \
> > diff --git a/gcc/testsuite/gcc.target/i386/pr95740.c 
> > b/gcc/testsuite/gcc.target/i386/pr95740.c
> > new file mode 100644
> > index 000..9bc7b862787
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr95740.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile { target { ! ia32 } } } */
> > +/* { dg-options "-msse2 -O2 -mtune-ctrl=use_incdec -masm=att -mfpmath=sse" 
> > } */
> > +/* { dg-final { scan-assembler-times {(?n)movd[\t ]*%xmm0.*%eax} 1 } } */
> > +/* { dg-final { scan-assembler-times {(?n)incl[\t ]*%eax} 1 } } */
> > +/* { dg-final { scan-assembler-times {(?n)movq[\t ]*%xmm0.*%rax} 1 } } */
> > +/* { dg-final { scan-assembler-times {(?n)incq[\t ]*%rax} 1 } } */
> > +
> > +int
> > +foo (float a)
> > +{
> > +  union{
> > +int b;
> > +float a;}u;
> > +  u.a = a;
> > +  return u.b + 1;
> > +}
> > +
> > +long long
> > +foo1 (double a)
> > +{
> > +  union{
> > +long long b;
> > +double a;}u;
> > +  u.a = a;
> > +  return u.b + 1;
> > +}
> > --
> > 2.18.1
> >
>
>
> --
> BR,
> Hongtao


[SVE] PR96463 - Optimise svld1rq from vectors

2021-12-02 Thread Prathamesh Kulkarni via Gcc-patches
Hi Richard,
I have attached a WIP untested patch for PR96463.
IIUC, the PR suggests to transform
lhs = svld1rq ({-1, -1, ...}, &v[0])
into:
lhs = vec_perm_expr
if v is vector of 4 elements, and each element is 32 bits on little
endian target ?

I am sorry if this sounds like a silly question, but I am not sure how
to convert a vector of type int32x4_t into svint32_t ? In the patch, I
simply used NOP_EXPR (which I expected to fail), and gave type error
during gimple verification:

svint32_t
foo (int32x4_t x)
{
  return svld1rq (svptrue_b8 (), &x[0]);
}

transformed to:
EMERGENCY DUMP:

svint32_t foo (int32x4_t x)
{
  svint32_t _3;
  __Int32x4_t _4;

   :
  _4 = VEC_PERM_EXPR ;
  _3 = (svint32_t) _4;
  return _3;

}

and ICE's with:
pr96463.c:8:1: error: invalid vector types in nop conversion
8 | }
  | ^
svint32_t
__Int32x4_t
_3 = (svint32_t) _4;
during GIMPLE pass: ccp

Could you please suggest how to proceed ?

Thanks,
Prathamesh
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc 
b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
index 02e42a71e5e..3834f33443a 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
@@ -44,6 +44,13 @@
 #include "aarch64-sve-builtins-shapes.h"
 #include "aarch64-sve-builtins-base.h"
 #include "aarch64-sve-builtins-functions.h"
+#include "print-tree.h"
+#include "gimple-pretty-print.h"
+
+/* ??? Including tree-ssanames.h requires including other header dependencies.
+   Just including the prototype for now.  */
+extern tree make_ssa_name_fn (struct function *, tree, gimple *,
+ unsigned int version = 0);
 
 using namespace aarch64_sve;
 
@@ -1207,6 +1214,52 @@ public:
 insn_code icode = code_for_aarch64_sve_ld1rq (e.vector_mode (0));
 return e.use_contiguous_load_insn (icode);
   }
+
+  gimple *
+  fold (gimple_folder &f) const OVERRIDE
+  {
+tree arg0 = gimple_call_arg (f.call, 0);
+tree arg1 = gimple_call_arg (f.call, 1);
+
+/* Transform:
+   lhs = svld1rq ({-1, -1, ... }, &v[0])
+   into:
+   tmp = vec_perm_expr.
+   lhs = nop_expr tmp
+   on little endian target.  */
+
+if (!BYTES_BIG_ENDIAN
+   && integer_all_onesp (arg0)
+   && TREE_CODE (arg1) == ADDR_EXPR)
+  {
+   tree t = TREE_OPERAND (arg1, 0);
+   if (TREE_CODE (t) == ARRAY_REF)
+ {
+   tree index = TREE_OPERAND (t, 1);
+   t = TREE_OPERAND (t, 0);
+   if (integer_zerop (index) && TREE_CODE (t) == VIEW_CONVERT_EXPR)
+ {
+   t = TREE_OPERAND (t, 0);
+   tree vectype = TREE_TYPE (t);
+   if (VECTOR_TYPE_P (vectype)
+   && known_eq (TYPE_VECTOR_SUBPARTS (vectype), 4u)
+   && wi::to_wide (TYPE_SIZE (vectype)) == 128)
+ {
+   tree new_temp = ::make_ssa_name_fn (cfun, vectype, NULL);
+   tree zero_vec = build_vector_from_val (vectype, index);
+   gimple *g = gimple_build_assign (new_temp, VEC_PERM_EXPR, 
t, t, zero_vec);
+   /* ??? How to convert between vector types if 
gimple_call_lhs (f.call) and
+  new_temp have different types ?  */
+   gimple *g2 = gimple_build_assign (gimple_call_lhs (f.call), 
NOP_EXPR, new_temp);
+   gsi_insert_before (f.gsi, g, GSI_SAME_STMT);
+   return g2;
+ }
+ }
+ }
+  }
+
+return NULL;
+  }
 };
 
 class svld1ro_impl : public load_replicate


Re: [PATCH] Allow loop crossing paths in back threader copier.

2021-12-02 Thread Richard Biener via Gcc-patches
On Wed, Dec 1, 2021 at 5:00 PM Aldy Hernandez  wrote:
>
> On Wed, Dec 1, 2021 at 2:36 PM Richard Biener
>  wrote:
> >
> > On Tue, Nov 30, 2021 at 4:48 PM Aldy Hernandez via Gcc-patches
> >  wrote:
> > >
> > > We are currently restricting loop crossing paths in the generic copier
> > > used by the back threader, but we should be able to handle them after
> > > loop_done has completed.
> >
> > But this isn't a cost thing but a correctness (as in, can we update
> > loops properly)
> > thing.  We are passing 'loop' down to copy_bbs and very much rely on
> > seeing either BBs of that loop or copying complete subloops of it.
> >
> > You will likely see excess loop fixup caused by this or ICEs in case the
> > caller of this function will not always set LOOPS_NEED_FIXUP
> > (not that copy_bbs handling of loops is perfect).
>
> Ughh, fair enough.
>
> In which case at least I've commented in the PR what the problem is
> here.  Perhaps we could benefit from a BB copier that could handle
> loops, which is way above my circle of competence ;-).

Well, more high level APIs exercising the copier are able to handle loops
but it really depends on the high-level transform what you need to do ;)

So for "loop-crossing" threadings it depends on the actual details what
the "loop transform" is that you are doing.  If you just rely on
LOOPS_NEED_FIXUP then for example a #pragma GCC unroll 0
might be lost and that is - counter to PROP_after_loop_opts - also
effective to prevent the RTL unroller from unrolling a loop.

What's probably OK is for example changing the code to only
prohibit copying loop headers:

+ /* We do not handle subloops, i.e. all the blocks must belong to the
+ same loop.  */
+   if (region[i]->loop_father != loop
 && region[i]->loop_father->header != region[i])
+ return false;

that should allow threadings covering an exit edge for example.  It's
reasonable (with some extra setup here!) to also handle the case
of entering and exiting a loop (the "subloop" the comment mentions),
but that requires us to do the loop structure copying/setup before
copying the BBs (maybe look at gimple_duplicate_sese_tail).

Note the above adjustment to handle exits does not necessarily result in correct
->loop_father assignment but LOOPS_NEED_FIXUP should be able to
deal with it without using loop annotations.  These kind of threads should also
be OK when loop opts are not finished yet.

Richard.


> Aldy
>


Re: [PATCH] Implement -fprofile-prefix-map.

2021-12-02 Thread Martin Liška

PING^1

On 11/11/21 17:39, Martin Liška wrote:

It's functionality that is analogous to -ffile-prefix-map, this time
for gcov purpose.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

 PR gcov-profile/96092

gcc/ChangeLog:

 * common.opt: New option.
 * coverage.c (coverage_begin_function): Emit filename with
 remap_profile_filename.
 * doc/invoke.texi: Document the new option.
 * file-prefix-map.c (add_profile_prefix_map): New.
 (remap_profile_filename): Likewise.
 * file-prefix-map.h (add_profile_prefix_map): Likewise.
 (remap_profile_filename): Likewise.
 * lto-opts.c (lto_write_options): Handle
 OPT_fprofile_prefix_map_.
 * opts-global.c (handle_common_deferred_options): Likewise.
 * opts.c (common_handle_option): Likewise.
 (gen_command_line_string): Likewise.
 * profile.c (output_location): Emit filename with
 remap_profile_filename.
---
  gcc/common.opt    |  4 
  gcc/coverage.c    |  3 ++-
  gcc/doc/invoke.texi   | 14 --
  gcc/file-prefix-map.c | 17 +
  gcc/file-prefix-map.h |  2 ++
  gcc/lto-opts.c    |  1 +
  gcc/opts-global.c |  4 
  gcc/opts.c    |  2 ++
  gcc/profile.c |  4 
  9 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index de9b848eda5..0b59b46b875 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2323,6 +2323,10 @@ fprofile-prefix-path=
  Common Joined RejectNegative Var(profile_prefix_path)
  Remove prefix from absolute path before mangling name for -fprofile-generate= 
and -fprofile-use=.

+fprofile-prefix-map=
+Common Joined RejectNegative Var(common_deferred_options) Defer
+-fprofile-prefix-map==    Map one directory name to another in GCOV 
coverage result.
+
  fprofile-generate
  Common
  Enable common options for generating profile info for profile feedback 
directed optimizations.
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 4daa3f9fc30..7f8b532cb52 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "profile.h"
  #include "diagnostic.h"
  #include "varasm.h"
+#include "file-prefix-map.h"

  #include "gcov-io.c"

@@ -646,7 +647,7 @@ coverage_begin_function (unsigned lineno_checksum, unsigned 
cfg_checksum)
    gcov_write_unsigned (DECL_ARTIFICIAL (current_function_decl)
     && !DECL_FUNCTION_VERSIONED (current_function_decl)
     && !DECL_LAMBDA_FUNCTION_P (current_function_decl));
-  gcov_write_filename (startloc.file);
+  gcov_write_filename (remap_profile_filename (startloc.file));
    gcov_write_unsigned (startloc.line);
    gcov_write_unsigned (startloc.column);

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2aba4c70b44..5fb6a8bfffe 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -606,7 +606,8 @@ Objective-C and Objective-C++ Dialects}.
  -fvtv-counts  -fvtv-debug @gol
  -finstrument-functions @gol
  -finstrument-functions-exclude-function-list=@var{sym},@var{sym},@dots{} @gol
--finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{}}
+-finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{}} @gol
+-fprofile-prefix-map=@var{old}=@var{new}

  @item Preprocessor Options
  @xref{Preprocessor Options,,Options Controlling the Preprocessor}.
@@ -2173,7 +2174,8 @@ files resided in directory @file{@var{new}} instead.  
Specifying this
  option is equivalent to specifying all the individual
  @option{-f*-prefix-map} options.  This can be used to make reproducible
  builds that are location independent.  See also
-@option{-fmacro-prefix-map} and @option{-fdebug-prefix-map}.
+@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
+@option{-fprofile-prefix-map}.

  @item -fplugin=@var{name}.so
  @opindex fplugin
@@ -15155,6 +15157,14 @@ In such setups 
@option{-fprofile-prefix-path=}@var{path} with @var{path}
  pointing to the base directory of the build can be used to strip the 
irrelevant
  part of the path and keep all file names relative to the main build directory.

+@item -fprofile-prefix-map=@var{old}=@var{new}
+@opindex fprofile-prefix-map
+When compiling files residing in directory @file{@var{old}}, record
+profiling information (with @option{--coverage})
+describing them as if the files resided in
+directory @file{@var{new}} instead.
+See also @option{-ffile-prefix-map}.
+
  @item -fprofile-update=@var{method}
  @opindex fprofile-update

diff --git a/gcc/file-prefix-map.c b/gcc/file-prefix-map.c
index ad242e5b9c5..290b4b2da33 100644
--- a/gcc/file-prefix-map.c
+++ b/gcc/file-prefix-map.c
@@ -92,6 +92,7 @@ remap_filename (file_prefix_map *maps, const char *filename)
  /* Linked lists of file_prefix_map structures.  */
  static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
  static file_prefix_map *debug_prefix_maps; /* -fdebug-prefix-map  */
+stat

Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Martin Liška

On 12/1/21 19:21, Andrew MacLeod wrote:

On 12/1/21 09:48, Martin Liška wrote:

On 12/1/21 15:34, Richard Biener wrote:

On Wed, Dec 1, 2021 at 3:25 PM Martin Liška  wrote:


On 12/1/21 15:19, Richard Biener wrote:

which is compute the range of 'lhs' on edge_true into predicate->true_range,
assign that same range to ->false_range and then invert it to get the
range on the false_edge.  What I am saying is that for better precision
you should do

   ranger->range_on_edge (predicate->false_range, edge_false, lhs);

rather than prematurely optimize this to the inversion of the true range
since yes, ranger is CFG sensitive and only the_last_ predicate on a
long CFG path is actually inverted.

What am I missing?


I might be misunderstood, but I think it's the problem defined here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584605.html

where I used the ranger->range_on_edge on the false_edge.


Ah, OK.  But then even the true_edge range is possibly wrong, no?


You are of course correct, I've just proved that in debugger ://


Consider

   for (;;)
  {
  if (a < 100)
    if (a > 50)  // unswitch on this
  /* .. */
  if (a < 120)
  /* ... */
  }

then you record [51, 99] for true_range of the a > 50 predicate and thus
simplification will simplify the if (a < 120) check, no?


Yep.



You can only record the range from the (CFG independent) a > 50 check,
thus [51, +INF] but of course at simplification time you can also use
the CFG context at each simplification location.


@Andrew: How can I easily get irange based just on a stmt? Something like 
fold_range
with int_range_max as the 3rd argument?


Sorry, I miss these things if I'm not directly CC'd a lot :-)

So you just want to know the basic range the stmt generates without context?    
Sure, what you say would be fine, but your want to initialize it to the type 
range:


Yes, I want to know range of LHS in a gcond statement and the same for cases in 
a gswitch statement.



int_range_max range (TREE_TYPE (name));

you can also simply trigger it using the current SSA_NAME_RANGE_INFO global  
values query instead of the default current contextual one... which , if there 
isnt a global range, will automatically use the range of the type of the 
argument.. so maybe just try

fold_range (r, stmt, get_global_range_query ())


Doing

  predicate->true_range = int_range_max (TREE_TYPE (lhs));
  fold_range (predicate->true_range, stmt, get_global_range_query ());
  predicate->true_range.debug();

gives me _Bool VARYING.

Martin



Andrew








Re: [RFC]PR103271 ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 12:37 AM Qing Zhao  wrote:
>
> Hi,
>
> As I studied, the issue with this bug is similar as PR102285:
>
> For the testing case:
>
> int
> qy (void)
> {
>   int tw = 4;
>   int fb[tw];
>   return fb[2]; /* { dg-warning "uninitialized" } */
> }
>
>  if compiled with
> /home/opc/Install/cross/bin/riscv64-unknown-linux-gnu-gcc -c -O1  
> -mno-strict-align -ftrivial-auto-var-init=pattern -fdump-tree-all t.c
>
> **t.c.032t.cpp1:
> int qy ()
> {
>   unsigned char fb.3[16];
>   int tw;
>   int[0:D.1494] * fb.1;
>   sizetype D.1494;
>   void * saved_stack.2_16;
>   int _27;
>
>:
>   saved_stack.2_16 = __builtin_stack_save ();
>   fb.1_25 = &fb.3;
>   (*fb.1_25) = .DEFERRED_INIT (16, 1, 1);
>   _27 = (*fb.1_25)[2];
>   fb.3 ={v} {CLOBBER};
>   __builtin_stack_restore (saved_stack.2_16);
>   return _27;
>
> }
>
> **t.c.033t.forwprop1:
> int qy ()
> {
>   unsigned char fb.3[16];
>   int tw;
>   int[0:D.1494] * fb.1;
>   sizetype D.1494;
>   void * saved_stack.2_16;
>   int _27;
>
>:
>   saved_stack.2_16 = __builtin_stack_save ();
>   MEM[(int[0:D.1494] *)&fb.3] = .DEFERRED_INIT (16, 1, 1);
>   _27 = MEM[(int[0:D.1494] *)&fb.3][2];
>   fb.3 ={v} {CLOBBER};
>   __builtin_stack_restore (saved_stack.2_16);
>   return _27;
>
> }
>
> the problem with the above IR for .DEFERRED_INIT expansion is:
>
> for MEM[(int[0:D.1494] *)&fb.3] = .DEFERRED_INIT (16, 1, 1)
>
> after the “forwprop1” phase, the original address that point to VLA fb.1 is 
> replaced with the the new address that point to fixed array fb.3, however, 
> the TREE_TYPE of the MEM still is kept the old VLA type int[0:D.1984]. Such 
> inconsistency causes the ICE during expanding of call to the above 
> .DEFERRED_INIT.
>
> If during the forwprop, when replacing “fb.1” with “&fb.3”, the TREE_TYPE of 
> the MEM_REF can be updated accordingly, we should fix this inconsistency of 
> the IR, and therefore fix the issue in DEFERRED_INIT expansion.
>
> So, I think that the fix should be in “forwprop”, and update the TREE_TYPE of 
> the MEM_REF accordingly.

Actually the IL is kept consistent by preserving the original
TREE_TYPE of the MEM_REF.  The issue is that deferred_init
expansion sticks to the original type rather than initializing the
underlying storage.

See my comments in the PR.

> What do you think on this?
>
> thanks.
>
> Qing
>
>


Re: [PATCH v2] fix spelling of -linker-output-auto-nolto-rel

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 8:22 AM Rasmus Villemoes  wrote:
>
> The transposition nolto -> notlo is confusing and it makes the long
> name even harder to read than it already is - I kept reading it as
> "not lo" until I realized it was a simple typo.

OK for trunk and branch.

Thanks,
Richard.

> Fixes: 5269b24605b1 (Silence warning in LTO mode on VxWorks)
>
> lto-plugin/
> * lto-plugin.c: Fix -linker-output-auto-notlo-rel ->
> -linker-output-auto-nolto-rel typo.
> (process_option): Adjust accordingly, accepting both old and
> new spelling.
>
> gcc/
> * config/vxworks.h (LTO_PLUGIN_SPEC): Adapt to corrected
> spelling of -linker-output-auto-nolto-rel.
> ---
> v2: still accept -linker-output-auto-notlo-rel in the plugin.
>
> Like this?
>
>  gcc/config/vxworks.h| 2 +-
>  lto-plugin/lto-plugin.c | 6 --
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
> index e41f16a51e8..bddf2c37f42 100644
> --- a/gcc/config/vxworks.h
> +++ b/gcc/config/vxworks.h
> @@ -306,4 +306,4 @@ extern void vxworks_emit_call_builtin___clear_cache (rtx 
> begin, rtx end);
> further incremental LTO linking.  We do not do repeated incremental 
> linking
> so silence the warning (instead of passing -flinker-output=nolto-rel).  */
>  #undef LTO_PLUGIN_SPEC
> -#define LTO_PLUGIN_SPEC "%{!mrtp:-plugin-opt=-linker-output-auto-notlo-rel}"
> +#define LTO_PLUGIN_SPEC "%{!mrtp:-plugin-opt=-linker-output-auto-nolto-rel}"
> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> index 6ab9822f369..b73483de994 100644
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -32,7 +32,7 @@ along with this program; see the file COPYING3.  If not see
> -nop: Instead of running lto-wrapper, pass the original to the plugin. 
> This
> only works if the input files are hybrid.
> -linker-output-known: Do not determine linker output
> -   -linker-output-auto-notlo-rel: Switch from rel to nolto-rel mode without
> +   -linker-output-auto-nolto-rel: Switch from rel to nolto-rel mode without
> warning.  This is used on systems like VxWorks (kernel) where the link is
> always partial and repeated incremental linking is generally not used.
> -sym-style={none,win32,underscore|uscore}
> @@ -1321,7 +1321,9 @@ process_option (const char *option)
>  {
>if (strcmp (option, "-linker-output-known") == 0)
>  linker_output_known = true;
> -  else if (strcmp (option, "-linker-output-auto-notlo-rel") == 0)
> +  /* Also accept "notlo" for backwards compatibility.  */
> +  else if ((strcmp (option, "-linker-output-auto-nolto-rel") == 0)
> +   || (strcmp (option, "-linker-output-auto-notlo-rel") == 0))
>  linker_output_auto_nolto_rel = true;
>else if (strcmp (option, "-debug") == 0)
>  debug = true;
> --
> 2.31.1
>


Re: [PATCH] Implement -fprofile-prefix-map.

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 12:43 PM Martin Liška  wrote:
>
> PING^1

OK.

> On 11/11/21 17:39, Martin Liška wrote:
> > It's functionality that is analogous to -ffile-prefix-map, this time
> > for gcov purpose.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >
> > Ready to be installed?
> > Thanks,
> > Martin
> >
> >  PR gcov-profile/96092
> >
> > gcc/ChangeLog:
> >
> >  * common.opt: New option.
> >  * coverage.c (coverage_begin_function): Emit filename with
> >  remap_profile_filename.
> >  * doc/invoke.texi: Document the new option.
> >  * file-prefix-map.c (add_profile_prefix_map): New.
> >  (remap_profile_filename): Likewise.
> >  * file-prefix-map.h (add_profile_prefix_map): Likewise.
> >  (remap_profile_filename): Likewise.
> >  * lto-opts.c (lto_write_options): Handle
> >  OPT_fprofile_prefix_map_.
> >  * opts-global.c (handle_common_deferred_options): Likewise.
> >  * opts.c (common_handle_option): Likewise.
> >  (gen_command_line_string): Likewise.
> >  * profile.c (output_location): Emit filename with
> >  remap_profile_filename.
> > ---
> >   gcc/common.opt|  4 
> >   gcc/coverage.c|  3 ++-
> >   gcc/doc/invoke.texi   | 14 --
> >   gcc/file-prefix-map.c | 17 +
> >   gcc/file-prefix-map.h |  2 ++
> >   gcc/lto-opts.c|  1 +
> >   gcc/opts-global.c |  4 
> >   gcc/opts.c|  2 ++
> >   gcc/profile.c |  4 
> >   9 files changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index de9b848eda5..0b59b46b875 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -2323,6 +2323,10 @@ fprofile-prefix-path=
> >   Common Joined RejectNegative Var(profile_prefix_path)
> >   Remove prefix from absolute path before mangling name for 
> > -fprofile-generate= and -fprofile-use=.
> >
> > +fprofile-prefix-map=
> > +Common Joined RejectNegative Var(common_deferred_options) Defer
> > +-fprofile-prefix-map==Map one directory name to another in 
> > GCOV coverage result.
> > +
> >   fprofile-generate
> >   Common
> >   Enable common options for generating profile info for profile feedback 
> > directed optimizations.
> > diff --git a/gcc/coverage.c b/gcc/coverage.c
> > index 4daa3f9fc30..7f8b532cb52 100644
> > --- a/gcc/coverage.c
> > +++ b/gcc/coverage.c
> > @@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "profile.h"
> >   #include "diagnostic.h"
> >   #include "varasm.h"
> > +#include "file-prefix-map.h"
> >
> >   #include "gcov-io.c"
> >
> > @@ -646,7 +647,7 @@ coverage_begin_function (unsigned lineno_checksum, 
> > unsigned cfg_checksum)
> > gcov_write_unsigned (DECL_ARTIFICIAL (current_function_decl)
> >  && !DECL_FUNCTION_VERSIONED (current_function_decl)
> >  && !DECL_LAMBDA_FUNCTION_P (current_function_decl));
> > -  gcov_write_filename (startloc.file);
> > +  gcov_write_filename (remap_profile_filename (startloc.file));
> > gcov_write_unsigned (startloc.line);
> > gcov_write_unsigned (startloc.column);
> >
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 2aba4c70b44..5fb6a8bfffe 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -606,7 +606,8 @@ Objective-C and Objective-C++ Dialects}.
> >   -fvtv-counts  -fvtv-debug @gol
> >   -finstrument-functions @gol
> >   -finstrument-functions-exclude-function-list=@var{sym},@var{sym},@dots{} 
> > @gol
> > --finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{}}
> > +-finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{}} 
> > @gol
> > +-fprofile-prefix-map=@var{old}=@var{new}
> >
> >   @item Preprocessor Options
> >   @xref{Preprocessor Options,,Options Controlling the Preprocessor}.
> > @@ -2173,7 +2174,8 @@ files resided in directory @file{@var{new}} instead.  
> > Specifying this
> >   option is equivalent to specifying all the individual
> >   @option{-f*-prefix-map} options.  This can be used to make reproducible
> >   builds that are location independent.  See also
> > -@option{-fmacro-prefix-map} and @option{-fdebug-prefix-map}.
> > +@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
> > +@option{-fprofile-prefix-map}.
> >
> >   @item -fplugin=@var{name}.so
> >   @opindex fplugin
> > @@ -15155,6 +15157,14 @@ In such setups 
> > @option{-fprofile-prefix-path=}@var{path} with @var{path}
> >   pointing to the base directory of the build can be used to strip the 
> > irrelevant
> >   part of the path and keep all file names relative to the main build 
> > directory.
> >
> > +@item -fprofile-prefix-map=@var{old}=@var{new}
> > +@opindex fprofile-prefix-map
> > +When compiling files residing in directory @file{@var{old}}, record
> > +profiling information (with @option{--coverage})
> > +describing them as if the files resided in
> > +directory @file{@var{new}} instead.
> > +S

Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 12:45 PM Martin Liška  wrote:
>
> On 12/1/21 19:21, Andrew MacLeod wrote:
> > On 12/1/21 09:48, Martin Liška wrote:
> >> On 12/1/21 15:34, Richard Biener wrote:
> >>> On Wed, Dec 1, 2021 at 3:25 PM Martin Liška  wrote:
> 
>  On 12/1/21 15:19, Richard Biener wrote:
> > which is compute the range of 'lhs' on edge_true into 
> > predicate->true_range,
> > assign that same range to ->false_range and then invert it to get the
> > range on the false_edge.  What I am saying is that for better precision
> > you should do
> >
> >ranger->range_on_edge (predicate->false_range, edge_false, lhs);
> >
> > rather than prematurely optimize this to the inversion of the true range
> > since yes, ranger is CFG sensitive and only the_last_ predicate on a
> > long CFG path is actually inverted.
> >
> > What am I missing?
> 
>  I might be misunderstood, but I think it's the problem defined here:
>  https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584605.html
> 
>  where I used the ranger->range_on_edge on the false_edge.
> >>>
> >>> Ah, OK.  But then even the true_edge range is possibly wrong, no?
> >>
> >> You are of course correct, I've just proved that in debugger ://
> >>
> >>> Consider
> >>>
> >>>for (;;)
> >>>   {
> >>>   if (a < 100)
> >>> if (a > 50)  // unswitch on this
> >>>   /* .. */
> >>>   if (a < 120)
> >>>   /* ... */
> >>>   }
> >>>
> >>> then you record [51, 99] for true_range of the a > 50 predicate and thus
> >>> simplification will simplify the if (a < 120) check, no?
> >>
> >> Yep.
> >>
> >>>
> >>> You can only record the range from the (CFG independent) a > 50 check,
> >>> thus [51, +INF] but of course at simplification time you can also use
> >>> the CFG context at each simplification location.
> >>
> >> @Andrew: How can I easily get irange based just on a stmt? Something like 
> >> fold_range
> >> with int_range_max as the 3rd argument?
> >>
> > Sorry, I miss these things if I'm not directly CC'd a lot :-)
> >
> > So you just want to know the basic range the stmt generates without 
> > context?Sure, what you say would be fine, but your want to initialize 
> > it to the type range:
>
> Yes, I want to know range of LHS in a gcond statement and the same for cases 
> in a gswitch statement.
>
> >
> > int_range_max range (TREE_TYPE (name));
> >
> > you can also simply trigger it using the current SSA_NAME_RANGE_INFO global 
> >  values query instead of the default current contextual one... which , if 
> > there isnt a global range, will automatically use the range of the type of 
> > the argument.. so maybe just try
> >
> > fold_range (r, stmt, get_global_range_query ())
>
> Doing
>
>predicate->true_range = int_range_max (TREE_TYPE (lhs));
>fold_range (predicate->true_range, stmt, get_global_range_query ());
>predicate->true_range.debug();
>
> gives me _Bool VARYING.

Likely because that gives a range for the bool result rather than
a range for the LHS of a LHS op RHS on the true or false edge.
I would guess no stmt level API gives that.  In previous VRP
incarnation assert expr discovery would yield the asserts
that hold for the LHS on the respective edge and from the asserts
ranges could be determined.

Richard.

>
> Martin
>
> >
> > Andrew
> >
> >
> >
> >
>


[PATCH] tree-optimization/103527 - always use thruth type forgather mask

2021-12-02 Thread Richard Biener via Gcc-patches
This makes sure to always use a truth type to build the gather
mask argument even when the target builtin prototype in the end
wants a float vector.

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

2021-12-02  Richard Biener  

PR tree-optimization/103527
* tree-vect-stmts.c (vect_build_gather_load_calls): Always
use a truth type for building the vector mask.
---
 gcc/tree-vect-stmts.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 9726450ab2d..e63bc2afdaa 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -2740,7 +2740,7 @@ vect_build_gather_load_calls (vec_info *vinfo, 
stmt_vec_info stmt_info,
   && (!mask
   || TREE_CODE (masktype) == INTEGER_TYPE
   || types_compatible_p (srctype, masktype)));
-  if (mask && TREE_CODE (masktype) == INTEGER_TYPE)
+  if (mask)
 masktype = truth_type_for (srctype);
 
   tree mask_halftype = masktype;
@@ -2893,7 +2893,8 @@ vect_build_gather_load_calls (vec_info *vinfo, 
stmt_vec_info stmt_info,
   if (masktype != real_masktype)
{
  tree utype, optype = TREE_TYPE (mask_op);
- if (TYPE_MODE (real_masktype) == TYPE_MODE (optype))
+ if (VECTOR_TYPE_P (real_masktype)
+ || TYPE_MODE (real_masktype) == TYPE_MODE (optype))
utype = real_masktype;
  else
utype = lang_hooks.types.type_for_mode (TYPE_MODE (optype), 1);
-- 
2.31.1


[PATCH] middle-end/103271 - avoid VLA init of register

2021-12-02 Thread Richard Biener via Gcc-patches
This avoids using VLA types to initalize a register with
-ftrivial-auto-var-init in some cases.

Bootstrapped and tested on x86_64-unknown-linux-gnu, also tested on
the case riscv ICEd on.

Pushed.

2021-12-02  Richard Biener  

PR middle-end/103271
* internal-fn.c (expand_DEFERRED_INIT): When the base
of the LHS is a decl with matching constant size use
that as the initialization target instead of an
eventual VLA typed one.
---
 gcc/internal-fn.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 6ac3460d538..08f94b7a17a 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3050,6 +3050,23 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
lhs_base = TREE_OPERAND (lhs_base, 0);
   reg_lhs = (mem_ref_refers_to_non_mem_p (lhs_base)
 || non_mem_decl_p (lhs_base));
+  /* If this expands to a register and the underlying decl is wrapped in
+a MEM_REF that just serves as an access type change expose the decl
+if it is of correct size.  This avoids a situation as in PR103271
+if the target does not support a direct move to the registers mode.  */
+  if (reg_lhs
+ && TREE_CODE (lhs_base) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (lhs_base, 0)) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0))
+ && integer_zerop (TREE_OPERAND (lhs_base, 1))
+ && tree_fits_uhwi_p (var_size)
+ && tree_int_cst_equal
+  (var_size,
+   DECL_SIZE_UNIT (TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0
+   {
+ lhs = TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0);
+ var_type = TREE_TYPE (lhs);
+   }
 }
 
   if (!reg_lhs)
-- 
2.31.1


Re: [PATCH] OpenMP: Ensure that offloaded variables are public

2021-12-02 Thread Andrew Stubbs

On 30/11/2021 16:54, Jakub Jelinek wrote:

Why does the GCN plugin or runtime need to know those vars?
It needs to know the single array that contains their addresses of course...


With older LLVM there were issues with relocations that made it 
impossible to link the the offload_var_table. This is why mkoffload 
deletes it. I've not tried it again recently, so it's possible we could 
completely rework the way these are processed in the plugin, but that's 
the hard option.


What it currently does is a symbol lookup for each named variable listed 
in the C wrapper used to embed the kernel. The lookup provided by the 
AMD runtime ignores symbols that are not exported, even if they are 
present in the ELF.



Actually, you've done it in ACCEL_COMPILER only, so
I assume linking the above two sources with -fopenmp into a single
binary or shared library will still work because LTO when reading
the byte-code in will remangle the names of those variables to something
where they are unique in that single *.s (or *.ptx) it emits.
But, if you put one of those TUs into a shared library and the other
into another shared library, I don't see how it can work anymore,
because both those ELF objects which will be in data sections of those
libraries might have clashing names.


The plugin loads each image file as an independent "executable". If 
there are multiple images then there *will be* duplicate symbols (e.g. 
"init_array") but this is not a problem because they're in a different 
context.


If there's a problem with duplicate symbols *within* a given image then 
we have a bigger problem because offload_var_table is referring to them 
by name. As you say, I presume the LTO stream-in is fixing up such 
conflicts.



If GCN can't support static variables (but isn't it ELF?) and there is no
other way than sacrifice offloading from multiple shared libraries or binary
in the same process, it at least shouldn't be done for targets which don't
need it (e.g. PTX) and shouldn't be done in the pass you've done it in
(because that means it will walk all the vars for each function it
processes, rather than just once).  So, better place would be e.g.
offload_handle_link_vars in lto/*.c or so.


GCN is ELF and can support static variables just fine ... as long as you 
don't want to poke values into them from the outside. We do not support 
any sort of dynamic libraries; kernels are statically linked relocatable 
executables (the AMD runtime expects the binary to be a dynamic object 
itself, but there's nothing hunting for dependencies, or anything like 
that).


I've tried modifying offload_handle_link_vars but that spot doesn't 
catch the omp_data_sizes variables emitted by 
libgomp.c-c++-common/target_42.c, which was one of the motivating examples.


It is true that my current placement visits all the symbols for every 
function, meaning that they are adjusted in an earlier iteration of a 
pass than you might expect. I couldn't find a single place that fixed 
this problem only in the amdgcn compiler and wasn't too late.


Do you have a suggestion how to not do this for other GPU targets? We 
can add another hook or macro, of course 


Thanks for the review!

Andrew


[PATCH][committed]middle-end: make bic-bitmask-18.c test more precise [PR103479]

2021-12-02 Thread Tamar Christina via Gcc-patches
Hi All,

This updates the testcase bic-bitmask-18.c to seach for " = 0;" as an expression
so it doesn't match any other partial expressions.

Regtested on aarch64-none-linux-gnu,
x86_64-pc-linux-gnu (with and without -m32) and no regressions.

Committed under the obvious rule.

Thanks,
Tamar

gcc/testsuite/ChangeLog:

* gcc.dg/bic-bitmask-18.c: Update regexpr to expect = 0;.

--- inline copy of patch -- 
diff --git a/gcc/testsuite/gcc.dg/bic-bitmask-18.c 
b/gcc/testsuite/gcc.dg/bic-bitmask-18.c
index 
9d11b3bf4a4f04f572d47d710499d97ea0b11936..70bab0c520321ba13c6dd7969d1b51708dc3c71f
 100644
--- a/gcc/testsuite/gcc.dg/bic-bitmask-18.c
+++ b/gcc/testsuite/gcc.dg/bic-bitmask-18.c
@@ -19,6 +19,6 @@ void fun2(uint32_t *x, int n)
 
 #include "bic-bitmask.h"
 
-/* { dg-final { scan-tree-dump-times {= 0} 1 dce7 { target vect_int } } } */
+/* { dg-final { scan-tree-dump-times { = 0;} 1 dce7 { target vect_int } } } */
 /* { dg-final { scan-tree-dump-not {\s+bic\s+} dce7 { target { aarch64*-*-* } 
} } } */
 


-- 
diff --git a/gcc/testsuite/gcc.dg/bic-bitmask-18.c b/gcc/testsuite/gcc.dg/bic-bitmask-18.c
index 9d11b3bf4a4f04f572d47d710499d97ea0b11936..70bab0c520321ba13c6dd7969d1b51708dc3c71f 100644
--- a/gcc/testsuite/gcc.dg/bic-bitmask-18.c
+++ b/gcc/testsuite/gcc.dg/bic-bitmask-18.c
@@ -19,6 +19,6 @@ void fun2(uint32_t *x, int n)
 
 #include "bic-bitmask.h"
 
-/* { dg-final { scan-tree-dump-times {= 0} 1 dce7 { target vect_int } } } */
+/* { dg-final { scan-tree-dump-times { = 0;} 1 dce7 { target vect_int } } } */
 /* { dg-final { scan-tree-dump-not {\s+bic\s+} dce7 { target { aarch64*-*-* } } } } */
 



Re: [PATCH] OpenMP: Ensure that offloaded variables are public

2021-12-02 Thread Jakub Jelinek via Gcc-patches
On Thu, Dec 02, 2021 at 12:36:30PM +, Andrew Stubbs wrote:
> On 30/11/2021 16:54, Jakub Jelinek wrote:
> > > Why does the GCN plugin or runtime need to know those vars?
> > > It needs to know the single array that contains their addresses of 
> > > course...
> 
> With older LLVM there were issues with relocations that made it impossible
> to link the the offload_var_table. This is why mkoffload deletes it. I've
> not tried it again recently, so it's possible we could completely rework the
> way these are processed in the plugin, but that's the hard option.
> 
> What it currently does is a symbol lookup for each named variable listed in
> the C wrapper used to embed the kernel. The lookup provided by the AMD
> runtime ignores symbols that are not exported, even if they are present in
> the ELF.

Would be nice to know what the relocation issue is or was, offload_var_table
shouldn't be different from other arrays containing pointers to static vars,
no?
If you delete it and have to do the lookups in the plugin, I understand that
then they need to be public...

> The plugin loads each image file as an independent "executable". If there
> are multiple images then there *will be* duplicate symbols (e.g.
> "init_array") but this is not a problem because they're in a different
> context.
> 
> If there's a problem with duplicate symbols *within* a given image then we
> have a bigger problem because offload_var_table is referring to them by
> name. As you say, I presume the LTO stream-in is fixing up such conflicts.

Ah, ok.

> I've tried modifying offload_handle_link_vars but that spot doesn't catch
> the omp_data_sizes variables emitted by libgomp.c-c++-common/target_42.c,
> which was one of the motivating examples.

Why doesn't catch it?  Is the variable created only post-IPA?
I'd think that it should have been created before IPA, streamed and
therefore I don't understand why you don't see it after streaming LTO in.

> It is true that my current placement visits all the symbols for every
> function, meaning that they are adjusted in an earlier iteration of a pass
> than you might expect. I couldn't find a single place that fixed this
> problem only in the amdgcn compiler and wasn't too late.
> 
> Do you have a suggestion how to not do this for other GPU targets? We can
> add another hook or macro, of course 

Certainly a target hook.  But I'd really like to understand why you don't
see those earlier.

Jakub



Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Martin Liška

On 12/2/21 13:01, Richard Biener wrote:

On Thu, Dec 2, 2021 at 12:45 PM Martin Liška  wrote:


On 12/1/21 19:21, Andrew MacLeod wrote:

On 12/1/21 09:48, Martin Liška wrote:

On 12/1/21 15:34, Richard Biener wrote:

On Wed, Dec 1, 2021 at 3:25 PM Martin Liška  wrote:


On 12/1/21 15:19, Richard Biener wrote:

which is compute the range of 'lhs' on edge_true into predicate->true_range,
assign that same range to ->false_range and then invert it to get the
range on the false_edge.  What I am saying is that for better precision
you should do

ranger->range_on_edge (predicate->false_range, edge_false, lhs);

rather than prematurely optimize this to the inversion of the true range
since yes, ranger is CFG sensitive and only the_last_ predicate on a
long CFG path is actually inverted.

What am I missing?


I might be misunderstood, but I think it's the problem defined here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584605.html

where I used the ranger->range_on_edge on the false_edge.


Ah, OK.  But then even the true_edge range is possibly wrong, no?


You are of course correct, I've just proved that in debugger ://


Consider

for (;;)
   {
   if (a < 100)
 if (a > 50)  // unswitch on this
   /* .. */
   if (a < 120)
   /* ... */
   }

then you record [51, 99] for true_range of the a > 50 predicate and thus
simplification will simplify the if (a < 120) check, no?


Yep.



You can only record the range from the (CFG independent) a > 50 check,
thus [51, +INF] but of course at simplification time you can also use
the CFG context at each simplification location.


@Andrew: How can I easily get irange based just on a stmt? Something like 
fold_range
with int_range_max as the 3rd argument?


Sorry, I miss these things if I'm not directly CC'd a lot :-)

So you just want to know the basic range the stmt generates without context?
Sure, what you say would be fine, but your want to initialize it to the type 
range:


Yes, I want to know range of LHS in a gcond statement and the same for cases in 
a gswitch statement.



int_range_max range (TREE_TYPE (name));

you can also simply trigger it using the current SSA_NAME_RANGE_INFO global  
values query instead of the default current contextual one... which , if there 
isnt a global range, will automatically use the range of the type of the 
argument.. so maybe just try

fold_range (r, stmt, get_global_range_query ())


Doing

predicate->true_range = int_range_max (TREE_TYPE (lhs));
fold_range (predicate->true_range, stmt, get_global_range_query ());
predicate->true_range.debug();

gives me _Bool VARYING.


Likely because that gives a range for the bool result rather than
a range for the LHS of a LHS op RHS on the true or false edge.


Yes :) I guess Andrew can help us.


I would guess no stmt level API gives that.  In previous VRP
incarnation assert expr discovery would yield the asserts
that hold for the LHS on the respective edge and from the asserts
ranges could be determined.


About the gswitch statements, I need similarly irange for a switch statement
on an edge e.

Thanks for help,
Martin



Richard.



Martin



Andrew










Re: [PATCH] c++, dyninit: Optimize C++ dynamic initialization by constants into DECL_INITIAL adjustment [PR102876]

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, 4 Nov 2021, Jakub Jelinek wrote:

> Hi!
> 
> When users don't use constexpr everywhere in initialization of namespace
> scope non-comdat vars and the initializers aren't constant when FE is
> looking at them, the FE performs dynamic initialization of those variables.
> But after inlining and some constant propagation, we often end up with
> just storing constants into those variables in the _GLOBAL__sub_I_*
> constructor.
> C++ gives us permission to change some of that dynamic initialization
> back into static initialization - https://eel.is/c++draft/basic.start.static#3
> For classes that need (dynamic) construction, I believe access to some var
> from other dynamic construction before that var is constructed is UB, but
> as the example in the above mentioned spot of C++:
> inline double fd() { return 1.0; }
> extern double d1;
> double d2 = d1; // unspecified:
> // either statically initialized to 0.0 or
> // dynamically initialized to 0.0 if d1 is
> // dynamically initialized, or 1.0 otherwise
> double d1 = fd();   // either initialized statically or dynamically to 1.0
> some vars can be used before they are dynamically initialized and the
> implementation can still optimize those into static initialization.
> 
> The following patch attempts to optimize some such cases back into
> DECL_INITIAL initializers and where possible (originally const vars without
> mutable members) put those vars back to .rodata etc.
> 
> Because we put all dynamic initialization from a single TU into one single
> function (well, originally one function per priority but typically inline
> those back into one function), we can either have a simpler approach
> (from the PR it seems that is what LLVM uses) where either we manage to
> optimize all dynamic initializers into constant in the TU, or nothing,
> or by adding some markup - in the form of a pair of internal functions in
> this patch - around each dynamic initialization that can be optimized,
> we can optimize each dynamic initialization separately.
> 
> The patch adds a new pass that is invoked (through gate check) only on
> DECL_ARTIFICIAL DECL_STATIC_CONSTRUCTOR functions, and looks there for
> sequences like:
>   .DYNAMIC_INIT_START (&b, 0);
>   b = 1;
>   .DYNAMIC_INIT_END (&b);
> or
>   .DYNAMIC_INIT_START (&e, 1);
>   # DEBUG this => &e.f
>   MEM[(struct S *)&e + 4B] ={v} {CLOBBER};
>   MEM[(struct S *)&e + 4B].a = 1;
>   MEM[(struct S *)&e + 4B].b = 2;
>   MEM[(struct S *)&e + 4B].c = 3;
>   # DEBUG BEGIN_STMT
>   MEM[(struct S *)&e + 4B].d = 6;
>   # DEBUG this => NULL
>   .DYNAMIC_INIT_END (&e);

So with

+/* Mark start and end of dynamic initialization of a variable.  */
+DEF_INTERNAL_FN (DYNAMIC_INIT_START, ECF_LEAF | ECF_NOTHROW, ". r ")
+DEF_INTERNAL_FN (DYNAMIC_INIT_END, ECF_LEAF | ECF_NOTHROW, ". r ")

there's nothing preventing code motion of unrelated stmts into
the block, but that should be harmless.  What it also does
is make 'e' aliased (because it's address is now taken), probably
relevant only for IPA/LTO or for statics.

The setup does not prevent CSEing the inits with uses from another
initializer - probably OK as well (if not then .DYNAMIC_INIT_END
should also be considered writing to 'e').

". r " also means it clobbers and uses all global memory, I think
we'd like to have it const + looping-pure-or-const.  ".cr " would
possibly achieve this, not sure about the looping part.


> (where between the pair of markers everything is either debug stmts or
> stores of constants into the variables or their parts).
> The pass needs to be done late enough so that after IPA all the needed
> constant propagation and perhaps loop unrolling is done, on the other
> side should be early enough so that if we can't optimize it, we can
> remove those .DYNAMIC_INIT* internal calls that could prevent some
> further optimizations (they have fnspec such that they pretend to read
> the corresponding variable).
> 
> Currently the optimization is only able to optimize cases where the whole
> variable is stored in a single store (typically scalar variables), or
> uses the native_{encode,interpret}* infrastructure to create or update
> the CONSTRUCTOR.  This means that except for the first category, we can't
> right now handle unions or anything that needs relocations (vars containing
> pointers to other vars or references).
> I think it would be nice to incrementally add before the native_* fallback
> some attempt to just create or update a CONSTRUCTOR if possible.  If we only
> see var.a.b.c.d[10].e = const; style of stores, this shouldn't be that hard
> as the whole access path is recorded there and we'd just need to decide what
> to do with unions if two or more union members are accessed.  And do a deep
> copy of the CONSTRUCTOR and try to efficiently update the copy afterwards
> (the CONSTRUCTORs should be sorted on increasing offsets of the
> members/elements, so doing an ordered vec insertion mig

Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 2:10 PM Martin Liška  wrote:
>
> On 12/2/21 13:01, Richard Biener wrote:
> > On Thu, Dec 2, 2021 at 12:45 PM Martin Liška  wrote:
> >>
> >> On 12/1/21 19:21, Andrew MacLeod wrote:
> >>> On 12/1/21 09:48, Martin Liška wrote:
>  On 12/1/21 15:34, Richard Biener wrote:
> > On Wed, Dec 1, 2021 at 3:25 PM Martin Liška  wrote:
> >>
> >> On 12/1/21 15:19, Richard Biener wrote:
> >>> which is compute the range of 'lhs' on edge_true into 
> >>> predicate->true_range,
> >>> assign that same range to ->false_range and then invert it to get the
> >>> range on the false_edge.  What I am saying is that for better 
> >>> precision
> >>> you should do
> >>>
> >>> ranger->range_on_edge (predicate->false_range, edge_false, 
> >>> lhs);
> >>>
> >>> rather than prematurely optimize this to the inversion of the true 
> >>> range
> >>> since yes, ranger is CFG sensitive and only the_last_ predicate on a
> >>> long CFG path is actually inverted.
> >>>
> >>> What am I missing?
> >>
> >> I might be misunderstood, but I think it's the problem defined here:
> >> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584605.html
> >>
> >> where I used the ranger->range_on_edge on the false_edge.
> >
> > Ah, OK.  But then even the true_edge range is possibly wrong, no?
> 
>  You are of course correct, I've just proved that in debugger ://
> 
> > Consider
> >
> > for (;;)
> >{
> >if (a < 100)
> >  if (a > 50)  // unswitch on this
> >/* .. */
> >if (a < 120)
> >/* ... */
> >}
> >
> > then you record [51, 99] for true_range of the a > 50 predicate and thus
> > simplification will simplify the if (a < 120) check, no?
> 
>  Yep.
> 
> >
> > You can only record the range from the (CFG independent) a > 50 check,
> > thus [51, +INF] but of course at simplification time you can also use
> > the CFG context at each simplification location.
> 
>  @Andrew: How can I easily get irange based just on a stmt? Something 
>  like fold_range
>  with int_range_max as the 3rd argument?
> 
> >>> Sorry, I miss these things if I'm not directly CC'd a lot :-)
> >>>
> >>> So you just want to know the basic range the stmt generates without 
> >>> context?Sure, what you say would be fine, but your want to initialize 
> >>> it to the type range:
> >>
> >> Yes, I want to know range of LHS in a gcond statement and the same for 
> >> cases in a gswitch statement.
> >>
> >>>
> >>> int_range_max range (TREE_TYPE (name));
> >>>
> >>> you can also simply trigger it using the current SSA_NAME_RANGE_INFO 
> >>> global  values query instead of the default current contextual one... 
> >>> which , if there isnt a global range, will automatically use the range of 
> >>> the type of the argument.. so maybe just try
> >>>
> >>> fold_range (r, stmt, get_global_range_query ())
> >>
> >> Doing
> >>
> >> predicate->true_range = int_range_max (TREE_TYPE (lhs));
> >> fold_range (predicate->true_range, stmt, get_global_range_query 
> >> ());
> >> predicate->true_range.debug();
> >>
> >> gives me _Bool VARYING.
> >
> > Likely because that gives a range for the bool result rather than
> > a range for the LHS of a LHS op RHS on the true or false edge.
>
> Yes :) I guess Andrew can help us.

some grepping and poking found be gimple_range_calc_op1 which should
eventually do the trick (for constant gimple_cond_rhs at least).

> > I would guess no stmt level API gives that.  In previous VRP
> > incarnation assert expr discovery would yield the asserts
> > that hold for the LHS on the respective edge and from the asserts
> > ranges could be determined.
>
> About the gswitch statements, I need similarly irange for a switch statement
> on an edge e.

Well, you simply want a range (and condition to generate the if from)
from a CASE_LABEL_EXPR.  Given that's either a single integer constant
or a RANGE_EXPR it should be easy to use some of the irange CTORs here.

Richard.

> Thanks for help,
> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin
> >>
> >>>
> >>> Andrew
> >>>
> >>>
> >>>
> >>>
> >>
>


[PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches

The following patch fixes

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

The patch was successfully bootstrapped and tested on x86-64. There is 
no test as the bug occurs on GCC built with sanitizing for an existing 
go test.
commit c6cf5ac1522c54b2ced98fc687e973a9ff17ba1e
Author: Vladimir N. Makarov 
Date:   Thu Dec 2 08:29:45 2021 -0500

[PR103437] Process multiplication overflow in priority calculation for allocno assignments

We process overflows in cost calculations but for huge functions
priority calculation can overflow as priority can be bigger the cost
used for it.  The patch fixes the problem.

gcc/ChangeLog:

PR rtl-optimization/103437
* ira-color.c (setup_allocno_priorities): Process multiplication
overflow.

diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 3d01c60800c..1f80cbea0e2 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2796,7 +2796,7 @@ static int *allocno_priorities;
 static void
 setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
 {
-  int i, length, nrefs, priority, max_priority, mult;
+  int i, length, nrefs, priority, max_priority, mult, diff;
   ira_allocno_t a;
 
   max_priority = 0;
@@ -2807,11 +2807,14 @@ setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
   ira_assert (nrefs >= 0);
   mult = floor_log2 (ALLOCNO_NREFS (a)) + 1;
   ira_assert (mult >= 0);
-  allocno_priorities[ALLOCNO_NUM (a)]
-	= priority
-	= (mult
-	   * (ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a))
-	   * ira_reg_class_max_nregs[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)]);
+  mult *= ira_reg_class_max_nregs[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)];
+  diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
+  /* Multiplication can overflow for very large functions.
+	 Check the overflow and constrain the result if necessary: */
+  if (__builtin_smul_overflow (mult, diff, &priority)
+	  || priority <= -INT_MAX)
+	priority = diff >= 0 ? INT_MAX : -INT_MAX;
+  allocno_priorities[ALLOCNO_NUM (a)] = priority;
   if (priority < 0)
 	priority = -priority;
   if (max_priority < priority)


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Jakub Jelinek via Gcc-patches
On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via Gcc-patches 
wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103437
> 
> The patch was successfully bootstrapped and tested on x86-64. There is no
> test as the bug occurs on GCC built with sanitizing for an existing go test.

> commit c6cf5ac1522c54b2ced98fc687e973a9ff17ba1e
> Author: Vladimir N. Makarov 
> Date:   Thu Dec 2 08:29:45 2021 -0500
> 
> [PR103437] Process multiplication overflow in priority calculation for 
> allocno assignments
> 
> We process overflows in cost calculations but for huge functions
> priority calculation can overflow as priority can be bigger the cost
> used for it.  The patch fixes the problem.
> 
> gcc/ChangeLog:
> 
> PR rtl-optimization/103437
> * ira-color.c (setup_allocno_priorities): Process multiplication
> overflow.
> 
> diff --git a/gcc/ira-color.c b/gcc/ira-color.c
> index 3d01c60800c..1f80cbea0e2 100644
> --- a/gcc/ira-color.c
> +++ b/gcc/ira-color.c
> @@ -2796,7 +2796,7 @@ static int *allocno_priorities;
>  static void
>  setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
>  {
> -  int i, length, nrefs, priority, max_priority, mult;
> +  int i, length, nrefs, priority, max_priority, mult, diff;
>ira_allocno_t a;
>  
>max_priority = 0;
> @@ -2807,11 +2807,14 @@ setup_allocno_priorities (ira_allocno_t 
> *consideration_allocnos, int n)
>ira_assert (nrefs >= 0);
>mult = floor_log2 (ALLOCNO_NREFS (a)) + 1;
>ira_assert (mult >= 0);
> -  allocno_priorities[ALLOCNO_NUM (a)]
> - = priority
> - = (mult
> -* (ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a))
> -* ira_reg_class_max_nregs[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)]);
> +  mult *= ira_reg_class_max_nregs[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)];
> +  diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
> +  /* Multiplication can overflow for very large functions.
> +  Check the overflow and constrain the result if necessary: */
> +  if (__builtin_smul_overflow (mult, diff, &priority)

I'm afraid we can't use __builtin_smul_overflow, not all system compilers
will have that.
But, as it is done in int and we kind of rely on int being 32-bit on host
and rely on long long being 64-bit, I think you can do something like:
  long long priorityll = (long long) mult * diff;
  priority = priorityll;
  if (priorityll != priority
...

> +   || priority <= -INT_MAX)
> + priority = diff >= 0 ? INT_MAX : -INT_MAX;
> +  allocno_priorities[ALLOCNO_NUM (a)] = priority;
>if (priority < 0)
>   priority = -priority;
>if (max_priority < priority)

Jakub



Re: [PATCH] Fix --help -Q output

2021-12-02 Thread Martin Liška

On 11/30/21 10:33, Richard Biener wrote:

and the "docs" say

   /* The switch is enabled when FLAG_VAR is nonzero.  */
   CLVC_BOOLEAN,


That's bogus, because real meaning of CLVC_BOOLEAN is that it holds an integer 
value.
It can be just true/false for a simple flag, can be 0,1,2 for things like 
flag_lifetime_dse,
or it can be an arbitrary integer value for things like params.

That said, I would suggest removing that..

Thoughts about the patch?
MartinFrom 7c086652b79c5ce2efcc503a2339127fc7302b20 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Mon, 29 Nov 2021 14:46:47 +0100
Subject: [PATCH] Fix --help -Q output

	PR middle-end/103438

gcc/ChangeLog:

	* config/s390/s390.c (s390_valid_target_attribute_inner_p):
	Use new enum CLVC_INTEGER.
	* opt-functions.awk: Use new CLVC_INTEGER.
	* opts-common.c (set_option): Likewise.
	(option_enabled): Return -1,0,1 for CLVC_INTEGER.
	(get_option_state): Use new CLVC_INTEGER.
	(control_warning_option): Likewise.
	* opts.h (enum cl_var_type): Likewise.
---
 gcc/config/s390/s390.c |  2 +-
 gcc/opt-functions.awk  |  2 +-
 gcc/opts-common.c  | 21 ++---
 gcc/opts.h |  4 ++--
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 510e7f58a3b..3a22f7833a9 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -15926,7 +15926,7 @@ s390_valid_target_attribute_inner_p (tree args,
 	  new_opts_set->x_target_flags |= mask;
 	}
 
-  else if (cl_options[opt].var_type == CLVC_BOOLEAN)
+  else if (cl_options[opt].var_type == CLVC_INTEGER)
 	{
 	  int value;
 
diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 9bc85604066..ffe4eb92027 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -303,7 +303,7 @@ function var_set(flags)
 		return "0, CLVC_STRING, 0"
 	if (flag_set_p("ByteSize", flags))
 		return "0, CLVC_SIZE, 0"
-	return "0, CLVC_BOOLEAN, 0"
+	return "0, CLVC_INTEGER, 0"
 }
 
 # Given that an option called NAME has flags FLAGS, return an initializer
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 9d1914ff2ff..f4b937acf33 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1458,7 +1458,7 @@ set_option (struct gcc_options *opts, struct gcc_options *opts_set,
 
   switch (option->var_type)
 {
-case CLVC_BOOLEAN:
+case CLVC_INTEGER:
 	if (option->cl_host_wide_int)
 	  {
 	*(HOST_WIDE_INT *) flag_var = value;
@@ -1586,7 +1586,8 @@ option_flag_var (int opt_index, struct gcc_options *opts)
 }
 
 /* Return 1 if option OPT_IDX is enabled in OPTS, 0 if it is disabled,
-   or -1 if it isn't a simple on-off switch.  */
+   or -1 if it isn't a simple on-off switch
+   (or if the value is unknown, typically set later in target).  */
 
 int
 option_enabled (int opt_idx, unsigned lang_mask, void *opts)
@@ -1606,11 +1607,17 @@ option_enabled (int opt_idx, unsigned lang_mask, void *opts)
   if (flag_var)
 switch (option->var_type)
   {
-  case CLVC_BOOLEAN:
+  case CLVC_INTEGER:
 	if (option->cl_host_wide_int)
-	  return *(HOST_WIDE_INT *) flag_var != 0;
+	  {
+	HOST_WIDE_INT v = *(HOST_WIDE_INT *) flag_var;
+	return v > 0 ? (v < 0 ? -1 : 1) : 0;
+	  }
 	else
-	  return *(int *) flag_var != 0;
+	  {
+	int v = *(int *) flag_var;
+	return v > 0 ? (v < 0 ? -1 : 1) : 0;
+	  }
 
   case CLVC_EQUAL:
 	if (option->cl_host_wide_int) 
@@ -1658,7 +1665,7 @@ get_option_state (struct gcc_options *opts, int option,
 
   switch (cl_options[option].var_type)
 {
-case CLVC_BOOLEAN:
+case CLVC_INTEGER:
 case CLVC_EQUAL:
 case CLVC_SIZE:
   state->data = flag_var;
@@ -1725,7 +1732,7 @@ control_warning_option (unsigned int opt_index, int kind, const char *arg,
   const struct cl_option *option = &cl_options[opt_index];
 
   /* -Werror=foo implies -Wfoo.  */
-  if (option->var_type == CLVC_BOOLEAN
+  if (option->var_type == CLVC_INTEGER
 	  || option->var_type == CLVC_ENUM
 	  || option->var_type == CLVC_SIZE)
 	{
diff --git a/gcc/opts.h b/gcc/opts.h
index f5bc9a3149c..4c2b77ec0f0 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -24,8 +24,8 @@ along with GCC; see the file COPYING3.  If not see
 
 /* Specifies how a switch's VAR_VALUE relates to its FLAG_VAR.  */
 enum cl_var_type {
-  /* The switch is enabled when FLAG_VAR is nonzero.  */
-  CLVC_BOOLEAN,
+  /* The switch is an integer value.  */
+  CLVC_INTEGER,
 
   /* The switch is enabled when FLAG_VAR == VAR_VALUE.  */
   CLVC_EQUAL,
-- 
2.34.0



Re: [PATCH] Fix --help -Q output

2021-12-02 Thread Richard Biener via Gcc-patches
On Thu, Dec 2, 2021 at 3:07 PM Martin Liška  wrote:
>
> On 11/30/21 10:33, Richard Biener wrote:
> > and the "docs" say
> >
> >/* The switch is enabled when FLAG_VAR is nonzero.  */
> >CLVC_BOOLEAN,
>
> That's bogus, because real meaning of CLVC_BOOLEAN is that it holds an 
> integer value.
> It can be just true/false for a simple flag, can be 0,1,2 for things like 
> flag_lifetime_dse,
> or it can be an arbitrary integer value for things like params.
>
> That said, I would suggest removing that..
>
> Thoughts about the patch?

+   return v > 0 ? (v < 0 ? -1 : 1) : 0;

the ?:s look odd to me, the above is equivalent to v >  0 ? 1 : 0, no?
Did you mean to make v > 0 v != 0?

> Martin


RE: [PATCH]middle-end cse: Make sure duplicate elements are not entered into the equivalence set [PR103404]

2021-12-02 Thread Tamar Christina via Gcc-patches
Hi All,

He's a respin of the patch which doesn't change the complexity of insert.

Richard S, since you approved the original patch could you take a look at this 
fix.

---

Hi All,

CSE uses equivalence classes to keep track of expressions that all have the same
values at the current point in the program.

Normal equivalences through SETs only insert and perform lookups in this set but
equivalence determined from comparisons, e.g.

(insn 46 44 47 7 (set (reg:CCZ 17 flags)
(compare:CCZ (reg:SI 105 [ iD.2893 ])
(const_int 0 [0]))) "cse.c":18:22 7 {*cmpsi_ccno_1}
 (expr_list:REG_DEAD (reg:SI 105 [ iD.2893 ])
(nil)))

creates the equivalence EQ on (reg:SI 105 [ iD.2893 ]) and (const_int 0 [0]).

This causes a merge to happen between the two equivalence sets denoted by
(const_int 0 [0]) and (reg:SI 105 [ iD.2893 ]) respectively.

The operation happens through merge_equiv_classes however this function has an
invariant that the classes to be merge not contain any duplicates.  This is
because it frees entries before merging.

The given testcase when using the supplied flags trigger an ICE due to the
equivalence set being

(rr) p dump_class (class1)
Equivalence chain for (reg:SI 105 [ iD.2893 ]):
(reg:SI 105 [ iD.2893 ])
$3 = void

(rr) p dump_class (class2)
Equivalence chain for (const_int 0 [0]):
(const_int 0 [0])
(reg:SI 97 [ _10 ])
(reg:SI 97 [ _10 ])
$4 = void

This happens because the original INSN being recorded is

(insn 18 17 24 2 (set (subreg:V1SI (reg:SI 97 [ _10 ]) 0)
(const_vector:V1SI [
(const_int 0 [0])
])) "cse.c":11:9 1363 {*movv1si_internal}
 (expr_list:REG_UNUSED (reg:SI 97 [ _10 ])
(nil)))

and we end up generating two equivalences. the first one is simply that
reg:SI 97 is 0.  The second one is that 0 can be extracted from the V1SI, so
subreg (subreg:V1SI (reg:SI 97) 0) 0 == 0.  This nested subreg gets folded away
to just reg:SI 97 and we re-insert the same equivalence.

This patch changes it so that once we figure out the bucket to insert into we
check if the equivalence set already contains the entry and if so just return
the existing entry and exit.

While doing so it also calculates the new insertion point such that this code
does not increase the worse case complexity of insert_with_costs.

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


Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

PR rtl-optimization/103404
* cse.c (insert_with_costs): Check if item exists already before adding
a new entry in the equivalence class.

gcc/testsuite/ChangeLog:

PR rtl-optimization/103404
* gcc.target/i386/pr103404.c: New test.

--- inline copy of patch ---

diff --git a/gcc/cse.c b/gcc/cse.c
index 
c1c7d0ca27b73c4b944b4719f95fece74e0358d5..be6be52376d3fb328ca6e5cced8a0456439a9a04
 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -1528,6 +1528,7 @@ insert_with_costs (rtx x, struct table_elt *classp, 
unsigned int hash,
   machine_mode mode, int cost, int reg_cost)
 {
   struct table_elt *elt;
+  struct table_elt *ins_loc = NULL, *next = NULL;
 
   /* If X is a register and we haven't made a quantity for it,
  something is wrong.  */
@@ -1537,6 +1538,51 @@ insert_with_costs (rtx x, struct table_elt *classp, 
unsigned int hash,
   if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
 add_to_hard_reg_set (&hard_regs_in_table, GET_MODE (x), REGNO (x));
 
+  /* We cannot allow a duplicate to be entered into the equivalence sets
+ and so we should perform a check before we do any allocations or
+ change the buckets and simultaneously determine final insertion
+ point.  */
+  if (classp)
+{
+  struct table_elt *p, *tmp;
+  ins_loc = classp->first_same_value;
+
+  /* Check the first element on its own.  */
+  if (exp_equiv_p (ins_loc->exp, x, 1, false))
+   return ins_loc;
+
+  /* And also cost it on its own as the conditions for it are slightly
+different from the others.  */
+  if (preferable (cost, reg_cost, ins_loc->cost, ins_loc->regcost) >= 0)
+   {
+ /* Skip over elements that are cheaper than the element being
+inserted as unequal costs means it can't be a duplicate.  */
+ for (p = ins_loc;
+  (next = p->next_same_value)
+&& preferable (next->cost, next->regcost, cost, reg_cost) < 0;
+  p = next)
+   ;
+
+ /* Record p as the place where we want to insert elt if we are to
+insert an element.  */
+ ins_loc = p;
+
+  /* The normal search stops when we encounter an element which has the 
same
+costs as us.  That's where we need to insert, but we still need to 
check
+the remaining list of equal cost things for a duplicate.  */
+  for (; p; p = tmp)
+   {
+ if (exp_equiv_p (p->exp, x, 1, false))
+   return p;
+
+ if (preferab

Re: [PATCH] Fix --help -Q output

2021-12-02 Thread Martin Liška

On 12/2/21 15:11, Richard Biener wrote:

the ?:s look odd to me, the above is equivalent to v >  0 ? 1 : 0, no?
Did you mean to make v > 0 v != 0?


Yeah, it's typo, should be 'v != 0 ? ...'.

Martin


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches



On 2021-12-02 09:00, Jakub Jelinek wrote:

On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via Gcc-patches 
wrote:

The following patch fixes

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

The patch was successfully bootstrapped and tested on x86-64. There is no
test as the bug occurs on GCC built with sanitizing for an existing go test.

I'm afraid we can't use __builtin_smul_overflow, not all system compilers
will have that.
But, as it is done in int and we kind of rely on int being 32-bit on host
and rely on long long being 64-bit, I think you can do something like:
   long long priorityll = (long long) mult * diff;
   priority = priorityll;
   if (priorityll != priority
...


My 1st version of the patch was based on long long but the standard does 
not guarantee that int size is smaller than long long size.  Although it 
is true for all targets supported by GCC.


Another solution would be to switching to int32_t instead of int for 
costs but it will require a lot of changes in RA code.


I see your point for usage system compiler different from GCC and LLVM.  
I guess I could change it to


#if __GNUC__ >= 5

current code

#else

long long code

#endif


What do you think?




Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Andrew MacLeod via Gcc-patches

On 12/2/21 06:45, Martin Liška wrote:

On 12/1/21 19:21, Andrew MacLeod wrote:

On 12/1/21 09:48, Martin Liška wrote:

On 12/1/21 15:34, Richard Biener wrote:

On Wed, Dec 1, 2021 at 3:25 PM Martin Liška  wrote:


On 12/1/21 15:19, Richard Biener wrote:
which is compute the range of 'lhs' on edge_true into 
predicate->true_range,
assign that same range to ->false_range and then invert it to get 
the
range on the false_edge.  What I am saying is that for better 
precision

you should do

   ranger->range_on_edge (predicate->false_range, edge_false, 
lhs);


rather than prematurely optimize this to the inversion of the 
true range

since yes, ranger is CFG sensitive and only the_last_ predicate on a
long CFG path is actually inverted.

What am I missing?


I might be misunderstood, but I think it's the problem defined here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584605.html

where I used the ranger->range_on_edge on the false_edge.


Ah, OK.  But then even the true_edge range is possibly wrong, no?


You are of course correct, I've just proved that in debugger ://


Consider

   for (;;)
  {
  if (a < 100)
    if (a > 50)  // unswitch on this
  /* .. */
  if (a < 120)
  /* ... */
  }

then you record [51, 99] for true_range of the a > 50 predicate and 
thus

simplification will simplify the if (a < 120) check, no?


Yep.



You can only record the range from the (CFG independent) a > 50 check,
thus [51, +INF] but of course at simplification time you can also use
the CFG context at each simplification location.


@Andrew: How can I easily get irange based just on a stmt? Something 
like fold_range

with int_range_max as the 3rd argument?


Sorry, I miss these things if I'm not directly CC'd a lot :-)

So you just want to know the basic range the stmt generates without 
context?    Sure, what you say would be fine, but your want to 
initialize it to the type range:


Yes, I want to know range of LHS in a gcond statement and the same for 
cases in a gswitch statement.




int_range_max range (TREE_TYPE (name));

you can also simply trigger it using the current SSA_NAME_RANGE_INFO 
global  values query instead of the default current contextual one... 
which , if there isnt a global range, will automatically use the 
range of the type of the argument.. so maybe just try


fold_range (r, stmt, get_global_range_query ())


Doing

  predicate->true_range = int_range_max (TREE_TYPE (lhs));
  fold_range (predicate->true_range, stmt, get_global_range_query 
());

  predicate->true_range.debug();

gives me _Bool VARYING.


wait, what  stmt are you asking for?  is this on something like:

if (a < 120)

?  Then if it doesnt now anything about 'a', you would expect to get 
bool varying because the stmt is a true/false


if you want to know the range of A on this, the instead, pick your edge, 
and ask ranger for the range of 'a' on the outgoing edge


Now, I guess what you are looking for is the range of a without any 
context?  Then you'll want to access the GORI engine directly.. try


ranger->gori().outgoing_edge_range_p (irange &r, edge e, tree name, 
*get_global_range_query ());


if you ask for 'a' on the true edge, it should give you [0,119] false 
edge should give you [120, +INF]



same thing works for switches... pick and edge and it'll give you the 
range of NAME on that edge, without any contextual information.


Anderw

PS  if you DO want contextual,  skip the final argument and it'll go 
directly to what ranger knows at this time, without any additional lookups.



Andrew




Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Jakub Jelinek via Gcc-patches
On Thu, Dec 02, 2021 at 09:23:20AM -0500, Vladimir Makarov wrote:
> 
> On 2021-12-02 09:00, Jakub Jelinek wrote:
> > On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via Gcc-patches 
> > wrote:
> > > The following patch fixes
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103437
> > > 
> > > The patch was successfully bootstrapped and tested on x86-64. There is no
> > > test as the bug occurs on GCC built with sanitizing for an existing go 
> > > test.
> > I'm afraid we can't use __builtin_smul_overflow, not all system compilers
> > will have that.
> > But, as it is done in int and we kind of rely on int being 32-bit on host
> > and rely on long long being 64-bit, I think you can do something like:
> >long long priorityll = (long long) mult * diff;
> >priority = priorityll;
> >if (priorityll != priority
> > ...
> > 
> > 
> My 1st version of the patch was based on long long but the standard does not
> guarantee that int size is smaller than long long size.  Although it is true
> for all targets supported by GCC.
> 
> Another solution would be to switching to int32_t instead of int for costs
> but it will require a lot of changes in RA code.
> 
> I see your point for usage system compiler different from GCC and LLVM.  I
> guess I could change it to
> 
> #if __GNUC__ >= 5

#ifdef __has_builtin
# if __has_builtin(__builtin_smul_overflow)
would be the best check.
And you can just gcc_assert (sizeof (long long) >= 2 * sizeof (int));
in the fallback code ;)

Jakub



Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches



On 2021-12-02 09:29, Jakub Jelinek wrote:

On Thu, Dec 02, 2021 at 09:23:20AM -0500, Vladimir Makarov wrote:

On 2021-12-02 09:00, Jakub Jelinek wrote:

On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via Gcc-patches 
wrote:

The following patch fixes

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

The patch was successfully bootstrapped and tested on x86-64. There is no
test as the bug occurs on GCC built with sanitizing for an existing go test.

I'm afraid we can't use __builtin_smul_overflow, not all system compilers
will have that.
But, as it is done in int and we kind of rely on int being 32-bit on host
and rely on long long being 64-bit, I think you can do something like:
long long priorityll = (long long) mult * diff;
priority = priorityll;
if (priorityll != priority
...



My 1st version of the patch was based on long long but the standard does not
guarantee that int size is smaller than long long size.  Although it is true
for all targets supported by GCC.

Another solution would be to switching to int32_t instead of int for costs
but it will require a lot of changes in RA code.

I see your point for usage system compiler different from GCC and LLVM.  I
guess I could change it to

#if __GNUC__ >= 5

#ifdef __has_builtin
# if __has_builtin(__builtin_smul_overflow)
would be the best check.
And you can just gcc_assert (sizeof (long long) >= 2 * sizeof (int));
in the fallback code ;)


I used static_assert in my 1st patch version.  I think it is better than 
gcc_assert..


I'll commit patch fix today.  Thank you for your feedback, Jakub.



[committed] libphobos: Push all callee-save registers on the stack before GC scan on ARM

2021-12-02 Thread Iain Buclaw via Gcc-patches
Hi,

This is the library fix for PR103520 that also prevents the garbage
collector from releasing live memory.  However this requires that the
host compiler has been patched with this fix, so the GC will remain
disabled in the D front-end for now until enough time has passed for
this to have trickled down into enough releases.

Bootstrapped and regression tested on aarch64-linux-gnu, committed to
mainline and backported to the release branches.

Regards,
Iain

---
libphobos/ChangeLog:

* libdruntime/core/thread/osthread.d (callWithStackShell): Push all
callee-save registers on the stack for AArch64 and ARM.
---
 libphobos/libdruntime/core/thread/osthread.d | 29 
 1 file changed, 29 insertions(+)

diff --git a/libphobos/libdruntime/core/thread/osthread.d 
b/libphobos/libdruntime/core/thread/osthread.d
index 653adb91a7d..c9bc1305ad0 100644
--- a/libphobos/libdruntime/core/thread/osthread.d
+++ b/libphobos/libdruntime/core/thread/osthread.d
@@ -1487,6 +1487,35 @@ in (fn)
 }}
 sp = cast(void*)®s[0];
 }
+else version (AArch64)
+{
+// Callee-save registers, x19-x28 according to AAPCS64, section
+// 5.1.1.  Include x29 fp because it optionally can be a callee
+// saved reg
+size_t[11] regs = void;
+// store the registers in pairs
+asm pure nothrow @nogc
+{
+"stp x19, x20, %0" : "=m" (regs[ 0]), "=m" (regs[1]);
+"stp x21, x22, %0" : "=m" (regs[ 2]), "=m" (regs[3]);
+"stp x23, x24, %0" : "=m" (regs[ 4]), "=m" (regs[5]);
+"stp x25, x26, %0" : "=m" (regs[ 6]), "=m" (regs[7]);
+"stp x27, x28, %0" : "=m" (regs[ 8]), "=m" (regs[9]);
+"str x29, %0"  : "=m" (regs[10]);
+"mov %0, sp"   : "=r" (sp);
+}
+}
+else version (ARM)
+{
+// Callee-save registers, according to AAPCS, section 5.1.1.
+// arm and thumb2 instructions
+size_t[8] regs = void;
+asm pure nothrow @nogc
+{
+"stm %0, {r4-r11}" : : "r" (regs.ptr) : "memory";
+"mov %0, sp"   : "=r" (sp);
+}
+}
 else
 {
 __builtin_unwind_init();
-- 
2.30.2



Re: [PATCH] c++: Handle auto(x) in parameter-declaration-clause [PR103401]

2021-12-02 Thread Marek Polacek via Gcc-patches
On Wed, Dec 01, 2021 at 11:24:58PM -0500, Jason Merrill wrote:
> On 12/1/21 10:16, Marek Polacek wrote:
> > In C++23, auto(x) is valid, so decltype(auto(x)) should also be valid,
> > so
> > 
> >void f(decltype(auto(0)));
> > 
> > should be just as
> > 
> >void f(int);
> > 
> > but currently, everytime we see 'auto' in a parameter-declaration-clause,
> > we try to synthesize_implicit_template_parm for it, creating a new template
> > parameter list.  The code above actually has us calling s_i_t_p twice;
> > once from cp_parser_decltype_expr -> cp_parser_postfix_expression which
> > fails and then again from cp_parser_decltype_expr -> cp_parser_expression.
> > So it looks like we have f and we accept ill-formed code.
> > 
> > So we need to be more careful about synthesizing the implicit template
> > parameter.  cp_parser_postfix_expression looked like a sensible place.
> 
> Does this cover other uses of auto in decltype, such as
> 
> void f(decltype(new auto{0}));

Yes: the clearing of auto_is_implicit_function_template_parm_p will happen here
too.

However, I'm noticing this:

  void f1(decltype(new auto{0}));
  void f2(decltype(new int{0}));

  void
  g ()
  {
int i;
void f3(decltype(new auto{0}));
void f4(decltype(new int{0}));
f1 (&i); // error: no matching function for call to f1(int*)
 // couldn't deduce template parameter auto:1
f2 (&i);
f3 (&i);
f4 (&i);
  }
 
I think the error we issue is bogus.  (My patch doesn't change this.  clang++
accepts.)  Should I file a PR (and investigate)?

> ?  Should we adjust this flag in cp_parser_decltype along with all the other
> flags?

I think that's possible, but wouldn't cover auto in default arguments, or array
bounds.
 
Thanks,

Marek



Re: [PATCH] gcc: vxworks: fix providing stdint.h header

2021-12-02 Thread Olivier Hainque via Gcc-patches
Hi Rasmus,

Some new on this.

> On 20 Nov 2021, at 09:07, Olivier Hainque  wrote:
> 
> I'll check how our build sequence proceeds.

Turns out our build succeeds thanks to the presence
of a vendor version of stdint.h in the VxWorks6/7 header dirs
and we're lucky that the need to pre-include yvals.h isn't
showing up during the libraries' build.

It is of course not good to use one version during the build
of libraries then install an alternate version that will be
used by programs afterwards.

The attached patch achieves the same kind of thing you
initiated, only reusing a method previously introduced
for glimits.h instead of adding a new use_gcc_stdint value,
which seems a bit less intrusive to me.

This introduces an indirect dependency on the VxWorks version.h
for vxcrtstuff objects, for which we then need to apply the same
tricks as for libgcc2 regarding include paths (to select the system
header instead of the gcc one).

I have had a few succesful builds and tests with this,
for both VxWorks 6 and VxWorks 7 configurations.

Bootstrap and regtest on x86°64-linux in progress.

cc'ed Alex who proposed the T_GLIMITS_H change.

Olivier


2021-02-12  Olivier Hainque  
Rasmus Villemoes 

gcc/
* Makefile.in (T_STDINT_GCC_H): New variable, path to
stdint-gcc.h that a target configuration may override when
use_gcc_stdint is "provide".
(stmp-int-hdrs): Depend on it and copy that for
USE_GCC_INT=provide.
* config.gcc (vxworks): Revert to use_gcc_stdint=provide.
* config/t-vxworks (T_STDINT_GCC_H): Define, as vxw-stdint-gcc.h.
(vxw-stdint-gcc.h): New target, produced from the original
stdint-gcc.h.
(vxw-glimits.h): Use an automatic variable to designate the
first and only prerequisite.
* config/vxworks/stdint.h: Remove.

libgcc/
* config/t-vxworks: Set CRTSTUFF_T_CFLAGS to
$(LIBGCC2_INCLUDES).
* config/t-vxworks7: Likewise.



0001-Provide-vxworks-alternate-stdint.h-during-the-build.patch
Description: Binary data


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Christophe Lyon via Gcc-patches
On Thu, Dec 2, 2021 at 3:38 PM Vladimir Makarov via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

>
> On 2021-12-02 09:29, Jakub Jelinek wrote:
> > On Thu, Dec 02, 2021 at 09:23:20AM -0500, Vladimir Makarov wrote:
> >> On 2021-12-02 09:00, Jakub Jelinek wrote:
> >>> On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via
> Gcc-patches wrote:
>  The following patch fixes
> 
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103437
> 
>  The patch was successfully bootstrapped and tested on x86-64. There
> is no
>  test as the bug occurs on GCC built with sanitizing for an existing
> go test.
> >>> I'm afraid we can't use __builtin_smul_overflow, not all system
> compilers
> >>> will have that.
> >>> But, as it is done in int and we kind of rely on int being 32-bit on
> host
> >>> and rely on long long being 64-bit, I think you can do something like:
> >>> long long priorityll = (long long) mult * diff;
> >>> priority = priorityll;
> >>> if (priorityll != priority
> >>> ...
> >>>
> >>>
> >> My 1st version of the patch was based on long long but the standard
> does not
> >> guarantee that int size is smaller than long long size.  Although it is
> true
> >> for all targets supported by GCC.
> >>
> >> Another solution would be to switching to int32_t instead of int for
> costs
> >> but it will require a lot of changes in RA code.
> >>
> >> I see your point for usage system compiler different from GCC and
> LLVM.  I
> >> guess I could change it to
> >>
> >> #if __GNUC__ >= 5
> > #ifdef __has_builtin
> > # if __has_builtin(__builtin_smul_overflow)
> > would be the best check.
> > And you can just gcc_assert (sizeof (long long) >= 2 * sizeof (int));
> > in the fallback code ;)
>
> I used static_assert in my 1st patch version.  I think it is better than
> gcc_assert..
>
> I'll commit patch fix today.  Thank you for your feedback, Jakub.
>
>
Thanks, I confirm I am seeing build failures with gcc-4.8.5 ;-)

Christophe


Re: [PATCH] Fix --help -Q output

2021-12-02 Thread Martin Liška

On 12/2/21 15:22, Martin Liška wrote:

On 12/2/21 15:11, Richard Biener wrote:

the ?:s look odd to me, the above is equivalent to v >  0 ? 1 : 0, no?
Did you mean to make v > 0 v != 0?


Yeah, it's typo, should be 'v != 0 ? ...'.

Martin


There's a tested patch can bootstrap on x86_64-linux-gnu and survives 
regression tests.

Ready to be installed?
Thanks,
MartinFrom d311b2370e6ab7a7aa36d666311a98d4e2ae8b35 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Mon, 29 Nov 2021 14:46:47 +0100
Subject: [PATCH] Fix --help -Q output

	PR middle-end/103438

gcc/ChangeLog:

	* config/s390/s390.c (s390_valid_target_attribute_inner_p):
	Use new enum CLVC_INTEGER.
	* opt-functions.awk: Use new CLVC_INTEGER.
	* opts-common.c (set_option): Likewise.
	(option_enabled): Return -1,0,1 for CLVC_INTEGER.
	(get_option_state): Use new CLVC_INTEGER.
	(control_warning_option): Likewise.
	* opts.h (enum cl_var_type): Likewise.
---
 gcc/config/s390/s390.c |  2 +-
 gcc/opt-functions.awk  |  2 +-
 gcc/opts-common.c  | 21 ++---
 gcc/opts.h |  4 ++--
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 510e7f58a3b..3a22f7833a9 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -15926,7 +15926,7 @@ s390_valid_target_attribute_inner_p (tree args,
 	  new_opts_set->x_target_flags |= mask;
 	}
 
-  else if (cl_options[opt].var_type == CLVC_BOOLEAN)
+  else if (cl_options[opt].var_type == CLVC_INTEGER)
 	{
 	  int value;
 
diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 9bc85604066..ffe4eb92027 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -303,7 +303,7 @@ function var_set(flags)
 		return "0, CLVC_STRING, 0"
 	if (flag_set_p("ByteSize", flags))
 		return "0, CLVC_SIZE, 0"
-	return "0, CLVC_BOOLEAN, 0"
+	return "0, CLVC_INTEGER, 0"
 }
 
 # Given that an option called NAME has flags FLAGS, return an initializer
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 9d1914ff2ff..ef2130e318f 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1458,7 +1458,7 @@ set_option (struct gcc_options *opts, struct gcc_options *opts_set,
 
   switch (option->var_type)
 {
-case CLVC_BOOLEAN:
+case CLVC_INTEGER:
 	if (option->cl_host_wide_int)
 	  {
 	*(HOST_WIDE_INT *) flag_var = value;
@@ -1586,7 +1586,8 @@ option_flag_var (int opt_index, struct gcc_options *opts)
 }
 
 /* Return 1 if option OPT_IDX is enabled in OPTS, 0 if it is disabled,
-   or -1 if it isn't a simple on-off switch.  */
+   or -1 if it isn't a simple on-off switch
+   (or if the value is unknown, typically set later in target).  */
 
 int
 option_enabled (int opt_idx, unsigned lang_mask, void *opts)
@@ -1606,11 +1607,17 @@ option_enabled (int opt_idx, unsigned lang_mask, void *opts)
   if (flag_var)
 switch (option->var_type)
   {
-  case CLVC_BOOLEAN:
+  case CLVC_INTEGER:
 	if (option->cl_host_wide_int)
-	  return *(HOST_WIDE_INT *) flag_var != 0;
+	  {
+	HOST_WIDE_INT v = *(HOST_WIDE_INT *) flag_var;
+	return v != 0 ? (v < 0 ? -1 : 1) : 0;
+	  }
 	else
-	  return *(int *) flag_var != 0;
+	  {
+	int v = *(int *) flag_var;
+	return v != 0 ? (v < 0 ? -1 : 1) : 0;
+	  }
 
   case CLVC_EQUAL:
 	if (option->cl_host_wide_int) 
@@ -1658,7 +1665,7 @@ get_option_state (struct gcc_options *opts, int option,
 
   switch (cl_options[option].var_type)
 {
-case CLVC_BOOLEAN:
+case CLVC_INTEGER:
 case CLVC_EQUAL:
 case CLVC_SIZE:
   state->data = flag_var;
@@ -1725,7 +1732,7 @@ control_warning_option (unsigned int opt_index, int kind, const char *arg,
   const struct cl_option *option = &cl_options[opt_index];
 
   /* -Werror=foo implies -Wfoo.  */
-  if (option->var_type == CLVC_BOOLEAN
+  if (option->var_type == CLVC_INTEGER
 	  || option->var_type == CLVC_ENUM
 	  || option->var_type == CLVC_SIZE)
 	{
diff --git a/gcc/opts.h b/gcc/opts.h
index f5bc9a3149c..4c2b77ec0f0 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -24,8 +24,8 @@ along with GCC; see the file COPYING3.  If not see
 
 /* Specifies how a switch's VAR_VALUE relates to its FLAG_VAR.  */
 enum cl_var_type {
-  /* The switch is enabled when FLAG_VAR is nonzero.  */
-  CLVC_BOOLEAN,
+  /* The switch is an integer value.  */
+  CLVC_INTEGER,
 
   /* The switch is enabled when FLAG_VAR == VAR_VALUE.  */
   CLVC_EQUAL,
-- 
2.34.0



Re: [PATCH] Loop unswitching: support gswitch statements.

2021-12-02 Thread Martin Liška

On 12/2/21 15:27, Andrew MacLeod wrote:

ranger->gori().outgoing_edge_range_p (irange &r, edge e, tree name, 
*get_global_range_query ());


Thank you! It works for me!

Martin


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches


On 2021-12-02 10:52, Christophe Lyon wrote:



On Thu, Dec 2, 2021 at 3:38 PM Vladimir Makarov via Gcc-patches 
 wrote:



On 2021-12-02 09:29, Jakub Jelinek wrote:
> On Thu, Dec 02, 2021 at 09:23:20AM -0500, Vladimir Makarov wrote:
>> On 2021-12-02 09:00, Jakub Jelinek wrote:
>>> On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via
Gcc-patches wrote:
 The following patch fixes

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

 The patch was successfully bootstrapped and tested on x86-64.
There is no
 test as the bug occurs on GCC built with sanitizing for an
existing go test.
>>> I'm afraid we can't use __builtin_smul_overflow, not all
system compilers
>>> will have that.
>>> But, as it is done in int and we kind of rely on int being
32-bit on host
>>> and rely on long long being 64-bit, I think you can do
something like:
>>>         long long priorityll = (long long) mult * diff;
>>>         priority = priorityll;
>>>         if (priorityll != priority
>>> ...
>>>
>>>
>> My 1st version of the patch was based on long long but the
standard does not
>> guarantee that int size is smaller than long long size. 
Although it is true
>> for all targets supported by GCC.
>>
>> Another solution would be to switching to int32_t instead of
int for costs
>> but it will require a lot of changes in RA code.
>>
>> I see your point for usage system compiler different from GCC
and LLVM.  I
>> guess I could change it to
>>
>> #if __GNUC__ >= 5
> #ifdef __has_builtin
> # if __has_builtin(__builtin_smul_overflow)
> would be the best check.
> And you can just gcc_assert (sizeof (long long) >= 2 * sizeof
(int));
> in the fallback code ;)

I used static_assert in my 1st patch version.  I think it is
better than
gcc_assert..

I'll commit patch fix today.  Thank you for your feedback, Jakub.


Thanks, I confirm I am seeing build failures with gcc-4.8.5 ;-)

I've committed the following patch with the backup code.  Sorry for 
inconvenience.


commit 0eb22e619c294efb0f007178a230cac413dccb87
Author: Vladimir N. Makarov 
Date:   Thu Dec 2 10:55:59 2021 -0500

[PR103437] Use long long multiplication as backup for overflow processing

__builtin_smul_overflow can be unavailable for some C++ compilers.
Add long long multiplication as backup for overflow processing.

gcc/ChangeLog:
PR rtl-optimization/103437
* ira-color.c (setup_allocno_priorities): Use long long
multiplication as backup for overflow processing.

diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 1f80cbea0e2..3b19a58e1f0 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2797,6 +2797,7 @@ static void
 setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
 {
   int i, length, nrefs, priority, max_priority, mult, diff;
+  bool overflow_backup_p = true;
   ira_allocno_t a;
 
   max_priority = 0;
@@ -2811,9 +2812,25 @@ setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
   diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
   /* Multiplication can overflow for very large functions.
 	 Check the overflow and constrain the result if necessary: */
+#ifdef __has_builtin
+#if __has_builtin(__builtin_smul_overflow)
+  overflow_backup_p = false;
   if (__builtin_smul_overflow (mult, diff, &priority)
 	  || priority <= -INT_MAX)
 	priority = diff >= 0 ? INT_MAX : -INT_MAX;
+#endif
+#endif
+  if (overflow_backup_p)
+	{
+	  static_assert
+	(sizeof (long long) >= 2 * sizeof (int),
+	 "overflow code does not work for such int and long long sizes");
+	  long long priorityll = (long long) mult * diff;
+	  if (priorityll < -INT_MAX || priorityll > INT_MAX)
+	priority = diff >= 0 ? INT_MAX : -INT_MAX;
+	  else
+	priority = priorityll;
+	}
   allocno_priorities[ALLOCNO_NUM (a)] = priority;
   if (priority < 0)
 	priority = -priority;


Re: [PATCH] OpenMP: Ensure that offloaded variables are public

2021-12-02 Thread Andrew Stubbs

On 02/12/2021 12:58, Jakub Jelinek wrote:

I've tried modifying offload_handle_link_vars but that spot doesn't catch
the omp_data_sizes variables emitted by libgomp.c-c++-common/target_42.c,
which was one of the motivating examples.


Why doesn't catch it?  Is the variable created only post-IPA?
I'd think that it should have been created before IPA, streamed and
therefore I don't understand why you don't see it after streaming LTO in.


On closer inspection it does, in fact, catch it as you'd expect, but 
then the variable is no longer marked public when it gets to 
pass_omp_target_link::execute, so something somewhere is resetting it. 
More investigation is needed


Andrew


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Jakub Jelinek via Gcc-patches
On Thu, Dec 02, 2021 at 11:03:46AM -0500, Vladimir Makarov wrote:
> --- a/gcc/ira-color.c
> +++ b/gcc/ira-color.c
> @@ -2797,6 +2797,7 @@ static void
>  setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
>  {
>int i, length, nrefs, priority, max_priority, mult, diff;
> +  bool overflow_backup_p = true;
>ira_allocno_t a;
>  
>max_priority = 0;
> @@ -2811,9 +2812,25 @@ setup_allocno_priorities (ira_allocno_t 
> *consideration_allocnos, int n)
>diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
>/* Multiplication can overflow for very large functions.
>Check the overflow and constrain the result if necessary: */
> +#ifdef __has_builtin
> +#if __has_builtin(__builtin_smul_overflow)
> +  overflow_backup_p = false;
>if (__builtin_smul_overflow (mult, diff, &priority)
> || priority <= -INT_MAX)
>   priority = diff >= 0 ? INT_MAX : -INT_MAX;
> +#endif
> +#endif
> +  if (overflow_backup_p)
> + {
> +   static_assert
> + (sizeof (long long) >= 2 * sizeof (int),
> +  "overflow code does not work for such int and long long sizes");
> +   long long priorityll = (long long) mult * diff;
> +   if (priorityll < -INT_MAX || priorityll > INT_MAX)
> + priority = diff >= 0 ? INT_MAX : -INT_MAX;
> +   else
> + priority = priorityll;
> + }

This will require that long long is at least twice as large as int
everywhere, I thought you wanted to do that only when
__builtin_smul_overflow isn't available.
That would be
#ifdef __has_builtin
#if __has_builtin(__builtin_smul_overflow)
#define HAS_SMUL_OVERFLOW
#endif
#endif
#ifdef HAS_SMUL_OVERFLOW
  if (__builtin_smul_overflow (mult, diff, &priority)
  || priority <= -INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
#else
  static_assert (sizeof (long long) >= 2 * sizeof (int),
 "overflow code does not work for int wider"
 "than half of long long");
  long long priorityll = (long long) mult * diff;
  if (priorityll < -INT_MAX || priorityll > INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
  else
priority = priorityll;
#endif
Why priority <= -INT_MAX in the first case though,
shouldn't that be < -INT_MAX ?

Jakub



[Ada] Fix possible memory corruption for hostnames longer than 1024 bytes

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
When a hostname has more than 1024 characters, Constraint_Error is
raised if the runtime is compiled with checks on, otherwise a memory
corruption occurs. Use the constant NI_MAXHOST to ensure that the
appropriate buffer size is allocated for the hostnames.

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

gcc/ada/

* libgnat/g-socket.ads (Max_Name_Length): Initialize with
NI_MAXHOST.diff --git a/gcc/ada/libgnat/g-socket.ads b/gcc/ada/libgnat/g-socket.ads
--- a/gcc/ada/libgnat/g-socket.ads
+++ b/gcc/ada/libgnat/g-socket.ads
@@ -1558,8 +1558,10 @@ private
 
No_Sock_Addr : constant Sock_Addr_Type := (Family_Inet, No_Inet_Addr, 0);
 
-   Max_Name_Length : constant := 64;
-   --  The constant MAXHOSTNAMELEN is usually set to 64
+   Max_Name_Length : constant := SOSC.NI_MAXHOST;
+   --  Most systems don't provide constants that specify the maximum size
+   --  of either a FQDN or a service name. In order to aid applications in
+   --  allocating buffers, the constant NI_MAXHOST is defined in .
 
subtype Name_Index is Natural range 1 .. Max_Name_Length;
 




[Ada] More intuitive names in sanity-checking of derived types

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Routine Check_Derived_Type used generic names for local variables, e.g.
Elmt, List, Subp. This patch renames them to be more intuitive, in
particular, using Derived_ and Parent_ prefixes for variables related to
derived and parent types, respectively.

Cleanup related to expansion of dispatching equality for GNATprove.

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

gcc/ada/

* sem_ch3.adb (Check_Derived_Type): Rename local variables; fix
style in comment.diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb
--- a/gcc/ada/sem_ch3.adb
+++ b/gcc/ada/sem_ch3.adb
@@ -16369,12 +16369,12 @@ package body Sem_Ch3 is
   
 
   function Check_Derived_Type return Boolean is
- E: Entity_Id;
- Elmt : Elmt_Id;
- List : Elist_Id;
- New_Subp : Entity_Id;
- Op_Elmt  : Elmt_Id;
- Subp : Entity_Id;
+ E: Entity_Id;
+ Derived_Elmt : Elmt_Id;
+ Derived_Op   : Entity_Id;
+ Derived_Ops  : Elist_Id;
+ Parent_Elmt  : Elmt_Id;
+ Parent_Op: Entity_Id;
 
   begin
  --  Traverse list of entities in the current scope searching for
@@ -16389,7 +16389,7 @@ package body Sem_Ch3 is
--  Disable this test if Derived_Type completes an incomplete
--  type because in such case more primitives can be added
--  later to the list of primitives of Derived_Type by routine
-   --  Process_Incomplete_Dependents
+   --  Process_Incomplete_Dependents.
 
return True;
 end if;
@@ -16397,13 +16397,13 @@ package body Sem_Ch3 is
 Next_Entity (E);
  end loop;
 
- List := Collect_Primitive_Operations (Derived_Type);
- Elmt := First_Elmt (List);
+ Derived_Ops := Collect_Primitive_Operations (Derived_Type);
 
- Op_Elmt := First_Elmt (Op_List);
- while Present (Op_Elmt) loop
-Subp := Node (Op_Elmt);
-New_Subp := Node (Elmt);
+ Derived_Elmt := First_Elmt (Derived_Ops);
+ Parent_Elmt  := First_Elmt (Op_List);
+ while Present (Parent_Elmt) loop
+Parent_Op  := Node (Parent_Elmt);
+Derived_Op := Node (Derived_Elmt);
 
 --  At this early stage Derived_Type has no entities with attribute
 --  Interface_Alias. In addition, such primitives are always
@@ -16411,31 +16411,31 @@ package body Sem_Ch3 is
 --  Therefore, if found we can safely stop processing pending
 --  entities.
 
-exit when Present (Interface_Alias (Subp));
+exit when Present (Interface_Alias (Parent_Op));
 
 --  Handle hidden entities
 
-if not Is_Predefined_Dispatching_Operation (Subp)
-  and then Is_Hidden (Subp)
+if not Is_Predefined_Dispatching_Operation (Parent_Op)
+  and then Is_Hidden (Parent_Op)
 then
-   if Present (New_Subp)
- and then Primitive_Names_Match (Subp, New_Subp)
+   if Present (Derived_Op)
+ and then Primitive_Names_Match (Parent_Op, Derived_Op)
then
-  Next_Elmt (Elmt);
+  Next_Elmt (Derived_Elmt);
end if;
 
 else
-   if not Present (New_Subp)
- or else Ekind (Subp) /= Ekind (New_Subp)
- or else not Primitive_Names_Match (Subp, New_Subp)
+   if No (Derived_Op)
+ or else Ekind (Parent_Op) /= Ekind (Derived_Op)
+ or else not Primitive_Names_Match (Parent_Op, Derived_Op)
then
   return False;
end if;
 
-   Next_Elmt (Elmt);
+   Next_Elmt (Derived_Elmt);
 end if;
 
-Next_Elmt (Op_Elmt);
+Next_Elmt (Parent_Elmt);
  end loop;
 
  return True;




[Ada] Proof of Boolean'Image and Boolean'Value

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This proves the functionality of the runtime support for these
attributes that print and parse a Boolean value.

As a side-effect of this proof, fix a possible range check failure in
System.Val_Bool.Value_Boolean and a possible overflow check failure in
System.Val_Util.Normalize_String.

GNATprove is run with switches --level=2.

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

gcc/ada/

* libgnat/s-imgboo.adb: Mark in SPARK.
* libgnat/s-imgboo.ads: Mark in SPARK. Change from Pure to
Preelaborate unit in order to be able to depend on
System.Val_Bool.
(Image_Boolean): Functionally specify the result of the
procedure by calling System.Val_Bool.Value_Boolean on the
result.
* libgnat/s-valboo.adb: Mark in SPARK.
(First_Non_Space_Ghost): New ghost function.
(Value_Boolean): Change type of L and F to avoid possible range
check failure on empty Str.
* libgnat/s-valboo.ads: Mark in SPARK. Duplicate with-clause
from body in the spec to be able to call
System.Val_Util.Only_Space_Ghost in the contract.
(First_Non_Space_Ghost): New ghost function computing the first
non-space character in a string.
(Is_Boolean_Image_Ghost): New ghost function computing whether a
string is the image of a boolean value.
(Value_Boolean): Add in precondition the conditions to avoid
raising Constraint_Error. This precondition is never executed,
and only used in proof, thanks to the use of pragma
Assertion_Policy. Given that precondition, the postcondition can
simply check the first non-space character to decide whether
True or False is read.
* libgnat/s-valuti.adb: Mark in SPARK, but use SPARK_Mode Off on
all subprograms not yet proved.
(Bad_Value): Annotate expected exception.
(Normalize_String): Rewrite to avoid possible overflow when
incrementing F in the first loop. Add loop invariants.
* libgnat/s-valuti.ads: Mark in SPARK.
(Bad_Value): Add Depends contract to avoid warning on unused S.
(Only_Space_Ghost): New ghost function to query if string has
only space in the specified range.
(Normalize_String): Add functional contract.
(Scan_Exponent): Mark spec as not in SPARK as this function has
side-effects.diff --git a/gcc/ada/libgnat/s-imgboo.adb b/gcc/ada/libgnat/s-imgboo.adb
--- a/gcc/ada/libgnat/s-imgboo.adb
+++ b/gcc/ada/libgnat/s-imgboo.adb
@@ -29,7 +29,17 @@
 --  --
 --
 
-package body System.Img_Bool is
+--  Ghost code, loop invariants and assertions in this unit are meant for
+--  analysis only, not for run-time checking, as it would be too costly
+--  otherwise. This is enforced by setting the assertion policy to Ignore.
+
+pragma Assertion_Policy (Ghost  => Ignore,
+ Loop_Invariant => Ignore,
+ Assert => Ignore);
+
+package body System.Img_Bool
+  with SPARK_Mode
+is
 
---
-- Image_Boolean --


diff --git a/gcc/ada/libgnat/s-imgboo.ads b/gcc/ada/libgnat/s-imgboo.ads
--- a/gcc/ada/libgnat/s-imgboo.ads
+++ b/gcc/ada/libgnat/s-imgboo.ads
@@ -31,13 +31,33 @@
 
 --  Boolean'Image
 
-package System.Img_Bool is
-   pragma Pure;
+--  Preconditions in this unit are meant for analysis only, not for run-time
+--  checking, so that the expected exceptions are raised. This is enforced by
+--  setting the corresponding assertion policy to Ignore. Postconditions and
+--  contract cases should not be executed at runtime as well, in order not to
+--  slow down the execution of these functions.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Contract_Cases => Ignore,
+ Ghost  => Ignore);
+
+with System.Val_Bool;
+
+package System.Img_Bool
+  with SPARK_Mode, Preelaborate
+is
 
procedure Image_Boolean
  (V : Boolean;
   S : in out String;
-  P : out Natural);
+  P : out Natural)
+   with
+ Pre  => S'First = 1
+   and then (if V then S'Length >= 4 else S'Length >= 5),
+ Post => (if V then P = 4 else P = 5)
+   and then System.Val_Bool.Is_Boolean_Image_Ghost (S (1 .. P))
+   and then System.Val_Bool.Value_Boolean (S (1 .. P)) = V;
--  Computes Boolean'Image (V) and stores the result in S (1 .. P)
--  setting the resulting value of P. The caller guarantees that S
--  is long enough to hold the result, and that S'First is 1.


diff --git a/gcc/ada/libgnat/s-valboo.adb b/gcc/ada/libgnat/s-valboo.adb
--- a/gcc/ada/libgnat/s-valboo.adb
+++ b/gcc/ada/libgnat/s-valboo.adb
@@ -29,22 +29,47 @@
 --  --
 

[Ada] Proof of System.Val_Util utilities for 'Value support

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This proves the functionality of the utility procedures supporting the
implementation of 'Value attributes.

As a side-effect of this proof, fix possible range check failures.

GNATprove is run with switch --level=3.

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

gcc/ada/

* libgnat/s-valboo.adb (First_Non_Space_Ghost): Move to
utilities.
(Value_Boolean): Prefix call to First_Non_Space_Ghost.
* libgnat/s-valboo.ads (First_Non_Space_Ghost): Move to
utilities.
(Is_Boolean_Image_Ghost, Value_Boolean): Prefix call to
First_Non_Space_Ghost.
* libgnat/s-valuer.adb (Scan_Raw_Real): Adapt to change of
function Scan_Exponent to procedure.
* libgnat/s-valueu.adb (Scan_Raw_Unsigned): Adapt to change of
function Scan_Exponent to procedure.
* libgnat/s-valuti.adb (First_Non_Space_Ghost): Function moved
here.
(Last_Number_Ghost): New ghost query function.
(Scan_Exponent): Change function with side-effects into
procedure, to mark in SPARK. Prove procedure wrt contract.
Change type of local P to avoid possible range check failure (it
is not known whether this can be activated by callers).
(Scan_Plus_Sign, Scan_Sign): Change type of local P to avoid
possible range check failure. Add loop invariants and assertions
for proof.
(Scan_Trailing_Blanks): Add loop invariant.
(Scan_Underscore): Remove SPARK_Mode Off.
* libgnat/s-valuti.ads (First_Non_Space_Ghost): Function moved
here.
(Last_Number_Ghost, Only_Number_Ghost, Is_Natural_Format_Ghost,
Scan_Natural_Ghost): New ghost query functions.
(Scan_Plus_Sign, Scan_Sign, Scan_Exponent, Scan_Trailing_Blanks,
Scan_Underscore): Add functional contracts.diff --git a/gcc/ada/libgnat/s-valboo.adb b/gcc/ada/libgnat/s-valboo.adb
--- a/gcc/ada/libgnat/s-valboo.adb
+++ b/gcc/ada/libgnat/s-valboo.adb
@@ -43,19 +43,6 @@ package body System.Val_Bool
   with SPARK_Mode
 is
 
-   function First_Non_Space_Ghost (S : String) return Positive is
-   begin
-  for J in S'Range loop
- if S (J) /= ' ' then
-return J;
- end if;
-
- pragma Loop_Invariant (for all K in S'First .. J => S (K) = ' ');
-  end loop;
-
-  raise Program_Error;
-   end First_Non_Space_Ghost;
-
---
-- Value_Boolean --
---
@@ -68,7 +55,7 @@ is
begin
   Normalize_String (S, F, L);
 
-  pragma Assert (F = First_Non_Space_Ghost (S));
+  pragma Assert (F = System.Val_Util.First_Non_Space_Ghost (S));
 
   if S (F .. L) = "TRUE" then
  return True;


diff --git a/gcc/ada/libgnat/s-valboo.ads b/gcc/ada/libgnat/s-valboo.ads
--- a/gcc/ada/libgnat/s-valboo.ads
+++ b/gcc/ada/libgnat/s-valboo.ads
@@ -47,22 +47,11 @@ package System.Val_Bool
 is
pragma Preelaborate;
 
-   function First_Non_Space_Ghost (S : String) return Positive
-   with
- Ghost,
- Pre  => not System.Val_Util.Only_Space_Ghost (S, S'First, S'Last),
- Post => First_Non_Space_Ghost'Result in S'Range
-   and then S (First_Non_Space_Ghost'Result) /= ' '
-   and then System.Val_Util.Only_Space_Ghost
- (S, S'First, First_Non_Space_Ghost'Result - 1);
-   --  Ghost function that returns the index of the first non-space character
-   --  in S, which necessarily exists given the precondition on S.
-
function Is_Boolean_Image_Ghost (Str : String) return Boolean is
  (not System.Val_Util.Only_Space_Ghost (Str, Str'First, Str'Last)
 and then
   (declare
- F : constant Positive := First_Non_Space_Ghost (Str);
+ F : constant Positive := System.Val_Util.First_Non_Space_Ghost (Str);
begin
  (F <= Str'Last - 3
   and then Str (F) in 't' | 'T'
@@ -92,7 +81,8 @@ is
with
  Pre  => Is_Boolean_Image_Ghost (Str),
  Post =>
-   Value_Boolean'Result = (Str (First_Non_Space_Ghost (Str)) in 't' | 'T');
+   Value_Boolean'Result =
+ (Str (System.Val_Util.First_Non_Space_Ghost (Str)) in 't' | 'T');
--  Computes Boolean'Value (Str)
 
 end System.Val_Bool;


diff --git a/gcc/ada/libgnat/s-valuer.adb b/gcc/ada/libgnat/s-valuer.adb
--- a/gcc/ada/libgnat/s-valuer.adb
+++ b/gcc/ada/libgnat/s-valuer.adb
@@ -511,6 +511,8 @@ package body System.Value_R is
   Value : Uns;
   --  Mantissa as an Integer
 
+  Expon : Integer;
+
begin
   --  The default base is 10
 
@@ -643,7 +645,8 @@ package body System.Value_R is
   --  Update pointer and scan exponent
 
   Ptr.all := Index;
-  Scale := Scale + Scan_Exponent (Str, Ptr, Max, Real => True);
+  Scan_Exponent (Str, Ptr, Max, Expon, Real => True);
+  Scale := Scale + Expon;
 
   --  Here is where we check for a bad based number
 


diff --git a/gcc/ada/libgnat/s-valueu.adb b/gcc/ada/libgnat/s-valueu.adb
--- a/gcc/ada/libgnat/s-valueu.adb
+++ b/gcc/ada/libg

[Ada] Add contract to Ada.Task_Identification.Activation_Is_Complete

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Description of Activation_Is_Complete was amended in AI12-0231-1. This
routine raises a Program_Error when called with Null_Task_Id. Add an
explicit contract to make GNATprove aware of the restriction.

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

gcc/ada/

* libgnarl/a-taside.ads (Activation_Is_Complete): Add
precondition.diff --git a/gcc/ada/libgnarl/a-taside.ads b/gcc/ada/libgnarl/a-taside.ads
--- a/gcc/ada/libgnarl/a-taside.ads
+++ b/gcc/ada/libgnarl/a-taside.ads
@@ -92,6 +92,7 @@ is
 
function Activation_Is_Complete (T : Task_Id) return Boolean with
  Volatile_Function,
+ Pre=> T /= Null_Task_Id,
  Global => Tasking_State;
 
 private




[Ada] Proof of Interfaces.C with SPARK

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This proves the functionality of Interfaces.C with GNATprove.  Ada RM
specifications are added as comments in the spec, next to the
formalization of specifications as contracts.

As a side-effect of this proof, fix two errors in the 4 procedure
versions of To_Ada, which may raise a range check failure and an
overflow check failure.

GNATprove is run with switches --level=4 --timeout=120.

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

gcc/ada/

* libgnat/i-c.adb: Add ghost code.
(C_Length_Ghost): New ghost functions to query the C length of a
string.
(To_Ada): Insert constant Count_Cst where needed to comply with
SPARK.  Homogeneize code between variants for char, wchar_t,
char16_t and char32_t. Use char16_nul and char32_nul
systematically instead of their value. Fix the type of index To
to be Integer instead of Positive, to avoid a possible range
check failure on an empty Target. Insert an exit statement to
avoid a possible overflow failure when the last index in Target
is Natural'Last (possibly on a small string as well).
* libgnat/i-c.ads: Add contracts.
(C_Length_Ghost): New ghost functions to query the C length of a
string.
* libgnat/s-os_lib.adb: Remove pragma Compiler_Unit_Warning
causing a spurious error during compilation of GNAT, as this
pragma is not needed anymore now that we bootstrap (stage1) with
the base compiler runtime.

patch.diff.gz
Description: application/gzip


[Ada] vx7r2cert/light-tasking-rtp: undefined refs on ppc/ppc64

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Vx7r2cert ppc/ppc64 targets use the gcc toolchain, which requires a
slightly different forumulation of libraries for the
rts-light-tasking-rtp runtime.

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

gcc/ada/

* vxworks7-cert-rtp-link__ppcXX.spec: New file.
* Makefile.rtl: Use it.diff --git a/gcc/ada/Makefile.rtl b/gcc/ada/Makefile.rtl
--- a/gcc/ada/Makefile.rtl
+++ b/gcc/ada/Makefile.rtl
@@ -1186,7 +1186,7 @@ ifeq ($(strip $(filter-out powerpc% wrs vxworks vxworksspe vxworks7% vxworks7spe
 else
   GCC_SPEC_FILES+=vxworks7-rtp-base-link.spec
 endif
-GCC_SPEC_FILES+=vxworks7-cert-rtp-link.spec
+GCC_SPEC_FILES+=vxworks7-cert-rtp-link__ppcXX.spec
   else
 GCC_SPEC_FILES+=vxworks-$(ARCH_STR)-link.spec
 GCC_SPEC_FILES+=vxworks-cert-$(ARCH_STR)-link.spec


diff --git /dev/null b/gcc/ada/vxworks7-cert-rtp-link__ppcXX.spec
new file mode 100644
--- /dev/null
+++ b/gcc/ada/vxworks7-cert-rtp-link__ppcXX.spec
@@ -0,0 +1,10 @@
+*self_spec:
++ %{!nostdlib:-nodefaultlibs -nostartfiles}
+
+*link:
++ %{!nostdlib:%{mrtp:%{!shared: \
+ %(base_link) \
+ -lcert -lgnu \
+ -L%:getenv(VSB_DIR /usr/lib/common/objcert) \
+ -T%:getenv(VSB_DIR /usr/ldscripts/rtp.ld) \
+   }}}




[Ada] Remove duplicated condition in warnings about read-before-write

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Code for warnings about read-before-write of a variable had one
condition in the outer IF statement and then the very same condition in
an inner IF statement.

Cleanup related to spurious warning on 'Initialized.

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

gcc/ada/

* sem_warn.adb (Check_References): Remove redundant condition.diff --git a/gcc/ada/sem_warn.adb b/gcc/ada/sem_warn.adb
--- a/gcc/ada/sem_warn.adb
+++ b/gcc/ada/sem_warn.adb
@@ -1569,15 +1569,13 @@ package body Sem_Warn is
 --  For access types, UR was only set for dereferences,
 --  so the issue is that the value may be null.
 
-if not Is_Trivial_Subprogram (Scope (E1)) then
-   if not Warnings_Off_E1 then
-  if Is_Access_Type (Etype (Parent (UR))) then
- Error_Msg_N ("??`&.&` may be null!", UR);
-  else
- Error_Msg_N
-   ("??`&.&` may be referenced before "
-& "it has a value!", UR);
-  end if;
+if not Warnings_Off_E1 then
+   if Is_Access_Type (Etype (Parent (UR))) then
+  Error_Msg_N ("??`&.&` may be null!", UR);
+   else
+  Error_Msg_N
+("??`&.&` may be referenced before "
+ & "it has a value!", UR);
end if;
 end if;
 




[Ada] Simplify iteration over record components

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Iterate over record components with First_Component/Next_Component and
not with First_Entity/Next_Entity.

Change in Sem_Warn unit is related to spurious warning on 'Initialized;
other occurrences of the same pattern were found with grep.

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

gcc/ada/

* freeze.adb (Freeze_Entity): Replace First_Entity/Next_Entity
with First_Component/Next_Component; remove condition with Ekind
equal to E_Component.
* sem_ch13.adb (Check_Record_Representation_Clause): Likewise
for component-or-discriminant.
* sem_util.adb (Is_Fully_Initialized_Type): Likewise; rename Ent
to a more specific Comp.
* sem_warn.adb (Check_References): Likewise.diff --git a/gcc/ada/freeze.adb b/gcc/ada/freeze.adb
--- a/gcc/ada/freeze.adb
+++ b/gcc/ada/freeze.adb
@@ -6328,11 +6328,9 @@ package body Freeze is
 --  to the components of Rec.
 
  begin
-Comp := First_Entity (E);
+Comp := First_Component (E);
 while Present (Comp) loop
-   if Ekind (Comp) = E_Component
- and then Has_Delayed_Aspects (Comp)
-   then
+   if Has_Delayed_Aspects (Comp) then
   if not Rec_Pushed then
  Push_Scope (E);
  Rec_Pushed := True;
@@ -6348,7 +6346,7 @@ package body Freeze is
   Analyze_Aspects_At_Freeze_Point (Comp);
end if;
 
-   Next_Entity (Comp);
+   Next_Component (Comp);
 end loop;
 
 --  Pop the scope if Rec scope has been pushed on the scope stack


diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb
--- a/gcc/ada/sem_ch13.adb
+++ b/gcc/ada/sem_ch13.adb
@@ -12284,26 +12284,18 @@ package body Sem_Ch13 is
 --  Find maximum bit of any component of the parent type
 
 Parent_Last_Bit := UI_From_Int (System_Address_Size - 1);
-Pcomp := First_Entity (Tagged_Parent);
+Pcomp := First_Component_Or_Discriminant (Tagged_Parent);
 while Present (Pcomp) loop
-   if Ekind (Pcomp) in E_Discriminant | E_Component then
-  if Present (Component_Bit_Offset (Pcomp))
-and then Known_Static_Esize (Pcomp)
-  then
- Parent_Last_Bit :=
-   UI_Max
- (Parent_Last_Bit,
-  Component_Bit_Offset (Pcomp) + Esize (Pcomp) - 1);
-  end if;
-   else
-
-  --  Skip anonymous types generated for constrained array
-  --  or record components.
-
-  null;
+   if Present (Component_Bit_Offset (Pcomp))
+ and then Known_Static_Esize (Pcomp)
+   then
+  Parent_Last_Bit :=
+UI_Max
+  (Parent_Last_Bit,
+   Component_Bit_Offset (Pcomp) + Esize (Pcomp) - 1);
end if;
 
-   Next_Entity (Pcomp);
+   Next_Component_Or_Discriminant (Pcomp);
 end loop;
  end if;
   end;


diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -17803,15 +17803,14 @@ package body Sem_Util is
  --  Otherwise see if all record components are initialized
 
  declare
-Ent : Entity_Id;
+Comp : Entity_Id;
 
  begin
-Ent := First_Entity (Typ);
-while Present (Ent) loop
-   if Ekind (Ent) = E_Component
- and then (No (Parent (Ent))
-or else No (Expression (Parent (Ent
- and then not Is_Fully_Initialized_Type (Etype (Ent))
+Comp := First_Component (Typ);
+while Present (Comp) loop
+   if (No (Parent (Comp))
+or else No (Expression (Parent (Comp
+ and then not Is_Fully_Initialized_Type (Etype (Comp))
 
   --  Special VM case for tag components, which need to be
   --  defined in this case, but are never initialized as VMs
@@ -17819,12 +17818,12 @@ package body Sem_Util is
   --  uninitialized case. Note that this applies both to the
   --  uTag entry and the main vtable pointer (CPP_Class case).
 
- and then (Tagged_Type_Expansion or else not Is_Tag (Ent))
+ and then (Tagged_Type_Expansion or else not Is_Tag (Comp))
then
   return False;
end if;
 
-   Next_Entity (Ent);
+   Next_Component (Comp);
 end loop;
  end;
 


diff --git a/gcc/ada/sem_warn.adb b/gcc/ada/sem_warn.adb
--- a/gcc/ada/sem_warn.adb
+++ b/gcc/ada/sem_warn.adb
@@ -1548,18 +1548,17 @@ package body Se

[Ada] Don't allow entry in implicit with chain to be ghost

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
When we add an entry to the implicit with chain of a unit while adding
an RTS unit, we must not mark it as a ignored ghost statement because it
points to the next with in the chain.

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

gcc/ada/

* rtsfind.adb (Maybe_Add_With): Ensure that the added "with" is
never marked as ignored ghost code.diff --git a/gcc/ada/rtsfind.adb b/gcc/ada/rtsfind.adb
--- a/gcc/ada/rtsfind.adb
+++ b/gcc/ada/rtsfind.adb
@@ -1248,9 +1248,10 @@ package body Rtsfind is
   --  for this unit to the current compilation unit.
 
   declare
- LibUnit : constant Node_Id := Unit (Cunit (U.Unum));
- Clause  : Node_Id;
- Withn   : Node_Id;
+ LibUnit  : constant Node_Id := Unit (Cunit (U.Unum));
+ Saved_GM : constant Ghost_Mode_Type := Ghost_Mode;
+ Clause   : Node_Id;
+ Withn: Node_Id;
 
   begin
  Clause := U.First_Implicit_With;
@@ -1262,11 +1263,18 @@ package body Rtsfind is
 Clause := Next_Implicit_With (Clause);
  end loop;
 
+ --  We want to make sure that the "with" we create below isn't
+ --  marked as ignored ghost code because this list may be walked
+ --  later, after ignored ghost code is converted to a null
+ --  statement.
+
+ Ghost_Mode := None;
  Withn :=
Make_With_Clause (Standard_Location,
  Name =>
Make_Unit_Name
  (U, Defining_Unit_Name (Specification (LibUnit;
+ Ghost_Mode := Saved_GM;
 
  Set_Corresponding_Spec  (Withn, U.Entity);
  Set_First_Name  (Withn);




[Ada] Refactor nested loops in warning on unassigned out parameter

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
For warning about unassigned out parameter we had a loop over all formal
parameters which contained another loop over all formal parameters. This
was inefficient.

Cleanup related to spurious warnings about 'Initialized.

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

gcc/ada/

* sem_warn.adb (Warn_On_Unassigned_Out_Parameter): Move inner
loop at the beginning of subprogram, so it is executed only
once; fix order in the "add an ad hoc" phrase.diff --git a/gcc/ada/sem_warn.adb b/gcc/ada/sem_warn.adb
--- a/gcc/ada/sem_warn.adb
+++ b/gcc/ada/sem_warn.adb
@@ -4237,8 +4237,7 @@ package body Sem_Warn is
  (Return_Node : Node_Id;
   Scope_Id: Entity_Id)
is
-  Form  : Entity_Id;
-  Form2 : Entity_Id;
+  Form : Entity_Id;
 
begin
   --  Ignore if procedure or return statement does not come from source
@@ -4249,6 +4248,26 @@ package body Sem_Warn is
  return;
   end if;
 
+  --  Before we issue the warning, add an ad hoc defence against the most
+  --  common case of false positives with this warning which is the case
+  --  where there is a Boolean OUT parameter that has been set, and whose
+  --  meaning is "ignore the values of the other parameters". We can't of
+  --  course reliably tell this case at compile time, but the following
+  --  test kills a lot of false positives, without generating a significant
+  --  number of false negatives (missed real warnings).
+
+  Form := First_Formal (Scope_Id);
+  while Present (Form) loop
+ if Ekind (Form) = E_Out_Parameter
+   and then Root_Type (Etype (Form)) = Standard_Boolean
+   and then not Never_Set_In_Source_Check_Spec (Form)
+ then
+return;
+ end if;
+
+ Next_Formal (Form);
+  end loop;
+
   --  Loop through formals
 
   Form := First_Formal (Scope_Id);
@@ -4263,27 +4282,6 @@ package body Sem_Warn is
and then Is_Scalar_Type (Etype (Form))
and then not Present (Unset_Reference (Form))
  then
---  Before we issue the warning, an add ad hoc defence against the
---  most common case of false positives with this warning which is
---  the case where there is a Boolean OUT parameter that has been
---  set, and whose meaning is "ignore the values of the other
---  parameters". We can't of course reliably tell this case at
---  compile time, but the following test kills a lot of false
---  positives, without generating a significant number of false
---  negatives (missed real warnings).
-
-Form2 := First_Formal (Scope_Id);
-while Present (Form2) loop
-   if Ekind (Form2) = E_Out_Parameter
- and then Root_Type (Etype (Form2)) = Standard_Boolean
- and then not Never_Set_In_Source_Check_Spec (Form2)
-   then
-  return;
-   end if;
-
-   Next_Formal (Form2);
-end loop;
-
 --  Here all conditions are met, record possible unset reference
 
 Set_Unset_Reference (Form, Return_Node);




[Ada] Split spec and body of expression function with Subprogram_Variant

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Due to a latent bug with freezing, the expression function with
Subprogram_Variant which was recently added to the System.Val_Util unit
triggers a crash in CodePeer.

Ordinary compilation was not affected by this bug, because of the
Assertion_Policy (Ghost => Ignore) applied to this unit.

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

gcc/ada/

* libgnat/s-valuti.ads (Scan_Natural_Ghost): Split body from
spec and put it into private part, so that GNATprove can pick it
both when analysing the unit and its clients.diff --git a/gcc/ada/libgnat/s-valuti.ads b/gcc/ada/libgnat/s-valuti.ads
--- a/gcc/ada/libgnat/s-valuti.ads
+++ b/gcc/ada/libgnat/s-valuti.ads
@@ -218,21 +218,6 @@ is
   P   : Natural;
   Acc : Natural)
   return Natural
-   is
- (if Str (P) = '_' then
-Scan_Natural_Ghost (Str, P + 1, Acc)
-  else
-(declare
-   Shift_Acc : constant Natural :=
- Acc * 10 + (Character'Pos (Str (P)) - Character'Pos ('0'));
- begin
-   (if P = Str'Last
-  or else Str (P + 1) not in '0' .. '9' | '_'
-  or else Shift_Acc >= Integer'Last / 10
-then
-  Shift_Acc
-else
-  Scan_Natural_Ghost (Str, P + 1, Shift_Acc
with
  Ghost,
  Subprogram_Variant => (Increases => P),
@@ -352,4 +337,31 @@ is
--  no check for this case, the caller must ensure this condition is met.
pragma Warnings (GNATprove, On, """Ptr"" is not modified");
 
+private
+
+   
+   -- Scan_Natural_Ghost --
+   
+
+   function Scan_Natural_Ghost
+ (Str : String;
+  P   : Natural;
+  Acc : Natural)
+  return Natural
+   is
+ (if Str (P) = '_' then
+Scan_Natural_Ghost (Str, P + 1, Acc)
+  else
+(declare
+   Shift_Acc : constant Natural :=
+ Acc * 10 + (Character'Pos (Str (P)) - Character'Pos ('0'));
+ begin
+   (if P = Str'Last
+  or else Str (P + 1) not in '0' .. '9' | '_'
+  or else Shift_Acc >= Integer'Last / 10
+then
+  Shift_Acc
+else
+  Scan_Natural_Ghost (Str, P + 1, Shift_Acc;
+
 end System.Val_Util;




[Ada] Separate building of equality from other dispatching routines

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
For GNAT we create dispatching equality together with other dispatching
routines (e.g. for stream reading and writing). For GNATprove we need to
create only the dispatching equality. This patch separates it, so that
it can be created from a SPARK-specific expansion.

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

gcc/ada/

* exp_ch3.adb (Make_Predefined_Primitive_Specs): Move code for
spec of dispatching equality.
(Predefined_Primitive_Bodies): Move code for body if dispatching
equality.
(Make_Predefined_Primitive_Eq_Spec): Separated code for spec of
dispatching equality.
(Predefined_Primitive_Eq_Body): Separated code for body of
dispatching equality.
* exp_ch3.ads: Update.diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -287,13 +287,7 @@ package body Exp_Ch3 is
--  controlled components that require finalization actions (the deep
--  in the name refers to the fact that the action applies to components).
--
-   --  The list is returned in Predef_List. The Parameter Renamed_Eq either
-   --  returns the value Empty, or else the defining unit name for the
-   --  predefined equality function in the case where the type has a primitive
-   --  operation that is a renaming of predefined equality (but only if there
-   --  is also an overriding user-defined equality function). The returned
-   --  Renamed_Eq will be passed to the corresponding parameter of
-   --  Predefined_Primitive_Bodies.
+   --  The list of specs is returned in Predef_List
 
function Has_New_Non_Standard_Rep (T : Entity_Id) return Boolean;
--  Returns True if there are representation clauses for type T that are not
@@ -10366,13 +10360,13 @@ package body Exp_Ch3 is
   return Decl_List;
end Make_Null_Procedure_Specs;
 
-   -
-   -- Make_Predefined_Primitive_Specs --
-   -
+   ---
+   -- Make_Predefined_Primitive_Eq_Spec --
+   ---
 
-   procedure Make_Predefined_Primitive_Specs
+   procedure Make_Predefined_Primitive_Eq_Spec
  (Tag_Typ : Entity_Id;
-  Predef_List : out List_Id;
+  Predef_List : List_Id;
   Renamed_Eq  : out Entity_Id)
is
   function Is_Predefined_Eq_Renaming (Prim : Node_Id) return Boolean;
@@ -10394,10 +10388,10 @@ package body Exp_Ch3 is
 
   --  Local variables
 
-  Loc   : constant Source_Ptr := Sloc (Tag_Typ);
-  Res   : constant List_Id:= New_List;
-  Eq_Name   : Name_Id := Name_Op_Eq;
-  Eq_Needed : Boolean;
+  Loc : constant Source_Ptr := Sloc (Tag_Typ);
+
+  Eq_Name   : Name_Id := Name_Op_Eq;
+  Eq_Needed : Boolean := True;
   Eq_Spec   : Node_Id;
   Prim  : Elmt_Id;
 
@@ -10405,10 +10399,141 @@ package body Exp_Ch3 is
   --  Set to True if Tag_Typ has a primitive that renames the predefined
   --  equality operator. Used to implement (RM 8-5-4(8)).
 
-  use Exp_Put_Image;
-
--  Start of processing for Make_Predefined_Primitive_Specs
 
+   begin
+  Renamed_Eq := Empty;
+
+  Prim := First_Elmt (Primitive_Operations (Tag_Typ));
+  while Present (Prim) loop
+
+ --  If a primitive is encountered that renames the predefined equality
+ --  operator before reaching any explicit equality primitive, then we
+ --  still need to create a predefined equality function, because calls
+ --  to it can occur via the renaming. A new name is created for the
+ --  equality to avoid conflicting with any user-defined equality.
+ --  (Note that this doesn't account for renamings of equality nested
+ --  within subpackages???)
+
+ if Is_Predefined_Eq_Renaming (Node (Prim)) then
+Has_Predef_Eq_Renaming := True;
+Eq_Name := New_External_Name (Chars (Node (Prim)), 'E');
+
+ --  User-defined equality
+
+ elsif Is_User_Defined_Equality (Node (Prim)) then
+if No (Alias (Node (Prim)))
+  or else Nkind (Unit_Declaration_Node (Node (Prim))) =
+N_Subprogram_Renaming_Declaration
+then
+   Eq_Needed := False;
+   exit;
+
+--  If the parent is not an interface type and has an abstract
+--  equality function explicitly defined in the sources, then the
+--  inherited equality is abstract as well, and no body can be
+--  created for it.
+
+elsif not Is_Interface (Etype (Tag_Typ))
+  and then Present (Alias (Node (Prim)))
+  and then Comes_From_Source (Alias (Node (Prim)))
+  and then Is_Abstract_Subprogram (Alias (Node (Prim)))
+then
+   Eq_Needed := False;
+   exit;
+
+--  If the type has an equality func

[Ada] Enable expansion of dispatching equality for GNATprove

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
For GNAT the dispatching equality is generated in expansion of freezing
nodes. For GNATprove this expansion needs to be specifically enabled and
must occur in a carefully setup context.

Previously type freezing for GNATprove only involved building of the DIC
procedure, which didn't require much context. Now freezing also involves
the dispatching equality, which must be analyzed in a full context with
a proper visibility (as otherwise the generated equality will duplicate
the one created when a type is derived).

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

gcc/ada/

* exp_ch13.ads (Expand_N_Freeze_Entity): Add note about a SPARK
twin.
* exp_ch3.ads (Freeze_Type): Likewise.
* exp_spark.adb (Expand_SPARK_N_Freeze_Entity): Mimic what is
done in Freeze_Entity.
(SPARK_Freeze_Type): Mimic what is done in Freeze_Type; add call
to Make_Predefined_Primitive_Eq_Spec.diff --git a/gcc/ada/exp_ch13.ads b/gcc/ada/exp_ch13.ads
--- a/gcc/ada/exp_ch13.ads
+++ b/gcc/ada/exp_ch13.ads
@@ -32,6 +32,9 @@ package Exp_Ch13 is
procedure Expand_N_Attribute_Definition_Clause  (N : Node_Id);
procedure Expand_N_Free_Statement   (N : Node_Id);
procedure Expand_N_Freeze_Entity(N : Node_Id);
+   --  Note: for GNATprove we have a minimal variant of this routine in
+   --  Exp_SPARK.Expand_SPARK_N_Freeze_Entity. They need to be kept in sync.
+
procedure Expand_N_Record_Representation_Clause (N : Node_Id);
 
 end Exp_Ch13;


diff --git a/gcc/ada/exp_ch3.ads b/gcc/ada/exp_ch3.ads
--- a/gcc/ada/exp_ch3.ads
+++ b/gcc/ada/exp_ch3.ads
@@ -114,6 +114,9 @@ package Exp_Ch3 is
--  delete the node if it is present just for front end purpose and we don't
--  want Gigi to see the node. This function can't delete the node itself
--  since it would confuse any remaining processing of the freeze node.
+   --
+   --  Note: for GNATprove we have a minimal variant of this routine in
+   --  Exp_SPARK.SPARK_Freeze_Type. They need to be kept in sync.
 
function Get_Simple_Init_Val
  (Typ  : Entity_Id;


diff --git a/gcc/ada/exp_spark.adb b/gcc/ada/exp_spark.adb
--- a/gcc/ada/exp_spark.adb
+++ b/gcc/ada/exp_spark.adb
@@ -29,15 +29,22 @@ with Einfo;  use Einfo;
 with Einfo.Entities; use Einfo.Entities;
 with Einfo.Utils;use Einfo.Utils;
 with Exp_Attr;
+with Exp_Ch3;
 with Exp_Ch4;
 with Exp_Ch5;use Exp_Ch5;
 with Exp_Dbug;   use Exp_Dbug;
 with Exp_Util;   use Exp_Util;
+with Ghost;  use Ghost;
 with Namet;  use Namet;
 with Nlists; use Nlists;
 with Nmake;  use Nmake;
+with Opt;use Opt;
+with Restrict;   use Restrict;
+with Rident; use Rident;
 with Rtsfind;use Rtsfind;
 with Sem;use Sem;
+with Sem_Aux;use Sem_Aux;
+with Sem_Ch7;use Sem_Ch7;
 with Sem_Ch8;use Sem_Ch8;
 with Sem_Prag;   use Sem_Prag;
 with Sem_Res;use Sem_Res;
@@ -62,8 +69,10 @@ package body Exp_SPARK is
procedure Expand_SPARK_N_Delta_Aggregate (N : Node_Id);
--  Perform delta-aggregate-specific expansion
 
-   procedure Expand_SPARK_N_Freeze_Type (E : Entity_Id);
-   --  Build the DIC procedure of a type when needed, if not already done
+   procedure Expand_SPARK_N_Freeze_Entity (N : Node_Id);
+   --  Do a minimal expansion of freeze entities required by GNATprove. It is
+   --  a subset of what is done for GNAT in Exp_Ch13.Expand_N_Freeze_Entity.
+   --  Those two routines should be kept in sync.
 
procedure Expand_SPARK_N_Loop_Statement (N : Node_Id);
--  Perform loop-statement-specific expansion
@@ -80,6 +89,20 @@ package body Exp_SPARK is
procedure Expand_SPARK_Delta_Or_Update (Typ : Entity_Id; Aggr : Node_Id);
--  Common expansion for attribute Update and delta aggregates
 
+   procedure SPARK_Freeze_Type (N : Node_Id);
+   --  Do a minimal type freezing required by GNATprove. It is a subset of what
+   --  is done for GNAT in Exp_Ch3.Freeze_Type. Those two routines should be
+   --  kept in sync.
+   --
+   --  Currently in freezing we build the spec of dispatching equality. This
+   --  spec is needed to properly resolve references to the equality operator.
+   --  The body is not needed, because proof knows how to directly synthesize a
+   --  logical meaning for it. Also, for tagged types with extension the
+   --  expanded body would compare the _parent component, which is
+   --  intentionally not generated in the GNATprove mode.
+   --
+   --  We build the DIC procedure body here as well.
+
--
-- Expand_SPARK --
--
@@ -140,8 +163,12 @@ package body Exp_SPARK is
 Expand_SPARK_N_Op_Ne (N);
 
  when N_Freeze_Entity =>
+--  Currently we only expand type freeze entities, so ignore other
+--  freeze entites, because it is expensive to create a suitable
+--  freezing environment.
+
 if Is_Type

[Ada] Inline all calls in Ada.Task_Identification

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Code cleanup; all routines in Ada.Task_Identification had pragma Inline
except Activation_Is_Complete (and Image, which should rather stay like
that).

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

gcc/ada/

* libgnarl/a-taside.ads (Activation_Is_Complete): Add pragma
Inline.diff --git a/gcc/ada/libgnarl/a-taside.ads b/gcc/ada/libgnarl/a-taside.ads
--- a/gcc/ada/libgnarl/a-taside.ads
+++ b/gcc/ada/libgnarl/a-taside.ads
@@ -94,6 +94,7 @@ is
  Volatile_Function,
  Pre=> T /= Null_Task_Id,
  Global => Tasking_State;
+   pragma Inline (Activation_Is_Complete);
 
 private
pragma SPARK_Mode (Off);




[Ada] Enhance freezing code for instantiations

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This makes it possible for the freezing code to let the back-end
establish a proper order of elaboration of package and subprogram
instantiations in more cases, in particular with circularities, by
placing freeze nodes for them later in the expanded code in these cases.

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

gcc/ada/

* sem_ch12.adb (Freeze_Package_Instance): Consistently consider
the freeze node of the parent and use large inequality for
Slocs.
(Freeze_Subprogram_Instance): Likewise.
(Insert_Freeze_Node_For_Instance): For an instance in a package
spec with no source body that immediately follows, consider the
body of the package for the placement of the freeze node and go
to the outer level if there is no such body.diff --git a/gcc/ada/sem_ch12.adb b/gcc/ada/sem_ch12.adb
--- a/gcc/ada/sem_ch12.adb
+++ b/gcc/ada/sem_ch12.adb
@@ -732,9 +732,10 @@ package body Sem_Ch12 is
--  associated freeze node. Insert the freeze node before the first source
--  body which follows immediately after N. If no such body is found, the
--  freeze node is inserted at the end of the declarative region which
-   --  contains N. This can also be invoked to insert the freeze node of a
-   --  package that encloses an instantiation, in which case N may denote an
-   --  arbitrary node.
+   --  contains N, unless the instantiation is done in a package spec that is
+   --  not at library level, in which case it is inserted at the outer level.
+   --  This can also be invoked to insert the freeze node of a package that
+   --  encloses an instantiation, in which case N may denote an arbitrary node.
 
procedure Install_Formal_Packages (Par : Entity_Id);
--  Install the visible part of any formal of the parent that is a formal
@@ -9208,8 +9209,7 @@ package body Sem_Ch12 is
--  of a package declaration, and the inner instance is in
--  the corresponding private part.
 
-   if Parent (List_Containing (Get_Unit_Instantiation_Node
- (Par_Id)))
+   if Parent (List_Containing (Freeze_Node (Par_Id)))
 = Parent (List_Containing (N))
  and then Sloc (Freeze_Node (Par_Id)) <= Sloc (N)
then
@@ -9266,7 +9266,8 @@ package body Sem_Ch12 is
  --  the body.
 
  elsif In_Same_List (Freeze_Node (Par_Id), Parent (N))
-   and then Sloc (Freeze_Node (Par_Id)) < Sloc (Parent (N))
+   and then
+ Sloc (Freeze_Node (Par_Id)) <= Sloc (Parent (N))
  then
 Insert_Freeze_Node_For_Instance
   (Parent (N), Freeze_Node (Enclosing));
@@ -9403,7 +9404,7 @@ package body Sem_Ch12 is
  --  after that of Parent_Inst. This relation is established by
  --  comparing the Slocs of Parent_Inst freeze node and Inst.
 
- elsif In_Same_List (Get_Unit_Instantiation_Node (Par_Id), N)
+ elsif In_Same_List (Freeze_Node (Par_Id), N)
and then Sloc (Freeze_Node (Par_Id)) <= Sloc (N)
  then
 Insert_Freeze_Node_For_Instance (N, F_Node);
@@ -9844,12 +9845,6 @@ package body Sem_Ch12 is
  (N  : Node_Id;
   F_Node : Node_Id)
is
-  Decl : Node_Id;
-  Decls: List_Id;
-  Inst : Entity_Id;
-  Par_Inst : Node_Id;
-  Par_N: Node_Id;
-
   function Enclosing_Body (N : Node_Id) return Node_Id;
   --  Find enclosing package or subprogram body, if any. Freeze node may
   --  be placed at end of current declarative list if previous instance
@@ -9908,138 +9903,183 @@ package body Sem_Ch12 is
  return Empty;
   end Previous_Instance;
 
+  --  Local variables
+
+  Decl : Node_Id;
+  Decls: List_Id;
+  Inst : Entity_Id;
+  Par_Inst : Node_Id;
+  Par_N: Node_Id;
+
--  Start of processing for Insert_Freeze_Node_For_Instance
 
begin
-  if not Is_List_Member (F_Node) then
- Decl  := N;
- Decls := List_Containing (N);
- Par_N := Parent (Decls);
- Inst  := Entity (F_Node);
+  --  Nothing to do if the freeze node has already been inserted
 
- --  When processing a subprogram instantiation, utilize the actual
- --  subprogram instantiation rather than its package wrapper as it
- --  carries all the context information.
+  if Is_List_Member (F_Node) then
+ return;
+  end if;
 
- if Is_Wrapper_Package (Inst) then
-Inst := Related_Instance (Inst);
- end if;
+  Inst := Entity (F_Node);
 
- Par_Inst := Parent (Inst);
+  --  When processing a subprogram instantiation, utilize the actual
+  --  subprogram instantiation rather than its package wrapper as it
+  --  carries all

[Ada] Remove extra space after assignment symbol

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Fix style issues in GNAT after spotting a similar problem in one of the
tests for GNATprove. These are easy to find with grep, but still require
a manual inspection, because we want to preserve layout in declaration
lists like:

  A :=   1;
  B :=  10;
  C := 100;

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

gcc/ada/

* exp_aggr.adb, exp_ch6.adb, par-ch4.adb, sem_ch13.adb: Remove
extra space after ":=" symbol.
* gen_il-gen.adb: Likewise; add missing headerbox.diff --git a/gcc/ada/exp_aggr.adb b/gcc/ada/exp_aggr.adb
--- a/gcc/ada/exp_aggr.adb
+++ b/gcc/ada/exp_aggr.adb
@@ -7546,11 +7546,11 @@ package body Exp_Aggr is
  Parameter_Associations => Params));
  end if;
 
- Loop_Stat :=  Make_Implicit_Loop_Statement
- (Node => N,
-  Identifier   => Empty,
-  Iteration_Scheme => L_Iteration_Scheme,
-  Statements   => Stats);
+ Loop_Stat := Make_Implicit_Loop_Statement
+(Node => N,
+ Identifier   => Empty,
+ Iteration_Scheme => L_Iteration_Scheme,
+ Statements   => Stats);
  Append (Loop_Stat, Aggr_Code);
 
   end Expand_Iterated_Component;
@@ -7825,7 +7825,7 @@ package body Exp_Aggr is
--  required size for the aggregwte : call the provided
--  constructor rather than the Empty aggregate.
 
-   Index :=  Make_Op_Add (Loc,
+   Index := Make_Op_Add (Loc,
  Left_Opnd => New_Copy_Tree (Type_Low_Bound (Index_Type)),
  Right_Opnd => Make_Integer_Literal (Loc, Siz - 1));
 


diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb
--- a/gcc/ada/exp_ch6.adb
+++ b/gcc/ada/exp_ch6.adb
@@ -10206,7 +10206,7 @@ package body Exp_Ch6 is
   --  If function is inherited, a conversion may be necessary.
 
   if Nkind (Par) = N_Assignment_Statement then
- Last_Actual :=  Name (Par);
+ Last_Actual := Name (Par);
 
  if not Comes_From_Source (Orig_Func)
and then Etype (Orig_Func) /= Etype (Func_Id)


diff --git a/gcc/ada/gen_il-gen.adb b/gcc/ada/gen_il-gen.adb
--- a/gcc/ada/gen_il-gen.adb
+++ b/gcc/ada/gen_il-gen.adb
@@ -2135,9 +2135,13 @@ package body Gen_IL.Gen is
 
procedure One_Comp (F : Field_Enum);
 
+   --
+   -- One_Comp --
+   --
+
procedure One_Comp (F : Field_Enum) is
   pragma Annotate (Codepeer, Modified, Field_Table);
-  Offset : constant Field_Offset :=  Field_Table (F).Offset;
+  Offset : constant Field_Offset := Field_Table (F).Offset;
begin
   if First_Time then
  First_Time := False;


diff --git a/gcc/ada/par-ch4.adb b/gcc/ada/par-ch4.adb
--- a/gcc/ada/par-ch4.adb
+++ b/gcc/ada/par-ch4.adb
@@ -3482,7 +3482,7 @@ package body Ch4 is
New_Node (N_Loop_Parameter_Specification, Prev_Token_Ptr);
  Set_Defining_Identifier (Loop_Spec, Id);
 
- Choice :=  First (Discrete_Choices (Assoc_Node));
+ Choice := First (Discrete_Choices (Assoc_Node));
  Assoc_Node :=
New_Node (N_Iterated_Element_Association, Prev_Token_Ptr);
  Set_Loop_Parameter_Specification (Assoc_Node, Loop_Spec);


diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb
--- a/gcc/ada/sem_ch13.adb
+++ b/gcc/ada/sem_ch13.adb
@@ -15550,7 +15550,7 @@ package body Sem_Ch13 is
 Add_Unnamed_Subp := Subp;
 
  elsif Op_Name = Name_New_Indexed then
-New_Indexed_Subp :=  Subp;
+New_Indexed_Subp := Subp;
 
  elsif Op_Name = Name_Assign_Indexed then
 Assign_Indexed_Subp := Subp;




[Ada] Cleanups related to expansion of dispatching primitives

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Assorted cleanups related to expansion of dispatching primitives on
derived types for GNATprove; semantics of the compiler is unaffected.

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

gcc/ada/

* doc/gnat_rm/standard_and_implementation_defined_restrictions.rst
(No_Dispatching_Calls): Fix whitespace in example code.
* gnat_rm.texi: Regenerate.
* exp_ch13.adb (Expand_N_Freeze_Entity): Replace low-level
membership test with a high-level wrapper.
* exp_ch3.adb (Expand_Freeze_Record_Type): Remove unnecessary
initialization of list of wrapper declarations and unnecessary
guard for list of their bodies (if no bodies are created then
Append_Freeze_Actions is a no-op).diff --git a/gcc/ada/doc/gnat_rm/standard_and_implementation_defined_restrictions.rst b/gcc/ada/doc/gnat_rm/standard_and_implementation_defined_restrictions.rst
--- a/gcc/ada/doc/gnat_rm/standard_and_implementation_defined_restrictions.rst
+++ b/gcc/ada/doc/gnat_rm/standard_and_implementation_defined_restrictions.rst
@@ -239,7 +239,7 @@ The following example indicates constructs that violate this restriction.
   with Pkg; use Pkg;
   procedure Example is
 procedure Test (O : T'Class) is
-  N : Natural  := O'Size;--  Error: Dispatching call
+  N : Natural := O'Size; --  Error: Dispatching call
   C : T'Class := O;  --  Error: implicit Dispatching Call
 begin
   if O in DT'Class then  --  OK   : Membership test


diff --git a/gcc/ada/exp_ch13.adb b/gcc/ada/exp_ch13.adb
--- a/gcc/ada/exp_ch13.adb
+++ b/gcc/ada/exp_ch13.adb
@@ -491,7 +491,7 @@ package body Exp_Ch13 is
   --  a constrained type extension with inherited discriminants.
 
   if Is_Type (E_Scope)
-and then Ekind (E_Scope) not in Concurrent_Kind
+and then not Is_Concurrent_Type (E_Scope)
   then
  E_Scope := Scope (E_Scope);
 


diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -5495,7 +5495,7 @@ package body Exp_Ch3 is
   Comp_Typ: Entity_Id;
   Predef_List : List_Id;
 
-  Wrapper_Decl_List : List_Id := No_List;
+  Wrapper_Decl_List : List_Id;
   Wrapper_Body_List : List_Id := No_List;
 
   Renamed_Eq : Node_Id := Empty;
@@ -5906,9 +5906,7 @@ package body Exp_Ch3 is
  --  Ada 2005 (AI-391): If any wrappers were created for nonoverridden
  --  inherited functions, then add their bodies to the freeze actions.
 
- if Present (Wrapper_Body_List) then
-Append_Freeze_Actions (Typ, Wrapper_Body_List);
- end if;
+ Append_Freeze_Actions (Typ, Wrapper_Body_List);
 
  --  Create extra formals for the primitive operations of the type.
  --  This must be done before analyzing the body of the initialization


diff --git a/gcc/ada/gnat_rm.texi b/gcc/ada/gnat_rm.texi
--- a/gcc/ada/gnat_rm.texi
+++ b/gcc/ada/gnat_rm.texi
@@ -12621,7 +12621,7 @@ end Pkg;
 with Pkg; use Pkg;
 procedure Example is
   procedure Test (O : T'Class) is
-N : Natural  := O'Size;--  Error: Dispatching call
+N : Natural := O'Size; --  Error: Dispatching call
 C : T'Class := O;  --  Error: implicit Dispatching Call
   begin
 if O in DT'Class then  --  OK   : Membership test




[Ada] Use bracket aggregates in Ada2022

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Make bracket syntax array and container aggregates available with
-gnat2022. Parenthesis syntax is not accepted anymore for container
aggregate but can still be used for array aggregates. The latter is
considered obsolete and a warning is emitted with -gnatwj.

The warning is also temporarily disabled in GNAT mode to allow for
easier transition.

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

gcc/ada/

* exp_imgv.adb (Append_Table_To): Add new parameter to
Make_Aggregate call.
* gen_il-fields.ads (Opt_Field_Enum):
Add Is_Parenthesis_Aggregate and Is_Enum_Array_Aggregate.
* gen_il-gen-gen_nodes.adb (Union): Add Is_Enum_Array_Aggregate
and Is_Parenthesis_Aggregate field to N_Aggregate.
* libgnarl/s-interr.adb (User_Handler, User_Entry, Blocked)
(Ignored, Last_Unblocker, Server_ID): Likewise.
* libgnarl/s-intman.ads (Keep_Unmasked, Reserve): Likewise.
* libgnarl/s-intman__posix.adb (Exception_Interrupts)
(Initialize): Likewise.
* libgnarl/s-mudido__affinity.adb (Create): Likewise.
* libgnarl/s-osinte__linux.ads (Unmasked, Reserved): Likewise.
* libgnarl/s-taprop__linux.adb (Create_Task, Set_Task_Affinity)
* libgnarl/s-tasdeb.adb (Trace_On): Likewise.
* libgnarl/s-tasdeb.ads (Known_Tasks): Likewise.
* libgnarl/s-tasinf__linux.ads (Any_CPU, No_CPU): Likewise.
* libgnarl/s-taskin.adb (Initialize_ATCB): Likewise.
* libgnarl/s-taskin.ads (Ada_Task_Control_Block): Likewise.
* libgnarl/s-tasren.adb (Default_Treatment)
(New_State): Likewise.
* libgnarl/s-tassta.adb (Trace_Unhandled_Exception_In_Task):
Likewise.
* libgnarl/s-tataat.adb (Index_Array): Likewise.
* libgnarl/s-tpobop.adb (New_State): Likewise.
* libgnat/a-calend.adb (Cumulative_Days_Before_Month)
(Leap_Second_Times): Likewise.
* libgnat/a-calend.ads (Days_In_Month): Likewise.
* libgnat/a-cfinve.adb (Insert): Likewise.
* libgnat/a-chahan.adb (Char_Map): Likewise.
* libgnat/a-chtgbo.adb (Clear): Likewise.
* libgnat/a-cobove.adb ("&", Insert, To_Vector): Likewise.
* libgnat/a-cofove.adb (Insert, To_Vector): Likewise.
* libgnat/a-cohata.ads (Hash_Table_Type): Likewise.
* libgnat/a-coinve.adb (Merge, Insert, Insert_Space): Likewise.
* libgnat/a-convec.adb (Insert, To_Vector): Likewise.
* libgnat/a-coprnu.ads (Primes): Likewise.
* libgnat/a-direct.adb (Empty_String): Use regular "" instead
of aggregate.
(Start_Search_Internal, Name_Case_Equivalence, Search)
(Start_Search, Start_Search_Internal): Use bracket for
aggregate.
* libgnat/a-direct.ads (Start_Search,Search): Likewise.
* libgnat/a-direio.adb (Zeroes): Likewise.
* libgnat/a-nbnbre.adb (Leading_Padding, Trailing_Padding)
(Numerator_Image): Likewise.
* libgnat/a-ngrear.adb (Jacobi): Likewise.
* libgnat/a-stbubo.adb (Get_UTF_8): Likewise.
* libgnat/a-stbufo.adb (Put): Likewise.
* libgnat/a-stbuun.adb (Get_UTF_8): Likewise.
* libgnat/a-stbuut.adb (Put_7bit, Put_Character)
(Put_Wide_Character, Put_Wide_Wide_Character): Likewise.
* libgnat/a-stmaco.ads (Control_Set,Graphic_Set,Letter_Set)
(Lower_Set, Upper_Set, Basic_Set, Decimal_Digit_Set)
(Hexadecimal_Digit_Set, Alphanumeric_Set, Special_Set)
(ISO_646_Set): Likewise.
* libgnat/a-strbou.ads (Insert, Tail, "*", Replicate)
(Null_Bounded_String): Likewise.
* libgnat/a-strfix.ads (Head, Tail): Likewise.
* libgnat/a-strmap.adb (To_Domain, Lemma_Is_Sorted): Likewise.
* libgnat/a-strmap.ads (Null_Set): Likewise.
* libgnat/a-strsup.adb (Super_Head, Super_Replicate)
(Super_Tail): Likewise.
* libgnat/a-strsup.ads (Super_Head, Super_Tail, Times)
(Super_Replicate): Likewise.
* libgnat/a-sttebu.adb (Put_UTF8, Wide_Put_UTF_16): Likewise.
* libgnat/a-stuten.ads (BOM_16): Likewise.
* libgnat/a-stwibo.ads (Null_Bounded_Wide_String): Likewise.
* libgnat/a-stwima.ads (Null_Range): Likewise.
* libgnat/a-stwisu.adb (Super_Head, Super_Replicate)
(Super_Tail): Likewise.
* libgnat/a-stzbou.ads
(Null_Bounded_Wide_Wide_String): Likewise.
* libgnat/a-stzmap.ads (Null_Range): Likewise.
* libgnat/a-stzsup.adb (Super_Head, Super_Replicate)
(Super_Tail, Super_Trim): Likewise.
* libgnat/a-swmwco.ads (Control_Ranges, Graphic_Ranges)
(Letter_Ranges, Lower_Ranges, Upeer_Ranges, Basic_Ranges)
(Decimal_Digit_Ranges, Hexadecimal_Digit_Ranges)
(Alphanumeric_Ranges, Special_Graphic_Ranges, ISO_646_Ranges)
(Character_Ranges, Lower_Case_Mapping, Upper_Case_Mapping)
(Basic_Mapping): Likewise.
* libgnat/a-szmzco.ads (Control_Ranges, Graphic_Ranges

[PING] [PATCH] rs6000: testsuite: Add rop_ok effective-target function

2021-12-02 Thread Peter Bergner via Gcc-patches
I'd like to ping the following patch.

The test case in the previously OK'd fix for PR101324 depends on this change,
so I've had to hold off committing it until this is in.  Thanks.

  https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584208.html

Peter



Message-ID: 


This patch adds a new effective-target function that tests whether
it is safe to emit the ROP-protect instructions and updates the
ROP test cases to use it.

Segher, as we discussed offline, this uses the double [] which you said
isn't needed in general regex's, but for some reason is needed in the gcc
testsuite regex.

Tested on powerpc64le*-linux with no regressions.  Ok for mainline?

Peter


gcc/testsuite/
* lib/target-supports.exp (check_effective_target_rop_ok): New function.
* gcc.target/powerpc/rop-1.c: Use it.
* gcc.target/powerpc/rop-2.c: Likewise.
* gcc.target/powerpc/rop-3.c: Likewise.
* gcc.target/powerpc/rop-4.c: Likewise.
* gcc.target/powerpc/rop-5.c: Likewise.


[Ada] Cleanup insertion of single freezing actions

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Replace calls to Ensure_Freeze_Node & Append_Freeze_Actions (plural)
with a simple call to Append_Freeze_Action (singular), which calls
Ensure_Freeze_Node itself.

Cleanup related to expansion of dispatching primitives for GNATprove;
semantics is unaffected.

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

gcc/ada/

* exp_util.adb (Append_Freeze_Action): Tune whitespace to make
the code look similar to Append_Freeze_Actions, which takes a
List_Id.
* sem_ch6.adb (Analyze_Return_Type): Cleanup with
Append_Freeze_Action.
* exp_ch3.adb (Build_Access_Subprogram_Wrapper_Body): Likewise.
* sem_ch3.adb (Build_Access_Subprogram_Wrapper): Likewise.
* contracts.adb (Add_Indirect_Call_Wrapper): Remove extra call
to Ensure_Freeze_Node.
(Add_Call_Helper): Likewise.
* freeze.adb (Check_Inherited_Conditions): Likewise.
(Attribute_Renaming): Likewise.
* sem_ch8.adb: Likewise.diff --git a/gcc/ada/contracts.adb b/gcc/ada/contracts.adb
--- a/gcc/ada/contracts.adb
+++ b/gcc/ada/contracts.adb
@@ -3761,7 +3761,6 @@ package body Contracts is
 
  ICW_Decl := Build_ICW_Decl;
 
- Ensure_Freeze_Node (Tagged_Type);
  Append_Freeze_Action (Tagged_Type, ICW_Decl);
  Analyze (ICW_Decl);
 
@@ -4034,7 +4033,6 @@ package body Contracts is
 
  --  Add the helper to the freezing actions of the tagged type
 
- Ensure_Freeze_Node   (Tagged_Type);
  Append_Freeze_Action (Tagged_Type, Helper_Decl);
  Analyze (Helper_Decl);
 


diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -580,8 +580,7 @@ package body Exp_Ch3 is
 
   --  Place body in list of freeze actions for the type.
 
-  Ensure_Freeze_Node (Type_Id);
-  Append_Freeze_Actions (Type_Id, New_List (Body_Node));
+  Append_Freeze_Action (Type_Id, Body_Node);
end Build_Access_Subprogram_Wrapper_Body;
 
---


diff --git a/gcc/ada/exp_util.adb b/gcc/ada/exp_util.adb
--- a/gcc/ada/exp_util.adb
+++ b/gcc/ada/exp_util.adb
@@ -458,7 +458,6 @@ package body Exp_Util is
   else
  Append (N, Actions (Fnode));
   end if;
-
end Append_Freeze_Action;
 
---


diff --git a/gcc/ada/freeze.adb b/gcc/ada/freeze.adb
--- a/gcc/ada/freeze.adb
+++ b/gcc/ada/freeze.adb
@@ -2044,9 +2044,8 @@ package body Freeze is
--  node of the record type declaration to ensure that it will
--  override the internal primitive built by Derive_Subprogram.
 
-   Ensure_Freeze_Node (R);
-
if Late_Overriding then
+  Ensure_Freeze_Node (R);
   Insert_Before_And_Analyze (Freeze_Node (R), DTW_Decl);
else
   Append_Freeze_Action (R, DTW_Decl);


diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb
--- a/gcc/ada/sem_ch3.adb
+++ b/gcc/ada/sem_ch3.adb
@@ -6815,8 +6815,7 @@ package body Sem_Ch3 is
   --  in a dispatch table.
 
   if not GNATprove_Mode then
- Ensure_Freeze_Node (Id);
- Append_Freeze_Actions (Id, New_List (New_Decl));
+ Append_Freeze_Action (Id, New_Decl);
 
   --  Under GNATprove mode there is no such problem but we do not declare
   --  it in the freezing actions since they are not analyzed under this


diff --git a/gcc/ada/sem_ch6.adb b/gcc/ada/sem_ch6.adb
--- a/gcc/ada/sem_ch6.adb
+++ b/gcc/ada/sem_ch6.adb
@@ -2715,13 +2715,11 @@ package body Sem_Ch6 is
   end if;
 
else
-  Ensure_Freeze_Node (Typ);
-
   declare
  IR : constant Node_Id := Make_Itype_Reference (Sloc (N));
   begin
  Set_Itype (IR, Etype (Designator));
- Append_Freeze_Actions (Typ, New_List (IR));
+ Append_Freeze_Action (Typ, IR);
   end;
end if;
 


diff --git a/gcc/ada/sem_ch8.adb b/gcc/ada/sem_ch8.adb
--- a/gcc/ada/sem_ch8.adb
+++ b/gcc/ada/sem_ch8.adb
@@ -4689,7 +4689,6 @@ package body Sem_Ch8 is
 if Is_Tagged_Type (Etype (P))
   and then In_Open_Scopes (Scope (Etype (P)))
 then
-   Ensure_Freeze_Node (Etype (P));
Append_Freeze_Action (Etype (P), Body_Node);
 else
Rewrite (N, Body_Node);




[Ada] Cleanup detection of suspension objects

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Current implementation of Is_Suspension_Object is a leftover from an old
code of Is_Descendant_Of_Suspension_Object, which used RTE_Available and
indeed couldn't be called from GNATprove.

Now Is_Descendant_Of_Suspension_Object can work with Is_RTE, which can
be safely called from GNATprove.

Cleanup only; behaviour of GNAT and GNATprove is not affected.

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

gcc/ada/

* rtsfind.ads (RE_Id, RE_Unit_Table): Add RE_Suspension_Object.
* sem_util.adb (Is_Descendant_Of_Suspension_Object): Use Is_RTE.
(Is_Suspension_Object): Remove body.
* sem_util.ads (Is_Suspension_Object): Remove spec.
* snames.ads-tmpl (Name_Suspension_Object): Remove, now
unreferenced.diff --git a/gcc/ada/rtsfind.ads b/gcc/ada/rtsfind.ads
--- a/gcc/ada/rtsfind.ads
+++ b/gcc/ada/rtsfind.ads
@@ -626,6 +626,7 @@ package Rtsfind is
  RE_Wait_For_Release,-- Ada.Synchronous_Barriers
 
  RE_Suspend_Until_True,  -- Ada.Synchronous_Task_Control
+ RE_Suspension_Object,   -- Ada.Synchronous_Task_Control
 
  RE_Access_Level,-- Ada.Tags
  RE_Alignment,   -- Ada.Tags
@@ -2311,6 +2312,7 @@ package Rtsfind is
  RE_Wait_For_Release => Ada_Synchronous_Barriers,
 
  RE_Suspend_Until_True   => Ada_Synchronous_Task_Control,
+ RE_Suspension_Object=> Ada_Synchronous_Task_Control,
 
  RE_Access_Level => Ada_Tags,
  RE_Alignment=> Ada_Tags,


diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -17236,7 +17236,7 @@ package body Sem_Util is
 
  --  The current type is a match
 
- if Is_Suspension_Object (Cur_Typ) then
+ if Is_RTE (Cur_Typ, RE_Suspension_Object) then
 return True;
 
  --  Stop the traversal once the root of the derivation chain has been
@@ -21123,28 +21123,6 @@ package body Sem_Util is
   return True;
end Is_Suitable_Primitive;
 
-   --
-   -- Is_Suspension_Object --
-   --
-
-   function Is_Suspension_Object (Id : Entity_Id) return Boolean is
-   begin
-  --  This approach does an exact name match rather than to rely on
-  --  RTSfind. Routine Is_Effectively_Volatile is used by clients of the
-  --  front end at point where all auxiliary tables are locked and any
-  --  modifications to them are treated as violations. Do not tamper with
-  --  the tables, instead examine the Chars fields of all the scopes of Id.
-
-  return
-Chars (Id) = Name_Suspension_Object
-  and then Present (Scope (Id))
-  and then Chars (Scope (Id)) = Name_Synchronous_Task_Control
-  and then Present (Scope (Scope (Id)))
-  and then Chars (Scope (Scope (Id))) = Name_Ada
-  and then Present (Scope (Scope (Scope (Id
-  and then Scope (Scope (Scope (Id))) = Standard_Standard;
-   end Is_Suspension_Object;
-

-- Is_Synchronized_Object --



diff --git a/gcc/ada/sem_util.ads b/gcc/ada/sem_util.ads
--- a/gcc/ada/sem_util.ads
+++ b/gcc/ada/sem_util.ads
@@ -2440,10 +2440,6 @@ package Sem_Util is
--  Determine whether arbitrary subprogram Subp_Id may act as a primitive of
--  an arbitrary tagged type.
 
-   function Is_Suspension_Object (Id : Entity_Id) return Boolean;
-   --  Determine whether arbitrary entity Id denotes Suspension_Object defined
-   --  in Ada.Synchronous_Task_Control.
-
function Is_Synchronized_Object (Id : Entity_Id) return Boolean;
--  Determine whether entity Id denotes an object and if it does, whether
--  this object is synchronized as specified in SPARK RM 9.1. To qualify as


diff --git a/gcc/ada/snames.ads-tmpl b/gcc/ada/snames.ads-tmpl
--- a/gcc/ada/snames.ads-tmpl
+++ b/gcc/ada/snames.ads-tmpl
@@ -1401,7 +1401,6 @@ package Snames is
--  e.g. Name_UP_RESULT corresponds to the name "RESULT".
 
Name_UP_RESULT: constant Name_Id := N + $;
-   Name_Suspension_Object: constant Name_Id := N + $;
Name_Synchronous_Task_Control : constant Name_Id := N + $;
 
--  Names used to implement iterators over predefined containers




[Ada] Add pragma Annotate for CodePeer analysis

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
CodePeer issues a spurious message on the analysis of the loop variant
in s-widthi.adb. Similarly to the same for s-widthu.adb, add a pragma
Annotate to justify this message.

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

gcc/ada/

* libgnat/s-widthi.adb: Add pragma Annotate.diff --git a/gcc/ada/libgnat/s-widthi.adb b/gcc/ada/libgnat/s-widthi.adb
--- a/gcc/ada/libgnat/s-widthi.adb
+++ b/gcc/ada/libgnat/s-widthi.adb
@@ -163,6 +163,9 @@ begin
  pragma Loop_Invariant (Pow = Big_10 ** (W - 2));
  pragma Loop_Invariant (Big (T) = Big (T_Init) / Pow);
  pragma Loop_Variant (Decreases => T);
+ pragma Annotate
+   (CodePeer, False_Positive,
+"validity check", "confusion on generated code");
   end loop;
 
   declare




[Ada] Proof of support units for 'Width on signed integers

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This replicates the proofs performed for 'Width on modular integers to
the units that support 'Width on signed integers. Also add a minimal
postcondition to the System.Width_U.

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

gcc/ada/

* libgnat/s-widint.ads: Mark in SPARK.
* libgnat/s-widlli.ads: Likewise.
* libgnat/s-widllli.ads: Likewise.
* libgnat/s-widlllu.ads: Likewise.
* libgnat/s-widllu.ads: Disable ghost/contract.
* libgnat/s-widthi.adb: Replicate and adapt the proof from
s-widthu.adb.
* libgnat/s-widthi.ads: Add minimal postcondition.
* libgnat/s-widthu.adb: Fix comments in the modular case.
* libgnat/s-widthu.ads: Add minimal postcondition.
* libgnat/s-widuns.ads: Disable ghost/contract.diff --git a/gcc/ada/libgnat/s-widint.ads b/gcc/ada/libgnat/s-widint.ads
--- a/gcc/ada/libgnat/s-widint.ads
+++ b/gcc/ada/libgnat/s-widint.ads
@@ -31,9 +31,22 @@
 
 --  Width attribute for signed integers up to Integer
 
+--  Preconditions in this unit are meant for analysis only, not for run-time
+--  checking, so that the expected exceptions are raised. This is enforced by
+--  setting the corresponding assertion policy to Ignore. Postconditions and
+--  contract cases should not be executed at runtime as well, in order not to
+--  slow down the execution of these functions.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Contract_Cases => Ignore,
+ Ghost  => Ignore);
+
 with System.Width_I;
 
-package System.Wid_Int is
+package System.Wid_Int
+  with SPARK_Mode
+is
 
function Width_Integer is new Width_I (Integer);
pragma Pure_Function (Width_Integer);


diff --git a/gcc/ada/libgnat/s-widlli.ads b/gcc/ada/libgnat/s-widlli.ads
--- a/gcc/ada/libgnat/s-widlli.ads
+++ b/gcc/ada/libgnat/s-widlli.ads
@@ -31,9 +31,22 @@
 
 --  Width attribute for signed integers larger than Integer
 
+--  Preconditions in this unit are meant for analysis only, not for run-time
+--  checking, so that the expected exceptions are raised. This is enforced by
+--  setting the corresponding assertion policy to Ignore. Postconditions and
+--  contract cases should not be executed at runtime as well, in order not to
+--  slow down the execution of these functions.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Contract_Cases => Ignore,
+ Ghost  => Ignore);
+
 with System.Width_I;
 
-package System.Wid_LLI is
+package System.Wid_LLI
+  with SPARK_Mode
+is
 
function Width_Long_Long_Integer is new Width_I (Long_Long_Integer);
pragma Pure_Function (Width_Long_Long_Integer);


diff --git a/gcc/ada/libgnat/s-widllli.ads b/gcc/ada/libgnat/s-widllli.ads
--- a/gcc/ada/libgnat/s-widllli.ads
+++ b/gcc/ada/libgnat/s-widllli.ads
@@ -31,9 +31,22 @@
 
 --  Width attribute for signed integers larger than Long_Long_Integer
 
+--  Preconditions in this unit are meant for analysis only, not for run-time
+--  checking, so that the expected exceptions are raised. This is enforced by
+--  setting the corresponding assertion policy to Ignore. Postconditions and
+--  contract cases should not be executed at runtime as well, in order not to
+--  slow down the execution of these functions.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Contract_Cases => Ignore,
+ Ghost  => Ignore);
+
 with System.Width_I;
 
-package System.Wid_LLLI is
+package System.Wid_LLLI
+  with SPARK_Mode
+is
 
function Width_Long_Long_Long_Integer is
  new Width_I (Long_Long_Long_Integer);


diff --git a/gcc/ada/libgnat/s-widlllu.ads b/gcc/ada/libgnat/s-widlllu.ads
--- a/gcc/ada/libgnat/s-widlllu.ads
+++ b/gcc/ada/libgnat/s-widlllu.ads
@@ -31,6 +31,17 @@
 
 --  Width attribute for modular integers larger than Long_Long_Integer
 
+--  Preconditions in this unit are meant for analysis only, not for run-time
+--  checking, so that the expected exceptions are raised. This is enforced by
+--  setting the corresponding assertion policy to Ignore. Postconditions and
+--  contract cases should not be executed at runtime as well, in order not to
+--  slow down the execution of these functions.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Contract_Cases => Ignore,
+ Ghost  => Ignore);
+
 with System.Width_U;
 with System.Unsigned_Types;
 


diff --git a/gcc/ada/libgnat/s-widllu.ads b/gcc/ada/libgnat/s-widllu.ads
--- a/gcc/ada/libgnat/s-widllu.ads
+++ b/gcc/ada/libgnat/s-widllu.ads
@@ -31,6 +31,17 @@
 
 --  Width attribute for modular integers larger than Integer
 
+--  Preconditions in this unit are meant for analysis only, not f

[Ada] Amend proof of System.Arith_Double to remove justifications

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
The proof of System.Arith_Double contained 11 occurrences of pragma
Annotate to justify unproved checks, 10 of which for calls to
Raise_Error denoting a case where the input leads to a division by zero
or an overflow, and one for a loop invariant regarding an assumption on
Single_Size. That should not have been necessary, as the preconditions
of subprograms are meant to prevent division by zero and overflows, and
the asumption on Single_Size can be stated and checks when proving each
instance of the generic.

Amend the proof to remove all check justifications.

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

gcc/ada/

* libgnat/s-aridou.adb (Log_Single_Size, Big_0): New ghost
constants.
(Lemma_Mult_Non_Negative, Lemma_Mult_Non_Positive,
Lemma_Not_In_Range_Big2xx64): New lemmas on big integers.
(Double_Divide): Remove justifications. Amend for that local
lemma Prove_Overflow_Case.
(Scaled_Divide): Remove justifications. Insert for that local
lemmas Prove_Negative_Dividend, Prove_Positive_Dividend and
Prove_Q_Too_Big, and amend local lemma Prove_Overflow.  To prove
the loop invariant on (Shift mod 2 = 0), introduce local ghost
variable Iter to count loop iterations, and relate its value to
the value of Shift through Log_Single_Size, with the help of
local lemma Prove_Power. Deal with proof regression by adding
new local lemma Prove_First_Iteration and local ghost variable
D123.
* libgnat/s-arit64.ads (Multiply_With_Ovflo_Check64): Remove
unnecessary Pure_Function on function as package is Pure.diff --git a/gcc/ada/libgnat/s-aridou.adb b/gcc/ada/libgnat/s-aridou.adb
--- a/gcc/ada/libgnat/s-aridou.adb
+++ b/gcc/ada/libgnat/s-aridou.adb
@@ -55,6 +55,15 @@ is
Double_Size : constant Natural := Double_Int'Size;
Single_Size : constant Natural := Double_Int'Size / 2;
 
+   --  Log of Single_Size in base 2, so that Single_Size = 2 ** Log_Single_Size
+   Log_Single_Size : constant Natural :=
+ (case Single_Size is
+when 32  => 5,
+when 64  => 6,
+when 128 => 7,
+when others => raise Program_Error)
+   with Ghost;
+
--  Power-of-two constants. Use the names Big_2xx32, Big_2xx63 and Big_2xx64
--  even if Single_Size might not be 32 and Double_Size might not be 64, as
--  this facilitates code and proof understanding, compared to more generic
@@ -66,6 +75,9 @@ is
pragma Warnings
  (Off, "non-static constant in preelaborated unit",
   Reason => "Ghost code is not compiled");
+   Big_0 : constant Big_Integer :=
+ Big (Double_Uns'(0))
+   with Ghost;
Big_2xx32 : constant Big_Integer :=
  Big (Double_Int'(2 ** Single_Size))
with Ghost;
@@ -198,6 +210,20 @@ is
  Ghost,
  Post => abs (X * Y) = abs X * abs Y;
 
+   procedure Lemma_Mult_Non_Negative (X, Y : Big_Integer)
+   with
+ Ghost,
+ Pre  => (X >= Big_0 and then Y >= Big_0)
+   or else (X <= Big_0 and then Y <= Big_0),
+ Post => X * Y >= Big_0;
+
+   procedure Lemma_Mult_Non_Positive (X, Y : Big_Integer)
+   with
+ Ghost,
+ Pre  => (X <= Big_0 and then Y >= Big_0)
+   or else (X >= Big_0 and then Y <= Big_0),
+ Post => X * Y <= Big_0;
+
procedure Lemma_Abs_Rem_Commutation (X, Y : Big_Integer)
with
  Ghost,
@@ -407,6 +433,11 @@ is
  Pre  => Y /= 0,
  Post => X rem Y = X rem (-Y);
 
+   procedure Lemma_Not_In_Range_Big2xx64
+   with
+ Post => not In_Double_Int_Range (Big_2xx64)
+   and then not In_Double_Int_Range (-Big_2xx64);
+
procedure Lemma_Powers_Of_2 (M, N : Natural)
with
  Ghost,
@@ -551,7 +582,10 @@ is
procedure Lemma_Mult_Commutation (X, Y : Double_Int) is null;
procedure Lemma_Mult_Commutation (X, Y, Z : Double_Uns) is null;
procedure Lemma_Mult_Distribution (X, Y, Z : Big_Integer) is null;
+   procedure Lemma_Mult_Non_Negative (X, Y : Big_Integer) is null;
+   procedure Lemma_Mult_Non_Positive (X, Y : Big_Integer) is null;
procedure Lemma_Neg_Rem (X, Y : Big_Integer) is null;
+   procedure Lemma_Not_In_Range_Big2xx64 is null;
procedure Lemma_Rem_Commutation (X, Y : Double_Uns) is null;
procedure Lemma_Rem_Is_Ident (X, Y : Big_Integer) is null;
procedure Lemma_Rem_Sign (X, Y : Big_Integer) is null;
@@ -722,21 +756,15 @@ is
 
   --  Local lemmas
 
-  function Is_Division_By_Zero_Case return Boolean is
-(Y = 0 or else Z = 0)
-  with Ghost;
-
-  function Is_Overflow_Case return Boolean is
-(not In_Double_Int_Range (Big (X) / (Big (Y) * Big (Z
-  with
-Ghost,
-Pre => Y /= 0 and Z /= 0;
-
   procedure Prove_Overflow_Case
   with
 Ghost,
 Pre  => X = Double_Int'First and then Big (Y) * Big (Z) = -1,
-Post => Is_Overflow_Case;
+Post => not In_Double_Int_Range (Big (X) / (Big (Y) * Big (Z)))
+  and then not In_Double_Int_Range
+(Roun

[Ada] Reset internal flags for -gnatD and -gnatG

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This resets the internal flags associated with the -gnatD and -gnatG
switches once the generated code is printed, so that it does not end
up being printed twice in case something goes wrong after this point.

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

gcc/ada/

* sprint.adb (Source_Dump): Set both Print_Generated_Code and
Debug_Generated_Code to False at the end.diff --git a/gcc/ada/sprint.adb b/gcc/ada/sprint.adb
--- a/gcc/ada/sprint.adb
+++ b/gcc/ada/sprint.adb
@@ -673,6 +673,11 @@ package body Sprint is
 end if;
  end loop;
   end if;
+
+  --  See above for the rationale, but we cannot do it earlier for them
+
+  Print_Generated_Code := False;
+  Debug_Generated_Code := False;
end Source_Dump;
 
-




[Ada] Fix obsolete array aggregate warning being triggered by expanded code

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
Filter out nodes not coming from source before emitting the warning.

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

gcc/ada/

* sem_aggr.adb (Resolve_Array_Aggregate): Filter out nodes not
coming from source before emitting the warning.diff --git a/gcc/ada/sem_aggr.adb b/gcc/ada/sem_aggr.adb
--- a/gcc/ada/sem_aggr.adb
+++ b/gcc/ada/sem_aggr.adb
@@ -1807,6 +1807,7 @@ package body Sem_Aggr is
 and then not Is_Enum_Array_Aggregate (N)
 and then Is_Parenthesis_Aggregate (N)
 and then Nkind (Parent (N)) /= N_Qualified_Expression
+and then Comes_From_Source (N)
   then
  Error_Msg_N
("?j?array aggregate using () is an" &




[Ada] Invalid memory access on finalization of class-wide type

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This patch is the first part of a correction for issues in the compiler
whereby finalization of a heap-allocated class-wide type may cause an
invalid memory read at runtime when the type in question contains a
component whose type has alignment specified.

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

gcc/ada/

* gcc-interface/decl.c (gnat_to_gnu_entity): Skip normal
processing for Itypes that are E_Class_Wide_Subtype with
Equivalent_Type set.diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -335,11 +335,14 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 
   /* Since a use of an itype is a definition, process it as such if it is in
  the main unit, except for E_Access_Subtype because it's actually a use
- of its base type, see below.  */
+ of its base type, and for E_Class_Wide_Subtype with an Equivalent_Type
+ because it's actually a use of the latter type.  */
   if (!definition
   && is_type
   && Is_Itype (gnat_entity)
   && Ekind (gnat_entity) != E_Access_Subtype
+  && !(Ekind (gnat_entity) == E_Class_Wide_Subtype
+	   && Present (Equivalent_Type (gnat_entity)))
   && !present_gnu_tree (gnat_entity)
   && In_Extended_Main_Code_Unit (gnat_entity))
 {




[Ada] Proof of System.Arith_32 for double arithmetic on 32bits

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This replicates in a simpler setting the proofs performed for generic
unit System.Arith_Double.

Note that it makes a difference to declare functions Big as expression
functions here instead of renamings, as some checks are not proved with
renamings, so expression functions are used instead.

GNATprove is run with switches --level=3 --prover=all.

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

gcc/ada/

* libgnat/s-arit32.adb: Add ghost instances and lemmas.
(Scaled_Divide32): Add ghost code to prove. Minor code
modification to return early in error when divisor is zero.
* libgnat/s-arit32.ads: Add ghost instances and utilities.
(Scaled_Divide32): Add contract.diff --git a/gcc/ada/libgnat/s-arit32.adb b/gcc/ada/libgnat/s-arit32.adb
--- a/gcc/ada/libgnat/s-arit32.adb
+++ b/gcc/ada/libgnat/s-arit32.adb
@@ -29,9 +29,24 @@
 --  --
 --
 
+--  Preconditions, postconditions, ghost code, loop invariants and assertions
+--  in this unit are meant for analysis only, not for run-time checking, as it
+--  would be too costly otherwise. This is enforced by setting the assertion
+--  policy to Ignore.
+
+pragma Assertion_Policy (Pre=> Ignore,
+ Post   => Ignore,
+ Ghost  => Ignore,
+ Loop_Invariant => Ignore,
+ Assert => Ignore);
+
+with Ada.Numerics.Big_Numbers.Big_Integers_Ghost;
+use Ada.Numerics.Big_Numbers.Big_Integers_Ghost;
 with Ada.Unchecked_Conversion;
 
-package body System.Arith_32 is
+package body System.Arith_32
+  with SPARK_Mode
+is
 
pragma Suppress (Overflow_Check);
pragma Suppress (Range_Check);
@@ -43,27 +58,65 @@ package body System.Arith_32 is
 
function To_Int is new Ada.Unchecked_Conversion (Uns32, Int32);
 
+   package Unsigned_Conversion is new Unsigned_Conversions (Int => Uns32);
+
+   function Big (Arg : Uns32) return Big_Integer is
+ (Unsigned_Conversion.To_Big_Integer (Arg))
+   with Ghost;
+
+   package Unsigned_Conversion_64 is new Unsigned_Conversions (Int => Uns64);
+
+   function Big (Arg : Uns64) return Big_Integer is
+ (Unsigned_Conversion_64.To_Big_Integer (Arg))
+   with Ghost;
+
+   pragma Warnings
+ (Off, "non-preelaborable call not allowed in preelaborated unit",
+  Reason => "Ghost code is not compiled");
+   Big_0 : constant Big_Integer :=
+ Big (Uns32'(0))
+   with Ghost;
+   Big_2xx32 : constant Big_Integer :=
+ Big (Uns32'(2 ** 32 - 1)) + 1
+   with Ghost;
+   Big_2xx64 : constant Big_Integer :=
+ Big (Uns64'(2 ** 64 - 1)) + 1
+   with Ghost;
+   pragma Warnings
+ (On, "non-preelaborable call not allowed in preelaborated unit");
+
---
-- Local Subprograms --
---
 
function "abs" (X : Int32) return Uns32 is
  (if X = Int32'First
-  then 2**31
+  then Uns32'(2**31)
   else Uns32 (Int32'(abs X)));
--  Convert absolute value of X to unsigned. Note that we can't just use
--  the expression of the Else since it overflows for X = Int32'First.
 
+   function Lo (A : Uns64) return Uns32 is (Uns32 (A and (2 ** 32 - 1)));
+   --  Low order half of 64-bit value
+
function Hi (A : Uns64) return Uns32 is (Uns32 (Shift_Right (A, 32)));
--  High order half of 64-bit value
 
-   function To_Neg_Int (A : Uns32) return Int32;
+   function To_Neg_Int (A : Uns32) return Int32
+   with
+ Annotate => (GNATprove, Terminating),
+ Pre  => In_Int32_Range (-Big (A)),
+ Post => Big (To_Neg_Int'Result) = -Big (A);
--  Convert to negative integer equivalent. If the input is in the range
--  0 .. 2**31, then the corresponding nonpositive signed integer (obtained
--  by negating the given value) is returned, otherwise constraint error is
--  raised.
 
-   function To_Pos_Int (A : Uns32) return Int32;
+   function To_Pos_Int (A : Uns32) return Int32
+   with
+ Annotate => (GNATprove, Terminating),
+ Pre  => In_Int32_Range (Big (A)),
+ Post => Big (To_Pos_Int'Result) = Big (A);
--  Convert to positive integer equivalent. If the input is in the range
--  0 .. 2**31 - 1, then the corresponding nonnegative signed integer is
--  returned, otherwise constraint error is raised.
@@ -72,6 +125,168 @@ package body System.Arith_32 is
pragma No_Return (Raise_Error);
--  Raise constraint error with appropriate message
 
+   --
+   -- Local Lemmas --
+   --
+
+   procedure Lemma_Abs_Commutation (X : Int32)
+   with
+ Ghost,
+ Post => abs (Big (X)) = Big (Uns32'(abs X));
+
+   procedure Lemma_Abs_Div_Commutation (X, Y : Big_Integer)
+   with
+ Ghost,
+ Pre  => Y /= 0,
+ Post => abs (X / Y) = abs X / abs Y;
+
+   procedure Lemma_Abs_Mult_Commutation 

[Ada] Fix packing for array component with discriminated part

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This restores the packing for a record type that contains a component of
an array type whose component type is a record type with a subcomponent
that is of a discriminated record type with variable size.

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

gcc/ada/

* gcc-interface/gigi.h (aggregate_type_contains_array_p): Delete.
(type_has_variable_size): Declare.
* gcc-interface/decl.c (adjust_packed): Return 0 only if the field
type is an array with variable size.
* gcc-interface/utils.c (aggregate_type_contains_array_p): Make
static and remove SELF_REFERENTIAL parameter.
(type_has_variable_size): Make public.
(create_field_decl): Adjust call to aggregate_type_contains_array_p.diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -7221,13 +7221,12 @@ choices_to_gnu (tree gnu_operand, Node_Id gnat_choices)
 static int
 adjust_packed (tree field_type, tree record_type, int packed)
 {
-  /* If the field contains an array with self-referential size, we'd better
- not pack it because this would misalign it and, therefore, cause large
- temporaries to be created in case we need to take the address of the
- field.  See addressable_p and the notes on the addressability issues
- for further details.  */
-  if (AGGREGATE_TYPE_P (field_type)
-  && aggregate_type_contains_array_p (field_type, true))
+  /* If the field is an array of variable size, we'd better not pack it because
+ this would misalign it and, therefore, probably cause large temporarie to
+ be created in case we need to take its address.  See addressable_p and the
+ notes on the addressability issues for further details.  */
+  if (TREE_CODE (field_type) == ARRAY_TYPE
+  && type_has_variable_size (field_type))
 return 0;
 
   /* In the other cases, we can honor the packing.  */


diff --git a/gcc/ada/gcc-interface/gigi.h b/gcc/ada/gcc-interface/gigi.h
--- a/gcc/ada/gcc-interface/gigi.h
+++ b/gcc/ada/gcc-interface/gigi.h
@@ -838,10 +838,9 @@ extern tree get_base_type (tree type);
in bits.  If we don't know anything about the alignment, return 0.  */
 extern unsigned int known_alignment (tree exp);
 
-/* Return true if TYPE, an aggregate type, contains (or is) an array.
-   If SELF_REFERENTIAL is true, then an additional requirement on the
-   array is that it be self-referential.  */
-extern bool aggregate_type_contains_array_p (tree type, bool self_referential);
+/* Return true if TYPE is a type with variable size or a padding type with a
+   field of variable size or a record that has a field with such a type.  */
+extern bool type_has_variable_size (tree type);
 
 /* Return true if VALUE is a multiple of FACTOR. FACTOR must be a power
of 2.  */


diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -2902,12 +2902,10 @@ create_var_decl (tree name, tree asm_name, tree type, tree init,
   return var_decl;
 }
 
-/* Return true if TYPE, an aggregate type, contains (or is) an array.
-   If SELF_REFERENTIAL is true, then an additional requirement on the
-   array is that it be self-referential.  */
+/* Return true if TYPE, an aggregate type, contains (or is) an array.  */
 
-bool
-aggregate_type_contains_array_p (tree type, bool self_referential)
+static bool
+aggregate_type_contains_array_p (tree type)
 {
   switch (TREE_CODE (type))
 {
@@ -2918,14 +2916,13 @@ aggregate_type_contains_array_p (tree type, bool self_referential)
 	tree field;
 	for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
 	  if (AGGREGATE_TYPE_P (TREE_TYPE (field))
-	  && aggregate_type_contains_array_p (TREE_TYPE (field),
-		  self_referential))
+	  && aggregate_type_contains_array_p (TREE_TYPE (field)))
 	return true;
 	return false;
   }
 
 case ARRAY_TYPE:
-  return self_referential ? type_contains_placeholder_p (type) : true;
+  return true;
 
 default:
   gcc_unreachable ();
@@ -2935,7 +2932,7 @@ aggregate_type_contains_array_p (tree type, bool self_referential)
 /* Return true if TYPE is a type with variable size or a padding type with a
field of variable size or a record that has a field with such a type.  */
 
-static bool
+bool
 type_has_variable_size (tree type)
 {
   tree field;
@@ -3037,7 +3034,7 @@ create_field_decl (tree name, tree type, tree record_type, tree size, tree pos,
 		 || (!pos && type_has_variable_size (type))
 		 || (!pos
 		 && AGGREGATE_TYPE_P (type)
-		 && aggregate_type_contains_array_p (type, false
+		 && aggregate_type_contains_array_p (type
 SET_DECL_ALIGN (field_decl, BITS_PER_UNIT);
 
   /* Bump the alignment if need be, either for bitfield/packing purposes or




[Ada] Do not back-annotate maximum size for limited types

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This prevents gigi from back-annotating a maximum size for the Esize of
limited record and concurrent types, in keeping with the implementation
of Analyze_Object_Declaration for objects of these types.

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

gcc/ada/

* gcc-interface/decl.c (gnat_to_gnu_entity): Do not back-annotate a
maximum size for the Esize of limited record and concurrent types.diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -4426,8 +4426,12 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	  tree size = TYPE_SIZE (gnu_type);
 
 	  /* If the size is self-referential, annotate the maximum value
-	 after saturating it, if need be, to avoid a No_Uint value.  */
-	  if (CONTAINS_PLACEHOLDER_P (size))
+	 after saturating it, if need be, to avoid a No_Uint value.
+	 But do not do it for cases where Analyze_Object_Declaration
+	 in Sem_Ch3 would build a default subtype for objects.  */
+	  if (CONTAINS_PLACEHOLDER_P (size)
+	  && !Is_Limited_Record (gnat_entity)
+	  && !Is_Concurrent_Type (gnat_entity))
 	{
 	  const unsigned int align
 		= UI_To_Int (Alignment (gnat_entity)) * BITS_PER_UNIT;




[Ada] Remove obsolete a-assert

2021-12-02 Thread Pierre-Marie de Rodat via Gcc-patches
This package is a leftover after the change for using the base
compiler's runtime during bootstrap. It is not used anymore.

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

gcc/ada/

* gcc-interface/a-assert.ads, gcc-interface/a-assert.adb: Remove.diff --git a/gcc/ada/gcc-interface/a-assert.adb /dev/null
deleted file mode 100644
--- a/gcc/ada/gcc-interface/a-assert.adb
+++ /dev/null
@@ -1,52 +0,0 @@
---
---  --
--- GNAT RUN-TIME COMPONENTS --
---  --
---   A D A . A S S E R T--
---  --
--- B o d y  --
---  --
--- Copyright (C) 2007-2021, Free Software Foundation, Inc.  --
---  --
--- GNAT is free software;  you can  redistribute it  and/or modify it under --
--- terms of the  GNU General Public License as published  by the Free Soft- --
--- ware  Foundation;  either version 3,  or (at your option) any later ver- --
--- sion.  GNAT is distributed in the hope that it will be useful, but WITH- --
--- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
--- or FITNESS FOR A PARTICULAR PURPOSE. --
---  --
--- As a special exception under Section 7 of GPL version 3, you are granted --
--- additional permissions described in the GCC Runtime Library Exception,   --
--- version 3.1, as published by the Free Software Foundation.   --
---  --
--- You should have received a copy of the GNU General Public License and--
--- a copy of the GCC Runtime Library Exception along with this program; --
--- see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see--
--- .  --
---  --
--- GNAT was originally developed  by the GNAT team at  New York University. --
--- Extensive contributions were provided by Ada Core Technologies Inc.  --
---  --
---
-
-package body Ada.Assertions is
-
-   
-   -- Assert --
-   
-
-   procedure Assert (Check : Boolean) is
-   begin
-  if Check = False then
- raise Ada.Assertions.Assertion_Error;
-  end if;
-   end Assert;
-
-   procedure Assert (Check : Boolean; Message : String) is
-   begin
-  if Check = False then
- raise Ada.Assertions.Assertion_Error with Message;
-  end if;
-   end Assert;
-
-end Ada.Assertions;


diff --git a/gcc/ada/gcc-interface/a-assert.ads /dev/null
deleted file mode 100644
--- a/gcc/ada/gcc-interface/a-assert.ads
+++ /dev/null
@@ -1,50 +0,0 @@
---
---  --
--- GNAT RUN-TIME COMPONENTS --
---  --
---   A D A . A S S E R T I O N S--
---  --
---Copyright (C) 2015-2021, Free Software Foundation, Inc.   --
---  --
--- S p e c  --
---  --
--- This specification is derived from the Ada Reference Manual for use with --
--- GNAT. The copyright notice above, and the license provisions that follow --
--- apply solely to the contracts that have been added.  --
---  --
--- GNAT is free software;  you can  redistribute it  and/or modify it under --
--- terms of the  GNU General Public License as published  by the Free Soft- --
--- ware  Foundation;  either version 3,  or (at your option) any later ver- --
--- sion.  GNAT is distributed in the hope that it will be useful, but WITH- --
--- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
--- or FITNESS FOR A PARTICULAR PURPOSE.   

Re: [PATCH] OpenMP: Ensure that offloaded variables are public

2021-12-02 Thread Andrew Stubbs

On 02/12/2021 16:05, Andrew Stubbs wrote:

On 02/12/2021 12:58, Jakub Jelinek wrote:
I've tried modifying offload_handle_link_vars but that spot doesn't 
catch
the omp_data_sizes variables emitted by 
libgomp.c-c++-common/target_42.c,

which was one of the motivating examples.


Why doesn't catch it?  Is the variable created only post-IPA?
I'd think that it should have been created before IPA, streamed and
therefore I don't understand why you don't see it after streaming LTO in.


On closer inspection it does, in fact, catch it as you'd expect, but 
then the variable is no longer marked public when it gets to 
pass_omp_target_link::execute, so something somewhere is resetting it. 
More investigation is needed


The "whole-program" pass is removing the public flag. That's probably 
working as intended, and I assume it is run for offload code on purpose?


My original patch puts the flag back after this point, so it works fine.

Andrew


Re: [PATCH] stddef.h: add support for musl typedef macro guards

2021-12-02 Thread Jeff Law via Gcc-patches




On 11/26/2021 9:57 AM, soeren--- via Gcc-patches wrote:

From: Sören Tempel 

The stddef.h header checks/sets various hardcoded toolchain/os specific
macro guards to prevent redefining types such as ptrdiff_t, wchar_t, or
size_t. However, without this patch, the file does not check/set the
typedef macro guards for musl libc. This causes types such as size_t to
be defined twice for files which include both musl's stddef.h as well as
GCC's ginclude/stddef.h. This is, for example, the case for
libgo/sysinfo.c. If libgo/sysinfo.c has multiple typedefs for size_t
this confuses -fdump-go-spec and causes size_t not to be included in the
generated type definitions thereby causing a gcc-go compilation failure
on Alpine Linux Edge (which uses musl libc) with the following error:

sysinfo.go:7765:13: error: use of undefined type '_size_t'
 7765 | type Size_t _size_t
  | ^
libcall_posix.go:49:35: error: non-integer len argument in make
   49 | b := make([]byte, len)
  |

This commit fixes this issue by ensuring that ptrdiff_t, wchar_t, and size_t
are only defined once in the pre-processed libgo/sysinfo.c file by enhancing
gcc/ginclude/stddef.h with musl-specific typedef macro guards.

gcc/ChangeLog:

* ginclude/stddef.h (__DEFINED_ptrdiff_t): Add support for musl
libc typedef macro guard.
(__DEFINED_size_t): Ditto.
(__DEFINED_wchar_t): Ditto.
So what doesn't make sense here is how both stddef.h files get 
included.  That's the core problem I think you need to resolve.


Jeff



Re: [PATCH] rs6000: Builtins test changes for test_fpscr_[d]rn_builtin_error.c

2021-12-02 Thread Bill Schmidt via Gcc-patches
Hi!

On 12/1/21 5:00 PM, Segher Boessenkool wrote:
> On Thu, Nov 18, 2021 at 10:36:52AM -0600, Bill Schmidt wrote:
>> Hi!  This is the last patch broken out of the previous test suite patch
>> for the new builtins support.
> Whew :-)
>
>> One advantage of the new builtins support is uniform error messages for
>> arguments with restricted values.  Previously this was done in many places
>> in an ad hoc manner, with little uniformity.  This patch adjusts the
>> expected error messages accordingly.
>>
>> All such error messages are now one of the following:
>>   "argument %d must be a %d-bit unsigned literal"
>>   "argument %d must be a literal between %d and %d, inclusive"
>>   "argument %d must be a variable or a literal between %d and %d, inclusive"
>>   "argument %d must be either a literal %d or a literal %d"
>>
>> These messages were chosen to require the fewest changes from previous
>> messages while still introducing uniformity.  This patch adjusts error
>> messages for some cases where this produces changed messages.  In
>> particular, some messages are improved because previously they did not
>> admit the possibility that an argument could hold a variable.
> Same comment as on the previous patch.  But, okay for trunk.  Thanks!

Thank you for all of the reviews!  I combined recent patches that need to go
upstream together to avoid bisect problems, and pushed them as r12-5752.
The new built-in infrastructure is now enabled!

I'll start working on a patch series to remove all the no longer needed code.

Thanks again for all of your help in getting this work reviewed, and for
all the improvements as a result!

Bill

>
>
> Segher


Re: [PATCH] OpenMP: Ensure that offloaded variables are public

2021-12-02 Thread Jakub Jelinek via Gcc-patches
On Thu, Dec 02, 2021 at 04:31:36PM +, Andrew Stubbs wrote:
> On 02/12/2021 16:05, Andrew Stubbs wrote:
> > On 02/12/2021 12:58, Jakub Jelinek wrote:
> > > > I've tried modifying offload_handle_link_vars but that spot
> > > > doesn't catch
> > > > the omp_data_sizes variables emitted by
> > > > libgomp.c-c++-common/target_42.c,
> > > > which was one of the motivating examples.
> > > 
> > > Why doesn't catch it?  Is the variable created only post-IPA?
> > > I'd think that it should have been created before IPA, streamed and
> > > therefore I don't understand why you don't see it after streaming LTO in.
> > 
> > On closer inspection it does, in fact, catch it as you'd expect, but
> > then the variable is no longer marked public when it gets to
> > pass_omp_target_link::execute, so something somewhere is resetting it.
> > More investigation is needed
> 
> The "whole-program" pass is removing the public flag. That's probably
> working as intended, and I assume it is run for offload code on purpose?

So you'd stick it somewhere into e.g. symbol_table::compile
after ipa_passes call, guarded with #ifdef ACCEL_COMPILER ?

Jakub



[committed] libstdc++: Restore unconditional atomic load in COW std::string

2021-12-02 Thread Jonathan Wakely via Gcc-patches
On Wed, 1 Dec 2021 at 18:24, Jonathan Wakely wrote:
>
> On Wed, 1 Dec 2021 at 18:16, Florian Weimer wrote:
> >
> > * Jonathan Wakely via Libstdc:
> >
> > > diff --git a/libstdc++-v3/include/bits/cow_string.h 
> > > b/libstdc++-v3/include/bits/cow_string.h
> > > index ced395b80b8..4fae1d02981 100644
> > > --- a/libstdc++-v3/include/bits/cow_string.h
> > > +++ b/libstdc++-v3/include/bits/cow_string.h
> > > @@ -105,7 +105,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > *  destroy the empty-string _Rep object.
> > > *
> > > *  All but the last paragraph is considered pretty conventional
> > > -   *  for a C++ string implementation.
> > > +   *  for a Copy-On-Write C++ string implementation.
> > >*/
> > >// 21.3  Template class basic_string
> > >template
> > > @@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > // so we need to use an atomic load. However, _M_is_leaked
> > > // predicate does not change concurrently (i.e. the string is 
> > > either
> > > // leaked or not), so a relaxed load is enough.
> > > -   return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
> > > -#else
> > > -   return this->_M_refcount < 0;
> > > +   if (!__gnu_cxx::__is_single_threaded())
> > > + return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 
> > > 0;
> > >  #endif
> > > +   return this->_M_refcount < 0;
> > >   }
> >
> > Relaxed MO loads of word-size values on all current architectures only
> > have a compiler barrier, so I think the optimization makes things worse?
>
> Hmm, yes.
>
> > (I doubt the conditional lack of a compiler barrier leads to
> > optimization improvements elsewhere.)
>
> Probably not. I'll revert the change to _M_is_leaked() and just keep
> it for _M_is_shared().
>
> Thanks for pointing that out.

Reverted by the attached patch, tested powerpc64le-linux.
commit b5a568683f71b4a8b1e4e45a43484398e9a66ff2
Author: Jonathan Wakely 
Date:   Wed Dec 1 20:58:58 2021

libstdc++: Restore unconditional atomic load in COW std::string

The relaxed load is already optimal, checking the __single_threaded
global before doing a non-atomic load isn't an optimization.

libstdc++-v3/ChangeLog:

* include/bits/cow_string.h (basic_string::_M_is_leaked()):
Revert change to check __is_single_threaded() before using
atomic load.

diff --git a/libstdc++-v3/include/bits/cow_string.h 
b/libstdc++-v3/include/bits/cow_string.h
index d6ddf3489d1..389b39583e4 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  // so we need to use an atomic load. However, _M_is_leaked
  // predicate does not change concurrently (i.e. the string is either
  // leaked or not), so a relaxed load is enough.
- if (!__gnu_cxx::__is_single_threaded())
-   return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
-#endif
+ return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
+#else
  return this->_M_refcount < 0;
+#endif
}
 
bool


Re: [PATCH] libstdc++: Remove broken std::allocator base classes [PR103340]

2021-12-02 Thread Jonathan Wakely via Gcc-patches
On Wed, 24 Nov 2021 at 13:25, Jonathan Wakely via Libstdc++
 wrote:
>
> I plan to commit this Real Soon. Please yell if you need these
> alternative std::allocator back-ends to stay (and explain how you're
> using them when they've  been broken for years, and start sending test
> results to the gcc-testresults mailing list, and ideally offer to
> maintain them).
>
> Tested powerpc64le-linux.

Nobody has objected or said they need these so I'm going ahead with
removing them.

This version of the patch also removes the
config/allocator/*_allocator_base.h headers that are no longer needed.

Tested powerpc64le-linux, pushed to trunk.
commit e2e98f524fdb80c16e3395f20fee930fbcad5562
Author: Jonathan Wakely 
Date:   Wed Dec 1 16:30:30 2021

libstdc++: Remove broken std::allocator base classes [PR103340]

The bitmap_allocator, __mt_alloc and __pool_alloc extensions are no
longer suitable for use as the base class of std::allocator, because
they have not been updated to meet the C++20 requirements.  There is a
patch attached to PR 103340 which addresses that, but more work would be
needed to solve the linking errors that occur when the library is
configured to use them.

Using --enable-libstdcxx-allocator=bitmap wouldn't even bootstrap for
the past few years, and I can't find any gcc-testresults reports using
any of these allocators. This patch removes the configure option to use
these as the std::allocator base class. The allocators are still in the
tree and can be used directly, you just can't configure the library to
use one of them as the base class of std::allocator.

libstdc++-v3/ChangeLog:

PR libstdc++/103340
PR libstdc++/103400
PR libstdc++/103381
* acinclude.m4 (GLIBCXX_ENABLE_ALLOCATOR): Remove mt, bitmap
and pool options.
* configure: Regenerate.
* config/allocator/bitmap_allocator_base.h: Removed.
* config/allocator/mt_allocator_base.h: Removed.
* config/allocator/pool_allocator_base.h: Removed.
* doc/xml/manual/allocator.xml: Update.
* doc/xml/manual/configure.xml: Update.
* doc/xml/manual/evolution.xml: Document removal.
* doc/xml/manual/mt_allocator.xml: Editorial tweaks.
* doc/html/manual/*: Regenerate.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 71321055de7..6d9a8875e31 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2599,7 +2599,7 @@ AC_DEFUN([GLIBCXX_ENABLE_ALLOCATOR], [
   AC_MSG_CHECKING([for std::allocator base class])
   GLIBCXX_ENABLE(libstdcxx-allocator,auto,[[[=KIND]]],
 [use KIND for target std::allocator base],
-[permit new|malloc|mt|bitmap|pool|yes|no|auto])
+[permit new|malloc|yes|no|auto])
 
   # If they didn't use this option switch, or if they specified --enable
   # with no specific model, we'll have to look for one.  If they
@@ -2631,26 +2631,14 @@ AC_DEFUN([GLIBCXX_ENABLE_ALLOCATOR], [
 
   # Set configure bits for specified locale package
   case ${enable_libstdcxx_allocator_flag} in
-bitmap)
-  ALLOCATOR_H=config/allocator/bitmap_allocator_base.h
-  ALLOCATOR_NAME=__gnu_cxx::bitmap_allocator
-  ;;
 malloc)
   ALLOCATOR_H=config/allocator/malloc_allocator_base.h
   ALLOCATOR_NAME=__gnu_cxx::malloc_allocator
   ;;
-mt)
-  ALLOCATOR_H=config/allocator/mt_allocator_base.h
-  ALLOCATOR_NAME=__gnu_cxx::__mt_alloc
-  ;;
 new)
   ALLOCATOR_H=config/allocator/new_allocator_base.h
   ALLOCATOR_NAME=__gnu_cxx::new_allocator
   ;;
-pool)
-  ALLOCATOR_H=config/allocator/pool_allocator_base.h
-  ALLOCATOR_NAME=__gnu_cxx::__pool_alloc
-  ;;
   esac
 
   GLIBCXX_CONDITIONAL(ENABLE_ALLOCATOR_NEW,
diff --git a/libstdc++-v3/config/allocator/bitmap_allocator_base.h 
b/libstdc++-v3/config/allocator/bitmap_allocator_base.h
deleted file mode 100644
index 40ccbf03449..000
--- a/libstdc++-v3/config/allocator/bitmap_allocator_base.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// Base to std::allocator -*- C++ -*-
-
-// Copyright (C) 2004-2021 Free Software Foundation, Inc.
-//
-// This file is part of the GNU ISO C++ Library.  This library is free
-// software; you can redistribute it and/or modify it under the
-// terms of the GNU General Public License as published by the
-// Free Software Foundation; either version 3, or (at your option)
-// any later version.
-
-// This library is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// Under Section 7 of GPL version 3, you are granted additional
-// permissions described in the GCC Runtime Library Exception, version
-// 3.1, as published by the Free Software Foundation.
-
-// You should have re

[committed] libstdc++: Allow exception classes to move fully-dynamic strings

2021-12-02 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64le-linux using --enable-fully-dynamic-string and
x86_64-linux using the default config. Pushed to trunk.


The move constructor for the fully-dynamic std::basic_string was not
noexcept until recently, so the std::logic_error and std::runtime_error
move constructors were defined to make non-throwing copies of their
string members, instead of potentially-throwing moves.

Now that move construction is always noexecpt, the exception classes can
always move the string. The fully-dynamic string move assignment was
always noexcept, so I don't know why I special-cased the move assignment
operators of the exception classes. That can be changed too.

libstdc++-v3/ChangeLog:

* src/c++11/cow-stdexcept.cc [_GLIBCXX_FULY_DYNAMIC_STRING]
(logic_error, runtime_error): Remove custom definitions.
---
 libstdc++-v3/src/c++11/cow-stdexcept.cc | 20 
 1 file changed, 20 deletions(-)

diff --git a/libstdc++-v3/src/c++11/cow-stdexcept.cc 
b/libstdc++-v3/src/c++11/cow-stdexcept.cc
index 751c3302303..2d47acb55b2 100644
--- a/libstdc++-v3/src/c++11/cow-stdexcept.cc
+++ b/libstdc++-v3/src/c++11/cow-stdexcept.cc
@@ -57,8 +57,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // These operations are noexcept even though copying a COW string is not,
   // but we know that the string member in an exception has not been "leaked"
   // so copying is a simple reference count increment.
-  // For the fully dynamic string moves are not noexcept (due to needing to
-  // allocate an empty string) so we just define the moves as copies here.
 
   logic_error::logic_error(const logic_error& e) noexcept
   : exception(e), _M_msg(e._M_msg) { }
@@ -66,19 +64,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   logic_error& logic_error::operator=(const logic_error& e) noexcept
   { _M_msg = e._M_msg; return *this; }
 
-#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
   logic_error::logic_error(logic_error&& e) noexcept = default;
 
   logic_error&
   logic_error::operator=(logic_error&& e) noexcept = default;
-#else
-  logic_error::logic_error(logic_error&& e) noexcept
-  : exception(e), _M_msg(e._M_msg) { }
-
-  logic_error&
-  logic_error::operator=(logic_error&& e) noexcept
-  { _M_msg = e._M_msg; return *this; }
-#endif
 
   runtime_error::runtime_error(const runtime_error& e) noexcept
   : exception(e), _M_msg(e._M_msg) { }
@@ -87,19 +76,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   runtime_error::operator=(const runtime_error& e) noexcept
   { _M_msg = e._M_msg; return *this; }
 
-#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
   runtime_error::runtime_error(runtime_error&& e) noexcept = default;
 
   runtime_error&
   runtime_error::operator=(runtime_error&& e) noexcept = default;
-#else
-  runtime_error::runtime_error(runtime_error&& e) noexcept
-  : exception(e), _M_msg(e._M_msg) { }
-
-  runtime_error&
-  runtime_error::operator=(runtime_error&& e) noexcept
-  { _M_msg = e._M_msg; return *this; }
-#endif
 
   // New C++11 constructors:
 
-- 
2.31.1



[PATCH] libstdc++: Do not leak empty COW strings

2021-12-02 Thread Jonathan Wakely via Gcc-patches
Apart from "don't bother changing the COW string", does anybody see a
reason we shouldn't do this? This passes all tests for normal COW
strings and fully-dynamic COW strings.


When non-const references, pointers or iterators are obtained to the
contents of a COW std::basic_string, the implementation has to assume it
could result in a write to the contents. If the string was previously
shared, it does the "copy-on-write" step of creating a new copy of the
data that is not shared by another object.  It also marks the string as
"leaked", so that no future copies of it will share ownership either.

However, if the string is empty then the only character in the sequence
is the terminating null, and modifying that is undefined behaviour. This
means that non-const references/pointers/iterators to an empty string
are effectively const. Since no direct modification is possible, there
is no need to "leak" the string, it can be safely shared with other
objects. This avoids unnecessary allocations to create new copies of
empty strings that can't be modified anyway.

We already did this optimization for strings that share ownership of the
static _S_empty_rep() object, but not for strings that have non-zero
capacity, and not for fully-dynamic-strings (where the _S_empty_rep()
object is never used).

With this change we avoid two allocations in the return statement:

  std::string s;
  s.reserve(1);   // allocate
  std::string s2 = s;
  std::string s3 = s;
  return s[0] + s2[0] + s3[0]; // leak+allocate twice

libstdc++-v3/ChangeLog:

* include/bits/cow_string.h (basic_string::_M_leak_hard): Do not
reallocate an empty string.
---
 libstdc++-v3/include/bits/cow_string.h | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/include/bits/cow_string.h 
b/libstdc++-v3/include/bits/cow_string.h
index 389b39583e4..b21a7422246 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -3366,10 +3366,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 basic_string<_CharT, _Traits, _Alloc>::
 _M_leak_hard()
 {
-#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
-  if (_M_rep() == &_S_empty_rep())
+  // No need to create a new copy of an empty string when a non-const
+  // reference/pointer/iterator into it is obtained. Modifying the
+  // trailing null character is undefined, so the ref/pointer/iterator
+  // is effectively const anyway.
+  if (this->empty())
return;
-#endif
+
   if (_M_rep()->_M_is_shared())
_M_mutate(0, 0, 0);
   _M_rep()->_M_set_leaked();
-- 
2.31.1



[PATCH] libstdc++: Fix non-reserved name in std::allocator base class [PR64135]

2021-12-02 Thread Jonathan Wakely via Gcc-patches
I'd like to push this to trunk to fix PR64135.

It think this fixes the use of a non-reserved name, while preserving the
ABI as far as class layout goes. It will break any programs which rely
on std::allocator having a __gnu_cxx::new_allocator base class, e.g. so
that is_convertible<__gnu_cxx::new_allocator&, std::allocator&> is
true, but I don't see why anybody would do that.

...

The possible base classes of std::allocator are new_allocator and
malloc_allocator, which both cause a non-reserved name to be declared in
every program that includes the definition of std::allocator. This is
non-conforming.

This change replaces __gnu_cxx::new_allocator with std::__new_allocator
which is identical except for using a reserved name. The non-standard
extension __gnu_cxx::new_allocator is preserved as a thin wrapper over
std::__new_allocator. There is no problem with the extension using a
non-reserved name now that it's not included by default in other
headers.

The same change could be done to __gnu_cxx::malloc_allocator but as it's
not the default configuration it can wait.

libstdc++-v3/ChangeLog:

PR libstdc++/64135
* config/allocator/new_allocator_base.h: Include
 instead of .
(__allocator_base): Use std::__new_allocator instead of
__gnu_cxx::new_allocator.
* include/Makefile.am: Add bits/new_allocator.h.
* include/Makefile.in: Regenerate.
* include/experimental/memory_resource (new_delete_resource):
Use std::__new_allocator instead of __gnu_cxx::new_allocator.
* include/ext/new_allocator.h (new_allocator): Derive from
std::__new_allocator. Move implementation to ...
* include/bits/new_allocator.h: New file.
* testsuite/20_util/allocator/64135.cc: New test.
---
 .../config/allocator/new_allocator_base.h |  11 +-
 libstdc++-v3/include/Makefile.am  |   1 +
 libstdc++-v3/include/Makefile.in  |   1 +
 libstdc++-v3/include/bits/new_allocator.h | 223 ++
 .../include/experimental/memory_resource  |   4 +-
 libstdc++-v3/include/ext/new_allocator.h  | 157 +---
 .../testsuite/20_util/allocator/64135.cc  |  45 
 7 files changed, 279 insertions(+), 163 deletions(-)
 create mode 100644 libstdc++-v3/include/bits/new_allocator.h
 create mode 100644 libstdc++-v3/testsuite/20_util/allocator/64135.cc

diff --git a/libstdc++-v3/config/allocator/new_allocator_base.h 
b/libstdc++-v3/config/allocator/new_allocator_base.h
index 7c52fef63de..a139f2fb668 100644
--- a/libstdc++-v3/config/allocator/new_allocator_base.h
+++ b/libstdc++-v3/config/allocator/new_allocator_base.h
@@ -30,7 +30,7 @@
 #ifndef _GLIBCXX_CXX_ALLOCATOR_H
 #define _GLIBCXX_CXX_ALLOCATOR_H 1
 
-#include 
+#include 
 
 #if __cplusplus >= 201103L
 namespace std
@@ -38,18 +38,17 @@ namespace std
   /**
*  @brief  An alias to the base class for std::allocator.
*
-   *  Used to set the std::allocator base class to
-   *  __gnu_cxx::new_allocator.
+   *  Used to set the std::allocator base class to std::__new_allocator.
*
*  @ingroup allocators
*  @tparam  _Tp  Type of allocated object.
 */
   template
-using __allocator_base = __gnu_cxx::new_allocator<_Tp>;
+using __allocator_base = __new_allocator<_Tp>;
 }
 #else
-// Define new_allocator as the base class to std::allocator.
-# define __allocator_base  __gnu_cxx::new_allocator
+// Define __new_allocator as the base class to std::allocator.
+# define __allocator_base  __new_allocator
 #endif
 
 #ifndef _GLIBCXX_SANITIZE_STD_ALLOCATOR
diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 25a8d9c8a41..f1cf79615c8 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -159,6 +159,7 @@ bits_headers = \
${bits_srcdir}/mofunc_impl.h \
${bits_srcdir}/move.h \
${bits_srcdir}/move_only_function.h \
+   ${bits_srcdir}/new_allocator.h \
${bits_srcdir}/node_handle.h \
${bits_srcdir}/ostream.tcc \
${bits_srcdir}/ostream_insert.h \
diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
index 47a5d985049..4e4a240831a 100644
--- a/libstdc++-v3/include/Makefile.in
+++ b/libstdc++-v3/include/Makefile.in
@@ -509,6 +509,7 @@ bits_headers = \
${bits_srcdir}/mofunc_impl.h \
${bits_srcdir}/move.h \
${bits_srcdir}/move_only_function.h \
+   ${bits_srcdir}/new_allocator.h \
${bits_srcdir}/node_handle.h \
${bits_srcdir}/ostream.tcc \
${bits_srcdir}/ostream_insert.h \
diff --git a/libstdc++-v3/include/bits/new_allocator.h 
b/libstdc++-v3/include/bits/new_allocator.h
new file mode 100644
index 000..4d85612720d
--- /dev/null
+++ b/libstdc++-v3/include/bits/new_allocator.h
@@ -0,0 +1,223 @@
+// Allocator that wraps operator new -*- C++ -*-
+
+// Copyright (C) 2001-2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Lib

Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches



On 2021-12-02 11:13, Jakub Jelinek wrote:

On Thu, Dec 02, 2021 at 11:03:46AM -0500, Vladimir Makarov wrote:

--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2797,6 +2797,7 @@ static void
  setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
  {
int i, length, nrefs, priority, max_priority, mult, diff;
+  bool overflow_backup_p = true;
ira_allocno_t a;
  
max_priority = 0;

@@ -2811,9 +2812,25 @@ setup_allocno_priorities (ira_allocno_t 
*consideration_allocnos, int n)
diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
/* Multiplication can overflow for very large functions.
 Check the overflow and constrain the result if necessary: */
+#ifdef __has_builtin
+#if __has_builtin(__builtin_smul_overflow)
+  overflow_backup_p = false;
if (__builtin_smul_overflow (mult, diff, &priority)
  || priority <= -INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
+#endif
+#endif
+  if (overflow_backup_p)
+   {
+ static_assert
+   (sizeof (long long) >= 2 * sizeof (int),
+"overflow code does not work for such int and long long sizes");
+ long long priorityll = (long long) mult * diff;
+ if (priorityll < -INT_MAX || priorityll > INT_MAX)
+   priority = diff >= 0 ? INT_MAX : -INT_MAX;
+ else
+   priority = priorityll;
+   }

So simple problem and so many details :)

This will require that long long is at least twice as large as int
everywhere, I thought you wanted to do that only when
__builtin_smul_overflow isn't available.


That is not critical as GCC and probably all others C++ compiler support 
only targets with this assertion.  I guess it is better to find this 
problem earlier on targets (if any) where it is not true *independently* 
on used compiler.


So it is difficult for me to know what is better.  Probably for 
GCC/Clang oriented world, your variant is better as it permits to 
compile the code by GCC even on targets where the assertion is false.



That would be
#ifdef __has_builtin
#if __has_builtin(__builtin_smul_overflow)
#define HAS_SMUL_OVERFLOW
#endif
#endif
#ifdef HAS_SMUL_OVERFLOW
   if (__builtin_smul_overflow (mult, diff, &priority)
  || priority <= -INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
#else
   static_assert (sizeof (long long) >= 2 * sizeof (int),
 "overflow code does not work for int wider"
 "than half of long long");
   long long priorityll = (long long) mult * diff;
   if (priorityll < -INT_MAX || priorityll > INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
   else
priority = priorityll;
#endif
Why priority <= -INT_MAX in the first case though,
shouldn't that be < -INT_MAX ?


My thought was to avoid 'always false' warning for non two's compliment 
binary representation targets.  As I remember C++17 started to require 
only two-compliment integers.  If we require to use only c++17 and 
upper, then probably it is better to fix it.


In any case, I feel these details are not my area of expertise. If you 
believe I should do these changes, please confirm that you want these 
changes and I'll definitely do this.  Thank you, Jakub.








[wwwdocs] Document --enable-libstdcxx-allocator changes

2021-12-02 Thread Jonathan Wakely via Gcc-patches
Pushed to wwwdocs.


---
 htdocs/gcc-12/changes.html | 5 +
 1 file changed, 5 insertions(+)

diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html
index 000501fb..c2b87a53 100644
--- a/htdocs/gcc-12/changes.html
+++ b/htdocs/gcc-12/changes.html
@@ -41,6 +41,11 @@ a work-in-progress.
 member instead of a literal 0, as this is portable to other
 C++ implementations.
   
+  The configuration option --enable-libstdcxx-allocator
+no longer supports the bitmap, mt, and
+pool arguments. Those configurations had been broken for
+some time.
+  
   
 Fortran:
 OpenMP code using the omp_lib.h include file can no longer be
-- 
2.31.1



[PATCH] libiberty rust-demangle, ignore .suffix

2021-12-02 Thread Mark Wielaard
Rust v0 symbols can have a .suffix because if compiler transformations.
These can be ignored it the demangled name. Which is what this patch
implements). But an alternative implementation could be to follow what
C++ does and represent these as [clone .suffix] tagged onto the
demangled name. But this seems somewhat confusing since it results in
a demangled name that cannot be mangled again. And it would mean
trying to decode compiler internal naming.

https://bugs.kde.org/show_bug.cgi?id=445916
https://github.com/rust-lang/rust/issues/60705

libiberty/Changelog

* rust-demangle.c (rust_demangle_callback): Ignore everything
after '.' char in sym for v0.
* testsuite/rust-demangle-expected: Add new .suffix testcase.
---
 libiberty/rust-demangle.c  | 4 
 libiberty/testsuite/rust-demangle-expected | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index 449941b56dc1..dee9eb64df79 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -1340,6 +1340,10 @@ rust_demangle_callback (const char *mangled, int options,
   /* Rust symbols (v0) use only [_0-9a-zA-Z] characters. */
   for (p = rdm.sym; *p; p++)
 {
+  /* Rust v0 symbols can have '.' suffixes, ignore those.  */
+  if (rdm.version == 0 && *p == '.')
+   break;
+
   rdm.sym_len++;
 
   if (*p == '_' || ISALNUM (*p))
diff --git a/libiberty/testsuite/rust-demangle-expected 
b/libiberty/testsuite/rust-demangle-expected
index 7dca315d0054..bc32ed85f882 100644
--- a/libiberty/testsuite/rust-demangle-expected
+++ b/libiberty/testsuite/rust-demangle-expected
@@ -295,3 +295,9 @@ _RMCs4fqI2P2rA04_13const_genericINtB0_4CharKc2202_E
 --format=auto
 _RNvNvMCs4fqI2P2rA04_13const_genericINtB4_3FooKpE3foo3FOO
 >::foo::FOO
+#
+# Suffixes
+#
+--format=rust
+_RNvMs0_NtCs5l0EXMQXRMU_21rustc_data_structures17obligation_forestINtB5_16ObligationForestNtNtNtCsdozMG8X9FIu_21rustc_trait_selection6traits7fulfill26PendingPredicateObligationE22register_obligation_atB1v_.llvm.8517020237817239694
+>::register_obligation_at
-- 
2.18.4



Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches



On 2021-12-02 12:06, Vladimir Makarov wrote:


On 2021-12-02 11:13, Jakub Jelinek wrote:

On Thu, Dec 02, 2021 at 11:03:46AM -0500, Vladimir Makarov wrote:

--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2797,6 +2797,7 @@ static void
  setup_allocno_priorities (ira_allocno_t *consideration_allocnos, 
int n)

  {
    int i, length, nrefs, priority, max_priority, mult, diff;
+  bool overflow_backup_p = true;
    ira_allocno_t a;
      max_priority = 0;
@@ -2811,9 +2812,25 @@ setup_allocno_priorities (ira_allocno_t 
*consideration_allocnos, int n)

    diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
    /* Multiplication can overflow for very large functions.
   Check the overflow and constrain the result if necessary: */
+#ifdef __has_builtin
+#if __has_builtin(__builtin_smul_overflow)
+  overflow_backup_p = false;
    if (__builtin_smul_overflow (mult, diff, &priority)
    || priority <= -INT_MAX)
  priority = diff >= 0 ? INT_MAX : -INT_MAX;
+#endif
+#endif
+  if (overflow_backup_p)
+    {
+  static_assert
+    (sizeof (long long) >= 2 * sizeof (int),
+ "overflow code does not work for such int and long long 
sizes");

+  long long priorityll = (long long) mult * diff;
+  if (priorityll < -INT_MAX || priorityll > INT_MAX)
+    priority = diff >= 0 ? INT_MAX : -INT_MAX;
+  else
+    priority = priorityll;
+    }

So simple problem and so many details :)

This will require that long long is at least twice as large as int
everywhere, I thought you wanted to do that only when
__builtin_smul_overflow isn't available.


That is not critical as GCC and probably all others C++ compiler 
support only targets with this assertion.  I guess it is better to 
find this problem earlier on targets (if any) where it is not true 
*independently* on used compiler.


So it is difficult for me to know what is better.  Probably for 
GCC/Clang oriented world, your variant is better as it permits to 
compile the code by GCC even on targets where the assertion is false.



After some more considerations, I think you are right and the backup 
code should be conditional.  Because otherwise, there is no sense to use 
code with __builtin_smul_overflow.  I'll do the changes.





Re: [PATCH] libiberty rust-demangle, ignore .suffix

2021-12-02 Thread Eduard-Mihai Burtescu
On Thu, Dec 2, 2021, at 19:17, Mark Wielaard wrote:
> Rust v0 symbols can have a .suffix because if compiler transformations.

For some context, the suffix comes from LLVM (I believe as part of its LTO).
If this were a semantic part of a Rust symbol, it would have an encoding within 
v0 (as we already do for e.g. shims).

That also means that for consistency, suffixes like these should be handled 
uniformly for both v0 and legacy (as rustc-demangle does), since LLVM doesn't 
distinguish.

You may even be able to get Clang to generate C++ mangled symbols with ".llvm." 
suffixes, with enough application of LTO.
This is not unlike GCC ".clone" suffixes, AFAIK.
Sadly I don't think there's a way to handle both as "outside the symbol", 
without hardcoding ".llvm." in the implementation.

I don't recall the libiberty demangling API having any provisions for the 
demangler deciding that a mangled symbol "stops early", which would maybe allow 
for a more general solution.

Despite all that, if it helps in practice, I would still not mind this patch 
landing in its current form, I just wanted to share my perspective on the 
larger issue.

Thanks,
- Eddy B.


Re: [SVE] PR96463 - Optimise svld1rq from vectors

2021-12-02 Thread Richard Sandiford via Gcc-patches
Prathamesh Kulkarni  writes:
> Hi Richard,
> I have attached a WIP untested patch for PR96463.
> IIUC, the PR suggests to transform
> lhs = svld1rq ({-1, -1, ...}, &v[0])
> into:
> lhs = vec_perm_expr
> if v is vector of 4 elements, and each element is 32 bits on little
> endian target ?
>
> I am sorry if this sounds like a silly question, but I am not sure how
> to convert a vector of type int32x4_t into svint32_t ? In the patch, I
> simply used NOP_EXPR (which I expected to fail), and gave type error
> during gimple verification:

It should be possible in principle to have a VEC_PERM_EXPR in which
the operands are Advanced SIMD vectors and the result is an SVE vector.

E.g., the dup in the PR would be something like this:

foo (int32x4_t a)
{
  svint32_t _2;

  _2 = VEC_PERM_EXPR ;
  return _2;
}

where the final operand can be built using:

  int source_nelts = TYPE_VECTOR_SUBPARTS (…rhs type…).to_constant ();
  vec_perm_builder sel (TYPE_VECTOR_SUBPARTS (…lhs type…), source_nelts, 1);
  for (int i = 0; i < source_nelts; ++i)
sel.quick_push (i);

I'm not sure how well-tested that combination is though.  It might need
changes to target-independent code.

Thanks,
Richard


Re: [PR103437] [committed] IRA: Process multiplication overflow in priority calculation for allocno assignments

2021-12-02 Thread Vladimir Makarov via Gcc-patches


On 2021-12-02 12:21, Vladimir Makarov via Gcc-patches wrote:


On 2021-12-02 12:06, Vladimir Makarov wrote:



So simple problem and so many details :)

This will require that long long is at least twice as large as int
everywhere, I thought you wanted to do that only when
__builtin_smul_overflow isn't available.


That is not critical as GCC and probably all others C++ compiler 
support only targets with this assertion.  I guess it is better to 
find this problem earlier on targets (if any) where it is not true 
*independently* on used compiler.


So it is difficult for me to know what is better.  Probably for 
GCC/Clang oriented world, your variant is better as it permits to 
compile the code by GCC even on targets where the assertion is false.



After some more considerations, I think you are right and the backup 
code should be conditional.  Because otherwise, there is no sense to 
use code with __builtin_smul_overflow.  I'll do the changes.



Here is one more patch I've committed.  Jakub, thank your for the 
discussion and your patience.


commit a72b8f376a176c620f1c1c684f2eee2016e6b4c3
Author: Vladimir N. Makarov 
Date:   Thu Dec 2 12:31:28 2021 -0500

[PR103437] Make backup code for overflow conditional

Switch off long long variant overflow code by preprocessor if the
build compiler has __builtin_smul_overflow.

gcc/ChangeLog:
PR rtl-optimization/103437
* ira-color.c (setup_allocno_priorities): Switch off backup code
for overflow if compiler has __builtin_smul_overflow.  Use <
for comparison with -INT_MAX.

diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 3b19a58e1f0..a1b02776e77 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2797,7 +2797,6 @@ static void
 setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
 {
   int i, length, nrefs, priority, max_priority, mult, diff;
-  bool overflow_backup_p = true;
   ira_allocno_t a;
 
   max_priority = 0;
@@ -2810,27 +2809,27 @@ setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
   ira_assert (mult >= 0);
   mult *= ira_reg_class_max_nregs[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)];
   diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
-  /* Multiplication can overflow for very large functions.
-	 Check the overflow and constrain the result if necessary: */
 #ifdef __has_builtin
 #if __has_builtin(__builtin_smul_overflow)
-  overflow_backup_p = false;
+#define HAS_SMUL_OVERFLOW
+#endif
+#endif
+  /* Multiplication can overflow for very large functions.
+	 Check the overflow and constrain the result if necessary: */
+#ifdef HAS_SMUL_OVERFLOW
   if (__builtin_smul_overflow (mult, diff, &priority)
-	  || priority <= -INT_MAX)
+	  || priority < -INT_MAX)
 	priority = diff >= 0 ? INT_MAX : -INT_MAX;
+#else
+  static_assert
+	(sizeof (long long) >= 2 * sizeof (int),
+	 "overflow code does not work for such int and long long sizes");
+  long long priorityll = (long long) mult * diff;
+  if (priorityll < -INT_MAX || priorityll > INT_MAX)
+	priority = diff >= 0 ? INT_MAX : -INT_MAX;
+  else
+	priority = priorityll;
 #endif
-#endif
-  if (overflow_backup_p)
-	{
-	  static_assert
-	(sizeof (long long) >= 2 * sizeof (int),
-	 "overflow code does not work for such int and long long sizes");
-	  long long priorityll = (long long) mult * diff;
-	  if (priorityll < -INT_MAX || priorityll > INT_MAX)
-	priority = diff >= 0 ? INT_MAX : -INT_MAX;
-	  else
-	priority = priorityll;
-	}
   allocno_priorities[ALLOCNO_NUM (a)] = priority;
   if (priority < 0)
 	priority = -priority;


Re: [PATCH] c++: Handle auto(x) in parameter-declaration-clause [PR103401]

2021-12-02 Thread Jason Merrill via Gcc-patches

On 12/2/21 10:27, Marek Polacek wrote:

On Wed, Dec 01, 2021 at 11:24:58PM -0500, Jason Merrill wrote:

On 12/1/21 10:16, Marek Polacek wrote:

In C++23, auto(x) is valid, so decltype(auto(x)) should also be valid,
so

void f(decltype(auto(0)));

should be just as

void f(int);

but currently, everytime we see 'auto' in a parameter-declaration-clause,
we try to synthesize_implicit_template_parm for it, creating a new template
parameter list.  The code above actually has us calling s_i_t_p twice;
once from cp_parser_decltype_expr -> cp_parser_postfix_expression which
fails and then again from cp_parser_decltype_expr -> cp_parser_expression.
So it looks like we have f and we accept ill-formed code.

So we need to be more careful about synthesizing the implicit template
parameter.  cp_parser_postfix_expression looked like a sensible place.


Does this cover other uses of auto in decltype, such as

void f(decltype(new auto{0}));


Yes: the clearing of auto_is_implicit_function_template_parm_p will happen here
too.

However, I'm noticing this:

   void f1(decltype(new auto{0}));
   void f2(decltype(new int{0}));

   void
   g ()
   {
 int i;
 void f3(decltype(new auto{0}));
 void f4(decltype(new int{0}));
 f1 (&i); // error: no matching function for call to f1(int*)
  // couldn't deduce template parameter auto:1
 f2 (&i);
 f3 (&i);
 f4 (&i);
   }
  
I think the error we issue is bogus.  (My patch doesn't change this.  clang++

accepts.)  Should I file a PR (and investigate)?


That certainly suggests that auto_is_implicit_function_template_parm_p 
isn't getting cleared soon enough for f1.



?  Should we adjust this flag in cp_parser_decltype along with all the other
flags?


I think that's possible, but wouldn't cover auto in default arguments, or array
bounds.


I guess cp_parser_sizeof_operand would need the same change.

Do we currently handle auto in default arguments wrong?  Ah, I see that 
we currently set auto_is_... for the whole parameter declaration clause, 
rather than just for the decl-specifier-seq of parameters as the 
standard specifies:


"A placeholder-type-specifier of the form type-constraint opt auto can 
be used as a decl-specifier of the decl-specifier-seq of a 
parameter-declaration of a function declaration or lambda-expression"


Jason



Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-12-02 Thread Gerald Pfeifer
On Sat, 27 Nov 2021, Petter Tomner wrote:
> Ye it is supposed to compile cleanly for 32bit too. 
> 
> I pushed a patch for it as a "free for all". With %zu specifiers.

Thank you, Petter.  I just updated the lang/gcc12-devel port in FreeBSD
to Sunday's snapshot that has those changes, so we shall learn soon if
there's still some problem (though I expect we'll be good).

Gerald


Re: [vect-patterns] Refactor widen_plus/widen_minus as internal_fns

2021-12-02 Thread Richard Sandiford via Gcc-patches
[Review for patch 1]

Joel Hutton  writes:
> From e7b3017b7b5879204e9d61760a85cc84beeb4fe0 Mon Sep 17 00:00:00 2001
> From: Joel Hutton 
> Date: Wed, 25 Aug 2021 14:31:15 +0100
> Subject: [PATCH 1/3] [vect-patterns] Refactor to allow internal_fn's
>
> Hi all,
>
> This refactor allows widening patterns (such as widen_plus/widen_minus) to be 
> represented as
> either internal_fns or tree_codes.
>
> [vect-patterns] Refactor as internal_fn's
>
> Refactor vect-patterns to allow patterns to be internal_fns starting
> with widening_plus/minus patterns
>
> gcc/ChangeLog:
>
>   * gimple-match.h (class code_helper): Move code_helper class to more
> visible function.
>   * internal-fn.h (internal_fn_name): Add internal_fn range check.
>   * tree-vect-patterns.c (vect_recog_widen_op_pattern): Change
> function prototype.
>   * tree-vect-stmts.c (vect_gen_widened_results_half): Refactor to use
> code_helper, build internal_fns
>   (vect_create_vectorized_promotion_stmts): Refactor to use
> code_helper.
>   (vectorizable_conversion): Refactor to use code_helper.
>   (supportable_widening_operation): Refactor to use code_helper.
>   (supportable_narrowing_operation): Refactor to use code_helper.
>   * tree-vectorizer.h (supportable_widening_operation): Refactor to use 
> code_helper.
>   (supportable_narrowing_operation): Refactor to use code_helper.
>   * tree.h (class code_helper):  Refactor to use code_helper.

Sorry for two things.  First, this review is very late.  Second, I'd
forgotten about this code_helper stuff.  I had a nagging feeling that
we'd discussed moving and consolidating code_helper, but I couldn't
remember the context :-(

This code has therefore changed quite a bit in the meantime.  I think
moving it to tree.h is still a good thing to do.

> diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
> index 854cbcff390..4a8ea67e62f 100644
> --- a/gcc/tree-vect-patterns.c
> +++ b/gcc/tree-vect-patterns.c
> @@ -1245,7 +1245,7 @@ vect_recog_sad_pattern (vec_info *vinfo,
>  static gimple *
>  vect_recog_widen_op_pattern (vec_info *vinfo,
>stmt_vec_info last_stmt_info, tree *type_out,
> -  tree_code orig_code, tree_code wide_code,
> +  tree_code orig_code, code_helper wide_code_or_ifn,

I think it'd be better to keep the original “wide_code” name and try to
remove as many places as possible in which switching based on tree_code
or internal_fn is necessary.  The recent gimple-match.h patches should
help with that, but more routines might be needed.

Same comment for the other name changes.

>bool shift_p, const char *name)
>  {
>gimple *last_stmt = last_stmt_info->stmt;
> @@ -1288,15 +1288,16 @@ vect_recog_widen_op_pattern (vec_info *vinfo,
>vecctype = get_vectype_for_scalar_type (vinfo, ctype);
>  }
>  
> -  enum tree_code dummy_code;
> +  code_helper dummy_c_or_ifn;
>int dummy_int;
>auto_vec dummy_vec;
>if (!vectype
>|| !vecitype
>|| !vecctype
> -  || !supportable_widening_operation (vinfo, wide_code, last_stmt_info,
> +  || !supportable_widening_operation (vinfo, wide_code_or_ifn,
> +   last_stmt_info,
> vecitype, vectype,
> -   &dummy_code, &dummy_code,
> +   &dummy_c_or_ifn, &dummy_c_or_ifn,
> &dummy_int, &dummy_vec))
>  return NULL;
>  
> @@ -1309,8 +1310,16 @@ vect_recog_widen_op_pattern (vec_info *vinfo,
>  2, oprnd, half_type, unprom, vectype);
>  
>tree var = vect_recog_temp_ssa_var (itype, NULL);
> -  gimple *pattern_stmt = gimple_build_assign (var, wide_code,
> -   oprnd[0], oprnd[1]);
> +  gimple *pattern_stmt;
> +  if (wide_code_or_ifn.is_tree_code ())
> +pattern_stmt = gimple_build_assign (var, wide_code_or_ifn,
> + oprnd[0], oprnd[1]);
> +  else
> +{
> +  internal_fn fn = as_internal_fn ((combined_fn) wide_code_or_ifn);
> +  pattern_stmt = gimple_build_call_internal (fn, 2, oprnd[0], oprnd[1]);
> +  gimple_call_set_lhs (pattern_stmt, var);
> +}

For example, I think we should hide this inside a new:

  gimple_build (var, wide_code, oprnd[0], oprnd[1]);

that works directly on code_helper, similarly to the new code_helper
gimple_build interfaces.

(Unfortunately, I don't think we can use gimple_build directly here.)

> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 2284ad069e4..d5e1619fabc 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -4504,7 +4504,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, 
> stmt_vec_info stmt_info,
> STMT_INFO is the original scalar stmt that we are vectorizing.  */
>  
>  stat

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

2021-12-02 Thread Jeff Law via Gcc-patches




On 11/18/2021 11:23 PM, Jojo R wrote:


— Jojo
在 2021年11月19日 +0800 AM12:13,Jeff Law ,写道:



On 11/16/2021 7:20 PM, Jojo R via Gcc-patches wrote:

— Jojo
在 2021年11月16日 +0800 PM8:12,Richard Biener
,写道:

On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
 wrote:

Skip renaming if instruction is noop move, and it will
been removed for performance.

Is there any (target specific) testcase you can add? Such
commits are
problematic
when later bisected to since the intent isn't clear.

I made a issue in bugzilla, please check it, thanks.

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

So what Richi is asking is can you construct a testcase for the
testsuite?  Having a BZ is helpful because we can reference it in the
commit message, but a test, even if it's target specific, is even
better
from a long term maintenance standpoint.

I found this issue from the ISA extension vector of risc-v target, and
It has not been upstream by now, normal test case without vector isa
Is difficult to construct for this patch, but I think it’s simple and 
useful for

other ISAs or targets, or recommit this patch after our risc-v Vector ISA
Is ready on master branch ?

Any suggestions ?
So I tried to trigger this on x86, but wasn't able with relatively light 
testing.


My recommendation would be to add a little comment like this before the 
change:


/* If this insn is a noop move, then do not rename in this chain as doing so
    would inhibit removal of the noop move.  */

OK with that change.

Thanks for your patience.

jeff


Re: [PATCH] pch, v2: Add support for PCH for relocatable executables

2021-12-02 Thread Jeff Law via Gcc-patches




On 11/18/2021 1:04 AM, Jakub Jelinek wrote:

On Mon, Nov 08, 2021 at 08:48:07PM +0100, Jakub Jelinek via Gcc-patches wrote:

On Mon, Nov 08, 2021 at 12:46:04PM +0100, Jakub Jelinek via Gcc-patches wrote:

So, if we want to make PCH work for PIEs, I'd say we can:
1) add a new GTY option, say callback, which would act like
skip for non-PCH and for PCH would make us skip it but
remember for address bias translation
2) drop the skip for tree_translation_unit_decl::language
3) change get_unnamed_section to have const char * as
last argument instead of const void *, change
unnamed_section::data also to const char * and update
everything related to that
4) maybe add a host hook whether it is ok to support binaries
changing addresses (the only thing I'm worried is if
some host that uses function descriptors allocates them
dynamically instead of having them somewhere in the
executable)
5) maybe add a gengtype warning if it sees in GTY tracked
structure a function pointer without that new callback
option

So, here is 1), 2), 3) implemented.  With this patch alone,
g++.dg/pch/system-2.C test ICEs.  This is because GCC 12 has added
function::x_range_query member, which is set to &global_ranges on
cfun creation and is:
   range_query * GTY ((skip)) x_range_query;
which means when a PIE binary writes PCH and a PIE binary loaded
at a different address loads it, cfun->x_range_query might be a garbage
pointer.  We can either apply a patch like the attached one after
this inline patch, but then probably callback is misnamed and we should
rename it to relocate_and_skip or something similar.  Or we could
e.g. during gimplification overwrite cfun->x_range_query = &global_ranges.
Other than that make check-gcc check-g++ passes RUNTESTFLAGS=pch.exp.

Note, on stdc++.h.gch/O2g.gch there are just those 10 relocations without
the second patch, with it a few more, but nothing huge.  And for non-PIEs
there isn't really any extra work on the load side except freading two scalar
values and fseek.

Here is an updated version (the previous version doesn't apply any longer
in gt_pch_restore due to the line_maps saving/restoring changes).

Bootstrapped/regtested on x86_64-linux and i686-linux with
--enable-host-shared in addition to normal configure options (and without
Ada which doesn't even with the above options build PIC libgnat.a and links
it) and a hack:
--- gcc/configure.ac.jj 2021-10-28 22:12:31.569299780 +0200
+++ gcc/configure.ac2021-11-09 11:34:33.453776105 +0100
@@ -7566,7 +7566,7 @@ AC_CACHE_CHECK([for -no-pie option],
   [gcc_cv_no_pie=no])
 LDFLAGS="$saved_LDFLAGS"])
  if test "$gcc_cv_no_pie" = "yes"; then
-  NO_PIE_FLAG="-no-pie"
+  NO_PIE_FLAG="-pie"
  fi
  AC_SUBST([NO_PIE_FLAG])
  
to force binaries be PIE, no regressions against non-PIC/PIE build

without this patch.  Ok for trunk?

2021-11-18  Jakub Jelinek  

PR pch/71934
gcc/
* ggc.h (gt_pch_note_callback): Declare.
* gengtype.h (enum typekind): Add TYPE_CALLBACK.
(callback_type): Declare.
* gengtype.c (dbgprint_count_type_at): Handle TYPE_CALLBACK.
(callback_type): New variable.
(process_gc_options): Add CALLBACK argument, handle callback
option.
(set_gc_used_type): Adjust process_gc_options caller, if callback,
set type to &callback_type.
(output_mangled_typename): Handle TYPE_CALLBACK.
(walk_type): Likewise.  Handle callback option.
(write_types_process_field): Handle TYPE_CALLBACK.
(write_types_local_user_process_field): Likewise.
(write_types_local_process_field): Likewise.
(write_root): Likewise.
(dump_typekind): Likewise.
(dump_type): Likewise.
* gengtype-state.c (type_lineloc): Handle TYPE_CALLBACK.
(state_writer::write_state_callback_type): New method.
(state_writer::write_state_type): Handle TYPE_CALLBACK.
(read_state_callback_type): New function.
(read_state_type): Handle TYPE_CALLBACK.
* ggc-common.c (callback_vec): New variable.
(gt_pch_note_callback): New function.
(gt_pch_save): Stream out gt_pch_save function address and relocation
table.
(gt_pch_restore): Stream in saved gt_pch_save function address and
relocation table and apply relocations if needed.
* doc/gty.texi (callback): Document new GTY option.
* varasm.c (get_unnamed_section): Change callback argument's type and
last argument's type from const void * to const char *.
(output_section_asm_op): Change argument's type from const void *
to const char *, remove unnecessary cast.
* tree-core.h (struct tree_translation_unit_decl): Drop GTY((skip))
from language member.
* output.h (unnamed_section_callback): Change argument type from
const void * to const char *.
(struct unnamed_section): Use GTY((callback)) instead of GTY((skip))
  

[committed] wwwdocs: git: Fix casing of GCC

2021-12-02 Thread Gerald Pfeifer
We've been referring to the project as GCC since GCC 2.95.
---
 htdocs/git.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/htdocs/git.html b/htdocs/git.html
index 493b5734..5fbd98bf 100644
--- a/htdocs/git.html
+++ b/htdocs/git.html
@@ -327,7 +327,7 @@ in Git.
   by mailto:ja...@gcc.gnu.org";>Jason Merrill.
 
   gccgo
-  This branch is for the Go front end to gcc.  For more information
+  This branch is for the Go front end to GCC.  For more information
 about the Go programming language,
 see https://go.dev/";>go.dev.  The
 branch is maintained by Ian Lance Taylor.  Patches should be
-- 
2.34.0


[committed] analyzer: add regression test for leak false +ve [PR103526]

2021-12-02 Thread David Malcolm via Gcc-patches
Successfully regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-5757-g38a0ee2649ef236ea2763bb9cfc42dc917c7d3fd.

gcc/testsuite/ChangeLog:
PR analyzer/103526
* gcc.dg/analyzer/pr103526.c: New test.

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

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103526.c 
b/gcc/testsuite/gcc.dg/analyzer/pr103526.c
new file mode 100644
index 000..39d60fd853e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103526.c
@@ -0,0 +1,50 @@
+#include 
+#include 
+
+struct game_state {
+   const char *word;
+   char   *word_state;
+};
+
+const char *const teststr = "test string";
+
+static struct game_state *
+game_new(void)
+{
+   struct game_state tmp = {0};
+   struct game_state *rval = NULL;
+   size_t wordlen;
+
+   tmp.word = teststr;
+   wordlen = strlen(tmp.word);
+   if ((tmp.word_state = malloc(wordlen+1)) == NULL)
+   goto err;
+   if ((rval = malloc(sizeof(*rval))) == NULL)
+   goto err;
+   memcpy(rval, &tmp, sizeof(*rval));
+
+   return (rval);
+err:
+   free(tmp.word_state);
+   free(rval);
+   return (NULL);
+} /* { dg-bogus "leak" } */
+
+static void
+game_free(struct game_state *game)
+{
+   if (game == NULL)
+   return;
+   free(game->word_state);
+   free(game);
+}
+
+int
+main(void)
+{
+   struct game_state *game;
+   if ((game = game_new()) == NULL)
+   exit(1);
+   game_free(game);
+   exit(0);
+}
-- 
2.26.3



[committed] doc: Remove references to FreeBSD 1 and 2

2021-12-02 Thread Gerald Pfeifer
FreeBSD 1 and FreeBSD 2, both still a.out, have been end of life for
over two decades and GCC has not been supporting them for ages, too,
so simply remove references.

gcc:
* doc/install.texi (*-*-freebsd*): Remove references to
FreeBSD 1 and FreeBSD 2.
---
 gcc/doc/install.texi | 4 
 1 file changed, 4 deletions(-)

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 6f999a2fd5a..549da22aa16 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -3935,10 +3935,6 @@ This configuration is intended for embedded systems.
 @end html
 @anchor{x-x-freebsd}
 @heading *-*-freebsd*
-Support for FreeBSD 1 was discontinued in GCC 3.2.  Support for
-FreeBSD 2 (and any mutant a.out variants of FreeBSD 3) was
-discontinued in GCC 4.0.
-
 In order to better utilize FreeBSD base system functionality and match
 the configuration of the system compiler, GCC 4.5 and above as well as
 GCC 4.4 past 2010-06-20 leverage SSP support in libc (which is present
-- 
2.34.0


  1   2   >