Re: reorg.c (fill_slots_from_thread): Reinstate code typoed out in "Remove CC0".
Hi! On Sat, May 15, 2021 at 08:41:32PM +0200, Hans-Peter Nilsson wrote: > The typo here, is obviously mistaken removal of lines next > to a line that was validly removed. Targets affected are > those with a delay-slot *and* defining TARGET_FLAGS_REGNUM. > In-tree, a git-grep says the only ones matching are CRIS, > h8300 and visium. The code removal has the effect of > wrong-code, not reverting the effect of r11-2814. Which explains why I didn't see it (I don't run emulators for those archs, and no hardware either). I do build cross-compilers for all archs that are supported by Linux, and then Linux itself by that. h8300 built fine, and was identical before and after the cc0 removal. This did catch another case where I deleted too much (I think I just hit "dd" too often there :-) ) I think here it was just caused by my misevaluating the original code: && (!HAVE_cc0 || (! (reg_mentioned_p (cc0_rtx, pat) && (! own_thread || ! sets_cc0_p (pat) which was a bit hard to parse for my feeble mind. Count the number of !s, for one thing, I must have taken a wrong turn somewhere. This is just && !(HAVE_cc0 && reg_mentioned_p (cc0_rtx, pat) && !(own_thread && sets_cc0_p (pat))) which is clearer at least for the cc0 removal :-) (In general, && is almost always more natural than ||, easier to read; and breaking it up into multiple statements is better still of course). > I'm "guessing" it was the effect of an incorrect conflict > resolution in preparatory work for the r12-440 / > bd1cd0d0e0fe / "Remove CC0" commit, when rebasing a related > branch, and not testing any of the affected targets. It isn't, it was there in my dev branch already (which still is on gcc.gnu.org if you want to check; last time I rebased the version there was in December). > Either > way, the effect was a btest-gcc.sh state of "regress-1152" > for cris-elf. FWIW, I wrote the removed code (sans the > validly removed cc0 line), a part of what was committed at > 2020-08-24 as 0e6c51de8ec47 / r11-2814. > > This commit gets cris-elf test-results back to a sane state > (tested at 0ffdbc85d9a6 / r12-761). > > gcc: > * reorg.c (fill_slots_from_thread): Reinstate code typoed out in > "Remove CC0". Thank you for fixing it! And sorry for the mistake. Cheers, Segher
[PATCH v3] bpf.2: Use standard types and attributes
Some manual pages are already using C99 syntax for integral
types 'uint32_t', but some aren't. There are some using kernel
syntax '__u32'. Fix those.
Both the kernel and the standard types are 100% binary compatible,
and the source code differences between them are very small, and
not important in a manual page:
- Some of them are implemented with different underlying types
(e.g., s64 is always long long, while int64_t may be long long
or long, depending on the arch). This causes the following
differences.
- length modifiers required by printf are different, resulting in
a warning ('-Wformat=').
- pointer assignment causes a warning:
('-Wincompatible-pointer-types'), but there aren't any pointers
in this page.
But, AFAIK, all of those warnings can be safely ignored, due to
the binary compatibility between the types.
...
Some pages also document attributes, using GNU syntax
'__attribute__((xxx))'. Update those to use the shorter and more
portable C11 keywords such as 'alignas()' when possible, and C2x
syntax '[[gnu::xxx]]' elsewhere, which hasn't been standardized
yet, but is already implemented in GCC, and available through
either --std=c2x or any of the --std=gnu... options.
The standard isn't very clear on how to use alignas() or
[[]]-style attributes, and the GNU documentation isn't better, so
the following link is a useful experiment about the different
alignment syntaxes:
__attribute__((aligned())), alignas(), and [[gnu::aligned()]]:
Signed-off-by: Alejandro Colomar
Nacked-by: Alexei Starovoitov
Nacked-by: Greg Kroah-Hartman
Acked-by: Zack Weinberg
Cc: LKML
Cc: glibc
Cc: GCC
Cc: bpf
Cc: David Laight
Cc: Joseph Myers
Cc: Florian Weimer
Cc: Daniel Borkmann
---
man2/bpf.2 | 49 -
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index 6e1ffa198..04b8fbcef 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -186,41 +186,40 @@ commands:
.PP
.in +4n
.EX
-union bpf_attr {
+union [[gnu::aligned(8)]] bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
- in a map */
+uint32_tmap_type;
+uint32_tkey_size;/* size of key in bytes */
+uint32_tvalue_size; /* size of value in bytes */
+uint32_tmax_entries; /* maximum number of entries
+in a map */
};
-struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
- commands */
-__u32 map_fd;
-__aligned_u64 key;
+struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY commands */
+uint32_tmap_fd;
+uint64_t alignas(8) key;
union {
-__aligned_u64 value;
-__aligned_u64 next_key;
+uint64_t alignas(8) value;
+uint64_t alignas(8) next_key;
};
-__u64 flags;
+uint64_tflags;
};
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
-__aligned_u64 insns; /* \(aqconst struct bpf_insn *\(aq */
-__aligned_u64 license;/* \(aqconst char *\(aq */
-__u32 log_level; /* verbosity level of verifier */
-__u32 log_size; /* size of user buffer */
-__aligned_u64 log_buf;/* user supplied \(aqchar *\(aq
- buffer */
-__u32 kern_version;
- /* checked when prog_type=kprobe
- (since Linux 4.1) */
+uint32_tprog_type;
+uint32_tinsn_cnt;
+uint64_t alignas(8) insns; /* \(aqconst struct bpf_insn *\(aq */
+uint64_t alignas(8) license; /* \(aqconst char *\(aq */
+uint32_tlog_level; /* verbosity level of verifier */
+uint32_tlog_size; /* size of user buffer */
+uint64_t alignas(8) log_buf; /* user supplied \(aqchar *\(aq
+ buffer */
+uint32_tkern_version;
+ /* checked when prog_type=kprobe
+ (since Linux 4.1) */
.\" commit 2541517c32be2531e0da59dfd7efc1ce844644f5
};
-} __attribute__((aligned(8)));
+};
.EE
.in
.\"
--
2.31.1
Re: [PATCH 1/4] rs6000: Add -mrop-protect and -mprivileged flags
Series pushed with recommended changes. Thank you! Bill On 5/14/21 10:55 AM, Segher Boessenkool wrote: On Thu, May 13, 2021 at 10:34:54PM -0500, Bill Schmidt via Gcc-patches wrote: +mprivileged +Target Var(rs6000_privileged) Init(0) +Enable generation of instructions that require privileged state. That isn't quite it -- it will generate "p" insns instead of the usual ones, it isn't just that it is allowed to generate the "p" insns. +@item -mprivileged +@itemx -mno-privileged +@opindex mprivileged +@opindex mno-privileged +Generate (do not generate) instructions for privileged state. That is better. But may I suggest: "Generate code that will run in privileged state." Okay for trunk and 11 with whatever you end up with. Thanks! Segher
doc/tm.texi.in (Condition Code): Tweak post-cc0 wording.
Ok to commit?
When eyeballing the r12-440 / bd1cd0d0e0fe / "Remove
CC0" commit, I noticed parts that could be improved.
Regarding the first change: at first I thought that just
removing the word "better" was the best choice, as the
compared part (cc0) was apparently removed, but the
paragraph after the one in the patch (still) does speak of
"implicit setting" (i.e. cc0-style), but now as hypothetical
reasoning. So, just add that to clarify what is not-better.
The second change is just that there's no non-modern
representation, so the "Modern" qualifier is just confusing.
gcc:
* doc/tm.texi.in (Condition Code): Tweak post-cc0 wording.
* doc/tm.texi: Regenerate.
---
gcc/doc/tm.texi| 4 ++--
gcc/doc/tm.texi.in | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index d8e3de14af1a..9009eaecc4de 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4269,7 +4269,7 @@ or @code{TARGET_MAX_ANCHOR_OFFSET} is set to a nonzero
value.
@cindex condition code status
Condition codes in GCC are represented as registers,
-which provides better schedulability for
+which provides better schedulability than implicit clobbering for
architectures that do have a condition code register, but on which
most instructions do not affect it. The latter category includes
most RISC machines.
@@ -4300,7 +4300,7 @@ specified already in the compare instruction. In this
case, you are not
interested in most macros in this section.
@menu
-* MODE_CC Condition Codes:: Modern representation of condition codes.
+* MODE_CC Condition Codes:: Representation of condition codes.
@end menu
@node MODE_CC Condition Codes
--
2.11.0
brgds, H-P
reorg.c (fill_slots_from_thread): Reinstate code typoed out in "Remove CC0".
Committed as obvious.
The typo here, is obviously mistaken removal of lines next
to a line that was validly removed. Targets affected are
those with a delay-slot *and* defining TARGET_FLAGS_REGNUM.
In-tree, a git-grep says the only ones matching are CRIS,
h8300 and visium. The code removal has the effect of
wrong-code, not reverting the effect of r11-2814.
I'm "guessing" it was the effect of an incorrect conflict
resolution in preparatory work for the r12-440 /
bd1cd0d0e0fe / "Remove CC0" commit, when rebasing a related
branch, and not testing any of the affected targets. Either
way, the effect was a btest-gcc.sh state of "regress-1152"
for cris-elf. FWIW, I wrote the removed code (sans the
validly removed cc0 line), a part of what was committed at
2020-08-24 as 0e6c51de8ec47 / r11-2814.
This commit gets cris-elf test-results back to a sane state
(tested at 0ffdbc85d9a6 / r12-761).
gcc:
* reorg.c (fill_slots_from_thread): Reinstate code typoed out in
"Remove CC0".
---
gcc/reorg.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/gcc/reorg.c b/gcc/reorg.c
index 4595f9a541f0..7f06a6f6d092 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -2375,6 +2375,16 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx
condition,
if (! insn_references_resource_p (trial, , true)
&& ! insn_sets_resource_p (trial, filter_flags ? : , true)
&& ! insn_sets_resource_p (trial, , true)
+ /* If we're handling sets to the flags register specially, we
+only allow an insn into a delay-slot, if it either:
+- doesn't set the flags register,
+- the "set" of the flags register isn't used (clobbered),
+- insns between the delay-slot insn and the trial-insn
+as accounted in "set", have not affected the flags register. */
+ && (! filter_flags
+ || ! insn_sets_resource_p (trial, _res, true)
+ || find_regno_note (trial, REG_UNUSED, targetm.flags_regnum)
+ || ! TEST_HARD_REG_BIT (set.regs, targetm.flags_regnum))
&& ! can_throw_internal (trial))
{
rtx_insn *prior_insn;
--
2.11.0
brgds, H-P
Re: [PATCH] phiopt: Optimize (x <=> y) cmp z [PR94589]
On Fri, 14 May 2021, Jakub Jelinek via Gcc-patches wrote:
On Thu, May 06, 2021 at 09:42:41PM +0200, Marc Glisse wrote:
We can probably do it in 2 steps, first something like
(for cmp (eq ne)
(simplify
(cmp (bit_and:c @0 @1) @0)
(cmp (@0 (bit_not! @1)) { build_zero_cst (TREE_TYPE (@0)); })))
to get rid of the double use, and then simplify X==0 to X<=~C if C is a
mask 111...000 (I thought we already had a function to detect such masks, or
the 000...111, but I can't find them anymore).
I agree that the comparison seems preferable, although if X is signed, the
way GIMPLE represents types will add an inconvenient cast. And I think VRP
already manages to use the bit test to derive a range.
I've tried the second step, but it unfortunately regresses
+FAIL: gcc.dg/ipa/propbits-2.c scan-tree-dump-not optimized "fail_test"
+FAIL: gcc.dg/tree-ssa/loop-42.c scan-tree-dump-not ivcanon "under assumptions "
so maybe it is better to keep these cases as the users wrote them.
Thank you for trying it, the patch looks good (it would probably also work
on GENERIC), but indeed it is just a canonicalization, not necessarily
worth breaking other stuff. This seems to point to another missed
optimization. Looking at propbits-2.c, if I rewrite the condition in f1 as
if ((unsigned)x <= 0xf)
I see later in VRP
# RANGE [0, 4294967295] NONZERO 15
x.2_1 = (unsigned intD.9) x_4(D);
if (x.2_1 <= 15)
where we fail to derive a range from the nonzero bits. Maybe VRP expects
IPA-CP should have done it already and doesn't bother recomputing it.
I understand this may not be a priority though, that's fine.
I didn't look at loop-42.c.
--
Marc Glisse
Re: [PATCH] tree-sra: Avoid refreshing into const base decls (PR 100453)
Hi, On Fri, May 14 2021, Eric Botcazou wrote: >> Looks like this caused: >> >> === acats tests === >> FAIL: c41325a >> FAIL: c45347d >> FAIL: c74402a >> FAIL: c95085m >> FAIL: cc3601a >> >> === gnat tests === >> >> FAIL: gnat.dg/addr12.adb (test for excess errors) >> UNRESOLVED: gnat.dg/addr12.adb compilation failed to produce executable >> FAIL: gnat.dg/addr12_a.adb (test for excess errors) >> FAIL: gnat.dg/bip_overlay.adb (test for excess errors) >> FAIL: gnat.dg/global.adb (test for excess errors) >> FAIL: gnat.dg/spark1.adb (test for errors, line 8) >> FAIL: gnat.dg/spark1.adb (test for excess errors) >> FAIL: gnat.dg/sync2.adb (test for excess errors) >> FAIL: gnat.dg/synchronized1.adb (test for excess errors) > > Yes, it did, as well as PR boostrap/100597 probably. > Sorry, I forgot I do not test Ada on Aarch64 (where I chose to do the testing because the architecture has DECL_IN_CONSTANT_POOL constnts). I have reverted the commit until I figure out what is going on. Sorry again, Martin
