[PATCH] rs6000: Handle unresolved overloaded builtin [PR105485]
Hi,
PR105485 exposes that new builtin function framework doesn't handle
unresolved overloaded builtin function well. With new builtin
function support, we don't have builtin info for any overloaded
rs6000_gen_builtins enum, since they are expected to be resolved to
one specific instance. So when function rs6000_gimple_fold_builtin
faces one unresolved overloaded builtin, the access for builtin info
becomes out of bound and gets ICE then.
We should not try to fold one unresolved overloaded builtin there
and as the previous support we should emit one error message during
expansion phase like "unresolved overload for builtin ...".
Bootstrapped and regtested on powerpc64-linux-gnu P8 and
powerpc64le-linux-gnu P9 and P10.
Is it ok for trunk?
BR,
Kewen
-
PR target/105485
gcc/ChangeLog:
* config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add
the handling for unresolved overloaded builtin function.
(rs6000_expand_builtin): Likewise.
gcc/testsuite/ChangeLog:
* g++.target/powerpc/pr105485.C: New test.
---
gcc/config/rs6000/rs6000-builtin.cc | 13 +
gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +
2 files changed, 22 insertions(+)
create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C
diff --git a/gcc/config/rs6000/rs6000-builtin.cc
b/gcc/config/rs6000/rs6000-builtin.cc
index e925ba9fad9..e102305c90c 100644
--- a/gcc/config/rs6000/rs6000-builtin.cc
+++ b/gcc/config/rs6000/rs6000-builtin.cc
@@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
enum tree_code bcode;
gimple *g;
+ /* For an unresolved overloaded builtin, return early here since there
+ is no builtin info for it and we are unable to fold it. */
+ if (fn_code > RS6000_OVLD_NONE)
+return false;
+
size_t uns_fncode = (size_t) fn_code;
enum insn_code icode = rs6000_builtin_info[uns_fncode].icode;
const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname;
@@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /*
subtarget */,
tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
enum rs6000_gen_builtins fcode
= (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl);
+
+ /* Emit error message if it's an unresolved overloaded builtin. */
+ if (fcode > RS6000_OVLD_NONE)
+{
+ error ("unresolved overload for builtin %qF", fndecl);
+ return const0_rtx;
+}
+
size_t uns_fcode = (size_t)fcode;
enum insn_code icode = rs6000_builtin_info[uns_fcode].icode;
diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C
b/gcc/testsuite/g++.target/powerpc/pr105485.C
new file mode 100644
index 000..a3b8290df8c
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/pr105485.C
@@ -0,0 +1,9 @@
+/* It's to verify no ICE here, ignore error/warning messages since
+ they are not test points here. */
+/* { dg-excess-errors "pr105485" } */
+
+template void __builtin_vec_vslv();
+typedef __attribute__((altivec(vector__))) char T;
+T b (T c, T d) {
+return __builtin_vec_vslv(c, d);
+}
Re: [PATCH] [Middle-end] Enhance final_value_replacement_loop to handle bitwise induction.
On Wed, May 11, 2022 at 4:45 PM Richard Biener via Gcc-patches
wrote:
>
> On Mon, May 9, 2022 at 7:19 AM liuhongt wrote:
> >
> > This patch will enable below optimization:
> >
> > {
> > - int bit;
> > - long long unsigned int _1;
> > - long long unsigned int _2;
> > -
> > [local count: 46707768]:
> > -
> > - [local count: 1027034057]:
> > - # tmp_11 = PHI
> > - # bit_13 = PHI
> > - _1 = 1 << bit_13;
> > - _2 = ~_1;
> > - tmp_8 = _2 & tmp_11;
> > - bit_9 = bit_13 + -3;
> > - if (bit_9 != -3(OVF))
> > -goto ; [95.65%]
> > - else
> > -goto ; [4.35%]
> > -
> > - [local count: 46707768]:
> > - return tmp_8;
> > + tmp_12 = tmp_6(D) & 7905747460161236406;
> > + return tmp_12;
> >
> > }
> >
> >
> > Boostrapped and regtested on x86_64-pc-linux-gnu{-m32,}
> > Ok for trunk?
> >
> > gcc/ChangeLog:
> >
> > PR middle-end/103462
> > * match.pd (bitwise_induction_p): New match.
> > * tree-scalar-evolution.c (gimple_bitwise_induction_p):
> > Declare.
> > (analyze_and_compute_bitwise_induction_effect): New function.
> > (enum bit_op_kind): New enum.
> > (final_value_replacement_loop): Enhanced to handle bitwise
> > induction.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/i386/pr103462-1.c: New test.
> > * gcc.target/i386/pr103462-2.c: New test.
> > * gcc.target/i386/pr103462-3.c: New test.
> > * gcc.target/i386/pr103462-4.c: New test.
> > * gcc.target/i386/pr103462-5.c: New test.
> > * gcc.target/i386/pr103462-6.c: New test.
> > ---
> > gcc/match.pd | 7 +
> > gcc/testsuite/gcc.target/i386/pr103462-1.c | 111 +
> > gcc/testsuite/gcc.target/i386/pr103462-2.c | 45 ++
> > gcc/testsuite/gcc.target/i386/pr103462-3.c | 111 +
> > gcc/testsuite/gcc.target/i386/pr103462-4.c | 46 ++
> > gcc/testsuite/gcc.target/i386/pr103462-5.c | 111 +
> > gcc/testsuite/gcc.target/i386/pr103462-6.c | 46 ++
> > gcc/tree-scalar-evolution.cc | 178 -
> > 8 files changed, 654 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-1.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-2.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-3.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-4.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-5.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr103462-6.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 6d691d302b3..24ff5f9e6a8 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -7746,3 +7746,10 @@ and,
> >== TYPE_UNSIGNED (TREE_TYPE (@3
> > && single_use (@4)
> > && single_use (@5
> > +
> > +(for bit_op (bit_and bit_ior bit_xor)
> > + (match (bitwise_induction_p @0 @2 @3)
> > + (bit_op:c (nop_convert1? (bit_not2?@0 (convert3? (lshift integer_onep@1
> > @2 @3)))
> > +
> > +(match (bitwise_induction_p @0 @2 @3)
> > + (bit_not (nop_convert1? (bit_xor@0 (convert2? (lshift integer_onep@1
> > @2)) @3
> > diff --git a/gcc/testsuite/gcc.target/i386/pr103462-1.c
> > b/gcc/testsuite/gcc.target/i386/pr103462-1.c
> > new file mode 100644
> > index 000..1dc4c2acad6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr103462-1.c
> > @@ -0,0 +1,111 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -fdump-tree-sccp-details" } */
> > +/* { dg-final { scan-tree-dump-times {final value replacement} 12 "sccp" }
> > } */
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo (unsigned long long tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit += 3)
> > +tmp &= ~(1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo1 (unsigned long long tmp)
> > +{
> > + for (int bit = 63; bit >= 0; bit -= 3)
> > +tmp &= ~(1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo2 (unsigned long long tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit += 3)
> > +tmp &= (1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo3 (unsigned long long tmp)
> > +{
> > + for (int bit = 63; bit >= 0; bit -= 3)
> > +tmp &= (1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo4 (unsigned long long tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit += 3)
> > +tmp |= ~(1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo5 (unsigned long long tmp)
> > +{
> > + for (int bit = 63; bit >= 0; bit -= 3)
> > +tmp |= ~(1ULL << bit);
> > + return tmp;
> > +}
> > +
> > +unsigned long long
> > +__attribute__((noipa))
> > +foo6 (unsigned long long tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit += 3)
> >
Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD
on 2022/5/13 04:16, Segher Boessenkool wrote: > Hi Piotr, > > On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote: >> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. > > Declared, not defined. These are required to be real functions (on all > platforms that have these functions), not macros or inlines or whatever. > Piotr's reply "FreeBSD doesn't have this function in libm, it's implemented in /usr/include/fenv.h." from [1] made me feel like it's a definition instead of declaration. So I thought the check should use AC_LINK_IFELSE instead, since one fenv.h which doesn't have the definition can still pass the proposed AC_COMPILE_IFELSE check. I just did a further search, the powerpc fenv.h [2] does include the definition of feenableexcept. By comparison, the x86 fenv.h [3] doesn't. But I'm not sure if it's the same as what Piotr's environments have. Hope it's similar. :-) [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593193.html [2] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/powerpc/fenv.h [3] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/x86/fenv.h BR, Kewen
Re: [PATCH v4, rs6000] Add a combine pattern for CA minus one [PR95737]
Hi Haochen,
on 2022/5/13 09:07, HAO CHEN GUI wrote:
> Hi,
>This patch adds a combine pattern for "CA minus one". As CA only has two
> values (0 or 1), we could convert following pattern
> (sign_extend:DI (plus:SI (reg:SI 98 ca)
> (const_int -1 [0x]
> to
>(plus:DI (reg:DI 98 ca)
> (const_int -1 [0x])))
>With this patch, one unnecessary sign extend is eliminated.
>
>Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.
>
> ChangeLog
> 2022-05-13 Haochen Gui
>
> gcc/
> PR target/95737
> * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define.
>
Nit: (*extenddi_ca_minus_one): New define_insn_and_split.
> gcc/testsuite/
> PR target/95737
> * gcc.target/powerpc/pr95737.c: New.
>
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 64049a6e521..483a93956f8 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -2353,6 +2353,19 @@ (define_insn "subf3_carry_in_xx"
>"subfe %0,%0,%0"
>[(set_attr "type" "add")])
>
> +(define_insn_and_split "*extenddi_ca_minus_one"
> + [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
> + (sign_extend:DI (plus:SI (reg:SI CA_REGNO)
> + (const_int -1]
> + ""
> + "#"
> + ""
> + [(parallel [(set (match_dup 0)
> +(plus:DI (reg:DI CA_REGNO)
> + (const_int -1)))
> + (clobber (reg:DI CA_REGNO))])]
> + ""
> +)
>
> (define_insn "@neg2"
>[(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr95737.c
> b/gcc/testsuite/gcc.target/powerpc/pr95737.c
> new file mode 100644
> index 000..d4d6a4198cf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c
> @@ -0,0 +1,10 @@
> +/* PR target/95737 */
> +/* { dg-do compile { target lp64 } } */> +/* { dg-options "-O2 -mno-isel" }
> */
Nit: It seems good to put one comment for the reason why we need this
special -mno-isel, like something: Power9 leverages isel for this case,
force -mno-isel to keep the test point valid on Power9 and later."
> +/* { dg-final { scan-assembler-not {\mextsw\M} } } */
> +
> +
> +unsigned long negativeLessThan (unsigned long a, unsigned long b)
> +{
> + return -(a < b);
> +}
OK with those nits fixed, but please wait for a few days, just in case
Segher/David have more comments.
BR,
Kewen
[PATCH] testsuite: opt: Fix const7.C for RISC-V.
Similar to patch 593993, RISC-V needs to limit symbols send in sdata.
Thanks for Palmer's help.
gcc/testsuite/ChangeLog:
* g++.dg/opt/const7.C: Don't use small data on RISC-V.
---
gcc/testsuite/g++.dg/opt/const7.C | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/g++.dg/opt/const7.C
b/gcc/testsuite/g++.dg/opt/const7.C
index 5bcf94897a8..50d902beae1 100644
--- a/gcc/testsuite/g++.dg/opt/const7.C
+++ b/gcc/testsuite/g++.dg/opt/const7.C
@@ -1,6 +1,7 @@
// PR c++/104142
// { dg-do compile { target c++11 } }
// { dg-additional-options -Wunused-variable }
+// { dg-options "-msmall-data-limit=0" { target { riscv*-*-* } } }
struct B { B()=default; };
static const B b_var; // { dg-bogus "" }
--
2.25.1
[PATCH v4, rs6000] Add a combine pattern for CA minus one [PR95737]
Hi,
This patch adds a combine pattern for "CA minus one". As CA only has two
values (0 or 1), we could convert following pattern
(sign_extend:DI (plus:SI (reg:SI 98 ca)
(const_int -1 [0x]
to
(plus:DI (reg:DI 98 ca)
(const_int -1 [0x])))
With this patch, one unnecessary sign extend is eliminated.
Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
Is this okay for trunk? Any recommendations? Thanks a lot.
ChangeLog
2022-05-13 Haochen Gui
gcc/
PR target/95737
* config/rs6000/rs6000.md (extenddi_ca_minus_one): Define.
gcc/testsuite/
PR target/95737
* gcc.target/powerpc/pr95737.c: New.
patch.diff
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 64049a6e521..483a93956f8 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -2353,6 +2353,19 @@ (define_insn "subf3_carry_in_xx"
"subfe %0,%0,%0"
[(set_attr "type" "add")])
+(define_insn_and_split "*extenddi_ca_minus_one"
+ [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
+ (sign_extend:DI (plus:SI (reg:SI CA_REGNO)
+(const_int -1]
+ ""
+ "#"
+ ""
+ [(parallel [(set (match_dup 0)
+ (plus:DI (reg:DI CA_REGNO)
+ (const_int -1)))
+ (clobber (reg:DI CA_REGNO))])]
+ ""
+)
(define_insn "@neg2"
[(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
diff --git a/gcc/testsuite/gcc.target/powerpc/pr95737.c
b/gcc/testsuite/gcc.target/powerpc/pr95737.c
new file mode 100644
index 000..d4d6a4198cf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c
@@ -0,0 +1,10 @@
+/* PR target/95737 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -mno-isel" } */
+/* { dg-final { scan-assembler-not {\mextsw\M} } } */
+
+
+unsigned long negativeLessThan (unsigned long a, unsigned long b)
+{
+ return -(a < b);
+}
Re: [PATCH v3 1/2] rs6000: Move g++.dg powerpc PR tests to g++.target
Hi
> On 12 May 2022, at 23:38, Segher Boessenkool
> wrote:
> On Mon, Apr 18, 2022 at 12:15:34PM -0500, Paul A. Clarke wrote:
>> -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
>> +/* Never tested on darwin, so skip there. */
>> +/* { dg-skip-if "" { *-*-darwin* } } */
>
> That is probably the reason for the skip, but it is a lousy reason, and
> not a good precedent to create. It is much better to let the Darwin
> maintainer deal with this *if* it fails. If you have proof it fails on
> Darwin, just say *that* (or even say *why* it fails!)
For the record, if there’s a specific patch you’d like tested on Darwin,
I am happy to try and fit it into the schedule (the machine is slow for
modern codebases, so don’t expect immediate answers).
Iain
Re: [PATCH v3 1/2] rs6000: Move g++.dg powerpc PR tests to g++.target
Hi!
On Mon, Apr 18, 2022 at 12:15:34PM -0500, Paul A. Clarke wrote:
> -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* Never tested on darwin, so skip there. */
> +/* { dg-skip-if "" { *-*-darwin* } } */
That is probably the reason for the skip, but it is a lousy reason, and
not a good precedent to create. It is much better to let the Darwin
maintainer deal with this *if* it fails. If you have proof it fails on
Darwin, just say *that* (or even say *why* it fails!)
Other than that this looks good.
Segher
Re: [PATCH v2] RISC-V: Implement C[LT]Z_DEFINED_VALUE_AT_ZERO
On Thu, 12 May 2022 11:33:34 PDT (-0700), philipp.toms...@vrull.eu wrote:
The Zbb support has introduced ctz and clz to the backend, but some
transformations in GCC need to know what the value of c[lt]z at zero
is. This affects how the optab is generated and may suppress use of
CLZ/CTZ in tree passes.
Among other things, this is needed for the transformation of
table-based ctz-implementations, such as in deepsjeng, to work
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90838).
Prior to this change, the test case from PR90838 would compile to
on RISC-V targets with Zbb:
myctz:
lui a4,%hi(.LC0)
ld a4,%lo(.LC0)(a4)
neg a5,a0
and a5,a5,a0
mul a5,a5,a4
lui a4,%hi(.LANCHOR0)
addia4,a4,%lo(.LANCHOR0)
srlia5,a5,58
sh2add a5,a5,a4
lw a0,0(a5)
ret
After this change, we get:
myctz:
ctz a0,a0
andia0,a0,63
ret
Testing this with deepsjeng_r (from SPEC 2017) against QEMU, this
shows a clear reduction in dynamic instruction count:
- before 1961888067076
- after 1907928279874 (2.75% reduction)
This also merges the various target-specific test-cases (for x86-64,
aarch64 and riscv) within gcc.dg/pr90838.c.
This extends the macros (i.e., effective-target keywords) used in
testing (lib/target-supports.exp) to reliably distinguish between RV32
and RV64 via __riscv_xlen (i.e., the integer register bitwidth) :
testing for ILP32 could be misleading (as ILP32 is a valid memory
model for 64bit systems).
gcc/ChangeLog:
* config/riscv/riscv.h (CLZ_DEFINED_VALUE_AT_ZERO): Implement.
(CTZ_DEFINED_VALUE_AT_ZERO): Same.
* doc/sourcebuild.texi: add documentation for RISC-V specific
test target keywords
gcc/testsuite/ChangeLog:
* gcc.dg/pr90838.c: Add additional flags (dg-additional-options)
when compiling for riscv64 and subsume gcc.target/aarch64/pr90838.c
and gcc.target/i386/pr95863-2.c.
* gcc.target/riscv/zbb-ctz.c: New test.
* gcc.target/aarch64/pr90838.c: Removed.
* gcc.target/i386/pr95863-2.c: Removed.
* lib/target-supports.exp: Recognize RV32 or RV64 via XLEN
Signed-off-by: Philipp Tomsich
Signed-off-by: Manolis Tsamis
Co-developed-by: Manolis Tsamis
---
Changes in v2:
- Address review comments
- Merge the different target-specific testcases for CLZ into one
- Add RV32 tests
- Fix pr90383.c testcase for x86_64
gcc/config/riscv/riscv.h | 5 ++
gcc/doc/sourcebuild.texi | 12
gcc/testsuite/gcc.dg/pr90838.c | 25 +
gcc/testsuite/gcc.target/aarch64/pr90838.c | 64 --
gcc/testsuite/gcc.target/i386/pr95863-2.c | 27 -
gcc/testsuite/lib/target-supports.exp | 30 ++
6 files changed, 72 insertions(+), 91 deletions(-)
delete mode 100644 gcc/testsuite/gcc.target/aarch64/pr90838.c
delete mode 100644 gcc/testsuite/gcc.target/i386/pr95863-2.c
Reviewed-by: Palmer Dabbelt
Acked-by: Palmer Dabbelt
For the RISC-V bits, though presumably we need a global reviewer to
handle the non-RISC-V stuff.
Thanks!
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 8a4d2cf7f85..b191606edb4 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -1004,4 +1004,9 @@ extern void riscv_remove_unneeded_save_restore_calls
(void);
#define HARD_REGNO_RENAME_OK(FROM, TO) riscv_hard_regno_rename_ok (FROM, TO)
+#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+ ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
+#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+ ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
+
#endif /* ! GCC_RISCV_H */
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 613ac29967b..71c04841df2 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2420,6 +2420,18 @@ PowerPC target pre-defines macro _ARCH_PWR9 which means
the @code{-mcpu}
setting is Power9 or later.
@end table
+@subsection RISC-V specific attributes
+
+@table @code
+
+@item rv32
+Test system has an integer register width of 32 bits.
+
+@item rv64
+Test system has an integer register width of 64 bits.
+
+@end table
+
@subsubsection Other hardware attributes
@c Please keep this table sorted alphabetically.
diff --git a/gcc/testsuite/gcc.dg/pr90838.c b/gcc/testsuite/gcc.dg/pr90838.c
index 41c5dab9a5c..ae8652f3c39 100644
--- a/gcc/testsuite/gcc.dg/pr90838.c
+++ b/gcc/testsuite/gcc.dg/pr90838.c
@@ -1,5 +1,8 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-forwprop2-details" } */
+/* { dg-additional-options "-mbmi" { target { { i?86-*-* x86_64-*-* } && { ! {
ia32 } } } } } */
+/* { dg-additional-options "-march=rv64gc_zbb" { target { lp64 && riscv64*-*-*
} } } */
+/* { dg-additional-options "-march=rv32gc_zbb" { target { ilp32 &&
riscv64*-*-* } } } */
int ctz1 (unsigned x)
{
@@ -56,4 +59,26 @@ int ctz4 (unsigned
Re: [PATCH v3 2/2] rs6000: Remove a few needless 'lp64' contraints.
Hi! On Mon, Apr 18, 2022 at 12:15:35PM -0500, Paul A. Clarke wrote: > A few tests need not be restricted to 'lp64', so remove the restriction. > > A few of those need a simple change to the DejaGnu directives to suppress > '-mcmodel' flags for '-m32'. Okay for trunk. Thanks! Segher
Re: [PING PATCH v3 0/2] rs6000: Move g++.dg powerpc tests to g++.target
ping
On Mon, Apr 18, 2022 at 12:15:33PM -0500, Paul A. Clarke via Gcc-patches wrote:
> v3: moved "not tested on Darwin" changes into 1/2, where they belong.
>
> v2:
> - v1 patches 1/3 and 2/3 have been merged after reviews / approval.
> - Previous 3/3 is now 1/2, and new 2/2 is per review from Segher...
>
> Some tests in g++.dg are target-specific for powerpc. Move those to
> g++.target/powerpc. Update the DejaGnu directives as needed, since
> the target restriction is perhaps no longer needed when residing in the
> target-specific powerpc subdirectory.
>
> In addition (new patch 2/2), as suggested by Segher, remove 'lp64' restriction
> for a handful of tests, protecting uses of '-mcmodel' flag with
> dg-additional-options.
>
> Tested on Linux/Power9 (BE) and Linux Power8 (LE 32 and 64), full "make
> check".
>
> OK for trunk?
>
> Paul A. Clarke (2):
> rs6000: Move g++.dg powerpc PR tests to g++.target
> rs6000: Remove a few needless 'lp64' contraints.
>
> gcc/testsuite/g++.dg/pr65240-1.C | 8
> gcc/testsuite/g++.dg/pr65240-2.C | 8
> gcc/testsuite/g++.dg/pr65240-3.C | 8
> gcc/testsuite/g++.target/powerpc/pr65240-1.C | 9 +
> gcc/testsuite/g++.target/powerpc/pr65240-2.C | 9 +
> gcc/testsuite/g++.target/powerpc/pr65240-3.C | 9 +
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-4.C | 5 +++--
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240.h | 0
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65242.C | 5 +++--
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr67211.C | 5 +++--
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr69667.C | 5 +++--
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr71294.C | 2 +-
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84264.C | 2 +-
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84279.C | 5 +++--
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr85657.C | 2 +-
> gcc/testsuite/{g++.dg => g++.target/powerpc}/pr93974.C | 0
> 16 files changed, 45 insertions(+), 37 deletions(-)
> delete mode 100644 gcc/testsuite/g++.dg/pr65240-1.C
> delete mode 100644 gcc/testsuite/g++.dg/pr65240-2.C
> delete mode 100644 gcc/testsuite/g++.dg/pr65240-3.C
> create mode 100644 gcc/testsuite/g++.target/powerpc/pr65240-1.C
> create mode 100644 gcc/testsuite/g++.target/powerpc/pr65240-2.C
> create mode 100644 gcc/testsuite/g++.target/powerpc/pr65240-3.C
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-4.C (68%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240.h (100%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65242.C (93%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr67211.C (91%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr69667.C (97%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr71294.C (96%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84264.C (79%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84279.C (89%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr85657.C (90%)
> rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr93974.C (100%)
>
> --
> 2.27.0
>
Re: [PATCH] libgo: Recognize off64_t / loff_t type definition of musl libc
On Thu, May 12, 2022 at 11:23 AM Sören Tempel via Gcc-patches wrote: > > The off64_t type is used for defining Offset_t: > > > https://github.com/golang/gofrontend/blob/4bdff733a0c2a9ddc3eff104b1be03df058a79c4/libgo/mksysinfo.sh#L406-L410 > > On musl, _HAVE_OFF64_T is defined since autoconf doesn't mind it > being defined as a macro but -fdump-go-spec does, hence you end up > with the following compilation error (even with your patch applied): Ah, thanks. > Apart from off64_t stuff, there is only one minor issue (see below). > > > index 7e2b98ba6..487099a33 100644 > > --- a/libgo/configure.ac > > +++ b/libgo/configure.ac > > @@ -579,7 +579,7 @@ AC_C_BIGENDIAN > > + > > +CFLAGS_hold="$CFLAGS" > > +CFLAGS="$OSCFLAGS $CFLAGS" > > AC_CHECK_TYPES([loff_t]) > > +CFLAGS="$CFLAGS_hold" > > The AC_CHECK_TYPES invocation is missing an include of fcntl.h (which > defines loff_t in musl) and as such fails and causes libgo compilation > to fail with "reference to undefined name '_libgo_loff_t_type'" as > HAVE_LOFF_T is not defined. The invocation needs to be changed to: > > AC_CHECK_TYPES([loff_t], [], [], [[#include ]]) > > and this needs to be adjusted accordingly in configure as well. Hmmm, I added fcntl.h to AC_CHECK_HEADERS. I thought that would be enough to cause it to be included in future tests. Perhaps not. Ian
Re: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
On Thu, 12 May 2022, Jason Merrill wrote:
> On 5/12/22 14:14, Patrick Palka wrote:
> > On Thu, 12 May 2022, Patrick Palka wrote:
> >
> > > After r13-332-g88459c3965e2a2, it looks like we can safely remove the
> > > NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which simplifies its
> > > semantics.
> > >
> > > And TMPL_ARGS_LEVEL should verify the level argument is sane in the
> > > one-dimensional vector case. This change uncovered a couple of latent
> > > bugs: in try_class_unification, we weren't correctly copying
> > > multidimensional targs, and in unify_pack_expansion it seems an
> > > inequality test needs to be reversed (not sure about this latter fix,
> > > the test was added in r8-6178-g2625472ffa519e FWIW).
> > >
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > trunk? Also tested on cmcstl2 and range-v3.
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
> > > (TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
> > > TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
> > > * pt.cc (try_class_unification): Correctly copy multidimensional
> > > targs.
> > > (unify_pack_expansion): Fix level comparison.
> > > ---
> > > gcc/cp/cp-tree.h | 5 +++--
> > > gcc/cp/pt.cc | 12 ++--
> > > 2 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > > index b6961a796af..f681d32ac93 100644
> > > --- a/gcc/cp/cp-tree.h
> > > +++ b/gcc/cp/cp-tree.h
> > > @@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
> > > /* Nonzero if the template arguments is actually a vector of vectors,
> > > rather than just a vector. */
> > > #define TMPL_ARGS_HAVE_MULTIPLE_LEVELS(NODE) \
> > > - (NODE && TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
> > > + (TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
> > > && TREE_CODE (TREE_VEC_ELT (NODE, 0)) == TREE_VEC)
> > > /* The depth of a template argument vector. When called directly by
> > > @@ -3783,7 +3783,8 @@ struct GTY(()) lang_decl {
> > > args is level 1, not level 0. */
> > > #define TMPL_ARGS_LEVEL(ARGS, LEVEL)\
> > > (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (ARGS)\
> > > - ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) : (ARGS))
> > > + ? TREE_VEC_ELT (ARGS, (LEVEL) - 1)\
> > > + : (gcc_checking_assert (LEVEL == 1), (ARGS)))
> > > /* Set the LEVELth level of the template ARGS to VAL. This macro does
> > > not work with single-level argument vectors. */
> > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > index 2c7c5f8bb5d..75b21e5c88a 100644
> > > --- a/gcc/cp/pt.cc
> > > +++ b/gcc/cp/pt.cc
> > > @@ -23398,8 +23398,6 @@ static tree
> > > try_class_unification (tree tparms, tree targs, tree parm, tree arg,
> > > bool explain_p)
> > > {
> > > - tree copy_of_targs;
> > > -
> > > if (!CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (arg))
> > > return NULL_TREE;
> > > else if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
> > > @@ -23438,17 +23436,19 @@ try_class_unification (tree tparms, tree targs,
> > > tree parm, tree arg,
> > >because there are two ways to unify base classes of S<0, 1, 2>
> > >with S. If we kept the already deduced knowledge, we
> > >would reject the possibility I=1. */
> > > - copy_of_targs = make_tree_vec (TREE_VEC_LENGTH (targs));
> > > + targs = copy_template_args (targs);
> > > + for (tree& targ : tree_vec_range (INNERMOST_TEMPLATE_ARGS (targs)))
> > > +targ = NULL_TREE;
> >
> > It occurred to me that we don't need to make a copy of (the TREE_VECs
> > for) the outer template arguments, so it might be better to use
> > copy_node manually instead of copy_template_args. But this redundant
> > copying shouldn't matter much if we make sure to ggc_free it afterwards.
> > So here's v2 which additionally makes try_class_unification ggc_free
> > this copy of targs (bootstrap and regtest in progress):
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
> >
> > After r13-332-g88459c3965e2a2, it looks like we can safely remove the
> > NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which should make its
> > uses easier to reason about.
>
> because we should have given up before trying to check that?
*nod* I was going to mention that in particular it makes
TMPL_ARGS_LEVEL (NULL, 1) invalid instead of yielding NULL, but on
second thought yielding NULL kind of makes sense -- in some other places
we already treat NULL args like an empty TREE_VEC, so it seems
reasonable that TMPL_ARGS_LEVEL acts like the identity function for both
NULL and an empty TREE_VEC. So I'm not too sure anymore that removing
the NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS is the way to go; I'm
going to remove this part of the patch, which is a non-functional change
anyway.
>
> > And TMPL_ARGS_LEVEL should verify the
[PATCH] gdc 9, 10 and 11 bug fix
Greetings.
No compiler has any business rejecting files for the sole crime of being
symlinked to. The following applies, modulo patch fuzz, to the 9, 10 and 11
series of compilers.
Given my use of shadow trees, this bug attempted to prevent me from building
12.1.0. The D-based gdc in 12.1.0 and up does not exhibit this quirky
behaviour.
Please Reply-To-All.
Thanks and have a great day.
Marc.
Signed-off-by: Marc Aurèle La France
diff -aNpRruz -X /etc/diff.excludes gcc-11.3.0/gcc/d/dmd/root/filename.c
devel-11.3.0/gcc/d/dmd/root/filename.c
--- gcc-11.3.0/gcc/d/dmd/root/filename.c2022-04-21 01:58:53.151586473
-0600
+++ devel-11.3.0/gcc/d/dmd/root/filename.c 2022-05-11 16:30:54.398488793
-0600
@@ -522,7 +522,7 @@ const char *FileName::safeSearchPath(Strings *path, const
char *name)
// "strncmp(cpath, cname, %i)=%i\n", exists(cname),
// strlen(cpath), strncmp(cpath, cname, strlen(cpath)));
// exists and name is *really* a "child" of path
-if (exists(cname) && strncmp(cpath, cname, strlen(cpath)) == 0)
+if (exists(cname))
{
::free(const_cast(cpath));
const char *p = mem.xstrdup(cname);
Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD
Hi Piotr, On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote: > FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. Declared, not defined. These are required to be real functions (on all platforms that have these functions), not macros or inlines or whatever. So what library *does* have these functions? It would be much neater to do the similar to the libm test but with that other library. Just libc, maybe? The answer to this will probably also answer Ke Wen's questions, or at least just point to a good and more general way of solving this :-) Maybe the AC_CHECK_LIB could just use GCC_CHECK_MATH_FUNC or such? You also need a changelog entry here. Something like 2022-05-12 Piotr Kubaj libgfortran/ * configure.ac (powerpc*-freebsd*): Define HAVE_FEENABLEEXCEPT if declares the feenableexcept function. * configure: Regenerate. > +case x$target in > + xpowerpc*-freebsd*) > +AC_CACHE_CHECK([for fenv.h and feenableexcept], have_feenableexcept, [ > + AC_COMPILE_IFELSE([AC_LANG_PROGRAM( > +[[ #include ]], > +[[ feenableexcept(FE_DIVBYZERO | FE_INVALID); ]])], > +[ have_feenableexcept="yes" ], > +[ have_feenableexcept="no" ])]) Please get rid of all surplus whitespace here. Imitate existing tests, if you need inspiration :-) Segher
Re: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
On 5/12/22 14:14, Patrick Palka wrote:
On Thu, 12 May 2022, Patrick Palka wrote:
After r13-332-g88459c3965e2a2, it looks like we can safely remove the
NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which simplifies its
semantics.
And TMPL_ARGS_LEVEL should verify the level argument is sane in the
one-dimensional vector case. This change uncovered a couple of latent
bugs: in try_class_unification, we weren't correctly copying
multidimensional targs, and in unify_pack_expansion it seems an
inequality test needs to be reversed (not sure about this latter fix,
the test was added in r8-6178-g2625472ffa519e FWIW).
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk? Also tested on cmcstl2 and range-v3.
gcc/cp/ChangeLog:
* cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
(TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
* pt.cc (try_class_unification): Correctly copy multidimensional
targs.
(unify_pack_expansion): Fix level comparison.
---
gcc/cp/cp-tree.h | 5 +++--
gcc/cp/pt.cc | 12 ++--
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b6961a796af..f681d32ac93 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
/* Nonzero if the template arguments is actually a vector of vectors,
rather than just a vector. */
#define TMPL_ARGS_HAVE_MULTIPLE_LEVELS(NODE) \
- (NODE && TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
+ (TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
&& TREE_CODE (TREE_VEC_ELT (NODE, 0)) == TREE_VEC)
/* The depth of a template argument vector. When called directly by
@@ -3783,7 +3783,8 @@ struct GTY(()) lang_decl {
args is level 1, not level 0. */
#define TMPL_ARGS_LEVEL(ARGS, LEVEL) \
(TMPL_ARGS_HAVE_MULTIPLE_LEVELS (ARGS) \
- ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) : (ARGS))
+ ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) \
+ : (gcc_checking_assert (LEVEL == 1), (ARGS)))
/* Set the LEVELth level of the template ARGS to VAL. This macro does
not work with single-level argument vectors. */
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2c7c5f8bb5d..75b21e5c88a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -23398,8 +23398,6 @@ static tree
try_class_unification (tree tparms, tree targs, tree parm, tree arg,
bool explain_p)
{
- tree copy_of_targs;
-
if (!CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (arg))
return NULL_TREE;
else if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
@@ -23438,17 +23436,19 @@ try_class_unification (tree tparms, tree targs, tree
parm, tree arg,
because there are two ways to unify base classes of S<0, 1, 2>
with S. If we kept the already deduced knowledge, we
would reject the possibility I=1. */
- copy_of_targs = make_tree_vec (TREE_VEC_LENGTH (targs));
+ targs = copy_template_args (targs);
+ for (tree& targ : tree_vec_range (INNERMOST_TEMPLATE_ARGS (targs)))
+targ = NULL_TREE;
It occurred to me that we don't need to make a copy of (the TREE_VECs
for) the outer template arguments, so it might be better to use
copy_node manually instead of copy_template_args. But this redundant
copying shouldn't matter much if we make sure to ggc_free it afterwards.
So here's v2 which additionally makes try_class_unification ggc_free
this copy of targs (bootstrap and regtest in progress):
-- >8 --
Subject: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
After r13-332-g88459c3965e2a2, it looks like we can safely remove the
NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which should make its
uses easier to reason about.
because we should have given up before trying to check that?
And TMPL_ARGS_LEVEL should verify the level argument is valid in the
one-dimensional vector case. This change uncovered a couple of latent
issues: in try_class_unification, we weren't correctly copying
multidimensional targs, and in unify_pack_expansion it seems an
inequality test needs to be reversed. This patch fixes both these
issues, and in passing makes the former function ggc_free the copy of
targs.
OK.
gcc/cp/ChangeLog:
* cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
(TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
* pt.cc (try_class_unification): Correctly copy multidimensional
targs. Free the copy of targs.
(unify_pack_expansion): Fix level comparison.
---
gcc/cp/cp-tree.h | 5 +++--
gcc/cp/pt.cc | 28 ++--
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b6961a796af..c28a3311dde 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
/*
[PATCH v2] RISC-V: Implement C[LT]Z_DEFINED_VALUE_AT_ZERO
The Zbb support has introduced ctz and clz to the backend, but some
transformations in GCC need to know what the value of c[lt]z at zero
is. This affects how the optab is generated and may suppress use of
CLZ/CTZ in tree passes.
Among other things, this is needed for the transformation of
table-based ctz-implementations, such as in deepsjeng, to work
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90838).
Prior to this change, the test case from PR90838 would compile to
on RISC-V targets with Zbb:
myctz:
lui a4,%hi(.LC0)
ld a4,%lo(.LC0)(a4)
neg a5,a0
and a5,a5,a0
mul a5,a5,a4
lui a4,%hi(.LANCHOR0)
addia4,a4,%lo(.LANCHOR0)
srlia5,a5,58
sh2add a5,a5,a4
lw a0,0(a5)
ret
After this change, we get:
myctz:
ctz a0,a0
andia0,a0,63
ret
Testing this with deepsjeng_r (from SPEC 2017) against QEMU, this
shows a clear reduction in dynamic instruction count:
- before 1961888067076
- after 1907928279874 (2.75% reduction)
This also merges the various target-specific test-cases (for x86-64,
aarch64 and riscv) within gcc.dg/pr90838.c.
This extends the macros (i.e., effective-target keywords) used in
testing (lib/target-supports.exp) to reliably distinguish between RV32
and RV64 via __riscv_xlen (i.e., the integer register bitwidth) :
testing for ILP32 could be misleading (as ILP32 is a valid memory
model for 64bit systems).
gcc/ChangeLog:
* config/riscv/riscv.h (CLZ_DEFINED_VALUE_AT_ZERO): Implement.
(CTZ_DEFINED_VALUE_AT_ZERO): Same.
* doc/sourcebuild.texi: add documentation for RISC-V specific
test target keywords
gcc/testsuite/ChangeLog:
* gcc.dg/pr90838.c: Add additional flags (dg-additional-options)
when compiling for riscv64 and subsume gcc.target/aarch64/pr90838.c
and gcc.target/i386/pr95863-2.c.
* gcc.target/riscv/zbb-ctz.c: New test.
* gcc.target/aarch64/pr90838.c: Removed.
* gcc.target/i386/pr95863-2.c: Removed.
* lib/target-supports.exp: Recognize RV32 or RV64 via XLEN
Signed-off-by: Philipp Tomsich
Signed-off-by: Manolis Tsamis
Co-developed-by: Manolis Tsamis
---
Changes in v2:
- Address review comments
- Merge the different target-specific testcases for CLZ into one
- Add RV32 tests
- Fix pr90383.c testcase for x86_64
gcc/config/riscv/riscv.h | 5 ++
gcc/doc/sourcebuild.texi | 12
gcc/testsuite/gcc.dg/pr90838.c | 25 +
gcc/testsuite/gcc.target/aarch64/pr90838.c | 64 --
gcc/testsuite/gcc.target/i386/pr95863-2.c | 27 -
gcc/testsuite/lib/target-supports.exp | 30 ++
6 files changed, 72 insertions(+), 91 deletions(-)
delete mode 100644 gcc/testsuite/gcc.target/aarch64/pr90838.c
delete mode 100644 gcc/testsuite/gcc.target/i386/pr95863-2.c
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 8a4d2cf7f85..b191606edb4 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -1004,4 +1004,9 @@ extern void riscv_remove_unneeded_save_restore_calls
(void);
#define HARD_REGNO_RENAME_OK(FROM, TO) riscv_hard_regno_rename_ok (FROM, TO)
+#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+ ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
+#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+ ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
+
#endif /* ! GCC_RISCV_H */
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 613ac29967b..71c04841df2 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2420,6 +2420,18 @@ PowerPC target pre-defines macro _ARCH_PWR9 which means
the @code{-mcpu}
setting is Power9 or later.
@end table
+@subsection RISC-V specific attributes
+
+@table @code
+
+@item rv32
+Test system has an integer register width of 32 bits.
+
+@item rv64
+Test system has an integer register width of 64 bits.
+
+@end table
+
@subsubsection Other hardware attributes
@c Please keep this table sorted alphabetically.
diff --git a/gcc/testsuite/gcc.dg/pr90838.c b/gcc/testsuite/gcc.dg/pr90838.c
index 41c5dab9a5c..ae8652f3c39 100644
--- a/gcc/testsuite/gcc.dg/pr90838.c
+++ b/gcc/testsuite/gcc.dg/pr90838.c
@@ -1,5 +1,8 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-forwprop2-details" } */
+/* { dg-additional-options "-mbmi" { target { { i?86-*-* x86_64-*-* } && { ! {
ia32 } } } } } */
+/* { dg-additional-options "-march=rv64gc_zbb" { target { lp64 && riscv64*-*-*
} } } */
+/* { dg-additional-options "-march=rv32gc_zbb" { target { ilp32 &&
riscv64*-*-* } } } */
int ctz1 (unsigned x)
{
@@ -56,4 +59,26 @@ int ctz4 (unsigned long x)
return table[(lsb * magic) >> 58];
}
+/* { dg-final { scan-tree-dump-times {= \.CTZ} 4 "forwprop2" { target { {
i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } } */
+/* { dg-final { scan-assembler-times "tzcntq\t" 1 { target
[committed] libstdc++: Remove whitespace before preprocessor directives
Tested x86_64-linux, pushed to trunk.
-- >8 --
These are harmless, but also unnecessary and inconsistent (and their
removal was requested by PR libstdc++/17632).
libstdc++-v3/ChangeLog:
* config/locale/dragonfly/numeric_members.cc: Remove whitespace.
* config/locale/gnu/numeric_members.cc: Likewise.
* include/bits/locale_facets_nonio.h: Likewise.
* libsupc++/typeinfo: Likewise.
---
.../config/locale/dragonfly/numeric_members.cc | 2 +-
libstdc++-v3/config/locale/gnu/numeric_members.cc | 2 +-
libstdc++-v3/include/bits/locale_facets_nonio.h| 4 ++--
libstdc++-v3/libsupc++/typeinfo| 10 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/libstdc++-v3/config/locale/dragonfly/numeric_members.cc
b/libstdc++-v3/config/locale/dragonfly/numeric_members.cc
index 51a69dc60e1..b3cf4add78e 100644
--- a/libstdc++-v3/config/locale/dragonfly/numeric_members.cc
+++ b/libstdc++-v3/config/locale/dragonfly/numeric_members.cc
@@ -229,7 +229,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
delete [] _M_data->_M_grouping;
delete _M_data;
}
- #endif
+#endif
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
diff --git a/libstdc++-v3/config/locale/gnu/numeric_members.cc
b/libstdc++-v3/config/locale/gnu/numeric_members.cc
index 591573f3057..c714d6a544f 100644
--- a/libstdc++-v3/config/locale/gnu/numeric_members.cc
+++ b/libstdc++-v3/config/locale/gnu/numeric_members.cc
@@ -267,7 +267,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
delete [] _M_data->_M_grouping;
delete _M_data;
}
- #endif
+#endif
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
diff --git a/libstdc++-v3/include/bits/locale_facets_nonio.h
b/libstdc++-v3/include/bits/locale_facets_nonio.h
index 71a82af5ee3..75aef5b4d69 100644
--- a/libstdc++-v3/include/bits/locale_facets_nonio.h
+++ b/libstdc++-v3/include/bits/locale_facets_nonio.h
@@ -348,8 +348,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
- // Include host and configuration specific timepunct functions.
- #include
+// Include host and configuration specific timepunct functions.
+#include
namespace std _GLIBCXX_VISIBILITY(default)
{
diff --git a/libstdc++-v3/libsupc++/typeinfo b/libstdc++-v3/libsupc++/typeinfo
index 3018a510fd5..376e82ba614 100644
--- a/libstdc++-v3/libsupc++/typeinfo
+++ b/libstdc++-v3/libsupc++/typeinfo
@@ -74,11 +74,11 @@ namespace __cxxabiv1
// By default follow the old inline rules to avoid ABI changes.
#ifndef __GXX_TYPEINFO_EQUALITY_INLINE
- #if !__GXX_WEAK__
-#define __GXX_TYPEINFO_EQUALITY_INLINE 0
- #else
-#define __GXX_TYPEINFO_EQUALITY_INLINE 1
- #endif
+# if !__GXX_WEAK__
+#define __GXX_TYPEINFO_EQUALITY_INLINE 0
+# else
+#define __GXX_TYPEINFO_EQUALITY_INLINE 1
+# endif
#endif
namespace std
--
2.34.1
Re: [PATCH] libgo: Recognize off64_t / loff_t type definition of musl libc
Hi Ian, Thanks for following up on this, comments below. Ian Lance Taylor wrote: > I see uses of loff_t but I don't see any uses of off64_t. The off64_t type is used for defining Offset_t: https://github.com/golang/gofrontend/blob/4bdff733a0c2a9ddc3eff104b1be03df058a79c4/libgo/mksysinfo.sh#L406-L410 On musl, _HAVE_OFF64_T is defined since autoconf doesn't mind it being defined as a macro but -fdump-go-spec does, hence you end up with the following compilation error (even with your patch applied): sysinfo.go:7896:15: error: use of undefined type '_off64_t' 7896 | type Offset_t _off64_t > We don't have to treat loff_t differently based on whether it is a > macro, we can just use a different name. Sounds good to me. > Does this patch work for you? Apart from off64_t stuff, there is only one minor issue (see below). > index 7e2b98ba6..487099a33 100644 > --- a/libgo/configure.ac > +++ b/libgo/configure.ac > @@ -579,7 +579,7 @@ AC_C_BIGENDIAN > + > +CFLAGS_hold="$CFLAGS" > +CFLAGS="$OSCFLAGS $CFLAGS" > AC_CHECK_TYPES([loff_t]) > +CFLAGS="$CFLAGS_hold" The AC_CHECK_TYPES invocation is missing an include of fcntl.h (which defines loff_t in musl) and as such fails and causes libgo compilation to fail with "reference to undefined name '_libgo_loff_t_type'" as HAVE_LOFF_T is not defined. The invocation needs to be changed to: AC_CHECK_TYPES([loff_t], [], [], [[#include ]]) and this needs to be adjusted accordingly in configure as well. Sincerely, Sören
Re: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
On Thu, 12 May 2022, Patrick Palka wrote:
> After r13-332-g88459c3965e2a2, it looks like we can safely remove the
> NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which simplifies its
> semantics.
>
> And TMPL_ARGS_LEVEL should verify the level argument is sane in the
> one-dimensional vector case. This change uncovered a couple of latent
> bugs: in try_class_unification, we weren't correctly copying
> multidimensional targs, and in unify_pack_expansion it seems an
> inequality test needs to be reversed (not sure about this latter fix,
> the test was added in r8-6178-g2625472ffa519e FWIW).
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk? Also tested on cmcstl2 and range-v3.
>
> gcc/cp/ChangeLog:
>
> * cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
> (TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
> TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
> * pt.cc (try_class_unification): Correctly copy multidimensional
> targs.
> (unify_pack_expansion): Fix level comparison.
> ---
> gcc/cp/cp-tree.h | 5 +++--
> gcc/cp/pt.cc | 12 ++--
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index b6961a796af..f681d32ac93 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
> /* Nonzero if the template arguments is actually a vector of vectors,
> rather than just a vector. */
> #define TMPL_ARGS_HAVE_MULTIPLE_LEVELS(NODE) \
> - (NODE && TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
> + (TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
> && TREE_CODE (TREE_VEC_ELT (NODE, 0)) == TREE_VEC)
>
> /* The depth of a template argument vector. When called directly by
> @@ -3783,7 +3783,8 @@ struct GTY(()) lang_decl {
> args is level 1, not level 0. */
> #define TMPL_ARGS_LEVEL(ARGS, LEVEL) \
>(TMPL_ARGS_HAVE_MULTIPLE_LEVELS (ARGS) \
> - ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) : (ARGS))
> + ? TREE_VEC_ELT (ARGS, (LEVEL) - 1)\
> + : (gcc_checking_assert (LEVEL == 1), (ARGS)))
>
> /* Set the LEVELth level of the template ARGS to VAL. This macro does
> not work with single-level argument vectors. */
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 2c7c5f8bb5d..75b21e5c88a 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -23398,8 +23398,6 @@ static tree
> try_class_unification (tree tparms, tree targs, tree parm, tree arg,
> bool explain_p)
> {
> - tree copy_of_targs;
> -
>if (!CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (arg))
> return NULL_TREE;
>else if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
> @@ -23438,17 +23436,19 @@ try_class_unification (tree tparms, tree targs,
> tree parm, tree arg,
> because there are two ways to unify base classes of S<0, 1, 2>
> with S. If we kept the already deduced knowledge, we
> would reject the possibility I=1. */
> - copy_of_targs = make_tree_vec (TREE_VEC_LENGTH (targs));
> + targs = copy_template_args (targs);
> + for (tree& targ : tree_vec_range (INNERMOST_TEMPLATE_ARGS (targs)))
> +targ = NULL_TREE;
It occurred to me that we don't need to make a copy of (the TREE_VECs
for) the outer template arguments, so it might be better to use
copy_node manually instead of copy_template_args. But this redundant
copying shouldn't matter much if we make sure to ggc_free it afterwards.
So here's v2 which additionally makes try_class_unification ggc_free
this copy of targs (bootstrap and regtest in progress):
-- >8 --
Subject: [PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
After r13-332-g88459c3965e2a2, it looks like we can safely remove the
NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which should make its
uses easier to reason about.
And TMPL_ARGS_LEVEL should verify the level argument is valid in the
one-dimensional vector case. This change uncovered a couple of latent
issues: in try_class_unification, we weren't correctly copying
multidimensional targs, and in unify_pack_expansion it seems an
inequality test needs to be reversed. This patch fixes both these
issues, and in passing makes the former function ggc_free the copy of
targs.
gcc/cp/ChangeLog:
* cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
(TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
* pt.cc (try_class_unification): Correctly copy multidimensional
targs. Free the copy of targs.
(unify_pack_expansion): Fix level comparison.
---
gcc/cp/cp-tree.h | 5 +++--
gcc/cp/pt.cc | 28 ++--
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b6961a796af..c28a3311dde 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
/* Nonzero
Re: [PATCH] AArch64: Improve address rematerialization costs
Wilco Dijkstra writes: > Hi Richard, > >> But even if the costs are too high, the patch seems to be overcompensating. >> It doesn't make logical sense for an ADRP+LDR to be cheaper than an LDR. > > An LDR is not a replacement for ADRP+LDR, you need a store in addition the > original ADRP+LDR. Basically a simple spill would be comparing these 2 > sequences: > > ADRP x0, ... > LDR x0, [x0, ...] > STR x0, [SP, ...] > ... > LDR x0, [SP, ...] > > > ADRP x0, ... > LDR x0, [x0, ...] > ... > ADRP x0, ... > LDR x0, [x0, ...] > > Obviously it's far cheaper to do the latter than the former. Sure. Like I say, I'm not disagreeing with the intent of reducing spilling and promoting rematerialisation. I agree we should do that. I'm just disagreeing with the approach of using rtx_costs. The rtx_cost hook isn't being asked the question: is spilling this better value than rematerialising it? It's being asked for the cost of an operation, on the understanding that that cost will be compared with the cost of other operations. An ADRP+LDR operation then ought to be at least as costly as an LDR, because in a two-way comparison, it is. […] >> Maybe it would help to turn the question around for a minute. Can we >> describe the cases in which it's *better* for the RA to spill a constant >> address to the stack and reload it, rather than rematerialise on demand? > > Rematerialization is almost always better than spilling and reloading from the > stack. If the constant requires multiple instructions and there are more than > 2 > references it would be better for codesize to spill, but for performance it is > better to rematerialize unless there are many references. > > You also want to prefer rematerialization over spilling a different liferange > when > other aspects are comparable. Yeah, that's what I thought the answer would be. So the question is: why is the RA choosing to spill and reload rather than rematerialise these values? Does it not know how to rematerialise them, and so we rely on earlier passes not reusing the constants? Or does the RA know how but decides it isn't worthwhile, because of the way that the RA uses the target costs? If the latter, I would be much happier with a new hook that allows the target to force the RA to rematerialise a given value, if that's the heuristic we want to use when optimising for speed. Thanks, Richard
Re: [PATCH] x86: Document -mno-cet-switch
On Thu, May 12, 2022 at 12:15 AM Richard Biener
wrote:
>
> On Wed, May 11, 2022 at 9:03 PM Florian Weimer via Gcc-patches
> wrote:
> >
> > * H. J. Lu:
> >
> > > On Wed, May 11, 2022 at 11:45 AM Florian Weimer
> > > wrote:
> > >>
> > >> * H. J. Lu:
> > >>
> > >> >> NOTRACK avoids the need for ENDBR instructions, right? That's a
> > >> >> hardening improvement, so it should be used by default.
> > >> >
> > >> > NOTRACK weakens IBT since it disables IBT on the indirect jump
> > >> > instruction.
> > >> > GCC uses it in the jump table to avoid ENDBR.
> > >>
> > >> Typical jump table code looks like this:
> > >>
> > >> Dump of assembler code for function __cache_sysconf:
> > >>0x000f7a80 <+0>: endbr64
> > >>0x000f7a84 <+4>: sub$0xb9,%edi
> > >>0x000f7a8a <+10>:cmp$0xc,%edi
> > >>0x000f7a8d <+13>:ja 0xf7b70 <__cache_sysconf+240>
> > >>0x000f7a93 <+19>:lea0xba926(%rip),%rdx#
> > >> 0x1b23c0
> > >>0x000f7a9a <+26>:movslq (%rdx,%rdi,4),%rax
> > >>0x000f7a9e <+30>:add%rdx,%rax
> > >>0x000f7aa1 <+33>:notrack jmp *%rax
> > >>
> > >> There's no ENDBR instruction between range check, the address
> > >> computation, and the NOTRACK JMP, so it's not possible to redirect that
> > >> JMP to some other place.
> > >
> > > That is the assumption we made. RAX will always point to the valid
> > > address.
> >
> > Which means that NOTRACK should not weaken anything here. What am I
> > missing?
>
> Probably nothing. Still if the code computing the address to jump to
> is exploitable
> it might be useful. Like considering
>
> void foo (int *idx) { switch (*idx) { ...; default:
> __builtin_unreachable (); } }
>
> then GCC would elide the range check for the jump table lookup.
>
> I think the main reason for the NOTRACK is code size and the requirement to
> place extra ENDBR at each jump target (where any ENDBR is a possible
> valid target to use as gadget from a tracked brach).
>
> So NOTRACK branches are bad and ENDBR are both "bad"?
No, CET won't help here. __builtin_unreachable indicates truly unreachable
for any inputs. If not, __builtin_trap should be used instead.
> Richard.
>
> >
> > Thanks,
> > Florian
> >
--
H.J.
Re: [PATCH] AArch64: Improve address rematerialization costs
Hi Richard, > But even if the costs are too high, the patch seems to be overcompensating. > It doesn't make logical sense for an ADRP+LDR to be cheaper than an LDR. An LDR is not a replacement for ADRP+LDR, you need a store in addition the original ADRP+LDR. Basically a simple spill would be comparing these 2 sequences: ADRP x0, ... LDR x0, [x0, ...] STR x0, [SP, ...] ... LDR x0, [SP, ...] ADRP x0, ... LDR x0, [x0, ...] ... ADRP x0, ... LDR x0, [x0, ...] Obviously it's far cheaper to do the latter than the former. > Giving X zero cost means that a sequence like: > > (set (reg x0) X) > (set (reg x1) X) > > should stay as-is rather than be changed to: > > (set (reg x0) X) > (set (reg x1) (reg x0)) > > I don't think we want that for multi-instruction constants when > optimising for size. I don't believe this is a real problem. The cost queries for address constants come from register allocation, I don't see them affect other optimizations. > Yeah, I wasn't suggesting that we increase the spill costs. I'm saying I'm saying that because we've set the spill costs low on purpose to work around register allocation bugs. There have been some fixes since, so increasing the spill costs may now be feasible (but not trivial). > that we should look at whether the target-independent RA heuristics need > to change, whether new target hooks are needed, etc. We shouldn't go > into this with the assumption that the target-independent code is > invariant and that any fix must be in existing aarch64 hooks (rtx costs > or spill costs). But what bug do you think exists in target independent code? It behaves correctly once we supply more accurate costs. If there was no rematerialization irrespectively of the cost settings then you could claim there was a bug. > Maybe it would help to turn the question around for a minute. Can we > describe the cases in which it's *better* for the RA to spill a constant > address to the stack and reload it, rather than rematerialise on demand? Rematerialization is almost always better than spilling and reloading from the stack. If the constant requires multiple instructions and there are more than 2 references it would be better for codesize to spill, but for performance it is better to rematerialize unless there are many references. You also want to prefer rematerialization over spilling a different liferange when other aspects are comparable. Cheers, Wilco
Re: [PATCH] PR tree-optimization/83907: Improved memset handling in strlen pass.
On 2/20/2022 9:51 AM, Roger Sayle wrote: This patch implements the missed optimization enhancement PR 83907, by handling memset with a constant byte value in tree-ssa's strlen optimization pass. Effectively, this treats memset(dst,'x',3) as it would memcpy(dst,"xxx",3). This patch also includes a tweak to handle_store to address another missed optimization observed in the related test case pr83907-2.c. The consecutive byte stores to memory get coalesced into a vector write of a vector const, but unfortunately tree-ssa-strlen's handle_store didn't previously handle the (unusual) case where the stored "string" starts with a zero byte but also contains non-zero bytes. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check with no new failures. Ok for mainline? 2022-02-20 Roger Sayle gcc/ChangeLog PR tree-optimization/83907 * tree-ssa-strlen.cc (handle_builtin_memset): Record a strinfo for memset with an constant char value. (handle_store): Improved handling of stores with a first byte of zero, but not storing_all_zeros_p. gcc/testsuite/ChangeLog PR tree-optimization/83907 * gcc.dg/tree-ssa/pr83907-1.c: New test case. * gcc.dg/tree-ssa/pr83907-2.c: New test case. OK after a re-test. Thanks for the ping. jeff
Re: [PATCH] [12/11/10] Fix invalid format warnings on Windows
On Wed, 11 May 2022, Joseph Myers wrote: I'd also like to check that "if mingw-w64 is configured to target UCRT" is not something that is necessarily known when GCC is built or from the command-line options passed to GCC. Because ideally one might expect the TARGET_OVERRIDES_FORMAT_ATTRIBUTES / TARGET_OVERRIDES_FORMAT_INIT definitions to handle things appropriately conditionally, so that printf attributes are handled as gnu_printf for the "if mingw-w64 is configured to target UCRT" case. Disregarding a built-in format attribute when one is also specified explicitly in the header, even though the two are not exactly equivalent attributes, as in this patch, seems more like the right approach in the case where the attributes in installed header (at the time GCC is run, not the time it is built) *are* the way in which GCC gets the "configured to target UCRT" information - as opposed to it being something available before the header is parsed. Indeed, the "configured to target X" information isn't available at the point when GCC is built - that gets set afterwards. And even while it is usually mostly fixed soon afterwards, you can even (with some amount of gotchas) change what CRT to build for by passing the appropriate defines that picks a different default in the mingw-w64 headers. Anyway - from the GCC point of view, it's not fixed, and whatever the parsed headers say, is the only thing that can be relied upon. So I think the approach of the patch is the right one, code style/issues notwithstanding. // Martin
Re: [PATCH] AArch64: Cleanup CPU option processing code
Wilco Dijkstra writes:
> Hi Richard,
>
>> Looks like you might have attached the old patch. The aarch64_option_restore
>> change is mentioned in the changelog but doesn't appear in the patch itself.
>
> Indeed, not sure how that happened. Here is the correct v2 anyway.
>
> Wilco
>
>
> The --with-cpu/--with-arch configure option processing not only checks valid
> arguments
> but also sets TARGET_CPU_DEFAULT with a CPU and extension bitmask. This
> isn't used
> however since a --with-cpu is translated into a -mcpu option which is
> processed as if
> written on the command-line (so TARGET_CPU_DEFAULT is never accessed).
>
> So remove all the complex processing and bitmask, and just validate the
> option.
> Fix a bug that always reports valid architecture extensions as invalid. As a
> result
> the CPU processing in aarch64.c can be simplified.
>
> Bootstrap OK, regress pass, OK for commit?
>
> ChangeLog:
> 2022-04-19 Wilco Dijkstra
>
> * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
> processing. Add support for architectural extensions.
> * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
> AARCH64_CPU_DEFAULT_FLAGS.
> (TARGET_CPU_NBITS): Remove.
> (TARGET_CPU_MASK): Remove.
> * config/aarch64/aarch64.cc (AARCH64_CPU_DEFAULT_FLAGS): Remove
> define.
> (get_tune_cpu): Assert CPU is always valid.
> (get_arch): Assert architecture is always valid.
> (aarch64_override_options): Cleanup CPU selection code and simplify
> logic.
> (aarch64_option_restore): Remove unnecessary checks on tune.
OK, thanks.
Richard
> ---
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index
> c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b
> 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4178,8 +4178,6 @@ case "${target}" in
> pattern=AARCH64_CORE
> fi
>
> - ext_mask=AARCH64_CPU_DEFAULT_FLAGS
> -
> # Find the base CPU or ARCH id in aarch64-cores.def or
> # aarch64-arches.def
> if [ x"$base_val" = x ] \
> @@ -4187,23 +4185,6 @@ case "${target}" in
> ${srcdir}/config/aarch64/$def \
> > /dev/null; then
>
> - if [ $which = arch ]; then
> - base_id=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/^[^,]*,[]*//' | \
> - sed -e 's/,.*$//'`
> - # Extract the architecture flags from
> aarch64-arches.def
> - ext_mask=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/)$//' | \
> - sed -e 's/^.*,//'`
> - else
> - base_id=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/^[^,]*,[]*//' | \
> - sed -e 's/,.*$//'`
> - fi
> -
> # Disallow extensions in --with-tune=cortex-a53+crc.
> if [ $which = tune ] && [ x"$ext_val" != x ]; then
> echo "Architecture extensions not supported in
> --with-$which=$val" 1>&2
> @@ -4234,25 +4215,7 @@ case "${target}" in
> grep "^\"$base_ext\""`
>
> if [ x"$base_ext" = x ] \
> - || [[ -n $opt_line ]]; then
> -
> - # These regexp extract the elements based on
> - # their group match index in the regexp.
> - ext_canon=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\2/"`
> - ext_on=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\3/"`
> - ext_off=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\4/"`
> -
> - if [ $ext = $base_ext ]; then
> - # Adding extension
> - ext_mask="("$ext_mask") | ("$ext_on"
> | "$ext_canon")"
> - else
> - # Removing extension
> - ext_mask="("$ext_mask") &
> ~("$ext_off" | "$ext_canon")"
> - fi
> -
> +
Re: [PATCH] AArch64: Improve address rematerialization costs
Hi, >> It's also said that chosen alternatives might be the reason that >> rematerialization >> is not choosen and alternatives are chosen based on reload heuristics, not >> based >> on actual costs. > > Thanks for the pointer. Yeah, it'd be interesting to know if this > is the same issue, although I fear the only way of knowing for sure > is to fix it first and see whether both targets benefit. ;-) I don't believe this is the same issue - there are lots of register allocation problems indeed, many are caused by the complex design. All the alternatives and register classes create a huge crossproduct, making it almost impossible to make good allocation decisions even if they were accurately costed. I've found that the correct way to deal with this is to reduce all this choice as much as possible. That means splitting instructions into simpler ones with fewer alternatives and register classes. You also need to block it from treating all register classes as equivalent - on AArch64 we had to force floating point values to be allocated to floating point registers (which is obviously how any register allocator should work by default), but maybe x86 doesn't do that yet. Cheers, Wilco
Re: [PATCH] AArch64: Cleanup CPU option processing code
Hi Richard,
> Looks like you might have attached the old patch. The aarch64_option_restore
> change is mentioned in the changelog but doesn't appear in the patch itself.
Indeed, not sure how that happened. Here is the correct v2 anyway.
Wilco
The --with-cpu/--with-arch configure option processing not only checks valid
arguments
but also sets TARGET_CPU_DEFAULT with a CPU and extension bitmask. This isn't
used
however since a --with-cpu is translated into a -mcpu option which is processed
as if
written on the command-line (so TARGET_CPU_DEFAULT is never accessed).
So remove all the complex processing and bitmask, and just validate the option.
Fix a bug that always reports valid architecture extensions as invalid. As a
result
the CPU processing in aarch64.c can be simplified.
Bootstrap OK, regress pass, OK for commit?
ChangeLog:
2022-04-19 Wilco Dijkstra
* config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
processing. Add support for architectural extensions.
* config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
AARCH64_CPU_DEFAULT_FLAGS.
(TARGET_CPU_NBITS): Remove.
(TARGET_CPU_MASK): Remove.
* config/aarch64/aarch64.cc (AARCH64_CPU_DEFAULT_FLAGS): Remove define.
(get_tune_cpu): Assert CPU is always valid.
(get_arch): Assert architecture is always valid.
(aarch64_override_options): Cleanup CPU selection code and simplify
logic.
(aarch64_option_restore): Remove unnecessary checks on tune.
---
diff --git a/gcc/config.gcc b/gcc/config.gcc
index
c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b
100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4178,8 +4178,6 @@ case "${target}" in
pattern=AARCH64_CORE
fi
- ext_mask=AARCH64_CPU_DEFAULT_FLAGS
-
# Find the base CPU or ARCH id in aarch64-cores.def or
# aarch64-arches.def
if [ x"$base_val" = x ] \
@@ -4187,23 +4185,6 @@ case "${target}" in
${srcdir}/config/aarch64/$def \
> /dev/null; then
- if [ $which = arch ]; then
- base_id=`grep "^$pattern(\"$base_val\"," \
- ${srcdir}/config/aarch64/$def | \
- sed -e 's/^[^,]*,[]*//' | \
- sed -e 's/,.*$//'`
- # Extract the architecture flags from
aarch64-arches.def
- ext_mask=`grep "^$pattern(\"$base_val\"," \
- ${srcdir}/config/aarch64/$def | \
- sed -e 's/)$//' | \
- sed -e 's/^.*,//'`
- else
- base_id=`grep "^$pattern(\"$base_val\"," \
- ${srcdir}/config/aarch64/$def | \
- sed -e 's/^[^,]*,[]*//' | \
- sed -e 's/,.*$//'`
- fi
-
# Disallow extensions in --with-tune=cortex-a53+crc.
if [ $which = tune ] && [ x"$ext_val" != x ]; then
echo "Architecture extensions not supported in
--with-$which=$val" 1>&2
@@ -4234,25 +4215,7 @@ case "${target}" in
grep "^\"$base_ext\""`
if [ x"$base_ext" = x ] \
- || [[ -n $opt_line ]]; then
-
- # These regexp extract the elements based on
- # their group match index in the regexp.
- ext_canon=`echo -e "$opt_line" | \
- sed -e "s/$sed_patt/\2/"`
- ext_on=`echo -e "$opt_line" | \
- sed -e "s/$sed_patt/\3/"`
- ext_off=`echo -e "$opt_line" | \
- sed -e "s/$sed_patt/\4/"`
-
- if [ $ext = $base_ext ]; then
- # Adding extension
- ext_mask="("$ext_mask") | ("$ext_on" |
"$ext_canon")"
- else
- # Removing extension
- ext_mask="("$ext_mask") & ~("$ext_off"
| "$ext_canon")"
- fi
-
+ || [ x"$opt_line" != x ]; then
true
else
echo "Unknown extension used in
--with-$which=$val" 1>&2
@@ -4261,10 +4224,6 @@ case "${target}"
Re: c++: Support module language-decl semantics
On Thu, 12 May 2022 at 12:18, Nathan Sidwell wrote: > > In modules purview, one can attach a declaration to the global module > (i.e. not the named module in whence the declaration appears), using a > language declaration: > >export module Foo; >extern "C++" void *operator new (std::size_t); > > This implements those semantics. > > Jonathan, I guess this might help with modularizing std? Nice, thanks. I'm going to give the 'std' module another try soon.
[PATCH] c++: improve a couple of TMPL_ARGS_* accessor macros
After r13-332-g88459c3965e2a2, it looks like we can safely remove the
NULL test from TMPL_ARGS_HAVE_MULTIPLE_LEVELS, which simplifies its
semantics.
And TMPL_ARGS_LEVEL should verify the level argument is sane in the
one-dimensional vector case. This change uncovered a couple of latent
bugs: in try_class_unification, we weren't correctly copying
multidimensional targs, and in unify_pack_expansion it seems an
inequality test needs to be reversed (not sure about this latter fix,
the test was added in r8-6178-g2625472ffa519e FWIW).
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk? Also tested on cmcstl2 and range-v3.
gcc/cp/ChangeLog:
* cp-tree.h (TMPL_ARGS_HAVE_MULTIPLE_LEVELS): Remove NULL test.
(TMPL_ARGS_LEVEL): Assert LEVEL is 1 when
TMPL_ARGS_HAVE_MULTIPLE_LEVELS is false.
* pt.cc (try_class_unification): Correctly copy multidimensional
targs.
(unify_pack_expansion): Fix level comparison.
---
gcc/cp/cp-tree.h | 5 +++--
gcc/cp/pt.cc | 12 ++--
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b6961a796af..f681d32ac93 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3766,7 +3766,7 @@ struct GTY(()) lang_decl {
/* Nonzero if the template arguments is actually a vector of vectors,
rather than just a vector. */
#define TMPL_ARGS_HAVE_MULTIPLE_LEVELS(NODE)\
- (NODE && TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
+ (TREE_VEC_LENGTH (NODE) && TREE_VEC_ELT (NODE, 0) \
&& TREE_CODE (TREE_VEC_ELT (NODE, 0)) == TREE_VEC)
/* The depth of a template argument vector. When called directly by
@@ -3783,7 +3783,8 @@ struct GTY(()) lang_decl {
args is level 1, not level 0. */
#define TMPL_ARGS_LEVEL(ARGS, LEVEL) \
(TMPL_ARGS_HAVE_MULTIPLE_LEVELS (ARGS) \
- ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) : (ARGS))
+ ? TREE_VEC_ELT (ARGS, (LEVEL) - 1) \
+ : (gcc_checking_assert (LEVEL == 1), (ARGS)))
/* Set the LEVELth level of the template ARGS to VAL. This macro does
not work with single-level argument vectors. */
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2c7c5f8bb5d..75b21e5c88a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -23398,8 +23398,6 @@ static tree
try_class_unification (tree tparms, tree targs, tree parm, tree arg,
bool explain_p)
{
- tree copy_of_targs;
-
if (!CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (arg))
return NULL_TREE;
else if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
@@ -23438,17 +23436,19 @@ try_class_unification (tree tparms, tree targs, tree
parm, tree arg,
because there are two ways to unify base classes of S<0, 1, 2>
with S. If we kept the already deduced knowledge, we
would reject the possibility I=1. */
- copy_of_targs = make_tree_vec (TREE_VEC_LENGTH (targs));
+ targs = copy_template_args (targs);
+ for (tree& targ : tree_vec_range (INNERMOST_TEMPLATE_ARGS (targs)))
+targ = NULL_TREE;
if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
{
- if (unify_bound_ttp_args (tparms, copy_of_targs, parm, arg, explain_p))
+ if (unify_bound_ttp_args (tparms, targs, parm, arg, explain_p))
return NULL_TREE;
return arg;
}
/* If unification failed, we're done. */
- if (unify (tparms, copy_of_targs, CLASSTYPE_TI_ARGS (parm),
+ if (unify (tparms, targs, CLASSTYPE_TI_ARGS (parm),
CLASSTYPE_TI_ARGS (arg), UNIFY_ALLOW_NONE, explain_p))
return NULL_TREE;
@@ -23649,7 +23649,7 @@ unify_pack_expansion (tree tparms, tree targs, tree
packed_parms,
/* Determine the index and level of this parameter pack. */
template_parm_level_and_index (parm_pack, , );
- if (level < levels)
+ if (level > levels)
continue;
/* Keep track of the parameter packs and their corresponding
--
2.36.1.63.gef9b086d95
Re: [PATCH] testsuite: Make AArch64 attributes_7.c generate unique report lines.
Christophe Lyon via Gcc-patches writes:
> This test is executed in both C and C++ modes, lines 98 and 100 pass
> in C++ and are xfail in C.
>
> This results in similar lines in gcc.sum and g++.sum, differing by the
> PASS or XFAIL prefix, which confuses compare_test: it reports these
> tests twice, as "now fail, but worked before" and "now work, but
> didn't before". To make the diagnostic different betweek C and C++, I
> use $tool in the dg-error comment field.
>
> 2022-05-06 Christophe Lyon
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/acle/general/attributes_7.c: Make
> diagnostic unique.
OK, thanks.
Richard
> ---
> .../gcc.target/aarch64/sve/acle/general/attributes_7.c| 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/attributes_7.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/attributes_7.c
> index 621666ce6c9..95be60591fb 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/attributes_7.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/attributes_7.c
> @@ -95,9 +95,9 @@ f (int c)
>fb = sb;
>
>(void) (c ? sb : sb);
> - (void) (c ? sb : fb); // { dg-error {type mismatch|different types} "" {
> xfail c } }
> + (void) (c ? sb : fb); // { dg-error {type mismatch|different types}
> "$tool" { xfail c } }
>
> - (void) (c ? fb : sb); // { dg-error {type mismatch|different types} "" {
> xfail c } }
> + (void) (c ? fb : sb); // { dg-error {type mismatch|different types}
> "$tool" { xfail c } }
>(void) (c ? fb : fb);
> }
Re: Refactor '-ldl' handling for libgomp proper and plugins
On Thu, May 12, 2022 at 02:39:14PM +0200, Thomas Schwinge wrote: > Hi! > > Again, no change in behavior, just refactoring, making things more > explicit, in preparation for other changes. OK to push the attached > "Refactor '-ldl' handling for libgomp proper and plugins"? > > By the way, this might also qualify as a first preparational step to > support dynamic loading schemes different from 'dl.so'/'dlopen'. ;-) > (But I'm not working on that.) Ok. > libgomp/ > * Makefile.am (libgomp_la_LIBADD): Initialize. > * plugin/configfrag.ac (DL_LIBS): New. > (PLUGIN_GCN_LIBS): Remove. > (PLUGIN_NVPTX_LIBS): Don't set in the 'PLUGIN_NVPTX_DYNAMIC' case. > * plugin/Makefrag.am (libgomp_la_LIBADD) > (libgomp_plugin_gcn_la_LIBADD): Consider '$(DL_LIBS)'. > (libgomp_plugin_nvptx_la_LIBADD) : Likewise. > * Makefile.in: Regenerate. > * config.h.in: Likewise. > * configure: Likewise. > * testsuite/Makefile.in: Likewise. Jakub
[Ada] Skip postponed validation checks with compilation errors
If there are compilation errors then gigi doesn't backannotate the AST with sizes, alignment, etc. The postponed compilation validation checks can then easily crash or give spurious errors. We now just skip them. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * gnat1drv.adb (Gnat1drv): Skip postponed checks when there are errors.diff --git a/gcc/ada/gnat1drv.adb b/gcc/ada/gnat1drv.adb --- a/gcc/ada/gnat1drv.adb +++ b/gcc/ada/gnat1drv.adb @@ -1256,7 +1256,6 @@ begin if Compilation_Errors then Treepr.Tree_Dump; - Post_Compilation_Validation_Checks; Errout.Finalize (Last_Call => True); Errout.Output_Messages; Namet.Finalize;
[Ada] Restore double quotes in debug printouts
A previous change in "Make debug printouts more robust" accidentally
removed double quotes around identifiers in debug printouts. This patch
restores those. So for example, we have:
Prev_Entity = Node #10 N_Defining_Identifier "foo" (Entity_Id=795)
and not:
Prev_Entity = Node #10 N_Defining_Identifier foo (Entity_Id=795)
This affects the output of -gnatdt, and certain calls normally done from
gdb.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* namet.ads, namet.adb (Write_Name_For_Debug): Add Quote
parameter to allow conditional addition of quotes. Note that
some calls to Write_Name_For_Debug, for example for file names,
shouldn't have quotes, as in some_package.adb:123:45.
* treepr.adb (Print_Name): Add double quotes around the name
using the above Quote parameters.diff --git a/gcc/ada/namet.adb b/gcc/ada/namet.adb
--- a/gcc/ada/namet.adb
+++ b/gcc/ada/namet.adb
@@ -1570,9 +1570,11 @@ package body Namet is
-- Write_Name_For_Debug --
--
- procedure Write_Name_For_Debug (Id : Name_Id) is
+ procedure Write_Name_For_Debug (Id : Name_Id; Quote : String := "") is
begin
if Is_Valid_Name (Id) then
+ Write_Str (Quote);
+
declare
Buf : Bounded_String (Max_Length => Natural (Length_Of_Name (Id)));
begin
@@ -1580,6 +1582,8 @@ package body Namet is
Write_Str (Buf.Chars (1 .. Buf.Length));
end;
+ Write_Str (Quote);
+
elsif Id = No_Name then
Write_Str ("");
diff --git a/gcc/ada/namet.ads b/gcc/ada/namet.ads
--- a/gcc/ada/namet.ads
+++ b/gcc/ada/namet.ads
@@ -433,9 +433,9 @@ package Namet is
-- Like Write_Name, except that the name written is the decoded name, as
-- described for Append_Decoded.
- procedure Write_Name_For_Debug (Id : Name_Id);
+ procedure Write_Name_For_Debug (Id : Name_Id; Quote : String := "");
-- Like Write_Name, except it tries to be robust in the presence of invalid
- -- data.
+ -- data, and valid names are surrounded by Quote.
function Name_Entries_Count return Nat;
-- Return current number of entries in the names table
diff --git a/gcc/ada/treepr.adb b/gcc/ada/treepr.adb
--- a/gcc/ada/treepr.adb
+++ b/gcc/ada/treepr.adb
@@ -1142,7 +1142,7 @@ package body Treepr is
procedure Print_Name (N : Name_Id) is
begin
if Phase = Printing then
- Write_Name_For_Debug (N);
+ Write_Name_For_Debug (N, Quote => );
end if;
end Print_Name;
[Ada] Reuse collective subtype for comparison operators where possible
Replace membership alternatives with N_Op_Compare. Code cleanup; semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_res.adb (Resolve_Actuals): Simplify with N_Op_Compare. * sem_util.adb (Replace_Null_Operand, Null_To_Null_Address_Convert_OK): Likewise.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -4163,12 +4163,7 @@ package body Sem_Res is -- marked with Any_Type. Since the operation has been resolved to -- the user-defined operator, that is irrelevant, so reset Etype. -if Nkind (Original_Node (N)) in N_Op_Eq - | N_Op_Ge - | N_Op_Gt - | N_Op_Le - | N_Op_Lt - | N_Op_Ne +if Nkind (Original_Node (N)) in N_Op_Compare and then not Is_Boolean_Type (Etype (N)) then Set_Etype (A, Etype (F)); 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 @@ -14189,9 +14189,7 @@ package body Sem_Util is begin pragma Assert (Relaxed_RM_Semantics); - pragma Assert -(Nkind (N) in - N_Null | N_Op_Eq | N_Op_Ge | N_Op_Gt | N_Op_Le | N_Op_Lt | N_Op_Ne); + pragma Assert (Nkind (N) in N_Null | N_Op_Compare); if Nkind (N) = N_Null then Rewrite (N, New_Occurrence_Of (RTE (RE_Null_Address), Sloc (N))); @@ -26079,9 +26077,7 @@ package body Sem_Util is if Nkind (N) = N_Null then return Present (Typ) and then Is_Descendant_Of_Address (Typ); - elsif Nkind (N) in - N_Op_Eq | N_Op_Ge | N_Op_Gt | N_Op_Le | N_Op_Lt | N_Op_Ne - then + elsif Nkind (N) in N_Op_Compare then declare L : constant Node_Id := Left_Opnd (N); R : constant Node_Id := Right_Opnd (N);
[Ada] Remove unreferenced Is_Selector_Name routine
Function Is_Selector_Name was added in 1995 while experimenting with default expressions in record declarations. This experiment was abandoned after one day and this routine was never used since then. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_util.ads (Is_Selector_Name): Remove spec. * sem_util.adb (Is_Selector_Name): Remove body.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 @@ -20972,37 +20972,6 @@ package body Sem_Util is return False; end Is_Reversible_Iterator; - -- - -- Is_Selector_Name -- - -- - - function Is_Selector_Name (N : Node_Id) return Boolean is - begin - if not Is_List_Member (N) then - declare -P : constant Node_Id := Parent (N); - begin -return Nkind (P) in N_Expanded_Name - | N_Generic_Association - | N_Parameter_Association - | N_Selected_Component - and then Selector_Name (P) = N; - end; - - else - declare -L : constant List_Id := List_Containing (N); -P : constant Node_Id := Parent (L); - begin -return (Nkind (P) = N_Discriminant_Association - and then Selector_Names (P) = L) - or else - (Nkind (P) = N_Component_Association - and then Choices (P) = L); - end; - end if; - end Is_Selector_Name; - - -- Is_Single_Concurrent_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 @@ -2337,12 +2337,6 @@ package Sem_Util is -- AI05-0139-2: Check whether Typ is derived from the predefined interface -- Ada.Iterator_Interfaces.Reversible_Iterator. - function Is_Selector_Name (N : Node_Id) return Boolean; - -- Given an N_Identifier node N, determines if it is a Selector_Name. - -- As described in Sinfo, Selector_Names are special because they - -- represent use of the N_Identifier node for a true identifier, when - -- normally such nodes represent a direct name. - function Is_Single_Concurrent_Object (Id : Entity_Id) return Boolean; -- Determine whether arbitrary entity Id denotes the anonymous object -- created for a single protected or single task type.
[Ada] Add more dummy names in Sem_Warn.Has_Junk_Name
To sync the names used in CodePeer (and SPARK) when filtering out
warnings on e.g. unused variables.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_warn.adb (Has_Junk_Name): Add more dummy names.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
@@ -2872,7 +2872,9 @@ package body Sem_Warn is
Match ("dummy") or else
Match ("ignore") or else
Match ("junk")or else
-Match ("unused");
+Match ("unuse") or else
+Match ("tmp") or else
+Match ("temp");
end Has_Junk_Name;
--
[Ada] Fix warning about generic subprograms withed but not referenced
For warnings about unreferenced entities and unused WITH clauses we typically exclude references outside of the extended main source unit. However, we include references to variables of formal private types to warn in the instance if the corresponding type is not a fully initialized type. This special case was unnecessarily applied to references to other entities of formal private types. In particular, we applied it to the references to generic functions, where the full initialization of the formal private type doesn't matter. As an extreme example, this special case was unnecessarily applied to "pragma Pure (Unchecked_Conversion)" in the runtime unit. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * lib-xref.adb (Generate_Reference): Fix comment and implement it precisely.diff --git a/gcc/ada/lib-xref.adb b/gcc/ada/lib-xref.adb --- a/gcc/ada/lib-xref.adb +++ b/gcc/ada/lib-xref.adb @@ -664,7 +664,7 @@ package body Lib.Xref is -- a default in an instance. -- We also set the referenced flag in a generic package that is not in - -- then main source unit, when the variable is of a formal private type, + -- the main source unit, when the object is of a formal private type, -- to warn in the instance if the corresponding type is not a fully -- initialized type. @@ -694,6 +694,7 @@ package body Lib.Xref is return; elsif Inside_A_Generic + and then Is_Object (E) and then Is_Generic_Type (Etype (E)) then Set_Referenced (E);
[Ada] Fix warning about generic subprograms withed but not instantiated
The compiler should warn when a generic subprogram unit is withed but not instantiated by the current main unit. This warning relies on flag Is_Instantiated, which was wrongly set when the generic unit was also withed and instantiated by some other unit. This change merely reverts a fix done 20 years ago, which worked around some spurious warning emitted with inlining enabled. Now the Is_Instantiated flag is set similarly for generic subprograms and generic packages. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * atree.adb, atree.ads: Move WITH clause from spec to body; to prevent new warnings stopping the bootstrap. * fmap.adb, fname-sf.adb, libgnat/a-direct.adb, libgnat/s-bignum.adb: Remove unnecessary WITH clauses; to prevent new warnings stopping the bootstrap. * sem_ch12.adb (Analyze_Subprogram_Instantiation): Only set Is_Instantiated flag when the instance is in the extended main source unit.diff --git a/gcc/ada/atree.adb b/gcc/ada/atree.adb --- a/gcc/ada/atree.adb +++ b/gcc/ada/atree.adb @@ -31,6 +31,7 @@ with Opt;use Opt; with Output; use Output; with Sinfo.Utils;use Sinfo.Utils; with System.Storage_Elements; +with Unchecked_Conversion; package body Atree is diff --git a/gcc/ada/atree.ads b/gcc/ada/atree.ads --- a/gcc/ada/atree.ads +++ b/gcc/ada/atree.ads @@ -52,7 +52,6 @@ with Types; use Types; with Seinfo; use Seinfo; with System; use System; with Table; -with Unchecked_Conversion; package Atree is diff --git a/gcc/ada/fmap.adb b/gcc/ada/fmap.adb --- a/gcc/ada/fmap.adb +++ b/gcc/ada/fmap.adb @@ -34,8 +34,6 @@ pragma Warnings (Off); with System.OS_Lib; use System.OS_Lib; pragma Warnings (On); -with Unchecked_Conversion; - with GNAT.HTable; package body Fmap is diff --git a/gcc/ada/fname-sf.adb b/gcc/ada/fname-sf.adb --- a/gcc/ada/fname-sf.adb +++ b/gcc/ada/fname-sf.adb @@ -30,8 +30,6 @@ with Osint; use Osint; with Types; use Types; with System.OS_Lib; use System.OS_Lib; -with Unchecked_Conversion; - package body Fname.SF is -- diff --git a/gcc/ada/libgnat/a-direct.adb b/gcc/ada/libgnat/a-direct.adb --- a/gcc/ada/libgnat/a-direct.adb +++ b/gcc/ada/libgnat/a-direct.adb @@ -38,7 +38,6 @@ use Ada.Directories.Hierarchical_File_Names; with Ada.Strings.Fixed; with Ada.Strings.Maps; use Ada.Strings.Maps; with Ada.Strings.Unbounded; use Ada.Strings.Unbounded; -with Ada.Unchecked_Conversion; with Ada.Unchecked_Deallocation; with Interfaces.C; diff --git a/gcc/ada/libgnat/s-bignum.adb b/gcc/ada/libgnat/s-bignum.adb --- a/gcc/ada/libgnat/s-bignum.adb +++ b/gcc/ada/libgnat/s-bignum.adb @@ -29,7 +29,6 @@ -- -- -- -with Ada.Unchecked_Conversion; with System.Generic_Bignums; with System.Secondary_Stack; use System.Secondary_Stack; with System.Shared_Bignums; use System.Shared_Bignums; 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 @@ -5708,9 +5708,9 @@ package body Sem_Ch12 is Set_Scope (Inst_Id, Current_Scope); Set_Entity (Gen_Id, Gen_Unit); - Set_Is_Instantiated (Gen_Unit); if In_Extended_Main_Source_Unit (N) then +Set_Is_Instantiated (Gen_Unit); Generate_Reference (Gen_Unit, N); end if;
[Ada] Remove useless pragma Warnings Off from runtime units
GNAT does not issue a warning anymore on a postcondition of True (used here to prevent inining inside GNATprove for proof). Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-valuei.ads: Remove pragma Warnings Off. * libgnat/s-valueu.ads: Same. * libgnat/s-valuti.ads: Same.diff --git a/gcc/ada/libgnat/s-valuei.ads b/gcc/ada/libgnat/s-valuei.ads --- a/gcc/ada/libgnat/s-valuei.ads +++ b/gcc/ada/libgnat/s-valuei.ads @@ -37,8 +37,6 @@ pragma Assertion_Policy (Pre=> Ignore, Contract_Cases => Ignore, Ghost => Ignore, Subprogram_Variant => Ignore); -pragma Warnings (Off, "postcondition does not mention function result"); --- True postconditions are used to avoid inlining for GNATprove with System.Val_Util; use System.Val_Util; diff --git a/gcc/ada/libgnat/s-valueu.ads b/gcc/ada/libgnat/s-valueu.ads --- a/gcc/ada/libgnat/s-valueu.ads +++ b/gcc/ada/libgnat/s-valueu.ads @@ -43,8 +43,6 @@ pragma Assertion_Policy (Pre=> Ignore, Contract_Cases => Ignore, Ghost => Ignore, Subprogram_Variant => Ignore); -pragma Warnings (Off, "postcondition does not mention function result"); --- True postconditions are used to avoid inlining for GNATprove with System.Val_Util; use System.Val_Util; 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 @@ -41,8 +41,6 @@ pragma Assertion_Policy (Pre=> Ignore, Post => Ignore, Contract_Cases => Ignore, Ghost => Ignore); -pragma Warnings (Off, "postcondition does not mention function result"); --- True postconditions are used to avoid inlining for GNATprove with System.Case_Util;
[Ada] Do not issue a warning on a postcondition of True or False
Do not issue a warning about the postcondition of a function not
mentioning its result when this postcondition is statically True or
False, as this is a specification of non-termination (for value False)
or a hint to SPARK prover for not inlining an expression function (for
value True). In any case, the warning brings no value here.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_util.adb (Check_Result_And_Post_State): Exempt trivial
post.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
@@ -4813,6 +4813,9 @@ package body Sem_Util is
-- and post-state. Prag is a [refined] postcondition or a contract-cases
-- pragma. Result_Seen is set when the pragma mentions attribute 'Result
+ function Is_Trivial_Boolean (N : Node_Id) return Boolean;
+ -- Determine whether source node N denotes "True" or "False"
+
---
-- Check_Result_And_Post_State_In_Pragma --
---
@@ -4836,9 +4839,6 @@ package body Sem_Util is
function Is_Function_Result (N : Node_Id) return Traverse_Result;
-- Attempt to find attribute 'Result in a subtree denoted by N
- function Is_Trivial_Boolean (N : Node_Id) return Boolean;
- -- Determine whether source node N denotes "True" or "False"
-
function Mentions_Post_State (N : Node_Id) return Boolean;
-- Determine whether a subtree denoted by N mentions any construct
-- that denotes a post-state.
@@ -5089,20 +5089,6 @@ package body Sem_Util is
end if;
end Is_Function_Result;
-
- -- Is_Trivial_Boolean --
-
-
- function Is_Trivial_Boolean (N : Node_Id) return Boolean is
- begin
-return
- Comes_From_Source (N)
-and then Is_Entity_Name (N)
-and then (Entity (N) = Standard_True
-or else
- Entity (N) = Standard_False);
- end Is_Trivial_Boolean;
-
-
-- Mentions_Post_State --
-
@@ -5202,6 +5188,20 @@ package body Sem_Util is
end if;
end Check_Result_And_Post_State_In_Pragma;
+
+ -- Is_Trivial_Boolean --
+
+
+ function Is_Trivial_Boolean (N : Node_Id) return Boolean is
+ begin
+ return
+ Comes_From_Source (N)
+ and then Is_Entity_Name (N)
+ and then (Entity (N) = Standard_True
+ or else
+ Entity (N) = Standard_False);
+ end Is_Trivial_Boolean;
+
-- Local variables
Items: constant Node_Id := Contract (Subp_Id);
@@ -5305,10 +5305,14 @@ package body Sem_Util is
elsif Present (Case_Prag) and then not Seen_In_Case then
Error_Msg_N ("contract cases do not mention result?.t?", Case_Prag);
- -- The function has postconditions only and they do not mention
- -- attribute 'Result.
+ -- The function has non-trivial postconditions only and they do not
+ -- mention attribute 'Result.
- elsif Present (Post_Prag) and then not Seen_In_Post then
+ elsif Present (Post_Prag)
+and then not Seen_In_Post
+and then not Is_Trivial_Boolean
+ (Get_Pragma_Arg (First (Pragma_Argument_Associations (Post_Prag
+ then
Error_Msg_N
("postcondition does not mention function result?.t?", Post_Prag);
end if;
[Ada] Make debug printouts more robust
This patch improves some debug printouts so that they avoid crashing on
invalid data.
In addition, the relevant code uses Global_Name_Buffer all over the
place. This patch cleans up some of those uses, in particular ones in
the same code as the robustness changes, and code called by that code.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* namet.ads, namet.adb (Write_Name_For_Debug): New more-robust
version of Write_Name.
(Destroy_Global_Name_Buffer): New procedure to help detect bugs
related to use of Global_Name_Buffer. Misc cleanup and comment
improvements. E.g. we don't need to document every detail of
debugging printouts, especially since they can change.
* uname.ads, uname.adb (Write_Unit_Name_For_Debug): New
more-robust version of Write_Unit_Name.
(Get_Unit_Name_String): Pass buffer in, instead of using the
global variable. Misc cleanup. Remove the "special fudge", which
is apparently not needed, and anyway the comment "the %s or %b
has already been eliminated" seems wrong.
(Write_Unit_Name): Call the new version of Get_Unit_Name_String.
* errout.adb (Set_Msg_Insertion_Unit_Name): Call the new version
of Get_Unit_Name_String. We pass the global variable here,
because it's too much trouble to disentangle such uses in
Errout.
* sem_util.ads, sem_util.adb, sem_dist.adb
(Get_Library_Unit_Name): New version of
Get_Library_Unit_Name_String that avoids usage of the global
variable.
* casing.ads, casing.adb, exp_prag.adb, exp_util.adb
(Set_All_Upper_Case): Remove. There is no need for a wrapper
here -- code is clearer without it.
* treepr.adb (Print_Name): Call Write_Name_For_Debug, which
deals with No_Name (etc), rather than duplicating that here.
Note that the call to Get_Name_String was superfluous.
(Tree_Dump): Call Write_Unit_Name_For_Debug instead of
Write_Unit_Name, which crashes if not Is_Valid_Name.
* erroutc.ads: Improve comments.
* erroutc.adb (Set_Msg_Name_Buffer): Call
Destroy_Global_Name_Buffer to detect potential bugs where it
incorrectly looks at the global variable.
* sinput.adb (Write_Location): Call Write_Name_For_Debug instead
of Write_Name, so it won't blow up on invalid data.
* sinput.ads: Improve comments; remove some verbosity.
* libgnat/s-imagef.adb: Fix typo in comment.diff --git a/gcc/ada/casing.adb b/gcc/ada/casing.adb
--- a/gcc/ada/casing.adb
+++ b/gcc/ada/casing.adb
@@ -105,15 +105,6 @@ package body Casing is
end if;
end Determine_Casing;
-
- -- Set_All_Upper_Case --
-
-
- procedure Set_All_Upper_Case is
- begin
- Set_Casing (All_Upper_Case);
- end Set_All_Upper_Case;
-
-- Set_Casing --
diff --git a/gcc/ada/casing.ads b/gcc/ada/casing.ads
--- a/gcc/ada/casing.ads
+++ b/gcc/ada/casing.ads
@@ -78,12 +78,6 @@ package Casing is
procedure Set_Casing (C : Casing_Type; D : Casing_Type := Mixed_Case);
-- Uses Buf => Global_Name_Buffer
- procedure Set_All_Upper_Case;
- pragma Inline (Set_All_Upper_Case);
- -- This procedure is called with an identifier name stored in Name_Buffer.
- -- On return, the identifier is converted to all upper case. The call is
- -- equivalent to Set_Casing (All_Upper_Case).
-
function Determine_Casing (Ident : Text_Buffer) return Casing_Type;
-- Determines the casing of the identifier/keyword string Ident. A special
-- test is made for SPARK_Mode which is considered to be mixed case, since
diff --git a/gcc/ada/errout.adb b/gcc/ada/errout.adb
--- a/gcc/ada/errout.adb
+++ b/gcc/ada/errout.adb
@@ -3760,7 +3760,7 @@ package body Errout is
Set_Msg_Str ("");
else
- Get_Unit_Name_String (Error_Msg_Unit_1, Suffix);
+ Get_Unit_Name_String (Global_Name_Buffer, Error_Msg_Unit_1, Suffix);
Set_Msg_Blank;
Set_Msg_Quote;
Set_Msg_Name_Buffer;
diff --git a/gcc/ada/erroutc.adb b/gcc/ada/erroutc.adb
--- a/gcc/ada/erroutc.adb
+++ b/gcc/ada/erroutc.adb
@@ -1468,6 +1468,7 @@ package body Erroutc is
procedure Set_Msg_Name_Buffer is
begin
Set_Msg_Str (Name_Buffer (1 .. Name_Len));
+ Destroy_Global_Name_Buffer;
end Set_Msg_Name_Buffer;
---
diff --git a/gcc/ada/erroutc.ads b/gcc/ada/erroutc.ads
--- a/gcc/ada/erroutc.ads
+++ b/gcc/ada/erroutc.ads
@@ -23,7 +23,7 @@
-- --
--
--- This packages contains global variables and routines common to error
+-- This package contains global variables and routines common to error
-- reporting packages,
[Ada] Remove front-end SJLJ processing
We no longer use the so called front-end SJLJ exception mechanism, so get rid of it. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * ali.adb, ali.ads, bcheck.adb, exp_ch11.adb, fe.h, gnat1drv.adb, opt.adb, opt.ads, targparm.adb, targparm.ads, lib-writ.adb: Get rid of Frontend_Exceptions processing. * libgnat/system-aix.ads, libgnat/system-darwin-arm.ads, libgnat/system-darwin-ppc.ads, libgnat/system-darwin-x86.ads, libgnat/system-djgpp.ads, libgnat/system-dragonfly-x86_64.ads, libgnat/system-freebsd.ads, libgnat/system-hpux-ia64.ads, libgnat/system-hpux.ads, libgnat/system-linux-alpha.ads, libgnat/system-linux-arm.ads, libgnat/system-linux-hppa.ads, libgnat/system-linux-ia64.ads, libgnat/system-linux-m68k.ads, libgnat/system-linux-mips.ads, libgnat/system-linux-ppc.ads, libgnat/system-linux-riscv.ads, libgnat/system-linux-s390.ads, libgnat/system-linux-sh4.ads, libgnat/system-linux-sparc.ads, libgnat/system-linux-x86.ads, libgnat/system-lynxos178-ppc.ads, libgnat/system-lynxos178-x86.ads, libgnat/system-mingw.ads, libgnat/system-qnx-aarch64.ads, libgnat/system-rtems.ads, libgnat/system-solaris-sparc.ads, libgnat/system-solaris-x86.ads, libgnat/system-vxworks-arm-rtp-smp.ads, libgnat/system-vxworks-arm-rtp.ads, libgnat/system-vxworks-arm.ads, libgnat/system-vxworks-e500-kernel.ads, libgnat/system-vxworks-e500-rtp-smp.ads, libgnat/system-vxworks-e500-rtp.ads, libgnat/system-vxworks-ppc-kernel.ads, libgnat/system-vxworks-ppc-rtp-smp.ads, libgnat/system-vxworks-ppc-rtp.ads, libgnat/system-vxworks-x86-kernel.ads, libgnat/system-vxworks-x86-rtp-smp.ads, libgnat/system-vxworks-x86-rtp.ads, libgnat/system-vxworks7-aarch64-rtp-smp.ads, libgnat/system-vxworks7-aarch64.ads, libgnat/system-vxworks7-arm-rtp-smp.ads, libgnat/system-vxworks7-arm.ads, libgnat/system-vxworks7-e500-kernel.ads, libgnat/system-vxworks7-e500-rtp-smp.ads, libgnat/system-vxworks7-e500-rtp.ads, libgnat/system-vxworks7-ppc-kernel.ads, libgnat/system-vxworks7-ppc-rtp-smp.ads, libgnat/system-vxworks7-ppc-rtp.ads, libgnat/system-vxworks7-ppc64-kernel.ads, libgnat/system-vxworks7-ppc64-rtp-smp.ads, libgnat/system-vxworks7-x86-kernel.ads, libgnat/system-vxworks7-x86-rtp-smp.ads, libgnat/system-vxworks7-x86-rtp.ads, libgnat/system-vxworks7-x86_64-kernel.ads, libgnat/system-vxworks7-x86_64-rtp-smp.ads: Remove Frontend_Exceptions line. * gcc-interface/decl.cc, gcc-interface/trans.cc (gnat_to_gnu_entity, gnat_to_gnu_subprog_type, gigi, gnat_to_gnu): Remove Front_End_SJLJ processing and always assume Back_End_Exceptions. patch.diff.gz Description: application/gzip
[Ada] Add ghost code to facilitate proof with SPARK
Proof of generic units for Long_Long_Long_Unsigned instantiations is harder for provers, as they have to deal with larger values. Add ghost code to make the proof easier. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-imageu.adb (Set_Image_Unsigned): Add lemma. * libgnat/s-valueu.adb (Scan_Raw_Unsigned): Add assertion.diff --git a/gcc/ada/libgnat/s-imageu.adb b/gcc/ada/libgnat/s-imageu.adb --- a/gcc/ada/libgnat/s-imageu.adb +++ b/gcc/ada/libgnat/s-imageu.adb @@ -210,6 +210,15 @@ package body System.Image_U is -- Ghost lemma to prove the value of a character corresponding to the -- next figure. + procedure Prove_Euclidian (Val, Quot, Rest : Uns) + with +Ghost, +Pre => Quot = Val / 10 + and then Rest = Val rem 10, +Post => Val = 10 * Quot + Rest; + -- Ghost lemma to prove the relation between the quotient/remainder of + -- division by 10 and the initial value. + procedure Prove_Hexa_To_Unsigned_Ghost (R : Uns) with Ghost, @@ -256,6 +265,7 @@ package body System.Image_U is - procedure Prove_Character_Val (R : Uns) is null; + procedure Prove_Euclidian (Val, Quot, Rest : Uns) is null; procedure Prove_Hexa_To_Unsigned_Ghost (R : Uns) is null; procedure Prove_Unchanged is null; @@ -347,6 +357,9 @@ package body System.Image_U is Acc => Value); if J /= Nb_Digits then +Prove_Euclidian (Val => Prev_Value, + Quot => Value, + Rest => Hexa_To_Unsigned_Ghost (S (P + J))); pragma Assert (Prev_Value = 10 * Value + Hexa_To_Unsigned_Ghost (S (P + J))); Prove_Iter_Scan 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/libgnat/s-valueu.adb @@ -590,6 +590,10 @@ package body System.Value_U is if Str (P) = Base_Char then Ptr.all := P + 1; pragma Assert (Ptr.all = Last_Num_Based + 2); + pragma Assert +(if not Overflow then + Based_Val = Scan_Based_Number_Ghost + (Str, P, Last_Num_Based, Base, Uval)); Lemma_End_Of_Scan (Str, P, Last_Num_Based, Base, Uval); pragma Assert (if not Overflow then Uval = Based_Val.Value); exit;
[Ada] Remove use of use-clauses in loaded runtime units
The spec of runtime units that may be loaded by the compiler should not contain use-clauses, for visibility to be correctly handled. Remove use-clauses that were introduced for the ghost big integers unit as part of the proof of runtime units. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-aridou.ads: Remove use-clause, add renames and subtypes. * libgnat/s-exponn.ads: Same. * libgnat/s-expont.ads: Same. * libgnat/s-widthu.ads: Same.diff --git a/gcc/ada/libgnat/s-aridou.ads b/gcc/ada/libgnat/s-aridou.ads --- a/gcc/ada/libgnat/s-aridou.ads +++ b/gcc/ada/libgnat/s-aridou.ads @@ -34,7 +34,6 @@ -- or intermediate results are longer than the result type. with Ada.Numerics.Big_Numbers.Big_Integers_Ghost; -use Ada.Numerics.Big_Numbers.Big_Integers_Ghost; generic @@ -67,20 +66,27 @@ is Contract_Cases => Ignore, Ghost => Ignore); - package Signed_Conversion is new Signed_Conversions (Int => Double_Int); + package BI_Ghost renames Ada.Numerics.Big_Numbers.Big_Integers_Ghost; + subtype Big_Integer is BI_Ghost.Big_Integer with Ghost; + subtype Big_Natural is BI_Ghost.Big_Natural with Ghost; + use type BI_Ghost.Big_Integer; + + package Signed_Conversion is + new BI_Ghost.Signed_Conversions (Int => Double_Int); function Big (Arg : Double_Int) return Big_Integer is (Signed_Conversion.To_Big_Integer (Arg)) with Ghost; - package Unsigned_Conversion is new Unsigned_Conversions (Int => Double_Uns); + package Unsigned_Conversion is + new BI_Ghost.Unsigned_Conversions (Int => Double_Uns); function Big (Arg : Double_Uns) return Big_Integer is (Unsigned_Conversion.To_Big_Integer (Arg)) with Ghost; function In_Double_Int_Range (Arg : Big_Integer) return Boolean is - (In_Range (Arg, Big (Double_Int'First), Big (Double_Int'Last))) + (BI_Ghost.In_Range (Arg, Big (Double_Int'First), Big (Double_Int'Last))) with Ghost; function Add_With_Ovflo_Check (X, Y : Double_Int) return Double_Int diff --git a/gcc/ada/libgnat/s-exponn.ads b/gcc/ada/libgnat/s-exponn.ads --- a/gcc/ada/libgnat/s-exponn.ads +++ b/gcc/ada/libgnat/s-exponn.ads @@ -32,7 +32,6 @@ -- Signed integer exponentiation (checks off) with Ada.Numerics.Big_Numbers.Big_Integers_Ghost; -use Ada.Numerics.Big_Numbers.Big_Integers_Ghost; generic @@ -41,7 +40,6 @@ generic package System.Exponn with Pure, SPARK_Mode is - -- 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 @@ -53,14 +51,18 @@ is Contract_Cases => Ignore, Ghost => Ignore); - package Signed_Conversion is new Signed_Conversions (Int => Int); + package BI_Ghost renames Ada.Numerics.Big_Numbers.Big_Integers_Ghost; + subtype Big_Integer is BI_Ghost.Big_Integer with Ghost; + use type BI_Ghost.Big_Integer; + + package Signed_Conversion is new BI_Ghost.Signed_Conversions (Int => Int); function Big (Arg : Int) return Big_Integer is (Signed_Conversion.To_Big_Integer (Arg)) with Ghost; function In_Int_Range (Arg : Big_Integer) return Boolean is - (In_Range (Arg, Big (Int'First), Big (Int'Last))) + (BI_Ghost.In_Range (Arg, Big (Int'First), Big (Int'Last))) with Ghost; function Expon (Left : Int; Right : Natural) return Int diff --git a/gcc/ada/libgnat/s-expont.ads b/gcc/ada/libgnat/s-expont.ads --- a/gcc/ada/libgnat/s-expont.ads +++ b/gcc/ada/libgnat/s-expont.ads @@ -32,7 +32,6 @@ -- Signed integer exponentiation (checks on) with Ada.Numerics.Big_Numbers.Big_Integers_Ghost; -use Ada.Numerics.Big_Numbers.Big_Integers_Ghost; generic @@ -41,7 +40,6 @@ generic package System.Expont with Pure, SPARK_Mode is - -- 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 @@ -53,14 +51,18 @@ is Contract_Cases => Ignore, Ghost => Ignore); - package Signed_Conversion is new Signed_Conversions (Int => Int); + package BI_Ghost renames Ada.Numerics.Big_Numbers.Big_Integers_Ghost; + subtype Big_Integer is BI_Ghost.Big_Integer with Ghost; + use type BI_Ghost.Big_Integer; + + package Signed_Conversion is new BI_Ghost.Signed_Conversions (Int => Int); function Big (Arg : Int) return Big_Integer is (Signed_Conversion.To_Big_Integer (Arg)) with Ghost; function In_Int_Range (Arg : Big_Integer) return Boolean is - (In_Range (Arg, Big (Int'First), Big (Int'Last))) + (BI_Ghost.In_Range (Arg, Big (Int'First), Big
[Ada] Fix support for ISO-8859-15 and IBM CP 850 encoding
In ISO-8859-15, code for "lower y with diaeresis" is used for the upper case. In IBM CP 850, upper and lower o with stroke were missing. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * csets.adb (Fold_Latin_9): Fix y with diaeresis. (Fold_IBM_PC_850): Fix o with stroke.diff --git a/gcc/ada/csets.adb b/gcc/ada/csets.adb --- a/gcc/ada/csets.adb +++ b/gcc/ada/csets.adb @@ -558,7 +558,7 @@ package body Csets is 'q' => 'Q', X_A8 => X_A6, 'r' => 'R', X_B8 => X_B4, 's' => 'S', X_BD => X_BC, - 't' => 'T', X_BE => X_FF, + 't' => 'T', X_BE => X_BE, 'u' => 'U', 'v' => 'V', 'w' => 'W', @@ -581,7 +581,7 @@ package body Csets is 'M' => 'M', X_CC => X_CC, X_DC => X_DC, 'N' => 'N', X_CD => X_CD, X_DD => X_DD, 'O' => 'O', X_CE => X_CE, X_DE => X_DE, - 'P' => 'P', X_CF => X_CF, X_DF => X_DF, X_FF => X_FF, + 'P' => 'P', X_CF => X_CF, X_DF => X_DF, X_FF => X_BE, 'Q' => 'Q', X_A6 => X_A6, 'R' => 'R', X_B4 => X_B4, 'S' => 'S', X_BC => X_BC, @@ -835,6 +835,8 @@ package body Csets is X_98 => X_98, -- y umlaut X_99 => X_99, -- O umlaut X_9A => X_9A, -- U umlaut + X_9B => X_9D, -- o with stroke + X_9D => X_9D, -- O with stroke X_A0 => X_B5, -- a acute X_A1 => X_D6, -- i acute
[Ada] Remove redundant guard against Any_String
A sequence of checks for a valid Boolean argument fails when applied to Any_String because its component is not of a Boolean type. The explicit guard was unnecessary; it was only needed when a First_Index applied to Any_String would crash, but this was fixed soon after this guard was added. Cleanup related to recent fixes for resolution of Boolean operators. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_type.adb (Valid_Boolean_Arg): Remove redundant guard.diff --git a/gcc/ada/sem_type.adb b/gcc/ada/sem_type.adb --- a/gcc/ada/sem_type.adb +++ b/gcc/ada/sem_type.adb @@ -3512,7 +3512,6 @@ package body Sem_Type is return True; elsif Is_Array_Type (T) -and then T /= Any_String and then Number_Dimensions (T) = 1 and then Is_Boolean_Type (Component_Type (T)) and then
[Ada] Accept raise expressions as operands of boolean operators
This patch restores the previous behaviour of a recently rewritten routine Sem_Ch4.Find_Boolean_Types for boolean operators where one of the operands is a raise-expression, e.g.: (raise Program_Error or else (X /= Y)) This change is required for the Entity field of the "or else" operator to be set. For GNAT it doesn't matter, but GNATprove requires this field to be set. The affected call sequence starts at Find_Boolean_Types, goes via Check_Right_Argument and Check_Boolean_Pair to Valid_Boolean_Arg, which ultimately guards a call to Add_One_Interp. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_type.adb (Valid_Boolean_Arg): Operands of Raise_Type are valid boolean arguments.diff --git a/gcc/ada/sem_type.adb b/gcc/ada/sem_type.adb --- a/gcc/ada/sem_type.adb +++ b/gcc/ada/sem_type.adb @@ -3507,6 +3507,7 @@ package body Sem_Type is or else Is_Modular_Integer_Type (T) or else T = Universal_Integer or else T = Any_Composite +or else T = Raise_Type then return True;
[Ada] Revamp analysis of conditional expressions
The current implementation is skewed toward the first dependent
expression and does not look into the interpretations of the others if
the first one is not overloaded, which can create spurious ambiguities.
And more precise error messages are now given if the types of the
dependent expressions are not compatible.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_ch4.adb (Analyze_Case_Expression): Rewrite.
(Analyze_If_Expression): Likewise.
(Possible_Type_For_Conditional_Expression): New function.
* sem_type.adb (Specific_Type): Retur the base type in more
cases.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb
--- a/gcc/ada/sem_ch4.adb
+++ b/gcc/ada/sem_ch4.adb
@@ -265,6 +265,22 @@ package body Sem_Ch4 is
-- these aspects can be achieved without larger modifications to the
-- two-pass resolution algorithm.
+ function Possible_Type_For_Conditional_Expression
+ (T1, T2 : Entity_Id) return Entity_Id;
+ -- Given two types T1 and T2 that are _not_ compatible, return a type that
+ -- may still be used as the possible type of a conditional expression whose
+ -- dependent expressions, or part thereof, have type T1 and T2 respectively
+ -- during the first phase of type resolution, or Empty if such a type does
+ -- not exist.
+
+ -- The typical example is an if_expression whose then_expression is of a
+ -- tagged type and whose else_expresssion is of an extension of this type:
+ -- the types are not compatible but such an if_expression can be legal if
+ -- its expected type is the 'Class of the tagged type, so the function will
+ -- return the tagged type in this case. If the expected type turns out to
+ -- be something else, including the tagged type itself, then an error will
+ -- be given during the second phase of type resolution.
+
procedure Remove_Abstract_Operations (N : Node_Id);
-- Ada 2005: implementation of AI-310. An abstract non-dispatching
-- operation is not a candidate interpretation.
@@ -1559,10 +1575,30 @@ package body Sem_Ch4 is
-
procedure Analyze_Case_Expression (N : Node_Id) is
+ Expr : constant Node_Id := Expression (N);
+ First_Alt : constant Node_Id := First (Alternatives (N));
+
+ First_Expr : Node_Id := Empty;
+ -- First expression in the case where there is some type information
+ -- available, i.e. there is not Any_Type everywhere, which can happen
+ -- because of some error.
+
+ Second_Expr : Node_Id := Empty;
+ -- Second expression as above
+
+ Wrong_Alt : Node_Id := Empty;
+ -- For error reporting
+
procedure Non_Static_Choice_Error (Choice : Node_Id);
-- Error routine invoked by the generic instantiation below when
-- the case expression has a non static choice.
+ procedure Check_Next_Expression (T : Entity_Id; Alt : Node_Id);
+ -- Check one interpretation of the next expression with type T
+
+ procedure Check_Expression_Pair (T1, T2 : Entity_Id; Alt : Node_Id);
+ -- Check first expression with type T1 and next expression with type T2
+
package Case_Choices_Analysis is new
Generic_Analyze_Choices
(Process_Associated_Node => No_OP);
@@ -1585,23 +1621,81 @@ package body Sem_Ch4 is
("choice given in case expression is not static!", Choice);
end Non_Static_Choice_Error;
- -- Local variables
+ ---
+ -- Check_Next_Expression --
+ ---
- Expr : constant Node_Id := Expression (N);
- Alt : Node_Id;
- Exp_Type : Entity_Id;
- Exp_Btype : Entity_Id;
+ procedure Check_Next_Expression (T : Entity_Id; Alt : Node_Id) is
+ Next_Expr : constant Node_Id := Expression (Alt);
- FirstX : Node_Id := Empty;
- -- First expression in the case for which there is some type information
- -- available, i.e. it is not Any_Type, which can happen because of some
- -- error, or from the use of e.g. raise Constraint_Error.
+ I : Interp_Index;
+ It: Interp;
- Others_Present : Boolean;
- -- Indicates if Others was present
+ begin
+ if Next_Expr = First_Expr then
+Check_Next_Expression (T, Next (Alt));
+return;
+ end if;
- Wrong_Alt : Node_Id := Empty;
- -- For error reporting
+ -- Loop through the interpretations of the next expression
+
+ if not Is_Overloaded (Next_Expr) then
+Check_Expression_Pair (T, Etype (Next_Expr), Alt);
+
+ else
+Get_First_Interp (Next_Expr, I, It);
+while Present (It.Typ) loop
+ Check_Expression_Pair (T, It.Typ, Alt);
+ Get_Next_Interp (I, It);
+end loop;
+ end if;
+ end Check_Next_Expression;
+
+ ---
+
[Ada] Plug loophole in Possible_Type_For_Conditional_Expression
We need to check that the designated subprograms of access-to-subprogram types are subtype conformant before registering a common interpretation. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch4.adb (Possible_Type_For_Conditional_Expression): Add test for subtype conformance in the cases of access-to-subprogram types.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb --- a/gcc/ada/sem_ch4.adb +++ b/gcc/ada/sem_ch4.adb @@ -7904,11 +7904,15 @@ package body Sem_Ch4 is elsif Is_Access_Subprogram_Attribute (T1) and then Is_Access_Subprogram_Attribute (T2) +and then + Subtype_Conformant (Designated_Type (T1), Designated_Type (T2)) then return T1; elsif Is_Access_Protected_Subprogram_Attribute (T1) and then Is_Access_Protected_Subprogram_Attribute (T2) +and then + Subtype_Conformant (Designated_Type (T1), Designated_Type (T2)) then return T1;
[Ada] Adapt CodePeer analysis of GNAT to changes in dependencies
New dependencies in s-imagei are causing issues for CodePeer analysis of GNAT source and libs. Updating the setup. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-imagef.adb: Justify false message from CodePeer.diff --git a/gcc/ada/libgnat/s-imagef.adb b/gcc/ada/libgnat/s-imagef.adb --- a/gcc/ada/libgnat/s-imagef.adb +++ b/gcc/ada/libgnat/s-imagef.adb @@ -433,6 +433,8 @@ package body System.Image_F is Digs (1 .. 2) := " 0"; Ndigs := 2; end if; + pragma Annotate (CodePeer, False_Positive, "test always true", + "no digits were output for zero"); Set_Decimal_Digits (Digs, Ndigs, S, P, Scale, Fore, Aft, Exp); end Set_Image_Fixed;
[Ada] Proof of 'Image support for signed integers
Prove System.Image_I, similarly to the proof done for System.Image_U. The contracts make the connection between the result of Image_Integer, the available space computed with System.Width_U and the result of 'Value as computed by Value_Integer. I/O units that now depend on non-pure units are also marked not Pure anymore. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-imagef.adb: Adapt to new signature of Image_I, by providing ghost imported subprograms. For now, no contract is used on these subprograms, as System.Image_F is not proved. * libgnat/s-imagef.ads: Add modular type Uns as formal parameter, to use in defining Int_Params for instantiating Image_I. * libgnat/s-imagei.adb: Add contracts and ghost code. * libgnat/s-imagei.ads: Replace Int formal parameter by package Int_Params, which bundles type Int and Uns with ghost subprograms. Add contracts. * libgnat/s-imfi128.ads: Adapt to new formal of Image_F. * libgnat/s-imfi32.ads: Adapt to new formal of Image_F. * libgnat/s-imfi64.ads: Adapt to new formal of Image_F. * libgnat/s-imgint.ads: Adapt to new formals of Image_I. * libgnat/s-imglli.ads: Adapt to new formals of Image_I. * libgnat/s-imgllli.ads: Adapt to new formals of Image_I. * libgnat/s-valint.ads: Adapt to new formals of Value_I. * libgnat/s-vallli.ads: Adapt to new formals of Value_I. * libgnat/s-vai.ads: Adapt to new formals of Value_I. * libgnat/s-valuei.adb (Prove_Scan_Only_Decimal_Ghost): New ghost lemma. * libgnat/s-valuei.ads: New formal parameters to prove the new lemma. * libgnat/s-valuti.ads (Int_Params): Define a generic package to be used as a trait-like formal parameter in Image_I and other generics that need to instantiate Image_I. * libgnat/s-widthu.ads (Big_10): Qualify the 10 literal. patch.diff.gz Description: application/gzip
[Ada] Introduce hardbool Machine_Attribute for Ada
Implement and document hardened booleans, from nonstandard boolean types with representation clauses to the extra validity checking performed on boolean types annotated with the "hardbool" Machine_Attribute pragma. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * doc/gnat_rm/security_hardening_features.rst (Hardened Booleans): New. * exp_util.adb (Adjust_Condition): Perform validity checking on hardbool-annotated types even with -gnatVT. * gnat_rm.texi: Regenerate. * gcc-interface/utils.cc (gnat_internal_attribute_table): Ignore hardbool.diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst @@ -160,3 +160,39 @@ files of the corresponding passes, through command line options They are separate options, however, because of the significantly different performance impact of the hardening transformations. + + +.. Hardened Booleans: + +Hardened Booleans += + +Ada has built-in support for introducing boolean types with +alternative representations, using representation clauses: + +.. code-block:: ada + + type HBool is new Boolean; + for HBool use (16#5a#, 16#a5#); + for HBool'Size use 8; + +When validity checking is enabled, the compiler will check that +variables of such types hold values corresponding to the selected +representations. + +There are multiple strategies for where to introduce validity checking +(see *-gnatV* options). Their goal is to guard against various kinds +of programming errors, and GNAT strives to omit checks when program +logic rules out an invalid value, and optimizers may further remove +checks found to be redundant. + +For additional hardening, the ``hardbool`` :samp:`Machine_Attribute` +pragma can be used to annotate boolean types with representation +clauses, so that expressions of such types used as conditions are +checked even when compiling with *-gnatVT*. + +.. code-block:: ada + + pragma Machine_Attribute (HBool, "hardbool"); + +Note that *-gnatVn* will disable even ``hardbool`` testing. 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 @@ -328,6 +328,72 @@ package body Exp_Util is -- procedure Adjust_Condition (N : Node_Id) is + + function Is_Hardbool_Type (T : Entity_Id) return Boolean; + -- Return True iff T is a type annotated with the + -- Machine_Attribute pragma "hardbool". + + -- + -- Is_Hardbool_Type -- + -- + + function Is_Hardbool_Type (T : Entity_Id) return Boolean is + + function Find_Hardbool_Pragma + (Id : Entity_Id) return Node_Id; + -- Return a Rep_Item associated with entity Id that + -- corresponds to the Hardbool Machine_Attribute pragma, if + -- any, or Empty otherwise. + + function Pragma_Arg_To_String (Item : Node_Id) return String is +(To_String (Strval (Expr_Value_S (Item; + -- Return the pragma argument Item as a String + + function Hardbool_Pragma_P (Item : Node_Id) return Boolean is +(Nkind (Item) = N_Pragma + and then + Pragma_Name (Item) = Name_Machine_Attribute + and then + Pragma_Arg_To_String + (Get_Pragma_Arg + (Next (First (Pragma_Argument_Associations (Item) + = "hardbool"); + -- Return True iff representation Item is a "hardbool" + -- Machine_Attribute pragma. + + -- + -- Find_Hardbool_Pragma -- + -- + + function Find_Hardbool_Pragma + (Id : Entity_Id) return Node_Id + is +Item : Node_Id; + + begin +if not Has_Gigi_Rep_Item (Id) then + return Empty; +end if; + +Item := First_Rep_Item (Id); +while Present (Item) loop + if Hardbool_Pragma_P (Item) then + return Item; + end if; + Item := Next_Rep_Item (Item); +end loop; + +return Empty; + end Find_Hardbool_Pragma; + + -- Start of processing for Is_Hardbool_Type + + begin + return Present (Find_Hardbool_Pragma (T)); + end Is_Hardbool_Type; + + -- Start of processing for Adjust_Condition + begin if No (N) then return; @@ -347,7 +413,10 @@ package body Exp_Util is -- Apply validity checking if needed - if Validity_Checks_On and Validity_Check_Tests then + if Validity_Checks_On + and then + (Validity_Check_Tests or else Is_Hardbool_Type (T)) + then
[Ada] Rewrite Sem_Ch4.Find_Boolean_Types
Using a straight implementation like the one in Find_Arithmetic_Types. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch4.adb (Find_Arithmetic_Types): Use local variables. (Find_Boolean_Types): Rewrite modeled on Find_Arithmetic_Types.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb --- a/gcc/ada/sem_ch4.adb +++ b/gcc/ada/sem_ch4.adb @@ -6450,11 +6450,6 @@ package body Sem_Ch4 is Op_Id : Entity_Id; N : Node_Id) is - Index1 : Interp_Index; - Index2 : Interp_Index; - It1: Interp; - It2: Interp; - procedure Check_Right_Argument (T : Entity_Id); -- Check right operand of operator @@ -6463,19 +6458,27 @@ package body Sem_Ch4 is -- procedure Check_Right_Argument (T : Entity_Id) is + I : Interp_Index; + It : Interp; + begin if not Is_Overloaded (R) then Check_Arithmetic_Pair (T, Etype (R), Op_Id, N); else -Get_First_Interp (R, Index2, It2); -while Present (It2.Typ) loop - Check_Arithmetic_Pair (T, It2.Typ, Op_Id, N); - Get_Next_Interp (Index2, It2); +Get_First_Interp (R, I, It); +while Present (It.Typ) loop + Check_Arithmetic_Pair (T, It.Typ, Op_Id, N); + Get_Next_Interp (I, It); end loop; end if; end Check_Right_Argument; + -- Local variables + + I : Interp_Index; + It : Interp; + -- Start of processing for Find_Arithmetic_Types begin @@ -6483,10 +6486,10 @@ package body Sem_Ch4 is Check_Right_Argument (Etype (L)); else - Get_First_Interp (L, Index1, It1); - while Present (It1.Typ) loop -Check_Right_Argument (It1.Typ); -Get_Next_Interp (Index1, It1); + Get_First_Interp (L, I, It); + while Present (It.Typ) loop +Check_Right_Argument (It.Typ); +Get_Next_Interp (I, It); end loop; end if; end Find_Arithmetic_Types; @@ -6500,86 +6503,77 @@ package body Sem_Ch4 is Op_Id : Entity_Id; N : Node_Id) is - Index : Interp_Index; - It: Interp; + procedure Check_Boolean_Pair (T1, T2 : Entity_Id); + -- Check operand pair of operator - procedure Check_Numeric_Argument (T : Entity_Id); - -- Special case for logical operations one of whose operands is an - -- integer literal. If both are literal the result is any modular type. + procedure Check_Right_Argument (T : Entity_Id); + -- Check right operand of operator - - -- Check_Numeric_Argument -- - + + -- Check_Boolean_Pair -- + + + procedure Check_Boolean_Pair (T1, T2 : Entity_Id) is + T : Entity_Id; - procedure Check_Numeric_Argument (T : Entity_Id) is begin - if T = Universal_Integer then -Add_One_Interp (N, Op_Id, Any_Modular); + if Valid_Boolean_Arg (T1) + and then Valid_Boolean_Arg (T2) + and then (Covers (T1 => T1, T2 => T2) + or else Covers (T1 => T2, T2 => T1)) + then +T := Specific_Type (T1, T2); + +if T = Universal_Integer then + T := Any_Modular; +end if; - elsif Is_Modular_Integer_Type (T) then Add_One_Interp (N, Op_Id, T); end if; - end Check_Numeric_Argument; + end Check_Boolean_Pair; - -- Start of processing for Find_Boolean_Types + -- + -- Check_Right_Argument -- + -- - begin - if not Is_Overloaded (L) then - if Etype (L) = Universal_Integer - or else Etype (L) = Any_Modular - then -if not Is_Overloaded (R) then - Check_Numeric_Argument (Etype (R)); + procedure Check_Right_Argument (T : Entity_Id) is + I : Interp_Index; + It : Interp; -else - Get_First_Interp (R, Index, It); - while Present (It.Typ) loop - Check_Numeric_Argument (It.Typ); - Get_Next_Interp (Index, It); - end loop; -end if; + begin + -- Defend against previous error - -- If operands are aggregates, we must assume that they may be - -- boolean arrays, and leave disambiguation for the second pass. - -- If only one is an aggregate, verify that the other one has an - -- interpretation as a boolean array + if Nkind (R) = N_Error then +null; - elsif Nkind (L) = N_Aggregate then -if Nkind (R) = N_Aggregate then - Add_One_Interp (N, Op_Id, Etype (L)); +
[Ada] Fix style checking rule for square brackets in Ada 2022 and above
The square bracket syntax introduced in Ada 2022 was incorrectly handled
by the style checker and incorrect spacing was enforced. The issue was
in part caused by the wide character syntax (support removed starting
from Ada 2022) that treats the square bracket as a valid identifier
character.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* csets.adb (Initialize): Only treat square bracket as valid
identifier character for Ada versions prior to Ada 2022.
* style.ads (Check_Left_Paren): Rename...
(Check_Left_Paren_Square_Bracket): ...to this.
* styleg.adb (Check_Left_Bracket): Rename...
(Check_Left_Paren_Square_Bracket): ...to this.
* styleg.ads (Check_Left_Paren): Rename...
(Check_Left_Paren_Square_Bracket): ...to this.
* scng.adb (Scan): Add check for spacing around left square
bracket and use new name for Check_Left_Paren_Square_Bracket.
* libgnat/a-szmzco.ads (Control_Ranges, Graphic_Ranges)
(Letter_Ranges, Decimal_Digit_Ranges, ISO_646_Ranges)
(Character_Ranges): Fix style (remove extra space).
* libgnat/a-swmwco.ads (Control_Ranges, Graphic_Ranges)
(Letter_Ranges, Decimal_Digit_Ranges, ISO_646_Ranges)
(Character_Ranges): Likewise.
* opt.adb (Set_Config_Switches): Remove [ from Identifier_Char
set.diff --git a/gcc/ada/csets.adb b/gcc/ada/csets.adb
--- a/gcc/ada/csets.adb
+++ b/gcc/ada/csets.adb
@@ -1145,12 +1145,13 @@ package body Csets is
Identifier_Char (J) := (Fold_Upper (J) /= ' ');
end loop;
- -- Always add [ as an identifier character to deal with the brackets
- -- notation for wide characters used in identifiers. Note that if
- -- we are not allowing wide characters in identifiers, then any use
- -- of this notation will be flagged as an error in Scan_Identifier.
+ -- Add [ as an identifier character to deal with the brackets notation
+ -- for wide characters used in identifiers for versions up to Ada 2012.
+ -- Note that if we are not allowing wide characters in identifiers, then
+ -- any use of this notation will be flagged as an error in
+ -- Scan_Identifier.
- Identifier_Char ('[') := True;
+ Identifier_Char ('[') := Ada_Version < Ada_2022;
-- Add entry for ESC if wide characters in use with a wide character
-- encoding method active that uses the ESC code for encoding.
diff --git a/gcc/ada/libgnat/a-swmwco.ads b/gcc/ada/libgnat/a-swmwco.ads
--- a/gcc/ada/libgnat/a-swmwco.ads
+++ b/gcc/ada/libgnat/a-swmwco.ads
@@ -66,27 +66,27 @@ private
subtype WC is Wide_Character;
Control_Ranges : aliased constant Wide_Character_Ranges :=
- [ (W.NUL, W.US),
- (W.DEL, W.APC)];
+ [(W.NUL, W.US),
+ (W.DEL, W.APC)];
Control_Set : constant Wide_Character_Set :=
(AF.Controlled with
Control_Ranges'Unrestricted_Access);
Graphic_Ranges : aliased constant Wide_Character_Ranges :=
- [ (W.Space, W.Tilde),
- (WC'Val (256), WC'Last)];
+ [(W.Space, W.Tilde),
+ (WC'Val (256), WC'Last)];
Graphic_Set : constant Wide_Character_Set :=
(AF.Controlled with
Graphic_Ranges'Unrestricted_Access);
Letter_Ranges: aliased constant Wide_Character_Ranges :=
- [ ('A', 'Z'),
- (W.LC_A,W.LC_Z),
- (W.UC_A_Grave, W.UC_O_Diaeresis),
- (W.UC_O_Oblique_Stroke, W.LC_O_Diaeresis),
- (W.LC_O_Oblique_Stroke, W.LC_Y_Diaeresis)];
+ [('A', 'Z'),
+ (W.LC_A,W.LC_Z),
+ (W.UC_A_Grave, W.UC_O_Diaeresis),
+ (W.UC_O_Oblique_Stroke, W.LC_O_Diaeresis),
+ (W.LC_O_Oblique_Stroke, W.LC_Y_Diaeresis)];
Letter_Set : constant Wide_Character_Set :=
(AF.Controlled with
@@ -126,7 +126,7 @@ private
Basic_Ranges'Unrestricted_Access);
Decimal_Digit_Ranges : aliased constant Wide_Character_Ranges :=
- [ ('0', '9')];
+ [('0', '9')];
Decimal_Digit_Set: constant Wide_Character_Set :=
(AF.Controlled with
@@ -167,14 +167,14 @@ private
Special_Graphic_Ranges'Unrestricted_Access);
ISO_646_Ranges : aliased constant Wide_Character_Ranges :=
- [ (W.NUL, W.DEL)];
+ [(W.NUL, W.DEL)];
ISO_646_Set : constant Wide_Character_Set :=
(AF.Controlled with
ISO_646_Ranges'Unrestricted_Access);
Character_Ranges : aliased constant Wide_Character_Ranges :=
- [ (W.NUL, WC'Val (255))];
+ [(W.NUL, WC'Val (255))];
Character_Set: constant Wide_Character_Set :=
(AF.Controlled with
diff --git a/gcc/ada/libgnat/a-szmzco.ads b/gcc/ada/libgnat/a-szmzco.ads
--- a/gcc/ada/libgnat/a-szmzco.ads
+++ b/gcc/ada/libgnat/a-szmzco.ads
@@ -66,27 +66,27 @@ private
subtype WC is
[Ada] Fix scope of block in expanded protected entry body
We expand protected entry body into procedure with a nested block. The scope of this block is naturally the enclosing procedure. However, the scope field was wrongly set to the entity of the enclosing procedure body (i.e. E_Subprogram_Body); now it is set to the entity of the enclosing procedure spec (i.e. E_Procedure). In general, the scope field should always points to spec, even for entities that are syntactically nested within a program unit body. The scope set to E_Subprogram_Body entity was only accepted because in gigi we had a protection for that, but this protection is now removed. This patch fixes a regression in a ravenscar testsuite test (c_rf02_06), where GNAT was crashing when compiling in configurable runtime mode with restriction No_Exception_Propagation. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch9.adb (Build_Protected_Entry): Set scope of the nested block to spec and not the body of the procedure created for a protected entry.diff --git a/gcc/ada/exp_ch9.adb b/gcc/ada/exp_ch9.adb --- a/gcc/ada/exp_ch9.adb +++ b/gcc/ada/exp_ch9.adb @@ -3707,7 +3707,8 @@ package body Exp_Ch9 is Analyze_Statements (Bod_Stmts); - Set_Scope (Entity (Identifier (First (Bod_Stmts))), Bod_Id); + Set_Scope (Entity (Identifier (First (Bod_Stmts))), + Protected_Body_Subprogram (Ent)); Reset_Scopes_To (First (Bod_Stmts), Entity (Identifier (First (Bod_Stmts;
[Ada] Fix typos in syntax for implementation-defined pragmas
Documentation cleanup. Spotted while looking at description of configuration pragmas. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * doc/gnat_rm/implementation_defined_pragmas.rst: (Export_Object, Import_Object, Short_Descriptors): Fix pragma syntax specification. * gnat_rm.texi: Regenerate.diff --git a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst --- a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst +++ b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst @@ -1985,10 +1985,10 @@ Syntax: :: - pragma Export_Object + pragma Export_Object ( [Internal =>] LOCAL_NAME [, [External =>] EXTERNAL_SYMBOL] - [, [Size =>] EXTERNAL_SYMBOL] + [, [Size =>] EXTERNAL_SYMBOL]); EXTERNAL_SYMBOL ::= IDENTIFIER @@ -2911,7 +2911,7 @@ Syntax: :: - pragma Import_Object + pragma Import_Object ( [Internal =>] LOCAL_NAME [, [External =>] EXTERNAL_SYMBOL] [, [Size =>] EXTERNAL_SYMBOL]); @@ -6031,7 +6031,7 @@ Syntax: .. code-block:: ada - pragma Short_Descriptors + pragma Short_Descriptors; This pragma is provided for compatibility with other Ada implementations. It 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 @@ -3426,10 +3426,10 @@ still allows the specification of parameter mechanisms. Syntax: @example -pragma Export_Object +pragma Export_Object ( [Internal =>] LOCAL_NAME [, [External =>] EXTERNAL_SYMBOL] - [, [Size =>] EXTERNAL_SYMBOL] + [, [Size =>] EXTERNAL_SYMBOL]); EXTERNAL_SYMBOL ::= IDENTIFIER @@ -4385,7 +4385,7 @@ is used. Syntax: @example -pragma Import_Object +pragma Import_Object ( [Internal =>] LOCAL_NAME [, [External =>] EXTERNAL_SYMBOL] [, [Size =>] EXTERNAL_SYMBOL]); @@ -7597,7 +7597,7 @@ There is no requirement that all units in a partition use this option. Syntax: @example -pragma Short_Descriptors +pragma Short_Descriptors; @end example This pragma is provided for compatibility with other Ada implementations. It
[Ada] Reuse generic string hash for invocation signatures
Use a generic string hash routine for hashing of invocation signatures. The System.String_Hash unit was added to GNAT in 2009 and shouldn't cause any bootstrap problems these days. To be safe, we don't use the GNAT.String_Hash renaming, which was added later. Cleanup related to a new restriction No_Uninitialized_Local_Scalars, which will also rely on GNAT hash tables. Semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * ali.adb (Hash): Reuse GNAT.String_Hash.Hash and don't pollute the Name_Id table.diff --git a/gcc/ada/ali.adb b/gcc/ada/ali.adb --- a/gcc/ada/ali.adb +++ b/gcc/ada/ali.adb @@ -33,6 +33,7 @@ with Snames; use Snames; with GNAT; use GNAT; with GNAT.Dynamic_HTables; use GNAT.Dynamic_HTables; +with System.String_Hash; package body ALI is @@ -578,20 +579,18 @@ package body ALI is function Hash (IS_Rec : Invocation_Signature_Record) return Bucket_Range_Type is + function String_Hash is new System.String_Hash.Hash +(Char_Type => Character, + Key_Type => String, + Hash_Type => Bucket_Range_Type); + Buffer : Bounded_String (2052); - IS_Nam : Name_Id; begin - -- The hash is obtained in the following manner: - -- - --* A String signature based on the scope, name, line number, column - -- number, and locations, in the following format: + -- The hash is obtained from a signature based on the scope, name, line + -- number, column number, and locations, in the following format: -- -- scope__name__line_column__locations - -- - --* The String is converted into a Name_Id - -- - --* The absolute value of the Name_Id is used as the hash Append (Buffer, IS_Rec.Scope); Append (Buffer, "__"); @@ -606,8 +605,7 @@ package body ALI is Append (Buffer, IS_Rec.Locations); end if; - IS_Nam := Name_Find (Buffer); - return Bucket_Range_Type (abs IS_Nam); + return String_Hash (To_String (Buffer)); end Hash;
[Ada] Document Aggregate_Individually_Assign as a configuration pragma
Pragma Aggregate_Individually_Assign was apparently forgotten to be listed in the GNAT User's Guide as a configuration pragma. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * doc/gnat_ugn/the_gnat_compilation_model.rst (Configuration Pragmas): Add Aggregate_Individually_Assign; otherwise the list is complete except for some obsoleted pragmas, which most likely are intentionally omitted. * gnat_ugn.texi: Regenerate.diff --git a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst --- a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst +++ b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst @@ -1402,6 +1402,7 @@ recognized by GNAT:: Ada_12 Ada_2012 Ada_2022 + Aggregate_Individually_Assign Allow_Integer_Address Annotate Assertion_Policy diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi --- a/gcc/ada/gnat_ugn.texi +++ b/gcc/ada/gnat_ugn.texi @@ -2808,6 +2808,7 @@ Ada_2005 Ada_12 Ada_2012 Ada_2022 +Aggregate_Individually_Assign Allow_Integer_Address Annotate Assertion_Policy
[PATCH] rtl-optimization/105577 - RTL DSE and non-call EH
When one of the first two stages of DSE removes a throwing stmt
we have to purge dead EH edges before the DF re-analyze fires off
a fast DCE since that cannot cope with the situation.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
I'll add a testcase when it has finished reducing in time.
Richard.
2022-05-12 Richard Biener
PR rtl-optimization/105577
* dse.cc (rest_of_handle_dse): Make sure to purge dead EH
edges before running fast DCE via df_analyze.
---
gcc/dse.cc | 10 ++
1 file changed, 10 insertions(+)
diff --git a/gcc/dse.cc b/gcc/dse.cc
index a58c17e0ccb..30c11cee034 100644
--- a/gcc/dse.cc
+++ b/gcc/dse.cc
@@ -3682,6 +3682,16 @@ rest_of_handle_dse (void)
dse_step0 ();
dse_step1 ();
+ /* DSE can eliminate potentially-trapping MEMs.
+ Remove any EH edges associated with them, since otherwise
+ DF_LR_RUN_DCE will complain later. */
+ if ((locally_deleted || globally_deleted)
+ && cfun->can_throw_non_call_exceptions
+ && purge_all_dead_edges ())
+{
+ free_dominance_info (CDI_DOMINATORS);
+ delete_unreachable_blocks ();
+}
dse_step2_init ();
if (dse_step2 ())
{
--
2.35.3
Refactor '-ldl' handling for libgomp proper and plugins
Hi! Again, no change in behavior, just refactoring, making things more explicit, in preparation for other changes. OK to push the attached "Refactor '-ldl' handling for libgomp proper and plugins"? By the way, this might also qualify as a first preparational step to support dynamic loading schemes different from 'dl.so'/'dlopen'. ;-) (But I'm not working on that.) Grüße Thomas - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 >From 223f9e18819bce6b51642c8aebb7272655d12e6f Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Thu, 5 May 2022 23:01:36 +0200 Subject: [PATCH] Refactor '-ldl' handling for libgomp proper and plugins Instead of implicit global 'LIBS="-ldl $LIBS"' via 'AC_CHECK_LIB', make '-ldl' explicit for libgomp proper, and clean up 'PLUGIN_GCN_LIBS', 'PLUGIN_NVPTX_LIBS' accordingly. libgomp/ * Makefile.am (libgomp_la_LIBADD): Initialize. * plugin/configfrag.ac (DL_LIBS): New. (PLUGIN_GCN_LIBS): Remove. (PLUGIN_NVPTX_LIBS): Don't set in the 'PLUGIN_NVPTX_DYNAMIC' case. * plugin/Makefrag.am (libgomp_la_LIBADD) (libgomp_plugin_gcn_la_LIBADD): Consider '$(DL_LIBS)'. (libgomp_plugin_nvptx_la_LIBADD) : Likewise. * Makefile.in: Regenerate. * config.h.in: Likewise. * configure: Likewise. * testsuite/Makefile.in: Likewise. --- libgomp/Makefile.am | 1 + libgomp/Makefile.in | 8 libgomp/config.h.in | 3 --- libgomp/configure | 16 libgomp/plugin/Makefrag.am| 8 ++-- libgomp/plugin/configfrag.ac | 9 +++-- libgomp/testsuite/Makefile.in | 2 +- 7 files changed, 19 insertions(+), 28 deletions(-) diff --git a/libgomp/Makefile.am b/libgomp/Makefile.am index f8b2a06d63e..428f7a9dab5 100644 --- a/libgomp/Makefile.am +++ b/libgomp/Makefile.am @@ -56,6 +56,7 @@ endif libgomp_version_info = -version-info $(libtool_VERSION) libgomp_la_LDFLAGS = $(libgomp_version_info) $(libgomp_version_script) \ $(lt_host_flags) +libgomp_la_LIBADD = libgomp_la_DEPENDENCIES = $(libgomp_version_dep) libgomp_la_LINK = $(LINK) $(libgomp_la_LDFLAGS) diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in index 51252b89462..c47b6bd6153 100644 --- a/libgomp/Makefile.in +++ b/libgomp/Makefile.in @@ -125,7 +125,7 @@ target_triplet = @target@ @PLUGIN_NVPTX_DYNAMIC_FALSE@@PLUGIN_NVPTX_TRUE@ -DPLUGIN_NVPTX_LINK_LIBCUDA # 'dlopen'ing the CUDA Driver library vs. linking it. -@PLUGIN_NVPTX_DYNAMIC_TRUE@@PLUGIN_NVPTX_TRUE@am__append_3 = $(PLUGIN_NVPTX_LIBS) +@PLUGIN_NVPTX_DYNAMIC_TRUE@@PLUGIN_NVPTX_TRUE@am__append_3 = $(DL_LIBS) @PLUGIN_NVPTX_DYNAMIC_FALSE@@PLUGIN_NVPTX_TRUE@am__append_4 = $(PLUGIN_NVPTX_LIBS) @PLUGIN_GCN_TRUE@am__append_5 = libgomp-plugin-gcn.la @USE_FORTRAN_TRUE@am__append_6 = openacc.f90 @@ -219,7 +219,6 @@ libgomp_plugin_nvptx_la_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC \ $(libgomp_plugin_nvptx_la_LDFLAGS) $(LDFLAGS) -o $@ @PLUGIN_NVPTX_TRUE@am_libgomp_plugin_nvptx_la_rpath = -rpath \ @PLUGIN_NVPTX_TRUE@ $(toolexeclibdir) -libgomp_la_LIBADD = @USE_FORTRAN_TRUE@am__objects_1 = openacc.lo am_libgomp_la_OBJECTS = alloc.lo atomic.lo barrier.lo critical.lo \ env.lo error.lo icv.lo icv-device.lo iter.lo iter_ull.lo \ @@ -384,6 +383,7 @@ CUDA_DRIVER_LIB = @CUDA_DRIVER_LIB@ CYGPATH_W = @CYGPATH_W@ DEFS = @DEFS@ DEPDIR = @DEPDIR@ +DL_LIBS = @DL_LIBS@ DSYMUTIL = @DSYMUTIL@ DUMPBIN = @DUMPBIN@ ECHO_C = @ECHO_C@ @@ -441,7 +441,6 @@ PACKAGE_URL = @PACKAGE_URL@ PACKAGE_VERSION = @PACKAGE_VERSION@ PATH_SEPARATOR = @PATH_SEPARATOR@ PERL = @PERL@ -PLUGIN_GCN_LIBS = @PLUGIN_GCN_LIBS@ PLUGIN_NVPTX_CPPFLAGS = @PLUGIN_NVPTX_CPPFLAGS@ PLUGIN_NVPTX_LDFLAGS = @PLUGIN_NVPTX_LDFLAGS@ PLUGIN_NVPTX_LIBS = @PLUGIN_NVPTX_LIBS@ @@ -553,6 +552,7 @@ libgomp_version_info = -version-info $(libtool_VERSION) libgomp_la_LDFLAGS = $(libgomp_version_info) $(libgomp_version_script) \ $(lt_host_flags) +libgomp_la_LIBADD = $(DL_LIBS) libgomp_la_DEPENDENCIES = $(libgomp_version_dep) libgomp_la_LINK = $(LINK) $(libgomp_la_LDFLAGS) libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c env.c \ @@ -586,7 +586,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c env.c \ @PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LDFLAGS = $(libgomp_plugin_gcn_version_info) \ @PLUGIN_GCN_TRUE@ $(lt_host_flags) -@PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LIBADD = libgomp.la $(PLUGIN_GCN_LIBS) +@PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LIBADD = libgomp.la $(DL_LIBS) @PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LIBTOOLFLAGS = --tag=disable-static nodist_noinst_HEADERS = libgomp_f.h nodist_libsubinclude_HEADERS = omp.h openacc.h acc_prof.h diff --git a/libgomp/config.h.in b/libgomp/config.h.in index d971ea38c46..46d3eac1e61 100644 --- a/libgomp/config.h.in +++ b/libgomp/config.h.in @@ -51,9 +51,6 @@ /* Define to 1
Re: libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'
On 4/28/22 15:45, Thomas Schwinge wrote: Hi Tom! On 2022-04-08T09:35:44+0200, Tom de Vries wrote: On 4/8/22 00:27, Thomas Schwinge wrote: On 2017-01-13T19:11:23+0100, Jakub Jelinek wrote: Especially for distributions it is undesirable to need to have proprietary CUDA libraries and headers installed when building GCC. --- libgomp/plugin/configfrag.ac.jj 2017-01-13 12:07:56.0 +0100 +++ libgomp/plugin/configfrag.ac 2017-01-13 17:33:26.608240936 +0100 + PLUGIN_NVPTX_CPPFLAGS='-I$(srcdir)/plugin/cuda' + PLUGIN_NVPTX_LIBS='-ldl' + PLUGIN_NVPTX_DYNAMIC=1 +AC_DEFINE_UNQUOTED([PLUGIN_NVPTX_DYNAMIC], [$PLUGIN_NVPTX_DYNAMIC], + [Define to 1 if the NVIDIA plugin should dlopen libcuda.so.1, 0 if it should be linked against it.]) Actually, the conditionals leading to 'PLUGIN_NVPTX_DYNAMIC=1' here do control two orthogonal aspects; OK to disentangle that with the attached "libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'"? we discussed dropping --with-cuda, so do I understand it correctly that you now propose to drop --with-cuda and --with-cuda-driver-lib but intend to keep --with-cuda-driver-include ? No, I think you're reading too much into this first patch. ;-) The goal with this patch is just to help disentangle two orthogonal concepts (as described in the commit log), and then... Can you explain what user or maintainer scenario is served by this? ... in a next step, we may indeed remove the current user-visible '--with-cuda-driver' etc., but keep the underlying functionality available for the developers. That's to address the point you'd made in the "Proposal to remove '--with-cuda-driver'" thread: that it still "could be useful for debugging / comparison purposes" -- and especially for development purposes, in my opinion: if you develop CUDA API-level changes in the libgomp nvptx plugin, it's likely to be easier to just use the full CUDA toolkit 'cuda.h' and directly link against libcuda (so that you've got all symbols etc. available), and only once you know what exactly you need, update GCC's 'include/cuda/cuda.h' and 'libgomp/plugin/cuda-lib.def'. With that hopefully clarified, OK to push the re-attached "libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'"? Ack, understood, thanks for the detailed explanation. LGTM. Thanks, - Tom Is there a problem with using gcc's cuda.h? No, all good. Grüße Thomas - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
[PING^2] libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'
Hi! Another ping -- Jakub maybe? This is a simple refactor; no change in behavior. I'll soon post further patches depending on this. Grüße Thomas On 2022-05-05T21:18:47+0200, I wrote: > Hi! > > Ping. > > > Grüße > Thomas > > > On 2022-04-28T15:45:20+0200, I wrote: >> Hi Tom! >> >> On 2022-04-08T09:35:44+0200, Tom de Vries wrote: >>> On 4/8/22 00:27, Thomas Schwinge wrote: On 2017-01-13T19:11:23+0100, Jakub Jelinek wrote: > Especially for distributions it is undesirable to need to have proprietary > CUDA libraries and headers installed when building GCC. > --- libgomp/plugin/configfrag.ac.jj 2017-01-13 12:07:56.0 +0100 > +++ libgomp/plugin/configfrag.ac 2017-01-13 17:33:26.608240936 +0100 > + PLUGIN_NVPTX_CPPFLAGS='-I$(srcdir)/plugin/cuda' > + PLUGIN_NVPTX_LIBS='-ldl' > + PLUGIN_NVPTX_DYNAMIC=1 > +AC_DEFINE_UNQUOTED([PLUGIN_NVPTX_DYNAMIC], [$PLUGIN_NVPTX_DYNAMIC], > + [Define to 1 if the NVIDIA plugin should dlopen libcuda.so.1, 0 if it > should be linked against it.]) Actually, the conditionals leading to 'PLUGIN_NVPTX_DYNAMIC=1' here do control two orthogonal aspects; OK to disentangle that with the attached "libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'"? >> >>> we discussed dropping --with-cuda, so do I understand it correctly that >>> you now propose to drop --with-cuda and --with-cuda-driver-lib but >>> intend to keep --with-cuda-driver-include ? >> >> No, I think you're reading too much into this first patch. ;-) >> >> The goal with this patch is just to help disentangle two orthogonal >> concepts (as described in the commit log), and then... >> >>> Can you explain what user or maintainer scenario is served by this? >> >> ... in a next step, we may indeed remove the current user-visible >> '--with-cuda-driver' etc., but keep the underlying functionality >> available for the developers. That's to address the point you'd made in >> the "Proposal to remove '--with-cuda-driver'" thread: that it still >> "could be useful for debugging / comparison purposes" -- and especially >> for development purposes, in my opinion: if you develop CUDA API-level >> changes in the libgomp nvptx plugin, it's likely to be easier to just use >> the full CUDA toolkit 'cuda.h' and directly link against libcuda (so that >> you've got all symbols etc. available), and only once you know what >> exactly you need, update GCC's 'include/cuda/cuda.h' and >> 'libgomp/plugin/cuda-lib.def'. >> >> With that hopefully clarified, OK to push the re-attached >> "libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into >> 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'"? >> >>> Is >>> there a problem with using gcc's cuda.h? >> >> No, all good. >> >> >> Grüße >> Thomas - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 >From 72e5a2271348fe167713dd3b2afbcd988274bf7c Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Thu, 7 Apr 2022 23:10:16 +0200 Subject: [PATCH] libgomp nvptx plugin: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA' Including the GCC-shipped 'include/cuda/cuda.h' vs. system and 'dlopen'ing the CUDA Driver library vs. linking it are separate concerns. libgomp/ * plugin/Makefrag.am: Handle 'PLUGIN_NVPTX_DYNAMIC'. * plugin/configfrag.ac (PLUGIN_NVPTX_DYNAMIC): Change 'AC_DEFINE_UNQUOTED' into 'AM_CONDITIONAL'. * plugin/plugin-nvptx.c: Split 'PLUGIN_NVPTX_DYNAMIC' into 'PLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H' and 'PLUGIN_NVPTX_LINK_LIBCUDA'. * Makefile.in: Regenerate. * config.h.in: Likewise. * configure: Likewise. --- libgomp/Makefile.in | 26 +++--- libgomp/config.h.in | 4 libgomp/configure | 21 +++-- libgomp/plugin/Makefrag.am| 16 +++- libgomp/plugin/configfrag.ac | 3 +-- libgomp/plugin/plugin-nvptx.c | 4 ++-- 6 files changed, 52 insertions(+), 22 deletions(-) diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in index 1d55f4b65e2..51252b89462 100644 --- a/libgomp/Makefile.in +++ b/libgomp/Makefile.in @@ -119,8 +119,16 @@ build_triplet = @build@ host_triplet = @host@ target_triplet = @target@ @PLUGIN_NVPTX_TRUE@am__append_1 = libgomp-plugin-nvptx.la -@PLUGIN_GCN_TRUE@am__append_2 = libgomp-plugin-gcn.la -@USE_FORTRAN_TRUE@am__append_3 = openacc.f90 + +# Including the GCC-shipped 'include/cuda/cuda.h' vs. system . +@PLUGIN_NVPTX_DYNAMIC_FALSE@@PLUGIN_NVPTX_TRUE@am__append_2 = -DPLUGIN_NVPTX_INCLUDE_SYSTEM_CUDA_H \ +@PLUGIN_NVPTX_DYNAMIC_FALSE@@PLUGIN_NVPTX_TRUE@ -DPLUGIN_NVPTX_LINK_LIBCUDA + +# 'dlopen'ing
libgomp plugins: Don't 'AC_SUBST' and 'AC_DEFINE_UNQUOTED' for 'PLUGIN_GCN', 'PLUGIN_NVPTX' (was: [PATCH 7/10] OpenACC 2.0 support for libgomp - OpenACC runtime, NVidia PTX/CUDA plugin)
Hi! On 2014-09-23T19:19:31+0100, Julian Brown wrote: > This patch contains the bulk of the OpenACC 2.0 runtime support, > building around, or on top of, the OpenMP 4.0 support (as previously > posted or already extant upstream) where we could. [...] > --- a/libgomp/Makefile.am > +++ b/libgomp/Makefile.am > +if PLUGIN_NVPTX For this 'if' here (later moved into 'libgomp/plugin/Makefrag.am'), we do need the 'AM_CONDITIONAL'... > --- a/libgomp/Makefile.in > +++ b/libgomp/Makefile.in > +PLUGIN_NVPTX = @PLUGIN_NVPTX@ ..., but this here (and similar elsewhere) due to 'AC_SUBST'... > --- a/libgomp/config.h.in > +++ b/libgomp/config.h.in > +/* Define to 1 if the NVIDIA plugin is built, 0 if not. */ > +#undef PLUGIN_NVPTX ..., and this here due to 'AC_DEFINE_UNQUOTED' have always been unused, so we may clean those up: > --- a/libgomp/configure.ac > +++ b/libgomp/configure.ac > +AC_SUBST(PLUGIN_NVPTX) > +AM_CONDITIONAL([PLUGIN_NVPTX], [test $PLUGIN_NVPTX = 1]) > +AC_DEFINE_UNQUOTED([PLUGIN_NVPTX], [$PLUGIN_NVPTX], > + [Define to 1 if the NVIDIA plugin is built, 0 if not.]) Later also cargo-culted for other libgomp plugins, where the same "unused" reasoning applies likewise. Pushed to master branch commit edbd2b1caaa79d68467418a4571c3b09f9602805 "libgomp plugins: Don't 'AC_SUBST' and 'AC_DEFINE_UNQUOTED' for 'PLUGIN_GCN', 'PLUGIN_NVPTX'", see attached. Grüße Thomas - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 >From edbd2b1caaa79d68467418a4571c3b09f9602805 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Thu, 5 May 2022 23:10:23 +0200 Subject: [PATCH] libgomp plugins: Don't 'AC_SUBST' and 'AC_DEFINE_UNQUOTED' for 'PLUGIN_GCN', 'PLUGIN_NVPTX' Nothing ever used these. libgomp/ * plugin/configfrag.ac: Don't 'AC_SUBST' and 'AC_DEFINE_UNQUOTED' for 'PLUGIN_GCN', 'PLUGIN_NVPTX'. * Makefile.in: Regenerate. * config.h.in: Likewise. * configure: Likewise. * testsuite/Makefile.in: Likewise. --- libgomp/Makefile.in | 2 -- libgomp/config.h.in | 6 -- libgomp/configure | 18 ++ libgomp/plugin/configfrag.ac | 6 -- libgomp/testsuite/Makefile.in | 2 -- 5 files changed, 2 insertions(+), 32 deletions(-) diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in index f2712aa5133..1d55f4b65e2 100644 --- a/libgomp/Makefile.in +++ b/libgomp/Makefile.in @@ -431,9 +431,7 @@ PACKAGE_URL = @PACKAGE_URL@ PACKAGE_VERSION = @PACKAGE_VERSION@ PATH_SEPARATOR = @PATH_SEPARATOR@ PERL = @PERL@ -PLUGIN_GCN = @PLUGIN_GCN@ PLUGIN_GCN_LIBS = @PLUGIN_GCN_LIBS@ -PLUGIN_NVPTX = @PLUGIN_NVPTX@ PLUGIN_NVPTX_CPPFLAGS = @PLUGIN_NVPTX_CPPFLAGS@ PLUGIN_NVPTX_LDFLAGS = @PLUGIN_NVPTX_LDFLAGS@ PLUGIN_NVPTX_LIBS = @PLUGIN_NVPTX_LIBS@ diff --git a/libgomp/config.h.in b/libgomp/config.h.in index e702625ab6e..5611ed925ad 100644 --- a/libgomp/config.h.in +++ b/libgomp/config.h.in @@ -170,12 +170,6 @@ /* Define to the version of this package. */ #undef PACKAGE_VERSION -/* Define to 1 if the GCN plugin is built, 0 if not. */ -#undef PLUGIN_GCN - -/* Define to 1 if the NVIDIA plugin is built, 0 if not. */ -#undef PLUGIN_NVPTX - /* Define to 1 if the NVIDIA plugin should dlopen libcuda.so.1, 0 if it should be linked against it. */ #undef PLUGIN_NVPTX_DYNAMIC diff --git a/libgomp/configure b/libgomp/configure index 3de8eb2641f..be675a6b8ab 100755 --- a/libgomp/configure +++ b/libgomp/configure @@ -674,11 +674,9 @@ offload_additional_options offload_targets offload_plugins PLUGIN_GCN_LIBS -PLUGIN_GCN PLUGIN_NVPTX_LIBS PLUGIN_NVPTX_LDFLAGS PLUGIN_NVPTX_CPPFLAGS -PLUGIN_NVPTX CUDA_DRIVER_LIB CUDA_DRIVER_INCLUDE libtool_VERSION @@ -11414,7 +11412,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11417 "configure" +#line 11415 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -11520,7 +11518,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11523 "configure" +#line 11521 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -15220,12 +15218,10 @@ PLUGIN_NVPTX_DYNAMIC=0 - PLUGIN_GCN=0 PLUGIN_GCN_LIBS= - # Parse '--enable-offload-targets', figure out the corresponding libgomp # plugins, and configure to find the corresponding offload compilers. # 'offload_plugins' and 'offload_targets' will be populated in the same order. @@ -15373,11 +15369,6 @@ else fi -cat >>confdefs.h <<_ACEOF -#define PLUGIN_NVPTX $PLUGIN_NVPTX -_ACEOF - - cat >>confdefs.h <<_ACEOF #define PLUGIN_NVPTX_DYNAMIC $PLUGIN_NVPTX_DYNAMIC _ACEOF @@ -15391,11 +15382,6 @@ else fi -cat >>confdefs.h <<_ACEOF -#define PLUGIN_GCN $PLUGIN_GCN -_ACEOF - - # Check
[PATCH] tree-optimization/105562 - avoid uninit diagnostic with better FRE
We can avoid some uninit diagnostics by making FRE disambiguate
against CLOBBERs since any aliasing there would invoke undefined
behavior for a read we are looking up.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed
2022-05-12 Richard Biener
PR tree-optimization/105562
* tree-ssa-sccvn.cc (vn_reference_lookup_3): Disambiguate
against all CLOBBER defs if there's not an obvious must-alias
and we are not doing redundant store elimination.
(vn_walk_cb_data::redundant_store_removal_p): New field.
(vn_reference_lookup_pieces): Initialize it.
(vn_reference_lookup): Add argument to specify if we are
doing redundant store removal.
(eliminate_dom_walker::eliminate_stmt): Specify we do.
* tree-ssa-sccvn.h (vn_reference_lookup): Adjust.
* g++.dg/warn/uninit-pr105562.C: New testcase.
---
gcc/testsuite/g++.dg/warn/uninit-pr105562.C | 10 ++
gcc/tree-ssa-sccvn.cc | 37 -
gcc/tree-ssa-sccvn.h| 2 +-
3 files changed, 40 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/uninit-pr105562.C
diff --git a/gcc/testsuite/g++.dg/warn/uninit-pr105562.C
b/gcc/testsuite/g++.dg/warn/uninit-pr105562.C
new file mode 100644
index 000..ec3a5503c80
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/uninit-pr105562.C
@@ -0,0 +1,10 @@
+// { dg-require-effective-target c++11 }
+// { dg-options "-O -Wall -fno-strict-aliasing" }
+
+#include
+
+int main()
+{
+ std::regex a(".");
+ std::regex b(std::move(a));
+}
diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 5cb3dc2ab3a..ed68557f0b2 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -1799,11 +1799,13 @@ struct pd_data
struct vn_walk_cb_data
{
vn_walk_cb_data (vn_reference_t vr_, tree orig_ref_, tree *last_vuse_ptr_,
- vn_lookup_kind vn_walk_kind_, bool tbaa_p_, tree mask_)
+ vn_lookup_kind vn_walk_kind_, bool tbaa_p_, tree mask_,
+ bool redundant_store_removal_p_)
: vr (vr_), last_vuse_ptr (last_vuse_ptr_), last_vuse (NULL_TREE),
mask (mask_), masked_result (NULL_TREE), vn_walk_kind (vn_walk_kind_),
- tbaa_p (tbaa_p_), saved_operands (vNULL), first_set (-2),
- first_base_set (-2), known_ranges (NULL)
+ tbaa_p (tbaa_p_), redundant_store_removal_p (redundant_store_removal_p_),
+ saved_operands (vNULL), first_set (-2), first_base_set (-2),
+ known_ranges (NULL)
{
if (!last_vuse_ptr)
last_vuse_ptr = _vuse;
@@ -1862,6 +1864,7 @@ struct vn_walk_cb_data
tree masked_result;
vn_lookup_kind vn_walk_kind;
bool tbaa_p;
+ bool redundant_store_removal_p;
vec saved_operands;
/* The VDEFs of partial defs we come along. */
@@ -2620,6 +2623,19 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void
*data_,
return NULL;
}
+ /* When the def is a CLOBBER we can optimistically disambiguate
+against it since any overlap it would be undefined behavior.
+Avoid this for obvious must aliases to save compile-time though.
+We also may not do this when the query is used for redundant
+store removal. */
+ if (!data->redundant_store_removal_p
+ && gimple_clobber_p (def_stmt)
+ && !operand_equal_p (ao_ref_base (_ref), base, OEP_ADDRESS_OF))
+ {
+ *disambiguate_only = TR_DISAMBIGUATE;
+ return NULL;
+ }
+
/* Besides valueizing the LHS we can also use access-path based
disambiguation on the original non-valueized ref. */
if (!ref->ref
@@ -3604,7 +3620,8 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set,
{
ao_ref r;
unsigned limit = param_sccvn_max_alias_queries_per_access;
- vn_walk_cb_data data (, NULL_TREE, NULL, kind, true, NULL_TREE);
+ vn_walk_cb_data data (, NULL_TREE, NULL, kind, true, NULL_TREE,
+ false);
vec ops_for_ref;
if (!valueized_p)
ops_for_ref = vr1.operands;
@@ -3649,12 +3666,14 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type
set,
MASK is either NULL_TREE, or can be an INTEGER_CST if the result of the
load is bitwise anded with MASK and so we are only interested in a subset
of the bits and can ignore if the other bits are uninitialized or
- not initialized with constants. */
+ not initialized with constants. When doing redundant store removal
+ the caller has to set REDUNDANT_STORE_REMOVAL_P. */
tree
vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
vn_reference_t *vnresult, bool tbaa_p,
-tree *last_vuse_ptr, tree mask)
+tree *last_vuse_ptr, tree mask,
+bool redundant_store_removal_p)
{
vec operands;
struct vn_reference_s vr1;
@@ -3732,7 +3751,8 @@ vn_reference_lookup (tree op, tree vuse,
Re: [PATCH v2] Strip of a vector load which is only used partially.
On Thu, May 12, 2022 at 3:48 AM Hongtao Liu wrote:
>
> On Tue, May 10, 2022 at 2:54 PM Richard Biener via Gcc-patches
> wrote:
> >
> > On Mon, May 9, 2022 at 7:11 AM liuhongt via Gcc-patches
> > wrote:
> > >
> > > Here's adjused patch.
> > > Ok for trunk?
> > >
> > > Optimize
> > >
> > > _4 = VEC_PERM_EXPR <_1, _1, { 4, 5, 6, 7, 4, 5, 6, 7 }>;
> > > _5 = BIT_FIELD_REF <_4, 128, 0>;
> > >
> > > to
> > >
> > > _5 = BIT_FIELD_REF <_1, 128, 128>;
> > >
> > > gcc/ChangeLog:
> > >
> > > PR tree-optimization/102583
> > > * tree-ssa-forwprop.cc (simplify_bitfield_ref): Extended to a
> > > contiguous stride in the VEC_PERM_EXPR.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/i386/pr102583.c: New test.
> > > * gcc.target/i386/pr92645-2.c: Adjust testcase.
> > > * gcc.target/i386/pr92645-3.c: Ditto.
> > > ---
> > > gcc/testsuite/gcc.target/i386/pr102583.c | 30
> > > gcc/testsuite/gcc.target/i386/pr92645-2.c | 4 +-
> > > gcc/testsuite/gcc.target/i386/pr92645-3.c | 4 +-
> > > gcc/tree-ssa-forwprop.cc | 89 ---
> > > 4 files changed, 96 insertions(+), 31 deletions(-)
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102583.c
> > >
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr102583.c
> > > b/gcc/testsuite/gcc.target/i386/pr102583.c
> > > new file mode 100644
> > > index 000..4ef2f296d0c
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr102583.c
> > > @@ -0,0 +1,30 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-mavx512f -O2" } */
> > > +/* { dg-final { scan-assembler-times {(?n)vcvtdq2ps[ \t]+32\(%.*%ymm} 1
> > > } } */
> > > +/* { dg-final { scan-assembler-times {(?n)vcvtdq2ps[ \t]+16\(%.*%xmm} 1
> > > } } */
> > > +/* { dg-final { scan-assembler-times {(?n)vmovq[ \t]+16\(%.*%xmm} 1 {
> > > target { ! ia32 } } } } */
> > > +/* { dg-final { scan-assembler-not {(?n)vpermd[ \t]+.*%zmm} } } */
> > > +
> > > +typedef int v16si __attribute__((vector_size(64)));
> > > +typedef float v8sf __attribute__((vector_size(32)));
> > > +typedef float v4sf __attribute__((vector_size(16)));
> > > +typedef float v2sf __attribute__((vector_size(8)));
> > > +
> > > +v8sf part (v16si *srcp)
> > > +{
> > > + v16si src = *srcp;
> > > + return (v8sf) { (float)src[8], (float) src[9], (float)src[10],
> > > (float)src[11],
> > > + (float)src[12], (float)src[13], (float)src[14], (float)src[15] };
> > > +}
> > > +
> > > +v4sf part1 (v16si *srcp)
> > > +{
> > > + v16si src = *srcp;
> > > + return (v4sf) { (float)src[4], (float)src[5], (float)src[6],
> > > (float)src[7] };
> > > +}
> > > +
> > > +v2sf part2 (v16si *srcp)
> > > +{
> > > + v16si src = *srcp;
> > > + return (v2sf) { (float)src[4], (float)src[5] };
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr92645-2.c
> > > b/gcc/testsuite/gcc.target/i386/pr92645-2.c
> > > index d34ed3aa8e5..f0608de938a 100644
> > > --- a/gcc/testsuite/gcc.target/i386/pr92645-2.c
> > > +++ b/gcc/testsuite/gcc.target/i386/pr92645-2.c
> > > @@ -29,6 +29,6 @@ void odd (v2si *dst, v4si *srcp)
> > > }
> > >
> > > /* { dg-final { scan-tree-dump-times "BIT_FIELD_REF" 4 "cddce1" } } */
> > > -/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 3 "cddce1" } } */
> > > +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 3 "cddce1" { xfail
> > > *-*-* } } } */
> > > /* Ideally highpart extraction would elide the permutation as well. */
> > > -/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "cddce1" { xfail
> > > *-*-* } } } */
> > > +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "cddce1" } } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr92645-3.c
> > > b/gcc/testsuite/gcc.target/i386/pr92645-3.c
> > > index 9c08c9fb632..691011195c9 100644
> > > --- a/gcc/testsuite/gcc.target/i386/pr92645-3.c
> > > +++ b/gcc/testsuite/gcc.target/i386/pr92645-3.c
> > > @@ -32,6 +32,6 @@ void odd (v4sf *dst, v8si *srcp)
> > > /* Four conversions, on the smaller vector type, to not convert excess
> > > elements. */
> > > /* { dg-final { scan-tree-dump-times " = \\\(vector\\\(4\\\) float\\\)"
> > > 4 "cddce1" } } */
> > > -/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 3 "cddce1" } } */
> > > +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 3 "cddce1" { xfail
> > > *-*-* } } } */
> > > /* Ideally highpart extraction would elide the VEC_PERM_EXPR as well. */
> > > -/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "cddce1" { xfail
> > > *-*-* } } } */
> > > +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "cddce1" } } */
> > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
> > > index 484491fa1c5..f91f738895d 100644
> > > --- a/gcc/tree-ssa-forwprop.cc
> > > +++ b/gcc/tree-ssa-forwprop.cc
> > > @@ -2334,8 +2334,10 @@ simplify_bitfield_ref (gimple_stmt_iterator *gsi)
> > >gimple *stmt = gsi_stmt (*gsi);
> > >gimple *def_stmt;
> > >tree op, op0, op1;
> > >
c++: Support module language-decl semantics
In modules purview, one can attach a declaration to the global module
(i.e. not the named module in whence the declaration appears), using a
language declaration:
export module Foo;
extern "C++" void *operator new (std::size_t);
This implements those semantics.
Jonathan, I guess this might help with modularizing std?
nathan
--
Nathan SidwellFrom 78c8b0b980341f28de96da518a38bf85bbd24d98 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 9 May 2022 04:51:32 -0700
Subject: [PATCH] c++: Support module language-decl semantics
In modules purview, one can attach a declaration to the global module
(i.e. not the named module in whence the declaration appears), using a
language declaration:
export module Foo;
extern "C++" void *operator new (std::size_t);
This implements those semantics.
gcc/cp/
* parser.cc (cp_parser_linkage_specification): Implement
global module attachment semantics.
gcc/testsuite/
* g++.dg/modules/lang-3_a.C: New.
* g++.dg/modules/lang-3_b.C: New.
* g++.dg/modules/lang-3_c.C: New.
---
gcc/cp/parser.cc| 3 +++
gcc/testsuite/g++.dg/modules/lang-3_a.C | 17 +
gcc/testsuite/g++.dg/modules/lang-3_b.C | 18 ++
gcc/testsuite/g++.dg/modules/lang-3_c.C | 12
4 files changed, 50 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/lang-3_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/lang-3_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/lang-3_c.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 84f379c7bff..8969ed0076a 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -16189,6 +16189,8 @@ cp_parser_linkage_specification (cp_parser* parser, tree prefix_attr)
linkage = get_identifier (TREE_STRING_POINTER (linkage));
/* We're now using the new linkage. */
+ unsigned saved_module = module_kind;
+ module_kind &= ~MK_ATTACH;
push_lang_context (linkage);
/* Preserve the location of the innermost linkage specification,
@@ -16235,6 +16237,7 @@ cp_parser_linkage_specification (cp_parser* parser, tree prefix_attr)
/* We're done with the linkage-specification. */
pop_lang_context ();
+ module_kind = saved_module;
/* Restore location of parent linkage specification, if any. */
parser->innermost_linkage_specification_location = saved_location;
diff --git a/gcc/testsuite/g++.dg/modules/lang-3_a.C b/gcc/testsuite/g++.dg/modules/lang-3_a.C
new file mode 100644
index 000..1c6fa1fa2e9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lang-3_a.C
@@ -0,0 +1,17 @@
+// { dg-additional-options "-fmodules-ts -Wno-pedantic" }
+module;
+# 4 __FILE__ 1
+void Quux ();
+# 6 "" 2
+export module bob;
+// { dg-module-cmi bob }
+
+extern "C++"
+{
+export void Bar () {}
+export void Quux ();
+void Baz () {}
+}
+
+// { dg-final { scan-assembler {_Z3Barv:} } }
+// { dg-final { scan-assembler {_Z3Bazv:} } }
diff --git a/gcc/testsuite/g++.dg/modules/lang-3_b.C b/gcc/testsuite/g++.dg/modules/lang-3_b.C
new file mode 100644
index 000..17300ece699
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lang-3_b.C
@@ -0,0 +1,18 @@
+// { dg-additional-options -fmodules-ts }
+import bob;
+
+void Foo ()
+{
+ Bar ();
+ Baz (); // { dg-error "was not declared" }
+ Quux ();
+}
+
+void Bar ();
+void Baz ();
+
+void Quux ()
+{
+ Bar ();
+ Baz ();
+}
diff --git a/gcc/testsuite/g++.dg/modules/lang-3_c.C b/gcc/testsuite/g++.dg/modules/lang-3_c.C
new file mode 100644
index 000..ca18db72ad8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lang-3_c.C
@@ -0,0 +1,12 @@
+// { dg-additional-options -fmodules-ts }
+module bob;
+
+void Foo ()
+{
+ Bar ();
+ Baz ();
+}
+
+extern "C++" void Bar ();
+extern "C++" void Baz ();
+
--
2.30.2
Re: [1/2] PR96463 - aarch64 specific changes
Prathamesh Kulkarni writes:
> On Wed, 11 May 2022 at 12:44, Richard Sandiford
> wrote:
>>
>> Prathamesh Kulkarni writes:
>> > On Fri, 6 May 2022 at 16:00, Richard Sandiford
>> > wrote:
>> >>
>> >> Prathamesh Kulkarni writes:
>> >> > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>> >> > b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>> >> > index c24c0548724..1ef4ea2087b 100644
>> >> > --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>> >> > +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>> >> > @@ -44,6 +44,14 @@
>> >> > #include "aarch64-sve-builtins-shapes.h"
>> >> > #include "aarch64-sve-builtins-base.h"
>> >> > #include "aarch64-sve-builtins-functions.h"
>> >> > +#include "aarch64-builtins.h"
>> >> > +#include "gimple-ssa.h"
>> >> > +#include "tree-phinodes.h"
>> >> > +#include "tree-ssa-operands.h"
>> >> > +#include "ssa-iterators.h"
>> >> > +#include "stringpool.h"
>> >> > +#include "value-range.h"
>> >> > +#include "tree-ssanames.h"
>> >>
>> >> Minor, but: I think the preferred approach is to include "ssa.h"
>> >> rather than include some of these headers directly.
>> >>
>> >> >
>> >> > using namespace aarch64_sve;
>> >> >
>> >> > @@ -1207,6 +1215,56 @@ public:
>> >> > insn_code icode = code_for_aarch64_sve_ld1rq (e.vector_mode (0));
>> >> > return e.use_contiguous_load_insn (icode);
>> >> >}
>> >> > +
>> >> > + gimple *
>> >> > + fold (gimple_folder ) const OVERRIDE
>> >> > + {
>> >> > +tree arg0 = gimple_call_arg (f.call, 0);
>> >> > +tree arg1 = gimple_call_arg (f.call, 1);
>> >> > +
>> >> > +/* Transform:
>> >> > + lhs = svld1rq ({-1, -1, ... }, arg1)
>> >> > + into:
>> >> > + tmp = mem_ref [(int * {ref-all}) arg1]
>> >> > + lhs = vec_perm_expr.
>> >> > + on little endian target. */
>> >> > +
>> >> > +if (!BYTES_BIG_ENDIAN
>> >> > + && integer_all_onesp (arg0))
>> >> > + {
>> >> > + tree lhs = gimple_call_lhs (f.call);
>> >> > + auto simd_type = aarch64_get_simd_info_for_type (Int32x4_t);
>> >>
>> >> Does this work for other element sizes? I would have expected it
>> >> to be the (128-bit) Advanced SIMD vector associated with the same
>> >> element type as the SVE vector.
>> >>
>> >> The testcase should cover more than just int32x4_t -> svint32_t,
>> >> just to be sure.
>> > In the attached patch, it obtains corresponding advsimd type with:
>> >
>> > tree eltype = TREE_TYPE (lhs_type);
>> > unsigned nunits = 128 / TREE_INT_CST_LOW (TYPE_SIZE (eltype));
>> > tree vectype = build_vector_type (eltype, nunits);
>> >
>> > While this seems to work with different element sizes, I am not sure if
>> > it's
>> > the correct approach ?
>>
>> Yeah, that looks correct. Other SVE code uses aarch64_vq_mode
>> to get the vector mode associated with a .Q “element”, so an
>> alternative would be:
>>
>> machine_mode vq_mode = aarch64_vq_mode (TYPE_MODE (eltype)).require ();
>> tree vectype = build_vector_type_for_mode (eltype, vq_mode);
>>
>> which is more explicit about wanting an Advanced SIMD vector.
>>
>> >> > +
>> >> > + tree elt_ptr_type
>> >> > + = build_pointer_type_for_mode (simd_type.eltype, VOIDmode,
>> >> > true);
>> >> > + tree zero = build_zero_cst (elt_ptr_type);
>> >> > +
>> >> > + /* Use element type alignment. */
>> >> > + tree access_type
>> >> > + = build_aligned_type (simd_type.itype, TYPE_ALIGN
>> >> > (simd_type.eltype));
>> >> > +
>> >> > + tree tmp = make_ssa_name_fn (cfun, access_type, 0);
>> >> > + gimple *mem_ref_stmt
>> >> > + = gimple_build_assign (tmp, fold_build2 (MEM_REF, access_type,
>> >> > arg1, zero));
>> >>
>> >> Long line. Might be easier to format by assigning the fold_build2 result
>> >> to a temporary variable.
>> >>
>> >> > + gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT);
>> >> > +
>> >> > + tree mem_ref_lhs = gimple_get_lhs (mem_ref_stmt);
>> >> > + tree vectype = TREE_TYPE (mem_ref_lhs);
>> >> > + tree lhs_type = TREE_TYPE (lhs);
>> >>
>> >> Is this necessary? The code above supplied the types and I wouldn't
>> >> have expected them to change during the build process.
>> >>
>> >> > +
>> >> > + int source_nelts = TYPE_VECTOR_SUBPARTS (vectype).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);
>> >> > +
>> >> > + vec_perm_indices indices (sel, 1, source_nelts);
>> >> > + gcc_checking_assert (can_vec_perm_const_p (TYPE_MODE (lhs_type),
>> >> > indices));
>> >> > + tree mask = vec_perm_indices_to_tree (lhs_type, indices);
>> >> > + return gimple_build_assign (lhs, VEC_PERM_EXPR, mem_ref_lhs,
>> >> > mem_ref_lhs, mask);
>> >>
>> >> Nit: long line.
>> >>
>> >> > + }
>> >> > +
>> >> > +return NULL;
>> >> > + }
>> >> > };
>> >> >
>> >> > class svld1ro_impl : public load_replicate
>> >> > diff
Re: [PATCH] Use more ARRAY_SIZE.
Le 11/05/2022 à 10:17, Martin Liška a écrit : On 5/9/22 14:03, Richard Biener wrote: On Thu, May 5, 2022 at 4:30 PM Martin Liška wrote: On 5/5/22 14:58, Iain Buclaw wrote: This D front-end change doesn't look right to me, besides the slight Hello. Sorry, I've re-read the patch and fixed some places where the macro usage was wrong. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. The middle-end parts are OK. I'd say in files where ARRAY_SIZE is already used it's OK to introduce more uses. Otherwise I defer to the more specific maintainers if they like this or not. All right, CCing the following maintainers for other parts: - David for JIT and Analyzer - Tobias for Fortran part - Jason for C-family part OK for fortran.
[OG11-committed] Fortran: Fix proc pointer as elemental arg handling (was: Fortran: Fix finalization resolution with deep copy (was: [Patch][Stage 1] Fortran/OpenMP: Support mapping of DT with allocat
Found another issue related to polymorphic deep copy (see description in the
commit log).
I plan to start submitting the deep-mapping patches in separate pieces to
mainline later during Stage 1.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
commit ee954d940c41614c75be11b9d1de9079c37044d7
Author: Tobias Burnus
Date: Thu May 12 10:39:58 2022 +0200
Fortran: Fix proc pointer as elemental arg handling
The vtab's _callback function calls the elemental 'cb'
cb (var(:)%comp, comp_types_vtable._callback);
which gets called in a scalarization loop as 'var' might be a
nonscalar. Without the patch, that got translated as:
D.1234 = _types_vtable._callback
...
cb (&(*D.4060)[S.3 + D.4071], );
where 'D.1234' is function_type. With the patch, it remains a pointer;
i.e. D.1234 = comp... and 'cb (..., D.1234)', avoiding ME ICE.
Note: Fortran (F2018, C15100) requires that dummy arguments are
dummy data objects, which rules out dummy procs/proc-pointer dummies,
which is enforced in resolve_fl_procedure.
Thus, this change only affects the internally generated code.
gcc/fortran/ChangeLog:
* trans-array.c (gfc_scalar_elemental_arg_saved_as_reference):
Return true for attr.proc_pointer expressions.
gcc/testsuite/ChangeLog:
* gfortran.dg/finalize_38.f90: Compile with -Ofast.
---
gcc/fortran/trans-array.c | 6 ++
gcc/testsuite/gfortran.dg/finalize_38.f90 | 1 +
2 files changed, 7 insertions(+)
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0d637162509..8e5277594ab 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -3025,6 +3025,12 @@ gfc_scalar_elemental_arg_saved_as_reference (gfc_ss_info * ss_info)
&& gfc_expr_is_variable (ss_info->expr))
return true;
+ /* Proc pointers: avoid creating a non-pointer function temporary;
+ should only get used internally due to constraints. */
+ if (!ss_info->data.scalar.needs_temporary &&
+ gfc_expr_attr (ss_info->expr).proc_pointer)
+return true;
+
/* Otherwise the expression is evaluated to a temporary variable before the
scalarization loop. */
return false;
diff --git a/gcc/testsuite/gfortran.dg/finalize_38.f90 b/gcc/testsuite/gfortran.dg/finalize_38.f90
index 442e1753311..99c288784d6 100644
--- a/gcc/testsuite/gfortran.dg/finalize_38.f90
+++ b/gcc/testsuite/gfortran.dg/finalize_38.f90
@@ -1,4 +1,5 @@
! { dg-do compile }
+! { dg-additional-options "-Ofast" }
!
! Check finalization
!
RE: [PATCH] [i386]Add combine splitter to transform pxor/pcmpeqb/pmovmskb/cmp 0xffff to ptest.
> -Original Message-
> From: Uros Bizjak
> Sent: Thursday, May 12, 2022 5:12 PM
> To: Jiang, Haochen
> Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao ; Hongyu
> Wang
> Subject: Re: [PATCH] [i386]Add combine splitter to transform
> pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
>
> On Thu, May 12, 2022 at 5:01 AM Jiang, Haochen
> wrote:
> >
> > Hi all,
> >
> > I just refined this patch with more explanation in commit message.
>
> The ChangeLog entry should in fact read as:
>
> PR target/104371
> * config/i386/sse.md (vi1avx2const): New define_mode_attr.
> (pxor/pcmpeqb/pmovmskb/cmp 0x to ptest splitter):
> New define_split pattern.
Fixed ChangeLog with this.
Thx,
Haochen
>
> Please see [1].
>
> [1] https://www.gnu.org/prep/standards/html_node/Change-
> Logs.html#Change-Logs
>
> OK with the fixed ChangeLog.
>
> Uros.
>
> > No code change compare to last change, which removed ix86_match_ccmode.
> >
> > Ok for trunk?
> >
> > BRs,
> > Haochen
> >
> > > -Original Message-
> > > From: Jiang, Haochen
> > > Sent: Saturday, May 7, 2022 9:55 AM
> > > To: Uros Bizjak
> > > Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao
> > > Subject: RE: [PATCH] [i386]Add combine splitter to transform
> > > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Uros Bizjak
> > > > Sent: Friday, May 6, 2022 4:59 PM
> > > > To: Jiang, Haochen
> > > > Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao
> > > > Subject: Re: [PATCH] [i386]Add combine splitter to transform
> > > > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > > >
> > > > On Fri, May 6, 2022 at 10:01 AM Haochen Jiang
> > > >
> > > > wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > This patch aims to add a combine splitter to transform
> > > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > > > >
> > > > > Regtested on x86_64-pc-linux-gnu. Ok for trunk?
> > > > >
> > > > > BRs,
> > > > > Haochen
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > PR target/104371
> > > > > * config/i386/sse.md: Add new define_mode_attr and
> > > > > define_split.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > PR target/104371
> > > > > * gcc.target/i386/pr104371-1.c: New test.
> > > > > * gcc.target/i386/pr104371-2.c: Ditto.
> > > > > ---
> > > > > gcc/config/i386/sse.md | 19 +++
> > > > > gcc/testsuite/gcc.target/i386/pr104371-1.c | 14 ++
> > > > > gcc/testsuite/gcc.target/i386/pr104371-2.c | 14 ++
> > > > > 3 files changed, 47 insertions(+) create mode 100644
> > > > > gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > > create mode 100755 gcc/testsuite/gcc.target/i386/pr104371-2.c
> > > > >
> > > > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> > > > > index 7b791def542..71afda73c8f 100644
> > > > > --- a/gcc/config/i386/sse.md
> > > > > +++ b/gcc/config/i386/sse.md
> > > > > @@ -20083,6 +20083,25 @@
> > > > > (set_attr "prefix" "maybe_vex")
> > > > > (set_attr "mode" "SI")])
> > > > >
> > > > > +;; Optimize pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > > > > +(define_mode_attr vi1avx2const
> > > > > + [(V32QI "0x") (V16QI "0x")])
> > > > > +
> > > > > +(define_split
> > > > > + [(set (reg:CCZ FLAGS_REG)
> > > > > + (compare:CCZ (unspec:SI
> > > > > + [(eq:VI1_AVX2
> > > > > + (match_operand:VI1_AVX2 0
> > > > > "vector_operand")
> > > > > + (match_operand:VI1_AVX2 1
> > > > > "const0_operand"))]
> > > > > + UNSPEC_MOVMSK)
> > > > > +(match_operand 2 "const_int_operand")))]
> > > > > + "TARGET_SSE4_1 && ix86_match_ccmode (insn, CCmode)
> > > >
> > > > No need to use ix86_match_ccmode here, the pattern is already
> > > > limited to CCZmode,
> > > >
> > > > Uros.
> > > >
> > >
> > > Removed this condition in my new patch, also make the testcase
> > > change according to Hongyu's review.
> > >
> > > Is the patch Ok for trunk?
> > >
> > > Haochen
> > >
> > > > > + && (INTVAL (operands[2]) == (int) ())"
> > > > > + [(set (reg:CC FLAGS_REG)
> > > > > + (unspec:CC [(match_dup 0)
> > > > > + (match_dup 0)]
> > > > > + UNSPEC_PTEST))])
> > > > > +
> > > > > (define_expand "sse2_maskmovdqu"
> > > > >[(set (match_operand:V16QI 0 "memory_operand")
> > > > > (unspec:V16QI [(match_operand:V16QI 1
> > > > > "register_operand") diff --git
> > > > > a/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > > b/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > > new file mode 100644
> > > > > index 000..df7c0b074e3
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > > @@ -0,0 +1,14 @@
> > > > > +/* { dg-do compile } */
> > > > > +/* { dg-options "-O2 -msse4" } */
> > > > > +/* { dg-final { scan-assembler "ptest\[ \\t\]" } } */
> > > > >
Re: [1/2] PR96463 - aarch64 specific changes
On Wed, 11 May 2022 at 12:44, Richard Sandiford
wrote:
>
> Prathamesh Kulkarni writes:
> > On Fri, 6 May 2022 at 16:00, Richard Sandiford
> > wrote:
> >>
> >> Prathamesh Kulkarni writes:
> >> > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >> > b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >> > index c24c0548724..1ef4ea2087b 100644
> >> > --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >> > +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >> > @@ -44,6 +44,14 @@
> >> > #include "aarch64-sve-builtins-shapes.h"
> >> > #include "aarch64-sve-builtins-base.h"
> >> > #include "aarch64-sve-builtins-functions.h"
> >> > +#include "aarch64-builtins.h"
> >> > +#include "gimple-ssa.h"
> >> > +#include "tree-phinodes.h"
> >> > +#include "tree-ssa-operands.h"
> >> > +#include "ssa-iterators.h"
> >> > +#include "stringpool.h"
> >> > +#include "value-range.h"
> >> > +#include "tree-ssanames.h"
> >>
> >> Minor, but: I think the preferred approach is to include "ssa.h"
> >> rather than include some of these headers directly.
> >>
> >> >
> >> > using namespace aarch64_sve;
> >> >
> >> > @@ -1207,6 +1215,56 @@ public:
> >> > insn_code icode = code_for_aarch64_sve_ld1rq (e.vector_mode (0));
> >> > return e.use_contiguous_load_insn (icode);
> >> >}
> >> > +
> >> > + gimple *
> >> > + fold (gimple_folder ) const OVERRIDE
> >> > + {
> >> > +tree arg0 = gimple_call_arg (f.call, 0);
> >> > +tree arg1 = gimple_call_arg (f.call, 1);
> >> > +
> >> > +/* Transform:
> >> > + lhs = svld1rq ({-1, -1, ... }, arg1)
> >> > + into:
> >> > + tmp = mem_ref [(int * {ref-all}) arg1]
> >> > + lhs = vec_perm_expr.
> >> > + on little endian target. */
> >> > +
> >> > +if (!BYTES_BIG_ENDIAN
> >> > + && integer_all_onesp (arg0))
> >> > + {
> >> > + tree lhs = gimple_call_lhs (f.call);
> >> > + auto simd_type = aarch64_get_simd_info_for_type (Int32x4_t);
> >>
> >> Does this work for other element sizes? I would have expected it
> >> to be the (128-bit) Advanced SIMD vector associated with the same
> >> element type as the SVE vector.
> >>
> >> The testcase should cover more than just int32x4_t -> svint32_t,
> >> just to be sure.
> > In the attached patch, it obtains corresponding advsimd type with:
> >
> > tree eltype = TREE_TYPE (lhs_type);
> > unsigned nunits = 128 / TREE_INT_CST_LOW (TYPE_SIZE (eltype));
> > tree vectype = build_vector_type (eltype, nunits);
> >
> > While this seems to work with different element sizes, I am not sure if it's
> > the correct approach ?
>
> Yeah, that looks correct. Other SVE code uses aarch64_vq_mode
> to get the vector mode associated with a .Q “element”, so an
> alternative would be:
>
> machine_mode vq_mode = aarch64_vq_mode (TYPE_MODE (eltype)).require ();
> tree vectype = build_vector_type_for_mode (eltype, vq_mode);
>
> which is more explicit about wanting an Advanced SIMD vector.
>
> >> > +
> >> > + tree elt_ptr_type
> >> > + = build_pointer_type_for_mode (simd_type.eltype, VOIDmode, true);
> >> > + tree zero = build_zero_cst (elt_ptr_type);
> >> > +
> >> > + /* Use element type alignment. */
> >> > + tree access_type
> >> > + = build_aligned_type (simd_type.itype, TYPE_ALIGN
> >> > (simd_type.eltype));
> >> > +
> >> > + tree tmp = make_ssa_name_fn (cfun, access_type, 0);
> >> > + gimple *mem_ref_stmt
> >> > + = gimple_build_assign (tmp, fold_build2 (MEM_REF, access_type,
> >> > arg1, zero));
> >>
> >> Long line. Might be easier to format by assigning the fold_build2 result
> >> to a temporary variable.
> >>
> >> > + gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT);
> >> > +
> >> > + tree mem_ref_lhs = gimple_get_lhs (mem_ref_stmt);
> >> > + tree vectype = TREE_TYPE (mem_ref_lhs);
> >> > + tree lhs_type = TREE_TYPE (lhs);
> >>
> >> Is this necessary? The code above supplied the types and I wouldn't
> >> have expected them to change during the build process.
> >>
> >> > +
> >> > + int source_nelts = TYPE_VECTOR_SUBPARTS (vectype).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);
> >> > +
> >> > + vec_perm_indices indices (sel, 1, source_nelts);
> >> > + gcc_checking_assert (can_vec_perm_const_p (TYPE_MODE (lhs_type),
> >> > indices));
> >> > + tree mask = vec_perm_indices_to_tree (lhs_type, indices);
> >> > + return gimple_build_assign (lhs, VEC_PERM_EXPR, mem_ref_lhs,
> >> > mem_ref_lhs, mask);
> >>
> >> Nit: long line.
> >>
> >> > + }
> >> > +
> >> > +return NULL;
> >> > + }
> >> > };
> >> >
> >> > class svld1ro_impl : public load_replicate
> >> > diff --git a/gcc/config/aarch64/aarch64.cc
> >> > b/gcc/config/aarch64/aarch64.cc
> >> > index f650abbc4ce..47810fec804 100644
> >> > --- a/gcc/config/aarch64/aarch64.cc
> >> > +++
Re: [PATCH] [i386]Add combine splitter to transform pxor/pcmpeqb/pmovmskb/cmp 0xffff to ptest.
On Thu, May 12, 2022 at 5:01 AM Jiang, Haochen wrote:
>
> Hi all,
>
> I just refined this patch with more explanation in commit message.
The ChangeLog entry should in fact read as:
PR target/104371
* config/i386/sse.md (vi1avx2const): New define_mode_attr.
(pxor/pcmpeqb/pmovmskb/cmp 0x to ptest splitter):
New define_split pattern.
Please see [1].
[1] https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs
OK with the fixed ChangeLog.
Uros.
> No code change compare to last change, which removed ix86_match_ccmode.
>
> Ok for trunk?
>
> BRs,
> Haochen
>
> > -Original Message-
> > From: Jiang, Haochen
> > Sent: Saturday, May 7, 2022 9:55 AM
> > To: Uros Bizjak
> > Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao
> > Subject: RE: [PATCH] [i386]Add combine splitter to transform
> > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> >
> >
> >
> > > -Original Message-
> > > From: Uros Bizjak
> > > Sent: Friday, May 6, 2022 4:59 PM
> > > To: Jiang, Haochen
> > > Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao
> > > Subject: Re: [PATCH] [i386]Add combine splitter to transform
> > > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > >
> > > On Fri, May 6, 2022 at 10:01 AM Haochen Jiang
> > > wrote:
> > > >
> > > > Hi all,
> > > >
> > > > This patch aims to add a combine splitter to transform
> > pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > > >
> > > > Regtested on x86_64-pc-linux-gnu. Ok for trunk?
> > > >
> > > > BRs,
> > > > Haochen
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > PR target/104371
> > > > * config/i386/sse.md: Add new define_mode_attr and define_split.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > PR target/104371
> > > > * gcc.target/i386/pr104371-1.c: New test.
> > > > * gcc.target/i386/pr104371-2.c: Ditto.
> > > > ---
> > > > gcc/config/i386/sse.md | 19 +++
> > > > gcc/testsuite/gcc.target/i386/pr104371-1.c | 14 ++
> > > > gcc/testsuite/gcc.target/i386/pr104371-2.c | 14 ++
> > > > 3 files changed, 47 insertions(+)
> > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > create mode 100755 gcc/testsuite/gcc.target/i386/pr104371-2.c
> > > >
> > > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index
> > > > 7b791def542..71afda73c8f 100644
> > > > --- a/gcc/config/i386/sse.md
> > > > +++ b/gcc/config/i386/sse.md
> > > > @@ -20083,6 +20083,25 @@
> > > > (set_attr "prefix" "maybe_vex")
> > > > (set_attr "mode" "SI")])
> > > >
> > > > +;; Optimize pxor/pcmpeqb/pmovmskb/cmp 0x to ptest.
> > > > +(define_mode_attr vi1avx2const
> > > > + [(V32QI "0x") (V16QI "0x")])
> > > > +
> > > > +(define_split
> > > > + [(set (reg:CCZ FLAGS_REG)
> > > > + (compare:CCZ (unspec:SI
> > > > + [(eq:VI1_AVX2
> > > > + (match_operand:VI1_AVX2 0 "vector_operand")
> > > > + (match_operand:VI1_AVX2 1
> > > > "const0_operand"))]
> > > > + UNSPEC_MOVMSK)
> > > > +(match_operand 2 "const_int_operand")))]
> > > > + "TARGET_SSE4_1 && ix86_match_ccmode (insn, CCmode)
> > >
> > > No need to use ix86_match_ccmode here, the pattern is already limited to
> > > CCZmode,
> > >
> > > Uros.
> > >
> >
> > Removed this condition in my new patch, also make the testcase change
> > according to
> > Hongyu's review.
> >
> > Is the patch Ok for trunk?
> >
> > Haochen
> >
> > > > + && (INTVAL (operands[2]) == (int) ())"
> > > > + [(set (reg:CC FLAGS_REG)
> > > > + (unspec:CC [(match_dup 0)
> > > > + (match_dup 0)]
> > > > + UNSPEC_PTEST))])
> > > > +
> > > > (define_expand "sse2_maskmovdqu"
> > > >[(set (match_operand:V16QI 0 "memory_operand")
> > > > (unspec:V16QI [(match_operand:V16QI 1 "register_operand") diff
> > > > --git a/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > b/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > new file mode 100644
> > > > index 000..df7c0b074e3
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/i386/pr104371-1.c
> > > > @@ -0,0 +1,14 @@
> > > > +/* { dg-do compile } */
> > > > +/* { dg-options "-O2 -msse4" } */
> > > > +/* { dg-final { scan-assembler "ptest\[ \\t\]" } } */
> > > > +/* { dg-final { scan-assembler-not "pxor\[ \\t\]" } } */
> > > > +/* { dg-final { scan-assembler-not "pcmpeqb\[ \\t\]" } } */
> > > > +/* { dg-final { scan-assembler-not "pmovmskb\[ \\t\]" } } */
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +bool is_zero(__m128i x)
> > > > +{
> > > > + return _mm_movemask_epi8(_mm_cmpeq_epi8(x, _mm_setzero_si128()))
> > > ==
> > > > +0x; }
> > > > diff --git a/gcc/testsuite/gcc.target/i386/pr104371-2.c
> > > > b/gcc/testsuite/gcc.target/i386/pr104371-2.c
> > > > new file mode 100755
> > > > index 000..f0d0afd5897
> > > > --- /dev/null
> > > > +++
Re: [PATCH] AArch64: Improve address rematerialization costs
Richard Biener writes: > On Wed, May 11, 2022 at 2:23 PM Richard Sandiford via Gcc-patches > wrote: >> >> Wilco Dijkstra writes: >> > Hi Richard, >> > >> >> Yeah, I'm not disagreeing with any of that. It's just a question of >> >> whether the problem should be fixed by artificially lowering the general >> >> rtx costs with one particular user (RA spill costs) in mind, or whether >> >> it should be fixed by making the RA spill code take the factors above >> >> into account. >> > >> > The RA spill code already works fine on immediates but not on address >> > constants. And the reason is that the current rtx costs for addresses are >> > set artificially high without justification (I checked the patch that >> > increased >> > the costs but there was nothing explaining why it was beneficial). >> >> But even if the costs are too high, the patch seems to be overcompensating. >> It doesn't make logical sense for an ADRP+LDR to be cheaper than an LDR. >> >> Giving X zero cost means that a sequence like: >> >> (set (reg x0) X) >> (set (reg x1) X) >> >> should stay as-is rather than be changed to: >> >> (set (reg x0) X) >> (set (reg x1) (reg x0)) >> >> I don't think we want that for multi-instruction constants when >> optimising for size. >> >> > It's certainly possible to experiment with increasing the spill costs, but >> > that >> > won't improve the issue with address constants unless they are at least >> > doubled. >> > And it has the effect of halving all rtx costs in the register allocator >> > which is >> > likely to cause regressions. So we'd need to adjust many rtx costs to keep >> > the >> > allocator working, plus fix any further regressions this causes. >> >> Yeah, I wasn't suggesting that we increase the spill costs. I'm saying >> that we should look at whether the target-independent RA heuristics need >> to change, whether new target hooks are needed, etc. We shouldn't go >> into this with the assumption that the target-independent code is >> invariant and that any fix must be in existing aarch64 hooks (rtx costs >> or spill costs). >> >> Maybe it would help to turn the question around for a minute. Can we >> describe the cases in which it's *better* for the RA to spill a constant >> address to the stack and reload it, rather than rematerialise on demand? > > From the discussion in PR102178 it seems that LRA cannot rematerialize > all "constants" (though here it is constant pool loads). Some constants > might also not be 'constant'. See the PR for more fun "spilling" behavior > on x86_64. > > It's also said that chosen alternatives might be the reason that > rematerialization > is not choosen and alternatives are chosen based on reload heuristics, not > based > on actual costs. Thanks for the pointer. Yeah, it'd be interesting to know if this is the same issue, although I fear the only way of knowing for sure is to fix it first and see whether both targets benefit. ;-) Richard
Re: [PATCH] AArch64: Cleanup CPU option processing code
Wilco Dijkstra via Gcc-patches writes:
> Hi Richard,
>
>> Although invoking ./cc1 directly only half-works with --with-arch,
>> it half-works well-enough that I'd still like to keep it working.
>> But I agree we should apply your change first, then I can follow up
>> with a patch to make --with-* work with ./cc1 later. (I have a version
>> locally and the net result is much simpler than the status quo, as well
>> as hopefully actually working properly.)
>
> That sounds good indeed. Is that changing TARGET_CPU_DEFAULT into
> a string can could just be parsed like a -mcpu option?
Yeah, it emulates the DRIVER_SELF_SPECS stuff using string macros.
>> Do we still need both selected_arch and explicit_arch? explicit_arch
>> seems a misnomer now, since it includes implicit as well as explicit
>> choices. Same for selected_tune and explicit_tune_core.
>
> At the moment we do since these are settings that must be saved/restored
> since they can be overridden.
Right, but I was wondering if we could save/restore them based on the
selected_* values instead. However…
> However it may be possible to do further cleanups to remove some of this.
…I agree that might as well be a separate clean-up.
> I also wonder whether we can remove the internal override feature
> (override_tune_string) since that further complicates the tunings, and
> I'm not convinced that it is either useful or being used at all.
It's a developer option rather than a user-facing thing. I found it
really useful when doing the Neoverse V1 tuning, and there are some
tests that rely on it. I'd prefer to keep it if possible.
>> aarch64_option_restore has:
>>
>> if (opts->x_explicit_tune_core == aarch64_none
>> && opts->x_explicit_arch != aarch64_no_arch)
>> selected_tune = _cores[selected_arch->ident];
>> else
>> selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
>>
>> Is the “if” condition ever true, or can we now restore the tune
>> info unconditionally?
>
> Yes that was added a year or so after I created this patch, so this
> is now redundant. I've removed it in v2:
>
>
> The --with-cpu/--with-arch configure option processing not only checks valid
> arguments
> but also sets TARGET_CPU_DEFAULT with a CPU and extension bitmask. This
> isn't used
> however since a --with-cpu is translated into a -mcpu option which is
> processed as if
> written on the command-line (so TARGET_CPU_DEFAULT is never accessed).
>
> So remove all the complex processing and bitmask, and just validate the
> option.
> Fix a bug that always reports valid architecture extensions as invalid. As a
> result
> the CPU processing in aarch64.c can be simplified.
>
> Bootstrap OK, regress pass, OK for commit?
>
> ChangeLog:
> 2022-04-19 Wilco Dijkstra
>
> * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
> processing. Add support for architectural extensions.
> * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
> AARCH64_CPU_DEFAULT_FLAGS.
> (TARGET_CPU_NBITS): Remove.
> (TARGET_CPU_MASK): Remove.
> * config/aarch64/aarch64.cc (AARCH64_CPU_DEFAULT_FLAGS): Remove
> define.
> (get_tune_cpu): Assert CPU is always valid.
> (get_arch): Assert architecture is always valid.
> (aarch64_override_options): Cleanup CPU selection code and simplify
> logic.
> (aarch64_option_restore): Remove unnecessary checks on tune.
Looks like you might have attached the old patch. The aarch64_option_restore
change is mentioned in the changelog but doesn't appear in the patch itself.
Thanks,
Richard
>
> ---
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index
> c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b
> 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4178,8 +4178,6 @@ case "${target}" in
> pattern=AARCH64_CORE
> fi
>
> - ext_mask=AARCH64_CPU_DEFAULT_FLAGS
> -
> # Find the base CPU or ARCH id in aarch64-cores.def or
> # aarch64-arches.def
> if [ x"$base_val" = x ] \
> @@ -4187,23 +4185,6 @@ case "${target}" in
> ${srcdir}/config/aarch64/$def \
> > /dev/null; then
>
> - if [ $which = arch ]; then
> - base_id=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/^[^,]*,[]*//' | \
> - sed -e 's/,.*$//'`
> - # Extract the architecture flags from
> aarch64-arches.def
> - ext_mask=`grep "^$pattern(\"$base_val\"," \
> -${srcdir}/config/aarch64/$def | \
> -sed -e 's/)$//' | \
> -
[PATCH] Make gimple_build main workers more flexible
The following makes the main gimple_build API take a gimple_stmt_iterator, whether to insert before or after and an iterator update argument to make it more convenient to use in certain situations (see the tree-vect-generic.cc hunks for an example). It also makes the case we insert into the IL somewhat distinct from inserting into a standalone sequence in that it simplifies built expressions the same way as inserting and calling fold_stmt (..., follow_all_ssa_edges) would. When inserting into a standalone sequence we restrict simplification to defs within the currently building sequence. The patch only amends the tree_code gimple_build API, I will followup with converting the rest as well. The patch got larger than intended because the template forwarders now use gsi_last which introduces a dependency on gimple-iterator.h requiring mass #include re-org across the tree. There are two frontend specific files including gimple-fold.h just for some padding clearing stuff - I've removed the include and instead moved the declarations to fold-const.h (but not the implementations). Otherwise I'd have to include half of the middle-end headers in those files which I didn't much like. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. I've built a cc1 cross to s390x and trust the other target changes (just re-ordering, not adding of a header). Comments? Thanks, Richard. 2022-05-12 Richard Biener gcc/cp/ * constexpr.cc: Remove gimple-fold.h include. gcc/c-family/ * c-omp.cc: Remove gimple-fold.h include. gcc/analyzer/ * supergraph.cc: Re-order gimple-fold.h include. gcc/ * gimple-fold.cc (gimple_build): Adjust for new main API. * gimple-fold.h (gimple_build): New main APIs with iterator, insert direction and iterator update. (gimple_build): New forwarder template. (clear_padding_type_may_have_padding_p): Remove. (clear_type_padding_in_mask): Likewise. (arith_overflowed_p): Likewise. * fold-const.h (clear_padding_type_may_have_padding_p): Declare. (clear_type_padding_in_mask): Likewise. (arith_overflowed_p): Likewise. * tree-vect-generic.cc (gimplify_build3): Use main gimple_build API. (gimplify_build2): Likewise. (gimplify_build1): Likewise. * ubsan.cc (ubsan_expand_ptr_ifn): Likewise, avoid extra compare stmt. * gengtype.cc (open_base_files): Re-order includes. * builtins.cc: Re-order gimple-fold.h include. * calls.cc: Likewise. * cgraphbuild.cc: Likewise. * cgraphunit.cc: Likewise. * config/rs6000/rs6000-builtin.cc: Likewise. * config/rs6000/rs6000-call.cc: Likewise. * config/rs6000/rs6000.cc: Likewise. * config/s390/s390.cc: Likewise. * expr.cc: Likewise. * fold-const.cc: Likewise. * function-tests.cc: Likewise. * gimple-match-head.cc: Likewise. * gimple-range-fold.cc: Likewise. * gimple-ssa-evrp-analyze.cc: Likewise. * gimple-ssa-evrp.cc: Likewise. * gimple-ssa-sprintf.cc: Likewise. * gimple-ssa-warn-access.cc: Likewise. * gimplify.cc: Likewise. * graphite-isl-ast-to-gimple.cc: Likewise. * ipa-cp.cc: Likewise. * ipa-devirt.cc: Likewise. * ipa-prop.cc: Likewise. * omp-low.cc: Likewise. * pointer-query.cc: Likewise. * range-op.cc: Likewise. * tree-cfg.cc: Likewise. * tree-if-conv.cc: Likewise. * tree-inline.cc: Likewise. * tree-object-size.cc: Likewise. * tree-ssa-ccp.cc: Likewise. * tree-ssa-dom.cc: Likewise. * tree-ssa-forwprop.cc: Likewise. * tree-ssa-ifcombine.cc: Likewise. * tree-ssa-loop-ivcanon.cc: Likewise. * tree-ssa-math-opts.cc: Likewise. * tree-ssa-pre.cc: Likewise. * tree-ssa-propagate.cc: Likewise. * tree-ssa-reassoc.cc: Likewise. * tree-ssa-sccvn.cc: Likewise. * tree-ssa-strlen.cc: Likewise. * tree-ssa.cc: Likewise. * value-pointer-equiv.cc: Likewise. * vr-values.cc: Likewise. gcc/testsuite/ * gcc.dg/plugin/diagnostic_group_plugin.c: Reorder or remove gimple-fold.h include. * gcc.dg/plugin/diagnostic_plugin_show_trees.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_inlining.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_metadata.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_paths.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_string_literals.c: Likewise. * gcc.dg/plugin/diagnostic_plugin_test_tree_expression_range.c: Likewise. * gcc.dg/plugin/finish_unit_plugin.c: Likewise. * gcc.dg/plugin/ggcplug.c: Likewise. * gcc.dg/plugin/must_tail_call_plugin.c:
Re: [PATCH] [PR105455] Set edge probabilities when hardening conditionals
On Thu, May 12, 2022 at 3:48 AM Alexandre Oliva via Gcc-patches
wrote:
>
>
> When turning unconditional edges into conditional, as in
> gimple-harden-conditionals.cc:insert_check_and_trap, the newly-created
> edge's probability comes out uninitialized, while the previously
> unconditional edge's probability is presumably
> profile_probability::always.
>
> Mixing initialized and uninitialized probabilities before expand
> breaks predict.cc:force_edge_cold: the initialized probability may end
> up copied to a REG_BR_PROB note in a conditional branch insn, but if
> force_edge_cold is called on that edge, it will find another edge with
> uninitialized probability and assume the note is absent. Later on,
> rtl_verify_edges complains that the note does not match the
> probability modified by force_edge_cold in the edge.
>
> This patch sets probabilities for edges affected by hardening of
> conditionals, both the newly-created edges to trap blocks and the
> previously-unconditional edges, so that the former are considered
> never taken, while the latter are confirmed as always taken.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
OK.
>
> for gcc/ChangeLog
>
> PR rtl-optimization/105455
> * gimple-harden-conditionals.cc (insert_check_and_trap): Set
> probabilities for newly-conditional edges.
>
> for gcc/testsuite/ChangeLog
>
> PR rtl-optimization/105455
> * gcc.dg/pr105455.c: New.
> ---
> gcc/gimple-harden-conditionals.cc |2 ++
> gcc/testsuite/gcc.dg/pr105455.c | 17 +
> 2 files changed, 19 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/pr105455.c
>
> diff --git a/gcc/gimple-harden-conditionals.cc
> b/gcc/gimple-harden-conditionals.cc
> index 28c4810f0a78e..240d0e5acd87a 100644
> --- a/gcc/gimple-harden-conditionals.cc
> +++ b/gcc/gimple-harden-conditionals.cc
> @@ -254,8 +254,10 @@ insert_check_and_trap (location_t loc,
> gimple_stmt_iterator *gsip,
> equality. */
>single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU;
>single_succ_edge (chk)->flags |= neg_true_false_flag;
> + single_succ_edge (chk)->probability = profile_probability::always ();
>edge e = make_edge (chk, trp, true_false_flag);
>e->goto_locus = loc;
> + e->probability = profile_probability::never ();
>
>if (dom_info_available_p (CDI_DOMINATORS))
> set_immediate_dominator (CDI_DOMINATORS, trp, chk);
> diff --git a/gcc/testsuite/gcc.dg/pr105455.c b/gcc/testsuite/gcc.dg/pr105455.c
> new file mode 100644
> index 0..81e9154baa1c3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr105455.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fharden-conditional-branches -funroll-loops --param
> max-loop-header-insns=1" } */
> +
> +__attribute__ ((cold)) void
> +bar (void);
> +
> +void
> +foo (int x)
> +{
> + if (x)
> +{
> + int i;
> +
> + for (i = 0; i < 101; ++i)
> +bar ();
> +}
> +}
>
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts. Ask me about
Re: [PATCH] Avoid visiting newly-created blocks in harden-conditionals
On Thu, May 12, 2022 at 3:47 AM Alexandre Oliva via Gcc-patches
wrote:
>
>
> Reverse iteration over blocks, in gimple-harden-conditionals.cc, was
> supposed to avoid visiting blocks introduced by hardening and
> introducing further reversed conditionals and traps for them, but
> newly-created blocks may be inserted before the current block, as
> shown by the PR105455 testcase.
>
> New blocks use and increment last block number, so test the block
> index against the initial last block number to skip new blocks.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
OK. Note since FOR_EACH_BB_* is basically an arbitrary order
you could also have iterated over the basic-block array itself up
until its original size. Note you are relying on an implementation
detail here, it might be better to mark blocks visited or iterate
over the CFG in a more defined manner (you could compute
an RPO basic block vector which by definition wouldn't include
any newly introduced blocks either - or to avoid a DFS walk,
gather blocks into an array with the FOR_EACH_BB_* method
and then iterate over the array).
Richard.
>
> for gcc/ChangeLog
>
> * gimple-harden-conditionals.cc
> (pass_harden_conditional_branches::execute): Skip new blocks.
> (pass_harden_compares::execute): Likewise.
> ---
> gcc/gimple-harden-conditionals.cc | 401
> +++--
> 1 file changed, 211 insertions(+), 190 deletions(-)
>
> diff --git a/gcc/gimple-harden-conditionals.cc
> b/gcc/gimple-harden-conditionals.cc
> index c7e5e077a74f6..28c4810f0a78e 100644
> --- a/gcc/gimple-harden-conditionals.cc
> +++ b/gcc/gimple-harden-conditionals.cc
> @@ -301,9 +301,18 @@ insert_edge_check_and_trap (location_t loc, edge e,
> unsigned int
> pass_harden_conditional_branches::execute (function *fun)
> {
> + int orig_last_block = last_basic_block_for_fn (fun);
> +
>basic_block bb;
>FOR_EACH_BB_REVERSE_FN (bb, fun)
> {
> + /* Despite our backwards iteration on basic blocks, sometimes
> +split_edge will insert the new block before the block we're
> +hardening, and then we'd harden the hardening block. Skip
> +newly-created blocks to avoid that. */
> + if (bb->index >= orig_last_block)
> + continue;
> +
>gimple_stmt_iterator gsi = gsi_last_bb (bb);
>
>if (gsi_end_p (gsi))
> @@ -383,6 +392,8 @@ non_eh_succ_edge (basic_block bb, edge *ehp = NULL)
> unsigned int
> pass_harden_compares::execute (function *fun)
> {
> + int orig_last_block = last_basic_block_for_fn (fun);
> +
>basic_block bb;
>/* Go backwards over BBs and stmts, so that, even if we split the
> block multiple times to insert a cond_expr after each compare we
> @@ -390,198 +401,208 @@ pass_harden_compares::execute (function *fun)
> stmt exactly once, and not visiting newly-added blocks or
> stmts. */
>FOR_EACH_BB_REVERSE_FN (bb, fun)
> -for (gimple_stmt_iterator gsi = gsi_last_bb (bb);
> -!gsi_end_p (gsi); gsi_prev ())
> - {
> - gassign *asgn = dyn_cast (gsi_stmt (gsi));
> - if (!asgn)
> - continue;
> -
> - /* Turn:
> -
> - z = x op y;
> -
> - into:
> -
> - z = x op y;
> - z' = x' cop y';
> - if (z == z') __builtin_trap ();
> -
> - where cop is a complementary boolean operation to op; and x'
> - and y' hold the same value as x and y, but in a way that does
> - not enable the compiler to optimize the redundant compare
> - away.
> - */
> -
> - enum tree_code op = gimple_assign_rhs_code (asgn);
> -
> - enum tree_code cop;
> -
> - switch (op)
> - {
> - case EQ_EXPR:
> - case NE_EXPR:
> - case GT_EXPR:
> - case GE_EXPR:
> - case LT_EXPR:
> - case LE_EXPR:
> - case LTGT_EXPR:
> - case UNEQ_EXPR:
> - case UNGT_EXPR:
> - case UNGE_EXPR:
> - case UNLT_EXPR:
> - case UNLE_EXPR:
> - case ORDERED_EXPR:
> - case UNORDERED_EXPR:
> - cop = invert_tree_comparison (op,
> - HONOR_NANS
> - (gimple_assign_rhs1 (asgn)));
> -
> - if (cop == ERROR_MARK)
> - /* ??? Can we do better? */
> - continue;
> +{
> + /* Despite our backwards iteration on basic blocks, sometimes
> +split_edge will insert the new block before the block we're
> +hardening, and then we'd harden the hardening block. Skip
> +newly-created blocks to avoid that. */
> + if (bb->index >= orig_last_block)
> + continue;
>
> - break;
> -
> - /* ??? Maybe handle these too? */
> - case TRUTH_NOT_EXPR:
> - /* ??? The code below assumes binary ops, it would have to
> - be adjusted for TRUTH_NOT_EXPR, since it's unary. */
> -
Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator
On Thu, May 12, 2022 at 3:37 AM Eugene Rozenfeld
wrote:
>
> In my case this is not exactly what the FIXME in the comment above says. The
> count is 0 even before
> the initial scaling happens. I hit this case with some changes I'm working on
> to enable per-instruction discriminators for AutoFDO.
>
> I looked into saving/restoring the old counts but it would be awkward to do.
> The initial scaling happens here:
>
> if (skip_vector)
> {
> split_edge (loop_preheader_edge (loop));
>
> /* Due to the order in which we peel prolog and epilog, we first
> propagate probability to the whole loop. The purpose is to
> avoid adjusting probabilities of both prolog and vector loops
> separately. Note in this case, the probability of epilog loop
> needs to be scaled back later. */
> basic_block bb_before_loop = loop_preheader_edge (loop)->src;
> if (prob_vector.initialized_p ())
> {
> scale_bbs_frequencies (_before_loop, 1, prob_vector);
> scale_loop_profile (loop, prob_vector, 0);
> }
> }
>
> The scaling happens before we do the cloning for the epilog so to
> save/restore the counts
> we would need to maintain a mapping between the original basic blocks and the
> cloned basic blocks in the epilog.
There is one already - after the epilogue is copied you can use
get_bb_original (epilouge_bb) to get at the block it was copied from.
It could also be that we can rely on the basic-block order to be
preserved between the copies
(I _think_ that would work ... but then assert () for this using
get_bb_original ()). That means
the simplest fix would be to have an auto_vec and initialize
that from the
BB counts in loop body order (we also have exactly two BBs in all
peeled loops ...
But note we only scaled the scalar if-converted loop but eventually
used the not if-coverted copy
for the epilogue (if not vectorizing the epilogue), so indiscriminate
scaling back looks wrong in
some cases (I'd have to double-check the details here).
> I'd like to get my simple fix in since it makes things better even if it
> doesn't address the issue mentioned
> In the FIXME.
But don't you need to check that
bbs[i]->count.apply_probability (prob_vector) is not zero instead of checking
that bbs[i].count is not zero?
Richard.
> -Original Message-
> From: Richard Biener
> Sent: Monday, May 09, 2022 12:42 AM
> To: Eugene Rozenfeld ; Jan Hubicka
>
> Cc: gcc-patches@gcc.gnu.org
> Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0
> denominator
>
> On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches
> wrote:
> >
> > Calling count.apply_scale with a 0 denominator causes an assert.
> > This change guards against that.
> >
> > Tested on x86_64-pc-linux-gnu.
> >
> > gcc/ChangeLog:
> > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying
> > scale with 0 denominator.
> > ---
> > gcc/tree-vect-loop-manip.cc | 9 +
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> > index 1d4337eb261..db54ae69e45 100644
> > --- a/gcc/tree-vect-loop-manip.cc
> > +++ b/gcc/tree-vect-loop-manip.cc
> > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree
> > niters, tree nitersm1,
> > get lost if we scale down to 0. */
> > basic_block *bbs = get_loop_body (epilog);
> > for (unsigned int i = 0; i < epilog->num_nodes; i++)
> > - bbs[i]->count = bbs[i]->count.apply_scale
> > -(bbs[i]->count,
> > - bbs[i]->count.apply_probability
> > - (prob_vector));
> > + if (bbs[i]->count.nonzero_p ())
> > + bbs[i]->count = bbs[i]->count.apply_scale
> > + (bbs[i]->count,
> > + bbs[i]->count.apply_probability
> > + (prob_vector));
>
> So exactly what the FIXME in the comment above says happens. It
> might be better
> to save/restore the old counts if the intent is to get them back. I'm not
> exactly sure where the other scaling happens though.
>
> Richard.
>
>
>
> > free (bbs);
> > }
> >
> > --
> > 2.25.1
Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD
Hi Piotr, Thanks for doing this, some comments are inlined. on 2022/5/11 07:32, Piotr Kubaj via Gcc-patches wrote: > Is there anything more required? > > On 22-05-03 12:33:43, Piotr Kubaj wrote: >> Here are gmake check-gfortran results requested by FX. >> >> Before patching: >> === gfortran Summary === >> >> # of expected passes65106 >> # of unexpected failures6 >> # of expected failures 262 >> # of unsupported tests 367 >> >> After patching: >> === gfortran Summary === >> >> # of expected passes65384 >> # of unexpected failures6 >> # of expected failures 262 >> # of unsupported tests 373 >> Nice! >> In both cases, unexpected failures are: >> FAIL: gfortran.dg/pr98076.f90 -O0 execution test >> FAIL: gfortran.dg/pr98076.f90 -O1 execution test >> FAIL: gfortran.dg/pr98076.f90 -O2 execution test >> FAIL: gfortran.dg/pr98076.f90 -O3 -fomit-frame-pointer -funroll-loops >> -fpeel-loops -ftracer -finline-functions execution test >> FAIL: gfortran.dg/pr98076.f90 -O3 -g execution test >> FAIL: gfortran.dg/pr98076.f90 -Os execution test >> >> But this seems unrelated to my patch.>> >> On 22-05-03 12:21:12, pku...@freebsd.org wrote: >>> From: Piotr Kubaj >>> >>> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. >>> >>> Signed-off-by: Piotr Kubaj >>> --- >>> libgfortran/configure| 41 +++- >>> libgfortran/configure.ac | 17 - >>> 2 files changed, 56 insertions(+), 2 deletions(-) >>> ... snip >>> # At least for glibc, clock_gettime is in librt. But don't >>> # pull that in if it still doesn't give us the function we want. This >>> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac >>> index 97cc490cb5e..2dd6d345b22 100644 >>> --- a/libgfortran/configure.ac >>> +++ b/libgfortran/configure.ac >>> @@ -602,8 +602,23 @@ fi >>> # Check whether we have a __float128 type, depends on >>> enable_libquadmath_support >>> LIBGFOR_CHECK_FLOAT128 >>> >>> +case x$target in >>> + xpowerpc*-freebsd*) >>> +AC_CACHE_CHECK([for fenv.h and feenableexcept], have_feenableexcept, [ >>> + AC_COMPILE_IFELSE([AC_LANG_PROGRAM( >>> +[[ #include ]], >>> +[[ feenableexcept(FE_DIVBYZERO | FE_INVALID); ]])], >>> +[ have_feenableexcept="yes" ], >>> +[ have_feenableexcept="no" ])]) >>> +if test "x$have_feenableexcept" = "xyes"; then >>> + AC_DEFINE(HAVE_FEENABLEEXCEPT,1,[fenv.h includes feenableexcept]) >>> +fi; >>> +;; >>> + *) As your explanation in [1], IMHO it's not a good idea to take powerpc*-freebsd* specially here, for your mentioned triples that may also suffer this same issue, we will have to update this hunk for them in future. How about: do the glibc check first as before, if it fails then do one further general check (for all) with one compilation on fenv.h as what your patch proposes. Besides, IMHO it should use AC_LINK_IFELSE, since one fenv.h which doesn't implement feenableexcept could also pass the check. And there is one AC_CHECK_HEADERS_ONCE checking for fenv.h existence, not sure if it's worth to guarding the further check with its result. [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593248.html BR, Kewen
Re: [PATCH] x86: Document -mno-cet-switch
On Wed, May 11, 2022 at 9:03 PM Florian Weimer via Gcc-patches
wrote:
>
> * H. J. Lu:
>
> > On Wed, May 11, 2022 at 11:45 AM Florian Weimer wrote:
> >>
> >> * H. J. Lu:
> >>
> >> >> NOTRACK avoids the need for ENDBR instructions, right? That's a
> >> >> hardening improvement, so it should be used by default.
> >> >
> >> > NOTRACK weakens IBT since it disables IBT on the indirect jump
> >> > instruction.
> >> > GCC uses it in the jump table to avoid ENDBR.
> >>
> >> Typical jump table code looks like this:
> >>
> >> Dump of assembler code for function __cache_sysconf:
> >>0x000f7a80 <+0>: endbr64
> >>0x000f7a84 <+4>: sub$0xb9,%edi
> >>0x000f7a8a <+10>:cmp$0xc,%edi
> >>0x000f7a8d <+13>:ja 0xf7b70 <__cache_sysconf+240>
> >>0x000f7a93 <+19>:lea0xba926(%rip),%rdx# 0x1b23c0
> >>0x000f7a9a <+26>:movslq (%rdx,%rdi,4),%rax
> >>0x000f7a9e <+30>:add%rdx,%rax
> >>0x000f7aa1 <+33>:notrack jmp *%rax
> >>
> >> There's no ENDBR instruction between range check, the address
> >> computation, and the NOTRACK JMP, so it's not possible to redirect that
> >> JMP to some other place.
> >
> > That is the assumption we made. RAX will always point to the valid
> > address.
>
> Which means that NOTRACK should not weaken anything here. What am I
> missing?
Probably nothing. Still if the code computing the address to jump to
is exploitable
it might be useful. Like considering
void foo (int *idx) { switch (*idx) { ...; default:
__builtin_unreachable (); } }
then GCC would elide the range check for the jump table lookup.
I think the main reason for the NOTRACK is code size and the requirement to
place extra ENDBR at each jump target (where any ENDBR is a possible
valid target to use as gadget from a tracked brach).
So NOTRACK branches are bad and ENDBR are both "bad"?
Richard.
>
> Thanks,
> Florian
>
RE: [PATCH] PR tree-optimization/83907: Improved memset handling in strlen pass.
Hi Jeff, Any chance you could take a look at this patch, now that we're back in stage1? Thanks in advance, Roger > -Original Message- > From: Jeff Law > Sent: 02 March 2022 19:33 > To: Roger Sayle ; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] PR tree-optimization/83907: Improved memset handling in > strlen pass. > > On 2/20/2022 9:51 AM, Roger Sayle wrote: > > This patch implements the missed optimization enhancement PR 83907, by > > handling memset with a constant byte value in tree-ssa's strlen > > optimization pass. Effectively, this treats memset(dst,'x',3) as it > > would memcpy(dst,"xxx",3). > > > > This patch also includes a tweak to handle_store to address another > > missed optimization observed in the related test case pr83907-2.c. > > The consecutive byte stores to memory get coalesced into a vector > > write of a vector const, but unfortunately tree-ssa-strlen's > > handle_store didn't previously handle the (unusual) case where the > > stored "string" starts with a zero byte but also contains non-zero > > bytes. > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > > and make -k check with no new failures. Ok for mainline? > > > > > > 2022-02-20 Roger Sayle > > > > gcc/ChangeLog > > PR tree-optimization/83907 > > * tree-ssa-strlen.cc (handle_builtin_memset): Record a strinfo > > for memset with an constant char value. > > (handle_store): Improved handling of stores with a first byte > > of zero, but not storing_all_zeros_p. > > > > gcc/testsuite/ChangeLog > > PR tree-optimization/83907 > > * gcc.dg/tree-ssa/pr83907-1.c: New test case. > > * gcc.dg/tree-ssa/pr83907-2.c: New test case. > Deferring to stage1. > jeff
Re: [PATCH] opts: improve option suggestion
On 5/11/22 20:49, David Malcolm wrote:
> On Wed, 2022-05-11 at 16:49 +0200, Martin Liška wrote:
>> In case where we have 2 equally good candidates like
>> -ftrivial-auto-var-init=
>> -Wtrivial-auto-var-init
>>
>> for -ftrivial-auto-var-init, we should take the candidate that
>> has a difference in trailing sign symbol.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> PR driver/105564
>>
>> gcc/ChangeLog:
>>
>> * spellcheck.cc (test_find_closest_string): Add new test.
>> * spellcheck.h (class best_match): Prefer a difference in
>> trailing sign symbol.
>> ---
>> gcc/spellcheck.cc | 9 +
>> gcc/spellcheck.h | 17 ++---
>> 2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/spellcheck.cc b/gcc/spellcheck.cc
>> index 3e58344f510..f728573331f 100644
>> --- a/gcc/spellcheck.cc
>> +++ b/gcc/spellcheck.cc
>> @@ -464,6 +464,15 @@ test_find_closest_string ()
>> ASSERT_STREQ ("DWARF_GNAT_ENCODINGS_ALL",
>> find_closest_string ("DWARF_GNAT_ENCODINGS_all",
>> ));
>> +
>> + /* Example from PR 105564 where option name with missing equal
>> + sign should win. */
>> + candidates.truncate (0);
>> + candidates.safe_push ("-Wtrivial-auto-var-init");
>> + candidates.safe_push ("-ftrivial-auto-var-init=");
>> + ASSERT_STREQ ("-ftrivial-auto-var-init=",
>> + find_closest_string ("-ftrivial-auto-var-init",
>> + ));
>> }
>>
>> /* Test data for test_metric_conditions. */
>> diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
>> index 9b6223695be..9111ea08fc3 100644
>> --- a/gcc/spellcheck.h
>> +++ b/gcc/spellcheck.h
>> @@ -128,11 +128,22 @@ class best_match
>>
>> /* Otherwise, compute the distance and see if the candidate
>> has beaten the previous best value. */
>> + const char *candidate_str = candidate_traits::get_string
>> (candidate);
>> edit_distance_t dist
>> - = get_edit_distance (m_goal, m_goal_len,
>> - candidate_traits::get_string (candidate),
>> - candidate_len);
>> + = get_edit_distance (m_goal, m_goal_len, candidate_str,
>> candidate_len);
>> +
>> + bool is_better = false;
>> if (dist < m_best_distance)
>> + is_better = true;
>> + else if (dist == m_best_distance)
>> + {
>> + /* Prefer a candidate has a difference in trailing sign
>> character. */
>> + if (candidate_str[candidate_len - 1] == '='
>> + && m_goal[m_goal_len - 1] != '=')
>> + is_better = true;
>> + }
>
> Thanks for working on this.
>
> Maybe the comment should read:
>
> /* Prefer a candidate that inserts a trailing '=',
>so that for
> "-ftrivial-auto-var-init"
>we suggest
> "-ftrivial-auto-var-init="
>rather than
> "-Wtrivial-auto-var-init". */
I welcome the comment suggestion.
>
> Is the logic correct? It's comparing the candidate with the goal,
> rather than with the current best.
Yes, that's basically saying that there's a difference in the trailing sign
char.
> What if both the candidate and the
> current best both add a trailing equal sign?
Then there's a difference somewhere else, and we would follow edit_distance_t.
>
> I find the array access of the final character suspicious - is there
> any chance that either of the lengths could be zero? I don't think so,
> but maybe we should bulletproof things, and move the "is better"
> comparison to a subroutine?
I think one can't have empty string here.
Sending updated version of the patch where I added the comment.
Ready to be installed?
Thanks,
Martin
>
> Hope this is constructive
> Dave
>
>> +
>> + if (is_better)
>> {
>> m_best_distance = dist;
>> m_best_candidate = candidate;
>
>
From ec1dfdc15d1703a3148ecdd269f6dfd4a1b7a40b Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Wed, 11 May 2022 16:07:25 +0200
Subject: [PATCH] opts: improve option suggestion
In case where we have 2 equally good candidates like
-ftrivial-auto-var-init=
-Wtrivial-auto-var-init
for -ftrivial-auto-var-init, we should take the candidate that
has a difference in trailing sign symbol.
PR driver/105564
gcc/ChangeLog:
* spellcheck.cc (test_find_closest_string): Add new test.
* spellcheck.h (class best_match): Prefer a difference in
trailing sign symbol.
---
gcc/spellcheck.cc | 9 +
gcc/spellcheck.h | 24 +---
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/gcc/spellcheck.cc b/gcc/spellcheck.cc
index 3e58344f510..f728573331f 100644
--- a/gcc/spellcheck.cc
+++ b/gcc/spellcheck.cc
@@ -464,6 +464,15 @@ test_find_closest_string ()
ASSERT_STREQ ("DWARF_GNAT_ENCODINGS_ALL",
find_closest_string ("DWARF_GNAT_ENCODINGS_all",
));
+
Re: [PATCH] Use more ARRAY_SIZE.
On Wed, May 11, 2022 at 10:01 PM David Malcolm via Gcc-patches wrote: > > On Wed, 2022-05-11 at 15:04 -0400, Eric Gallager wrote: > > On Wed, May 11, 2022 at 9:53 AM David Malcolm via Gcc-patches > > wrote: > > > > > > On Thu, 2022-05-05 at 14:16 +0200, Martin Liška wrote: > > > > Patch can bootstrap on x86_64-linux-gnu and survives regression > > > > tests. > > > > > > > > Ready to be installed? > > > > > > I looked over the changes to: > > > > > > gcc/analyzer/*.cc > > > gcc/input.cc > > > gcc/jit/*.cc > > > > > > and they look good to me. > > > > > > I assume that you enabled "jit" during the testing, right? (it's > > > not > > > in --enable-languages=all, which is a perennial source of issues). > > > > > > > So, what remains to be done to include "jit" in "all" anyways? Are > > there still some platforms left that it's not supported on yet or > > something? > > IIRC the issue was that it requires --enable-host-shared, which slows > down the compiler for everyone else by a few percent (since it requires > the compiler to be built as position-independent code). And with all symbols exported aka interposable. Maybe we could funnel in -fno-semantic-interposition ... (and see if it makes a difference) Richard. > > Dave >
Re: [PATCH] opts: improve option suggestion
On Wed, May 11, 2022 at 8:50 PM David Malcolm via Gcc-patches
wrote:
>
> On Wed, 2022-05-11 at 16:49 +0200, Martin Liška wrote:
> > In case where we have 2 equally good candidates like
> > -ftrivial-auto-var-init=
> > -Wtrivial-auto-var-init
> >
> > for -ftrivial-auto-var-init, we should take the candidate that
> > has a difference in trailing sign symbol.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >
> > Ready to be installed?
> > Thanks,
> > Martin
> >
> > PR driver/105564
> >
> > gcc/ChangeLog:
> >
> > * spellcheck.cc (test_find_closest_string): Add new test.
> > * spellcheck.h (class best_match): Prefer a difference in
> > trailing sign symbol.
> > ---
> > gcc/spellcheck.cc | 9 +
> > gcc/spellcheck.h | 17 ++---
> > 2 files changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/spellcheck.cc b/gcc/spellcheck.cc
> > index 3e58344f510..f728573331f 100644
> > --- a/gcc/spellcheck.cc
> > +++ b/gcc/spellcheck.cc
> > @@ -464,6 +464,15 @@ test_find_closest_string ()
> >ASSERT_STREQ ("DWARF_GNAT_ENCODINGS_ALL",
> > find_closest_string ("DWARF_GNAT_ENCODINGS_all",
> > ));
> > +
> > + /* Example from PR 105564 where option name with missing equal
> > + sign should win. */
> > + candidates.truncate (0);
> > + candidates.safe_push ("-Wtrivial-auto-var-init");
> > + candidates.safe_push ("-ftrivial-auto-var-init=");
> > + ASSERT_STREQ ("-ftrivial-auto-var-init=",
> > + find_closest_string ("-ftrivial-auto-var-init",
> > +));
> > }
> >
> > /* Test data for test_metric_conditions. */
> > diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
> > index 9b6223695be..9111ea08fc3 100644
> > --- a/gcc/spellcheck.h
> > +++ b/gcc/spellcheck.h
> > @@ -128,11 +128,22 @@ class best_match
> >
> > /* Otherwise, compute the distance and see if the candidate
> > has beaten the previous best value. */
> > +const char *candidate_str = candidate_traits::get_string
> > (candidate);
> > edit_distance_t dist
> > - = get_edit_distance (m_goal, m_goal_len,
> > - candidate_traits::get_string (candidate),
> > - candidate_len);
> > + = get_edit_distance (m_goal, m_goal_len, candidate_str,
> > candidate_len);
> > +
> > +bool is_better = false;
> > if (dist < m_best_distance)
> > + is_better = true;
> > +else if (dist == m_best_distance)
> > + {
> > + /* Prefer a candidate has a difference in trailing sign
> > character. */
> > + if (candidate_str[candidate_len - 1] == '='
> > + && m_goal[m_goal_len - 1] != '=')
> > + is_better = true;
> > + }
>
> Thanks for working on this.
>
> Maybe the comment should read:
>
> /* Prefer a candidate that inserts a trailing '=',
>so that for
> "-ftrivial-auto-var-init"
>we suggest
> "-ftrivial-auto-var-init="
>rather than
> "-Wtrivial-auto-var-init". */
>
> Is the logic correct? It's comparing the candidate with the goal,
> rather than with the current best. What if both the candidate and the
> current best both add a trailing equal sign?
>
> I find the array access of the final character suspicious - is there
> any chance that either of the lengths could be zero? I don't think so,
> but maybe we should bulletproof things, and move the "is better"
> comparison to a subroutine?
I think for this case the logic would be to prefer to stick to the
-W or -f _prefix_ the user gave and give that a "boost" so
we do not suggest an optimization option for a warning one.
That is, when given -Wfoo we should not suggest -ffoo-bar
but -Wfoo-longbar (just made up example), even if the
-ffoo-bar looks a much better match.
But maybe that's better handled by better selecting the initial
candidate set rather than doing the last disambiguation.
Richard.
> Hope this is constructive
> Dave
>
> > +
> > +if (is_better)
> >{
> > m_best_distance = dist;
> > m_best_candidate = candidate;
>
>
Re: [wwwdocs PATCH v2] document zero-width field ABI changes on MIPS
On Wed, 6 Apr 2022, Xi Ruoyao via Gcc-patches wrote: > Document ABI changes in r12-7961, 7962, and 8023. Ok for wwwdocs? Thank you! Note is deprecated, and we generally use id= attributes. I made this change, and also expanded the anchor from just "ABI" which is very generic to more specific text. Pushed on top of the original changes. Gerald diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html index 60ed4b32..d645b15f 100644 --- a/htdocs/gcc-12/changes.html +++ b/htdocs/gcc-12/changes.html @@ -709,8 +709,8 @@ function Multiply (S1, S2 : Sign) return Sign is MIPS - The ABI passing arguments - containing zero-width fields (for example, C/C++ zero-width + The ABI passing arguments + containing zero-width fields (for example, C/C++ zero-width bit-fields, GNU C/C++ zero-length arrays, and GNU C empty structs) has changed. Now a zero-width field will not prevent an aligned 64-bit floating-point field next to it from being passed through @@ -718,8 +718,8 @@ function Multiply (S1, S2 : Sign) return Sign is GCC releases. GCC 12 on MIPS will report such incompatibilities as an inform unless -Wno-psabi is used. - The ABI returning values - containing C++17 empty bases has changed. Now an empty base will + The ABI returning values + containing C++17 empty bases has changed. Now an empty base will not prevent an aggregate containing only one or two floating-point fields from being returned through FPR. This is compatible with GCC 6 and earlier, but incompatible with GCC 7 through 11. GCC 12 on
Re: [PATCH] Extend --with-zstd documentation
On Wed, May 11, 2022 at 5:10 PM Bruno Haible wrote:
>
> The patch that was so far added for documenting --with-zstd is pretty
> minimal:
> - it refers to undocumented options --with-zstd-include and
> --with-zstd-lib;
> - it suggests that --with-zstd can be used without an argument;
> - it does not clarify how this option applies to cross-compilation.
>
> How about adding the same details as for the --with-isl,
> --with-isl-include, --with-isl-lib options, mutatis mutandis? This patch
> does that.
Sounds good!
OK.
Thanks,
Richard.
> PR other/105527
>
> gcc/ChangeLog:
>
> * doc/install.texi (Configuration): Add more details about
> --with-zstd.
> Document --with-zstd-include and --with-zstd-lib
> ---
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 042241e9fad..ed0d1d882c3 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -2360,10 +2360,20 @@ default is derived from glibc's behavior. When glibc
> clamps float_t to double,
> GCC follows and enables the option. For other cross compiles, the default is
> disabled.
>
> -@item --with-zstd
> -Specify prefix directory for installed zstd library.
> -Equivalent to @option{--with-zstd-include=PATH/include} plus
> -@option{--with-zstd-lib=PATH/lib}.
> +@item --with-zstd=@var{pathname}
> +@itemx --with-zstd-include=@var{pathname}
> +@itemx --with-zstd-lib=@var{pathname}
> +If you do not have the @code{zstd} library installed in a standard
> +location and you want to build GCC, you can explicitly specify the
> +directory where it is installed (@samp{--with-zstd=@/@var{zstdinstalldir}}).
> +The @option{--with-zstd=@/@var{zstdinstalldir}} option is shorthand for
> +@option{--with-zstd-lib=@/@var{zstdinstalldir}/lib} and
> +@option{--with-zstd-include=@/@var{zstdinstalldir}/include}. If this
> +shorthand assumption is not correct, you can use the explicit
> +include and lib options directly.
> +
> +These flags are applicable to the host platform only. When building
> +a cross compiler, they will not be used to configure target libraries.
> @end table
>
> @subheading Cross-Compiler-Specific Options
>
>
>
Re: [PATCH] Add -fcf-check-attribute=[yes|no|none] for Linux kernel
On Wed, 11 May 2022, H.J. Lu wrote:
> On Tue, May 10, 2022 at 11:39 PM Richard Biener wrote:
> >
> > On Tue, 10 May 2022, H.J. Lu wrote:
> >
> > > When compiling Linux kernel with -fcf-protection=branch to enable x86
> > > Indiret Branch Tracking (IBT), ENDBR is added to all global functions.
> > > This creates more "legal" forward edges than necessary. -mmanual-endbr
> > > provides a way to insert ENDBR instruction at function entry only via
> > > the 'cf_check' function attribute and programmers can add the 'cf_check'
> > > function attribute to functions which can be reached by indirect branch.
> > >
> > > Add -fcf-check-attribute=[yes|no|none] to imply "cf_check" or "nocf_check"
> > > function attributes so that GCC can produce a diagnostic when there is a
> > > mismatch in cf_check or nocf_check function attributes.
> > >
> > > This has been tested on Linux kernel.
> >
> > I don't quite understand the purpose but wouldn't
> > -fcf-check-attribute=[cf|nocf|off] be better than having no and none?
> > Since 'off' is the default you need 'off' only to be able to negate
> > an earlier cf/nocf?
>
> This option adds implied "cf_check" or "nocf_check" attributes if there
> is none. I need an option not to add them by default.I can use
> -fcf-check-attribute=none or -fcf-check-attribute=off. Which one is
> better?
I guess 'none' is then better, but still -fcf-check-attribute=[cf|nocf]
is then better matching than -fcf-check-attribute=[yes|no].
Richard.
> Thanks.
>
> > > gcc/
> > >
> > > PR target/102953
> > > * attribs.cc (decl_attributes): Add implied cf_check or nocf_check
> > > function attributes.
> > > * common.opt: Add -fcf-check-attribute=.
> > > * flag-types.h (cf_check_attribute): New.
> > > * doc/invoke.texi: Document -fcf-check-attribute=.
> > >
> > > gcc/c/
> > >
> > > PR target/102953
> > > * c-decl.cc (diagnose_mismatched_decls): Check implied cf_check
> > > and nocf_check function attributes.
> > >
> > > gcc/testsuite/
> > >
> > > PR target/102953
> > > * gcc.target/i386/pr102953-3.c: New test.
> > > * gcc.target/i386/pr102953-4.c: Likewise.
> > > * gcc.target/i386/pr102953-5.c: Likewise.
> > > * gcc.target/i386/pr102953-6.c: Likewise.
> > > ---
> > > gcc/attribs.cc | 19 +++
> > > gcc/c/c-decl.cc| 22 +-
> > > gcc/common.opt | 16
> > > gcc/doc/invoke.texi| 12
> > > gcc/flag-types.h | 8
> > > gcc/testsuite/gcc.target/i386/pr102953-3.c | 8
> > > gcc/testsuite/gcc.target/i386/pr102953-4.c | 7 +++
> > > gcc/testsuite/gcc.target/i386/pr102953-5.c | 7 +++
> > > gcc/testsuite/gcc.target/i386/pr102953-6.c | 8
> > > 9 files changed, 106 insertions(+), 1 deletion(-)
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-3.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-4.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-5.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-6.c
> > >
> > > diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> > > index b219f878042..34e8707eac1 100644
> > > --- a/gcc/attribs.cc
> > > +++ b/gcc/attribs.cc
> > > @@ -694,6 +694,25 @@ decl_attributes (tree *node, tree attributes, int
> > > flags,
> > > attributes = tree_cons (get_identifier ("no_icf"), NULL,
> > > attributes);
> > > }
> > >
> > > + /* -fcf-check-attribute=[yes|no] implies "cf_check" or "nocf_check"
> > > + function attribute. */
> > > + if (TREE_CODE (*node) == FUNCTION_DECL
> > > + && flag_cf_check_attribute != CF_CHECK_ATTRIBUTE_NONE
> > > + && !fndecl_built_in_p (*node)
> > > + && lookup_attribute ("nocf_check",
> > > +DECL_ATTRIBUTES (*node)) == nullptr
> > > + && lookup_attribute ("cf_check",
> > > +DECL_ATTRIBUTES (*node)) == nullptr
> > > + && (!attributes
> > > + || (lookup_attribute ("nocf_check", attributes) == nullptr
> > > + && lookup_attribute ("cf_check", attributes) == nullptr)))
> > > +{
> > > + const char *attr = (flag_cf_check_attribute ==
> > > CF_CHECK_ATTRIBUTE_YES
> > > + ? "cf_check" : "nocf_check");
> > > + attributes = tree_cons (get_identifier (attr), nullptr,
> > > + attributes);
> > > +}
> > > +
> > >targetm.insert_attributes (*node, );
> > >
> > >/* Note that attributes on the same declaration are not necessarily
> > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > index c701f07befe..787c39dc0fe 100644
> > > --- a/gcc/c/c-decl.cc
> > > +++ b/gcc/c/c-decl.cc
> > > @@ -2133,7 +2133,27 @@ diagnose_mismatched_decls (tree newdecl, tree
> > > olddecl,
> > > error ("conflicting type
[committed] openmp: Add omp_all_memory support (C/C++ only so far)
Hi! The ugly part is that OpenMP 5.1 made omp_all_memory a reserved identifier which isn't allowed to be used anywhere but in the depend clause, this is against how everything else has been handled in OpenMP so far (where some identifiers could have special meaning in some OpenMP clauses or pragmas but not elsewhere). The patch handles it by making it a conditional keyword (for -fopenmp only) and emitting a better diagnostics when it is used in a primary expression. Having a nicer diagnostics when e.g. trying to do int omp_all_memory; or int *omp_all_memory[10]; etc. would mean changing too many spots and hooking into name lookups to reject declaring any such symbols would be too ugly and I'm afraid there are way too many spots where one can introduce a name (variables, functions, namespaces, struct, enum, enumerators, template arguments, ...). Otherwise, the handling is quite simple, normal depend clauses lower into addresses of variables being handed over to the library, for omp_all_memory I'm using NULL pointers. omp_all_memory can only be used with inout or out depend kinds and means that a task is dependent on all previously created sibling tasks that have any dependency (of any depend kind) and that any later created sibling tasks will be dependent on it if they have any dependency. Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk. 2022-05-12 Jakub Jelinek gcc/ * gimplify.cc (gimplify_omp_depend): Don't build_fold_addr_expr if null_pointer_node. (gimplify_scan_omp_clauses): Likewise. * tree-pretty-print.cc (dump_omp_clause): Print null_pointer_node as omp_all_memory. gcc/c-family/ * c-common.h (enum rid): Add RID_OMP_ALL_MEMORY. * c-omp.cc (c_finish_omp_depobj): Don't build_fold_addr_expr if null_pointer_node. gcc/c/ * c-parser.cc (c_parse_init): Register omp_all_memory as keyword if flag_openmp. (c_parser_postfix_expression): Diagnose uses of omp_all_memory in postfix expressions. (c_parser_omp_variable_list): Handle omp_all_memory in depend clause. * c-typeck.cc (c_finish_omp_clauses): Handle omp_all_memory keyword in depend clause as null_pointer_node, diagnose invalid uses. gcc/cp/ * lex.cc (init_reswords): Register omp_all_memory as keyword if flag_openmp. * parser.cc (cp_parser_primary_expression): Diagnose uses of omp_all_memory in postfix expressions. (cp_parser_omp_var_list_no_open): Handle omp_all_memory in depend clause. * semantics.cc (finish_omp_clauses): Handle omp_all_memory keyword in depend clause as null_pointer_node, diagnose invalid uses. * pt.cc (tsubst_omp_clause_decl): Pass through omp_all_memory. gcc/testsuite/ * c-c++-common/gomp/all-memory-1.c: New test. * c-c++-common/gomp/all-memory-2.c: New test. * c-c++-common/gomp/all-memory-3.c: New test. * g++.dg/gomp/all-memory-1.C: New test. * g++.dg/gomp/all-memory-2.C: New test. libgomp/ * libgomp.h (struct gomp_task): Add depend_all_memory member. * task.c (gomp_init_task): Initialize depend_all_memory. (gomp_task_handle_depend): Handle omp_all_memory. (gomp_task_run_post_handle_depend_hash): Clear parent->depend_all_memory if equal to current task. (gomp_task_maybe_wait_for_dependencies): Handle omp_all_memory. * testsuite/libgomp.c-c++-common/depend-1.c: New test. * testsuite/libgomp.c-c++-common/depend-2.c: New test. * testsuite/libgomp.c-c++-common/depend-3.c: New test. --- gcc/gimplify.cc.jj 2022-05-09 09:09:20.402472032 +0200 +++ gcc/gimplify.cc 2022-05-11 09:32:01.961698970 +0200 @@ -8623,7 +8623,8 @@ gimplify_omp_depend (tree *list_p, gimpl } if (error_operand_p (TREE_VALUE (t))) return 2; - TREE_VALUE (t) = build_fold_addr_expr (TREE_VALUE (t)); + if (TREE_VALUE (t) != null_pointer_node) + TREE_VALUE (t) = build_fold_addr_expr (TREE_VALUE (t)); r = build4 (ARRAY_REF, ptr_type_node, array, cnts[i], NULL_TREE, NULL_TREE); tem = build2_loc (OMP_CLAUSE_LOCATION (c), MODIFY_EXPR, @@ -8650,7 +8651,8 @@ gimplify_omp_depend (tree *list_p, gimpl } if (error_operand_p (OMP_CLAUSE_DECL (c))) return 2; - OMP_CLAUSE_DECL (c) = build_fold_addr_expr (OMP_CLAUSE_DECL (c)); + if (OMP_CLAUSE_DECL (c) != null_pointer_node) + OMP_CLAUSE_DECL (c) = build_fold_addr_expr (OMP_CLAUSE_DECL (c)); if (gimplify_expr (_CLAUSE_DECL (c), pre_p, NULL, is_gimple_val, fb_rvalue) == GS_ERROR) return 2; @@ -10346,12 +10348,15 @@ gimplify_scan_omp_clauses (tree *list_p, remove = true; break; } -
