Re: [PATCH] rx: avoid adding setpsw for rx_cmpstrn when len is const

[Keith Packard]

> Were you able to actually trigger cases where operands[3] is a 
> CONST_INT?  Again, it's not entirely clear if that can actually happen 
> given the operand predicates on the cmpstrnsi expander.

(sorry for the long delay; I'm just getting back to reviewing my pending
GCC patches)

Good call. No, with the current predicates, you'll never get a constant
there. Looking at the only other machine implementing this pattern, sh,
they're using 'nonmemory_operand' instead of 'register_operand':

        (define_expand "cmpstrnsi"
          [(set (match_operand:SI 0 "register_operand")
                (compare:SI (match_operand:BLK 1 "memory_operand")
                            (match_operand:BLK 2 "memory_operand")))
           (use (match_operand:SI 3 "nonmemory_operand"))
           (use (match_operand:SI 4 "immediate_operand"))]
          "TARGET_SH1 && optimize"

Given that the first thing the rx pattern does is move the values into
the fixed registers, I changed the code with this patch:

diff --git a/gcc/config/rx/rx.md b/gcc/config/rx/rx.md
index 8c7974d69a5..13380f44d7d 100644
--- a/gcc/config/rx/rx.md
+++ b/gcc/config/rx/rx.md
@@ -2541,19 +2541,24 @@ (define_expand "cmpstrnsi"
        (unspec_volatile:SI [(match_operand:BLK 1 "memory_operand")     ;; 
String1
                             (match_operand:BLK 2 "memory_operand")]    ;; 
String2
                            UNSPEC_CMPSTRN))
-   (use (match_operand:SI                       3 "register_operand"))  ;; Max 
Length
+   (use (match_operand:SI                       3 "nonmemory_operand")) ;; Max 
Length
    (match_operand:SI                            4 "immediate_operand")] ;; 
Known Align
   "rx_allow_string_insns"
   {
     bool const_len = CONST_INT_P(operands[3]);
     if (const_len)
     {
+      fprintf(stderr, "Constant cmpstrnsi %ld\n", (long) INTVAL(operands[3]));
       if (INTVAL(operands[3]) == 0)
       {
         emit_move_insn (operands[0], operands[3]);
         DONE;
       }
     }
+    else
+    {
+      fprintf(stderr, "Non-constant cmpstrnsi\n");
+    }
 
     rtx str1 = gen_rtx_REG (SImode, 1);
     rtx str2 = gen_rtx_REG (SImode, 2);

With this change, building picolibc generates 304 Non-constant messages
and 160 Constant messages. And the test suite still passes, so it
appears to work as desired. Thanks so much for your review. I'll respin
this patch.

-- 
-keith

signature.asc
Description: PGP signature