diff mbox

[v2,1/2,i386] cmpstrnsi needs string length

Message ID 1479324140.6279.30.camel@linux.vnet.ibm.com
State New
Headers show

Commit Message

Aaron Sawdey Nov. 16, 2016, 7:22 p.m. UTC
This patch adds a test to the cmpstrnsi pattern in i386.md so that it
will bail out (FAIL) if neither of the strings is a constant string. It
can only work as a proper strncmp if the length is not longer than both
of the strings. This change is required if expand_builtin_strncmp is
going to try expansion of strncmp when neither string argument is
constant. I've also changed the pattern to indicate that operand 3 may
be clobbered (if it happens to be in cx already).

2016-11-16  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>

	* config/i386/i386.md (cmpstrnsi): New test to bail out if neither
	string input is a string constant.  Clobber length argument.


-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

Comments

Bernd Schmidt Nov. 17, 2016, 11:09 a.m. UTC | #1
On 11/16/2016 08:22 PM, Aaron Sawdey wrote:
> I've also changed the pattern to indicate that operand 3 may

> be clobbered (if it happens to be in cx already).


That part looks like a meaningless change since the pattern in that 
define_expand is never used (there's a DONE at the end).

I don't think this is really a concern anyway (the pattern checks for CX 
in fixed_regs) and I suspect we wouldn't get a hard register in this 
expander in other cases.

Other than that it looks OK to me, please wait a day or two to see if 
Uros has objections.


Bernd
Uros Bizjak Nov. 17, 2016, 11:11 a.m. UTC | #2
On Wed, Nov 16, 2016 at 8:22 PM, Aaron Sawdey
<acsawdey@linux.vnet.ibm.com> wrote:
> This patch adds a test to the cmpstrnsi pattern in i386.md so that it

> will bail out (FAIL) if neither of the strings is a constant string. It

> can only work as a proper strncmp if the length is not longer than both

> of the strings. This change is required if expand_builtin_strncmp is

> going to try expansion of strncmp when neither string argument is

> constant. I've also changed the pattern to indicate that operand 3 may

> be clobbered (if it happens to be in cx already).

>

> 2016-11-16  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>

>

>         * config/i386/i386.md (cmpstrnsi): New test to bail out if neither

>         string input is a string constant.  Clobber length argument.


Please leave out use -> clobber change. We never reach pattern
generation, and expanders only look at the predicate of operand 3.

OK witth the above change.

Uros.
diff mbox

Patch

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 242428)
+++ gcc/config/i386/i386.md	(working copy)
@@ -16898,7 +16898,7 @@ 
   [(set (match_operand:SI 0 "register_operand")
 	(compare:SI (match_operand:BLK 1 "general_operand")
 		    (match_operand:BLK 2 "general_operand")))
-   (use (match_operand 3 "general_operand"))
+   (clobber (match_operand 3 "general_operand"))
    (use (match_operand 4 "immediate_operand"))]
   ""
 {
@@ -16911,6 +16911,21 @@ 
   if (fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
     FAIL;
 
+  /* One of the strings must be a constant.  If so, expand_builtin_strncmp()
+     will have rewritten the length arg to be the minimum of the const string
+     length and the actual length arg.  If both strings are the same and
+     shorter than the length arg, repz cmpsb will not stop at the 0 byte and
+     will incorrectly base the results on chars past the 0 byte.  */
+  tree t1 = MEM_EXPR (operands[1]);
+  tree t2 = MEM_EXPR (operands[2]);
+  if (!((t1 && TREE_CODE (t1) == MEM_REF
+         && TREE_CODE (TREE_OPERAND (t1, 0)) == ADDR_EXPR
+         && TREE_CODE (TREE_OPERAND (TREE_OPERAND (t1, 0), 0)) == STRING_CST)
+      || (t2 && TREE_CODE (t2) == MEM_REF
+          && TREE_CODE (TREE_OPERAND (t2, 0)) == ADDR_EXPR
+          && TREE_CODE (TREE_OPERAND (TREE_OPERAND (t2, 0), 0)) == STRING_CST)))
+    FAIL;
+
   out = operands[0];
   if (!REG_P (out))
     out = gen_reg_rtx (SImode);