Re: [committed] Add missing T register clobber for SH port

2020-03-26 Thread Jeff Law via Gcc-patches
On Thu, 2020-03-26 at 12:38 -0600, Jeff Law wrote:
> On Wed, 2020-03-25 at 23:10 -0400, Hans-Peter Nilsson wrote:
> > On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote:
> > 
> > The patch you sent, as well as what you committed as r10-7383,
> > was just a ChangeLog entry.
> > 
> > > Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
> > > progress (won't finish for ~12 hours).  No new test as it's covered by
> > > vector-
> > > compare-1 in the testsuite.
> > 
> > ...so I guess you've noticed by the time you read this. :]
> :-)
> 
> Here's the actual patch:
> 
> commit 48817fbd7616f086ac7bb1dd38b862f78762c9b8
> Author: Jeff Law 
> Date:   Wed Mar 25 14:33:08 2020 -0600
> 
> Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern
> clobbering T reg without expressing that in its RTL.
> 
> PR rtl-optimization/90275
> * config/sh/sh.md (mov_neg_si_t): Clobber the T register in 
> the
> pattern.
And sadly, the only thing this fixed was the regressions recently flagged by the
tester.  It didn't fix any of the pre-existing failures.  Sigh.

jeff
> 



Re: [committed] Add missing T register clobber for SH port

2020-03-26 Thread Jeff Law via Gcc-patches
On Wed, 2020-03-25 at 23:10 -0400, Hans-Peter Nilsson wrote:
> On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote:
> 
> The patch you sent, as well as what you committed as r10-7383,
> was just a ChangeLog entry.
> 
> > Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
> > progress (won't finish for ~12 hours).  No new test as it's covered by
> > vector-
> > compare-1 in the testsuite.
> 
> ...so I guess you've noticed by the time you read this. :]
:-)

Here's the actual patch:

commit 48817fbd7616f086ac7bb1dd38b862f78762c9b8
Author: Jeff Law 
Date:   Wed Mar 25 14:33:08 2020 -0600

Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern
clobbering T reg without expressing that in its RTL.

PR rtl-optimization/90275
* config/sh/sh.md (mov_neg_si_t): Clobber the T register in the
pattern.

diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
index 4a1797160cf..fc80278a395 100644
--- a/gcc/config/sh/sh.md
+++ b/gcc/config/sh/sh.md
@@ -8395,9 +8395,15 @@
 ;; Store (negated) T bit as all zeros or ones in a reg.
 ;; subcRn,Rn   ! Rn = Rn - Rn - T; T = T
 ;; not Rn,Rn   ! Rn = 0 - Rn
+;;
+;; Note the call to sh_split_treg_set_expr may clobber
+;; the T reg.  We must express this, even though it's
+;; not immediately obvious this pattern changes the
+;; T register.
 (define_insn_and_split "mov_neg_si_t"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-   (neg:SI (match_operand 1 "treg_set_expr")))]
+   (neg:SI (match_operand 1 "treg_set_expr")))
+   (clobber (reg:SI T_REG))]
   "TARGET_SH1"
 {
   gcc_assert (t_reg_operand (operands[1], VOIDmode));



Re: [committed] Add missing T register clobber for SH port

2020-03-25 Thread Hans-Peter Nilsson
On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote:

The patch you sent, as well as what you committed as r10-7383,
was just a ChangeLog entry.

> Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
> progress (won't finish for ~12 hours).  No new test as it's covered by vector-
> compare-1 in the testsuite.

...so I guess you've noticed by the time you read this. :]

brgds, H-P


[committed] Add missing T register clobber for SH port

2020-03-25 Thread Jeff Law via Gcc-patches

Whee, more fallout from the pr90275 changes.  Thankfully this time it's a target
issue.

The sh4/sh4eb ports are failing vector-compare-1 execution tests after the cse
changes.  They only run once a week, so this wasn't caught immediately and took
some time to reproduce and analyze.

Ultimately the problem is one define_insn_and_split which, when split, may
clobber the T register.  However, the define_insn_and_split does not show this 
in
the insn pattern.  This can lead to various passes assuming the previous value 
in
the T register is still valid when in fact, it's going to be clobbered.  The CSE
changes made this scenario more likely and it showed up as execution failures in
the sh4/sh4eb testsuite.

All the other places where we use sh_split_treg_set_expr have the appropriate
clobber of the T register.

Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
progress (won't finish for ~12 hours).  No new test as it's covered by vector-
compare-1 in the testsuite.

Jeff




commit eeb0c7c07133634eb5e98ba0348392684a763c95
Author: Jeff Law 
Date:   Wed Mar 25 14:12:32 2020 -0600

Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern clobbering 
T reg without expressing that in its RTL.

PR rtl-optimization/90275
* config/sh/sh.md (mov_neg_si_t): Clobber the T register in the
pattern.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 06b06ab68de..3ad7a7aae0d 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2020-03-25  Jeff Law  
+
+   PR rtl-optimization/90275
+   * config/sh/sh.md (mov_neg_si_t): Clobber the T register in the
+   pattern.
+
 2020-03-25  Jakub Jelinek  
 
PR target/94292