Re: [PATCH] doc: clarify the situation with pointer arithmetic
On Wed, 22 Jan 2020, Joseph Myers wrote: > On Tue, 21 Jan 2020, Richard Biener wrote: > > > Second. Fact is RTL does not distinguish between pointers and > > integers and thus any attempt to make something valid when you > > use integers and invalid when you use pointers is not going to work. > > That simply means that an earlier stage in the compiler would need to mark > cases of integers converted from pointers in some way as "could have > provenance of any address-exposed object" (that being the effect of > PNVI-ae models) unless it can prove the offsets etc. applied do keep the > pointer within the original object. We already have that, it's called points-to analysis which computes "points-to sets" even for integers. But even there it assumes you can't leave an object via arithmetic, doing that would be quite bad for optimization in the current state or prohibitly expensive since it would require exact offset tracking. Now invent something that transfers this to RTL. Patches to the main piece where the issue triggers (find_base_term + base_alias_check and the very similar find_base_value) welcome. Richard.
Re: [PATCH] Prune invalid filename due to makefile syntax.
On Tue, Jan 21, 2020 at 5:40 PM Nathan Sidwell wrote: > > On 1/21/20 11:26 AM, Richard Biener wrote: > > On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka > > wrote: > > >> I would say we want to fix that... Even GCC gets idea to put # info > >> filenames and I am sure except for HHVM there are others. > > > > Can't w just quote them somehow? > > IIUC there are two issues > > 1) Make's syntax. Its quoting is baroque, (and incomplete?). See the > comment in libcpp/mkdeps.c, which I think is curtesy of ZackW. I see > that the just-released Make 4.3 has changed something to do with # and > its need to be quoted in some contexts. Ah, maybe it's worth to export this functionality to libiberty? > 2) Bad chars for the underlying filesystem. But, if the temp files are > on the same FS as the thing they're temping for, then there shouldn't be > a conflict. If they are different, then one might hope the temp-fs > allows a superset. Yeah, so I'd suggest to simply avoid the bad names being created from the WPA stage and instead sanitize it there already. That would make it also consistent when debugging. Not to say that using a tempfile by default when not doing -save-temps would be appropriate anyway. Richard. > nathan > > -- > Nathan Sidwell
[PATCH 3/4] [ARC] Save mlo/mhi registers when ISR.
ARC600 when configured with mul64 instructions uses mlo and mhi registers to store the 64 result of the multiplication. In the ARC600 ISA documentation we have the next register configuration when ARC600 is configured only with mul64 extension: Register | Name | Use -+--+ r57 | mlo | Multiply low 32 bits, read only r58 | mmid | Multiply middle 32 bits, read only r59 | mhi | Multiply high 32 bits, read only - When used for Co-existence configurations we have for mul64 the next registers used: Register | Name | Use -+--+ r58 | mlo | Multiply low 32 bits, read only r59 | mhi | Multiply high 32 bits, read only - Note that mlo/mhi assignment doesn't swap when bigendian CPU configuration is used. The compiler will always use r58 for mlo, regardless of the configuration choosen to ensure mlo/mhi correct splitting. Fixing mlo to the right register number is done at assembly time. The dwarf info is also notified via DBX_... macro. Both mlo/mhi registers needs to saved when ISR happens using a custom sequence. gcc/ -xx-xx Claudiu Zissulescu * config/arc/arc-protos.h (gen_mlo): Remove. (gen_mhi): Likewise. * config/arc/arc.c (AUX_MULHI): Define. (arc_must_save_reister): Special handling for r58/59. (arc_compute_frame_size): Consider mlo/mhi registers. (arc_save_callee_saves): Emit fp/sp move only when emit_move paramter is true. (arc_conditional_register_usage): Remove TARGET_BIG_ENDIAN from mlo/mhi name selection. (arc_restore_callee_saves): Don't early restore blink when ISR. (arc_expand_prologue): Add mlo/mhi saving. (arc_expand_epilogue): Add mlo/mhi restoring. (gen_mlo): Remove. (gen_mhi): Remove. * config/arc/arc.h (DBX_REGISTER_NUMBER): Correct register numbering when MUL64 option is used. (DWARF2_FRAME_REG_OUT): Define. * config/arc/arc.md (arc600_stall): New pattern. (VUNSPEC_ARC_ARC600_STALL): Define. (mulsi64): Use correct mlo/mhi registers. (mulsi_600): Clean it up. * config/arc/predicates.md (mlo_operand): Remove any dependency on TARGET_BIG_ENDIAN. (mhi_operand): Likewise. testsuite/ -xx-xx Claudiu Zissulescu * gcc.target/arc/code-density-flag.c: Update test. * gcc.target/arc/interrupt-6.c: Likewise. --- gcc/config/arc/arc-protos.h | 2 - gcc/config/arc/arc.c | 279 +++--- gcc/config/arc/arc.h | 27 +- gcc/config/arc/arc.md | 43 ++- gcc/config/arc/predicates.md | 4 +- .../gcc.target/arc/code-density-flag.c| 1 + gcc/testsuite/gcc.target/arc/interrupt-6.c| 2 +- 7 files changed, 220 insertions(+), 138 deletions(-) diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h index 2a13b0f19f0..c72d78e3b9e 100644 --- a/gcc/config/arc/arc-protos.h +++ b/gcc/config/arc/arc-protos.h @@ -75,8 +75,6 @@ extern int arc_hazard (rtx_insn *, rtx_insn *); extern int arc_write_ext_corereg (rtx); extern rtx gen_acc1 (void); extern rtx gen_acc2 (void); -extern rtx gen_mlo (void); -extern rtx gen_mhi (void); extern bool arc_branch_size_unknown_p (void); struct arc_ccfsm; extern void arc_ccfsm_record_condition (rtx, bool, rtx_insn *, diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c index 25d66e9cac9..fc174361b02 100644 --- a/gcc/config/arc/arc.c +++ b/gcc/config/arc/arc.c @@ -213,6 +213,9 @@ static int rgf_banked_register_count; /* FPX AUX registers. */ #define AUX_DPFP_START 0x301 +/* ARC600 MULHI register. */ +#define AUX_MULHI 0x12 + /* A nop is needed between a 4 byte insn that sets the condition codes and a branch that uses them (the same isn't true for an 8 byte insn that sets the condition codes). Set by arc_ccfsm_advance. Used by @@ -1919,8 +1922,8 @@ arc_conditional_register_usage (void) this way, we don't have to carry clobbers of that reg around in every isntruction that modifies mlo and/or mhi. */ strcpy (rname57, ""); - strcpy (rname58, TARGET_BIG_ENDIAN ? "mhi" : "mlo"); - strcpy (rname59, TARGET_BIG_ENDIAN ? "mlo" : "mhi"); + strcpy (rname58, "mlo"); + strcpy (rname59, "mhi"); } /* The nature of arc_tp_regno is actually something more like a global @@ -2711,8 +2714,6 @@ arc_must_save_register (int regno, struct function *func, bool special_p) case R55_REG: case R56_REG: case R57_REG: -case R58_REG: -case R59_REG: /* The Extension Registers. */ if (ARC_INTERRUPT_P (fn_type) && (df_regs_ever_live_p (RETURN_ADDR_REGNUM) @@ -2723,6 +2724,20 @@ arc_must_save_register (int re
[PATCH 1/4] [ARC] Make libgcc compatible with ARC's reduced register set config.
ARC processors can work with a reduced register set (i.e. registers r4-r9 and r16-r25 are not available). This option can be enabled passing -mrf16 option to the compiler, or by using -mcpu=em_mini CPU configuration. Using RF16 config requires all the hand-made assembly files used in libgcc to have the corresponding RF16 object attribute set. This patch qualifies the relevant hand-made assembly files to RF16 config, and also adds generic c-functions for the one which are not. libgcc/ -xx-xx Claudiu Zissulescu * config/arc/crti.S: Add RF16 object attribute. * config/arc/crtn.S: Likewise. * config/arc/crttls.S: Likewise. * config/arc/lib1funcs.S: Likewise. * config/arc/fp-hack.h (ARC_OPTFPE): Define. * config/arc/lib2funcs.c: New file. * config/arc/t-arc: Add lib2funcs to LIB2ADD. --- libgcc/config/arc/crti.S | 5 ++ libgcc/config/arc/crtn.S | 5 ++ libgcc/config/arc/crttls.S| 6 +++ libgcc/config/arc/fp-hack.h | 5 +- libgcc/config/arc/lib1funcs.S | 27 +-- libgcc/config/arc/lib2funcs.c | 88 +++ libgcc/config/arc/t-arc | 1 + 7 files changed, 120 insertions(+), 17 deletions(-) create mode 100644 libgcc/config/arc/lib2funcs.c diff --git a/libgcc/config/arc/crti.S b/libgcc/config/arc/crti.S index 297ddc75ad5..e05a78903a0 100644 --- a/libgcc/config/arc/crti.S +++ b/libgcc/config/arc/crti.S @@ -28,6 +28,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # This file contains the stack frame setup for contents of the .fini and # .init sections. +#ifdef __ARC_RF16__ + /* Use object attributes to inform other tools this file is + safe for RF16 configuration. */ + .arc_attribute Tag_ARC_ABI_rf16, 1 +#endif .section .init .global _init .word 0 diff --git a/libgcc/config/arc/crtn.S b/libgcc/config/arc/crtn.S index fc6197fb2fc..37eac5e6ada 100644 --- a/libgcc/config/arc/crtn.S +++ b/libgcc/config/arc/crtn.S @@ -28,6 +28,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # This file just makes sure that the .fini and .init sections do in # fact return. This file is the last thing linked into any executable. +#ifdef __ARC_RF16__ + /* Use object attributes to inform other tools this file is + safe for RF16 configuration. */ + .arc_attribute Tag_ARC_ABI_rf16, 1 +#endif .section .init pop_s blink j_s [blink] diff --git a/libgcc/config/arc/crttls.S b/libgcc/config/arc/crttls.S index 4c8faf9454f..b5aebf19ffb 100644 --- a/libgcc/config/arc/crttls.S +++ b/libgcc/config/arc/crttls.S @@ -33,6 +33,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see the executable file might be covered by the GNU General Public License. */ +#ifdef __ARC_RF16__ + /* Use object attributes to inform other tools this file is + safe for RF16 configuration. */ + .arc_attribute Tag_ARC_ABI_rf16, 1 +#endif + #if (__ARC_TLS_REGNO__ != -1) /* ANSI concatenation macros. */ diff --git a/libgcc/config/arc/fp-hack.h b/libgcc/config/arc/fp-hack.h index 86b63d9cfa5..65cdcdd4cc6 100644 --- a/libgcc/config/arc/fp-hack.h +++ b/libgcc/config/arc/fp-hack.h @@ -30,7 +30,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #define ARC_FP_DEBUG 1 #define FINE_GRAINED_LIBRARIES -#define ARC_OPTFPE (defined (__ARC700__) || defined (__ARC_FPX_QUARK__)) + +#if defined (__ARC700__) || defined (__ARC_FPX_QUARK__) +#define ARC_OPTFPE 1 +#endif #if !ARC_OPTFPE || ARC_FP_DEBUG #define L_pack_sf diff --git a/libgcc/config/arc/lib1funcs.S b/libgcc/config/arc/lib1funcs.S index 1ada0fed74d..cc54b4022ec 100644 --- a/libgcc/config/arc/lib1funcs.S +++ b/libgcc/config/arc/lib1funcs.S @@ -53,7 +53,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #define ENDFUNC0(X) .Lfe_##X: .size X,.Lfe_##X-X #define ENDFUNC(X) ENDFUNC0(X) - +#ifdef __ARC_RF16__ + /* Use object attributes to inform other tools this file is + safe for RF16 configuration. */ + .arc_attribute Tag_ARC_ABI_rf16, 1 +#endif #ifdef L_mulsi3 .section .text @@ -232,6 +236,7 @@ SYM(__umulsi3_highpart): #endif #endif /* L_umulsidi3 */ +#ifndef __ARC_RF16__ #ifdef L_muldi3 .section .text .align 4 @@ -285,6 +290,7 @@ SYM(__muldi3): #endif /* __LITTLE_ENDIAN__ */ ENDFUNC(__muldi3) #endif /* L_muldi3 */ +#endif /* !__ARC_RF16__ */ #ifdef L_umulsi3_highpart #include "ieee-754/arc-ieee-754.h" @@ -544,12 +550,6 @@ SYM(__udivmodsi4): SYM(__udivsi3): b @SYM(__udivmodsi4) ENDFUNC(__udivsi3) -#if 0 /* interferes with linux loader */ - .section .__arc_profile_forward, "a" - .long SYM(__udivsi3) - .long SYM(__udivmodsi4) - .long 65536 -#endif #endif /* L_udivsi3 */ @@ -948,12 +948,6 @@ SYM(__umodsi3): j.d [r7] mo
[PATCH 2/4] [ARC] Propagate uncached type attribute.
Like `packed` type attribute, the ARC's `uncached` type attribute
needs to be propagated to each member of the struct where it is used.
Fix this behavior and add a test.
gcc/
-xx-xx Claudiu Zissulescu
* config/arc/arc.c (arc_is_uncached_mem_p): Check struct
attributes if needed.
testsuite/
-xx-xx Claudiu Zissulescu
* gcc.target/arc/uncached-3.c: New test.
[ARC] Fix pending errors with uncached type attribute.
uncached type attribute applied to a structure needs to be propagated
to its members, triggering the .di flag for any access of the struct
members. Also, any complex CFG manipulation may drop memory pointer
type attributes, leading to the impossibility to discriminate the
direct accesses from normal ones. To solve this issue, we will treat
the direct memory accessed specially via unspecs.
gcc/
-xx-xx Claudiu Zissulescu
Petro Karashchenko
* config/arc/arc.c (prepare_move_operands): Generate special
unspec instruction for direct access.
(arc_isuncached_mem_p): Propagate uncached attribute to each
structure member.
* config/arc/arc.md (VUNSPEC_ARC_LDDI): Define.
(VUNSPEC_ARC_STDI): Likewise.
(ALLI): New mode iterator.
(mALLI): New mode attribute.
(lddi): New instruction pattern.
(stdi): Likewise.
(stdidi_split): Split instruction for architectures which are not
supporting ll64 option.
(lddidi_split): Likewise.
testsuite/
-xx-xx Claudiu Zissulescu
Petro Karashchenko
* gcc.target/arc/uncached-4.c: New file.
* gcc.target/arc/uncached-5.c: Likewise.
* gcc.target/arc/uncached-6.c: Likewise.
* gcc.target/arc/uncached-7.c: Likewise.
* gcc.target/arc/uncached-8.c: Likewise.
* gcc.target/arc/arc.exp (ll64): New predicate.
---
gcc/config/arc/arc.c | 118 ++
gcc/config/arc/arc.md | 60 +++
gcc/testsuite/gcc.target/arc/arc.exp | 9 ++
gcc/testsuite/gcc.target/arc/uncached-1.c | 2 +-
gcc/testsuite/gcc.target/arc/uncached-2.c | 2 +-
gcc/testsuite/gcc.target/arc/uncached-3.c | 22
gcc/testsuite/gcc.target/arc/uncached-4.c | 42
gcc/testsuite/gcc.target/arc/uncached-5.c | 29 ++
gcc/testsuite/gcc.target/arc/uncached-6.c | 35 +++
gcc/testsuite/gcc.target/arc/uncached-7.c | 11 ++
gcc/testsuite/gcc.target/arc/uncached-8.c | 33 ++
11 files changed, 321 insertions(+), 42 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-3.c
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-4.c
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-5.c
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-6.c
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-7.c
create mode 100644 gcc/testsuite/gcc.target/arc/uncached-8.c
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index c0b2a8de2f1..25d66e9cac9 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -9208,49 +9208,71 @@ arc_get_aux_arg (rtx pat, int *auxr)
bool
prepare_move_operands (rtx *operands, machine_mode mode)
{
- /* First handle aux attribute. */
- if (mode == SImode
- && (MEM_P (operands[0]) || MEM_P (operands[1])))
+ if ((MEM_P (operands[0]) || MEM_P (operands[1]))
+ && SCALAR_INT_MODE_P (mode))
{
- rtx tmp;
- int auxr = 0;
- if (MEM_P (operands[0]) && arc_is_aux_reg_p (operands[0]))
+ /* First handle aux attribute. */
+ if (mode == SImode)
{
- /* Save operation. */
- if (arc_get_aux_arg (operands[0], &auxr))
+ rtx tmp;
+ int auxr = 0;
+ if (MEM_P (operands[0]) && arc_is_aux_reg_p (operands[0]))
{
- tmp = gen_reg_rtx (SImode);
- emit_move_insn (tmp, GEN_INT (auxr));
+ /* Save operation. */
+ if (arc_get_aux_arg (operands[0], &auxr))
+ {
+ tmp = gen_reg_rtx (SImode);
+ emit_move_insn (tmp, GEN_INT (auxr));
+ }
+ else
+ tmp = XEXP (operands[0], 0);
+
+ operands[1] = force_reg (SImode, operands[1]);
+ emit_insn (gen_rtx_UNSPEC_VOLATILE
+(VOIDmode, gen_rtvec (2, operands[1], tmp),
+ VUNSPEC_ARC_SR));
+ return true;
}
- else
+ if (MEM_P (operands[1]) && arc_is_aux_reg_p (operands[1]))
{
- tmp = XEXP (operands[0], 0);
+ if (arc_get_aux_arg (operands[1], &auxr))
+ {
+ tmp = gen_reg_rtx (SImode);
+ emit_move_insn (tmp, GEN_INT (auxr));
+ }
+ else
+ {
+ tmp = XEXP (operands[1], 0);
+ gcc_assert (GET_CODE (tmp) == SYMBOL_REF);
+ }
+ /* Load operation. *
[PATCH 4/4] [ARC] Update ARC600 multiplication cost.
The ARC's 600 multiplication instruction can accept signed 12 bit instructions. gcc/ -xx-xx Claudiu Zissulescu * config/arc/arc.c (arc_rtx_costs): Update mul64 cost. --- gcc/config/arc/arc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c index fc174361b02..1800687ef12 100644 --- a/gcc/config/arc/arc.c +++ b/gcc/config/arc/arc.c @@ -5887,6 +5887,12 @@ arc_rtx_costs (rtx x, machine_mode mode, int outer_code, nolimm = true; if (satisfies_constraint_Clo (x)) nolimm = true; + break; + case MULT: + if (TARGET_MUL64_SET) + if (SIGNED_INT12 (INTVAL (x))) + nolimm = true; + break; default: break; } -- 2.24.1
Re: Fix updating of call_stmt_site_hash
On Tue, Jan 21, 2020 at 8:09 PM Jan Hubicka wrote:
>
> > On January 21, 2020 4:37:31 PM GMT+01:00, Jan Hubicka
> > wrote:
> > >Hi,
> > >this patch fixes ICE causes by call stmt site hash going out of sync.
> > >For
> > >speculative edges it is assumed to contain a direct call so if we are
> > >removing it hashtable needs to be updated. I realize that the code is
> > >ugly
> > >but I will leave cleanup for next stage1.
> > >
> > >Bootstrapped/regtested x86_64-linux. This patch makes it possible to
> > >build
> > >Firefox again.
> >
> > It even looks quadratic? Can't you simply lookup the stmt?
>
> I am not sure what you mean by looking up the stmt. To go from stmt to
> call edge you either walk the callees chain or use call_site_hash that
> populates itself when you have more than 100 callees. The situation
> this code solves is that with speculative calls have multiple direct
> edges, one indirect and multiple refs associated with the speculated
> call stmt. Now one of direct edges gets removed and at this moment one
> needs to look up different one to re-populate the hash.
I see. I thought that maybe we have the hash record stmt -> { set of edges }
but if we have only stmt -> single-edge then indeed that's bad.
> I meant to fix the ICE first, but indeed resolve_speuclations and
> redirect_stmt_to_callee is O(num_callees+num_refs) in worst case here
> which needs to be fixed (though the case is quite rare in practice).
At least isolating this "detail" into a function might be nice...
> One can keep direct edges in a block in the lists (with verifier help),
> but still looking up referneces is bit hard since they are in vector and
> that is re-numbered on inserts and deletes. I plan to fix this
> incrementally (we want also refs hash and verifier for the caches, plus
> this all is memory use critical)
Thanks,
Richard.
>
> Honza
Re: [PATCH] powerpc: Fix ICE with fp conditional move (PR target/93073)
Segher Boessenkool wrote: On Mon, Jan 20, 2020 at 11:52:55PM +0100, Jakub Jelinek wrote: PR target/93073 * config/rs6000/rs6000.c (rs6000_emit_cmove): Punt for compare_mode other than SFmode or DFmode. "If using fsel, punt for..." etc. + /* Don't allow compare_mode other than SFmode or DFmode, for others there + is no fsel instruction. */ + if (compare_mode != SFmode && compare_mode != DFmode) +return 0; And move this a bit later, to right after /* At this point we know we can use fsel. */ please? Okay for trunk with that change, and for release branches a bit later (wait for AIX and Darwin test results if you can?) I did not see any regressions on powerpc-darwin9 for r10-6104 c.f r10-6081. thanks Iain
[PATCH] aarch64: Fix aarch64_expand_subvti constant handling [PR93335]
Hi!
The two patterns that call aarch64_expand_subvti ensure that {low,high}_in1
is a register, while {low,high}_in2 can be a register or immediate.
subdi3_compare1_imm uses the aarch64_plus_immediate predicate for its last
two operands (the value and negated value), but aarch64_expand_subvti calls
it whenever low_in2 is a CONST_INT, which leads to ICEs during vregs pass,
as the emitted insn is not recognized as valid subdi3_compare1_imm.
The following patch fixes that by only using subdi3_compare1_imm if it is ok
to do so, and otherwise force the constant into register and use the
non-immediate version - subdi3_compare1.
Furthermore, previously the code was calling force_reg on high_in2 only if
low_in2 is CONST_INT, on the (reasonable) assumption is that only if low_in2
is a CONST_INT, high_in2 can be non-REG, but with the above changes even in
the else we might have CONST_INT and force_reg doesn't do anything if the
operand is already a REG, so this patch calls it unconditionally.
Bootstrapped/regtested on aarch64-linux, ok for trunk and 9.3?
2020-01-22 Jakub Jelinek
PR target/93335
* config/aarch64/aarch64.c (aarch64_expand_subvti): Only use
gen_subdi3_compare1_imm if low_in2 satisfies aarch64_plus_immediate
predicate, not whenever it is CONST_INT. Otherwise, force_reg it.
Call force_reg on high_in2 unconditionally.
* gcc.c-torture/compile/pr93335.c: New test.
--- gcc/config/aarch64/aarch64.c.jj 2020-01-21 17:52:54.932504456 +0100
+++ gcc/config/aarch64/aarch64.c2020-01-21 20:04:09.992921849 +0100
@@ -20193,14 +20193,15 @@ aarch64_expand_subvti (rtx op0, rtx low_
}
else
{
- if (CONST_INT_P (low_in2))
+ if (aarch64_plus_immediate (low_in2, DImode))
+ emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
+ GEN_INT (-INTVAL (low_in2;
+ else
{
- high_in2 = force_reg (DImode, high_in2);
- emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
- GEN_INT (-INTVAL (low_in2;
+ low_in2 = force_reg (DImode, low_in2);
+ emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
}
- else
- emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
+ high_in2 = force_reg (DImode, high_in2);
if (unsigned_p)
emit_insn (gen_usubdi3_carryinC (high_dest, high_in1, high_in2));
--- gcc/testsuite/gcc.c-torture/compile/pr93335.c.jj2020-01-21
20:04:27.728654103 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr93335.c 2020-01-21
20:03:55.384142397 +0100
@@ -0,0 +1,98 @@
+/* PR target/93335 */
+/* { dg-do compile { target int128 } } */
+
+int
+f1 (unsigned int x)
+{
+ return __builtin_sub_overflow_p (x, 4096, (unsigned __int128) 0);
+}
+
+int
+f2 (unsigned int x)
+{
+ return __builtin_sub_overflow_p (x, 4097, (unsigned __int128) 0);
+}
+
+int
+f3 (int x)
+{
+ return __builtin_sub_overflow_p (x, 4096, (__int128) 0);
+}
+
+int
+f4 (int x)
+{
+ return __builtin_sub_overflow_p (x, 4097, (__int128) 0);
+}
+
+int
+f5 (unsigned int x)
+{
+ return __builtin_sub_overflow_p (x, -4096, (unsigned __int128) 0);
+}
+
+int
+f6 (unsigned int x)
+{
+ return __builtin_sub_overflow_p (x, -4097, (unsigned __int128) 0);
+}
+
+int
+f7 (int x)
+{
+ return __builtin_sub_overflow_p (x, -4096, (__int128) 0);
+}
+
+int
+f8 (int x)
+{
+ return __builtin_sub_overflow_p (x, -4097, (__int128) 0);
+}
+
+int
+f9 (unsigned int x)
+{
+ return __builtin_add_overflow_p (x, 4096, (unsigned __int128) 0);
+}
+
+int
+f10 (unsigned int x)
+{
+ return __builtin_add_overflow_p (x, 4097, (unsigned __int128) 0);
+}
+
+int
+f11 (int x)
+{
+ return __builtin_add_overflow_p (x, 4096, (__int128) 0);
+}
+
+int
+f12 (int x)
+{
+ return __builtin_add_overflow_p (x, 4097, (__int128) 0);
+}
+
+int
+f13 (unsigned int x)
+{
+ return __builtin_add_overflow_p (x, -4096, (unsigned __int128) 0);
+}
+
+int
+f14 (unsigned int x)
+{
+ return __builtin_add_overflow_p (x, -4097, (unsigned __int128) 0);
+}
+
+int
+f15 (int x)
+{
+ return __builtin_add_overflow_p (x, -4096, (__int128) 0);
+}
+
+int
+f16 (int x)
+{
+ return __builtin_add_overflow_p (x, -4097, (__int128) 0);
+}
Jakub
[wwwdocs] Improve the presentation of our instructions on checksuming.
On the way remove a note about binary snapshots that's been commented for a while. Pushed. Gerald --- htdocs/snapshots.html | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/htdocs/snapshots.html b/htdocs/snapshots.html index 6567a857..7776d95e 100644 --- a/htdocs/snapshots.html +++ b/htdocs/snapshots.html @@ -37,17 +37,11 @@ files so that autoconf et al aren't needed. The release script generates the file sha512.sum that provides a 512-bit checksum for the tarball and other files in the snapshot directory. Use the following command -to verify the sources: - -sha512sum --check --ignore-missing sha512.sum +to verify the sources: - + +sha512sum --check --ignore-missing sha512.sum + -- 2.24.1
[committed] openmp: Teach omp_code_to_statement about rest of OpenMP statements (PR93329)
Hi!
The omp_code_to_statement function added with the initial OpenACC support
only handled small subset of the OpenMP statements, leading to ICE if
any other OpenMP directive appeared inside of OpenACC directive.
The following patch teaches it to handle the rest and adds testcase that
verifies that, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.
2020-01-22 Jakub Jelinek
PR fortran/93329
* openmp.c (omp_code_to_statement): Handle remaining EXEC_OMP_*
cases.
* gfortran.dg/goacc/pr93329.f90: New test.
--- gcc/fortran/openmp.c.jj 2020-01-12 11:54:36.594410678 +0100
+++ gcc/fortran/openmp.c2020-01-21 16:05:21.271520015 +0100
@@ -5898,6 +5898,81 @@ omp_code_to_statement (gfc_code *code)
return ST_OMP_PARALLEL_WORKSHARE;
case EXEC_OMP_DO:
return ST_OMP_DO;
+case EXEC_OMP_ATOMIC:
+ return ST_OMP_ATOMIC;
+case EXEC_OMP_BARRIER:
+ return ST_OMP_BARRIER;
+case EXEC_OMP_CANCEL:
+ return ST_OMP_CANCEL;
+case EXEC_OMP_CANCELLATION_POINT:
+ return ST_OMP_CANCELLATION_POINT;
+case EXEC_OMP_FLUSH:
+ return ST_OMP_FLUSH;
+case EXEC_OMP_DISTRIBUTE:
+ return ST_OMP_DISTRIBUTE;
+case EXEC_OMP_DISTRIBUTE_PARALLEL_DO:
+ return ST_OMP_DISTRIBUTE_PARALLEL_DO;
+case EXEC_OMP_DISTRIBUTE_PARALLEL_DO_SIMD:
+ return ST_OMP_DISTRIBUTE_PARALLEL_DO_SIMD;
+case EXEC_OMP_DISTRIBUTE_SIMD:
+ return ST_OMP_DISTRIBUTE_SIMD;
+case EXEC_OMP_DO_SIMD:
+ return ST_OMP_DO_SIMD;
+case EXEC_OMP_SIMD:
+ return ST_OMP_SIMD;
+case EXEC_OMP_TARGET:
+ return ST_OMP_TARGET;
+case EXEC_OMP_TARGET_DATA:
+ return ST_OMP_TARGET_DATA;
+case EXEC_OMP_TARGET_ENTER_DATA:
+ return ST_OMP_TARGET_ENTER_DATA;
+case EXEC_OMP_TARGET_EXIT_DATA:
+ return ST_OMP_TARGET_EXIT_DATA;
+case EXEC_OMP_TARGET_PARALLEL:
+ return ST_OMP_TARGET_PARALLEL;
+case EXEC_OMP_TARGET_PARALLEL_DO:
+ return ST_OMP_TARGET_PARALLEL_DO;
+case EXEC_OMP_TARGET_PARALLEL_DO_SIMD:
+ return ST_OMP_TARGET_PARALLEL_DO_SIMD;
+case EXEC_OMP_TARGET_SIMD:
+ return ST_OMP_TARGET_SIMD;
+case EXEC_OMP_TARGET_TEAMS:
+ return ST_OMP_TARGET_TEAMS;
+case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE:
+ return ST_OMP_TARGET_TEAMS_DISTRIBUTE;
+case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO:
+ return ST_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO;
+case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD:
+ return ST_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD;
+case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_SIMD:
+ return ST_OMP_TARGET_TEAMS_DISTRIBUTE_SIMD;
+case EXEC_OMP_TARGET_UPDATE:
+ return ST_OMP_TARGET_UPDATE;
+case EXEC_OMP_TASKGROUP:
+ return ST_OMP_TASKGROUP;
+case EXEC_OMP_TASKLOOP:
+ return ST_OMP_TASKLOOP;
+case EXEC_OMP_TASKLOOP_SIMD:
+ return ST_OMP_TASKLOOP_SIMD;
+case EXEC_OMP_TASKWAIT:
+ return ST_OMP_TASKWAIT;
+case EXEC_OMP_TASKYIELD:
+ return ST_OMP_TASKYIELD;
+case EXEC_OMP_TEAMS:
+ return ST_OMP_TEAMS;
+case EXEC_OMP_TEAMS_DISTRIBUTE:
+ return ST_OMP_TEAMS_DISTRIBUTE;
+case EXEC_OMP_TEAMS_DISTRIBUTE_PARALLEL_DO:
+ return ST_OMP_TEAMS_DISTRIBUTE_PARALLEL_DO;
+case EXEC_OMP_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD:
+ return ST_OMP_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD;
+case EXEC_OMP_TEAMS_DISTRIBUTE_SIMD:
+ return ST_OMP_TEAMS_DISTRIBUTE_SIMD;
+case EXEC_OMP_PARALLEL_DO:
+ return ST_OMP_PARALLEL_DO;
+case EXEC_OMP_PARALLEL_DO_SIMD:
+ return ST_OMP_PARALLEL_DO_SIMD;
+
default:
gcc_unreachable ();
}
--- gcc/testsuite/gfortran.dg/goacc/pr93329.f90.jj 2020-01-21
16:33:13.235417921 +0100
+++ gcc/testsuite/gfortran.dg/goacc/pr93329.f90 2020-01-21 16:33:03.043571159
+0100
@@ -0,0 +1,223 @@
+! PR fortran/93329
+! { dg-do compile { target fopenmp } }
+! { dg-additional-options "-fopenmp" }
+
+ integer :: x, y, z
+ integer :: a(32)
+!$acc kernels copyout(x)
+!$omp target map(from:x) ! { dg-error "OMP TARGET directive cannot be
specified within" }
+ x = 5
+!$omp end target
+!$acc end kernels
+ print *, x
+!$acc kernels
+!$omp atomic ! { dg-error "OMP ATOMIC directive cannot be
specified within" }
+ x = x + 1
+!$omp end atomic
+!$acc end kernels
+!$acc kernels
+!$omp barrier ! { dg-error "OMP BARRIER directive cannot be
specified within" }
+!$acc end kernels
+!$acc kernels
+!$omp cancel parallel ! { dg-error "OMP CANCEL directive cannot be
specified within" }
+!$acc end kernels
+!$acc kernels
+!$omp cancellation point parallel ! { dg-error "OMP CANCELLATION POINT
directive cannot be specified within" }
+!$acc end kernels
+!$acc kernels
+!$omp flush! { dg-error "OMP FLUSH directive cannot be
specified within" }
+!$acc end kernels
+!$acc kernels
+!$omp distribute ! { dg-error "OMP DISTRIBUTE direct
[committed] openmp: Fix up !$omp target parallel handling
Hi!
The PR93329 fix revealed we ICE on !$omp target parallel, this change fixes
that.
Another case where some OpenMP 4.5 support has been added, but I didn't have
time to add corresponding testsuite coverage yet (~ backports of the C/C++
patches added when the C/C++ support has been added).
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2020-01-22 Jakub Jelinek
* parse.c (parse_omp_structured_block): Handle ST_OMP_TARGET_PARALLEL.
* trans-openmp.c (gfc_trans_omp_target)
: Call pushlevel first.
* gfortran.dg/gomp/target-parallel1.f90: New test.
* gfortran.dg/goacc/pr93329.f90: Enable commented out target parallel
test.
--- gcc/fortran/parse.c.jj 2020-01-12 11:54:36.594410678 +0100
+++ gcc/fortran/parse.c 2020-01-21 16:43:05.175529082 +0100
@@ -5231,6 +5231,9 @@ parse_omp_structured_block (gfc_statemen
case ST_OMP_TARGET_DATA:
omp_end_st = ST_OMP_END_TARGET_DATA;
break;
+case ST_OMP_TARGET_PARALLEL:
+ omp_end_st = ST_OMP_END_TARGET_PARALLEL;
+ break;
case ST_OMP_TARGET_TEAMS:
omp_end_st = ST_OMP_END_TARGET_TEAMS;
break;
--- gcc/fortran/trans-openmp.c.jj 2020-01-12 11:54:36.603410542 +0100
+++ gcc/fortran/trans-openmp.c 2020-01-21 16:56:13.041723952 +0100
@@ -5357,6 +5357,7 @@ gfc_trans_omp_target (gfc_code *code)
{
stmtblock_t iblock;
+ pushlevel ();
gfc_start_block (&iblock);
tree inner_clauses
= gfc_trans_omp_clauses (&block, &clausesa[GFC_OMP_SPLIT_PARALLEL],
--- gcc/testsuite/gfortran.dg/gomp/target-parallel1.f90.jj 2020-01-21
17:14:41.636075090 +0100
+++ gcc/testsuite/gfortran.dg/gomp/target-parallel1.f90 2020-01-21
17:14:27.292291382 +0100
@@ -0,0 +1,4 @@
+!$omp target parallel
+ print *, 'Hello, world'
+!$omp end target parallel
+end
--- gcc/testsuite/gfortran.dg/goacc/pr93329.f90.jj 2020-01-21
16:33:03.043571159 +0100
+++ gcc/testsuite/gfortran.dg/goacc/pr93329.f90 2020-01-21 17:15:11.508624618
+0100
@@ -68,8 +68,8 @@
!$omp target exit data map(from: x)! { dg-error "OMP TARGET EXIT DATA
directive cannot be specified within" }
!$acc end kernels
!$acc kernels
-!!$omp target parallel
-!!$omp end target parallel
+!$omp target parallel ! { dg-error "OMP TARGET PARALLEL directive
cannot be specified within" }
+!$omp end target parallel
!$acc end kernels
!$acc kernels
!$omp target parallel do ! { dg-error "OMP TARGET PARALLEL DO directive
cannot be specified within" }
Jakub
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On Tue, Jan 21, 2020 at 06:50:13PM +, Richard Earnshaw (lists) wrote: > > Doesn't this use of [] have the same problem with git am? > > No, because only 'leading' [] blocks are removed - git mailinfo --help I've used openmp: Teach omp_code_to_statement about rest of OpenMP statements [PR93329] as the first line of a commit log and that [PR93329] part is completely gone from it. Jakub
[wwwdocs] Remove svn.html which is not referenced internally any longer.
Not only have I cleaned up all internal references, there's also a redirect from svn.html to git.html in place, so users should never get a 404. Pushed. Gerald --- htdocs/svn.html | 1219 --- 1 file changed, 1219 deletions(-) delete mode 100644 htdocs/svn.html diff --git a/htdocs/svn.html b/htdocs/svn.html deleted file mode 100644 index dc60e7c8.. --- a/htdocs/svn.html +++ /dev/null @@ -1,1219 +0,0 @@ - - - - - - - -GCC: Anonymous read-only SVN access -https://gcc.gnu.org/gcc.css"; /> - - - - - -GCC: Anonymous read-only SVN access - -Our SVN source repository is available read-only to the public at -large. That way you can pick up any version (including releases) of -GCC that is in our repository. - -In addition you can https://gcc.gnu.org/viewcvs/gcc/";>browse -our SVN history online. - -(Our web pages are managed via git.) - - -Using the SVN repository - -Assuming you have version 1.0.0 and higher of -http://subversion.tigris.org/";>Subversion installed, you can -check out the GCC sources using the following command: - - -svn checkout svn://gcc.gnu.org/svn/gcc/trunk SomeLocalDir - - -If you are behind a firewall that does not allow the svn protocol -through, you can replace svn:// with http://. -You may also need to modify your subversion servers file -(~/.subversion/servers) to set http-proxy-host and -http-proxy-port. You should only use the http protocol if -the svn protocol does not work; the http protocol has a higher server -overhead associated with it and will be slower. - -There is more https://gcc.gnu.org/wiki/SvnHelp";>information -about Subversion specifically for GCC in the GCC Wiki. - - - - -Generated files - -Our source tree contains a number of files that are generated -from other source files by build tools such as Bison, Autoconf, and -Gperf. Bison is now required when using SVN to access our sources, -but all other generated files are included in the source tree so that -GCC can be built without these build tools. The SVN checkout and -update operations do not insure that the timestamps of generated files -are later than those of the files they are generated from. The script -contrib/gcc_update updates the timestamps for all these -generated files. See the comments in that script for instructions on -running it. - -GCC's build system (in particular Make) uses file timestamps to -determine if a generated file needs to be updated by running a particular -build tool. Because of this, GCC's build system may believe that -a generated file needs regenerating even though its source has not -changed, and require a particular build tool to rebuild that generated -file. If the appropriate build tool is installed on your system, then -this will not be a problem. If you do not intend to make changes to -the source, you can avoid installing these build tools by running -contrib/gcc_update. - -There has been some discussion of removing these generated files -from GCC's SVN source tree (there is no discussion of removing them -from the released source tarballs). If that happens then -building GCC from the SVN source tree would require installing -the above mentioned build tools. Installing these build tools is not -particularly difficult, but can be time consuming especially if you -only occasionally install GCC on a particular system. - -The build tools that GCC uses are all available from the GNU -Project (see http://www.gnu.org";>http://www.gnu.org), -are often already available on many systems, and can often be found -already built for some systems. A partial list of these build tools -is: Autoconf, Bison, Xgettext, Automake, and Gperf. - -Conflicts when using svn update - -It is not uncommon to get svn conflict messages for some generated files -when updating your local sources from the SVN repository. Typically such -conflicts occur with autoconf generated files. - -As long as you haven't been making modifications to the generated files -or the generator files, it is safe to delete the offending file, then run -svn update again to get a new copy. - - -Branches and Tags - -A branch called branchname can be checked out with the -following command: - - -svn co svn://gcc.gnu.org/svn/gcc/branches/branchname gcc - - -(The release branch of the GCC SERIES release series -is named gcc-SERIES-branch.) - - -Similarly a tag called tagname can be checked out with the -following command: - - -svn co svn://gcc.gnu.org/svn/gcc/tags/tagname gcc - - -(The SVN tag for GCC X.Y.Z is of the form -gcc_X_Y_Z_release.) - - -The following list provides some representative examples: - - - gcc-5-branch - gcc-4_9-branch - gcc_4_9_3_release - gcc_4_9_2_release - gcc_4_9_1_release - gcc_4_9_0_release - gcc-4_8-branch - gcc-3_4-branch - - -To get a list of available branches, use the command: - - -svn ls svn://gcc.gnu.org/svn/gcc/branches - - - - - To find out the revision/date at which a branch or tag was created, use -the command svn log --stop-on-
Re: [wwwdocs] Add GCC10 IPA/LTO changes
> Hi, > here are some of changes of LTO/IPA done in GCC10. There is also > recursive cloning and some other stuff I will add incrementally as well > as some data on overall compile time/memory use improvements as we > reported in past years. I am still running tests and fixing bugs in this > area. > > Honza Ping... Honza > > diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html > index aca76825..0f0fce18 100644 > --- a/htdocs/gcc-10/changes.html > +++ b/htdocs/gcc-10/changes.html > @@ -50,12 +50,46 @@ a work-in-progress. > > General Improvements > > +The following GCC command line options have been introduced or > improved. > + > + href="https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-fprofile-partial-training";>-fprofile-partial-training > + can now be used to inform compiler that code paths not covered by the > + train run should not be optimized for size. > + > The following built-in functions have been introduced. > >__builtin_roundeven for the corresponding function from > ISO/IEC TS 18661. > > > +A large number of improvements to code generation have been made, > including > + but not limited to the following. > + > + Inter-procedural optimization improvements: > + > +Inter-procedural scalar replacement for aggregates (IPA-SRA) pass > was re-implemented to work at link-time. > + > + href="https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-finline-functions";>-finline-functions > + is now enabled at -O2 and was retuned for better code size > + versus runtime performance tradeofs. Inliner heuristics was also > + significantly sped up to avoid negativive impact to -flto > + -O2 compile times. > + > +Inliner heuristics and function clonning can now use value-range > +information to predict effectivity of individual > transformations. > +Selected --param values can now be specified at > +translation unit granuality. This includes all parameters controlling > +inliner. > +During link-time optimization the C++ One Definition Rule is used to > +increase precision of type based alias analysis. > + > + > + Profile driven optimization improvements: > + > +Profile maintenance during compilation was improved and hot/cold > code partitioning improved. > + > + > + > > > New Languages and Language specific improvements
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On 22/01/2020 09:07, Jakub Jelinek wrote: On Tue, Jan 21, 2020 at 06:50:13PM +, Richard Earnshaw (lists) wrote: Doesn't this use of [] have the same problem with git am? No, because only 'leading' [] blocks are removed - git mailinfo --help I've used openmp: Teach omp_code_to_statement about rest of OpenMP statements [PR93329] as the first line of a commit log and that [PR93329] part is completely gone from it. Jakub How did you apply the patch? R.
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On Wed, Jan 22, 2020 at 09:35:31AM +, Richard Earnshaw (lists) wrote: > On 22/01/2020 09:07, Jakub Jelinek wrote: > > On Tue, Jan 21, 2020 at 06:50:13PM +, Richard Earnshaw (lists) wrote: > > > > Doesn't this use of [] have the same problem with git am? > > > > > > No, because only 'leading' [] blocks are removed - git mailinfo --help > > > > I've used > > openmp: Teach omp_code_to_statement about rest of OpenMP statements > > [PR93329] > > as the first line of a commit log and that [PR93329] part is completely gone > > from it. > > How did you apply the patch? Just git commit -a and editing the log message. But I got the [PR.] not eaten in another commit, so ignore the above for now (although I'm very much convinced it was in there when I wrote the commit log). Jakub
Re: [wwwdocs] Add GCC10 IPA/LTO changes
On Mon, 30 Dec 2019, Jan Hubicka wrote: > here are some of changes of LTO/IPA done in GCC10. Quite a bit! :-) > +The following GCC command line options have been introduced or > improved. ...command-line... > + href="https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-fprofile-partial-training";>-fprofile-partial-training I suggest to use the mainline version of the docs unless you believe there is going to be significant changes (removals) in the future? https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html > + can now be used to inform compiler that code paths not covered by the > + train run should not be optimized for size. ...the compiler... (Is it "train run" or "training run"?) > +A large number of improvements to code generation have been made, > including > + but not limited to the following. Can you format for a smaller width here? Otherwise patches run a little wide. > +Inter-procedural scalar replacement for aggregates (IPA-SRA) pass > was re-implemented to work at link-time. The inter-procedural... > + href="https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-finline-functions";>-finline-functions > + is now enabled at -O2 and was retuned for better code size > + versus runtime performance tradeofs. Inliner heuristics was also ...trade-offs... (dash plus double f) > +Selected --param values can now be specified at > +translation unit granuality. This includes all parameters controlling > +inliner. ...the inliner > +Profile maintenance during compilation was improved and hot/cold How about "Profile maintenance during compilation and hot/cold code partitioning have been improved"? Okay with those changes. Thank you, Gerald
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On 21/01/2020 19:26, Segher Boessenkool wrote: Hi! Thanks for doing this. On Tue, Jan 21, 2020 at 02:52:00PM +, Richard Earnshaw (lists) wrote: This patch proposes some new (additional) rules for email subject lines when contributing to GCC. The goal is to make sure that, as far as possible, the subject for a patch will form a good summary when the message is committed to the repository if applied with 'git am'. +A high-quality e-mail subject line for a contribution contains the +following elements: + A brief summary You could stress that this is the one thing that really matters. And it's not a summary, it's much too brief for that (at most ~50 chars), but yup it should be something that allows *a human* to identify what this is. Well, it should all matter, or why are we requiring it? I'm happy to insert 'very' in front of 'brief' and to say elsewhere that the entire subject (less the leading [...] part, should rarely, if ever, go beyond 80 characters. Everything else is just convention. +A component tag is a short identifier that identifies the part of +the compiler being modified. This highlights to the relevant +maintainers that the patch may need their attention. Multiple +components may be listed if necessary. Each component tag should be +followed by a colon. Often people use aaa/bbb: if drilling down a bit that way helps keep the subject short (which is the *point* of all this: keep things better consumable for humans). The aaa: bbb: is really for when aaa and bbb are independent parts of the compiler and potentially needs multiple reviewers. aaa/bbb is when bbb is strictly a sub-component of aaa (for example, arm/SVE: would be an SVE related issue in the arm backend). I'm certainly not against having that. +The brief summary encapsulates in a few words the intent of the +change. For example: cleanup check_field_decls. It should start with a capital though. "Clean up blablal". (So no dot to end the sentence, this isn't a sentence). A capital helps the reader to quickly identify what is what, separate fluff from the core parts. There is a balance here to be made. I'm mindful of Gerald's concern that this is perhaps overly restrictive for new users already. I certainly think we should have a good house style, but getting too prescriptive just gets in the way of attracting good contributions. +Some large patch sets benefit from an introductory e-mail that +provides more context for the patch series and describes how the +patches have been broken up to provide for review. All non-trivial series, yeah. Maybe we should mention how v2 etc. of patch series should show what is changed? If there is a good standard practice for that at all :-) Dunno. I think that belongs primarily in the v2 0/n mail. It's not, after all, something that needs to be kept once the patches are applied. R. Segher
Re: [patch] contrib: script to create a new vendor branch
On 21/01/2020 23:23, Hans-Peter Nilsson wrote: From: "Richard Earnshaw (lists)" Date: Tue, 21 Jan 2020 14:36:32 +0100 Correction, the branch should be named /, so the push should be git push vendors/ / For example, for the ARM vendor, the push would be git push vendors/ARM ARM/ R. will work as expected. Run the script as contrib/git-add-vendor-branch.sh / the space must have previously been set up in the way git-fetch-vendor.sh expects. * git-add-vendor-bransh.sh: New file. (typo "bransh") Duh! will fix before push. Thanks, this and your previous reply certainly helped! Yes, I saw from the gcc-cvs list that you were merrily pushing now. Any chance of a git-add-user-branch.sh too, while you're at it? Or maybe it's just the corresponding push command in there, that's needed. On the todo list... unless someone beats me to it. Mean-time my gitwrite.html patch describes how to set up a user branch. R. brgds, H-P
Re: [patch] contrib: script to create a new vendor branch
On 22/01/2020 10:04, Richard Earnshaw (lists) wrote: On 21/01/2020 23:23, Hans-Peter Nilsson wrote: From: "Richard Earnshaw (lists)" Date: Tue, 21 Jan 2020 14:36:32 +0100 Correction, the branch should be named /, so the push should be git push vendors/ / For example, for the ARM vendor, the push would be git push vendors/ARM ARM/ R. will work as expected. Run the script as contrib/git-add-vendor-branch.sh / the space must have previously been set up in the way git-fetch-vendor.sh expects. * git-add-vendor-bransh.sh: New file. (typo "bransh") Duh! will fix before push. Thanks, this and your previous reply certainly helped! Yes, I saw from the gcc-cvs list that you were merrily pushing now. Any chance of a git-add-user-branch.sh too, while you're at it? Or maybe it's just the corresponding push command in there, that's needed. On the todo list... unless someone beats me to it. Mean-time my gitwrite.html patch describes how to set up a user branch. R. brgds, H-P Now pushed. R.
Re: [RFC c-common PATCH] PR c++/40752 - useless -Wconversion with short +=.
Hi. Just for the record, after the change 526.blender_r fails due to: blender/source/blender/blenlib/intern/math_color.c: In function 'rgb_float_to_uchar': blender/source/blender/blenlib/BLI_utildefines.h:251:22: error: conversion from 'float' to 'unsigned char' may change value [-Werror=float-conversion] 251 | #define FTOCHAR(val) ((CHECK_TYPE_INLINE(val, float)), \ | ^ blender/source/blender/blenlib/BLI_utildefines.h:257:13: note: in expansion of macro 'FTOCHAR' 257 | (v1)[0] = FTOCHAR((v2[0])); \ | ^~~ blender/source/blender/blenlib/intern/math_color.c:421:2: note: in expansion of macro 'F3TOCHAR3' 421 | F3TOCHAR3(col_f, r_col); | ^ where the project sets -Werror=float-conversion dues to: $ cat blender/source/blender/blenlib/BLI_strict_flags.h #ifdef __GNUC__ # if (__GNUC__ * 100 + __GNUC_MINOR__) >= 406 /* gcc4.6+ only */ #pragma GCC diagnostic error "-Wsign-compare" #pragma GCC diagnostic error "-Wconversion" # endif Martin
Re: Define HAVE_ for math long double functions declared in vxworks headers
On 21/01/20 21:50 -0300, Alexandre Oliva wrote: On Jan 3, 2020, Jonathan Wakely wrote: +#include +#ifdef HAVE_IEEEFP_H +# include +#endif +], [ + void (*f)(void) = (void (*)(void))$1; I wondered whether using ($1) here instead of just $1 would give any benefit. It would mean that function-like macros are ignored. The lack of parentheses after a functional macro name is enough to avoid its use as a macro. However, the test I used wouldn't avoid aliasing macros, such as #define foo bar, and it would pass even if the name was defined as an object rather than as a function. I've also come up with a way to test, during C compilation, that $1 is a function rather than anything else. If the test was in C++, we could presumably use template type resolution and static asserts to recognize function-typed expressions, though a possibility of overloading could make it fall apart. In theory the closer we are to the environment in which the test result will be used, the more likely we are to get the desired result, but I'm a little concerned about switching to a C++ test, precisely because of the math.h header that we might or might not override, and of unexpected overloading that system math headers might offer if they are C++-ready, which could then get us wrong test results. Any thoughts on this possibility? Meanwhile, here's what I'm testing now... It's supposed to be a strict incremental improvement, unless $1-&$1 unexpectedly compiles for some case I couldn't think of. Isn't allowing arithmetic on function pointers a GNU extension? It gets diagnosed with -pedantic. I think just adding the #undef to what you had originally is the best version.
Re: Define HAVE_ for math long double functions declared in vxworks headers
On Dez 25 2019, Alexandre Oliva wrote: > +dnl # Different versions and execution modes implement different > +dnl # subsets of these functions. Instead of hard-coding, test for C > +dnl # declarations in headers. The C primitives could be defined as > +dnl # macros, in which case the tests might fail, and we might have to > +dnl # switch to more elaborate tests. > +GLIBCXX_CHECK_MATH_DECLS([ > + acosl asinl atan2l atanl ceill cosl coshl expl fabsl floorl fmodl > + frexpl ldexpl log10l logl modfl powl sinl sinhl sqrtl tanl tanhl]) Why can't you use AC_CHECK_DECLS? Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: tolerate padding in mbstate_t
On 21/01/20 23:04 -0300, Alexandre Oliva wrote:
On Jan 21, 2020, Jonathan Wakely wrote:
On 21/01/20 21:36 -0300, Alexandre Oliva wrote:
Padding in mbstate_t objects may get the memcmp to fail.
Attempt to avoid the failure with zero initialization.
Regstrapped on x86_64-linux-gnu, and also tested on a platform that used
to fail because of padding in std::mbstate_t. Ok to install?
Under what conditions does this fail? Only for -std=gnu++98 and not
later standards, I assume?
Because since C++11 state_type() does perform zero-initialization of
the whole object (including padding) even if it has a default
constructor.
Err, IIUC it does so only for defaulted ctors; a user-supplied default
ctor (as in the target system) may leave padding bits uninitialized.
Yes, I misremembered.
Indeed, compiling the following snippet without optimization, with or
without -DCTOR, on x86_64, uses movw to initialize bar and the rest of
the word remains uninitialized with -DCTOR, even with -std=c++17, as we
do without -DCTOR with -std=c++98, whereas without -DCTOR and
-std=c++1[17] we use movq.
struct t {
long foo;
short bar;
#if CTOR
t() : foo(0), bar(0) {}
#endif
};
t f() {
t v = t();
return v;
}
Right. I was confusing it with this case:
struct t2 : t { };
t2 g() {
t2 v = t2();
return v;
}
This one *does* zero-initialize the padding in struct t, even though
it has a user-provided default constructor.
When optimiizing, we end up using movq in all cases, but that's probably
because of SRA.
I don't think so, that wouldn't work. I think pos02 could just be
removed from the test.
Will do.
Thanks, OK to commit.
Add News-feed item for git transition
We're missing a statement on the main news feed about the git transition. diff --git a/htdocs/index.html b/htdocs/index.html index 41bcfe18..ef85cc97 100644 --- a/htdocs/index.html +++ b/htdocs/index.html @@ -54,6 +54,10 @@ mission statement. News +GCC source repository converted to git. +[2020-01-13] +See the https://gcc.gnu.org/ml/gcc/2020-01/msg00204.html";>announcement. + GCC 7.5 released [2019-11-14]
Re: [PATCH] aarch64: Fix aarch64_expand_subvti constant handling [PR93335]
Jakub Jelinek writes:
> Hi!
>
> The two patterns that call aarch64_expand_subvti ensure that {low,high}_in1
> is a register, while {low,high}_in2 can be a register or immediate.
> subdi3_compare1_imm uses the aarch64_plus_immediate predicate for its last
> two operands (the value and negated value), but aarch64_expand_subvti calls
> it whenever low_in2 is a CONST_INT, which leads to ICEs during vregs pass,
> as the emitted insn is not recognized as valid subdi3_compare1_imm.
> The following patch fixes that by only using subdi3_compare1_imm if it is ok
> to do so, and otherwise force the constant into register and use the
> non-immediate version - subdi3_compare1.
> Furthermore, previously the code was calling force_reg on high_in2 only if
> low_in2 is CONST_INT, on the (reasonable) assumption is that only if low_in2
> is a CONST_INT, high_in2 can be non-REG, but with the above changes even in
> the else we might have CONST_INT and force_reg doesn't do anything if the
> operand is already a REG, so this patch calls it unconditionally.
>
> Bootstrapped/regtested on aarch64-linux, ok for trunk and 9.3?
>
> 2020-01-22 Jakub Jelinek
>
> PR target/93335
> * config/aarch64/aarch64.c (aarch64_expand_subvti): Only use
> gen_subdi3_compare1_imm if low_in2 satisfies aarch64_plus_immediate
> predicate, not whenever it is CONST_INT. Otherwise, force_reg it.
> Call force_reg on high_in2 unconditionally.
>
> * gcc.c-torture/compile/pr93335.c: New test.
OK, thanks.
Richard
> --- gcc/config/aarch64/aarch64.c.jj 2020-01-21 17:52:54.932504456 +0100
> +++ gcc/config/aarch64/aarch64.c 2020-01-21 20:04:09.992921849 +0100
> @@ -20193,14 +20193,15 @@ aarch64_expand_subvti (rtx op0, rtx low_
> }
>else
> {
> - if (CONST_INT_P (low_in2))
> + if (aarch64_plus_immediate (low_in2, DImode))
> + emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
> + GEN_INT (-INTVAL (low_in2;
> + else
> {
> - high_in2 = force_reg (DImode, high_in2);
> - emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
> - GEN_INT (-INTVAL (low_in2;
> + low_in2 = force_reg (DImode, low_in2);
> + emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
> }
> - else
> - emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
> + high_in2 = force_reg (DImode, high_in2);
>
>if (unsigned_p)
> emit_insn (gen_usubdi3_carryinC (high_dest, high_in1, high_in2));
> --- gcc/testsuite/gcc.c-torture/compile/pr93335.c.jj 2020-01-21
> 20:04:27.728654103 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr93335.c 2020-01-21
> 20:03:55.384142397 +0100
> @@ -0,0 +1,98 @@
> +/* PR target/93335 */
> +/* { dg-do compile { target int128 } } */
> +
> +int
> +f1 (unsigned int x)
> +{
> + return __builtin_sub_overflow_p (x, 4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f2 (unsigned int x)
> +{
> + return __builtin_sub_overflow_p (x, 4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f3 (int x)
> +{
> + return __builtin_sub_overflow_p (x, 4096, (__int128) 0);
> +}
> +
> +int
> +f4 (int x)
> +{
> + return __builtin_sub_overflow_p (x, 4097, (__int128) 0);
> +}
> +
> +int
> +f5 (unsigned int x)
> +{
> + return __builtin_sub_overflow_p (x, -4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f6 (unsigned int x)
> +{
> + return __builtin_sub_overflow_p (x, -4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f7 (int x)
> +{
> + return __builtin_sub_overflow_p (x, -4096, (__int128) 0);
> +}
> +
> +int
> +f8 (int x)
> +{
> + return __builtin_sub_overflow_p (x, -4097, (__int128) 0);
> +}
> +
> +int
> +f9 (unsigned int x)
> +{
> + return __builtin_add_overflow_p (x, 4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f10 (unsigned int x)
> +{
> + return __builtin_add_overflow_p (x, 4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f11 (int x)
> +{
> + return __builtin_add_overflow_p (x, 4096, (__int128) 0);
> +}
> +
> +int
> +f12 (int x)
> +{
> + return __builtin_add_overflow_p (x, 4097, (__int128) 0);
> +}
> +
> +int
> +f13 (unsigned int x)
> +{
> + return __builtin_add_overflow_p (x, -4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f14 (unsigned int x)
> +{
> + return __builtin_add_overflow_p (x, -4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f15 (int x)
> +{
> + return __builtin_add_overflow_p (x, -4096, (__int128) 0);
> +}
> +
> +int
> +f16 (int x)
> +{
> + return __builtin_add_overflow_p (x, -4097, (__int128) 0);
> +}
>
> Jakub
Re: [patch, fortran] Fix PR 85781, ICE on valid
Hi Thomas, hi all,
first, I have now attached a different fix for PR 85781 (= original
bug). Can you have a look?
I have the feeling (but didn't check) that your patch does not handle
the following variant of the test case: "print *, x(m:n)" (i.e. the
lower bound is not known at compile time).
I hope my patch covers all issues. – OK for the trunk?
Secondly:
On 1/21/20 7:32 PM, Thomas König wrote:
the attached patch fixes an ICE which could occur for empty
substrings (see test case).
I think one should rather fix the following issue.
I am not sure what you mean. Does that mean that fixing the following
issue will also fix PR 85781
I am no longer sure what I meant myself ;-)
I initially thought those are directly related – but they now look
related but independent bugs:
PR 85781 is about getting a non-ARRAY_TYPE (tree dump: "character") and
using it as ARRAY_TYPE (tree dump: "character[lb:ub]").
While PR93336 is about (1) using an ARRAY_TYPE when one should not. –
And, additionally, about missing diagnostic related to (2) bind(c) and
kind=4, (3) passing zero-length strings to non-zero-length dummy args,
(4) diagnostic about truncating too long strings (esp. if of
non-default, non-c_char kind).
Tobias
PR fortran/85781
* trans-expr.c (gfc_conv_substring): Handle non-ARRAY_TYPE strings
of Bind(C) procedures.
PR fortran/85781
* gfortran.dg/bind_c_char_2.f90: New.
* gfortran.dg/bind_c_char_3.f90: New.
* gfortran.dg/bind_c_char_4.f90: New.
* gfortran.dg/bind_c_char_5.f90: New.
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index e1c0fb271de..5825a4b8ce3 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -2334,8 +2334,12 @@ gfc_conv_substring (gfc_se * se, gfc_ref * ref, int kind,
else
tmp = build_fold_indirect_ref_loc (input_location,
se->expr);
- tmp = gfc_build_array_ref (tmp, start.expr, NULL);
- se->expr = gfc_build_addr_expr (type, tmp);
+ /* For BIND(C), a BT_CHARACTER is not an ARRAY_TYPE. */
+ if (TREE_CODE (TREE_TYPE (tmp)) == ARRAY_TYPE)
+ {
+ tmp = gfc_build_array_ref (tmp, start.expr, NULL);
+ se->expr = gfc_build_addr_expr (type, tmp);
+ }
}
/* Length = end + 1 - start. */
diff --git a/gcc/testsuite/gfortran.dg/bind_c_char_2.f90 b/gcc/testsuite/gfortran.dg/bind_c_char_2.f90
new file mode 100644
index 000..23a0cac2b4f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bind_c_char_2.f90
@@ -0,0 +1,50 @@
+! { dg-do run }
+!
+! PR fortran/85781
+!
+! Co-contributed by G. Steinmetz
+
+ use iso_c_binding, only: c_char
+ call s(c_char_'x', 1, 1)
+ call s(c_char_'x', 1, 0)
+ call s(c_char_'x', 0, -2)
+contains
+ subroutine s(x,m,n) bind(c)
+use iso_c_binding, only: c_char
+character(kind=c_char), value :: x
+call foo(x(m:n), m, n)
+if (n < m) then
+ if (len(x(m:n)) /= 0) stop 1
+ if (x(m:n) /= "") stop 2
+else if (n == 1) then
+ if (len(x(m:n)) /= 1) stop 1
+ if (x(m:n) /= "x") stop 2
+else
+ stop 14
+end if
+call foo(x(1:1), 1, 1)
+call foo(x(1:0), 1, 0)
+call foo(x(2:1), 2, 1)
+call foo(x(0:-4), 0, -4)
+
+call foo(x(1:), 1, 1)
+call foo(x(2:), 2, 1)
+call foo(x(:1), 1, 1)
+call foo(x(:0), 1, 0)
+
+if (n == 1) call foo(x(m:), m, n)
+if (m == 1) call foo(x(:n), m, n)
+ end
+ subroutine foo(str, m, n)
+character(len=*) :: str
+if (n < m) then
+ if (len(str) /= 0) stop 11
+ if (str /= "") stop 12
+else if (n == 1) then
+ if (len(str) /= 1) stop 13
+ if (str /= "x") stop 14
+else
+ stop 14
+end if
+ end
+end
diff --git a/gcc/testsuite/gfortran.dg/bind_c_char_3.f90 b/gcc/testsuite/gfortran.dg/bind_c_char_3.f90
new file mode 100644
index 000..01113aad0c5
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bind_c_char_3.f90
@@ -0,0 +1,51 @@
+! { dg-do run }
+! { dg-additional-options "-fcheck=all" }
+!
+! PR fortran/85781
+!
+! Co-contributed by G. Steinmetz
+
+ use iso_c_binding, only: c_char
+ call s(c_char_'x', 1, 1)
+ call s(c_char_'x', 1, 0)
+ call s(c_char_'x', 0, -2)
+contains
+ subroutine s(x,m,n) bind(c)
+use iso_c_binding, only: c_char
+character(kind=c_char), value :: x
+call foo(x(m:n), m, n)
+if (n < m) then
+ if (len(x(m:n)) /= 0) stop 1
+ if (x(m:n) /= "") stop 2
+else if (n == 1) then
+ if (len(x(m:n)) /= 1) stop 1
+ if (x(m:n) /= "x") stop 2
+else
+ stop 14
+end if
+call foo(x(1:1), 1, 1)
+call foo(x(1:0), 1, 0)
+call foo(x(2:1), 2, 1)
+call foo(x(0:-4), 0, -4)
+
+call foo(x(1:), 1, 1)
+call foo(x(2:), 2, 1)
+call foo(x(:1), 1, 1)
+call foo(x(:0), 1, 0)
+
+if (n == 1) call foo(x(m:), m, n)
+if (m == 1) call foo(x(:n), m, n)
+ end
+ subroutine foo(str, m, n)
+character(len=*) :: str
+if (n < m) then
+ if (len(str) /= 0) stop 11
+ if (str /= "") stop 12
+else if (n == 1) then
+ if (len(str)
[PATCH] cfgexpand: Update partition size when merging variables
cfgexpand sorts variables by decreasing size, so when merging a later
variable into an earlier one, there's usually no need to update the
merged size.
But for poly_int sizes, the sort function just uses a lexicographical
comparison of the coefficients, so e.g. 2X+2 comes before 0X+32.
Which is bigger depends on the runtime value of X.
This patch therefore takes the upper bound of the two sizes, which
is conservatively correct for variable-length vectors and a no-op
on other targets.
It's probably a bad idea to merge fixed-length and variable-length
variables in practice, but that's really an optimisation decision.
I think we should have this patch as a correctness fix either way.
This is easiest to test using the ACLE, but in principle it could happen
for autovectorised code too, e.g. when using OpenMP vector variables.
It's therefore a regression from GCC 8.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2020-01-22 Richard Sandiford
gcc/
* cfgexpand.c (union_stack_vars): Update the size.
gcc/testsuite/
* gcc.target/aarch64/sve/acle/general/stack_vars_1.c: New test.
---
gcc/cfgexpand.c | 3 ++
.../aarch64/sve/acle/general/stack_vars_1.c | 32 +++
2 files changed, 35 insertions(+)
create mode 100644
gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index fb0575b146e..9864e4344d2 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -862,6 +862,9 @@ union_stack_vars (size_t a, size_t b)
stack_vars[b].representative = a;
stack_vars[a].next = b;
+ /* Make sure A is big enough to hold B. */
+ stack_vars[a].size = upper_bound (stack_vars[a].size, stack_vars[b].size);
+
/* Update the required alignment of partition A to account for B. */
if (stack_vars[a].alignb < stack_vars[b].alignb)
stack_vars[a].alignb = stack_vars[b].alignb;
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
new file mode 100644
index 000..860fa67fbe6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
@@ -0,0 +1,32 @@
+/* { dg-do run { target aarch64_sve_hw } } */
+/* { dg-additional-options "-O2" } */
+
+#include
+
+struct s { int x[32]; };
+
+void __attribute__((noipa)) consume (void *ptr) {}
+
+void __attribute__((noipa))
+check_var (svint32_t *ptr)
+{
+ svbool_t pg = svptrue_b8 ();
+ if (svptest_any (pg, svcmpne (pg, *ptr, svindex_s32 (0, 1
+__builtin_abort ();
+}
+
+int
+main (void)
+{
+ svint32_t res = svindex_s32 (0, 1);
+ {
+__SVBool_t pg = svptrue_b8 ();
+consume (&pg);
+ }
+ {
+struct s zeros = { 0 };
+consume (&zeros);
+ }
+ check_var (&res);
+ return 0;
+}
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
Hi. I've updated the patch based on Honza's notes and installed it as 5f32f9cf13f99f6295591927950aaf98aa8dba91. Just for the record, there are stats for cc1plus for PGO bootstrap: before: == Stats for gcc == stats for indirect_call: total: 9210 invalid: 760 tracked values: 0 values: 6248 times (67.84%) 1 values: 1795 times (19.49%) 2 values: 220 times (2.39%) 3 values: 87 times (0.94%) 4 values: 100 times (1.09%) stats for topn: total: 1513 invalid: 220 tracked values: 0 values: 1028 times (67.94%) 1 values: 149 times (9.85%) 2 values: 50 times (3.30%) 3 values: 38 times (2.51%) 4 values: 28 times (1.85%) after: stats for indirect_call: total: 9210 invalid: 600 tracked values: 0 values: 6280 times (68.19%) 1 values: 1856 times (20.15%) 2 values: 264 times (2.87%) 3 values: 157 times (1.70%) 4 values: 53 times (0.58%) stats for topn: total: 1513 invalid: 178 tracked values: 0 values: 1034 times (68.34%) 1 values: 176 times (11.63%) 2 values: 68 times (4.49%) 3 values: 39 times (2.58%) 4 values: 18 times (1.19%) Martin
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
> Hi.
>
> I've updated the patch based on Honza's notes and installed it
> as 5f32f9cf13f99f6295591927950aaf98aa8dba91.
Thanks, you omitted the patch, but looking at git log I think there is
still problem I commented on in the previous mail:
for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
{
if (value == counters[2 * i])
{
if (use_atomic)
- __atomic_fetch_add (&counters[2 * i + 1], 1, __ATOMIC_RELAXED);
+ __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
+ __ATOMIC_RELAXED);
else
- counters[2 * i + 1]++;
+ counters[2 * i + 1] += GCOV_TOPN_VALUES;
return;
}
- else if (counters[2 * i + 1] == 0)
+ else if (counters[2 * i + 1] <= 0)
{
/* We found an empty slot. */
counters[2 * i] = value;
- counters[2 * i + 1] = 1;
+ counters[2 * i + 1] = GCOV_TOPN_VALUES;
return;
}
Lets say that sequence is Y X X Z Z X and N = 2
you will get
Y 2 _ 0
Y 2 X 2
Y 2 X 4
Y 1 X 3
Y 0 X 2
X 2 X 2
while it ought to be
Y 0 X 4
the loop really needs to walk all counters since non-zero entries are
not guaranteed to be grouped at the beggining.
(I think reordering counter is worse alternative especially for
concurent updates since we probably do not want N to be very large)
>
> Just for the record, there are stats for cc1plus for PGO bootstrap:
>
> before:
>
> == Stats for gcc ==
> stats for indirect_call:
> total: 9210
> invalid: 760
> tracked values:
> 0 values: 6248 times (67.84%)
> 1 values: 1795 times (19.49%)
> 2 values: 220 times (2.39%)
> 3 values: 87 times (0.94%)
> 4 values: 100 times (1.09%)
>
> stats for topn:
> total: 1513
> invalid: 220
> tracked values:
> 0 values: 1028 times (67.94%)
> 1 values: 149 times (9.85%)
> 2 values: 50 times (3.30%)
> 3 values: 38 times (2.51%)
> 4 values: 28 times (1.85%)
>
> after:
>
> stats for indirect_call:
> total: 9210
> invalid: 600
> tracked values:
> 0 values: 6280 times (68.19%)
> 1 values: 1856 times (20.15%)
> 2 values: 264 times (2.87%)
> 3 values: 157 times (1.70%)
> 4 values: 53 times (0.58%)
>
> stats for topn:
> total: 1513
> invalid: 178
> tracked values:
> 0 values: 1034 times (68.34%)
> 1 values: 176 times (11.63%)
> 2 values: 68 times (4.49%)
> 3 values: 39 times (2.58%)
> 4 values: 18 times (1.19%)
Thanks, so basically there are 2930 = 1856 + 264 + 157 + 53 + 600
trained entries. Out of that 1795 has always only one target, that
leaves 1135 entries with interesting behaviour wrt merging and we give
up on 600, that is about 50% of them.
Can you send me your stats script? I will try it out at Clang as well
and will regenerate firefox numbers after the problem above is solved.
Honza
>
> Martin
[PATCH Coroutines]Fix an ICE case in expanding co_await expression
Hi, Though function co_await_expander may need to be further revised, this simple patch fixes an ICE case in co_await_expander, Handle CO_AWAIT_EXPR in conversion in co_await_expander. Function co_await_expander expands CO_AWAIT_EXPR and inserts expanded code before result of co_await is used, however, it doesn't cover the type conversion case and leads to gimplify ICE. This patch fixes it. Bootstrap and test on x86_64. Is it OK? Thanks, bin gcc/cp 2020-01-22 Bin Cheng * coroutines.cc (co_await_expander): Handle type conversion case. gcc/testsuite 2020-01-22 Bin Cheng * g++.dg/coroutines/co-await-syntax-09-convert.C: New test. 0001-Handle-CO_AWAIT_EXPR-in-conversion-in-co_await_expan.patch Description: Binary data
[PATCH] tree-optimization/93381 fix integer offsetting in points-to analysis
We were incorrectly assuming a merge operation is conservative enough
for not explicitely handled operations but we also need to consider
offsetting within fields when field-sensitive analysis applies.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
Richard.
2020-22 Richard Biener
PR tree-optimization/93381
* tree-ssa-structalias.c (find_func_aliases): Assume offsetting
throughout, handle all conversions the same.
* gcc.dg/torture/pr93381.c: New testcase.
diff --git a/gcc/testsuite/gcc.dg/torture/pr93381.c
b/gcc/testsuite/gcc.dg/torture/pr93381.c
new file mode 100644
index 000..cec4b5d8daa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93381.c
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+
+static struct S { int *p1; int *p2; } s;
+typedef __UINTPTR_TYPE__ uintptr_t;
+int foo()
+{
+ int i = 1, j = 2;
+ struct S s;
+ int **p;
+ s.p1 = &i;
+ s.p2 = &j;
+ p = &s.p1;
+ uintptr_t pi = (uintptr_t)p;
+ pi = pi + sizeof (int *);
+ p = (int **)pi;
+ **p = 3;
+ return j;
+}
+
+int main()
+{
+ if (foo () != 3)
+__builtin_abort ();
+ return 0;
+}
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index f189f756261..416a26c996c 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5008,11 +5008,12 @@ find_func_aliases (struct function *fn, gimple *origt)
|| code == FLOOR_MOD_EXPR
|| code == ROUND_MOD_EXPR)
/* Division and modulo transfer the pointer from the LHS. */
- get_constraint_for_rhs (gimple_assign_rhs1 (t), &rhsc);
- else if ((CONVERT_EXPR_CODE_P (code)
- && !(POINTER_TYPE_P (gimple_expr_type (t))
-&& !POINTER_TYPE_P (TREE_TYPE (rhsop
+ get_constraint_for_ptr_offset (gimple_assign_rhs1 (t),
+ NULL_TREE, &rhsc);
+ else if (CONVERT_EXPR_CODE_P (code)
|| gimple_assign_single_p (t))
+ /* See through conversions, single RHS are handled by
+ get_constraint_for_rhs. */
get_constraint_for_rhs (rhsop, &rhsc);
else if (code == COND_EXPR)
{
@@ -5031,14 +5032,16 @@ find_func_aliases (struct function *fn, gimple *origt)
;
else
{
- /* All other operations are merges. */
+ /* All other operations are possibly offsetting merges. */
auto_vec tmp;
struct constraint_expr *rhsp;
unsigned i, j;
- get_constraint_for_rhs (gimple_assign_rhs1 (t), &rhsc);
+ get_constraint_for_ptr_offset (gimple_assign_rhs1 (t),
+NULL_TREE, &rhsc);
for (i = 2; i < gimple_num_ops (t); ++i)
{
- get_constraint_for_rhs (gimple_op (t, i), &tmp);
+ get_constraint_for_ptr_offset (gimple_op (t, i),
+NULL_TREE, &tmp);
FOR_EACH_VEC_ELT (tmp, j, rhsp)
rhsc.safe_push (*rhsp);
tmp.truncate (0);
Re: [PATCH] doc: clarify the situation with pointer arithmetic
On 1/22/20 8:32 AM, Richard Biener wrote: On Tue, 21 Jan 2020, Alexander Monakov wrote: On Tue, 21 Jan 2020, Richard Biener wrote: Fourth. That PNVI (I assume it's the whole pointer-provenance stuff) wants to get the "best" of both which can never be done since a compiler needs to have a way to be conservative - in this area it's conflicting conservative treatment which is impossible. This paragraph is unclear, I don't immediately see what the conflicting goals are. The rest is clear enough given the previous discussions I saw. Did you mean the restriction that you cannot do arithmetic involving two integers based on pointers, get a value corresponding to one of them, cast it back and get a pointer suitable for accessing either of two originally pointed-to objects? I don't see that as a conflict because it places a restriction on users, not the compiler. As far as I remember the discussions PNVI requires to track provenance for correctness, you may not miss or attach wrong provenance to a pointer and there's only "single" provenance, not "many" (aka, may point to A and B). I don't see how you can ever implement that. The PVNI variant preferred by the object model group is referred to as "PNVI-ae-udi" which stands for "PNVI exposed-address user- disambiguation." (The PNVI part stands for "Provenance Not Via Integers.) This base PVNI model basically prohibits provenance tracking via integers, making it possible for programs to derive pointers to unrelated objects via casts between pointers and integers (and modifying the integer in between the casts). This is considered a new restriction on implementations because the standard doesn't permit it (as you said upthread, all it specifies is that a pointer is equal to one obtained by casting the original to a intptr_t and back). The -ae-udi variant limits this restriction on implementations to escaped pointers and provides a means for users/programs to disambiguate between pointers to adjacent objects (i.e., a past the end pointer and one to the beginning of the object stored there). The latest proposal is in N2362, with an overview in N2378). At the last WG14 meeting there was broad discomfort with adopting the proposal for C2X because of the absence of implementation experience and concerns raised by implementers. The guidance to the study group was to target a separate technical specification for the proposal and allow time for implementation experience. If the feedback from implementers is positive (whatever that might mean) WG14 said it would consider adopting the model for a revision of C after C2X. Overall, the impact of the proposals as well as their goal is to constrain implementations to the (presumed) benefit of programs in terms of expressiveness. There are numerous examples of code that's currently treated as undefined by one or more compilers (as a consequence of optimizations) that the model makes valid. I'm not aware of any optimization opportunities the proposal might open up in GCC. Martin
Re: [PATCH] Fix PowerPC -fstack-clash-protection -mprefixed-addr ICE (PR target/93122)
Hi! On Mon, Jan 06, 2020 at 02:46:30PM -0600, Segher Boessenkool wrote: > > > Another option would be to: > > > 1) always use gen_add3_insn, but if it returns NULL use the extra > > > emit_move_insn (end_addr, GEN_INT (-rounded_size)); > > > and retry with end_addr instead of rs > > > 2) if the first gen_add3_insn returned NULL or if it created more than one > > > instruction, add the REG_FRAME_RELATED_EXPR note on the last insn > > > > > > Is that what you want to do instead? > > > > > > 2020-01-06 Jakub Jelinek > > > > > > PR target/93122 > > > * config/rs6000/rs6000-logue.c > > > (rs6000_emit_probe_stack_range_stack_clash): Only use gen_addr3_insn > > > if add_cint_operand predicate matches. Use rs instead of doing > > > GEN_INT again. > > > > > > * gcc.target/powerpc/pr93122.c: New test. > > I think this is fine, but give the PPC maintainers a few days to chime > > in. > > It's not okay, whether I'll need a few days or more to reply. But I will > reply soon :-) I'd like to ping this patch (or can code up the above option). Thanks. Jakub
[PATCH] cprop: Don't replace fixed hard regs with pseudos [PR93124]
One consequence of r276318 was that cselib now preserves sp-based
values across function calls. This in turn convinced cprop to
replace the clobber in:
(set (reg PSUEDO) (reg sp))
...
(call ...)
...
(clobber (mem:BLK (reg sp)))
with:
(clobber (mem:BLK (reg PSEUDO)))
But I doubt this could ever be an optimisation, regardless of what the
changed instruction is. Extending the lifetimes of pseudos can lead to
extra spills, whereas sp is available everywhere.
More generally, I don't think we should replace any fixed hard register
with a pseudo. Replacing them with a constant is still potentially
useful though, since we'll only make the change if the insn pattern
allows it.
This part 1 of the fix for PR93124. Part 2 contains the testcase.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2020-01-22 Richard Sandiford
gcc/
PR rtl-optimization/93124
* cprop.c (cprop_replace_with_reg_p): New function.
(cprop_insn, do_local_cprop): Use it.
---
gcc/cprop.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/gcc/cprop.c b/gcc/cprop.c
index 169ca804e33..5f6446984d8 100644
--- a/gcc/cprop.c
+++ b/gcc/cprop.c
@@ -261,6 +261,18 @@ cprop_reg_p (const_rtx x)
return REG_P (x) && !HARD_REGISTER_P (x);
}
+/* Return true if it is worth replacing register REGNO with a different
+ register. */
+
+static bool
+cprop_replace_with_reg_p (unsigned int regno)
+{
+ /* Replacing a fixed register with a pseudo can lead to extra spilling.
+ If REGNO is eliminable, replacing it could also lead to less efficient
+ eliminations. */
+ return !HARD_REGISTER_NUM_P (regno) || !fixed_regs[regno];
+}
+
/* Scan SET present in INSN and add an entry to the hash TABLE.
IMPLICIT is true if it's an implicit set, false otherwise. */
@@ -1098,6 +1110,7 @@ cprop_insn (rtx_insn *insn)
/* Copy propagation. */
else if (src_reg && cprop_reg_p (src_reg)
&& REGNO (src_reg) != regno
+ && cprop_replace_with_reg_p (regno)
&& try_replace_reg (reg_used, src_reg, insn))
{
changed_this_round = changed = 1;
@@ -1198,6 +1211,7 @@ do_local_cprop (rtx x, rtx_insn *insn)
if (cprop_constant_p (this_rtx))
newcnst = this_rtx;
if (cprop_reg_p (this_rtx)
+ && cprop_replace_with_reg_p (REGNO (x))
/* Don't copy propagate if it has attached REG_EQUIV note.
At this point this only function parameters should have
REG_EQUIV notes and if the argument slot is used somewhere
--
2.17.1
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
On 1/22/20 12:27 PM, Jan Hubicka wrote:
Hi.
I've updated the patch based on Honza's notes and installed it
as 5f32f9cf13f99f6295591927950aaf98aa8dba91.
Thanks, you omitted the patch, but looking at git log I think there is
still problem I commented on in the previous mail:
Whoops, sorry for that.
for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
{
if (value == counters[2 * i])
{
if (use_atomic)
- __atomic_fetch_add (&counters[2 * i + 1], 1, __ATOMIC_RELAXED);
+ __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
+ __ATOMIC_RELAXED);
else
- counters[2 * i + 1]++;
+ counters[2 * i + 1] += GCOV_TOPN_VALUES;
return;
}
- else if (counters[2 * i + 1] == 0)
+ else if (counters[2 * i + 1] <= 0)
{
/* We found an empty slot. */
counters[2 * i] = value;
- counters[2 * i + 1] = 1;
+ counters[2 * i + 1] = GCOV_TOPN_VALUES;
return;
}
Lets say that sequence is Y X X Z Z X and N = 2
you will get
Y 2 _ 0
Y 2 X 2
Y 2 X 4
Y 1 X 3
Y 0 X 2
X 2 X 2
while it ought to be
Y 0 X 4
the loop really needs to walk all counters since non-zero entries are
not guaranteed to be grouped at the beggining.
(I think reordering counter is worse alternative especially for
concurent updates since we probably do not want N to be very large)
It should be fixed in the attached patch.
Just for the record, there are stats for cc1plus for PGO bootstrap:
before:
== Stats for gcc ==
stats for indirect_call:
total: 9210
invalid: 760
tracked values:
0 values: 6248 times (67.84%)
1 values: 1795 times (19.49%)
2 values: 220 times (2.39%)
3 values: 87 times (0.94%)
4 values: 100 times (1.09%)
stats for topn:
total: 1513
invalid: 220
tracked values:
0 values: 1028 times (67.94%)
1 values: 149 times (9.85%)
2 values: 50 times (3.30%)
3 values: 38 times (2.51%)
4 values: 28 times (1.85%)
after:
stats for indirect_call:
total: 9210
invalid: 600
tracked values:
0 values: 6280 times (68.19%)
1 values: 1856 times (20.15%)
2 values: 264 times (2.87%)
3 values: 157 times (1.70%)
4 values: 53 times (0.58%)
stats for topn:
total: 1513
invalid: 178
tracked values:
0 values: 1034 times (68.34%)
1 values: 176 times (11.63%)
2 values: 68 times (4.49%)
3 values: 39 times (2.58%)
4 values: 18 times (1.19%)
Thanks, so basically there are 2930 = 1856 + 264 + 157 + 53 + 600
trained entries. Out of that 1795 has always only one target, that
leaves 1135 entries with interesting behaviour wrt merging and we give
up on 600, that is about 50% of them.
Can you send me your stats script? I will try it out at Clang as well
and will regenerate firefox numbers after the problem above is solved.
Sure:
https://github.com/marxin/script-misc/blob/master/gcov-dump-analysis.py
Make sure you have install gcov-dump in $PATH that corresponds to version
of .gcda files.
Is the patch fine for trunk?
Martin
Honza
Martin
>From 7c378b91be18fc258cc453a138a32405b8ac550c Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Wed, 22 Jan 2020 13:01:28 +0100
Subject: [PATCH] Fix TOP N counter update.
libgcc/ChangeLog:
2020-01-22 Martin Liska
PR tree-optimization/92924
* libgcov-profiler.c (__gcov_topn_values_profiler_body): First
try to find an existing value, then find an empty slot
if not found.
---
libgcc/libgcov-profiler.c | 46 ---
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index f45ef498a6e..506a9b68022 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -119,35 +119,37 @@ __gcov_topn_values_profiler_body (gcov_type *counters, gcov_type value,
++counters;
+ /* First try to find an existing value. */
+ int empty_counter = -1;
+
for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
+if (value == counters[2 * i])
+ {
+ if (use_atomic)
+ __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
+ __ATOMIC_RELAXED);
+ else
+ counters[2 * i + 1] += GCOV_TOPN_VALUES;
+ return;
+ }
+else if (counters[2 * i + 1] <= 0 && empty_counter == -1)
+ empty_counter = i;
+
+ /* Find an empty slot for a new value. */
+ if (empty_counter != -1)
{
- if (value == counters[2 * i])
- {
- if (use_atomic)
- __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
-__ATOMIC_RELAXED);
- else
- counters[2 * i + 1] += GCOV_TOPN_VALUES;
- return;
- }
- else if (counters[2 * i + 1] <= 0)
- {
- /* We found an empty slot. */
- counters[2 * i] = value;
- counters[2 * i + 1] = GCOV_TOPN_VALUES;
- return;
- }
+ coun
[PATCH] auto-inc-dec: Don't add incs/decs to bare CLOBBERs [PR93124]
In this PR, auto-inc-dec was trying to turn:
(set (reg X) (plus (reg X) (const_int N)))
(clobber (mem (reg X)))
into:
(clobber (mem (pre_modify (reg X) ...)))
But bare clobber insns are just there to describe dataflow. They're
not supposed to generate any code.
This is part 2 of the fix for PR93124. The reason we have the
clobber above is that cprop replaced (reg sp) with (reg X) in:
(clobber (mem (reg sp)))
Part 1 stops that from happening, but I still think we should
prevent auto-inc-dec from "optimising" bare USEs and CLOBBERs
as a belt-and-braces fix.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2020-01-22 Richard Sandiford
gcc/
PR rtl-optimization/93124
* auto-inc-dec.c (merge_in_block): Don't add auto inc/decs to
bare USE and CLOBBER insns.
gcc/testsuite/
* gcc.dg/torture/pr93124.c: New test.
---
gcc/auto-inc-dec.c | 12 +--
gcc/testsuite/gcc.dg/torture/pr93124.c | 44 ++
2 files changed, 53 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr93124.c
diff --git a/gcc/auto-inc-dec.c b/gcc/auto-inc-dec.c
index 268231ecaff..7d0d91403f3 100644
--- a/gcc/auto-inc-dec.c
+++ b/gcc/auto-inc-dec.c
@@ -1602,9 +1602,15 @@ merge_in_block (int max_reg, basic_block bb)
else
{
insn_is_add_or_inc = false;
- mem_insn.insn = insn;
- if (find_mem (&PATTERN (insn)))
- success_in_block++;
+ /* We can't use auto inc/dec for bare USEs and CLOBBERs,
+since they aren't supposed to generate any code. */
+ rtx_code code = GET_CODE (PATTERN (insn));
+ if (code != USE && code != CLOBBER)
+ {
+ mem_insn.insn = insn;
+ if (find_mem (&PATTERN (insn)))
+ success_in_block++;
+ }
}
/* If the inc insn was merged with a mem, the inc insn is gone
diff --git a/gcc/testsuite/gcc.dg/torture/pr93124.c
b/gcc/testsuite/gcc.dg/torture/pr93124.c
new file mode 100644
index 000..16bc8b54f14
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93124.c
@@ -0,0 +1,44 @@
+/* { dg-additional-options "-fno-rerun-cse-after-loop
-fno-guess-branch-probability -fno-tree-fre" } */
+
+int x;
+
+void fn2 ();
+void fn3 ();
+void fn4 ();
+void fn5 ();
+void fn6 ();
+
+void
+fn1 (void)
+{
+ int n;
+ for (n = 0;; ++n) {
+{
+ struct { char a[n]; } s;
+ fn2 (s);
+}
+struct { unsigned a[x]; } s;
+int i, b;
+for (i = 0; i < n; ++i)
+ ;
+fn2 (s);
+{
+ struct { char a[n]; } s;
+ int i;
+ for (i = 0; i < n; ++i)
+s.a[i] = i;
+ fn3 (s, s);
+}
+fn4 ();
+{
+ struct { unsigned a[n]; } s;
+ fn5 (s);
+}
+{
+ struct { char a[b]; } s;
+ for (; i < n;)
+s.a[i] = i;
+ fn6 (s);
+}
+ }
+}
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
> 2020-01-22 Martin Liska
>
> PR tree-optimization/92924
> * libgcov-profiler.c (__gcov_topn_values_profiler_body): First
> try to find an existing value, then find an empty slot
> if not found.
This looks good, one nit below.
> ---
> libgcc/libgcov-profiler.c | 46 ---
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
> index f45ef498a6e..506a9b68022 100644
> --- a/libgcc/libgcov-profiler.c
> +++ b/libgcc/libgcov-profiler.c
> @@ -119,35 +119,37 @@ __gcov_topn_values_profiler_body (gcov_type *counters,
> gcov_type value,
>
>++counters;
>
> + /* First try to find an existing value. */
> + int empty_counter = -1;
> +
>for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
> +if (value == counters[2 * i])
> + {
> + if (use_atomic)
> + __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
> + __ATOMIC_RELAXED);
> + else
> + counters[2 * i + 1] += GCOV_TOPN_VALUES;
> + return;
> + }
> +else if (counters[2 * i + 1] <= 0 && empty_counter == -1)
> + empty_counter = i;
I think you can save an instruction in the loop if you drop "&& empty_counter
== -1"
Thanks!
Honza
> +
> + /* Find an empty slot for a new value. */
> + if (empty_counter != -1)
> {
> - if (value == counters[2 * i])
> - {
> - if (use_atomic)
> - __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
> - __ATOMIC_RELAXED);
> - else
> - counters[2 * i + 1] += GCOV_TOPN_VALUES;
> - return;
> - }
> - else if (counters[2 * i + 1] <= 0)
> - {
> - /* We found an empty slot. */
> - counters[2 * i] = value;
> - counters[2 * i + 1] = GCOV_TOPN_VALUES;
> - return;
> - }
> + counters[2 * empty_counter] = value;
> + counters[2 * empty_counter + 1] = GCOV_TOPN_VALUES;
> + return;
> }
>
>/* We haven't found an empty slot, then decrement all
> counter values by one. */
>for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
> -{
> - if (use_atomic)
> - __atomic_fetch_sub (&counters[2 * i + 1], 1, __ATOMIC_RELAXED);
> - else
> - counters[2 * i + 1]--;
> -}
> +if (use_atomic)
> + __atomic_fetch_sub (&counters[2 * i + 1], 1, __ATOMIC_RELAXED);
> +else
> + counters[2 * i + 1]--;
> }
>
> #ifdef L_gcov_topn_values_profiler
> --
> 2.24.1
>
[committed] Extend r279588 to g++.dg/ext/sve-sizeless-1.C
In r279588 I'd for some reason only patched g++.dg/ext/sve-sizeless-2.C,
even though g++.dg/ext/sve-sizeless-1.C has the same problem.
Tested on aarch64-linux-gnu and aarch64_be-elf, applied.
Richard
2020-01-22 Richard Sandiford
gcc/testsuite/
* g++.dg/ext/sve-sizeless-1.C: Don't expect an error for alias
templates.
---
gcc/testsuite/g++.dg/ext/sve-sizeless-1.C | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/g++.dg/ext/sve-sizeless-1.C
b/gcc/testsuite/g++.dg/ext/sve-sizeless-1.C
index f82fb2a9a9e..7f829220c71 100644
--- a/gcc/testsuite/g++.dg/ext/sve-sizeless-1.C
+++ b/gcc/testsuite/g++.dg/ext/sve-sizeless-1.C
@@ -74,7 +74,7 @@ template class templated_struct5;
#if __cplusplus >= 201103L
template using typedef_sizeless1 = svint8_t;
-template using typedef_sizeless1 = svint8_t; // { dg-error
{redefinition of 'template using typedef_sizeless1 = svint8_t'} "" {
target c++11 } }
+template using typedef_sizeless1 = svint8_t;
template using array = T[2];
#endif
[committed] Fix gcc.target/aarch64/sve/sel_3.c for big-endian targets
A pasto in this test meant that we needed extra reverse instructions
for big-endian targets.
Tested on aarch64-linux-gnu and aarch64_be-elf, applied.
Richard
2020-01-22 Richard Sandiford
gcc/testsuite/
* gcc.target/aarch64/sve/sel_3.c (permute_vnx4sf): Take __SVFloat32_t
rather than __SVFloat16_t
---
gcc/testsuite/gcc.target/aarch64/sve/sel_3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/sel_3.c
b/gcc/testsuite/gcc.target/aarch64/sve/sel_3.c
index 0de1fae6d03..36ec15b7da6 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/sel_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/sel_3.c
@@ -82,7 +82,7 @@ permute_vnx8hf (__SVFloat16_t x, __SVFloat16_t y)
** ret
*/
__SVFloat32_t
-permute_vnx4sf (__SVFloat16_t x, __SVFloat16_t y)
+permute_vnx4sf (__SVFloat32_t x, __SVFloat32_t y)
{
return __builtin_shuffle ((vnx4sf) x, (vnx4sf) y, (vnx4si) MASK_8);
}
[committed] Skip gcc.target/aarch64/sve/tls_preserve* for emultls targets
These tests are supposed to be testing the tlsdesc handling and
so don't apply to emultls targets.
Tested on aarch64-linux-gnu and aarch64_be-elf, applied.
Richard
2020-01-22 Richard Sandiford
gcc/testsuite/
* gcc.target/aarch64/sve/tls_preserve_1.c: Require tls_native.
* gcc.target/aarch64/sve/tls_preserve_2.c: Likewise.
* gcc.target/aarch64/sve/tls_preserve_3.c: Likewise.
---
gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_1.c | 3 +--
gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_2.c | 3 +--
gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_3.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_1.c
b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_1.c
index 1d377703592..e90ac78a526 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_1.c
@@ -1,8 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O3 -fpic" } */
/* { dg-require-effective-target fpic } */
-
-/* Clobber highs do not need to be spilled around tls usage. */
+/* { dg-require-effective-target tls_native } */
typedef float v4si __attribute__ ((vector_size (16)));
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_2.c
b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_2.c
index 8d73ad72a7f..20e939fbb85 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_2.c
@@ -1,8 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O3 -fpic -msve-vector-bits=256 -fno-schedule-insns" } */
/* { dg-require-effective-target fpic } */
-
-/* Clobber highs must be spilled around tls usage. */
+/* { dg-require-effective-target tls_native } */
typedef float v8si __attribute__ ((vector_size (32)));
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_3.c
b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_3.c
index 17f39d184b1..a19908f29b6 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/tls_preserve_3.c
@@ -1,8 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O3 -fpic -msve-vector-bits=512 -fno-schedule-insns" } */
/* { dg-require-effective-target fpic } */
-
-/* Clobber highs must be spilled around tls usage. */
+/* { dg-require-effective-target tls_native } */
typedef float v16si __attribute__ ((vector_size (64)));
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
On 1/22/20 1:19 PM, Jan Hubicka wrote: I think you can save an instruction in the loop if you drop "&& empty_counter == -1" My motivation was to find a first empty slot. Which means we'll have less holes, but it's probably a micro-optimization. I'm going to install the patch without the "&& empty_counter == -1". Martin Thanks! Honza
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
On 1/22/20 12:09 PM, Martin Liška wrote: stats for indirect_call: total: 9210 invalid: 600 tracked values: 0 values: 6280 times (68.19%) 1 values: 1856 times (20.15%) 2 values: 264 times (2.87%) 3 values: 157 times (1.70%) 4 values: 53 times (0.58%) stats for topn: total: 1513 invalid: 178 tracked values: 0 values: 1034 times (68.34%) 1 values: 176 times (11.63%) 2 values: 68 times (4.49%) 3 values: 39 times (2.58%) 4 values: 18 times (1.19%) After the fix patch I get to: == Stats for gcc == stats for indirect_call: total: 9210 invalid: 518 tracked values: 0 values: 7746 times (84.10%) 1 values: 827 times (8.98%) 2 values: 53 times (0.58%) 3 values: 60 times (0.65%) 4 values:6 times (0.07%) stats for topn: total: 1513 invalid: 128 tracked values: 0 values: 1310 times (86.58%) 1 values: 49 times (3.24%) 2 values:9 times (0.59%) 3 values:4 times (0.26%) 4 values: 13 times (0.86%) Which seems to me a reasonable improvement. Martin
Re: [PATCH] Relax invalidation of TOP N counters in PGO.
> On 1/22/20 12:09 PM, Martin Liška wrote: > > stats for indirect_call: > > total: 9210 > > invalid: 600 > > tracked values: > > 0 values: 6280 times (68.19%) > > 1 values: 1856 times (20.15%) > > 2 values: 264 times (2.87%) > > 3 values: 157 times (1.70%) > > 4 values: 53 times (0.58%) > > > > stats for topn: > > total: 1513 > > invalid: 178 > > tracked values: > > 0 values: 1034 times (68.34%) > > 1 values: 176 times (11.63%) > > 2 values: 68 times (4.49%) > > 3 values: 39 times (2.58%) > > 4 values: 18 times (1.19%) > > After the fix patch I get to: > > == Stats for gcc == > stats for indirect_call: > total: 9210 > invalid: 518 > tracked values: > 0 values: 7746 times (84.10%) > 1 values: 827 times (8.98%) > 2 values: 53 times (0.58%) > 3 values: 60 times (0.65%) > 4 values:6 times (0.07%) > > Which seems to me a reasonable improvement. > Martin I am not too sure - the odd thing is that number of 0 values hash grown up from 6280 to 7746. (68->84%) Do you have any idea why? Honza
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On Wed, Jan 22, 2020 at 10:00:00AM +, Richard Earnshaw (lists) wrote: > On 21/01/2020 19:26, Segher Boessenkool wrote: > >On Tue, Jan 21, 2020 at 02:52:00PM +, Richard Earnshaw (lists) wrote: > >>+ A brief summary > > > >You could stress that this is the one thing that really matters. And > >it's not a summary, it's much too brief for that (at most ~50 chars), > >but yup it should be something that allows *a human* to identify what > >this is. > > > > Well, it should all matter, or why are we requiring it? Yes. > I'm happy to insert 'very' in front of 'brief' and to say elsewhere that > the entire subject (less the leading [...] part, should rarely, if ever, > go beyond 80 characters. The usual recommendation is 50 chars. Which is just about right with most MUAs. > >Everything else is just convention. > > > >>+A component tag is a short identifier that identifies the part of > >>+the compiler being modified. This highlights to the relevant > >>+maintainers that the patch may need their attention. Multiple > >>+components may be listed if necessary. Each component tag should be > >>+followed by a colon. > > > >Often people use aaa/bbb: if drilling down a bit that way helps keep the > >subject short (which is the *point* of all this: keep things better > >consumable for humans). > > The aaa: bbb: is really for when aaa and bbb are independent parts of > the compiler and potentially needs multiple reviewers. aaa/bbb is when > bbb is strictly a sub-component of aaa (for example, arm/SVE: would be > an SVE related issue in the arm backend). I'm certainly not against > having that. Excellent. > >>+The brief summary encapsulates in a few words the intent of the > >>+change. For example: cleanup check_field_decls. > > > >It should start with a capital though. "Clean up blablal". (So no > >dot to end the sentence, this isn't a sentence). A capital helps > >the reader to quickly identify what is what, separate fluff from the > >core parts. > > There is a balance here to be made. I'm mindful of Gerald's concern > that this is perhaps overly restrictive for new users already. I > certainly think we should have a good house style, but getting too > prescriptive just gets in the way of attracting good contributions. This is just basic email etiquette :-) But, it's very annoying when you look at badly formatted subjects, in "git log --oneline" for example, it is very obvious there (and long subjects are problematic there as well btw). Wrt balance: yes, that is one reason why I do not think we should require all the markup stuff. > >>+Some large patch sets benefit from an introductory e-mail that > >>+provides more context for the patch series and describes how the > >>+patches have been broken up to provide for review. > > > >All non-trivial series, yeah. > > > >Maybe we should mention how v2 etc. of patch series should show what is > >changed? If there is a good standard practice for that at all :-) > > Dunno. I think that belongs primarily in the v2 0/n mail. It's not, > after all, something that needs to be kept once the patches are applied. Sure. Ah, we could recommend that then, to make it clear in the vM 0/N of some series which patches changed how. My point is that it is a waste of time to everyone if a reviewer has to drag through everything every time; this is double true with git, it is easy to send updated versions of patch sets often. Segher
[PATCH] tree-optimization/93354 FRE redundant store removal validity fix
This fixes the validity check for redundant store removal by looking
at the actual stmt that represents the last (relevant) change to
the TBAA state rather than the imperfectly tracked alias set in the
expression hash table which then becomes unused (see a followup change
to get rid of that).
On the way this allows re-enabling of VN_WALKREWRITE, fixing PR91123.
This also exposes an out-of-bound array access in
real.c:clear_significand_below fixed as well.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2020-01-22 Richard Biener
PR tree-optimization/91123
PR tree-optimization/93381
* tree-ssa-sccvn.c (store_affects_tbaa_memory_state): New function.
(visit_reference_op_store): Use it to check validity of eliding
redundant stores.
(eliminate_dom_walker::eliminate_stmt): Likewise, allow lookup
to look through aggregate copies.
(vn_reference_lookup): Consistently set *last_use_ptr.
* real.c (clear_significand_below): Fix out-of-bound access.
* gcc.dg/torture/pr93354.c: New testcase.
* gcc.dg/tree-ssa/ssa-fre-85.c: Likewise.
diff --git a/gcc/real.c b/gcc/real.c
index a57e5df113f..46d8126efe4 100644
--- a/gcc/real.c
+++ b/gcc/real.c
@@ -420,7 +420,10 @@ clear_significand_below (REAL_VALUE_TYPE *r, unsigned int
n)
for (i = 0; i < w; ++i)
r->sig[i] = 0;
- r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
+ /* We are actually passing N == SIGNIFICAND_BITS which would result
+ in an out-of-bound access below. */
+ if (n % HOST_BITS_PER_LONG != 0)
+r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
}
/* Divide the significands of A and B, placing the result in R. Return
diff --git a/gcc/testsuite/gcc.dg/torture/pr93354.c
b/gcc/testsuite/gcc.dg/torture/pr93354.c
new file mode 100644
index 000..8ccf1e6d2c0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93354.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+
+typedef __INT32_TYPE__ int32_t;
+typedef __INT64_TYPE__ int64_t;
+struct X { int32_t i; int32_t j; };
+void foo (int64_t *z)
+{
+ ((struct X *)z)->i = 0x05060708;
+ ((struct X *)z)->j = 0x01020304;
+ *z = 0x0102030405060708;
+}
+
+int main()
+{
+ int64_t l = 0;
+ int64_t *p;
+ asm ("" : "=r" (p) : "0" (&l));
+ foo (p);
+ if (l != 0x0102030405060708)
+__builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
new file mode 100644
index 000..c50770caa2f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fstrict-aliasing -fdump-tree-fre1-details" } */
+
+struct X { int i; int j; };
+
+struct X x, y;
+void foo ()
+{
+ x.i = 1;
+ y = x;
+ y.i = 1; // redundant
+}
+
+/* { dg-final { scan-tree-dump "Deleted redundant store y.i" "fre1" } } */
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 0b8ee586139..ef272047bb3 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -3205,6 +3205,8 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind
kind,
return NULL_TREE;
}
+ if (last_vuse_ptr)
+*last_vuse_ptr = vr1.vuse;
return vn_reference_lookup_1 (&vr1, vnresult);
}
@@ -4543,6 +4545,23 @@ visit_reference_op_load (tree lhs, tree op, gimple *stmt)
}
+/* Returns whether the store to LHS affects dependence analysis compared
+ to the next preceeding stmt doing so at VUSE. */
+
+static bool
+store_affects_tbaa_memory_state (tree lhs, tree vuse)
+{
+ gassign *def = dyn_cast (SSA_NAME_DEF_STMT (vuse));
+ if (!def)
+return true;
+ alias_set_type lhs_set = get_alias_set (lhs);
+ alias_set_type def_set = get_alias_set (gimple_assign_lhs (def));
+ if (lhs_set != def_set
+ && !alias_set_subset_of (lhs_set, def_set))
+return true;
+ return false;
+}
+
/* Visit a store to a reference operator LHS, part of STMT, value number it,
and return true if the value number of the LHS has changed as a result. */
@@ -4575,7 +4594,8 @@ visit_reference_op_store (tree lhs, tree op, gimple *stmt)
Otherwise, the vdefs for the store are used when inserting into
the table, since the store generates a new memory state. */
- vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false);
+ tree last_vuse = NULL_TREE;
+ vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false, &last_vuse);
if (vnresult
&& vnresult->result)
{
@@ -4587,9 +4607,7 @@ visit_reference_op_store (tree lhs, tree op, gimple *stmt)
{
/* If the TBAA state isn't compatible for downstream reads
we cannot value-number the VDEFs the same. */
- alias_set_type set = get_alias_set (lhs);
- if (vnresult->set != set
- && ! alias_set_subset_of (set, vnresult->set))
+ if (store_affects_tbaa_memory_state (lhs, last_vuse))
resultsame = false;
Re: Add News-feed item for git transition
On Wed, 22 Jan 2020, Richard Earnshaw (lists) wrote: > We're missing a statement on the main news feed about the git transition. Lovely, thanks. Gerald PS: Lovely referring to you creating the patch, not the missing announcement. ;-)
Re: [PATCH] Fix PowerPC -fstack-clash-protection -mprefixed-addr ICE (PR target/93122)
Hi! On Mon, Jan 06, 2020 at 09:24:10AM +0100, Jakub Jelinek wrote: > 1) always use gen_add3_insn, but if it returns NULL use the extra > emit_move_insn (end_addr, GEN_INT (-rounded_size)); > and retry with end_addr instead of rs > 2) if the first gen_add3_insn returned NULL or if it created more than one > instruction, add the REG_FRAME_RELATED_EXPR note on the last insn 3) Don't use gen_add3_insn, generate the wanted insns directly. Why can't we do that? gen_add3_insn is fine for generic expansion, but that is not what we are doing here at all. And since not everything it can do is okay here at all apparently, it is a bad choice. It looks like your patch will pessimise code in some cases as well, btw? Segher
Re: [PATCH] cfgexpand: Update partition size when merging variables
On Wed, Jan 22, 2020 at 11:59 AM Richard Sandiford
wrote:
>
> cfgexpand sorts variables by decreasing size, so when merging a later
> variable into an earlier one, there's usually no need to update the
> merged size.
>
> But for poly_int sizes, the sort function just uses a lexicographical
> comparison of the coefficients, so e.g. 2X+2 comes before 0X+32.
> Which is bigger depends on the runtime value of X.
>
> This patch therefore takes the upper bound of the two sizes, which
> is conservatively correct for variable-length vectors and a no-op
> on other targets.
>
> It's probably a bad idea to merge fixed-length and variable-length
> variables in practice, but that's really an optimisation decision.
> I think we should have this patch as a correctness fix either way.
>
> This is easiest to test using the ACLE, but in principle it could happen
> for autovectorised code too, e.g. when using OpenMP vector variables.
> It's therefore a regression from GCC 8.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
OK.
Richard.
> Richard
>
>
> 2020-01-22 Richard Sandiford
>
> gcc/
> * cfgexpand.c (union_stack_vars): Update the size.
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/acle/general/stack_vars_1.c: New test.
> ---
> gcc/cfgexpand.c | 3 ++
> .../aarch64/sve/acle/general/stack_vars_1.c | 32 +++
> 2 files changed, 35 insertions(+)
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index fb0575b146e..9864e4344d2 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -862,6 +862,9 @@ union_stack_vars (size_t a, size_t b)
>stack_vars[b].representative = a;
>stack_vars[a].next = b;
>
> + /* Make sure A is big enough to hold B. */
> + stack_vars[a].size = upper_bound (stack_vars[a].size, stack_vars[b].size);
> +
>/* Update the required alignment of partition A to account for B. */
>if (stack_vars[a].alignb < stack_vars[b].alignb)
> stack_vars[a].alignb = stack_vars[b].alignb;
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
> new file mode 100644
> index 000..860fa67fbe6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c
> @@ -0,0 +1,32 @@
> +/* { dg-do run { target aarch64_sve_hw } } */
> +/* { dg-additional-options "-O2" } */
> +
> +#include
> +
> +struct s { int x[32]; };
> +
> +void __attribute__((noipa)) consume (void *ptr) {}
> +
> +void __attribute__((noipa))
> +check_var (svint32_t *ptr)
> +{
> + svbool_t pg = svptrue_b8 ();
> + if (svptest_any (pg, svcmpne (pg, *ptr, svindex_s32 (0, 1
> +__builtin_abort ();
> +}
> +
> +int
> +main (void)
> +{
> + svint32_t res = svindex_s32 (0, 1);
> + {
> +__SVBool_t pg = svptrue_b8 ();
> +consume (&pg);
> + }
> + {
> +struct s zeros = { 0 };
> +consume (&zeros);
> + }
> + check_var (&res);
> + return 0;
> +}
Re: [PATCH] Fix PowerPC -fstack-clash-protection -mprefixed-addr ICE (PR target/93122)
On Wed, Jan 22, 2020 at 08:06:03AM -0600, Segher Boessenkool wrote: > On Mon, Jan 06, 2020 at 09:24:10AM +0100, Jakub Jelinek wrote: > > 1) always use gen_add3_insn, but if it returns NULL use the extra > > emit_move_insn (end_addr, GEN_INT (-rounded_size)); > > and retry with end_addr instead of rs > > 2) if the first gen_add3_insn returned NULL or if it created more than one > > instruction, add the REG_FRAME_RELATED_EXPR note on the last insn > > 3) Don't use gen_add3_insn, generate the wanted insns directly. Why > can't we do that? gen_add3_insn is fine for generic expansion, but that > is not what we are doing here at all. And since not everything it can do > is okay here at all apparently, it is a bad choice. I don't see how not using gen_add3_insn would make anything easier, the problem isn't gen_add3_insn, but that the add expander has predicates that need to be satisfied, and even if I build the insn by hand, I'll still need to make sure the predicates are satisfied and for that will need to check them anyway. > It looks like your patch will pessimise code in some cases as well, btw? No, it will solely turn previous wrong-codes into something that works (== cases where gen_addr3_insn would previously fail). The 1)+2) variant could even improve code, as gen_addr3_insn could succeed even if we currently don't call it (perhaps generate more than one insn, but it still might be better than forcing the constant into register and then performing add that way). Jakub
[PATCH] avoid -Wrestrict on sprintf %p with destination as argument (PR 84919)
The early front-end only implementation of -Wrestrict that's still
present in GCC 10 issues a false postive for %p arguments that are
the same as the destination. Bug 84919 reports an instance of this
false positive in the Linux kernel.
That attached patch suppresses the front-end warning for the sprintf
family of functions, letting the sprintf pass that was in GCC 10
extended to also handle -Wrestrict for these functions, handle
them instead.
Tested on x86_64-linux.
Since this is a regression I'd like to commit the fix to GCC 10.
Martin
PR c/84919 - bogus -Wrestrict on sprintf %p with destination as argument
gcc/c-family/ChangeLog:
PR c/84919
* c-common.c (check_function_arguments): Avoid overlap checking
of sprintf functions.
gcc/testsuite/ChangeLog:
PR c/84919
* gcc.dg/Wrestrict-20.c: New test.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4a7f3a52335..4bb21772c64 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5732,8 +5732,26 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
if (warn_format)
check_function_sentinel (fntype, nargs, argarray);
- if (warn_restrict)
-warned_p |= check_function_restrict (fndecl, fntype, nargs, argarray);
+ if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
+{
+ switch (DECL_FUNCTION_CODE (fndecl))
+ {
+ case BUILT_IN_SPRINTF:
+ case BUILT_IN_SPRINTF_CHK:
+ case BUILT_IN_SNPRINTF:
+ case BUILT_IN_SNPRINTF_CHK:
+ /* Let the sprintf pass handle these. */
+ return warned_p;
+
+ default:
+ break;
+ }
+}
+
+ /* check_function_restrict sets the DECL_READ_P for arguments
+ so it must be called unconditionally. */
+ warned_p |= check_function_restrict (fndecl, fntype, nargs, argarray);
+
return warned_p;
}
diff --git a/gcc/testsuite/gcc.dg/Wrestrict-20.c b/gcc/testsuite/gcc.dg/Wrestrict-20.c
new file mode 100644
index 000..9826e7f4503
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wrestrict-20.c
@@ -0,0 +1,41 @@
+/* PR c/84919 - bogus -Wrestrict on sprintf %p with destination as argument
+ { dg-do compile }
+ -O2 isn't strictly necessary but setting also verifies that the sprintf/
+ strlen pass doesn't warn with non-constant arguments.
+ { dg-options "-O2 -Wall" } */
+
+extern int sprintf (char* restrict, const char* restrict, ...);
+extern int snprintf (char* restrict, __SIZE_TYPE__, const char* restrict, ...);
+
+char a[32];
+
+void test_warn (char *p)
+{
+ a[0] = 0;
+ sprintf (a, "a=%s", a); /* { dg-warning "-Wrestrict" } */
+
+ p = a;
+ char *q = p + 1;
+ sprintf (p, "a=%s", q); /* { dg-warning "-Wrestrict" } */
+}
+
+void test_nowarn_front_end (char *d)
+{
+ sprintf (d, "%p", d);
+ snprintf (d, 32, "%p", d);
+
+ sprintf (a, "p=%p", a);
+ snprintf (a, sizeof a, "%p", a);
+}
+
+void test_nowarn_sprintf_pass (char *d)
+{
+ char *q = d;
+
+ sprintf (d, "p=%p", q);
+ snprintf (d, 32, "p=%p", q);
+
+ q = a;
+ sprintf (a, "a=%p", q);
+ snprintf (a, sizeof a, "a=%p", q);
+}
Re: [patch, fortran] Fix PR 85781, ICE on valid
On Wed, Jan 22, 2020 at 11:59:12AM +0100, Tobias Burnus wrote: > > And, additionally, about missing diagnostic related to (2) bind(c) and > kind=4, > Are you sure there is a missing diagnostic? You need to add -Wc-binding-type or -Wall to your command line. subroutine p(c) bind(c) use iso_c_binding character(kind=4) c print *, c end % gfcx -Wall -c a.f90 a.f90:1:14: 1 | subroutine p(c) bind(c) | 1 Warning: Variable 'c' at (1) is a dummy argument of the BIND(C) procedure 'p' but may not be C interoperable [-Wc-binding-type] -- Steve
[C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Hi,
in this simple issue we either wrongly talked about variable template-id
in c++17 mode or ICEd in c++2a. I think we simply want to handle
concept-ids first, both as represented in c++17 mode and as represented
in c++2a mode. Tested x86_64-linux.
Thanks, Paolo.
///
Fix "PR c++/92804 ICE trying to use concept as a nested-name-specifier"
A rather simple ICE where we failed to properly check for concept-ids
uses in nested-name-specifiers.
Tested x86_64-linux.
/cp
PR c++/92804
* parser.c (cp_parser_nested_name_specifier_opt): Properly
diagnose concept-ids.
/testsuite
PR c++/92804
* g++.dg/concepts/pr92804.C: New.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..fe490d4a69f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6472,11 +6472,18 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
}
else
{
- /* Variable template. */
tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
- "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ {
+ /* Variable template. */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+ "%qD in nested-name-specifier", tid);
+ }
}
if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804.C
b/gcc/testsuite/g++.dg/concepts/pr92804.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target c++17 } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
[PATCH] gcc: Add new configure options to allow static libraries to be selected
The motivation behind this change is to make it easier for a user to link against static libraries on a target where dynamic libraries are the default library type (for example GNU/Linux). Further, my motivation is really for linking libraries into GDB, however, the binutils-gdb/config/ directory is a copy of gcc/config/ so changes for GDB need to be approved by the GCC project first. After making this change in the gcc/config/ directory I've run autoreconf on all of the configure scripts in the GCC tree and a couple have been updated, so I'll use one of these to describe what my change does. Consider libcpp, this library links against libiconv. Currently if the user builds on a system with both static and dynamic libiconv installed then autotools will pick up the dynamic libiconv by default. This is almost certainly the right thing to do. However, if the user wants to link against static libiconv then things are a little harder, they could remove the dynamic libiconv from their system, but this is probably a bad idea (other things might depend on that library), or the user can build their own version of libiconv, install it into a unique prefix, and then configure gcc using the --with-libiconv-prefix=DIR flag. This works fine, but is somewhat annoying, the static library available, I just can't get autotools to use it. My change then adds a new flag --with-libiconv-type=TYPE, where type is either auto, static, or shared. The default auto, ensures we keep the existing behaviour unchanged. If the user configures with --with-libiconv-type=static then the configure script will ignore any dynamic libiconv it finds, and will only look for a static libiconv, if no static libiconv is found then the configure will continue as though there is no libiconv at all available. Similarly a user can specify --with-libiconv-type=shared and force the use of shared libiconv, any static libiconv will be ignored. As I've implemented this change within the AC_LIB_LINKFLAGS_BODY macro then only libraries configured using the AC_LIB_LINKFLAGS or AC_LIB_HAVE_LINKFLAGS macros will gain the new configure flag. If this is accepted into GCC then there will be follow on patches for binutils and GDB to regenerate some configure scripts in those projects. For GCC only two configure scripts needed updated after this commit, libcpp and libstdc++-v3, both of which link against libiconv. config/ChangeLog: * lib-link.m4 (AC_LIB_LINKFLAGS_BODY): Add new --with-libXXX-type=... option. Use this to guide the selection of either a shared library or a static library. libcpp/ChangeLog: * configure: Regnerate. libstdc++-v3/ChangeLog: * configure: Regnerate. --- config/ChangeLog | 6 ++ config/lib-link.m4 | 22 -- libcpp/ChangeLog | 4 libcpp/configure | 29 +++-- libstdc++-v3/ChangeLog | 4 libstdc++-v3/configure | 47 --- 6 files changed, 85 insertions(+), 27 deletions(-) diff --git a/config/lib-link.m4 b/config/lib-link.m4 index eeb200d266d..662192e0a07 100644 --- a/config/lib-link.m4 +++ b/config/lib-link.m4 @@ -150,6 +150,11 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY], fi fi ]) + AC_LIB_ARG_WITH([lib$1-type], +[ --with-lib$1-type=TYPE type of library to search for (auto/static/shared) ], + [ with_lib$1_type=$withval ], [ with_lib$1_type=auto ]) + lib_type=`eval echo \$with_lib$1_type` + dnl Search the library and its dependencies in $additional_libdir and dnl $LDFLAGS. Using breadth-first-seach. LIB[]NAME= @@ -195,13 +200,13 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY], found_so= found_a= if test $use_additional = yes; then -if test -n "$shlibext" && test -f "$additional_libdir/lib$name.$shlibext"; then +if test -n "$shlibext" && test -f "$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then found_dir="$additional_libdir" found_so="$additional_libdir/lib$name.$shlibext" if test -f "$additional_libdir/lib$name.la"; then found_la="$additional_libdir/lib$name.la" fi -else +elif test x$lib_type != xshared; then if test -f "$additional_libdir/lib$name.$libext"; then found_dir="$additional_libdir" found_a="$additional_libdir/lib$name.$libext" @@ -217,13 +222,13 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY], case "$x" in -L*) dir=`echo "X$x" | sed -e 's/^X-L//'` - if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext"; then + if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext" && test x$lib_type != xstatic; then found_dir="$dir" found_so="$dir/lib$name.$shlibext" if test -f "$d
wwwdocs: Document the gcc git repository layout
The GCC git repository makes use of some non-standard refs so that by
default only the most active and useful branches and tags are fetched by
a git clone. This patch documents the additional branch and tag spaces
that exist on the server.
Joseph, have I got all of these right?
R.
diff --git a/htdocs/git.html b/htdocs/git.html
index 6100146e..6d3778af 100644
--- a/htdocs/git.html
+++ b/htdocs/git.html
@@ -197,6 +197,61 @@ To create a worktree for a new project branch based on master, do
git worktree add -b project ../project master
+Repository Layout
+
+By default, a git clone operation will only fetch the
+main development, release branches and their associated tags from the
+server. This will be sufficient for most users, but a number of
+additional branches can also be fetched if necessary.
+
+The following areas exist on the server:
+
+ refs/heads/master - The active development version of the
+compiler.
+ refs/heads/releases/* - Release branches.
+ refs/heads/devel/* - topic-specific development branches
+- see Active Development Branches.
+Branches and tags in this space may be moved to refs/dead once active
+development is completed or abandoned.
+ refs/tags/* - tags for all of the above branches.
+ refs/vendors/*/{heads,tags}/* - vendor-specific branches and
+tags. These are owned and maintained by the respective
+vendor, but made available publicly. Non-fast-forward pushes
+are permitted on these branches.
+ refs/users/*/{heads,tags}/* - personal developer branches and
+tags. These are owned and maintained by the individual developer.
+Non-fast-forward pushes are permitted on these branches.
+ refs/dead/heads/* - completed or abandoned development
+branches.
+ refs/meta - local configuration data for the commit hooks
+on the server.
+ refs/deleted/*/{heads,tags}/* - old branches and tags
+from the SVN repository that were deleted before the conversion to
+git.
+ refs/git-svn-old/* - various branches and tags from the
+git-svn pre-conversion git mirror.
+ refs/git-old/* - various branches and tags from the
+git-svn pre-conversion git mirror that were local to that git
+repository.
+
+
+You can download any of the additional branches by adding a suitable
+fetch specification to your local copy of the git repostiory. For
+example, if your remote is called 'origin' (the default with git
+clone) you can add the 'dead' development branches by running:
+
+
+git config --add remote.origin.fetch "+refs/dead/heads/*:refs/remotes/origin/dead/*"
+git config --add remote.origin.fetch "+refs/dead/tags/*:refs/tags/dead/*"
+git fetch origin
+
+
+which will place the dead branches in remotes/origin/dead
+ and the tags in tags/dead.
+
+You can use git ls-remote to get a complete list of
+refs that the server holds.
+
Active Development Branches
Re: [patch, fortran] Fix PR 85781, ICE on valid
Hi Steve,
On 1/22/20 4:02 PM, Steve Kargl wrote:
On Wed, Jan 22, 2020 at 11:59:12AM +0100, Tobias Burnus wrote:
And, additionally, about missing diagnostic related to (2) bind(c) and
kind=4,
Are you sure there is a missing diagnostic? You need to
add -Wc-binding-type or -Wall to your command line.
The problem with the ISO C warnings for character is that they are not
based on the KIND value but whether the variable comes from the
ISO_C_BINDING module or not.
Namely, for kind=1 and kind=4 you get the warning you have shown while
for kind=c_char and kind=c_int32_t you don't. (If you use "integer,
parameter :: mykind = c_char", the result is the same as for c_char as
this property is inherited.)
Numerically, 1 and c_char are the same – as are 4 and c_int32_t. I think
one *can* warn for using kind=1 as this is bad style (= assumption that
c_char is the same as "1" and the same as the default-kind character).
But "kind=4" is as bad as "kind=c_int32_t"! — And as noted in the PR,
using c_char and c_int32_t (!) will give a "character(kind=1)" (note the
kind=1!) dummy argument while kind=1 and kind=4 will give
"character(kind={1 or 4})[1:1]" (= ARRAY_TYPE).
That's complete nonsense and in some cases it does not even depend on
whether that's a BIND(C) procedure or not!
Cheers,
Tobias
Re: [patch, fortran] Fix PR 85781, ICE on valid
On Wed, Jan 22, 2020 at 04:43:49PM +0100, Tobias Burnus wrote:
> Hi Steve,
>
> On 1/22/20 4:02 PM, Steve Kargl wrote:
> > On Wed, Jan 22, 2020 at 11:59:12AM +0100, Tobias Burnus wrote:
> >> And, additionally, about missing diagnostic related to (2) bind(c) and
> >> kind=4,
> > Are you sure there is a missing diagnostic? You need to
> > add -Wc-binding-type or -Wall to your command line.
>
> The problem with the ISO C warnings for character is that they are not
> based on the KIND value but whether the variable comes from the
> ISO_C_BINDING module or not.
>
> Namely, for kind=1 and kind=4 you get the warning you have shown while
> for kind=c_char and kind=c_int32_t you don't. (If you use "integer,
> parameter :: mykind = c_char", the result is the same as for c_char as
> this property is inherited.)
>
> Numerically, 1 and c_char are the same – as are 4 and c_int32_t. I think
> one *can* warn for using kind=1 as this is bad style (= assumption that
> c_char is the same as "1" and the same as the default-kind character).
>
> But "kind=4" is as bad as "kind=c_int32_t"! — And as noted in the PR,
> using c_char and c_int32_t (!) will give a "character(kind=1)" (note the
> kind=1!) dummy argument while kind=1 and kind=4 will give
> "character(kind={1 or 4})[1:1]" (= ARRAY_TYPE).
>
> That's complete nonsense and in some cases it does not even depend on
> whether that's a BIND(C) procedure or not!
>
Not that I disagree with you, but this is what happens when
UTF-8 (kind=4) is wedged on top of ISO C Binding (kind=c_char),
which is wedged on top of a Fortran 95 compiler. Supposedly, one
can use attr.is_c_interop and attr.is_iso_c to determine if something
is interoperable (if it is consistently set) and if it is from
the ISO C Binding module.
The solution is to use unique kind values forall types (reserve
1-9 for integer, 11-19 for real, 21-29 for character, etc). Then
one simply needs to define a storage association mapping.
--
Steve
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
"Richard Earnshaw (lists)" writes: > On 21/01/2020 17:20, Jason Merrill wrote: >> On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: >>> On 21/01/2020 15:39, Jakub Jelinek wrote: On Tue, Jan 21, 2020 at 03:33:22PM +, Richard Earnshaw (lists) wrote: >> Some examples would be useful I'd say, e.g. it is unclear in what >> way you >> want the PR number to be appended, shall it be >> something: whatever words describe it PR12345 >> or >> something: whatever words describe it (PR12345) >> or >> something: whatever words describe it: PR12345 >> or >> something: whatever words describe it [PR12345] >> or something else? > > Glibc use "[BZ #]" - obviously BZ becomes PR, but after that, > I'm not > too worried. I'd be happy with [PR #], but if folk want > something else, > please say so quickly... [PR 12345] or [PR #12345] is bad, because the bugzilla won't underline it, it needs to be either PR12345 word, or PR component/12345 . >>> >>> ok, lets go with [PR] then. >> >> Doesn't this use of [] have the same problem with git am? > > No, because only 'leading' [] blocks are removed - git mailinfo --help > >> >> My summaries are often describing the bug I'm fixing, i.e. >> >> [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. >> >> which is also the first line of my ChangeLog entry. I think you are >> proposing >> >> [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs >> (PR91476) >> >> which can no longer be shared with the ChangeLog. >> > > I was trying to unify this with glibc. They specify the bug number at > the end of the line. > > We can diverge if it's generally felt to be important, but details like > this create needless friction for folk working in both communities. +1 for "component: Summary [PRn]" FWIW. PR bz-component/n works well for C++. The problem is that so many other PRs come under tree-optimization and rtl-optimization, which eat up a lot of subject line characters without narrowing things down very much. "cselib: ... [PRn]" is both shorter and more descriptive than "PR rtl-optimization/n - ", etc. Same idea for "PR target/n - ...": you then need to say which target you mean in the main summary, whereas "aarch64: [PRn]" makes it easier to keep the main summary short. Maybe that's just a problem with the bz classification though... Richard
Re: [patch, fortran] Fix PR 85781, ICE on valid
Hi Steve, On 1/22/20 4:54 PM, Steve Kargl wrote: Supposedly, one can use attr.is_c_interop and attr.is_iso_c to determine if something is interoperable (if it is consistently set) and if it is from the ISO C Binding module. I think this is fixable on two sides: * For the used tree type, one just has to check kind value (the integer) plus whether the dummy's procedure is BIND(C). Here the problem is that we do not always have this information available (i.e. it needs to be passed on). Not difficult, but one needs to find all spots and think of how to pass the information. * For the diagnostic, we can check whether the dummy argument is in bind(C) and do some additional diagnostic in this case (e.g. always warn for kind=4 with BIND(C) – or reject it unconditionally or with -std=... or …). * The other issue related to kind=4: Fortran permits to pass a too long string as actual argument; but this is only permitted in the standard for kind=1 and kind=c_char strings. In gfortran, this also works fine with kind=4 – but one might still want to warn (with some -Wall or -std=f... or ...). Likewise, the trimming happening in this case might indicate a bug in the program: "Hello World" as actual to character(len=10) is most likely a bug, even if this is valid Fortran. Cheers, Tobias
Re: [PATCH] Fix a couple of memory leaks in the C++ frontend
On Tue, 21 Jan 2020, Jason Merrill wrote: On 1/20/20 8:06 PM, Patrick Palka wrote: The leak in get_mapped_args is due to auto_vec not properly supporting destructible elements, in that auto_vec's destructor doesn't call the destructors of its elements. Hmm, perhaps vec should static_assert __is_trivial(T) when supported. Unfortunately this would break other seemingly well-behaved users of vec in which T doesn't have a trivial default constructor. Relaxing the condition to __has_trivial_copy(T) && __has_trivial_destructor(T) seems to be fine though. @@ -15344,6 +15344,7 @@ cp_literal_operator_id (const char* name) + strlen (name) + 10); sprintf (buffer, UDLIT_OP_ANSI_FORMAT, name); identifier = get_identifier (buffer); + free (buffer); I guess we should use XDELETEVEC to match XNEWVEC. OK with that change. Patch committed with that change. Jason
Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
On 1/22/20 10:22 AM, Paolo Carlini wrote: Hi, in this simple issue we either wrongly talked about variable template-id in c++17 mode or ICEd in c++2a. I think we simply want to handle concept-ids first, both as represented in c++17 mode and as represented in c++2a mode. Tested x86_64-linux. What happens if you try to use a function template/function concept name? Jason
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On Wed, Jan 22, 2020 at 04:05:37PM +, Richard Sandiford wrote: > "Richard Earnshaw (lists)" writes: > > On 21/01/2020 17:20, Jason Merrill wrote: > >> On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: > >>> On 21/01/2020 15:39, Jakub Jelinek wrote: > On Tue, Jan 21, 2020 at 03:33:22PM +, Richard Earnshaw (lists) > wrote: > >> Some examples would be useful I'd say, e.g. it is unclear in what > >> way you > >> want the PR number to be appended, shall it be > >> something: whatever words describe it PR12345 > >> or > >> something: whatever words describe it (PR12345) > >> or > >> something: whatever words describe it: PR12345 > >> or > >> something: whatever words describe it [PR12345] > >> or something else? > > > > Glibc use "[BZ #]" - obviously BZ becomes PR, but after that, > > I'm not > > too worried. I'd be happy with [PR #], but if folk want > > something else, > > please say so quickly... > > [PR 12345] or [PR #12345] is bad, because the bugzilla won't > underline it, > it needs to be either PR12345 word, or PR component/12345 . > >>> > >>> ok, lets go with [PR] then. > >> > >> Doesn't this use of [] have the same problem with git am? > > > > No, because only 'leading' [] blocks are removed - git mailinfo --help > > > >> > >> My summaries are often describing the bug I'm fixing, i.e. > >> > >> [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. > >> > >> which is also the first line of my ChangeLog entry. I think you are > >> proposing > >> > >> [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs > >> (PR91476) > >> > >> which can no longer be shared with the ChangeLog. > >> > > > > I was trying to unify this with glibc. They specify the bug number at > > the end of the line. > > > > We can diverge if it's generally felt to be important, but details like > > this create needless friction for folk working in both communities. > > +1 for "component: Summary [PRn]" FWIW. > > PR bz-component/n works well for C++. The problem is that so many > other PRs come under tree-optimization and rtl-optimization, which > eat up a lot of subject line characters without narrowing things down > very much. "cselib: ... [PRn]" is both shorter and more descriptive > than "PR rtl-optimization/n - ", etc. Yeah, the cselib version definitely looks preferable to me. What if a patch touches more than just the C++ FE, do we want "c,c++:"? Though kernel uses net: sched: act_ctinfo: fix memory leak locking/rwsem: Fix kernel crash when spinning on RWSEM_OWNER_UNKNOWN If a patch touches various spots in the optimizers, maybe we can just go with "tree-opt:" or "rtl:"? Further, I suppose multiple PRs fixed by a single patch would look like: c++: Implement DR 666 [PR57, PR12345] Marek
Re: [C++ PATCH v2] PR c++/92907 - noexcept does not consider "const" in member functions.
On Tue, Jan 21, 2020 at 10:40:09PM -0500, Jason Merrill wrote: > On 1/21/20 9:08 PM, Marek Polacek wrote: > > Here the problem is that if the noexcept specifier is used in the context > > of a const member function, const is not considered for the member > > variables, > > leading to a bogus error. g's const makes its 'this' const, so the first > > overload of f should be selected. > > > > In cp_parser_noexcept_specification_opt we inject 'this', but always > > unqualified: > > 25737 if (current_class_type) > > 25738 inject_this_parameter (current_class_type, > > TYPE_UNQUALIFIED); > > so we need to pass the function's qualifiers down here. In > > cp_parser_direct_declarator it's easy: use the just parsed cv_quals, in > > cp_parser_late_noexcept_specifier look at the 'this' parameter to figure it > > out. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? Not planning to > > backport it to 9, this is not really a regression. > > > > 2020-01-21 Marek Polacek > > > > PR c++/92907 - noexcept does not consider "const" in member functions. > > * parser.c (cp_parser_lambda_declarator_opt): Pass TYPE_UNQUALIFIED > > down to cp_parser_exception_specification_opt. > > (cp_parser_direct_declarator): Pass the function qualifiers to > > cp_parser_exception_specification_opt. > > (cp_parser_class_specifier_1): Pass the function declaration to > > cp_parser_late_noexcept_specifier. > > (cp_parser_late_noexcept_specifier): Add a tree parameter. Use it to > > pass the qualifiers of the function to > > cp_parser_noexcept_specification_opt. > > (cp_parser_noexcept_specification_opt): New cp_cv_quals parameter. > > Use it in inject_this_parameter. > > (cp_parser_exception_specification_opt): New cp_cv_quals parameter. > > Use it. > > (cp_parser_transaction): Pass TYPE_UNQUALIFIED to > > cp_parser_noexcept_specification_opt. > > (cp_parser_transaction_expression): Likewise. > > > > * g++.dg/cpp0x/noexcept56.C: New test. > > --- > > gcc/cp/parser.c | 53 + > > gcc/testsuite/g++.dg/cpp0x/noexcept56.C | 10 + > > 2 files changed, 46 insertions(+), 17 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept56.C > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > index caafbefda8e..e3566f9bd4d 100644 > > --- a/gcc/cp/parser.c > > +++ b/gcc/cp/parser.c > > @@ -11008,7 +11008,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, > > tree lambda_expr) > > /* Parse optional exception specification. */ > > exception_spec > > - = cp_parser_exception_specification_opt (parser, CP_PARSER_FLAGS_NONE); > > + = cp_parser_exception_specification_opt (parser, CP_PARSER_FLAGS_NONE, > > +TYPE_UNQUALIFIED); > > This seems wrong; a lambda op() is const unless explicitly 'mutable'. A bit > further down we have Indeed, I didn't change it for lambdas at all. > > quals = (LAMBDA_EXPR_MUTABLE_P (lambda_expr) > > ? TYPE_UNQUALIFIED : TYPE_QUAL_CONST); > > You can probably move that up? Here's what I did. Unfortunately it doesn't fix PR79620. -- >8 -- Here the problem is that if the noexcept specifier is used in the context of a const member function, const is not considered for the member variables, leading to a bogus error. g's const makes its 'this' const, so the first overload of f should be selected. In cp_parser_noexcept_specification_opt we inject 'this', but always unqualified: 25737 if (current_class_type) 25738 inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); so we need to pass the function's qualifiers down here. In cp_parser_direct_declarator it's easy: use the just parsed cv_quals, in cp_parser_late_noexcept_specifier look at the 'this' parameter to figure it out. Bootstrapped/regtested on x86_64-linux, ok for trunk? Not planning to backport it to 9, this is not really a regression. 2020-01-22 Marek Polacek PR c++/92907 - noexcept does not consider "const" in member functions. * parser.c (cp_parser_lambda_declarator_opt): Pass the proper qualifiers to cp_parser_exception_specification_opt. (cp_parser_direct_declarator): Pass the function qualifiers to cp_parser_exception_specification_opt. (cp_parser_class_specifier_1): Pass the function declaration to cp_parser_late_noexcept_specifier. (cp_parser_late_noexcept_specifier): Add a tree parameter. Use it to pass the qualifiers of the function to cp_parser_noexcept_specification_opt. (cp_parser_noexcept_specification_opt): New cp_cv_quals parameter. Use it in inject_this_parameter. (cp_parser_exception_specification_opt): New cp_cv_quals parameter. Use it. (cp_parser_transaction): Pass TYPE_UNQUALIFIED to cp_parser_noexcept_specificatio
Re: [PATCH, v2] wwwdocs: e-mail subject lines for contributions
On 22/01/2020 16:28, Marek Polacek wrote: On Wed, Jan 22, 2020 at 04:05:37PM +, Richard Sandiford wrote: "Richard Earnshaw (lists)" writes: On 21/01/2020 17:20, Jason Merrill wrote: On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: On 21/01/2020 15:39, Jakub Jelinek wrote: On Tue, Jan 21, 2020 at 03:33:22PM +, Richard Earnshaw (lists) wrote: Some examples would be useful I'd say, e.g. it is unclear in what way you want the PR number to be appended, shall it be something: whatever words describe it PR12345 or something: whatever words describe it (PR12345) or something: whatever words describe it: PR12345 or something: whatever words describe it [PR12345] or something else? Glibc use "[BZ #]" - obviously BZ becomes PR, but after that, I'm not too worried. I'd be happy with [PR #], but if folk want something else, please say so quickly... [PR 12345] or [PR #12345] is bad, because the bugzilla won't underline it, it needs to be either PR12345 word, or PR component/12345 . ok, lets go with [PR] then. Doesn't this use of [] have the same problem with git am? No, because only 'leading' [] blocks are removed - git mailinfo --help My summaries are often describing the bug I'm fixing, i.e. [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. which is also the first line of my ChangeLog entry. I think you are proposing [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs (PR91476) which can no longer be shared with the ChangeLog. I was trying to unify this with glibc. They specify the bug number at the end of the line. We can diverge if it's generally felt to be important, but details like this create needless friction for folk working in both communities. +1 for "component: Summary [PRn]" FWIW. PR bz-component/n works well for C++. The problem is that so many other PRs come under tree-optimization and rtl-optimization, which eat up a lot of subject line characters without narrowing things down very much. "cselib: ... [PRn]" is both shorter and more descriptive than "PR rtl-optimization/n - ", etc. Yeah, the cselib version definitely looks preferable to me. What if a patch touches more than just the C++ FE, do we want "c,c++:"? c-common: or c-family:? Though kernel uses net: sched: act_ctinfo: fix memory leak I'd read that as a patch that crosses the three separate components. locking/rwsem: Fix kernel crash when spinning on RWSEM_OWNER_UNKNOWN And this would be a patch that affectst the rwsem sub-component of locking. If a patch touches various spots in the optimizers, maybe we can just go with "tree-opt:" or "rtl:"? Not sure we want to get that prescriptive. Things are likely to change around anyway. rtl: would suggest it was one of the non-specific rtl-related issues, tree: similarly for a tree-related issue. Further, I suppose multiple PRs fixed by a single patch would look like: c++: Implement DR 666 [PR57, PR12345] Depends on the overall context. In the subject line I think it would be acceptable to reference just one, perhaps two PRs. If there are more, then something like [PRnnn, ...] would probably be an acceptable way of showing that more were referenced later in the body. Marek
[PATCH] Fix comparison of trees via tree_cmp
Hi David,
In function `tree_cmp` an invariant [1] is assumed which does not necessarily
exist. In case both input trees are finally compared via `strcmp`, then
tree_cmp (t1, t2) == -tree_cmp (t2, t1)
does not hold in general, since function `strcmp (x, y)` guarantees only that a
negative integer is returned in case xy.
Currently this breaks s390x where, for example, for certain inputs x,y
`tree_cmp (x, y) == 1` and `tree_cmp (y, x) == -2` hold. The attached patch
normalizes the output from `strcmp` to -1, 0, or 1 while using an auxiliary
function `sign` (stolen from the Hacker's Delight book ;-)).
Bootstrapped and tested on s390x. Any thoughts?
Cheers,
Stefan
[1]
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;a=blob;f=gcc/analyzer/region-model.cc;h=9474c6737d54d68f5b36893903cfa6d19df0efed;hb=HEAD#l1849
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 9474c6737d5..bdd9a97e5f8 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -62,6 +62,20 @@ along with GCC; see the file COPYING3. If not see
#if ENABLE_ANALYZER
+/* Normalize X to either -1, 0, or 1.
+
+ Basically performs the same as
+ if (x < 0) return -1;
+ else if (x > 0) return 1;
+ else return 0;
+ but in a concise way trying to prevent branches. */
+
+static int
+sign (int x)
+{
+ return (x > 0) - (x < 0);
+}
+
/* Dump T to PP in language-independent form, for debugging/logging/dumping
purposes. */
@@ -1768,8 +1782,8 @@ tree_cmp (const_tree t1, const_tree t2)
if (DECL_P (t1))
{
if (DECL_NAME (t1) && DECL_NAME (t2))
- return strcmp (IDENTIFIER_POINTER (DECL_NAME (t1)),
- IDENTIFIER_POINTER (DECL_NAME (t2)));
+ return sign (strcmp (IDENTIFIER_POINTER (DECL_NAME (t1)),
+IDENTIFIER_POINTER (DECL_NAME (t2;
else
{
if (DECL_NAME (t1))
@@ -1819,8 +1833,8 @@ tree_cmp (const_tree t1, const_tree t2)
}
case STRING_CST:
- return strcmp (TREE_STRING_POINTER (t1),
-TREE_STRING_POINTER (t2));
+ return sign (strcmp (TREE_STRING_POINTER (t1),
+ TREE_STRING_POINTER (t2)));
default:
gcc_unreachable ();
Re: [PATCH] Fix comparison of trees via tree_cmp
On Wed, 22 Jan 2020, Stefan Schulze Frielinghaus wrote: > Hi David, > > In function `tree_cmp` an invariant [1] is assumed which does not necessarily > exist. In case both input trees are finally compared via `strcmp`, then > > tree_cmp (t1, t2) == -tree_cmp (t2, t1) > > does not hold in general, since function `strcmp (x, y)` guarantees only that > a > negative integer is returned in case xy. > Currently this breaks s390x where, for example, for certain inputs x,y > `tree_cmp (x, y) == 1` and `tree_cmp (y, x) == -2` hold. The attached patch > normalizes the output from `strcmp` to -1, 0, or 1 while using an auxiliary > function `sign` (stolen from the Hacker's Delight book ;-)). > > Bootstrapped and tested on s390x. Any thoughts? It's more appropriate to fix the assert rather than the comparator, like gcc_assert (sign (reversed) == -sign (result)); But qsort_chk already checks that, and more, so why is the assert there? Shouldn't it be simply removed? Alexander
Re: [C++ PATCH v2] PR c++/92907 - noexcept does not consider "const" in member functions.
On 1/22/20 11:33 AM, Marek Polacek wrote: Here's what I did. Unfortunately it doesn't fix PR79620. No, I wouldn't expect it to, that looks like more of an issue with how is_lambda_ignored_entity skips captures in unevaluated context. + cp_cv_quals quals; + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)) +quals = cp_type_quals (class_of_this_parm (TREE_TYPE (decl))); + else +quals = TYPE_UNQUALIFIED; This could be cp_cv_quals quals = type_memfn_quals (TREE_TYPE (decl)); OK with that change. Jason
Re: [PATCH] Fix a couple of memory leaks in the C++ frontend
On 1/22/20 11:19 AM, Patrick Palka wrote: On Tue, 21 Jan 2020, Jason Merrill wrote: On 1/20/20 8:06 PM, Patrick Palka wrote: The leak in get_mapped_args is due to auto_vec not properly supporting destructible elements, in that auto_vec's destructor doesn't call the destructors of its elements. Hmm, perhaps vec should static_assert __is_trivial(T) when supported. Unfortunately this would break other seemingly well-behaved users of vec in which T doesn't have a trivial default constructor. Relaxing the condition to __has_trivial_copy(T) && __has_trivial_destructor(T) seems to be fine though. Sure, that makes sense; grow does try to default-construct elements. Jason
Re: [C++ PATCH v3] PR c++/92907 - noexcept does not consider "const" in member functions.
On Wed, Jan 22, 2020 at 12:10:12PM -0500, Jason Merrill wrote:
> On 1/22/20 11:33 AM, Marek Polacek wrote:
> > Here's what I did. Unfortunately it doesn't fix PR79620.
>
> No, I wouldn't expect it to, that looks like more of an issue with how
> is_lambda_ignored_entity skips captures in unevaluated context.
>
> > + cp_cv_quals quals;
> > + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
> > +quals = cp_type_quals (class_of_this_parm (TREE_TYPE (decl)));
> > + else
> > +quals = TYPE_UNQUALIFIED;
>
> This could be
>
> cp_cv_quals quals = type_memfn_quals (TREE_TYPE (decl));
>
> OK with that change.
Awesome, thanks! Here's what I'm going to commit, er, push:
Here the problem is that if the noexcept specifier is used in the context
of a const member function, const is not considered for the member variables,
leading to a bogus error. g's const makes its 'this' const, so the first
overload of f should be selected.
In cp_parser_noexcept_specification_opt we inject 'this', but always
unqualified:
25737 if (current_class_type)
25738 inject_this_parameter (current_class_type, TYPE_UNQUALIFIED);
so we need to pass the function's qualifiers down here. In
cp_parser_direct_declarator it's easy: use the just parsed cv_quals, in
cp_parser_late_noexcept_specifier look at the 'this' parameter to figure it
out.
2020-01-22 Marek Polacek
PR c++/92907 - noexcept does not consider "const" in member functions.
* parser.c (cp_parser_lambda_declarator_opt): Pass the proper
qualifiers to cp_parser_exception_specification_opt.
(cp_parser_direct_declarator): Pass the function qualifiers to
cp_parser_exception_specification_opt.
(cp_parser_class_specifier_1): Pass the function declaration to
cp_parser_late_noexcept_specifier.
(cp_parser_late_noexcept_specifier): Add a tree parameter. Use it to
pass the qualifiers of the function to
cp_parser_noexcept_specification_opt.
(cp_parser_noexcept_specification_opt): New cp_cv_quals parameter.
Use it in inject_this_parameter.
(cp_parser_exception_specification_opt): New cp_cv_quals parameter.
Use it.
(cp_parser_transaction): Pass TYPE_UNQUALIFIED to
cp_parser_noexcept_specification_opt.
(cp_parser_transaction_expression): Likewise.
* g++.dg/cpp0x/noexcept56.C: New test.
---
gcc/cp/parser.c | 55 -
gcc/testsuite/g++.dg/cpp0x/noexcept56.C | 10 +
2 files changed, 45 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept56.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ea32df92f9c..dc07dc55d9c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -246,7 +246,7 @@ static void cp_lexer_stop_debugging
static cp_token_cache *cp_token_cache_new
(cp_token *, cp_token *);
static tree cp_parser_late_noexcept_specifier
- (cp_parser *, tree);
+ (cp_parser *, tree, tree);
static void noexcept_override_late_checks
(tree, tree);
@@ -2388,11 +2388,11 @@ static tree cp_parser_exception_declaration
static tree cp_parser_throw_expression
(cp_parser *);
static tree cp_parser_exception_specification_opt
- (cp_parser *, cp_parser_flags);
+ (cp_parser *, cp_parser_flags, cp_cv_quals);
static tree cp_parser_type_id_list
(cp_parser *);
static tree cp_parser_noexcept_specification_opt
- (cp_parser *, cp_parser_flags, bool, bool *, bool);
+ (cp_parser *, cp_parser_flags, bool, bool *, bool, cp_cv_quals);
/* GNU Extensions */
@@ -10908,6 +10908,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
tree trailing_requires_clause = NULL_TREE;
cp_decl_specifier_seq lambda_specs;
clear_decl_specs (&lambda_specs);
+ /* A lambda op() is const unless explicitly 'mutable'. */
+ cp_cv_quals quals = TYPE_QUAL_CONST;
/* The template-parameter-list is optional, but must begin with
an opening angle if present. */
@@ -10999,6 +11001,7 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
if (lambda_specs.storage_class == sc_mutable)
{
LAMBDA_EXPR_MUTABLE_P (lambda_expr) = 1;
+ quals = TYPE_UNQUALIFIED;
if (lambda_specs.conflicting_specifiers_p)
error_at (lambda_specs.locations[ds_storage_class],
"duplicate %");
@@ -11008,7 +11011,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
/* Parse optional exception specification. */
exception_spec
- = cp_parser_exception_specification_opt (parser, CP_PARSER_FLAGS_NONE);
+ = cp_parser_exception_specification_opt (parser, CP_PARSER_FLAGS_NONE,
+quals);
std_attrs = cp_parser_std_attribute_spec_seq (parser);
@@ -11041,7 +11045,6 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
cp_decl_specifier_seq return
Re: [GCC][PATCH][AArch32] ACLE intrinsics bfloat16 vmmla and vfma for AArch32 AdvSIMD
Ping. I have read Richard Sandiford's comments on the AArch64 patches and I will apply what is relevant to this patch as well. Particularly, I will change the tests to use the exact input and output registers and I will change the types of the rtl patterns. On 12/20/19 6:44 PM, Delia Burduv wrote: > This patch adds the ARMv8.6 ACLE intrinsics for vmmla, vfmab and vfmat > as part of the BFloat16 extension. > (https://developer.arm.com/docs/101028/latest.) > The intrinsics are declared in arm_neon.h and the RTL patterns are > defined in neon.md. > Two new tests are added to check assembler output and lane indices. > > This patch depends on the Arm back-end patche. > (https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01448.html) > > Tested for regression on arm-none-eabi and armeb-none-eabi. I don't have > commit rights, so if this is ok can someone please commit it for me? > > gcc/ChangeLog: > > 2019-11-12 Delia Burduv > > * config/arm/arm_neon.h (vbfmmlaq_f32): New. > (vbfmlalbq_f32): New. > (vbfmlaltq_f32): New. > (vbfmlalbq_lane_f32): New. > (vbfmlaltq_lane_f32): New. > (vbfmlalbq_laneq_f32): New. > (vbfmlaltq_laneq_f32): New. > * config/arm/arm_neon_builtins.def (vbfmmla): New. > (vbfmab): New. > (vbfmat): New. > (vbfmab_lane): New. > (vbfmat_lane): New. > (vbfmab_laneq): New. > (vbfmat_laneq): New. > * config/arm/iterators.md (BF_MA): New int iterator. > (bt): New int attribute. > (VQXBF): Copy of VQX with V8BF. > (V_HALF): Added V8BF. > * config/arm/neon.md (neon_vbfmmlav8hi): New insn. > (neon_vbfmav8hi): New insn. > (neon_vbfma_lanev8hi): New insn. > (neon_vbfma_laneqv8hi): New expand. > (neon_vget_high): Changed iterator to VQXBF. > * config/arm/unspecs.md (UNSPEC_BFMMLA): New UNSPEC. > (UNSPEC_BFMAB): New UNSPEC. > (UNSPEC_BFMAT): New UNSPEC. > > 2019-11-12 Delia Burduv > > * gcc.target/arm/simd/bf16_ma_1.c: New test. > * gcc.target/arm/simd/bf16_ma_2.c: New test. > * gcc.target/arm/simd/bf16_mmla_1.c: New test.
Re: ACLE intrinsics: BFloat16 store (vst{q}_bf16) intrinsics for AArch32
Ping.
I will change the tests to use the exact input and output registers as
Richard Sandiford suggested for the AArch64 patches.
On 12/20/19 6:46 PM, Delia Burduv wrote:
> This patch adds the ARMv8.6 ACLE BFloat16 store intrinsics
> vst{q}_bf16 as part of the BFloat16 extension.
> (https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics)
>
>
> The intrinsics are declared in arm_neon.h .
> A new test is added to check assembler output.
>
> This patch depends on the Arm back-end patche.
> (https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01448.html)
>
> Tested for regression on arm-none-eabi and armeb-none-eabi. I don't have
> commit rights, so if this is ok can someone please commit it for me?
>
> gcc/ChangeLog:
>
> 2019-11-14 Delia Burduv
>
> * config/arm/arm_neon.h (bfloat16_t): New typedef.
> (bfloat16x4x2_t): New typedef.
> (bfloat16x8x2_t): New typedef.
> (bfloat16x4x3_t): New typedef.
> (bfloat16x8x3_t): New typedef.
> (bfloat16x4x4_t): New typedef.
> (bfloat16x8x4_t): New typedef.
> (vst2_bf16): New.
> (vst2q_bf16): New.
> (vst3_bf16): New.
> (vst3q_bf16): New.
> (vst4_bf16): New.
> (vst4q_bf16): New.
> * config/arm/arm-builtins.c (E_V2BFmode): New mode.
> (VAR13): New.
> (arm_simd_types[Bfloat16x2_t]):New type.
> * config/arm/arm-modes.def (V2BF): New mode.
> * config/arm/arm-simd-builtin-types.def
> (Bfloat16x2_t): New entry.
> * config/arm/arm_neon_builtins.def
> (vst2): Changed to VAR13 and added v4bf, v8bf
> (vst3): Changed to VAR13 and added v4bf, v8bf
> (vst4): Changed to VAR13 and added v4bf, v8bf
> * config/arm/iterators.md (VDXBF): New iterator.
> (VQ2BF): New iterator.
> (V_elem): Added V4BF, V8BF.
> (V_sz_elem): Added V4BF, V8BF.
> (V_mode_nunits): Added V4BF, V8BF.
> (q): Added V4BF, V8BF.
> *config/arm/neon.md (vst2): Used new iterators.
> (vst3): Used new iterators.
> (vst3qa): Used new iterators.
> (vst3qb): Used new iterators.
> (vst4): Used new iterators.
> (vst4qa): Used new iterators.
> (vst4qb): Used new iterators.
>
>
> gcc/testsuite/ChangeLog:
>
> 2019-11-14 Delia Burduv
>
> * gcc.target/arm/simd/bf16_vstn_1.c: New test.
Re: ACLE intrinsics: BFloat16 load intrinsics for AArch32
Ping.
I will change the tests to use the exact input and output registers as
Richard Sandiford suggested for the AArch64 patches.
On 12/20/19 6:48 PM, Delia Burduv wrote:
> This patch adds the ARMv8.6 ACLE BFloat16 load intrinsics vld{q}_bf16
> as part of the BFloat16 extension.
> (https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics)
>
>
> The intrinsics are declared in arm_neon.h .
> A new test is added to check assembler output.
>
> This patch depends on the Arm back-end patche.
> (https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01448.html)
>
> Tested for regression on arm-none-eabi and armeb-none-eabi. I don't have
> commit rights, so if this is ok can someone please commit it for me?
>
> gcc/ChangeLog:
>
> 2019-11-14 Delia Burduv
>
> * config/arm/arm_neon.h (bfloat16_t): New typedef.
> (bfloat16x4x2_t): New typedef.
> (bfloat16x8x2_t): New typedef.
> (bfloat16x4x3_t): New typedef.
> (bfloat16x8x3_t): New typedef.
> (bfloat16x4x4_t): New typedef.
> (bfloat16x8x4_t): New typedef.
> (vld2_bf16): New.
> (vld2q_bf16): New.
> (vld3_bf16): New.
> (vld3q_bf16): New.
> (vld4_bf16): New.
> (vld4q_bf16): New.
> (vld2_dup_bf16): New.
> (vld2q_dup_bf16): New.
> (vld3_dup_bf16): New.
> (vld3q_dup_bf16): New.
> (vld4_dup_bf16): New.
> (vld4q_dup_bf16): New.
> * config/arm/arm-builtins.c (E_V2BFmode): New mode.
> (VAR13): New.
> (arm_simd_types[Bfloat16x2_t]):New type.
> * config/arm/arm-modes.def (V2BF): New mode.
> * config/arm/arm-simd-builtin-types.def
> (Bfloat16x2_t): New entry.
> * config/arm/arm_neon_builtins.def
> (vld2): Changed to VAR13 and added v4bf, v8bf
> (vld2_dup): Changed to VAR8 and added v4bf, v8bf
> (vld3): Changed to VAR13 and added v4bf, v8bf
> (vld3_dup): Changed to VAR8 and added v4bf, v8bf
> (vld4): Changed to VAR13 and added v4bf, v8bf
> (vld4_dup): Changed to VAR8 and added v4bf, v8bf
> * config/arm/iterators.md (VDXBF): New iterator.
> (VQ2BF): New iterator.
> (V_elem): Added V4BF, V8BF.
> (V_sz_elem): Added V4BF, V8BF.
> (V_mode_nunits): Added V4BF, V8BF.
> (q): Added V4BF, V8BF.
> *config/arm/neon.md (vld2): Used new iterators.
> (vld2_dup): Used new iterators.
> (vld2_dupv8bf): New.
> (vst3): Used new iterators.
> (vst3qa): Used new iterators.
> (vst3qb): Used new iterators.
> (vld3_dup): Used new iterators.
> (vld3_dupv8bf): New.
> (vst4): Used new iterators.
> (vst4qa): Used new iterators.
> (vst4qb): Used new iterators.
> (vld4_dup): Used new iterators.
> (vld4_dupv8bf): New.
>
>
> gcc/testsuite/ChangeLog:
>
> 2019-11-14 Delia Burduv
>
> * gcc.target/arm/simd/bf16_vldn_1.c: New test.
[PATCH, v3] wwwdocs: e-mail subject lines for contributions
[updated based on v2 discussions] This patch proposes some new (additional) rules for email subject lines when contributing to GCC. The goal is to make sure that, as far as possible, the subject for a patch will form a good summary when the message is committed to the repository if applied with 'git am'. Where possible, I've tried to align these rules with those already in use for glibc, so that the differences are minimal and only where necessary. Some things that differ from existing practice (at least by some people) are: - Use ':' rather than '[]' - This is more git friendly and works with 'git am'. - Put bug numbers at the end of the line rather than the beginning. - The bug number is useful, but not as useful as the brief summary. Also, use the shortened form, as the topic part is more usefully conveyed in the proper topic field (see above). diff --git a/htdocs/contribute.html b/htdocs/contribute.html index 042ff069..558bae29 100644 --- a/htdocs/contribute.html +++ b/htdocs/contribute.html @@ -249,13 +249,143 @@ that ChangeLog entries may be included as part of the patch and diffs representing new files may be omitted, especially if large, since they can be accessed directly from the repository.) +E-mail subject lines + +Your contribution e-mail subject line will become the first line of +the commit message for your patch. + +A high-quality e-mail subject line for a contribution contains the +following elements: + + + A classifier + Component tags + An optional series identifier + A brief summary + An optional bug number + + +The entire line (excluding the classifier) should not exceed 75 +characters. + +Classifier + +The classifier identifies the type of contribution, for example a +patch, an RFC (request for comments) or a committed patch (where +approval is not necessary. The classifier should be written in upper +case and surrounded with square brackets. This is the only component +of the e-mail subject line that will not appear in the commit itself. +The classifier may optionally contain a version number (vN) and +a series marker (N/M). Examples are: + + + [PATCH] - a single patch + [PATCH v2] - the second version of a single patch + [PATCH 3/7] - the third patch in a series of seven +patches + [RFC] - a point of discussion, may contain a patch + [COMMITTED] - a patch that has already been committed. + + +Component tags + +A component tag is a short identifier that identifies the part of +the compiler being modified. This highlights to the relevant +maintainers that the patch may need their attention. Multiple +components may be listed if necessary. Each component tag should be +followed by a colon. For example, + + + libstdc++: + combine: + vax: testsuite: + + +Some large components may be subdivided into sub-components. If +the subcomponent name is not disctinct in its own right, you can use the +form component/sub-component:. + +Series identifier + +The series identifier is optional and is only relevant if a number of +patches are needed in order to effect an overall change. It should be +a short string that identifies the series (it is common to all +patches) and should be followed by a single dash surrounded by white +space. + +A Very Brief summary + +The brief summary encapsulates in a few words the intent of the +change. For example: cleanup check_field_decls. +Although, very short, the summary should be distinct so that it will +not be confused with other patches. + +Bug number + +If your patch relates a bug in the compiler for which there is an +existing PR number the bug number should be stated. Use the +short-form variant [PRn] without the bugzilla +component identifier and with no space between 'PR' and the number. +The body of the commit message should still contain the full form +(PR/n) within the body of +the commit message so that Bugzilla will correctly notice the +commit. If your patch relates to two bugs, then write +[PRn, PRm]. For multiple +bugs, just cite the most relevant one in the summary and use an +elipsis instead of the second, or subsequent PR numbers; list all the +related PRs in the body of the commit message in the normal way. + +It is not necessary to cite bugs that are closed as duplicates of +another unless there is something specific to that report that +is not covered by the parent bug. + +Other messages + +Some large patch sets benefit from an introductory e-mail that +provides more context for the patch series and describes how the +patches have been broken up to provide for review. The convention is +that such messages should follow the same format as described above, +but the patch number should be set to zero, for example: [PATCH +0/7]. Remember that the introductory message will not be +committed with the patches themselves, so it should not contain any +important information that is not also covered in the individual +patches. If you send a summary e-mail with a series
Re: [PATCH] Fix comparison of trees via tree_cmp
On Wed, Jan 22, 2020 at 08:08:32PM +0300, Alexander Monakov wrote: > > > On Wed, 22 Jan 2020, Stefan Schulze Frielinghaus wrote: > > > Hi David, > > > > In function `tree_cmp` an invariant [1] is assumed which does not > > necessarily > > exist. In case both input trees are finally compared via `strcmp`, then > > > > tree_cmp (t1, t2) == -tree_cmp (t2, t1) > > > > does not hold in general, since function `strcmp (x, y)` guarantees only > > that a > > negative integer is returned in case xy. > > Currently this breaks s390x where, for example, for certain inputs x,y > > `tree_cmp (x, y) == 1` and `tree_cmp (y, x) == -2` hold. The attached patch > > normalizes the output from `strcmp` to -1, 0, or 1 while using an auxiliary > > function `sign` (stolen from the Hacker's Delight book ;-)). > > > > Bootstrapped and tested on s390x. Any thoughts? > > It's more appropriate to fix the assert rather than the comparator, like > > gcc_assert (sign (reversed) == -sign (result)); > > But qsort_chk already checks that, and more, so why is the assert there? > Shouldn't it be simply removed? Yeah. Note there is also return DECL_UID (t1) - DECL_UID (t2); that also doesn't guarantee -1/0/1 return values, so the patch as posted isn't sufficient. Jakub
Re: [RFC c-common PATCH] PR c++/40752 - useless -Wconversion with short +=.
On 1/22/20 5:14 AM, Martin Liška wrote: Hi. Just for the record, after the change 526.blender_r fails due to: blender/source/blender/blenlib/intern/math_color.c: In function 'rgb_float_to_uchar': blender/source/blender/blenlib/BLI_utildefines.h:251:22: error: conversion from 'float' to 'unsigned char' may change value [-Werror=float-conversion] 251 | #define FTOCHAR(val) ((CHECK_TYPE_INLINE(val, float)), \ | ^ blender/source/blender/blenlib/BLI_utildefines.h:257:13: note: in expansion of macro 'FTOCHAR' 257 | (v1)[0] = FTOCHAR((v2[0])); \ | ^~~ blender/source/blender/blenlib/intern/math_color.c:421:2: note: in expansion of macro 'F3TOCHAR3' 421 | F3TOCHAR3(col_f, r_col); | ^ where the project sets -Werror=float-conversion dues to: $ cat blender/source/blender/blenlib/BLI_strict_flags.h #ifdef __GNUC__ # if (__GNUC__ * 100 + __GNUC_MINOR__) >= 406 /* gcc4.6+ only */ # pragma GCC diagnostic error "-Wsign-compare" # pragma GCC diagnostic error "-Wconversion" # endif Thanks, investigating. Jason
libgo patch committed: Update runtime_nanotime call
In the update of libgo to Go1.14beta1, the function runtime_nanotime was renamed to runtime_nanotime1. I missed one reference that is not compiled on x86 code. This patch fixes that. This fixes https://golang.org/issue/36694. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index dff5fb5bc70..61f01d739ff 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -c2225a76d1e15f28056596807ebbbc526d4c58da +94a5ff53df24c58c5e6629ce6720a02aa9986322 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. diff --git a/libgo/runtime/runtime_c.c b/libgo/runtime/runtime_c.c index e7951a051a6..18222c14465 100644 --- a/libgo/runtime/runtime_c.c +++ b/libgo/runtime/runtime_c.c @@ -97,7 +97,7 @@ runtime_cputicks(void) // Currently cputicks() is used in blocking profiler and to seed runtime·fastrand(). // runtime·nanotime() is a poor approximation of CPU ticks that is enough for the profiler. // randomNumber provides better seeding of fastrand. - return runtime_nanotime() + randomNumber; + return runtime_nanotime1() + randomNumber; #endif }
Re: [PATCH] analyzer: introduce namespace to avoid ODR clashes (PR 93307)
On Fri, 2020-01-17 at 16:54 -0500, David Malcolm wrote: > PR analyzer/93307 reports that in an LTO bootstrap, there are ODR > violations between: > - the "region" type: > gcc/analyzer/region-model.h:792 > vs: > gcc/sched-int.h:1443 > - the "constraint" type: > gcc/analyzer/constraint-manager.h:121 > vs: > gcc/tree-ssa-structalias.c:533 > > This patches solves this clash by putting all of the analyzer names > within a namespace. I chose "ana" as it is short (to save typing). > The analyzer selftests are moved from namespace "selftest" to > "ana::selftest". > > There are various places where the namespace has to be closed > and reopened, to allow e.g. for specializations of templates > in the global namespace. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for master? I was able to successfully reproduce the problem (via --with-build-config=bootstrap-lto), and verify that the patch fixed it (with another bootstrap and regression test), so I've pushed this to master, as r10-6151-g75038aa6aa5b562e6358108619d66ef2ccab9a53. Dave
[committed] Backports to 9.x branch
Hi!
I've backported these 15 commits from trunk to 9.x branch,
bootstrapped/regtested them on x86_64-linux and i686-linux and
committed.
Jakub
PR c++/92992
* call.c (convert_arg_to_ellipsis): For decltype(nullptr) arguments
that have side-effects use cp_build_compound_expr.
* g++.dg/cpp0x/nullptr45.C: New test.
---
gcc/cp/ChangeLog | 9 +
gcc/cp/call.c | 7 ++-
gcc/testsuite/ChangeLog| 8
gcc/testsuite/g++.dg/cpp0x/nullptr45.C | 24
4 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/nullptr45.C
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 0e1557c936e..f0e314fd354 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,12 @@
+2020-01-22 Jakub Jelinek
+
+ Backported from mainline
+ 2019-12-20 Jakub Jelinek
+
+ PR c++/92992
+ * call.c (convert_arg_to_ellipsis): For decltype(nullptr) arguments
+ that have side-effects use cp_build_compound_expr.
+
2020-01-21 Jason Merrill
PR c++/91476 - anon-namespace reference temp clash between TUs.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8e14e89d5d4..787a7ed387b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7543,7 +7543,12 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t
complain)
arg = convert_to_real_nofold (double_type_node, arg);
}
else if (NULLPTR_TYPE_P (arg_type))
-arg = null_pointer_node;
+{
+ if (TREE_SIDE_EFFECTS (arg))
+ arg = cp_build_compound_expr (arg, null_pointer_node, complain);
+ else
+ arg = null_pointer_node;
+}
else if (INTEGRAL_OR_ENUMERATION_TYPE_P (arg_type))
{
if (SCOPED_ENUM_P (arg_type))
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d76c0598245..2e507fdfe50 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-01-22 Jakub Jelinek
+
+ Backported from mainline
+ 2019-12-20 Jakub Jelinek
+
+ PR c++/92992
+ * g++.dg/cpp0x/nullptr45.C: New test.
+
2020-01-22 Joseph Myers
Backport from mainline:
diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr45.C
b/gcc/testsuite/g++.dg/cpp0x/nullptr45.C
new file mode 100644
index 000..3ff226803df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr45.C
@@ -0,0 +1,24 @@
+// PR c++/92992
+// { dg-do run { target c++11 } }
+
+int a;
+
+void
+bar (int, ...)
+{
+}
+
+decltype (nullptr)
+baz ()
+{
+ a++;
+ return nullptr;
+}
+
+int
+main ()
+{
+ bar (0, baz ());
+ if (a != 1)
+__builtin_abort ();
+}
--
2.20.1
PR c++/92438
* parser.c (cp_parser_constructor_declarator_p): If open paren
is followed by RID_ATTRIBUTE, skip over the attribute tokens and
try to parse type specifier.
* g++.dg/ext/attrib61.C: New test.
---
gcc/cp/ChangeLog| 10 ++
gcc/cp/parser.c | 17 -
gcc/testsuite/ChangeLog | 8
gcc/testsuite/g++.dg/ext/attrib61.C | 26 ++
4 files changed, 60 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/ext/attrib61.C
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index f0e314fd354..13601ce975e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,13 @@
+2020-01-22 Jakub Jelinek
+
+ Backported from mainline
+ 2019-12-26 Jakub Jelinek
+
+ PR c++/92438
+ * parser.c (cp_parser_constructor_declarator_p): If open paren
+ is followed by RID_ATTRIBUTE, skip over the attribute tokens and
+ try to parse type specifier.
+
2020-01-22 Jakub Jelinek
Backported from mainline
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4df5d5797dc..54b3522dc8e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27619,7 +27619,15 @@ cp_parser_constructor_declarator_p (cp_parser *parser,
cp_parser_flags flags,
/* A parameter declaration begins with a decl-specifier,
which is either the "attribute" keyword, a storage class
specifier, or (usually) a type-specifier. */
- && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
+ && (!cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
+ /* GNU attributes can actually appear both at the start of
+a parameter and parenthesized declarator.
+S (__attribute__((unused)) int);
+is a constructor, but
+S (__attribute__((unused)) foo) (int);
+is a function declaration. */
+ || (cp_parser_allow_gnu_extensions_p (parser)
+ && cp_next_tokens_can_be_gnu_attribute_p (parser)))
/* A parameter declaration can also begin with [[attribute]]. */
&& !cp_next_tokens_can_be_std_attribute_p (parser))
[committed] analyzer: testsuite fixes for alloca, getpass, and setjmp (PR 93316)
PR analyzer/93316 reports various testsuite failures where I
accidentally relied on properties of x86_64-pc-linux-gnu.
The following patch fixes them on sparc-sun-solaris2.11 (gcc211 in the
GCC compile farm), and, I hope, the other configurations showing
failures.
There may still be other failures for pattern-test-2.c, which I'm
tracking separately as PR analyzer/93291.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
tested on stage 1 on sparc-sun-solaris2.11.
gcc/analyzer/ChangeLog:
PR analyzer/93316
* analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as
"_setjmp".
gcc/testsuite/ChangeLog:
PR analyzer/93316
* gcc.dg/analyzer/data-model-1.c: Include .
* gcc.dg/analyzer/malloc-1.c: Likewise.
* gcc.dg/analyzer/malloc-callbacks.c (get_alloca): Return
__builtin_alloca rather than alloca.
* gcc.dg/analyzer/malloc-paths-8.c: Include .
* gcc.dg/analyzer/sensitive-1.c: Define __EXTENSIONS__ before
including unistd.h.
* gcc.dg/analyzer/setjmp-2.c: Replace include of
with "test-setjmp.h" and usage of setjmp with new SETJMP macro.
* gcc.dg/analyzer/setjmp-3.c: Likewise.
* gcc.dg/analyzer/setjmp-4.c: Likewise.
* gcc.dg/analyzer/setjmp-5.c: Likewise.
* gcc.dg/analyzer/setjmp-6.c: Likewise.
* gcc.dg/analyzer/setjmp-7.c: Likewise.
* gcc.dg/analyzer/setjmp-7a.c: Likewise.
* gcc.dg/analyzer/setjmp-8.c: Likewise.
* gcc.dg/analyzer/setjmp-9.c: Likewise.
* gcc.dg/analyzer/test-setjmp.h: New header.
---
gcc/analyzer/analyzer.cc | 3 ++-
gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 1 +
gcc/testsuite/gcc.dg/analyzer/malloc-1.c | 1 +
gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c | 4 +++-
gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c | 1 +
gcc/testsuite/gcc.dg/analyzer/sensitive-1.c | 4
gcc/testsuite/gcc.dg/analyzer/setjmp-2.c | 10 +-
gcc/testsuite/gcc.dg/analyzer/setjmp-3.c | 8
gcc/testsuite/gcc.dg/analyzer/setjmp-4.c | 12 ++--
gcc/testsuite/gcc.dg/analyzer/setjmp-5.c | 6 +++---
gcc/testsuite/gcc.dg/analyzer/setjmp-6.c | 4 ++--
gcc/testsuite/gcc.dg/analyzer/setjmp-7.c | 4 ++--
gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c| 8
gcc/testsuite/gcc.dg/analyzer/setjmp-8.c | 8
gcc/testsuite/gcc.dg/analyzer/setjmp-9.c | 8
gcc/testsuite/gcc.dg/analyzer/test-setjmp.h | 16
16 files changed, 62 insertions(+), 36 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/test-setjmp.h
diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc
index 2a3ffaee852..3884788ee9e 100644
--- a/gcc/analyzer/analyzer.cc
+++ b/gcc/analyzer/analyzer.cc
@@ -91,7 +91,8 @@ is_setjmp_call_p (const gimple *stmt)
{
/* TODO: is there a less hacky way to check for "setjmp"? */
if (const gcall *call = dyn_cast (stmt))
-if (is_special_named_call_p (call, "_setjmp", 1))
+if (is_special_named_call_p (call, "setjmp", 1)
+ || is_special_named_call_p (call, "_setjmp", 1))
return true;
return false;
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index 3a0108d0b8c..91685f578a4 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,6 +1,7 @@
#include
#include
#include
+#include
#include "analyzer-decls.h"
struct foo
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index 6e9a014272e..e2e279bd7fd 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,3 +1,4 @@
+#include
#include
#include
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
b/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
index eb5545e5da0..901ca5c46fd 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
@@ -12,7 +12,9 @@ get_malloc (void)
static allocator_t __attribute__((noinline))
get_alloca (void)
{
- return alloca;
+ /* On e.g. Solaris, alloca is a macro so we can't take its address;
+ use __builtin_alloca instead. */
+ return __builtin_alloca;
}
static deallocator_t __attribute__((noinline))
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
index bf858e04840..10b97a05402 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
@@ -1,6 +1,7 @@
/* { dg-additional-options "-fanalyzer-transitivity" } */
#include
+#include
#include
extern void do_stuff (const void *);
diff --git a/gcc/testsuite/gcc.dg/analyzer/sensitive-1.c
b/gcc/testsuite/gcc.dg/analyzer/sensitive-1.c
index 8c6b6074784..8114
Re: [PATCH] cprop: Don't replace fixed hard regs with pseudos [PR93124]
On Wed, 2020-01-22 at 12:02 +, Richard Sandiford wrote: > One consequence of r276318 was that cselib now preserves sp-based > values across function calls. This in turn convinced cprop to > replace the clobber in: > >(set (reg PSUEDO) (reg sp)) >... >(call ...) >... >(clobber (mem:BLK (reg sp))) > > with: > >(clobber (mem:BLK (reg PSEUDO))) > > But I doubt this could ever be an optimisation, regardless of what the > changed instruction is. Extending the lifetimes of pseudos can lead to > extra spills, whereas sp is available everywhere. > > More generally, I don't think we should replace any fixed hard register > with a pseudo. Replacing them with a constant is still potentially > useful though, since we'll only make the change if the insn pattern > allows it. > > This part 1 of the fix for PR93124. Part 2 contains the testcase. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > > > 2020-01-22 Richard Sandiford > > gcc/ > PR rtl-optimization/93124 > * cprop.c (cprop_replace_with_reg_p): New function. > (cprop_insn, do_local_cprop): Use it. In theory there may be cases where replacing a fixed hard register with a pseudo in turn might allow a allocation of the pseudo to a different hard register which *could* have a different cost. But in a CLOBBER insn, none of that should matter. Would it make sense to only do this on CLOBBERS? I'm not rejecting. Mostly I'm worried about unintended consequences and wondering if we narrow the cases where we're changing behavior that unintended consequences are less likely to pop up. Jeff
Re: [committed] analyzer: testsuite fixes for alloca, getpass, and setjmp (PR 93316)
On Wed, Jan 22, 2020 at 02:35:13PM -0500, David Malcolm wrote: > PR analyzer/93316 reports various testsuite failures where I > accidentally relied on properties of x86_64-pc-linux-gnu. > > The following patch fixes them on sparc-sun-solaris2.11 (gcc211 in the > GCC compile farm), and, I hope, the other configurations showing > failures. > > There may still be other failures for pattern-test-2.c, which I'm > tracking separately as PR analyzer/93291. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; > tested on stage 1 on sparc-sun-solaris2.11. > > gcc/analyzer/ChangeLog: > PR analyzer/93316 > * analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as > "_setjmp". Please see calls.c (special_function_p), you should treat certainly also sigsetjmp as a setjmp call, and similarly to special_function_p, skip over _ or __ prefixes before the setjmp or sigsetjmp name. Similarly for longjmp/siglongjmp. Jakub
[committed] analyzer: fix setjmp handling with -g (PR 93378)
PR analyzer/93378 reports an ICE at -O1 -g when analyzing a rewind via
longjmp to a setjmp call with.
The root cause is that the rewind_info_t::get_setjmp_call attempts to
locate the setjmp GIMPLE_CALL via within the exploded_node containing
it, but the exploded_node has two stmts: a GIMPLE_DEBUG, then the
GIMPLE_CALL, and so erroneously picks the GIMPLE_DEBUG, leading to
a failed as_a .
This patch reworks how the analyzer stores information about a setjmp
so that instead of storing an exploded_node *, it instead introduces
a "setjmp_record" struct, for use by both setjmp_svalue and
rewind_info_t. Hence we store the information directly, rather than
attempting to reconstruct it, fixing the bug.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
pushed to master as r10-6153-gfd9982bb0051d1a678191b684bb907d1ac177991.
gcc/analyzer/ChangeLog:
PR analyzer/93378
* engine.cc (setjmp_svalue::compare_fields): Update for
replacement of m_enode with m_setjmp_record.
(setjmp_svalue::add_to_hash): Likewise.
(setjmp_svalue::get_index): Rename...
(setjmp_svalue::get_enode_index): ...to this.
(setjmp_svalue::print_details): Update for replacement of m_enode
with m_setjmp_record.
(exploded_node::on_longjmp): Likewise.
* exploded-graph.h (rewind_info_t::m_enode_origin): Replace...
(rewind_info_t::m_setjmp_record): ...with this.
(rewind_info_t::rewind_info_t): Update for replacement of m_enode
with m_setjmp_record.
(rewind_info_t::get_setjmp_point): Likewise.
(rewind_info_t::get_setjmp_call): Likewise.
* region-model.cc (region_model::dump_summary_of_map): Likewise.
(region_model::on_setjmp): Likewise.
* region-model.h (struct setjmp_record): New struct.
(setjmp_svalue::m_enode): Replace...
(setjmp_svalue::m_setjmp_record): ...with this.
(setjmp_svalue::setjmp_svalue): Update for replacement of m_enode
with m_setjmp_record.
(setjmp_svalue::clone): Likewise.
(setjmp_svalue::get_index): Rename...
(setjmp_svalue::get_enode_index): ...to this.
(setjmp_svalue::get_exploded_node): Replace...
(setjmp_svalue::get_setjmp_record): ...with this.
gcc/testsuite/ChangeLog:
PR analyzer/93378
* gcc.dg/analyzer/setjmp-pr93378.c: New test.
---
gcc/analyzer/engine.cc| 18 +-
gcc/analyzer/exploded-graph.h | 15
gcc/analyzer/region-model.cc | 7 ++--
gcc/analyzer/region-model.h | 34 +++
.../gcc.dg/analyzer/setjmp-pr93378.c | 15
5 files changed, 65 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 53c93791a07..737ea1dd6e4 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -155,7 +155,7 @@ impl_region_model_context::on_unknown_change (svalue_id sid)
bool
setjmp_svalue::compare_fields (const setjmp_svalue &other) const
{
- return m_enode == other.m_enode;
+ return m_setjmp_record == other.m_setjmp_record;
}
/* Implementation of svalue::add_to_hash vfunc for setjmp_svalue. */
@@ -163,15 +163,15 @@ setjmp_svalue::compare_fields (const setjmp_svalue
&other) const
void
setjmp_svalue::add_to_hash (inchash::hash &hstate) const
{
- hstate.add_int (m_enode->m_index);
+ hstate.add_int (m_setjmp_record.m_enode->m_index);
}
/* Get the index of the stored exploded_node. */
int
-setjmp_svalue::get_index () const
+setjmp_svalue::get_enode_index () const
{
- return m_enode->m_index;
+ return m_setjmp_record.m_enode->m_index;
}
/* Implementation of svalue::print_details vfunc for setjmp_svalue. */
@@ -181,7 +181,7 @@ setjmp_svalue::print_details (const region_model &model
ATTRIBUTE_UNUSED,
svalue_id this_sid ATTRIBUTE_UNUSED,
pretty_printer *pp) const
{
- pp_printf (pp, "setjmp: EN: %i", m_enode->m_index);
+ pp_printf (pp, "setjmp: EN: %i", get_enode_index ());
}
/* Concrete implementation of sm_context, wiring it up to the rest of this
@@ -1172,11 +1172,11 @@ exploded_node::on_longjmp (exploded_graph &eg,
if (!setjmp_sval)
return;
+ const setjmp_record tmp_setjmp_record = setjmp_sval->get_setjmp_record ();
+
/* Build a custom enode and eedge for rewinding from the longjmp
call back to the setjmp. */
-
- const exploded_node *enode_origin = setjmp_sval->get_exploded_node ();
- rewind_info_t rewind_info (enode_origin);
+ rewind_info_t rewind_info (tmp_setjmp_record);
const gcall *setjmp_call = rewind_info.get_setjmp_call ();
const program_point &setjmp_point = rewind_info.get_setjmp_point ();
@@ -1217,7 +1217,7 @@ exploded_node::on_longjmp (exploded_graph &eg,
exploded_edge *eedge
= eg.add_edge (const_cast (this), next
Re: [PATCH] auto-inc-dec: Don't add incs/decs to bare CLOBBERs [PR93124]
On Wed, 2020-01-22 at 12:11 +, Richard Sandiford wrote: > In this PR, auto-inc-dec was trying to turn: > > (set (reg X) (plus (reg X) (const_int N))) > (clobber (mem (reg X))) > > into: > > (clobber (mem (pre_modify (reg X) ...))) > > But bare clobber insns are just there to describe dataflow. They're > not supposed to generate any code. > > This is part 2 of the fix for PR93124. The reason we have the > clobber above is that cprop replaced (reg sp) with (reg X) in: > > (clobber (mem (reg sp))) > > Part 1 stops that from happening, but I still think we should > prevent auto-inc-dec from "optimising" bare USEs and CLOBBERs > as a belt-and-braces fix. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > > > 2020-01-22 Richard Sandiford > > gcc/ > PR rtl-optimization/93124 > * auto-inc-dec.c (merge_in_block): Don't add auto inc/decs to > bare USE and CLOBBER insns. > > gcc/testsuite/ > * gcc.dg/torture/pr93124.c: New test. OK. Jeff >
Re: [PATCH 4/4] [ARC] Update ARC600 multiplication cost.
On Wed, 2020-01-22 at 10:14 +0200, Claudiu Zissulescu wrote: > The ARC's 600 multiplication instruction can accept signed 12 bit > instructions. > > gcc/ > -xx-xx Claudiu Zissulescu > > * config/arc/arc.c (arc_rtx_costs): Update mul64 cost. OK jeff >
Re: libgo patch committed: Update to Go1.14beta1
Hi Ian,
> I've committed a patch to update libgo to Go 1.14beta1. As usual with
> these updates the patch is far too large to include in this e-mail
> message. I've included the diffs for gccgo-specific files.
> Bootstrapped and ran Go tests on x86_64-pc-linux-gnu. Committed to
> mainline.
the patch broke Solaris bootstrap:
/vol/gcc/src/hg/master/local/libgo/go/runtime/os_only_solaris.go:11:1: error:
redefinition of 'getncpu'
11 | func getncpu() int32 {
| ^
/vol/gcc/src/hg/master/local/libgo/go/runtime/os3_solaris.go:20:1: note:
previous definition of 'getncpu' was here
20 | func getncpu() int32 {
| ^
There are 3 definitions in the Solaris/Illumos space:
* os_only_solaris.go is guarded by !illumos
* os3_solaris.go has no explicit guard
* illumos hat its own one in os_illumos.go
so the os3_solaris.go one can go.
/vol/gcc/src/hg/master/local/libgo/go/runtime/stubs2.go:40:3: error: osinit is
not defined
40 | //go:linkname osinit runtime.osinit
| ^
Upstream has a definition in os3_solaris.go.
The following patch allows compilation to succeed.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
diff --git a/libgo/go/runtime/os3_solaris.go b/libgo/go/runtime/os3_solaris.go
--- a/libgo/go/runtime/os3_solaris.go
+++ b/libgo/go/runtime/os3_solaris.go
@@ -17,12 +17,11 @@ func getPageSize() int32
//extern sysconf
func sysconf(int32) _C_long
-func getncpu() int32 {
- n := int32(sysconf(__SC_NPROCESSORS_ONLN))
- if n < 1 {
- return 1
+func osinit() {
+ ncpu = getncpu()
+ if physPageSize == 0 {
+ physPageSize = uintptr(getPageSize())
}
- return n
}
func sysargs(argc int32, argv **byte) {
Re: [PATCH 3/4] [ARC] Save mlo/mhi registers when ISR.
On Wed, 2020-01-22 at 10:14 +0200, Claudiu Zissulescu wrote: > ARC600 when configured with mul64 instructions uses mlo and mhi > registers to store the 64 result of the multiplication. In the ARC600 > ISA documentation we have the next register configuration when ARC600 > is configured only with mul64 extension: > > Register | Name | Use > -+--+ > r57 | mlo | Multiply low 32 bits, read only > r58 | mmid | Multiply middle 32 bits, read only > r59 | mhi | Multiply high 32 bits, read only > - > > When used for Co-existence configurations we have for mul64 the next > registers used: > > Register | Name | Use > -+--+ > r58 | mlo | Multiply low 32 bits, read only > r59 | mhi | Multiply high 32 bits, read only > - > > Note that mlo/mhi assignment doesn't swap when bigendian CPU > configuration is used. > > The compiler will always use r58 for mlo, regardless of the > configuration choosen to ensure mlo/mhi correct splitting. Fixing mlo > to the right register number is done at assembly time. The dwarf info > is also notified via DBX_... macro. Both mlo/mhi registers needs to > saved when ISR happens using a custom sequence. > > gcc/ > -xx-xx Claudiu Zissulescu > > * config/arc/arc-protos.h (gen_mlo): Remove. > (gen_mhi): Likewise. > * config/arc/arc.c (AUX_MULHI): Define. > (arc_must_save_reister): Special handling for r58/59. > (arc_compute_frame_size): Consider mlo/mhi registers. > (arc_save_callee_saves): Emit fp/sp move only when emit_move > paramter is true. > (arc_conditional_register_usage): Remove TARGET_BIG_ENDIAN from > mlo/mhi name selection. > (arc_restore_callee_saves): Don't early restore blink when ISR. > (arc_expand_prologue): Add mlo/mhi saving. > (arc_expand_epilogue): Add mlo/mhi restoring. > (gen_mlo): Remove. > (gen_mhi): Remove. > * config/arc/arc.h (DBX_REGISTER_NUMBER): Correct register > numbering when MUL64 option is used. > (DWARF2_FRAME_REG_OUT): Define. > * config/arc/arc.md (arc600_stall): New pattern. > (VUNSPEC_ARC_ARC600_STALL): Define. > (mulsi64): Use correct mlo/mhi registers. > (mulsi_600): Clean it up. > * config/arc/predicates.md (mlo_operand): Remove any dependency on > TARGET_BIG_ENDIAN. > (mhi_operand): Likewise. > > testsuite/ > -xx-xx Claudiu Zissulescu > * gcc.target/arc/code-density-flag.c: Update test. > * gcc.target/arc/interrupt-6.c: Likewise. Ugh. But OK. jeff >
Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Hi,
On 22/01/20 17:27, Jason Merrill wrote:
On 1/22/20 10:22 AM, Paolo Carlini wrote:
Hi,
in this simple issue we either wrongly talked about variable
template-id in c++17 mode or ICEd in c++2a. I think we simply want to
handle concept-ids first, both as represented in c++17 mode and as
represented in c++2a mode. Tested x86_64-linux.
What happens if you try to use a function template/function concept name?
AFAICS no ICEs, no regressions but indeed we can do better, tell
concepts from function templates. The below does that and passes testing
but I'm not sure if the wording is optimal, whether we always want to
talk about concept-id.
Thanks! Paolo.
///
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
tree fns = get_fns (tid);
if (OVL_SINGLE_P (fns))
tmpl = OVL_FIRST (fns);
- error_at (token->location, "function template-id %qD "
- "in nested-name-specifier", tid);
+ if (function_concept_p (fns))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ error_at (token->location, "function template-id "
+ "%qD in nested-name-specifier", tid);
}
else
{
- /* Variable template. */
tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
- "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ {
+ /* Variable template. */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+ "%qD in nested-name-specifier", tid);
+ }
}
if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C
b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target c++17 } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C
b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 000..98ec4ae598e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }
+
+template
+concept bool foo() { return true; }; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target *-*-* } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
Re: [PATCH] avoid -Wrestrict on sprintf %p with destination as argument (PR 84919)
On Wed, 2020-01-22 at 15:59 +0100, Martin Sebor wrote: > The early front-end only implementation of -Wrestrict that's still > present in GCC 10 issues a false postive for %p arguments that are > the same as the destination. Bug 84919 reports an instance of this > false positive in the Linux kernel. > > That attached patch suppresses the front-end warning for the sprintf > family of functions, letting the sprintf pass that was in GCC 10 > extended to also handle -Wrestrict for these functions, handle > them instead. > > Tested on x86_64-linux. > > Since this is a regression I'd like to commit the fix to GCC 10. In the BZ your comment indicates that it trips another bogus warning during bootstrap. Presumably you already fixed that? If so, OK. jeff
[committed] analyzer: fix usage of "_setjmp" in test for PR 93378
On Wed, 2020-01-22 at 14:56 -0500, David Malcolm wrote:
> PR analyzer/93378 reports an ICE at -O1 -g when analyzing a rewind
> via
> longjmp to a setjmp call with.
>
> The root cause is that the rewind_info_t::get_setjmp_call attempts to
> locate the setjmp GIMPLE_CALL via within the exploded_node containing
> it, but the exploded_node has two stmts: a GIMPLE_DEBUG, then the
> GIMPLE_CALL, and so erroneously picks the GIMPLE_DEBUG, leading to
> a failed as_a .
>
> This patch reworks how the analyzer stores information about a setjmp
> so that instead of storing an exploded_node *, it instead introduces
> a "setjmp_record" struct, for use by both setjmp_svalue and
> rewind_info_t. Hence we store the information directly, rather than
> attempting to reconstruct it, fixing the bug.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> pushed to master as r10-6153-
> gfd9982bb0051d1a678191b684bb907d1ac177991.
[...]
> diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
> b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
> new file mode 100644
> index 000..7934a40301d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
> @@ -0,0 +1,15 @@
> +/* { dg-additional-options "-O1 -g" } */
> +
> +#include
> +
> +jmp_buf buf;
> +
> +int
> +test (void)
> +{
> + if (_setjmp (buf) != 0)
> +return 0;
> +
> + longjmp (buf, 1);
> + return 1;
> +}
I realized belatedly that the above introduces an assumption that
declares a _setjmp, so I've pushed the following folloup,
having tested on x86_64-pc-linux-gnu and x86_64-pc-linux-gnu.
gcc/testsuite/ChangeLog:
PR analyzer/93378
* gcc.dg/analyzer/setjmp-pr93378.c: Use setjmp rather than
_setjmp.
---
gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
index 7934a40301d..73b94d4d36b 100644
--- a/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
+++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c
@@ -7,7 +7,7 @@ jmp_buf buf;
int
test (void)
{
- if (_setjmp (buf) != 0)
+ if (setjmp (buf) != 0)
return 0;
longjmp (buf, 1);
--
2.21.0
Re: [PATCH] Allow tree-ssa.exp to be run by itself
On Tue, 2020-01-21 at 17:56 -0800, apin...@marvell.com wrote: > From: Andrew Pinski > > tree-ssa testcases sometimes check autovect effective target > but does not set it up. On MIPS, those testcases fail with > some TCL error messages. This fixes the issue by calling > check_vect_support_and_set_flags inside tree-ssa.exp. > There might be other .exp files which need to be done this > way too but I have not checked all of them. > > OK? Tested on x86_64-linux-gnu and a cross to mips64-octeon-linux-gnu. > Both full run of the testsuite and running tree-ssa.exp by itself. > > Thanks, > Andrew Pinski > > testsuite/ChangeLog: > * tree-ssa.exp: Set DEFAULT_VECTCFLAGS and DEFAULT_VECTCFLAGS. > Call check_vect_support_and_set_flags also. OK jeff >
Re: [PATCH] testsuite: Add target/xfail argument to check-function-bodies
On Tue, 2020-01-21 at 14:59 +, Richard Sandiford wrote: > check-function-bodies allows individual function tests to be > annotated with target/xfail selectors, but sometimes it's > useful to have the same selector for all functions. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu, and with a following > ILP32 patch that needs it. OK to install? > > Richard > > > 2020-01-21 Richard Sandiford > > gcc/ > * doc/sourcebuild.texi (check-function-bodies): Add an > optional target/xfail selector. > > gcc/testsuite/ > * lib/scanasm.exp (check-function-bodies): Add an optional > target/xfail selector. OK jeff >
Re: [PATCH] Fix PR 93242: patchable-function-entry broken on MIPS
On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > On Sat, Jan 18, 2020 at 1:47 AM wrote: > > From: Andrew Pinski > > > > On MIPS, .set noreorder/reorder needs to emitted around > > the nop. The template for the nop instruction uses %(/%) to > > do that. But default_print_patchable_function_entry uses > > fprintf rather than output_asm_insn to output the instruction. > > > > This fixes the problem by using output_asm_insn to emit the nop > > instruction. > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > mips toolchain also. > > OK. FWIW, I think this may have broke the arc-elf port. I'm getting failures for the patchable function entry tests. It looks like the port wants to peek a the current_output_insn in its handling of an output punctuation characters and current_output_insn is NULL. jeff >
Re: [PATCH] gcc: Add new configure options to allow static libraries to be selected
On Wed, 2020-01-22 at 15:39 +, Andrew Burgess wrote: > The motivation behind this change is to make it easier for a user to > link against static libraries on a target where dynamic libraries are > the default library type (for example GNU/Linux). > > Further, my motivation is really for linking libraries into GDB, > however, the binutils-gdb/config/ directory is a copy of gcc/config/ > so changes for GDB need to be approved by the GCC project first. > > After making this change in the gcc/config/ directory I've run > autoreconf on all of the configure scripts in the GCC tree and a > couple have been updated, so I'll use one of these to describe what my > change does. > > Consider libcpp, this library links against libiconv. Currently if > the user builds on a system with both static and dynamic libiconv > installed then autotools will pick up the dynamic libiconv by > default. This is almost certainly the right thing to do. > > However, if the user wants to link against static libiconv then things > are a little harder, they could remove the dynamic libiconv from their > system, but this is probably a bad idea (other things might depend on > that library), or the user can build their own version of libiconv, > install it into a unique prefix, and then configure gcc using the > --with-libiconv-prefix=DIR flag. This works fine, but is somewhat > annoying, the static library available, I just can't get autotools to > use it. > > My change then adds a new flag --with-libiconv-type=TYPE, where type > is either auto, static, or shared. The default auto, ensures we keep > the existing behaviour unchanged. > > If the user configures with --with-libiconv-type=static then the > configure script will ignore any dynamic libiconv it finds, and will > only look for a static libiconv, if no static libiconv is found then > the configure will continue as though there is no libiconv at all > available. > > Similarly a user can specify --with-libiconv-type=shared and force the > use of shared libiconv, any static libiconv will be ignored. > > As I've implemented this change within the AC_LIB_LINKFLAGS_BODY macro > then only libraries configured using the AC_LIB_LINKFLAGS or > AC_LIB_HAVE_LINKFLAGS macros will gain the new configure flag. > > If this is accepted into GCC then there will be follow on patches for > binutils and GDB to regenerate some configure scripts in those > projects. > > For GCC only two configure scripts needed updated after this commit, > libcpp and libstdc++-v3, both of which link against libiconv. > > config/ChangeLog: > > * lib-link.m4 (AC_LIB_LINKFLAGS_BODY): Add new > --with-libXXX-type=... option. Use this to guide the selection of > either a shared library or a static library. > > libcpp/ChangeLog: > > * configure: Regnerate. > > libstdc++-v3/ChangeLog: > > * configure: Regnerate. s/Regnerate/Regenerate/ This isn't strictly a regression bugfix. But given the nature of these files I think we probably need to be a bit more lax and allow safe changes so that downstream uses can move forward independent of the gcc development and release schedule. So, OK. jeff >
Re: [PATCH 1/4] [ARC] Make libgcc compatible with ARC's reduced register set config.
On Wed, 2020-01-22 at 10:14 +0200, Claudiu Zissulescu wrote: > ARC processors can work with a reduced register set (i.e. registers > r4-r9 and r16-r25 are not available). This option can be enabled > passing -mrf16 option to the compiler, or by using -mcpu=em_mini CPU > configuration. Using RF16 config requires all the hand-made assembly > files used in libgcc to have the corresponding RF16 object attribute > set. > > This patch qualifies the relevant hand-made assembly files to > RF16 config, and also adds generic c-functions for the one which are > not. > > libgcc/ > -xx-xx Claudiu Zissulescu > > * config/arc/crti.S: Add RF16 object attribute. > * config/arc/crtn.S: Likewise. > * config/arc/crttls.S: Likewise. > * config/arc/lib1funcs.S: Likewise. > * config/arc/fp-hack.h (ARC_OPTFPE): Define. > * config/arc/lib2funcs.c: New file. > * config/arc/t-arc: Add lib2funcs to LIB2ADD. OK jeff >
Re: [PATCH] avoid -Wrestrict on sprintf %p with destination as argument (PR 84919)
On 1/22/20 9:35 PM, Jeff Law wrote: On Wed, 2020-01-22 at 15:59 +0100, Martin Sebor wrote: The early front-end only implementation of -Wrestrict that's still present in GCC 10 issues a false postive for %p arguments that are the same as the destination. Bug 84919 reports an instance of this false positive in the Linux kernel. That attached patch suppresses the front-end warning for the sprintf family of functions, letting the sprintf pass that was in GCC 10 extended to also handle -Wrestrict for these functions, handle them instead. Tested on x86_64-linux. Since this is a regression I'd like to commit the fix to GCC 10. In the BZ your comment indicates that it trips another bogus warning during bootstrap. Presumably you already fixed that? If so, OK. That was PR 92666, an unrelated C++ bug fixed recently by Jakub. This time, my bootstrap of this patch went fine. Martin
Re: [PATCH] Fix PR 93242: patchable-function-entry broken on MIPS
On Wed, Jan 22, 2020 at 12:48 PM Jeff Law wrote: > > On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > > On Sat, Jan 18, 2020 at 1:47 AM wrote: > > > From: Andrew Pinski > > > > > > On MIPS, .set noreorder/reorder needs to emitted around > > > the nop. The template for the nop instruction uses %(/%) to > > > do that. But default_print_patchable_function_entry uses > > > fprintf rather than output_asm_insn to output the instruction. > > > > > > This fixes the problem by using output_asm_insn to emit the nop > > > instruction. > > > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > > mips toolchain also. > > > > OK. > FWIW, I think this may have broke the arc-elf port. I'm getting > failures for the patchable function entry tests. It looks like the > port wants to peek a the current_output_insn in its handling of an > output punctuation characters and current_output_insn is NULL. I suspect arc-elf was failing beforehand; just not crashing the compiler :). Before this patch we would be printing out "nop%?" for arc-elf. The tests are "compile" so they would have "passed" but only because the tests was not trying to assemble them. If someone had tried to use this option of arc-elf, they would have ran into a similar problem as mips, printing out %? (in arc case). Thanks, Andrew Pinski > > jeff > > >
[PATCH] libsanitizer: Add missign file and regen Makefile.in
Hi all, I'm digginig out old patches and I want to complete the libasan support for FreeBSD x86_64. The below one was not that obvious when you have been away for the past years. In the last import the sanitizer_platform_limits_freebsd.cpp got forgotten. Fix this. Ok for trunk once it's open again? Thanks, Andreas libsanitizer/sanitizer_common: * Makefile.am: Add sanitizer_platform_limits_freebsd.cpp. * makefile.in: Regenerate diff --git a/libsanitizer/sanitizer_common/Makefile.am b/libsanitizer/sanitizer_ common/Makefile.am index df9c294151d..9653f27c09f 100644 --- a/libsanitizer/sanitizer_common/Makefile.am +++ b/libsanitizer/sanitizer_common/Makefile.am @@ -44,6 +44,7 @@ sanitizer_common_files = \ sanitizer_netbsd.cpp \ sanitizer_openbsd.cpp \ sanitizer_persistent_allocator.cpp \ + sanitizer_platform_limits_freebsd.cpp \ sanitizer_platform_limits_linux.cpp \ sanitizer_platform_limits_openbsd.cpp \ sanitizer_platform_limits_posix.cpp \
[PATCH]: gcc: Enable bits for sanitizer support on FreeBSD x86_64
Hi all,
this patch adds the necessary bits to enable asan support on FreeBSD x86_64.
Results will be produced over night.
Ok for trunk once it is open again?
TIA,
Andreas
gcc/
* config/i386/i386.h: Define a new macro: SUBTARGET_SHADOW_OFFSET.
* config/i386/i386.c (ix86_asan_shadow_offset): Use this macro.
* config/i386/darwin.h: Override the SUBTARGET_SHADOW_OFFSET macro.
* config/i386/freebsd.h: Likewise.
* config/freebsd.h (LIBASAN_EARLY_SPEC): Define.
LIBTSAN_EARLY_SPEC): Likewise. (LIBLSAN_EARLY_SPEC): Likewise.
libsanitizer:
* configure.tgt: Add x86_64- and i?86-*-freebsd* targets.
* asan/asan_interceptors.h: Define ASAN_INTERCEPT_SWAPCONTEXT for
FreeBSD.
From 93978ce66c4eeff0bde2f44f8c0809fc66165e5b Mon Sep 17 00:00:00 2001
From: Andreas Tobler
Date: Tue, 21 Jan 2020 22:17:09 +0100
diff --git a/gcc/config/freebsd.h b/gcc/config/freebsd.h
index d9d6be7a8c8..4b5140bae02 100644
--- a/gcc/config/freebsd.h
+++ b/gcc/config/freebsd.h
@@ -62,6 +62,27 @@ along with GCC; see the file COPYING3. If not see
#define USE_LD_AS_NEEDED 1
#endif
+/* Link -lasan early on the command line. For -static-libasan, don't link
+ it for -shared link, the executable should be compiled with -static-libasan
+ in that case, and for executable link with --{,no-}whole-archive around
+ it to force everything into the executable. And similarly for -ltsan
+ and -llsan. */
+#if defined(HAVE_LD_STATIC_DYNAMIC)
+#undef LIBASAN_EARLY_SPEC
+#define LIBASAN_EARLY_SPEC "%{!shared:libasan_preinit%O%s} " \
+ "%{static-libasan:%{!shared:" \
+ LD_STATIC_OPTION " --whole-archive -lasan --no-whole-archive " \
+ LD_DYNAMIC_OPTION "}}%{!static-libasan:-lasan -lpthread}"
+#undef LIBTSAN_EARLY_SPEC
+#define LIBTSAN_EARLY_SPEC "%{static-libtsan:%{!shared:" \
+ LD_STATIC_OPTION " --whole-archive -ltsan --no-whole-archive " \
+ LD_DYNAMIC_OPTION "}}%{!static-libtsan:-ltsan -lpthread}"
+#undef LIBLSAN_EARLY_SPEC
+#define LIBLSAN_EARLY_SPEC "%{static-liblsan:%{!shared:" \
+ LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
+ LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan -lpthread}"
+#endif
+
/[ Target stuff ]***/
/* All FreeBSD Architectures support the ELF object file format. */
diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index 1b94cb88c98..5b7b1e889a9 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -337,6 +337,12 @@ along with GCC; see the file COPYING3. If not see
#define SUBTARGET_INIT_BUILTINS
\
do { \
ix86_builtins[(int) IX86_BUILTIN_CFSTRING] \
+
+/* Define the shadow offset for asan. */
+#undef SUBTARGET_SHADOW_OFFSET
+#define SUBTARGET_SHADOW_OFFSET\
+ (TARGET_LP64 ? HOST_WIDE_INT_1 << 44 : HOST_WIDE_INT_1 << 29)
+
= darwin_init_cfstring_builtins ((unsigned) (IX86_BUILTIN_CFSTRING)); \
darwin_rename_builtins (); \
} while(0)
diff --git a/gcc/config/i386/freebsd.h b/gcc/config/i386/freebsd.h
index 8fb0f0aaf5c..9d66602142e 100644
--- a/gcc/config/i386/freebsd.h
+++ b/gcc/config/i386/freebsd.h
@@ -129,3 +129,7 @@ along with GCC; see the file COPYING3. If not see
#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+/* Define the shadow offsets for asan. */
+#undef SUBTARGET_SHADOW_OFFSET
+#define SUBTARGET_SHADOW_OFFSET\
+ (TARGET_LP64 ? HOST_WIDE_INT_1 << 46 : HOST_WIDE_INT_1 << 30)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffe60baa72a..71baca4e0c9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1396,9 +1396,7 @@ ix86_legitimate_combined_insn (rtx_insn *insn)
static unsigned HOST_WIDE_INT
ix86_asan_shadow_offset (void)
{
- return TARGET_LP64 ? (TARGET_MACHO ? (HOST_WIDE_INT_1 << 44)
-: HOST_WIDE_INT_C (0x7fff8000))
-: (HOST_WIDE_INT_1 << 29);
+ return SUBTARGET_SHADOW_OFFSET;
}
/* Argument support functions. */
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 943e9a5c783..18b27bb535b 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1321,6 +1321,13 @@ extern const char *host_detect_local_cpu (int argc,
const char **argv);
#define SUBTARGET_FRAME_POINTER_REQUIRED 0
#endif
+/* Define the shadow offset for asan. Other OS's can override in the
+ respective tm.h files. */
+#ifndef SUBTARGET_SHADOW_OFFSET
+#define SUBTARGET_SHADOW_OFFSET\
+ (TARGET_LP64 ? HOST_WIDE_INT_C (0x7fff8000) : HOST_WIDE_INT_1 << 29)
+#endif
+
/* Make sure we can access arbitrary call frames. */
#define SETUP_FRAME_ADDRESSES() ix86_setup_frame_addresses ()
diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
index 4d8ec02f96e..c48460a4d36 100644
--- a/libsanitizer/ChangeLog
+++ b/libsanitizer/ChangeLog
[PATCH] testsuite: Enable asan tests on FreeBSD x86_64
Hi all,
and here the tests which should succeed now.
Ok for trunk (as usual if open...)
TIA,
Andreas
testsuite:
* gcc.dg/asan/pr87930.c: Enable on x86_64 FreeBSD.
* c-c++-common/asan/asan-interface-1.c: Likewise.
* c-c++-common/asan/clone-test-1.c: Likewise.
* c-c++-common/asan/no-asan-stack.c: Likewise.
* c-c++-common/asan/pr59063-1.c: Likewise.
* c-c++-common/asan/pr59063-2.c: Likewise.
* g++.dg/asan/asan_test.C: Likewise.
* g++.dg/asan/asan_test_utils.h: Likewise.
* g++.dg/asan/interception-failure-test-1.C: Likewise.
* g++.dg/asan/interception-malloc-test-1.C: Likewise
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 70d7e8869e1..7e3534f84de 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,16 @@
+2020-01-22 Andreas Tobler
+
+ * gcc.dg/asan/pr87930.c: Enable on x86_64 FreeBSD.
+ * c-c++-common/asan/asan-interface-1.c: Likewise.
+ * c-c++-common/asan/clone-test-1.c: Likewise.
+ * c-c++-common/asan/no-asan-stack.c: Likewise.
+ * c-c++-common/asan/pr59063-1.c: Likewise.
+ * c-c++-common/asan/pr59063-2.c: Likewise.
+ * g++.dg/asan/asan_test.C: Likewise.
+ * g++.dg/asan/asan_test_utils.h: Likewise.
+ * g++.dg/asan/interception-failure-test-1.C: Likewise.
+ * g++.dg/asan/interception-malloc-test-1.C: Likewise.
+
2020-01-22 Marek Polacek
PR c++/93324 - ICE with -Wall on constexpr if.
diff --git a/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
b/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
index 33ed1b0e845..2bf1fae857f 100644
--- a/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
+++ b/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
@@ -1,6 +1,6 @@
/* Check that interface headers work. */
-/* { dg-do run { target { *-*-linux* } } } */
+/* { dg-do run { target { *-*-linux* *-*-freebsd* } } } */
#include
diff --git a/gcc/testsuite/c-c++-common/asan/clone-test-1.c
b/gcc/testsuite/c-c++-common/asan/clone-test-1.c
index c58c376f5df..b95ec32f9ef 100644
--- a/gcc/testsuite/c-c++-common/asan/clone-test-1.c
+++ b/gcc/testsuite/c-c++-common/asan/clone-test-1.c
@@ -1,7 +1,7 @@
/* Regression test for:
http://code.google.com/p/address-sanitizer/issues/detail?id=37 */
-/* { dg-do run { target { *-*-linux* } } } */
+/* { dg-do run { target { *-*-linux* x86_64-*-freebsd* } } } */
/* { dg-require-effective-target clone } */
/* { dg-require-effective-target hw } */
/* { dg-options "-D_GNU_SOURCE" } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-asan-stack.c
b/gcc/testsuite/c-c++-common/asan/no-asan-stack.c
index 59ae55b0b00..9d1d37cb6c4 100644
--- a/gcc/testsuite/c-c++-common/asan/no-asan-stack.c
+++ b/gcc/testsuite/c-c++-common/asan/no-asan-stack.c
@@ -1,4 +1,4 @@
-/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux* } && lp64 } } }
*/
+/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux*
x86_64-*-freebsd* } && lp64 } } } */
/* { dg-options "--param asan-stack=0" } */
#include
diff --git a/gcc/testsuite/c-c++-common/asan/pr59063-1.c
b/gcc/testsuite/c-c++-common/asan/pr59063-1.c
index a22db6a0d82..1cc7f6f9428 100644
--- a/gcc/testsuite/c-c++-common/asan/pr59063-1.c
+++ b/gcc/testsuite/c-c++-common/asan/pr59063-1.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { *-*-linux* } } } */
+/* { dg-do run { target { *-*-linux* *-*-freebsd* } } } */
#include
static int weak_gettime (clockid_t clk_id, struct timespec *tp)
diff --git a/gcc/testsuite/c-c++-common/asan/pr59063-2.c
b/gcc/testsuite/c-c++-common/asan/pr59063-2.c
index 759b7f24d09..63a547f033f 100644
--- a/gcc/testsuite/c-c++-common/asan/pr59063-2.c
+++ b/gcc/testsuite/c-c++-common/asan/pr59063-2.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { *-*-linux* } } } */
+/* { dg-do run { target { *-*-linux* *-*-freebsd* } } } */
/* { dg-options "-static-libasan" } */
#include
diff --git a/gcc/testsuite/g++.dg/asan/asan_test.C
b/gcc/testsuite/g++.dg/asan/asan_test.C
index f3f7626ef3b..6db37411caf 100644
--- a/gcc/testsuite/g++.dg/asan/asan_test.C
+++ b/gcc/testsuite/g++.dg/asan/asan_test.C
@@ -1,12 +1,13 @@
-// { dg-do run { target { { *-*-linux* } && { { ! { i?86-*-linux*
x86_64-*-linux* } } || sse2_runtime } } } }
+// { dg-do run { target { { *-*-linux* i?86-*-freebsd* x86_64-*-freebsd* } &&
{ { ! { i?86-*-linux* x86_64-*-linux* i?86-*-freebsd* x86_64-*-freebsd* } } ||
sse2_runtime } } } }
// { dg-skip-if "" { *-*-* } { "*" } { "-O2" } }
// { dg-skip-if "" { *-*-* } { "-flto" } { "" } }
// { dg-additional-sources "asan_globals_test-wrapper.cc" }
-// { dg-options "-std=c++11 -fsanitize=address -fno-builtin -Wall -Werror -g
-DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 -DASAN_HAS_BLACKLIST=0
-DSANITIZER_USE_DEJAGNU_GTEST=1 -lasan -lpthread -ldl" }
+// { dg-options "-std=c++11 -fsanitize=address -fno-builtin -Wall -Werror -g
-DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 -DASAN_HAS_BLACKLIST=0
-DSANITIZER_USE_DEJAGN
