Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.
On Sun, Jul 12, 2020 at 1:29 AM Roger Sayle wrote: > > > Even by my standards, this is an odd patch. This adds expanders to > i386.md requesting that integer truncations be represented in RTL > using SUBREGs. This exactly matches the (current) default behaviour > when TARGET_TRULY_NOOP_TRUNCATION is undefined. Hence this patch > is mostly for documentation/teaching purposes, so that i386.md is a > template or role model for the patterns that a backend should provide. > > As explained in my earlier post, defining TARGET_TRULY_NOOP_TRUNCATION > to always return false in the i386/x86_64 backend, results in 603 > additional unexpected failures. Adding these expanders avoids the > majority (412) of those regressions. > > I'm not proposing that i386/x86_64 redefine TARGET_TRULY_NOOP_TRUNCATION > but these placeholder expanders may provide the backend the flexibility > of that option in the future, but it also helps to test some less frequently > invoked corners of the middle-end. > > This patch has been tested on x86_64-pc-linux-gnu with a full "make > bootstrap" and "make -k check" with no new failures, indeed there > are (should be) absolutely no code changes with this patch. > > Might these changes be of interest? If not, at least this patch > is now archived on gcc-patches for future generations. It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation might be useful here - from what it says it suggests to me it applies to hardregs only and tells for example whether there are lower than word_mode precision registers like on x86_64 %al? Because for pseudos there's always the requirement to use subregs and doing (reg:SI 42) and (reg:QI 42) accesses to the same pseudo is invalid(?). The only user (after your patch) of this hook is in function.c for the function parameter setup btw. and I wonder if there's other hooks for the RA for example that provide duplicate functionality. Richard. > > 2020-07-12 Roger Sayle > > gcc/ChangeLog > * config/i386/i386.md (truncdi2, truncsi2, > trunchiqi2): New expanders to make the intended representation > of scalar integer truncations explicit. > > Thoughts? > Roger > -- > Roger Sayle > NextMove Software > Cambridge, UK > >
Re: [PATCH] middle-end: Remove truly_noop_truncation check from convert.c
On Sat, Jul 11, 2020 at 8:29 PM Roger Sayle wrote: > > > This patch eliminates a check of targetm.truly_noop_truncation from > the early middle-end, where the gimple/generic being generated by > GCC's front-ends is being inappropriately influenced by the target's > TRULY_NOOP_TRUNCATION. The (recent) intention of TRULY_NOOP_TRUNCATION > is to indicate that a backend requires explicit truncation instructions > rather than using SUBREGs to perform truncations. A long standing > (and probably unintentional) side-effect has been that this setting > also controls whether the middle-end narrows integer operations at > the tree-level. Understandably, GCC and its testsuite assume that > GIMPLE and GENERIC behave consistently across platforms, and alas > defining TRULY_NOOP_TRUNCATION away from the default triggers several > regressions (including gcc.dg/fold-rotate-1.c). > > Hopefully, I can appeal to a design philosophy argument that the > early middle-end (tree-ssa) passes should be as machine independent > as possible, and target settings influence the late middle-end (RTL) > passes. Most of the backend hooks were eliminated from "fold" and > the tree code with GCC 4.x, but perhaps TRULY_NOOP_TRUNCATION appears > to have survived, probably because it is so rarely used. It's only > defined by three (current) backends: gcn, mips and tilegx. Everywhere > else TRULY_NOOP_TRUNCATION is unconditionally true, so that > convert_to_integer_1 always calls do_narrow. > > This check of TRULY_NOOP_TRUNCATION (in convert_to_integer) dates back > to (before) gcc 0.9 in 1987. Mysteriously, at that point in time, all > targets defined TRULY_NOOP_TRUNCATION to 1, so perhaps only Richard > Stallman remembers why this code was ever added to the compiler. > Even if truncations require explicit instructions, that shouldn't > prevent the compiler from generating them (or using narrow integer > operations). > > The following patch has been tested on x86_64-pc-linux-gnu with > no regressions (where of course you wouldn't expect any changes), > and on a x86_64-pc-linux-gnu that defines TRULY_NOOP_TRUNCATION > to false, where it cures 17 of the regressions that are introduced. > It also fixes all of the regressions introduced by defining > TRULY_NOOP_TRUNCATION to false on nvptx-none. I had hoped to > also test on a MIPS target, but after over 10 days of bootstrap builds > on the GCC compile farm, gcc23.fsffrance.org has started experiencing > hardware issues (overheating?), so it may be a while before I can > fully confirm this change causes no problems (and potentially fixes > a few) on MIPS. > > Ok for mainline? OK. The other big early target dependence is of course LOGICAL_OP_NON_SHORT_CIRCUIT ... (aka BRANCH_COST). Thanks, Richard. > 2020-07-11 Roger Sayle > > gcc/ChangeLog > * convert.c (convert_to_integer_1): Narrow integer operations > even on targets that require explicit truncation instructions. > > Thanks in advance, > Roger > -- > Roger Sayle > NextMove Software > Cambridge, UK >
Re: [PATCH] rs6000: Define movsf_from_si2 to extract high part SF element from DImode[PR89310]
Hi,
On 2020/7/11 08:54, Segher Boessenkool wrote:
> Hi!
>
> On Fri, Jul 10, 2020 at 09:39:40AM +0800, luoxhu wrote:
>> OK, seems the md file needs a format tool too...
>
> Heh. Just make sure it looks good (that is, does what it looks like),
> looks like the rest, etc. It's hard to do anything nice with unspecs,
> [ ] lists do not format well.
>
+ "TARGET_NO_SF_SUBREG"
+ "#"
+ "&& vsx_reg_sfsubreg_ok (operands[0], SFmode)"
>>>
>>> Put this in the insn condition? And since this is just a predicate,
>>> you can just use it instead of gpc_reg_operand.
>>>
>>> (The split condition becomes "&& 1" then, not "").
>>
>> OK, this seems a bit strange as movsi_from_sf, movsf_from_si and
>> movdi_from_sf_zero_ext all use it as condition...
>
> Since in your case you *always* split, the split condition should be
> "always". There are no alternatives that do not split here.
>
>> And why vsx_reg_sfsubreg_ok allows "SF SUBREGS" and TARGET_NO_SF_SUBREG
>> "avoid (SUBREG:SF (REG:SI)", I guess they are not the same meaning? (The
>> TARGET_NO_SF_SUBREG is also copied from other similar defines.) Thanks.
>
> Good question. I do not know.
>
> Well... Since this define_insn* requires p8 *anyway*, we do not need
> any of these sf_subreg things? We always know for each one if it should
> be true or false.
Yes, removed the vsx_reg_sfsubreg_ok check.
>
>> + "TARGET_NO_SF_SUBREG"
>
> But here we should require p8 some other way, then.
TARGET_NO_SF_SUBREG is defined to TARGET_DIRECT_MOVE_64BIT, and
TARGET_DIRECT_MOVE_64BIT is TARGET_DIRECT_MOVE && TARGET_P8_VECTOR &&
TARGET_POWERPC64
which means TARGET_P8_VECTOR must be true for TARGET_NO_SF_SUBREG.
>
>> + (set_attr "isa" "p8v")])
>
> (This isn't enough, unfortunately).
>
Updated patch to removed the vsx_reg_sfsubreg_ok and ICE fix:
For extracting high part element from DImode register like:
{%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
split it before reload with "and mask" to avoid generating shift right
32 bit then shift left 32 bit. This pattern also exists in PR42475 and
PR67741, etc.
srdi 3,3,32
sldi 9,3,32
mtvsrd 1,9
xscvspdpn 1,1
=>
rldicr 3,3,0,31
mtvsrd 1,3
xscvspdpn 1,1
Bootstrap and regression tested pass on Power8-LE.
gcc/ChangeLog:
2020-07-13 Xionghu Luo
PR rtl-optimization/89310
* config/rs6000/rs6000.md (movsf_from_si2): New
define_insn_and_split.
gcc/testsuite/ChangeLog:
2020-07-13 Xionghu Luo
PR rtl-optimization/89310
* gcc.target/powerpc/pr89310.c: New test.
---
gcc/config/rs6000/rs6000.md| 31 ++
gcc/testsuite/gcc.target/powerpc/pr89310.c | 17
2 files changed, 48 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr89310.c
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 4fcd6a94022..480385ed4d2 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -7593,6 +7593,37 @@ (define_insn_and_split "movsf_from_si"
"*, *, p9v, p8v, *, *,
p8v,p8v, p8v, *")])
+;; For extracting high part element from DImode register like:
+;; {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
+;; split it before reload with "and mask" to avoid generating shift right
+;; 32 bit then shift left 32 bit.
+(define_insn_and_split "movsf_from_si2"
+ [(set (match_operand:SF 0 "gpc_reg_operand" "=wa")
+ (unspec:SF
+[(subreg:SI
+ (ashiftrt:DI
+ (match_operand:DI 1 "input_operand" "r")
+ (const_int 32))
+ 0)]
+UNSPEC_SF_FROM_SI))
+ (clobber (match_scratch:DI 2 "=r"))]
+ "TARGET_NO_SF_SUBREG"
+ "#"
+ "&& 1"
+ [(const_int 0)]
+{
+ if (GET_CODE (operands[2]) == SCRATCH)
+operands[2] = gen_reg_rtx (DImode);
+
+ rtx mask = GEN_INT (HOST_WIDE_INT_M1U << 32);
+ emit_insn (gen_anddi3 (operands[2], operands[1], mask));
+ emit_insn (gen_p8_mtvsrd_sf (operands[0], operands[2]));
+ emit_insn (gen_vsx_xscvspdpn_directmove (operands[0], operands[0]));
+ DONE;
+}
+ [(set_attr "length" "12")
+ (set_attr "type" "vecfloat")
+ (set_attr "isa" "p8v")])
;; Move 64-bit binary/decimal floating point
(define_expand "mov"
diff --git a/gcc/testsuite/gcc.target/powerpc/pr89310.c
b/gcc/testsuite/gcc.target/powerpc/pr89310.c
new file mode 100644
index 000..15e78509246
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr89310.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct s {
+ int i;
+ float f;
+};
+
+float
+foo (struct s arg)
+{
+ return arg.f;
+}
+
+/* { dg-final { scan-assembler-not {\msrdi\M} } } */
+/* { dg-final { scan-assembler-not {\msldi\M} } } */
+/* { dg-final { scan-assembler-times {\mrldicr\M} 1 } } */
--
2.21.0.777.g83232e3864
Re: [PATCH] ipa-devirt: Fix crash in obj_type_ref_class [PR95114]
On Sat, Jul 11, 2020 at 10:21 AM Richard Sandiford wrote: > > Jan Hubicka writes: > >> Jan Hubicka writes: > >> >> The testcase has failed since r9-5035, because obj_type_ref_class > >> >> tries to look up an ODR type when no ODR type information is > >> >> available. (The information was available earlier in the > >> >> compilation, but was freed during pass_ipa_free_lang_data.) > >> >> We then crash dereferencing the null get_odr_type result. > >> >> > >> >> The test passes with -O2. However, it fails again if -fdump-tree-all > >> >> is used, since obj_type_ref_class is called indirectly from the > >> >> dump routines. > >> >> > >> >> Other code seems to create ODR type entries on the fly by passing > >> >> "true" as the insert parameter. But since obj_type_ref_class is > >> >> used by dump routines, I guess it should have no side-effects and > >> >> should simply punt in this case. > >> > Thanks for looking into this! > >> > > >> > The ODR type hash is indeed supposed to be populated lazilly except for > >> > LTO when we populate it at stream in time. I think just making function > >> > return NULL in case it should not is fragile, because we may hit missed > >> > optimizations and other surprised elsewhere. > >> > > >> > What about adding new "for_dump" parameter set implicitly to false that > >> > will make the function return ret in case get_odr_type is NULL? > >> > For dumping it probably does not matter what ODR variant you see, since > >> > we end up with a type name in the dump anyway. > >> > >> I think there are two problematic cases (at least as far as this > >> testcase goes). One is the direct call to obj_type_ref_class in > >> tree-pretty-print.c. The other is an indirect call via > >> virtual_method_call_p, which is also called by tree-pretty-print.c. > >> So I guess both of them would need a "for_dump" parameter. > >> > >> TBH doing it that way feels fragile too. Even if we get it right now, > >> it's going to be hard to ensure that all indirect calls to > >> obj_type_ref_class > >> in future make the correct choice between lazy initialisation and remaining > >> side-effect free. > > > > Yep, it is not very pretty :(. In general the output of optimizations > > does not depend much on what is inserted into the type inheritance graph > > (because we must assume that types are derived in other units anyway) > > except for anonymous types. However speculative devirt is an exception > > here, so I guess we want to go woth for_dump markers (it is useful to > > have obj type ref pretty printed in a meaningful way). > > OK, this patch (belatedly) does that. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK for trunk and > GCC 9+? OK. Richard. > Thanks, > Richard > > > gcc/ > PR middle-end/95114 > * tree.h (virtual_method_call_p): Add a default-false parameter > that indicates whether the function is being called from dump > routines. > (obj_type_ref_class): Likewise. > * tree.c (virtual_method_call_p): Likewise. > * ipa-devirt.c (obj_type_ref_class): Likewise. Lazily add ODR > type information for the type when the parameter is false. > * tree-pretty-print.c (dump_generic_node): Update calls to > virtual_method_call_p and obj_type_ref_class accordingly. > > gcc/testsuite/ > PR middle-end/95114 > * g++.target/aarch64/pr95114.C: New test. > --- > gcc/ipa-devirt.c | 9 ++--- > gcc/testsuite/g++.target/aarch64/pr95114.C | 3 +++ > gcc/tree-pretty-print.c| 5 +++-- > gcc/tree.c | 7 --- > gcc/tree.h | 4 ++-- > 5 files changed, 18 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/g++.target/aarch64/pr95114.C > > diff --git a/gcc/tree.h b/gcc/tree.h > index cf546ed9491..866d9ba8fbc 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -5241,8 +5241,8 @@ extern location_t *block_nonartificial_location (tree); > extern location_t tree_nonartificial_location (tree); > extern tree block_ultimate_origin (const_tree); > extern tree get_binfo_at_offset (tree, poly_int64, tree); > -extern bool virtual_method_call_p (const_tree); > -extern tree obj_type_ref_class (const_tree ref); > +extern bool virtual_method_call_p (const_tree, bool = false); > +extern tree obj_type_ref_class (const_tree ref, bool = false); > extern bool types_same_for_odr (const_tree type1, const_tree type2); > extern bool contains_bitfld_component_ref_p (const_tree); > extern bool block_may_fallthru (const_tree); > diff --git a/gcc/tree.c b/gcc/tree.c > index 342da55bba7..3d9968fd7a0 100644 > --- a/gcc/tree.c > +++ b/gcc/tree.c > @@ -12810,10 +12810,11 @@ lhd_gcc_personality (void) > OBJ_TYPE_REF representing an virtual call of C++ method. > (As opposed to OBJ_TYPE_REF representing objc calls > through a cast where middle-end devirtualization machinery > - can't apply.) */ >
Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372
> From: Segher Boessenkool > Date: Tue, 7 Jul 2020 22:50:43 +0200 > I'll make a simpler patch. Thanks! You're welcome. So, you'll take care of the updated patch yourself? (I'll wait a month before sending an update either way.) brgds, H-P
Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372
> From: Segher Boessenkool
> Date: Tue, 7 Jul 2020 22:23:59 +0200
> Hi!
>
> On Tue, Jul 07, 2020 at 02:50:09AM +0200, Hans-Peter Nilsson wrote:
> > > On Mon, Jul 06, 2020 at 03:11:17AM +0200, Hans-Peter Nilsson wrote:
> > > > TL;DR: fixing a misdetection of what is a "simple move".
> > >
> > > That is not a very correct characterisation of what this does :-)
> >
> > That's apparently where we completely disagree. :-)
>
> Well, I wrote that code, I know what is considered "just a move" there.
You lost some context: I'm comparing before/after the
cc0-conversion for CRIS, where this is a misdetection (a false
negative) of a move and causes a performance-regression.
> You want to extend that, and that is fine, but this is not a bug. Taken
> to the extreme, anything in GCC is completely buggy, because it doesn't
> solve world hunger (yet!), following that line of thought.
Hyperboles are funny, but only to some extent.
> > > > Looking into performace degradation after de-cc0 for CRIS, I
> > > > noticed combine behaving badly; it changed a move and a
> > > > right-shift into two right-shifts, where the "combined" move was
> > > > not eliminated in later passes, and where the deficiency caused
> > > > an extra insn in a hot loop: crcu16 (and crcu32) in coremark.
> > > >
> > > > Before de-cc0, the insns input to combine looked like:
> > > >33: r58:SI=r56:SI 0>>r48:SI
> > > > REG_DEAD r56:SI
> > > >35: r37:HI=r58:SI#0
> > > > and after:
> > > >33: {r58:SI=r56:SI 0>>r48:SI;clobber dccr:CC;}
> > > > REG_DEAD r56:SI
> > > > REG_UNUSED dccr:CC
> > > >35: {r37:HI=r58:SI#0;clobber dccr:CC;}
> > > > REG_UNUSED dccr:CC
...
> > Here, the
> > end result is that it *added* an instruction to the hot loop.
>
> It did not. Combine *never* does that.
The "it" refers not just to combine here, but the compilation as
a whole.
The *end* result is an added instruction, since a move+shift was
changed into shift+shift and later passes couldn't eliminate it.
Just the thing your is_just_move was supposed to prevent.
> > It's a deficiency and it caused a performance regression, can't
> > argue with that.
>
> It did not cause any regression. It can be improved, sure, and thank
> you for pointing that out!
It caused a regression compared to the cc0 version, per above.
> > > (2-2 combinations are helpful on single-scalar and even
> > > in-order machines as well, btw).
> >
> > I certainly don't contest that the move can be eliminated, and
> > that most cost-effective 2-2 eliminations are helpful. (See my
> > other post about combine being eager with allowing same-cost
> > combinations.)
>
> I did not see that post, do you have a pointer?
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549416.html
> > > > At first I thought this was due to parallel-ignorant old code
> > > > but the "guilty" change is actually pretty recent. Regarding a
> > > > parallel with a clobber not being "just" a move, there's only
> > > > the two adjacent callers seen in the patch (obviously with the
> > > > rename), and their use exactly matches to check that the
> > > > argument is a single_set which is a move. It's always applied
> > > > to an rtx_insn, so I changed the type and name to avoid the
> > > > "just" issue. I had to adjust the type when calling single_set.
> > >
> > > But it is *not* supposed to be the same as single_set.
> >
> > (I'm not saying that a single_set is a move. But that's
> > obvious.) I guess you meant to say that a single_set with a
> > general_operand as a source (as in the patch) is not supposed to
> > be the same as a move. This is the only place in combine where
> > that distinction would be important. Why?
>
> It used to be that is_just_move was called on new*pat as well, so you
> simply *could not* use single_set there (you have no rtx_insn*).
Ah, history. Hard to detect deleted code.
> single_set also allows other insns, for example, multiple sets!
...where the other sets are unused. Not sure how that kind of
thing would get combined here, but if its combined cost is
beneficial, it would be a win, I guess.
> But
> testing shows that this does not actually happen often at all, so we
> could use single_set here and not change much at all for most other
> targets.
>
> > I think you're just overconcious about clobbers here.
>
> It has nothing to do with that.
>
> > > > I checked the original commit, c4c5ad1d6d1e1e a.k.a r263067 and
> > > > it seems parallels-as-sets were just overlooked and that this
> > >
> > > They were not. It causes regressions.
> >
> > Do you have some pointers to PR:s or something else backing that
> > statement, or is it your work-in-progress hinted below?
>
> I do not know what "work in progress" you mean?
I'm referring to your "I'll rerun some testing to show this.
It'll take a while." Are the regressions you refer to above
tracked in bugzilla or on some mailing list? (I'm just meaning
current code; if there was a hi
Re: [PATCH 2/2] doc/implement-c.texi: About same-as-scalar-type volatile aggregate accesses, PR94600
> From: Richard Biener > Date: Tue, 7 Jul 2020 09:00:22 +0200 > On Tue, Jul 7, 2020 at 6:03 AM Hans-Peter Nilsson via Gcc-patches > wrote: > > > > We say very little about reads and writes to aggregate / > > compound objects, just scalar objects (i.e. assignments don't > > cause reads). Let's lets say something safe about aggregate > > objects, but only for those that are the same size as a scalar > > type. > > > > There's an equal-sounding section (Volatiles) in extend.texi, > > but this seems a more appropriate place, as specifying the > > behavior of a standard qualifier. > > Hmm, might be true only up to word-mode size, not, say, __int128_t. I'm not saying a *single* read or write, I'm saying exactly as (many as) would happen for the integer type. > Also very likely only in case the object has the same alignment > as the naturally aligned integer type. Again, just as needed for the integer type. So WDYT about Martin Sebor's suggestion? brgds, H-P
Re: [PATCH 2/2] doc/implement-c.texi: About same-as-scalar-type volatile aggregate accesses, PR94600
> From: Martin Sebor > Date: Wed, 8 Jul 2020 02:09:37 +0200 > On 7/6/20 10:02 PM, Hans-Peter Nilsson via Gcc-patches wrote: > > We say very little about reads and writes to aggregate / > > compound objects, just scalar objects (i.e. assignments don't > > cause reads). Let's lets say something safe about aggregate > > objects, but only for those that are the same size as a scalar > > type. > > > > There's an equal-sounding section (Volatiles) in extend.texi, > > but this seems a more appropriate place, as specifying the > > behavior of a standard qualifier. > > > > gcc: > > PR middle-end/94600 > > * doc/implement-c.texi (Qualifiers implementation): Add blurb > > about access to the whole of a volatile aggregate object, only for > > same-size as scalar object. > > --- > > gcc/doc/implement-c.texi | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/gcc/doc/implement-c.texi b/gcc/doc/implement-c.texi > > index 692297b69c4..d64922b28ad 100644 > > --- a/gcc/doc/implement-c.texi > > +++ b/gcc/doc/implement-c.texi > > @@ -576,6 +576,10 @@ are of scalar types, the expression is interpreted by > > GCC as a read of > > the volatile object; in the other cases, the expression is only evaluated > > for its side effects. > > > > +When an object of aggregate type has the same size as a scalar type, GCC > > +handles an access to the whole of that volatile aggregate type object > > +equal to an access to that volatile same-sized scalar type object. > > The grammar is a bit off here making the sentence difficult to > parse and interpret. (Thanks for fixing that.) > Richard already pointed out the alignment > requirement But, treating scalar and aggregate types of certain sizes equal, *also* implies that alignment requirements are equal. > but I'm also wondering if the statement is meant to > apply to accesses by library functions such as memcpy. I definitely *do not* want this to cover library functions. Having said that, I think memcpy would be the only applicable one, as you'd need to cast away volatile for other library functions. Still: no. Not going there. > I suspect > it should only apply to assignments (either simple or atomic), > correct? Yes: I just want to generalized and extend to all supported integer types (certainly target-dependent), that what's goes for reading and writing a volatile 8- 16- and 32-bit integer (and 64 where sizeof(uintptr_t) == 8) also goes for an aggregate of the same size, see PR middle-end/94600. > Would something like this be more accurate? > >When an object of an aggregate type with the same size and >alignment as a scalar type S is the subject of a volatile >access by an assignment expression or an atomic function, >the access to it is performed as if the object's declared >type were volatile S. I'd say it's equal in terms of contents, but if it has better flow to it when read, I'm all for it. Thanks! Care to patchualize it? (Heh, grammar.) brgds, H-P
Re: [PATCH] RISC-V: Fix regular expression in target-specific test
Hi Simon: Thanks for your fix! Hi Jim: Yeah, I think I should check the dejagnu errors. On Sat, Jul 11, 2020 at 12:46 PM Jim Wilson wrote: > > On Fri, Jul 10, 2020 at 6:53 AM Simon Cook wrote: > > Some square brackets were missing escape characters, causing DejaGnu to > > try and call a proc with the name "at". > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/read-thread-pointer.c: Fix escaping on > > regular expression. > > Thanks. Committed. > > I noticed that the riscv-gnu-toolchain "make report" command doesn't > check for or report dejagnu errors, which is maybe how Kito missed > this. > > Jim
RE: [wwwdocs PATCH] remove tree-browser page and links
> On Thu, 9 Jul 2020, Richard Sandiford wrote: > >> I'm trying Tree Browser during debugging, but failed. > >> I found that tree-browser.c and tree-browser.def have been removed at > >> 2015-07-25. So, to avoid misunderstanding, can we remove this > >> tree-browser page too? > > Thanks for the patch. Seems like a good idea to me. I guess the only > > question is whether we should keep it around for historical purposes, > > but with a big banner to say that it's no longer up-to-date. I also > > don't know whether we try to avoid 404s on old pages. > > Thank you, Hujp and Richard! > > We generally try to avoid 404s. So in case we remove a page put in a > redirect. > You can do so via a new entry in wwwdocs/htdocs/.htaccess (which I believe is > self explanatory, and I'm happy to help, too). > Thank you, Richard and Gerald! Since the tree-ssa/index.html and tree-ssa/tree_browser.html are no longer up-to-date, I think it may be better to add a redirect than to update. Is there a page means the page which was redirected is no longer up-to-date? Or just redirect to index.html itself as the following, it's OK? Regards. Hujp --- diff --git a/htdocs/.htaccess b/htdocs/.htaccess index 18997d63..67ee474f 100644 --- a/htdocs/.htaccess +++ b/htdocs/.htaccess @@ -67,6 +67,7 @@ Redirect permanent /proj-optimize.html https://gcc.gnu.org/projects/optimize.ht Redirect permanent /projects.html https://gcc.gnu.org/projects/ Redirect permanent /projects/cxx1z.html https://gcc.gnu.org/projects/cxx-status.html#cxx1z Redirect permanent /projects/web.html https://gcc.gnu.org/about.html +Redirect permanent /projects/tree-ssa/tree-browser.html https://gcc.gnu.org/projects/tree-ssa/ Redirect permanent /reghunt-howto.html https://gcc.gnu.org/bugs/reghunt.html Redirect permanent /svn.html https://gcc.gnu.org/git.html Redirect permanent /svnwrite.html https://gcc.gnu.org/gitwrite.html -- > My recommendation in a case like this is to follow the approach of adding a > big > banner at the top as you suggest, Richard. That said, if the consensus by > you/those working in this area is to completely remove the page, that is > perfectly fine, too. > > Gerald >
Re: [PATCH 2/2] rs6000: Define define_insn_and_split to split unspec sldi+or to rldimi
On 2020/7/11 08:28, Segher Boessenkool wrote:
Hi!
On Thu, Jul 09, 2020 at 09:14:45PM -0500, Xiong Hu Luo wrote:
* config/rs6000/rs6000.md (rotl_unspec): New
define_insn_and_split.
+; rldimi with UNSPEC_SI_FROM_SF.
+(define_insn_and_split "*rotl_unspec"
Please have rotldi3_insert in the name. "unspec" in the name doesn't
really mean much... Can you put "sf" in the name, instead? So something
like "*rotldi3_insert_sf"?
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vector_float.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mdejagnu-cpu=power9" } */
This needs p9vector_ok (yes, that name doesn't make too much sense).
+vector float
+test (float *a, float *b, float *c, float *d)
+{
+ return (vector float){*a, *b, *c, *d};
+}
+
+/* { dg-final { scan-assembler-not {\mlxsspx\M} } } */
+/* { dg-final { scan-assembler-not {\mlfs\M} } } */
No lxssp or lfsx either... or the update forms...
/* { dg-final { scan-assembler-not {\mlxssp} } } */
/* { dg-final { scan-assembler-not {\mlfs} } } */
works fine (there are no other mnemonics starting with those strings).
+/* { dg-final { scan-assembler-times {\mlwz\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mrldimi\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mmtvsrdd\M} 1 } } */
Okay for trunk with those changes (or post again if you prefer). Thanks!
Thanks. The 2 patches are committed to trunk(r11-2043, r11-2044) after
modifications.
Xionghu
Re: [PATCH] [RISC-V] Add support for TLS stack protector canary access
On Tue, Jul 7, 2020 at 7:51 PM cooper wrote: > gcc/ > * config/riscv/riscv-opts.h (stack_protector_guard): New enum. > * config/riscv/riscv.c (riscv_option_override): Handle > the new options. > * config/riscv/riscv.md (stack_protect_set): New pattern to handle > flexible stack protector guard settings. > (stack_protect_set_): Ditto. > (stack_protect_test): Ditto. > (stack_protect_test_): Ditto. > * config/riscv/riscv.opt (mstack-protector-guard=, > mstack-protector-guard-reg=, mstack-protector-guard-offset=): New > options. > * doc/invoke.texi (Option Summary) [RISC-V Options]: > Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and > -mstack-protector-guard-offset=. > (RISC-V Options): Ditto. Overall this looks OK. Please don't include the ChangeLog in the diff. This is autogenerated from the git log above now. I notice in the epilogue I get ld a4, 8(sp) ld a5, 100(t6) xor a5, a4, a5 bne a5,zero,.L4 This looks like a security leak that the canary value is left in a4. The i386 implementation operates directly on memory without loading into registers. The rs6000 implementation is careful to load 0 into the other register in the stack_protector_test code after the xor. I think this is a bug in the aarch64 code that it isn't clearing the other register. And I think it is a bug in your code too. If we don't need to clear the canary from the two registers, then you should eliminate the xor and just use "bne a5,a4,.L4". But I think the way you have it is right, you just need to clear the a4 register after the xor. If I use an offset outside the const immediate range of -2048 to 2047 then I get an ICE. You either need to error on invalid offset, or do some extra work to get valid addresses regardless of the offset. And that maybe also requires clearing the extra registers after the load to avoid leaving the address of the canary in a register after the sequence depending on how secure this needs to be. rohan:2457$ ./xgcc -B./ -O -mstack-protector-guard=tls -mstack-protector-guard-reg=x31 -mstack-protector-guard-offset=2048 -S tmp.c -fstack-protector-all tmp.c: In function ‘main’: tmp.c:8:1: error: unrecognizable insn: 8 | } | ^ (insn 4 3 7 2 (parallel [ (set (mem/v/f/c:DI (plus:DI (reg/f:DI 67 virtual-stack-vars) (const_int -8 [0xfff8])) [1 D.1495+0 S8 A64]) (unspec:DI [ (mem:DI (plus:DI (reg:DI 31 t6) (const_int 2048 [0x800])) [0 S8 A64]) ] UNSPEC_FLE_QUIET)) (set (scratch:DI) (const_int 0 [0])) ]) "tmp.c":5:1 -1 (nil)) Jim
[PATCH] libgccjit: Handle truncation and extension for casts [PR 95498]
Hello.
As mentioned in bug 95498, some conversions do not work. After
investigation, it turns out that it's caused by multiple casts on an
expression where it should do a truncation/extension.
I added a testcase, but for some reasons, the tests only pass when ran
via `./testsuite/jit/test-cast.c.exe`, not when ran via `make check-jit
RUNTESTFLAGS="-v -v -v jit.exp=test-cast.c"`.
Furthermore, some other tests failed, but they also fail on master.
Also, I was under the impression that adding `STRIP_TYPE_NOPS (t_expr);`
in `playback::context::build_cast` would be a better fix for this, but
that doesn't fix the issue.
Am I missing something?
It's my first contribution to gcc, so I'd need help for fixing the tests
and also a confirmation that this is the best way to fix this issue.
Thanks.
>From 140333077a4c5f8ce80f7e5716eecc90781594fa Mon Sep 17 00:00:00 2001
From: Antoni Boucher
Date: Sun, 5 Jul 2020 19:07:30 -0400
Subject: [PATCH] This patch handles truncation and extension for casts in jit.
2020-07-12 Antoni Boucher
gcc/jit/
PR target/95498
* jit-playback.c: Add support to handle truncation and extension
in the convert function.
gcc/testsuite/
PR target/95498
* jit.dg/all-non-failing-tests.h: New test.
* jit.dg/test-cast.c: New test.
---
gcc/jit/jit-playback.c | 39
gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
gcc/testsuite/jit.dg/test-cast.c | 66
3 files changed, 104 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-cast.c
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 0fddf04da87..4f4a1080c36 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -61,22 +61,39 @@ along with GCC; see the file COPYING3. If not see
/* gcc::jit::playback::context::build_cast uses the convert.h API,
which in turn requires the frontend to provide a "convert"
- function, apparently as a fallback.
-
- Hence we provide this dummy one, with the requirement that any casts
- are handled before reaching this. */
+ function, apparently as a fallback for casts that can be simplified
+ (truncation, extension). */
extern tree convert (tree type, tree expr);
tree
convert (tree dst_type, tree expr)
{
- gcc_assert (gcc::jit::active_playback_ctxt);
- gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
- fprintf (stderr, "input expression:\n");
- debug_tree (expr);
- fprintf (stderr, "requested type:\n");
- debug_tree (dst_type);
- return error_mark_node;
+ tree t_ret = NULL;
+ t_ret = targetm.convert_to_type (dst_type, expr);
+ if (t_ret)
+ return t_ret;
+ enum tree_code dst_code = TREE_CODE (dst_type);
+ switch (dst_code)
+{
+case INTEGER_TYPE:
+case ENUMERAL_TYPE:
+ t_ret = convert_to_integer (dst_type, expr);
+ goto maybe_fold;
+
+default:
+ gcc_assert (gcc::jit::active_playback_ctxt);
+ gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
+ fprintf (stderr, "input expression:\n");
+ debug_tree (expr);
+ fprintf (stderr, "requested type:\n");
+ debug_tree (dst_type);
+ return error_mark_node;
+
+maybe_fold:
+ if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
+ t_ret = fold (t_ret);
+ return t_ret;
+}
}
namespace gcc {
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 632ab8cfb2e..a1481699b16 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -98,6 +98,13 @@
#undef create_code
#undef verify_code
+/* test-cast.c */
+#define create_code create_code_cast
+#define verify_code verify_code_cast
+#include "test-cast.c"
+#undef create_code
+#undef verify_code
+
/* test-compound-assignment.c */
#define create_code create_code_compound_assignment
#define verify_code verify_code_compound_assignment
@@ -354,6 +361,9 @@ const struct testcase testcases[] = {
{"calling_internal_function",
create_code_calling_internal_function,
verify_code_calling_internal_function},
+ {"cast",
+ create_code_cast,
+ verify_code_cast},
{"compound_assignment",
create_code_compound_assignment,
verify_code_compound_assignment},
diff --git a/gcc/testsuite/jit.dg/test-cast.c b/gcc/testsuite/jit.dg/test-cast.c
new file mode 100644
index 000..2b1e385ae40
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-cast.c
@@ -0,0 +1,66 @@
+#include
+#include
+#include
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+char
+my_casts (int x)
+{
+ return (char)(long) x;
+}
+ */
+ gcc_jit_type *int_type =
+gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_type *long_type =
+gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG);
+
[PATCH] aarch64: Delete duplicated option docs.
Noticed while reviewing the RISC-V -mstack-protector-guard docs. The
AArch64 section has two identical copies of the docs for this option.
* doc/invoke.texi (AArch64 Options): Delete duplicate
-mstack-protector-guard docs.
---
gcc/doc/invoke.texi | 18 --
1 file changed, 18 deletions(-)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 09bcc5b0f78..2d5803a781b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -17126,24 +17126,6 @@ and from what offset from that base register. There is
no default
register or offset as this is entirely for use within the Linux
kernel.
-@item -mstack-protector-guard=@var{guard}
-@itemx -mstack-protector-guard-reg=@var{reg}
-@itemx -mstack-protector-guard-offset=@var{offset}
-@opindex mstack-protector-guard
-@opindex mstack-protector-guard-reg
-@opindex mstack-protector-guard-offset
-Generate stack protection code using canary at @var{guard}. Supported
-locations are @samp{global} for a global canary or @samp{sysreg} for a
-canary in an appropriate system register.
-
-With the latter choice the options
-@option{-mstack-protector-guard-reg=@var{reg}} and
-@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
-which system register to use as base register for reading the canary,
-and from what offset from that base register. There is no default
-register or offset as this is entirely for use within the Linux
-kernel.
-
@item -mtls-dialect=desc
@opindex mtls-dialect=desc
Use TLS descriptors as the thread-local storage mechanism for dynamic accesses
--
2.17.1
RE: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>-Original Message-
>From: Gcc-patches [mailto:gcc-patches-boun...@gcc.gnu.org] On Behalf
>Of Tom de Vries
>Sent: Friday, July 3, 2020 6:52 AM
>To: Moore, Catherine ; Burnus, Tobias
>; gcc-patches ;
>Jakub Jelinek
>Cc: Schwinge, Thomas ; Stubbs,
>Andrew
>Subject: Re: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>
>On 6/26/20 9:35 PM, Moore, Catherine wrote:
>> Hi Tom,
>>
>> It doesn't look like you were explicitly cc'd on this patch and probably
>haven't seen it. Would you mind taking a look and approving the nvptx
>portions?
>>
>
>[ thanks for the ping, I think was explicitly cc-ed though. I'm usually
>not too fast in reviewing, and this time a vacation added to that. ]
>
>The patch looks good to me.
Thanks for the review.
Catherine
>
>I tried out the patch with one test-case and -pie -fPIC/-fpic already
>seems to works, so perhaps we could have at least one test-case
>exercising this in libgomp? That sounds easier to do than the
>shared-lib test-case.
>
>I think it's a good idea though to do fPIE/fpie as well, either in this
>patch or as follow-up.
>
>Thanks,
>- Tom
>
>> Thanks,
>> Catherine
>>
>>> -Original Message-
>>> From: Gcc-patches [mailto:gcc-patches-boun...@gcc.gnu.org] On
>Behalf
>>> Of Burnus, Tobias
>>> Sent: Tuesday, June 23, 2020 11:21 AM
>>> To: gcc-patches ; Jakub Jelinek
>>>
>>> Cc: Stubbs, Andrew ; Schwinge,
>Thomas
>>>
>>> Subject: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>>>
>>> If the offloading code is (only) in a library, one can come up
>>> with the idea to build those parts as shared library – and link
>>> it to the nonoffloading code.(*)
>>>
>>> Currently, this fails as the mkoffload calls the nonoffloading
>>> compiler without the -fpic/-fPIC flags, even though the compiler
>>> was originally invoked with those options. – And at some point,
>>> the linker then complains.
>>>
>>> This patch simply adds -fpic/-fPIC to the calls to the nonoffloading
>>> ("host") compiler, invoked from mkoffload, if they were present before.
>>>
>>> For the testcase at hand, this works with both AMDGCN and nvptx
>>> with the attached patch.
>>>
>>> OK for the trunk?
>>>
>>> Tobias
>>>
>>> PS: I think as mid-/longterm project it would be nice to test this
>>> in the testsuite, but that's unfortunately a larger task.
>>>
>>> (*) Thomas mentioned that this is supposed to work also in more
>>> complex cases than the one I outlined, although, that is probably
>>> currently the most common one.
>>>
>>> -
>>> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634
>München /
>>> Germany
>>> Registergericht München HRB 106955, Geschäftsführer: Thomas
>Heurung,
>>> Alexander Walter
[PATCH] nvptx: Support floating point reciprocal instructions.
The following patch addds support for PTX's rcp.rn.f32 and rcp.rn.f64
instructions. Note that the "rcp.rn" forms of this instruction
calculate the fully IEEE compliant result for the reciprocal, unlike
the rcp.approx variants that just provide fast approximations.
I'm undecided as to whether to prefix this instruction name with
"*" as this pattern is almost standard, so I can imagine the middle-end
potentially adding optab support for recip2 expansion at some
point. I'll go with the (backend) reviewer's recommendation?
This patch has been tested on nvptx-none hosted on x86_64-pc-linux-gnu
with "make" and "make check" with no new regressions.
Ok for mainline?
2020-07-12 Roger Sayle
gcc/ChangeLog
* config/nvptx/nvptx.md (recip2): New instruction.
gcc/testsuite/ChangeLog
* gcc.target/nvptx/recip-1.c: New test.
Thanks in advance,
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index 6545b81..ea6fc94 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -872,6 +872,15 @@
""
"%.\\tfma%#%t0\\t%0, %1, %2, %3;")
+(define_insn "recip2"
+ [(set (match_operand:SDFM 0 "nvptx_register_operand" "=R")
+ (div:SDFM
+ (match_operand:SDFM 2 "const_double_operand" "F")
+ (match_operand:SDFM 1 "nvptx_register_operand" "R")))]
+ "CONST_DOUBLE_P (operands[2])
+ && real_identical (CONST_DOUBLE_REAL_VALUE (operands[2]), &dconst1)"
+ "%.\\trcp%#%t0\\t%0, %1;")
+
(define_insn "div3"
[(set (match_operand:SDFM 0 "nvptx_register_operand" "=R")
(div:SDFM (match_operand:SDFM 1 "nvptx_register_operand" "R")
diff --git a/gcc/testsuite/gcc.target/nvptx/recip-1.c
b/gcc/testsuite/gcc.target/nvptx/recip-1.c
new file mode 100644
index 000..9a165f1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/nvptx/recip-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+double foo(double x)
+{
+ return 1.0/x;
+}
+
+float foof(float x)
+{
+ return 1.0f/x;
+}
+
+/* { dg-final { scan-assembler-times "rcp.rn.f64" 1 } } */
+/* { dg-final { scan-assembler-times "rcp.rn.f32" 1 } } */
+
PING [PATCH] x86-64: Define ASM_OUTPUT_ALIGNED_DECL_LOCAL
On Fri, Jun 26, 2020 at 2:02 PM H.J. Lu wrote: > > Define ASM_OUTPUT_ALIGNED_DECL_LOCAL for large local common symbol. > > gcc/ChangeLog: > > * config/i386/x86-64.h (ASM_OUTPUT_ALIGNED_DECL_LOCAL): New. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/pr95620.c: New test. PING: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549001.html -- H.J.
Re: [PATCH] x86: Require Linux target for PR target/93492 tests
Hi H.J., > Since -fpatchable-function-entry is only supported on Linux and used by > Linux kernel, require Linux target for PR target/93492 tests. > > PR target/93492 > * gcc.target/i386/pr93492-1.c: Require Linux target. > * gcc.target/i386/pr93492-2.c: Likewise. > * gcc.target/i386/pr93492-3.c: Likewise. > * gcc.target/i386/pr93492-4.c: Likewise. > * gcc.target/i386/pr93492-5.c: Likewise. Ok. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
