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).
Thanks,
Martin
---
gcc/asan.c | 2 +-
gcc/builtins.c | 66 ++++++++++++++++++++++++--------------------------
gcc/expr.c | 43 +++++++++++++++-----------------
gcc/expr.h | 2 +-
gcc/rtl.h | 17 ++++++++++++-
5 files changed, 70 insertions(+), 60 deletions(-)
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);
