On Wed, Jun 26, 2019 at 12:57:15PM +0200, Martin Liška wrote:
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1713,8 +1713,8 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn
> *before)
> rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
> top = convert_memory_address (ptr_mode, top);
> bot = convert_memory_address (ptr_mode, bot);
> - ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode,
> - top, ptr_mode, bot, ptr_mode);
> + emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode,
> + top, ptr_mode, bot, ptr_mode);
If you don't need the return value, you should use emit_library_call,
not emit_library_call_value.
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -3044,7 +3044,7 @@ expand_asm_stmt (gasm *stmt)
> }
> }
> }
> - unsigned nclobbers = clobber_rvec.length();
> + unsigned nclobbers;
Can you instead move the nclobbers declaration to the spot where it
is initialized (right now the second time), and add space before (
there too?
> --- a/gcc/config/i386/i386-expand.c
> +++ b/gcc/config/i386/i386-expand.c
> @@ -16051,8 +16051,6 @@ ix86_expand_rounddf_32 (rtx operand0, rtx operand1)
> mhalf = expand_simple_binop (mode, MINUS, half, one, NULL_RTX,
> 0, OPTAB_DIRECT);
>
> - /* Compensate. */
> - tmp = gen_reg_rtx (mode);
> /* xa2 = xa2 - (dxa > 0.5 ? 1 : 0) */
> tmp = ix86_expand_sse_compare_mask (UNGT, dxa, half, false);
> emit_insn (gen_rtx_SET (tmp, gen_rtx_AND (mode, one, tmp)));
Is this really desirable?
Just quick look suggests perhaps it wants to use one temporary
for the gen_reg_rtx (mode) and use that on the lhs of SET, and another one
as the last operand to AND holding ix86_expand_sse_compare_mask?
Though, such problem is in most of the ix86_expand_sse_compare_mask callers.
While ix86_expand_sse_compare_mask returns a result of gen_reg_rtx too,
I think it is better not to reuse pseudos for different values.
I'd say take out this change and deal with it with i386 maintainers
separately.
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -3483,8 +3483,7 @@ vectorizable_call (stmt_vec_info stmt_info,
> gimple_stmt_iterator *gsi,
> = gimple_build_call_internal_vec (ifn, vargs);
> gimple_call_set_lhs (call, half_res);
> gimple_call_set_nothrow (call, true);
> - new_stmt_info
> - = vect_finish_stmt_generation (stmt_info, call, gsi);
> + vect_finish_stmt_generation (stmt_info, call, gsi);
> if ((i & 1) == 0)
> {
> prev_res = half_res;
> @@ -3583,8 +3582,7 @@ vectorizable_call (stmt_vec_info stmt_info,
> gimple_stmt_iterator *gsi,
> gcall *call = gimple_build_call_internal_vec (ifn, vargs);
> gimple_call_set_lhs (call, half_res);
> gimple_call_set_nothrow (call, true);
> - new_stmt_info
> - = vect_finish_stmt_generation (stmt_info, call, gsi);
> + vect_finish_stmt_generation (stmt_info, call, gsi);
> if ((j & 1) == 0)
> {
> prev_res = half_res;
This looks wrong.
There should have been
SLP_TREE_VEC_STMTS (slp_node).quick_push (new_stmt_info);
in between for slp_node, or the usual code like:
if (cond_for_first)
STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt = new_stmt_info;
else
STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt_info;
prev_stmt_info = new_stmt_info;
otherwise. In any case, I think this should be dealt with separately.
Jakub
