diff mbox series

[committed] Fix length computation for movsi_insv on bfin

Message ID befaa4c5e3aed1444bbc4b561e2f9f2a56a9d1f6.camel@redhat.com
State New
Headers show
Series [committed] Fix length computation for movsi_insv on bfin | expand

Commit Message

Li, Pan2 via Gcc-patches March 11, 2020, 4:18 a.m. UTC 
The tester started spitting out these errors on bfin recently:

> Tests that now fail, but worked before (3 tests):
> 
> bfin-sim: c-c++-common/torture/vector-compare-1.c   -Os  (test for excess
> errors)
> bfin-sim: c-c++-common/torture/vector-compare-1.c   -Os  (test for excess
> errors)
> bfin-sim: gcc.c-torture/execute/scal-to-vec1.c   -Os  (test for excess errors)
> 

http://gcc.gnu.org/jenkins/job/bfin-elf/865/console

This can be clearly seen in the assembly files:

>  27 ???? 40E10100              R0.H = 1;       // 75   [c=4 l=2]  *movsi_insv/1
>  147 ???? 40E10200              R0.H = 2;       // 276  [c=4
> l=2]  *movsi_insv/1
>  296 ???? 40E10400              R0.H = 4;       // 555  [c=4
> l=2]  *movsi_insv/1
>  327 ???? 40E10100              R0.H = 1;       // 615  [c=4
> l=2]  *movsi_insv/1
>  461 ???? 40E10100              R0.H = 1;       // 834  [c=4
> l=2]  *movsi_insv/1
>  463 ???? 43E10100              R3.H = 1;       // 839  [c=4
> l=2]  *movsi_insv/1
>  465 ???? 42E10100              R2.H = 1;       // 844  [c=4
> l=2]  *movsi_insv/1
>  467 ???? 41E10100              R1.H = 1;       // 849  [c=4
> l=2]  *movsi_insv/1

According to comments in bfin.md insns with the type "mvi" should provide length
information directly rather than using the default length computation.  The
movsi_insv pattern fails to do that and the default computation gets it wrong
resulting in an out of range branch error from the assembler.

This patch adds an explicit length attribute to the movsi_insv insn.  For
alternative zero, we use the default computation.  For alternative one, the mvi
alternative we use a constant length of 4 which seems to make everything happy
again.

We can see the effect of fixing that in build #868 where I made the attached
patch available to the tester:

http://gcc.gnu.org/jenkins/job/bfin-elf/868/console

You'll note it longer complains about vector-compare-1.c at all nor does it
complain about scal-to-vec1 -Os.  Additionally scal-to-vec1.c now passes at -O1.


Committing to the trunk.

Jeff
commit 5115542a5cc17c5096e6e498c363e75d5bc14276
Author: Jeff Law <law@redhat.com>
Date:   Tue Mar 10 22:16:19 2020 -0600

    Fix length computation for movsi_insv which resulted in regressions due to out of range branches on the bfin port.
    
            * config/bfin/bfin.md (movsi_insv): Add length attribute.
diff mbox series

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 5b67b79745f..887a55097db 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@ 
+2020-03-10  Jeff Law  <law@redhat.com>
+
+	* config/bfin/bfin.md (movsi_insv): Add length attribute.
+
 2020-03-10  Jiufu Guo  <guojiufu@linux.ibm.com>
 
 	PR target/93709
diff --git a/gcc/config/bfin/bfin.md b/gcc/config/bfin/bfin.md
index bb71a377772..aecb8138181 100644
--- a/gcc/config/bfin/bfin.md
+++ b/gcc/config/bfin/bfin.md
@@ -752,7 +752,8 @@ 
   "@
    %d0 = %h1 << 0%!
    %d0 = %1;"
-  [(set_attr "type" "dsp32shiftimm,mvi")])
+  [(set_attr "type" "dsp32shiftimm,mvi")
+   (set_attr "length" "*,4")])
 
 (define_expand "insv"
   [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "")