diff mbox series

[committed,target/97040] Avoid using predefined insn name for instruction with different semantics

Message ID 9429e5eb-086a-baaf-f984-594cba8cc3c4@gmail.com
State New
Headers show
Series [committed,target/97040] Avoid using predefined insn name for instruction with different semantics | expand

Commit Message

Jeff Law Feb. 9, 2022, 7:14 p.m. UTC
This isn't technically a regression, but it only impacts the v850 target 
and fixes a long standing code correctness issue.

As outlined in slightly more detail in the PR, the v850 is using the 
pattern name "fnmasf4" and "fnmssf4" to generate fnmaf.s and fnmsf.s 
instructions respectively.

Unfortunately fnmasf4 is expected to produce (-a * b) + c and fnmssf4 
(-a * b) - c.  Those v850 instructions actually negate the entire result.

The fix is trivial.  Use a different pattern name so that the combiner 
can still generate those instructions, but prevent those instructions 
from being used to implement GCC's notion of what fnmas and fnmss should be.

This fixes pr97040 as well as a handful of testsuite failures for the 
v3e5 multilib.

Committed to the trunk.

Jeff
commit eefec38c992e3622a69de9667e91f0cafbff03cc
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Wed Feb 9 14:10:53 2022 -0500

    Avoid using predefined insn name for instruction with different semantics
    
    This isn't technically a regression, but it only impacts the v850 target and
    fixes a long standing code correctness issue.
    
    As outlined in slightly more detail in the PR, the v850 is using the pattern
    name "fnmasf4" and "fnmssf4" to generate fnmaf.s and fnmsf.s instructions
     respectively.
    
    Unfortunately fnmasf4 is expected to produce (-a * b) + c and
    fnmssf4 (-a * b) - c.  Those v850 instructions actually negate the entire
    result.
    
    The fix is trivial.  Use a different pattern name so that the combiner can
    still generate those instructions, but prevent those instructions from being
    used to implement GCC's notion of what fnmas and fnmss should be.
    
    This fixes pr97040 as well as a handful of testsuite failures for the v3e5
    multilib.
    
    gcc/
            PR target/97040
            * config/v850/v850.md (*v850_fnmasf4): Renamed from fnmasf4.
            (*v850_fnmssf4): Renamed from fnmssf4
diff mbox series

Patch

diff --git a/gcc/config/v850/v850.md b/gcc/config/v850/v850.md
index ed51157691b..6ca31e3f43f 100644
--- a/gcc/config/v850/v850.md
+++ b/gcc/config/v850/v850.md
@@ -2601,7 +2601,12 @@ 
    (set_attr "type" "fpu")])
 
 ;;; negative-multiply-add
-(define_insn "fnmasf4"
+;; Note the name on this and the following insn were previously fnmasf4
+;; and fnmssf4.  Those names are known to the gimple->rtl expanders and
+;; must implement specific semantics (negating one of the inputs to the
+;; multiplication).  The v850 instructions actually negate the entire
+;; result.  Thus the names have been changed and hidden.
+(define_insn "*v850_fnmasf4"
   [(set (match_operand:SF                 0 "register_operand" "=r")
 	(neg:SF (fma:SF (match_operand:SF 1 "register_operand" "r")
 			(match_operand:SF 2 "register_operand" "r")
@@ -2612,7 +2617,7 @@ 
    (set_attr "type" "fpu")])
 
 ;; negative-multiply-subtract
-(define_insn "fnmssf4"
+(define_insn "*v850_fnmssf4"
   [(set (match_operand:SF                         0 "register_operand" "=r")
 	(neg:SF (fma:SF (match_operand:SF         1 "register_operand" "r")
 			(match_operand:SF         2 "register_operand" "r")