[PATCH][PR tree-optimization/79090] Fix two minor DSE bugs
At one time I know I had the max_size == size test in
valid_ao_ref_for_dse. But it got lost at some point. This is what
caused the Ada failure.
Technically it'd be OK for the potentially dead store to have a variable
size as long as the later stores covered the entire range of the
potentially dead store. I doubt this happens enough to be worth checking.
The ppc64 big endian failures were more interesting. We had this in the IL:
memmove (dst, src, 0)
The trimming code assumes that there's at least one live byte in the
store, which obviously isn't the case here. The net result is we
compute an incorrect trim and the copy goes wild with incorrect
addresses and lengths. This is trivial to fix by validating that the
store has a nonzero length.
I was a bit curious how often this happened in practice because such a
call is trivially dead. ~80 during a bootstrap and a few dozen in the
testsuite. Given how trivial it is to detect and optimize, this patch
includes removal of such calls. This hunk makes the check for zero size
in valid_ao_ref_for_dse redundant, but I'd like to keep the check -- if
we add more builtin support without filtering zero size we'd regress again.
I wanted to get this in tonight given the Ada and ppc64 breakage so I
didn't do testcase reductions for the ppc test or the zero length
optimization (acats covers the Ada breakage). I'll get testcases done
Monday. I'll also finish generating suitable baselines for
ppc64-linux-gnu over the rest of the weekend to verify if this fixes all
the ppc64 regressions or just a subset of them.
Bootstrapped on x86_64-linux-gnu and ppc64-linux-gnu. Regression tested
on x86_64-linux-gnu. Also verified the ppc64 testresults are improved
(anything where sel-sched was faulting ought to be fixed now, maybe
others). Installing on the trunk.
Jeff
commit 04c5d9cab65e2f5b219f9610621bea06173b9cb8
Author: Jeff Law
Date: Sun Jan 15 01:37:32 2017 -0700
PR tree-optimization/79090
* tree-ssa-dse.c (valid_ao_ref_for_dse): Reject zero length and
variable length stores.
(compute_trims): Delete dead assignment to *trim_tail.
(dse_dom_walker::dse_optimize_stmt): Optimize mem* calls with
zero length.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 746f6af..36982c6 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2017-01-14 Jeff Law
+
+ PR tree-optimization/79090
+ * tree-ssa-dse.c (valid_ao_ref_for_dse): Reject zero length and
+ variable length stores.
+ (compute_trims): Delete dead assignment to *trim_tail.
+ (dse_dom_walker::dse_optimize_stmt): Optimize mem* calls with
+ zero length.
+
2017-01-14 Bernd Schmidt
PR rtl-optimization/78626
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 2e6f8ff..65dcb12 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -129,6 +129,8 @@ valid_ao_ref_for_dse (ao_ref *ref)
{
return (ao_ref_base (ref)
&& ref->max_size != -1
+ && ref->size != 0
+ && ref->max_size == ref->size
&& (ref->offset % BITS_PER_UNIT) == 0
&& (ref->size % BITS_PER_UNIT) == 0
&& (ref->size != -1));
@@ -221,7 +223,6 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head,
int *trim_tail)
the REF to compute the trims. */
/* Now identify how much, if any of the tail we can chop off. */
- *trim_tail = 0;
int last_orig = (ref->size / BITS_PER_UNIT) - 1;
int last_live = bitmap_last_set_bit (live);
*trim_tail = (last_orig - last_live) & ~0x1;
@@ -700,6 +701,16 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator
*gsi)
case BUILT_IN_MEMMOVE:
case BUILT_IN_MEMSET:
{
+ /* Occasionally calls with an explicit length of zero
+show up in the IL. It's pointless to do analysis
+on them, they're trivially dead. */
+ tree size = gimple_call_arg (stmt, 2);
+ if (integer_zerop (size))
+ {
+ delete_dead_call (gsi);
+ return;
+ }
+
gimple *use_stmt;
enum dse_store_status store_status;
m_byte_tracking_enabled
Re: libgo patch committed: Update to Go1.8rc1
In file included from ../../../libgo/runtime/runtime.h:113:0,
from ../../../libgo/runtime/go-libmain.c:15:
./runtime.inc:650:8: error: redefinition of 'struct siginfo'
struct siginfo {
^~~
In file included from /usr/include/signal.h:79:0,
from ../../../libgo/runtime/runtime.h:9,
from ../../../libgo/runtime/go-libmain.c:15:
/usr/include/bits/siginfo.h:46:16: note: originally defined here
typedef struct siginfo
^~~
In file included from ../../../libgo/runtime/runtime.h:113:0,
from ../../../libgo/runtime/go-main.c:17:
./runtime.inc:650:8: error: redefinition of 'struct siginfo'
struct siginfo {
^~~
In file included from /usr/include/signal.h:79:0,
from ../../../libgo/runtime/runtime.h:9,
from ../../../libgo/runtime/go-main.c:17:
/usr/include/bits/siginfo.h:46:16: note: originally defined here
typedef struct siginfo
^~~
In file included from ../../../libgo/runtime/runtime.h:113:0,
from ../../../libgo/runtime/aeshash.c:7:
./runtime.inc:650:8: error: redefinition of 'struct siginfo'
struct siginfo {
^~~
In file included from /usr/include/signal.h:79:0,
from ../../../libgo/runtime/runtime.h:9,
from ../../../libgo/runtime/aeshash.c:7:
/usr/include/bits/siginfo.h:46:16: note: originally defined here
typedef struct siginfo
^~~
make[4]: *** [libgobegin_a-go-main.o] Error 1
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
New Swedish PO file for 'gcc' (version 7.1-b20170101)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Swedish team of translators. The file is available at: http://translationproject.org/latest/gcc/sv.po (This file, 'gcc-7.1-b20170101.sv.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [v3 PATCH] PR libstdc++/78389
On Fri, Jan 13, 2017 at 9:26 AM, Ville Voutilainen
wrote:
> Update patch with splices for __carry added. Hopefully this resolves
> the remaining concerns that we had.
On rereading the patch today, the size calculation for merge() appears
to be backwards. [__first2, __last2) consists of the nodes not
transferred into *this, so the new size of __x should be __dist while
this->size() should be incremented by (__orig_size - __dist).
While the original test case in the patch passes (because __dist
happens to be 4 and __orig_size happens to be 8), I see the following
minimally modified test case failing on Wandbox:
std::list a{1, 2, 3, 4};
std::list b{5, 6, 7, 8, 9, 10, 11, 12};
try {
a.merge(b, ThrowingComparator{4});
} catch (...) {
}
assert(a.size() == std::distance(a.begin(), a.end()) && b.size() ==
std::distance(b.begin(), b.end()));
Sorry for not spotting this earlier.
Re: [v3 PATCH] PR libstdc++/78389
On 15 January 2017 at 18:42, Tim Song wrote: > On rereading the patch today, the size calculation for merge() appears > to be backwards. [__first2, __last2) consists of the nodes not > transferred into *this, so the new size of __x should be __dist while > this->size() should be incremented by (__orig_size - __dist). Ah, yes, I'm an idiot. Fixing...
Re: [v3 PATCH] PR libstdc++/78389
On 15 January 2017 at 19:01, Ville Voutilainen
wrote:
> On 15 January 2017 at 18:42, Tim Song wrote:
>> On rereading the patch today, the size calculation for merge() appears
>> to be backwards. [__first2, __last2) consists of the nodes not
>> transferred into *this, so the new size of __x should be __dist while
>> this->size() should be incremented by (__orig_size - __dist).
>
> Ah, yes, I'm an idiot. Fixing...
2017-01-15 Ville Voutilainen
PR libstdc++/78389
Fix backwards size adjustments.
* include/bits/list.tcc (merge(list&&)):
Fix backwards size adjustments.
(merge(list&&, _StrictWeakOrdering)): Likewise.
* testsuite/23_containers/list/operations/78389.cc: Add
better test for the sizes.
diff --git a/libstdc++-v3/include/bits/list.tcc
b/libstdc++-v3/include/bits/list.tcc
index 5be49a8..d80d569 100644
--- a/libstdc++-v3/include/bits/list.tcc
+++ b/libstdc++-v3/include/bits/list.tcc
@@ -406,8 +406,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__catch(...)
{
size_t __dist = std::distance(__first2, __last2);
- this->_M_inc_size(__dist);
- __x._M_set_size(__orig_size - __dist);
+ this->_M_inc_size(__orig_size - __dist);
+ __x._M_set_size(__dist);
__throw_exception_again;
}
}
@@ -454,8 +454,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__catch(...)
{
size_t __dist = std::distance(__first2, __last2);
- this->_M_inc_size(__dist);
- __x._M_set_size(__orig_size - __dist);
+ this->_M_inc_size(__orig_size - __dist);
+ __x._M_set_size(__dist);
__throw_exception_again;
}
}
diff --git a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
index 1cf9b0c..3002ba6 100644
--- a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
@@ -61,6 +61,8 @@ int main()
} catch (...) {
}
VERIFY(a.size() == 8 && b.size() == 4);
+ VERIFY(a.size() == std::distance(a.begin(), a.end()) &&
+b.size() == std::distance(b.begin(), b.end()));
std::list ax{1, 2, 3, 4};
std::list bx{5, 6, 7, 8, 9, 10, 11, 12};
throw_after_X = 5;
@@ -69,6 +71,8 @@ int main()
} catch (...) {
}
VERIFY(ax.size() == 8 && bx.size() == 4);
+ VERIFY(ax.size() == std::distance(ax.begin(), ax.end()) &&
+bx.size() == std::distance(bx.begin(), bx.end()));
std::list ay{5, 6, 7, 8, 9, 10, 11, 12};
try {
ay.sort(ThrowingComparator{5});
Re: [v3 PATCH] PR libstdc++/78389
On 15 January 2017 at 19:07, Ville Voutilainen wrote: > On 15 January 2017 at 19:01, Ville Voutilainen > wrote: >> On 15 January 2017 at 18:42, Tim Song wrote: >>> On rereading the patch today, the size calculation for merge() appears >>> to be backwards. [__first2, __last2) consists of the nodes not >>> transferred into *this, so the new size of __x should be __dist while >>> this->size() should be incremented by (__orig_size - __dist). >> >> Ah, yes, I'm an idiot. Fixing... > > 2017-01-15 Ville Voutilainen > > PR libstdc++/78389 > Fix backwards size adjustments. > * include/bits/list.tcc (merge(list&&)): > Fix backwards size adjustments. > (merge(list&&, _StrictWeakOrdering)): Likewise. > * testsuite/23_containers/list/operations/78389.cc: Add > better test for the sizes. Hmm, and yeah, your test uses a different throw-after number, so I should change the tests to do the same. :)
Re: [v3 PATCH] PR libstdc++/78389
On 15 January 2017 at 19:22, Ville Voutilainen
wrote:
> Hmm, and yeah, your test uses a different throw-after number, so I
> should change the tests to do the same. :)
In other words, like in the attached patch.
diff --git a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
index 3002ba6..4d8b7d2 100644
--- a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
@@ -57,20 +57,18 @@ int main()
std::list a{1, 2, 3, 4};
std::list b{5, 6, 7, 8, 9, 10, 11, 12};
try {
-a.merge(b, ThrowingComparator{5});
+a.merge(b, ThrowingComparator{4});
} catch (...) {
}
- VERIFY(a.size() == 8 && b.size() == 4);
VERIFY(a.size() == std::distance(a.begin(), a.end()) &&
b.size() == std::distance(b.begin(), b.end()));
std::list ax{1, 2, 3, 4};
std::list bx{5, 6, 7, 8, 9, 10, 11, 12};
- throw_after_X = 5;
+ throw_after_X = 4;
try {
ax.merge(bx);
} catch (...) {
}
- VERIFY(ax.size() == 8 && bx.size() == 4);
VERIFY(ax.size() == std::distance(ax.begin(), ax.end()) &&
bx.size() == std::distance(bx.begin(), bx.end()));
std::list ay{5, 6, 7, 8, 9, 10, 11, 12};
Re: libgo patch committed: Update to Go1.8rc1
Andreas Schwab writes:
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/go-libmain.c:15:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/go-libmain.c:15:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/go-main.c:17:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/go-main.c:17:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/aeshash.c:7:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/aeshash.c:7:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> make[4]: *** [libgobegin_a-go-main.o] Error 1
Same on Solaris. However, before I even get there, I hit
make[4]: *** No rule to make target 'golang_org/x/net/lif/lif.lo', needed by
'libgo.la'. Stop.
Fixed like this:
diff --git a/libgo/Makefile.am b/libgo/Makefile.am
--- a/libgo/Makefile.am
+++ b/libgo/Makefile.am
@@ -1133,7 +1133,7 @@ if LIBGO_IS_BSD
$(eval $(call PACKAGE_template,golang_org/x/net/route))
golang_org_x_net_route_lo = \
- golang_org/x/net/route/route.lo
+ golang_org/x/net/route.lo
golang_org_x_net_route_check = \
golang_org/x/net/route/check
@@ -1146,7 +1146,7 @@ if LIBGO_IS_SOLARIS
$(eval $(call PACKAGE_template,golang_org/x/net/lif))
golang_org_x_net_lif_lo = \
- golang_org/x/net/lif/lif.lo
+ golang_org/x/net/lif.lo
golang_org_x_net_lif_check = \
golang_org/x/net/lif/check
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH, i386] Avoid call to recog in ix86_legitimate_combined_insn
Hello!
Attached patch removes now unneeded call to recog in
ix86_legitimate_combined_insn.
Thanks Alan for his guidance!
2017-01-15 Uros Bizjak
* config/i386/i386.c (ix86_legitimate_combined_insn): Do not
call recog here. Assert that INSN_CODE (insn) is non-negative.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 244472)
+++ config/i386/i386.c (working copy)
@@ -8125,15 +8125,16 @@ ix86_return_pops_args (tree fundecl, tree funtype,
static bool
ix86_legitimate_combined_insn (rtx_insn *insn)
{
+ int i;
+
/* Check operand constraints in case hard registers were propagated
into insn pattern. This check prevents combine pass from
generating insn patterns with invalid hard register operands.
These invalid insns can eventually confuse reload to error out
with a spill failure. See also PRs 46829 and 46843. */
- if ((INSN_CODE (insn) = recog (PATTERN (insn), insn, 0)) >= 0)
-{
- int i;
+ gcc_assert (INSN_CODE (insn) >= 0);
+
extract_insn (insn);
preprocess_constraints (insn);
@@ -8192,7 +8193,6 @@ ix86_legitimate_combined_insn (rtx_insn *insn)
if (!win)
return false;
}
-}
return true;
}
Re: [PATCH] Fix RTL sharing bug in loop-doloop.c (PR target/79080)
On Sat, Jan 14, 2017 at 09:16:11PM -0700, Jeff Law wrote: > > The force_operand on complex count expression in doloop_modify can invoke > > various expander routines that are assuming there is rtl unsharing after > > them (which is the case for expansion). When later optimizations invoke > > the expander (e.g. expand_mult in this case), they use > > unshare_all_rtl_in_chain on the sequence. The following patch does that for > > doloop. The count expression is already known not to be shared with > > anything else (we do copy_rtx on it first and then create new rtls around it > > if needed), so for that if it occurs just once in the sequence, we don't > > need to unshare it. For subexpression of condition I'm not sure, which is > > why I've forced unsharing even if it occurs just once and is not shareable > > part of the condition like REG. > > > > Bootstrapped/regtested on powerpc64-linux, ok for trunk? > > > > 2017-01-14 Jakub Jelinek > > > > PR target/79080 > > * loop-doloop.c (doloop_modify): Call unshare_all_rtl_in_chain on > > sequence. Formatting fixes. > > (doloop_optimize): Formatting fixes. > > > > * gcc.dg/pr79080.c: New test. > So do we have a more general problem here -- we use force_operand in all > kinds of contexts, particularly in the RTL loop optimizers. If > force_operand can introduce undesirable sharing, then isn't more code prone > to this problem? I think it is not as bad. I think the problem is only when you force_operand 1) after expansion 2) with complicated expression. I think if you force_operand just with something that appears in some insn, it is very unlikely it will create something significantly more complex. It is just the case where something constructs a complex RTX expression and then produces insns using the force_operand. In the doloop case, it is desc->niter_expr expression, where possibly several insns in the IL together are used to compute the niter_expr. Or another case could be when using content of REG_EQUAL note and trying to force_operand it. > So just to be clear, I'm OK with this change as-is, I just worry we have > other places that are structure sharing assumption problems waiting to > happen. We have rtl sharing verification, while it has been defunct for about 3 years, it is in effect again for some time already and not everything in the RTL are changed during the last 3 years, before that it had to pass that verification too. I think it is fine to just wait if similar RTL sharing issues are reported next time, it is hard to guess where it would be actually needed and where it would be just a waste of time. Jakub
Re: libgo patch committed: Update to Go1.8rc1
On Sun, Jan 15, 2017 at 10:08 AM, Rainer Orth wrote: > Andreas Schwab writes: > > Same on Solaris. However, before I even get there, I hit > > make[4]: *** No rule to make target 'golang_org/x/net/lif/lif.lo', needed by > 'libgo.la'. Stop. > > Fixed like this: Thanks. Committed. Ian
Re: libgo patch committed: Update to Go1.8rc1
On Sun, Jan 15, 2017 at 3:12 AM, Andreas Schwab wrote:
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/go-libmain.c:15:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/go-libmain.c:15:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/go-main.c:17:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/go-main.c:17:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> In file included from ../../../libgo/runtime/runtime.h:113:0,
> from ../../../libgo/runtime/aeshash.c:7:
> ./runtime.inc:650:8: error: redefinition of 'struct siginfo'
> struct siginfo {
> ^~~
> In file included from /usr/include/signal.h:79:0,
> from ../../../libgo/runtime/runtime.h:9,
> from ../../../libgo/runtime/aeshash.c:7:
> /usr/include/bits/siginfo.h:46:16: note: originally defined here
> typedef struct siginfo
> ^~~
> make[4]: *** [libgobegin_a-go-main.o] Error 1
Thanks. I committed this patch, which I think should fix the problem.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 244483)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-a28b04adaeff8de61c336374db66b24ed96c30f0
+223cba75b947afc1ee5a13a60c15c66f6ff355c1
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/go/runtime/signal_gccgo.go
===
--- libgo/go/runtime/signal_gccgo.go(revision 244456)
+++ libgo/go/runtime/signal_gccgo.go(working copy)
@@ -46,15 +46,13 @@ func kill(pid _pid_t, sig uint32) int32
//extern setitimer
func setitimer(which int32, new *_itimerval, old *_itimerval) int32
-type siginfo _siginfo_t
-
type sigTabT struct {
flags int32
name string
}
type sigctxt struct {
- info *siginfo
+ info *_siginfo_t
ctxt unsafe.Pointer
}
@@ -128,9 +126,9 @@ func raiseproc(sig uint32) {
//go:nosplit
//go:nowritebarrierrec
-func sigfwd(fn uintptr, sig uint32, info *siginfo, ctx unsafe.Pointer) {
+func sigfwd(fn uintptr, sig uint32, info *_siginfo_t, ctx unsafe.Pointer) {
f1 := &[1]uintptr{fn}
- f2 := *(*func(uint32, *siginfo, unsafe.Pointer))(unsafe.Pointer(&f1))
+ f2 := *(*func(uint32, *_siginfo_t, unsafe.Pointer))(unsafe.Pointer(&f1))
f2(sig, info, ctx)
}
Index: libgo/go/runtime/signal_sighandler.go
===
--- libgo/go/runtime/signal_sighandler.go (revision 244456)
+++ libgo/go/runtime/signal_sighandler.go (working copy)
@@ -25,7 +25,7 @@ var crashing int32
// are not allowed.
//
//go:nowritebarrierrec
-func sighandler(sig uint32, info *siginfo, ctxt unsafe.Pointer, gp *g) {
+func sighandler(sig uint32, info *_siginfo_t, ctxt unsafe.Pointer, gp *g) {
_g_ := getg()
c := sigctxt{info, ctxt}
Index: libgo/go/runtime/signal_unix.go
===
--- libgo/go/runtime/signal_unix.go (revision 244456)
+++ libgo/go/runtime/signal_unix.go (working copy)
@@ -202,7 +202,7 @@ func sigpipe() {
// This is called by the signal handler, and the world may be stopped.
//go:nosplit
//go:nowritebarrierrec
-func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
+func sigtrampgo(sig uint32, info *_siginfo_t, ctx unsafe.Pointer) {
if sigfwdgo(sig, info, ctx) {
return
}
@@ -446,7 +446,7 @@ func badsignal(sig uintptr, c *sigctxt)
// This is called by the signal handler, and the world may be stopped.
//go:nosplit
//go:nowritebarrierrec
-func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
+func sigfwdgo(sig uint32, info *_siginfo_t, ctx unsafe.Pointer) bool {
if sig >= uint32(len(sigtable)) {
return false
}
Index: libgo/go/runtime/stubs.go
===
--- libgo/go/runtime/stubs.go (revision 244456)
+++ libgo/go/runtime/stubs.go (working copy)
@@ -485,10 +485,10 @@ func setSigactionHandler(*_sigaction
Re: [driver, doc] Support escaping special characters in specs
On 01/13/2017 05:59 AM, Rainer Orth wrote:
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma
@end table
+The switch matching text @code{S} in a %@{@code{S}@},
+%@{@code{S}:@code{X}@} or similar construct can use a backslash to
+ignore the special meaning of the character following it, thus allowing
+literal matching of a character that is otherwise specially treated.
+For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute
+@code{X} if the @option{-std=iso9899:1999} option were given.
+
I see this "%@{@code{..." markup appears in the paragraph just before
this, but it's wrong. The whole thing needs to be wrapped in @samp and
the nested @codes removed, like
s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/
etc.
I also suggest using the present tense here instead of the subjunctive...
s/would substitute/substitutes/
s/were given/is given/
-Sandra
Re: [PATCH v5] add -fprolog-pad=N,M option
On 01/13/2017 05:19 AM, Torsten Duwe wrote: Changes since v4: hopefully addressed all of Sandra's requests and suggestions concerning the documentation snippets, thanks for the feedback. If it still isn't clear, feel free to rephrase -- I'm a programmer, not a technical writer. Thanks, this makes more sense to me now. :-) -Sandra
Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
On Sat, Jan 14, 2017 at 09:19:52AM -0600, Segher Boessenkool wrote:
> On Sun, Jan 15, 2017 at 12:08:39AM +1030, Alan Modra wrote:
> > > Have you checked if/what this changes for some bigger code?
> >
> > Well, the bootstrap was all langs (minus ada due to not having ada
> > installed) and ppc32 multilibs were built. Plus the testsuite run
> > with RUNTESTFLAGS="--target_board=unix'{-m32,-m64}{-mlra,-mno-lra}'"
> >
> > I would have bootstrapped -m32 except the machine I used lacked 32-bit
> > gmp, mpfr, mpc and I was lazy.
> >
> > > Okay for trunk if there is nothing unexpected. Thanks!
> >
> > I guess I should at least build glibc.
>
> Yes exactly, something big that uses pic -- it is pretty obvious it won't
> change anything for non-pic.
glibc built fine without elf_high/elf_low with no regressions in its
testsuite. However, there is a problem in rs6000_emit_allocate_stack
-fstack-limit-symbol=SYMBOL code. This now might ICE if someone tries
to use the option with -fpic/PIC. I reckon the option combination to
be little used, so it shouldn't hurt to disable -fstack-limit-symbol
for PIC. (We were generating non-PIC for the trap, so we probably
would have gotten a complaint about text relocs in shared libraries.)
This revised patch has been bootstrapped and regression tested as
before, and tested with glibc too. OK?
PR target/79066
* config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
* config/rs6000/rs6000.c (rs6000_emit_allocate_stack): Don't allow
symbolic stack limit when pic.
testsuite/
* gcc.target/powerpc/pr79066.c: New.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 11394b2..2dd6bbe 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -27668,7 +27668,8 @@ rs6000_emit_allocate_stack (HOST_WIDE_INT size, rtx
copy_reg, int copy_off)
}
else if (GET_CODE (stack_limit_rtx) == SYMBOL_REF
&& TARGET_32BIT
- && DEFAULT_ABI == ABI_V4)
+ && DEFAULT_ABI == ABI_V4
+ && !flag_pic)
{
rtx toload = gen_rtx_CONST (VOIDmode,
gen_rtx_PLUS (Pmode,
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index b333474..f00334a 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10581,14 +10581,14 @@ (define_insn_and_split "*tocref"
(define_insn "elf_high"
[(set (match_operand:SI 0 "gpc_reg_operand" "=b*r")
(high:SI (match_operand 1 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"lis %0,%1@ha")
(define_insn "elf_low"
[(set (match_operand:SI 0 "gpc_reg_operand" "=r")
(lo_sum:SI (match_operand:SI 1 "gpc_reg_operand" "b")
(match_operand 2 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"la %0,%2@l(%1)")
;; Call and call_value insns
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79066.c
b/gcc/testsuite/gcc.target/powerpc/pr79066.c
new file mode 100644
index 000..86b2014
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79066.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { fpic && ilp32 } } } */
+/* { dg-options "-O2 -fpic" } */
+/* { dg-final { scan-assembler-not "lis.*@ha" } } */
+
+union U { double x; int i[2]; };
+
+double
+foo (double x)
+{
+ union U v;
+ v.i[0] = 0x7ff0;
+ v.i[1] = 0;
+ return x / v.x;
+}
--
Alan Modra
Australia Development Lab, IBM
[PATCH] PR79079 Fix __builtin_mul_overflow code gen for !TRULY_NOOP_TRUNCATION target
expand function on __builtin_mul_overflow is missing gen a truncdisi2 after muldi3 since it's just call gen_lowpart and it's not consider TRULY_NOOP_TRUNCATION == false, This patch has been run gcc testsuite for mips64, x86-64 and riscv64/riscv32, fix mips64 and riscv64 for builtin-arith-overflow-12.c/builtin-arith-overflow-p-12.c and no introduce new regression on x86-64 and riscv32 gcc/ChangeLog: 2017-01-16 Kito Cheng Kuan-Lin Chen PR target/PR79079 * gcc/internal-fn.c (expand_mul_overflow): Use convert_modes instead of gen_lowpart. From 7796f89dcfea0b583989a570d308ae5faf7213d1 Mon Sep 17 00:00:00 2001 From: Kito Cheng Date: Wed, 14 Sep 2016 15:06:28 +0800 Subject: [PATCH] Get the lowpart by convert_modes which checks TRULY_NOOP_TRUNCATION inside. --- gcc/internal-fn.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index c07b538..567cee4 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -1271,8 +1271,8 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0, tree arg1, res = expand_expr_real_2 (&ops, NULL_RTX, wmode, EXPAND_NORMAL); rtx hipart = expand_shift (RSHIFT_EXPR, wmode, res, prec, NULL_RTX, uns); - hipart = gen_lowpart (mode, hipart); - res = gen_lowpart (mode, res); + hipart = convert_modes (mode, wmode, hipart, uns); + res = convert_modes (mode, wmode, res, uns); if (uns) /* For the unsigned multiplication, there was overflow if HIPART is non-zero. */ @@ -1305,16 +1305,16 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0, tree arg1, unsigned int hprec = GET_MODE_PRECISION (hmode); rtx hipart0 = expand_shift (RSHIFT_EXPR, mode, op0, hprec, NULL_RTX, uns); - hipart0 = gen_lowpart (hmode, hipart0); - rtx lopart0 = gen_lowpart (hmode, op0); + hipart0 = convert_modes (hmode, mode, hipart0, uns); + rtx lopart0 = convert_modes (hmode, mode, op0, uns); rtx signbit0 = const0_rtx; if (!uns) signbit0 = expand_shift (RSHIFT_EXPR, hmode, lopart0, hprec - 1, NULL_RTX, 0); rtx hipart1 = expand_shift (RSHIFT_EXPR, mode, op1, hprec, NULL_RTX, uns); - hipart1 = gen_lowpart (hmode, hipart1); - rtx lopart1 = gen_lowpart (hmode, op1); + hipart1 = convert_modes (hmode, mode, hipart1, uns); + rtx lopart1 = convert_modes (hmode, mode, op1, uns); rtx signbit1 = const0_rtx; if (!uns) signbit1 = expand_shift (RSHIFT_EXPR, hmode, lopart1, hprec - 1, @@ -1505,11 +1505,12 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0, tree arg1, if (loxhi >> (bitsize / 2) == 0 (if uns). */ rtx hipartloxhi = expand_shift (RSHIFT_EXPR, mode, loxhi, hprec, NULL_RTX, 0); - hipartloxhi = gen_lowpart (hmode, hipartloxhi); + hipartloxhi = convert_modes (hmode, mode, hipartloxhi, uns); rtx signbitloxhi = const0_rtx; if (!uns) signbitloxhi = expand_shift (RSHIFT_EXPR, hmode, - gen_lowpart (hmode, loxhi), + convert_modes (hmode, mode, + loxhi, uns), hprec - 1, NULL_RTX, 0); do_compare_rtx_and_jump (signbitloxhi, hipartloxhi, NE, true, hmode, @@ -1519,7 +1520,8 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0, tree arg1, /* res = (loxhi << (bitsize / 2)) | (hmode) lo0xlo1; */ rtx loxhishifted = expand_shift (LSHIFT_EXPR, mode, loxhi, hprec, NULL_RTX, 1); - tem = convert_modes (mode, hmode, gen_lowpart (hmode, lo0xlo1), 1); + tem = convert_modes (mode, hmode, + convert_modes (hmode, mode, lo0xlo1, uns), 1); tem = expand_simple_binop (mode, IOR, loxhishifted, tem, res, 1, OPTAB_DIRECT); -- 2.7.4
Re: [PATCH] Fix RTL sharing bug in loop-doloop.c (PR target/79080)
On Sun, 15 Jan 2017, Jakub Jelinek wrote: > On Sat, Jan 14, 2017 at 09:16:11PM -0700, Jeff Law wrote: > > > The force_operand on complex count expression in doloop_modify can invoke > > > various expander routines that are assuming there is rtl unsharing after > > > them (which is the case for expansion). When later optimizations invoke > > > the expander (e.g. expand_mult in this case), they use > > > unshare_all_rtl_in_chain on the sequence. The following patch does that > > > for > > > doloop. The count expression is already known not to be shared with > > > anything else (we do copy_rtx on it first and then create new rtls around > > > it > > > if needed), so for that if it occurs just once in the sequence, we don't > > > need to unshare it. For subexpression of condition I'm not sure, which is > > > why I've forced unsharing even if it occurs just once and is not shareable > > > part of the condition like REG. > > > > > > Bootstrapped/regtested on powerpc64-linux, ok for trunk? > > > > > > 2017-01-14 Jakub Jelinek > > > > > > PR target/79080 > > > * loop-doloop.c (doloop_modify): Call unshare_all_rtl_in_chain on > > > sequence. Formatting fixes. > > > (doloop_optimize): Formatting fixes. > > > > > > * gcc.dg/pr79080.c: New test. > > So do we have a more general problem here -- we use force_operand in all > > kinds of contexts, particularly in the RTL loop optimizers. If > > force_operand can introduce undesirable sharing, then isn't more code prone > > to this problem? > > I think it is not as bad. I think the problem is only when you > force_operand 1) after expansion 2) with complicated expression. > I think if you force_operand just with something that appears in some insn, > it is very unlikely it will create something significantly more complex. > It is just the case where something constructs a complex RTX expression and > then produces insns using the force_operand. In the doloop case, it is > desc->niter_expr expression, where possibly several insns in the IL together > are used to compute the niter_expr. Or another case could be when using > content of REG_EQUAL note and trying to force_operand it. Maybe we want to have a force_operand_and_unshare () function then? Richard. > > So just to be clear, I'm OK with this change as-is, I just worry we have > > other places that are structure sharing assumption problems waiting to > > happen. > > We have rtl sharing verification, while it has been defunct for about 3 > years, it is in effect again for some time already and not everything in the > RTL are changed during the last 3 years, before that it had to pass that > verification too. I think it is fine to just wait if similar RTL sharing > issues are reported next time, it is hard to guess where it would be > actually needed and where it would be just a waste of time. > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] PR79079 Fix __builtin_mul_overflow code gen for !TRULY_NOOP_TRUNCATION target
On Mon, Jan 16, 2017 at 02:42:08PM +0800, Kito Cheng wrote: > 2017-01-16 Kito Cheng > Kuan-Lin Chen > > PR target/PR79079 > * gcc/internal-fn.c (expand_mul_overflow): Use convert_modes instead > of > gen_lowpart. No gcc/ prefix in gcc/ChangeLog. > @@ -1505,11 +1505,12 @@ expand_mul_overflow (location_t loc, tree lhs, tree > arg0, tree arg1, >if (loxhi >> (bitsize / 2) == 0 (if uns). */ > rtx hipartloxhi = expand_shift (RSHIFT_EXPR, mode, loxhi, hprec, > NULL_RTX, 0); > - hipartloxhi = gen_lowpart (hmode, hipartloxhi); > + hipartloxhi = convert_modes (hmode, mode, hipartloxhi, uns); While I think convert_modes on truncation should ignore the last argument, given that the corresponding expand_shift uses ins of 0, I think convert_modes should use 0 here too (the earlier convert_modes calls you've added are fine). > rtx signbitloxhi = const0_rtx; > if (!uns) > signbitloxhi = expand_shift (RSHIFT_EXPR, hmode, > - gen_lowpart (hmode, loxhi), > + convert_modes (hmode, mode, > + loxhi, uns), And here 0 as well (it is guarded by if (!uns) anyway, and it is what expand_shift will use too). >hprec - 1, NULL_RTX, 0); > > do_compare_rtx_and_jump (signbitloxhi, hipartloxhi, NE, true, hmode, > @@ -1519,7 +1520,8 @@ expand_mul_overflow (location_t loc, tree lhs, tree > arg0, tree arg1, > /* res = (loxhi << (bitsize / 2)) | (hmode) lo0xlo1; */ > rtx loxhishifted = expand_shift (LSHIFT_EXPR, mode, loxhi, hprec, > NULL_RTX, 1); > - tem = convert_modes (mode, hmode, gen_lowpart (hmode, lo0xlo1), 1); > + tem = convert_modes (mode, hmode, > +convert_modes (hmode, mode, lo0xlo1, uns), 1); And here 1. Ok for trunk with those changes. Jakub
Re: [PATCH] Fix RTL sharing bug in loop-doloop.c (PR target/79080)
On Mon, Jan 16, 2017 at 08:23:32AM +0100, Richard Biener wrote: > > I think it is not as bad. I think the problem is only when you > > force_operand 1) after expansion 2) with complicated expression. > > I think if you force_operand just with something that appears in some insn, > > it is very unlikely it will create something significantly more complex. > > It is just the case where something constructs a complex RTX expression and > > then produces insns using the force_operand. In the doloop case, it is > > desc->niter_expr expression, where possibly several insns in the IL together > > are used to compute the niter_expr. Or another case could be when using > > content of REG_EQUAL note and trying to force_operand it. > > Maybe we want to have a force_operand_and_unshare () function then? Not sure. 1) grepped around and this has been the only place that calls force_operand where the unsharing is known to be needed (perhaps so far). Other places use expand_simple_* (multiple), etc. directly 2) for the unsharing, it is good to know if the expression(s) you are forcing appear or may appear somewhere else in the IL or not. Such information wouldn't be known by force_operand_and_unshare, so it would always have to unshare everything, even if it occurs just once in the sequence. Jakub
