| Message ID | 1479324140.6279.30.camel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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);