| Message ID | 04aa4661-4fc6-310d-e05c-0ca6a561e477@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] Come up with memop_ret enum instead of int endp for memory operations. | expand |
On 11/23/18 8:06 AM, Martin Liška wrote: > Hi. > > It's patch proposal that suggests to use an enum instead of 'int endp' for > functions that expand memory move builtins. I've touch the code multiple times > and it always take me time to realize what the magic numbers 0, 1, 2 mean. > > Does the suggestion make sense? Do you like enum values? Can it be installed now? > If so I can finish the patch (few targets must be ported and ChangeLog entry is missing). I think an enum will make the code much more readable. I'm not sure the name choices are the most intuitive. Maybe something like this instead? RETURN_BEGIN, RETURN_END, RETURN_END_MINUS_1 (I suggest BEGIN rather than START because of C++ iterators, but I see the bigger readability gain in using RETURN over POINTER; IMO, it would be also nice to rename endp to retp or retmode or something like that). For the RETURN_END_MINUS_1 comment, I would just should say "return a pointer to the terminating null byte of the string," rather than "to the end of it." (The latter might seem to contradict RETURN_END). Martin
On 11/23/18 6:10 PM, Martin Sebor wrote: > On 11/23/18 8:06 AM, Martin Liška wrote: >> Hi. >> >> It's patch proposal that suggests to use an enum instead of 'int endp' for >> functions that expand memory move builtins. I've touch the code multiple times >> and it always take me time to realize what the magic numbers 0, 1, 2 mean. >> >> Does the suggestion make sense? Do you like enum values? Can it be installed now? >> If so I can finish the patch (few targets must be ported and ChangeLog entry is missing). > > I think an enum will make the code much more readable. > > I'm not sure the name choices are the most intuitive. Maybe > something like this instead? > > RETURN_BEGIN, RETURN_END, RETURN_END_MINUS_1 > > (I suggest BEGIN rather than START because of C++ iterators, but > I see the bigger readability gain in using RETURN over POINTER; > IMO, it would be also nice to rename endp to retp or retmode > or something like that). > > For the RETURN_END_MINUS_1 comment, I would just should say > "return a pointer to the terminating null byte of the string," > rather than "to the end of it." (The latter might seem to > contradict RETURN_END). > > Martin Thank you Martin for feedback. I'm sending tested version of the patch that survives bootstrap on x86_64-linux-gnu. Ready for trunk? Martin From 1709326b01079cfca299e2b57be18bd616954bf8 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 23 Nov 2018 15:51:05 +0100 Subject: [PATCH] Come up with memop_ret enum instead of int endp for memory operations. gcc/ChangeLog: 2018-11-26 Martin Liska <mliska@suse.cz> * asan.c (asan_emit_stack_protection): Use new enum values instead of int constants. * builtins.c (expand_builtin_memory_copy_args): Replace int type with memop_ret enum type. (expand_builtin_mempcpy_args): Likewise. (expand_builtin_memcpy): Use new enum values instead of int constants. Likewise. (expand_builtin_mempcpy): Likewise. (expand_movstr): Likewise. (expand_builtin_strcpy_args): Likewise. (expand_builtin_stpcpy_1): Likewise. (expand_builtin_strncpy): Likewise. (expand_builtin_memset_args): Likewise. * expr.c (move_by_pieces_d::finish_endp): Rename to ... (move_by_pieces_d::finish_retmode): ... this. (move_by_pieces): Change last argument type to memop_ret. (store_by_pieces): Use new enum values instead of int constants. (emit_block_move_hints): Likewise. (emit_push_insn): Likewise. (store_expr): Likewise. * expr.h (store_by_pieces): Change int to newly added enum type. * rtl.h (enum memop_ret): Define. (move_by_pieces): Use the enum type. --- gcc/asan.c | 2 +- gcc/builtins.c | 73 +++++++++++++++++++++++++------------------------- gcc/expr.c | 53 +++++++++++++++++------------------- gcc/expr.h | 2 +- gcc/rtl.h | 17 +++++++++++- 5 files changed, 79 insertions(+), 68 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index b2c41187b91..45906bf8fee 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1484,7 +1484,7 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, && can_store_by_pieces (sz, builtin_memset_read_str, &c, BITS_PER_UNIT, true)) store_by_pieces (shadow_mem, sz, builtin_memset_read_str, &c, - BITS_PER_UNIT, true, 0); + BITS_PER_UNIT, true, RETURN_BEGIN); else if (use_after_return_class >= 5 || !set_storage_via_setmem (shadow_mem, GEN_INT (sz), diff --git a/gcc/builtins.c b/gcc/builtins.c index ebde2db6e64..dcac49d8be1 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -126,10 +126,11 @@ static rtx builtin_memcpy_read_str (void *, HOST_WIDE_INT, scalar_int_mode); static rtx expand_builtin_memchr (tree, rtx); static rtx expand_builtin_memcpy (tree, rtx); static rtx expand_builtin_memory_copy_args (tree dest, tree src, tree len, - rtx target, tree exp, int endp); + rtx target, tree exp, + memop_ret retmode); static rtx expand_builtin_memmove (tree, rtx); static rtx expand_builtin_mempcpy (tree, rtx); -static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, int); +static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, memop_ret); static rtx expand_builtin_strcat (tree, rtx); static rtx expand_builtin_strcpy (tree, rtx); static rtx expand_builtin_strcpy_args (tree, tree, tree, rtx); @@ -3751,7 +3752,7 @@ expand_builtin_memcpy (tree exp, rtx target) check_memop_access (exp, dest, src, len); return expand_builtin_memory_copy_args (dest, src, len, target, exp, - /*endp=*/ 0); + /*retmode=*/ RETURN_BEGIN); } /* Check a call EXP to the memmove built-in for validity. @@ -3776,10 +3777,7 @@ expand_builtin_memmove (tree exp, rtx) /* Expand a call EXP to the mempcpy builtin. Return NULL_RTX if we failed; the caller should emit a normal call, otherwise try to get the result in TARGET, if convenient (and in - mode MODE if that's convenient). If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + mode MODE if that's convenient). */ static rtx expand_builtin_mempcpy (tree exp, rtx target) @@ -3812,20 +3810,17 @@ expand_builtin_mempcpy (tree exp, rtx target) return NULL_RTX; return expand_builtin_mempcpy_args (dest, src, len, - target, exp, /*endp=*/ 1); + target, exp, /*retmode=*/ RETURN_END); } /* Helper function to do the actual work for expand of memory copy family functions (memcpy, mempcpy, stpcpy). Expansing should assign LEN bytes - of memory from SRC to DEST and assign to TARGET if convenient. - If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + of memory from SRC to DEST and assign to TARGET if convenient. Return + value is based on RETMODE argument. */ static rtx expand_builtin_memory_copy_args (tree dest, tree src, tree len, - rtx target, tree exp, int endp) + rtx target, tree exp, memop_ret retmode) { const char *src_str; unsigned int src_align = get_pointer_alignment (src); @@ -3872,7 +3867,7 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, dest_mem = store_by_pieces (dest_mem, INTVAL (len_rtx), builtin_memcpy_read_str, CONST_CAST (char *, src_str), - dest_align, false, endp); + dest_align, false, retmode); dest_mem = force_operand (XEXP (dest_mem, 0), target); dest_mem = convert_memory_address (ptr_mode, dest_mem); return dest_mem; @@ -3883,9 +3878,10 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, /* Copy word part most expediently. */ enum block_op_methods method = BLOCK_OP_NORMAL; - if (CALL_EXPR_TAILCALL (exp) && (endp == 0 || target == const0_rtx)) + if (CALL_EXPR_TAILCALL (exp) + && (retmode == RETURN_BEGIN || target == const0_rtx)) method = BLOCK_OP_TAILCALL; - if (endp == 1 && target != const0_rtx) + if (retmode == RETURN_END && target != const0_rtx) method = BLOCK_OP_NO_LIBCALL_RET; dest_addr = emit_block_move_hints (dest_mem, src_mem, len_rtx, method, expected_align, expected_size, @@ -3899,11 +3895,11 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, dest_addr = convert_memory_address (ptr_mode, dest_addr); } - if (endp && target != const0_rtx) + if (retmode != RETURN_BEGIN && target != const0_rtx) { dest_addr = gen_rtx_PLUS (ptr_mode, dest_addr, len_rtx); /* stpcpy pointer to last byte. */ - if (endp == 2) + if (retmode == RETURN_END_MINUS_ONE) dest_addr = gen_rtx_MINUS (ptr_mode, dest_addr, const1_rtx); } @@ -3912,21 +3908,19 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, static rtx expand_builtin_mempcpy_args (tree dest, tree src, tree len, - rtx target, tree orig_exp, int endp) + rtx target, tree orig_exp, memop_ret retmode) { return expand_builtin_memory_copy_args (dest, src, len, target, orig_exp, - endp); + retmode); } /* Expand into a movstr instruction, if one is available. Return NULL_RTX if we failed, the caller should emit a normal call, otherwise try to - get the result in TARGET, if convenient. If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + get the result in TARGET, if convenient. + Return value is based on RETMODE argument. */ static rtx -expand_movstr (tree dest, tree src, rtx target, int endp) +expand_movstr (tree dest, tree src, rtx target, memop_ret retmode) { struct expand_operand ops[3]; rtx dest_mem; @@ -3937,25 +3931,25 @@ expand_movstr (tree dest, tree src, rtx target, int endp) dest_mem = get_memory_rtx (dest, NULL); src_mem = get_memory_rtx (src, NULL); - if (!endp) + if (retmode != RETURN_BEGIN) { target = force_reg (Pmode, XEXP (dest_mem, 0)); dest_mem = replace_equiv_address (dest_mem, target); } - create_output_operand (&ops[0], endp ? target : NULL_RTX, Pmode); + create_output_operand (&ops[0], retmode ? target : NULL_RTX, Pmode); create_fixed_operand (&ops[1], dest_mem); create_fixed_operand (&ops[2], src_mem); if (!maybe_expand_insn (targetm.code_for_movstr, 3, ops)) return NULL_RTX; - if (endp && target != const0_rtx) + if (retmode != RETURN_BEGIN && target != const0_rtx) { target = ops[0].value; /* movstr is supposed to set end to the address of the NUL terminator. If the caller requested a mempcpy-like return value, adjust it. */ - if (endp == 1) + if (retmode == RETURN_END) { rtx tem = plus_constant (GET_MODE (target), gen_lowpart (GET_MODE (target), target), 1); @@ -4044,7 +4038,7 @@ expand_builtin_strcpy_args (tree exp, tree dest, tree src, rtx target) return NULL_RTX; } - return expand_movstr (dest, src, target, /*endp=*/0); + return expand_movstr (dest, src, target, /*retmode=*/ RETURN_BEGIN); } /* Expand a call EXP to the stpcpy builtin. @@ -4091,14 +4085,16 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode) memset (&data, 0, sizeof (c_strlen_data)); if (!c_getstr (src, NULL) || !(len = c_strlen (src, 0, &data, 1))) - return expand_movstr (dst, src, target, /*endp=*/2); + return expand_movstr (dst, src, target, + /*retmode=*/ RETURN_END_MINUS_ONE); if (data.decl && !TREE_NO_WARNING (exp)) warn_string_no_nul (EXPR_LOCATION (exp), "stpcpy", src, data.decl); lenp1 = size_binop_loc (loc, PLUS_EXPR, len, ssize_int (1)); ret = expand_builtin_mempcpy_args (dst, src, lenp1, - target, exp, /*endp=*/2); + target, exp, + /*retmode=*/ RETURN_END_MINUS_ONE); if (ret) return ret; @@ -4132,7 +4128,8 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode) } } - return expand_movstr (dst, src, target, /*endp=*/2); + return expand_movstr (dst, src, target, + /*retmode=*/ RETURN_END_MINUS_ONE); } } @@ -4378,7 +4375,8 @@ expand_builtin_strncpy (tree exp, rtx target) dest_mem = get_memory_rtx (dest, len); store_by_pieces (dest_mem, tree_to_uhwi (len), builtin_strncpy_read_str, - CONST_CAST (char *, p), dest_align, false, 0); + CONST_CAST (char *, p), dest_align, false, + RETURN_BEGIN); dest_mem = force_operand (XEXP (dest_mem, 0), target); dest_mem = convert_memory_address (ptr_mode, dest_mem); return dest_mem; @@ -4523,7 +4521,7 @@ expand_builtin_memset_args (tree dest, tree val, tree len, val_rtx = force_reg (val_mode, val_rtx); store_by_pieces (dest_mem, tree_to_uhwi (len), builtin_memset_gen_str, val_rtx, dest_align, - true, 0); + true, RETURN_BEGIN); } else if (!set_storage_via_setmem (dest_mem, len_rtx, val_rtx, dest_align, expected_align, @@ -4546,7 +4544,8 @@ expand_builtin_memset_args (tree dest, tree val, tree len, builtin_memset_read_str, &c, dest_align, true)) store_by_pieces (dest_mem, tree_to_uhwi (len), - builtin_memset_read_str, &c, dest_align, true, 0); + builtin_memset_read_str, &c, dest_align, true, + RETURN_BEGIN); else if (!set_storage_via_setmem (dest_mem, len_rtx, gen_int_mode (c, val_mode), dest_align, expected_align, diff --git a/gcc/expr.c b/gcc/expr.c index 7ae3e37378c..ad0f95c90d0 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -1146,7 +1146,7 @@ class move_by_pieces_d : public op_by_pieces_d : op_by_pieces_d (to, false, from, true, NULL, NULL, len, align) { } - rtx finish_endp (int); + rtx finish_retmode (memop_ret); }; /* Return true if MODE can be used for a set of copies, given an @@ -1182,15 +1182,14 @@ move_by_pieces_d::generate (rtx op0, rtx op1, } /* Perform the final adjustment at the end of a string to obtain the - correct return value for the block operation. If ENDP is 1 return - memory at the end ala mempcpy, and if ENDP is 2 return memory the - end minus one byte ala stpcpy. */ + correct return value for the block operation. + Return value is based on RETMODE argument. */ rtx -move_by_pieces_d::finish_endp (int endp) +move_by_pieces_d::finish_retmode (memop_ret retmode) { gcc_assert (!m_reverse); - if (endp == 2) + if (retmode == RETURN_END_MINUS_ONE) { m_to.maybe_postinc (-1); --m_offset; @@ -1206,13 +1205,11 @@ move_by_pieces_d::finish_endp (int endp) ALIGN is maximum stack alignment we can assume. - If ENDP is 0 return to, if ENDP is 1 return memory at the end ala - mempcpy, and if ENDP is 2 return memory the end minus one byte ala - stpcpy. */ + Return value is based on RETMODE argument. */ rtx move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len, - unsigned int align, int endp) + unsigned int align, memop_ret retmode) { #ifndef PUSH_ROUNDING if (to == NULL) @@ -1223,8 +1220,8 @@ move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len, data.run (); - if (endp) - return data.finish_endp (endp); + if (retmode) + return data.finish_retmode (retmode); else return to; } @@ -1244,7 +1241,7 @@ class store_by_pieces_d : public op_by_pieces_d : op_by_pieces_d (to, false, NULL_RTX, true, cfn, cfn_data, len, align) { } - rtx finish_endp (int); + rtx finish_retmode (memop_ret); }; /* Return true if MODE can be used for a set of stores, given an @@ -1272,15 +1269,14 @@ store_by_pieces_d::generate (rtx op0, rtx op1, machine_mode) } /* Perform the final adjustment at the end of a string to obtain the - correct return value for the block operation. If ENDP is 1 return - memory at the end ala mempcpy, and if ENDP is 2 return memory the - end minus one byte ala stpcpy. */ + correct return value for the block operation. + Return value is based on RETMODE argument. */ rtx -store_by_pieces_d::finish_endp (int endp) +store_by_pieces_d::finish_retmode (memop_ret retmode) { gcc_assert (!m_reverse); - if (endp == 2) + if (retmode == RETURN_END_MINUS_ONE) { m_to.maybe_postinc (-1); --m_offset; @@ -1370,18 +1366,17 @@ can_store_by_pieces (unsigned HOST_WIDE_INT len, pointer which will be passed as argument in every CONSTFUN call. ALIGN is maximum alignment we can assume. MEMSETP is true if this is a memset operation and false if it's a copy of a constant string. - If ENDP is 0 return to, if ENDP is 1 return memory at the end ala - mempcpy, and if ENDP is 2 return memory the end minus one byte ala - stpcpy. */ + Return value is based on RETMODE argument. */ rtx store_by_pieces (rtx to, unsigned HOST_WIDE_INT len, rtx (*constfun) (void *, HOST_WIDE_INT, scalar_int_mode), - void *constfundata, unsigned int align, bool memsetp, int endp) + void *constfundata, unsigned int align, bool memsetp, + memop_ret retmode) { if (len == 0) { - gcc_assert (endp != 2); + gcc_assert (retmode != 2); return to; } @@ -1393,8 +1388,8 @@ store_by_pieces (rtx to, unsigned HOST_WIDE_INT len, store_by_pieces_d data (to, constfun, constfundata, len, align); data.run (); - if (endp) - return data.finish_endp (endp); + if (retmode) + return data.finish_retmode (retmode); else return to; } @@ -1624,7 +1619,7 @@ emit_block_move_hints (rtx x, rtx y, rtx size, enum block_op_methods method, } if (CONST_INT_P (size) && can_move_by_pieces (INTVAL (size), align)) - move_by_pieces (x, y, INTVAL (size), align, 0); + move_by_pieces (x, y, INTVAL (size), align, RETURN_BEGIN); else if (emit_block_move_via_movmem (x, y, size, align, expected_align, expected_size, min_size, max_size, probable_max_size)) @@ -4421,7 +4416,8 @@ emit_push_insn (rtx x, machine_mode mode, tree type, rtx size, && where_pad != stack_direction) anti_adjust_stack (gen_int_mode (extra, Pmode)); - move_by_pieces (NULL, xinner, INTVAL (size) - used, align, 0); + move_by_pieces (NULL, xinner, INTVAL (size) - used, align, + RETURN_BEGIN); } else #endif /* PUSH_ROUNDING */ @@ -5618,7 +5614,8 @@ store_expr (tree exp, rtx target, int call_param_p, CONST_CAST (char *, TREE_STRING_POINTER (str)), MEM_ALIGN (target), false, - exp_len > str_copy_len ? 1 : 0); + (exp_len > str_copy_len ? RETURN_END : + RETURN_BEGIN)); if (exp_len > str_copy_len) clear_storage (adjust_address (dest_mem, BLKmode, 0), GEN_INT (exp_len - str_copy_len), diff --git a/gcc/expr.h b/gcc/expr.h index 19028f0e6a4..6ae343d81f0 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -219,7 +219,7 @@ extern int can_store_by_pieces (unsigned HOST_WIDE_INT, MEMSETP is true if this is a real memset/bzero, not a copy. Returns TO + LEN. */ extern rtx store_by_pieces (rtx, unsigned HOST_WIDE_INT, by_pieces_constfn, - void *, unsigned int, bool, int); + void *, unsigned int, bool, memop_ret); /* Emit insns to set X from Y. */ extern rtx_insn *emit_move_insn (rtx, rtx); diff --git a/gcc/rtl.h b/gcc/rtl.h index dd3ce06295a..c2aaa9eff4b 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -4046,9 +4046,24 @@ extern void expand_null_return (void); extern void expand_naked_return (void); extern void emit_jump (rtx); +/* Memory operation built-ins differ by return value. Mapping + of the enum values is following: + - RETURN_BEGIN - return destination, e.g. memcpy + - RETURN_END - return destination + n, e.g. mempcpy + - RETURN_END_MINUS_ONE - return a pointer to the terminating + null byte of the string, e.g. strcpy +*/ + +enum memop_ret +{ + RETURN_BEGIN, + RETURN_END, + RETURN_END_MINUS_ONE +}; + /* In expr.c */ extern rtx move_by_pieces (rtx, rtx, unsigned HOST_WIDE_INT, - unsigned int, int); + unsigned int, memop_ret); extern poly_int64 find_args_size_adjust (rtx_insn *); extern poly_int64 fixup_args_size_notes (rtx_insn *, rtx_insn *, poly_int64);
On 11/26/18 7:03 AM, Martin Liška wrote: > On 11/23/18 6:10 PM, Martin Sebor wrote: >> On 11/23/18 8:06 AM, Martin Liška wrote: >>> Hi. >>> >>> It's patch proposal that suggests to use an enum instead of 'int endp' for >>> functions that expand memory move builtins. I've touch the code multiple times >>> and it always take me time to realize what the magic numbers 0, 1, 2 mean. >>> >>> Does the suggestion make sense? Do you like enum values? Can it be installed now? >>> If so I can finish the patch (few targets must be ported and ChangeLog entry is missing). >> I think an enum will make the code much more readable. >> >> I'm not sure the name choices are the most intuitive. Maybe >> something like this instead? >> >> RETURN_BEGIN, RETURN_END, RETURN_END_MINUS_1 >> >> (I suggest BEGIN rather than START because of C++ iterators, but >> I see the bigger readability gain in using RETURN over POINTER; >> IMO, it would be also nice to rename endp to retp or retmode >> or something like that). >> >> For the RETURN_END_MINUS_1 comment, I would just should say >> "return a pointer to the terminating null byte of the string," >> rather than "to the end of it." (The latter might seem to >> contradict RETURN_END). >> >> Martin > Thank you Martin for feedback. I'm sending tested version of the patch > that survives bootstrap on x86_64-linux-gnu. > > Ready for trunk? > Martin > > > 0001-Come-up-with-memop_ret-enum-instead-of-int-endp-for-.patch > > From 1709326b01079cfca299e2b57be18bd616954bf8 Mon Sep 17 00:00:00 2001 > From: marxin <mliska@suse.cz> > Date: Fri, 23 Nov 2018 15:51:05 +0100 > Subject: [PATCH] Come up with memop_ret enum instead of int endp for memory > operations. > > gcc/ChangeLog: > > 2018-11-26 Martin Liska <mliska@suse.cz> > > * asan.c (asan_emit_stack_protection): Use new enum values > instead of int constants. > * builtins.c (expand_builtin_memory_copy_args): Replace int > type with memop_ret enum type. > (expand_builtin_mempcpy_args): Likewise. > (expand_builtin_memcpy): Use new enum values > instead of int constants. Likewise. > (expand_builtin_mempcpy): Likewise. > (expand_movstr): Likewise. > (expand_builtin_strcpy_args): Likewise. > (expand_builtin_stpcpy_1): Likewise. > (expand_builtin_strncpy): Likewise. > (expand_builtin_memset_args): Likewise. > * expr.c (move_by_pieces_d::finish_endp): Rename to ... > (move_by_pieces_d::finish_retmode): ... this. > (move_by_pieces): Change last argument type to memop_ret. > (store_by_pieces): Use new enum values > instead of int constants. > (emit_block_move_hints): Likewise. > (emit_push_insn): Likewise. > (store_expr): Likewise. > * expr.h (store_by_pieces): Change int to newly added enum > type. > * rtl.h (enum memop_ret): Define. > (move_by_pieces): Use the enum type. > --- > gcc/asan.c | 2 +- > gcc/builtins.c | 73 +++++++++++++++++++++++++------------------------- > gcc/expr.c | 53 +++++++++++++++++------------------- > gcc/expr.h | 2 +- > gcc/rtl.h | 17 +++++++++++- > 5 files changed, 79 insertions(+), 68 deletions(-) > > rtx > store_by_pieces (rtx to, unsigned HOST_WIDE_INT len, > rtx (*constfun) (void *, HOST_WIDE_INT, scalar_int_mode), > - void *constfundata, unsigned int align, bool memsetp, int endp) > + void *constfundata, unsigned int align, bool memsetp, > + memop_ret retmode) > { > if (len == 0) > { > - gcc_assert (endp != 2); > + gcc_assert (retmode != 2); Don't you want this to be retmode != RETURN_END_MINUS_1 With that nit fixed, I think this is OK. jeff
diff --git a/gcc/asan.c b/gcc/asan.c index b2c41187b91..2abe628fd61 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1484,7 +1484,7 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, && can_store_by_pieces (sz, builtin_memset_read_str, &c, BITS_PER_UNIT, true)) store_by_pieces (shadow_mem, sz, builtin_memset_read_str, &c, - BITS_PER_UNIT, true, 0); + BITS_PER_UNIT, true, POINTER_START); else if (use_after_return_class >= 5 || !set_storage_via_setmem (shadow_mem, GEN_INT (sz), diff --git a/gcc/builtins.c b/gcc/builtins.c index ebde2db6e64..e38b4bd97cd 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -126,10 +126,11 @@ static rtx builtin_memcpy_read_str (void *, HOST_WIDE_INT, scalar_int_mode); static rtx expand_builtin_memchr (tree, rtx); static rtx expand_builtin_memcpy (tree, rtx); static rtx expand_builtin_memory_copy_args (tree dest, tree src, tree len, - rtx target, tree exp, int endp); + rtx target, tree exp, + memop_ret endp); static rtx expand_builtin_memmove (tree, rtx); static rtx expand_builtin_mempcpy (tree, rtx); -static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, int); +static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, memop_ret); static rtx expand_builtin_strcat (tree, rtx); static rtx expand_builtin_strcpy (tree, rtx); static rtx expand_builtin_strcpy_args (tree, tree, tree, rtx); @@ -3751,7 +3752,7 @@ expand_builtin_memcpy (tree exp, rtx target) check_memop_access (exp, dest, src, len); return expand_builtin_memory_copy_args (dest, src, len, target, exp, - /*endp=*/ 0); + /*endp=*/ POINTER_START); } /* Check a call EXP to the memmove built-in for validity. @@ -3776,10 +3777,7 @@ expand_builtin_memmove (tree exp, rtx) /* Expand a call EXP to the mempcpy builtin. Return NULL_RTX if we failed; the caller should emit a normal call, otherwise try to get the result in TARGET, if convenient (and in - mode MODE if that's convenient). If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + mode MODE if that's convenient). */ static rtx expand_builtin_mempcpy (tree exp, rtx target) @@ -3812,20 +3810,17 @@ expand_builtin_mempcpy (tree exp, rtx target) return NULL_RTX; return expand_builtin_mempcpy_args (dest, src, len, - target, exp, /*endp=*/ 1); + target, exp, /*endp=*/ POINTER_END); } /* Helper function to do the actual work for expand of memory copy family functions (memcpy, mempcpy, stpcpy). Expansing should assign LEN bytes - of memory from SRC to DEST and assign to TARGET if convenient. - If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + of memory from SRC to DEST and assign to TARGET if convenient. Return + value is based on ENDP argument. */ static rtx expand_builtin_memory_copy_args (tree dest, tree src, tree len, - rtx target, tree exp, int endp) + rtx target, tree exp, memop_ret endp) { const char *src_str; unsigned int src_align = get_pointer_alignment (src); @@ -3883,9 +3878,10 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, /* Copy word part most expediently. */ enum block_op_methods method = BLOCK_OP_NORMAL; - if (CALL_EXPR_TAILCALL (exp) && (endp == 0 || target == const0_rtx)) + if (CALL_EXPR_TAILCALL (exp) + && (endp == POINTER_START || target == const0_rtx)) method = BLOCK_OP_TAILCALL; - if (endp == 1 && target != const0_rtx) + if (endp == POINTER_END && target != const0_rtx) method = BLOCK_OP_NO_LIBCALL_RET; dest_addr = emit_block_move_hints (dest_mem, src_mem, len_rtx, method, expected_align, expected_size, @@ -3899,11 +3895,11 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, dest_addr = convert_memory_address (ptr_mode, dest_addr); } - if (endp && target != const0_rtx) + if (endp != POINTER_START && target != const0_rtx) { dest_addr = gen_rtx_PLUS (ptr_mode, dest_addr, len_rtx); /* stpcpy pointer to last byte. */ - if (endp == 2) + if (endp == POINTER_END_MINUS_ONE) dest_addr = gen_rtx_MINUS (ptr_mode, dest_addr, const1_rtx); } @@ -3912,7 +3908,7 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, static rtx expand_builtin_mempcpy_args (tree dest, tree src, tree len, - rtx target, tree orig_exp, int endp) + rtx target, tree orig_exp, memop_ret endp) { return expand_builtin_memory_copy_args (dest, src, len, target, orig_exp, endp); @@ -3920,13 +3916,11 @@ expand_builtin_mempcpy_args (tree dest, tree src, tree len, /* Expand into a movstr instruction, if one is available. Return NULL_RTX if we failed, the caller should emit a normal call, otherwise try to - get the result in TARGET, if convenient. If ENDP is 0 return the - destination pointer, if ENDP is 1 return the end pointer ala - mempcpy, and if ENDP is 2 return the end pointer minus one ala - stpcpy. */ + get the result in TARGET, if convenient. + Return value is based on ENDP argument. */ static rtx -expand_movstr (tree dest, tree src, rtx target, int endp) +expand_movstr (tree dest, tree src, rtx target, memop_ret endp) { struct expand_operand ops[3]; rtx dest_mem; @@ -3937,7 +3931,7 @@ expand_movstr (tree dest, tree src, rtx target, int endp) dest_mem = get_memory_rtx (dest, NULL); src_mem = get_memory_rtx (src, NULL); - if (!endp) + if (endp != POINTER_START) { target = force_reg (Pmode, XEXP (dest_mem, 0)); dest_mem = replace_equiv_address (dest_mem, target); @@ -3949,13 +3943,13 @@ expand_movstr (tree dest, tree src, rtx target, int endp) if (!maybe_expand_insn (targetm.code_for_movstr, 3, ops)) return NULL_RTX; - if (endp && target != const0_rtx) + if (endp != POINTER_START && target != const0_rtx) { target = ops[0].value; /* movstr is supposed to set end to the address of the NUL terminator. If the caller requested a mempcpy-like return value, adjust it. */ - if (endp == 1) + if (endp == POINTER_END) { rtx tem = plus_constant (GET_MODE (target), gen_lowpart (GET_MODE (target), target), 1); @@ -4044,7 +4038,7 @@ expand_builtin_strcpy_args (tree exp, tree dest, tree src, rtx target) return NULL_RTX; } - return expand_movstr (dest, src, target, /*endp=*/0); + return expand_movstr (dest, src, target, /*endp=*/ POINTER_START); } /* Expand a call EXP to the stpcpy builtin. @@ -4091,14 +4085,16 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode) memset (&data, 0, sizeof (c_strlen_data)); if (!c_getstr (src, NULL) || !(len = c_strlen (src, 0, &data, 1))) - return expand_movstr (dst, src, target, /*endp=*/2); + return expand_movstr (dst, src, target, + /*endp=*/ POINTER_END_MINUS_ONE); if (data.decl && !TREE_NO_WARNING (exp)) warn_string_no_nul (EXPR_LOCATION (exp), "stpcpy", src, data.decl); lenp1 = size_binop_loc (loc, PLUS_EXPR, len, ssize_int (1)); ret = expand_builtin_mempcpy_args (dst, src, lenp1, - target, exp, /*endp=*/2); + target, exp, + /*endp=*/ POINTER_END_MINUS_ONE); if (ret) return ret; @@ -4132,7 +4128,7 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode) } } - return expand_movstr (dst, src, target, /*endp=*/2); + return expand_movstr (dst, src, target, /*endp=*/ POINTER_END_MINUS_ONE); } } @@ -4378,7 +4374,8 @@ expand_builtin_strncpy (tree exp, rtx target) dest_mem = get_memory_rtx (dest, len); store_by_pieces (dest_mem, tree_to_uhwi (len), builtin_strncpy_read_str, - CONST_CAST (char *, p), dest_align, false, 0); + CONST_CAST (char *, p), dest_align, false, + POINTER_START); dest_mem = force_operand (XEXP (dest_mem, 0), target); dest_mem = convert_memory_address (ptr_mode, dest_mem); return dest_mem; @@ -4523,7 +4520,7 @@ expand_builtin_memset_args (tree dest, tree val, tree len, val_rtx = force_reg (val_mode, val_rtx); store_by_pieces (dest_mem, tree_to_uhwi (len), builtin_memset_gen_str, val_rtx, dest_align, - true, 0); + true, POINTER_START); } else if (!set_storage_via_setmem (dest_mem, len_rtx, val_rtx, dest_align, expected_align, @@ -4546,7 +4543,8 @@ expand_builtin_memset_args (tree dest, tree val, tree len, builtin_memset_read_str, &c, dest_align, true)) store_by_pieces (dest_mem, tree_to_uhwi (len), - builtin_memset_read_str, &c, dest_align, true, 0); + builtin_memset_read_str, &c, dest_align, true, + POINTER_START); else if (!set_storage_via_setmem (dest_mem, len_rtx, gen_int_mode (c, val_mode), dest_align, expected_align, diff --git a/gcc/expr.c b/gcc/expr.c index 7ae3e37378c..0215af53189 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -1146,7 +1146,7 @@ class move_by_pieces_d : public op_by_pieces_d : op_by_pieces_d (to, false, from, true, NULL, NULL, len, align) { } - rtx finish_endp (int); + rtx finish_endp (memop_ret); }; /* Return true if MODE can be used for a set of copies, given an @@ -1182,15 +1182,14 @@ move_by_pieces_d::generate (rtx op0, rtx op1, } /* Perform the final adjustment at the end of a string to obtain the - correct return value for the block operation. If ENDP is 1 return - memory at the end ala mempcpy, and if ENDP is 2 return memory the - end minus one byte ala stpcpy. */ + correct return value for the block operation. + Return value is based on ENDP argument. */ rtx -move_by_pieces_d::finish_endp (int endp) +move_by_pieces_d::finish_endp (memop_ret endp) { gcc_assert (!m_reverse); - if (endp == 2) + if (endp == POINTER_END_MINUS_ONE) { m_to.maybe_postinc (-1); --m_offset; @@ -1206,13 +1205,11 @@ move_by_pieces_d::finish_endp (int endp) ALIGN is maximum stack alignment we can assume. - If ENDP is 0 return to, if ENDP is 1 return memory at the end ala - mempcpy, and if ENDP is 2 return memory the end minus one byte ala - stpcpy. */ + Return value is based on ENDP argument. */ rtx move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len, - unsigned int align, int endp) + unsigned int align, memop_ret endp) { #ifndef PUSH_ROUNDING if (to == NULL) @@ -1244,7 +1241,7 @@ class store_by_pieces_d : public op_by_pieces_d : op_by_pieces_d (to, false, NULL_RTX, true, cfn, cfn_data, len, align) { } - rtx finish_endp (int); + rtx finish_endp (memop_ret); }; /* Return true if MODE can be used for a set of stores, given an @@ -1272,15 +1269,14 @@ store_by_pieces_d::generate (rtx op0, rtx op1, machine_mode) } /* Perform the final adjustment at the end of a string to obtain the - correct return value for the block operation. If ENDP is 1 return - memory at the end ala mempcpy, and if ENDP is 2 return memory the - end minus one byte ala stpcpy. */ + correct return value for the block operation. + Return value is based on ENDP argument. */ rtx -store_by_pieces_d::finish_endp (int endp) +store_by_pieces_d::finish_endp (memop_ret endp) { gcc_assert (!m_reverse); - if (endp == 2) + if (endp == POINTER_END_MINUS_ONE) { m_to.maybe_postinc (-1); --m_offset; @@ -1370,14 +1366,13 @@ can_store_by_pieces (unsigned HOST_WIDE_INT len, pointer which will be passed as argument in every CONSTFUN call. ALIGN is maximum alignment we can assume. MEMSETP is true if this is a memset operation and false if it's a copy of a constant string. - If ENDP is 0 return to, if ENDP is 1 return memory at the end ala - mempcpy, and if ENDP is 2 return memory the end minus one byte ala - stpcpy. */ + Return value is based on ENDP argument. */ rtx store_by_pieces (rtx to, unsigned HOST_WIDE_INT len, rtx (*constfun) (void *, HOST_WIDE_INT, scalar_int_mode), - void *constfundata, unsigned int align, bool memsetp, int endp) + void *constfundata, unsigned int align, bool memsetp, + memop_ret endp) { if (len == 0) { @@ -1624,7 +1619,7 @@ emit_block_move_hints (rtx x, rtx y, rtx size, enum block_op_methods method, } if (CONST_INT_P (size) && can_move_by_pieces (INTVAL (size), align)) - move_by_pieces (x, y, INTVAL (size), align, 0); + move_by_pieces (x, y, INTVAL (size), align, POINTER_START); else if (emit_block_move_via_movmem (x, y, size, align, expected_align, expected_size, min_size, max_size, probable_max_size)) @@ -4421,7 +4416,8 @@ emit_push_insn (rtx x, machine_mode mode, tree type, rtx size, && where_pad != stack_direction) anti_adjust_stack (gen_int_mode (extra, Pmode)); - move_by_pieces (NULL, xinner, INTVAL (size) - used, align, 0); + move_by_pieces (NULL, xinner, INTVAL (size) - used, align, + POINTER_START); } else #endif /* PUSH_ROUNDING */ @@ -5618,7 +5614,8 @@ store_expr (tree exp, rtx target, int call_param_p, CONST_CAST (char *, TREE_STRING_POINTER (str)), MEM_ALIGN (target), false, - exp_len > str_copy_len ? 1 : 0); + (exp_len > str_copy_len ? POINTER_END : + POINTER_START)); if (exp_len > str_copy_len) clear_storage (adjust_address (dest_mem, BLKmode, 0), GEN_INT (exp_len - str_copy_len), diff --git a/gcc/expr.h b/gcc/expr.h index 19028f0e6a4..6ae343d81f0 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -219,7 +219,7 @@ extern int can_store_by_pieces (unsigned HOST_WIDE_INT, MEMSETP is true if this is a real memset/bzero, not a copy. Returns TO + LEN. */ extern rtx store_by_pieces (rtx, unsigned HOST_WIDE_INT, by_pieces_constfn, - void *, unsigned int, bool, int); + void *, unsigned int, bool, memop_ret); /* Emit insns to set X from Y. */ extern rtx_insn *emit_move_insn (rtx, rtx); diff --git a/gcc/rtl.h b/gcc/rtl.h index dd3ce06295a..04372a3d066 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -4046,9 +4046,24 @@ extern void expand_null_return (void); extern void expand_naked_return (void); extern void emit_jump (rtx); +/* Memory operation built-ins differ by return value. Mapping + of the enum values is following: + - POINTER_START - return destination, e.g. memcpy + - POINTER_END - return destination + n, e.g. mempcpy + - POINTER_END_MINUS_ONE - return pointer to end of string (address + of the terminating null byte), e.g. strcpy +*/ + +enum memop_ret +{ + POINTER_START, + POINTER_END, + POINTER_END_MINUS_ONE +}; + /* In expr.c */ extern rtx move_by_pieces (rtx, rtx, unsigned HOST_WIDE_INT, - unsigned int, int); + unsigned int, memop_ret); extern poly_int64 find_args_size_adjust (rtx_insn *); extern poly_int64 fixup_args_size_notes (rtx_insn *, rtx_insn *, poly_int64);