Hi,
On Wed, Mar 23, 2016 at 02:43:17PM +0100, Martin Liska wrote:
> gcc/ChangeLog:
>
> 2016-03-23 Martin Liska <mli...@suse.cz>
>
> PR hsa/70391
> * hsa-gen.c (hsa_function_representation::update_cfg): New
> function.
> (convert_addr_to_flat_segment): Likewise.
> (gen_hsa_memory_set): New alignment argument.
> (gen_hsa_ctor_assignment): Likewise.
> (gen_hsa_insns_for_single_assignment): Provide alignment
> to gen_hsa_ctor_assignment.
> (gen_hsa_insns_for_direct_call): Add new argument.
> (expand_lhs_of_string_op): New function.
> (expand_string_operation_builtin): Likewise.
> (expand_memory_copy): New function.
> (expand_memory_set): New function.
> (gen_hsa_insns_for_call): Use HOST_WIDE_INT.
> (convert_switch_statements): Change signature.
> (generate_hsa): Use a return value of the function.
> (pass_gen_hsail::execute): Do not call
> convert_switch_statements here.
> * hsa-regalloc.c (hsa_regalloc): Call update_cfg.
> * hsa.h (hsa_function_representation::m_need_cfg_update):
> New flag.
> (hsa_function_representation::update_cfg): New function.
As we already discussed, update_cfg and m_need_cfg_update should
really be called differently, because CFG has already been modified
and only dominance needs to be re-computed. If you havent't thought
about any names yet, what about m_modified_cfg and update_dominance() ?
> ---
> gcc/hsa-gen.c | 372
> ++++++++++++++++++++++++++++++++++++++---------------
> gcc/hsa-regalloc.c | 1 +
> gcc/hsa.h | 9 +-
> 3 files changed, 275 insertions(+), 107 deletions(-)
>
> diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c
> index db39813..db7fc3d 100644
> --- a/gcc/hsa-gen.c
> +++ b/gcc/hsa-gen.c
> @@ -214,7 +214,7 @@ hsa_symbol::fillup_for_decl (tree decl)
> should be set to number of SSA names used in the function. */
>
> hsa_function_representation::hsa_function_representation
> - (tree fdecl, bool kernel_p, unsigned ssa_names_count)
> + (tree fdecl, bool kernel_p, unsigned ssa_names_count, bool need_cfg_update)
> : m_name (NULL),
> m_reg_count (0), m_input_args (vNULL),
> m_output_arg (NULL), m_spill_symbols (vNULL), m_global_symbols (vNULL),
> @@ -223,7 +223,8 @@ hsa_function_representation::hsa_function_representation
> m_in_ssa (true), m_kern_p (kernel_p), m_declaration_p (false),
> m_decl (fdecl), m_internal_fn (NULL), m_shadow_reg (NULL),
> m_kernel_dispatch_count (0), m_maximum_omp_data_size (0),
> - m_seen_error (false), m_temp_symbol_count (0), m_ssa_map ()
> + m_seen_error (false), m_temp_symbol_count (0), m_ssa_map (),
> + m_need_cfg_update (need_cfg_update)
> {
> int sym_init_len = (vec_safe_length (cfun->local_decls) / 2) + 1;;
> m_local_symbols = new hash_table <hsa_noop_symbol_hasher> (sym_init_len);
> @@ -319,6 +320,16 @@ hsa_function_representation::init_extra_bbs ()
> hsa_init_new_bb (EXIT_BLOCK_PTR_FOR_FN (cfun));
> }
>
> +void
> +hsa_function_representation::update_cfg ()
> +{
> + if (m_need_cfg_update)
> + {
> + free_dominance_info (CDI_DOMINATORS);
> + calculate_dominance_info (CDI_DOMINATORS);
> + }
> +}
> +
> hsa_symbol *
> hsa_function_representation::create_hsa_temporary (BrigType16_t type)
> {
> @@ -2246,30 +2257,14 @@ gen_hsa_addr_for_arg (tree tree_type, int index)
> return new hsa_op_address (sym);
> }
>
> -/* Generate HSA instructions that calculate address of VAL including all
> - necessary conversions to flat addressing and place the result into DEST.
> +/* Generate HSA instructions that process all necessary conversions
> + of an ADDR to flat addressing and place the result into DEST.
> Instructions are appended to HBB. */
>
> static void
> -gen_hsa_addr_insns (tree val, hsa_op_reg *dest, hsa_bb *hbb)
> +convert_addr_to_flat_segment (hsa_op_address *addr, hsa_op_reg *dest,
> + hsa_bb *hbb)
> {
> - /* Handle cases like tmp = NULL, where we just emit a move instruction
> - to a register. */
> - if (TREE_CODE (val) == INTEGER_CST)
> - {
> - hsa_op_immed *c = new hsa_op_immed (val);
> - hsa_insn_basic *insn = new hsa_insn_basic (2, BRIG_OPCODE_MOV,
> - dest->m_type, dest, c);
> - hbb->append_insn (insn);
> - return;
> - }
> -
> - hsa_op_address *addr;
> -
> - gcc_assert (dest->m_type == hsa_get_segment_addr_type (BRIG_SEGMENT_FLAT));
> - if (TREE_CODE (val) == ADDR_EXPR)
> - val = TREE_OPERAND (val, 0);
> - addr = gen_hsa_addr (val, hbb);
> hsa_insn_basic *insn = new hsa_insn_basic (2, BRIG_OPCODE_LDA);
> insn->set_op (1, addr);
> if (addr->m_symbol && addr->m_symbol->m_segment != BRIG_SEGMENT_GLOBAL)
> @@ -2298,6 +2293,34 @@ gen_hsa_addr_insns (tree val, hsa_op_reg *dest, hsa_bb
> *hbb)
> }
> }
>
> +/* Generate HSA instructions that calculate address of VAL including all
> + necessary conversions to flat addressing and place the result into DEST.
> + Instructions are appended to HBB. */
> +
> +static void
> +gen_hsa_addr_insns (tree val, hsa_op_reg *dest, hsa_bb *hbb)
> +{
> + /* Handle cases like tmp = NULL, where we just emit a move instruction
> + to a register. */
> + if (TREE_CODE (val) == INTEGER_CST)
> + {
> + hsa_op_immed *c = new hsa_op_immed (val);
> + hsa_insn_basic *insn = new hsa_insn_basic (2, BRIG_OPCODE_MOV,
> + dest->m_type, dest, c);
> + hbb->append_insn (insn);
> + return;
> + }
> +
> + hsa_op_address *addr;
> +
> + gcc_assert (dest->m_type == hsa_get_segment_addr_type (BRIG_SEGMENT_FLAT));
> + if (TREE_CODE (val) == ADDR_EXPR)
> + val = TREE_OPERAND (val, 0);
> + addr = gen_hsa_addr (val, hbb);
> +
> + convert_addr_to_flat_segment (addr, dest, hbb);
> +}
> +
> /* Return an HSA register or HSA immediate value operand corresponding to
> gimple operand OP. */
>
> @@ -2728,9 +2751,9 @@ gen_hsa_insns_for_store (tree lhs, hsa_op_base *src,
> hsa_bb *hbb)
> }
>
> /* Generate memory copy instructions that are going to be used
> - for copying a HSA symbol SRC_SYMBOL (or SRC_REG) to TARGET memory,
> + for copying a SRC memory to TARGET memory,
> represented by pointer in a register. MIN_ALIGN is minimal alignment
> - of provided HSA addresses. */
> + of provided HSA addresses. */
>
> static void
> gen_hsa_memory_copy (hsa_bb *hbb, hsa_op_address *target, hsa_op_address
> *src,
> @@ -2792,17 +2815,19 @@ build_memset_value (unsigned HOST_WIDE_INT constant,
> unsigned byte_size)
> }
>
> /* Generate memory set instructions that are going to be used
> - for setting a CONSTANT byte value to TARGET memory of SIZE bytes. */
> + for setting a CONSTANT byte value to TARGET memory of SIZE bytes.
> + MIN_ALIGN is minimal alignment of provided HSA addresses. */
>
> static void
> gen_hsa_memory_set (hsa_bb *hbb, hsa_op_address *target,
> unsigned HOST_WIDE_INT constant,
> - unsigned size)
> + unsigned size, BrigAlignment8_t min_align)
> {
> hsa_op_address *addr;
> hsa_insn_mem *mem;
>
> unsigned offset = 0;
> + unsigned min_byte_align = hsa_byte_alignment (min_align);
>
> while (size)
> {
> @@ -2816,6 +2841,9 @@ gen_hsa_memory_set (hsa_bb *hbb, hsa_op_address *target,
> else
> s = 1;
>
> + if (s > min_byte_align)
> + s = min_byte_align;
> +
> addr = new hsa_op_address (target->m_symbol, target->m_reg,
> target->m_imm_offset + offset);
>
> @@ -2832,10 +2860,12 @@ gen_hsa_memory_set (hsa_bb *hbb, hsa_op_address
> *target,
>
> /* Generate HSAIL instructions for a single assignment
> of an empty constructor to an ADDR_LHS. Constructor is passed as a
> - tree RHS and all instructions are appended to HBB. */
> + tree RHS and all instructions are appended to HBB. ALIGN is
> + alignment of the address. */
>
> void
> -gen_hsa_ctor_assignment (hsa_op_address *addr_lhs, tree rhs, hsa_bb *hbb)
> +gen_hsa_ctor_assignment (hsa_op_address *addr_lhs, tree rhs, hsa_bb *hbb,
> + BrigAlignment8_t align)
> {
> if (vec_safe_length (CONSTRUCTOR_ELTS (rhs)))
> {
> @@ -2845,7 +2875,7 @@ gen_hsa_ctor_assignment (hsa_op_address *addr_lhs, tree
> rhs, hsa_bb *hbb)
> }
>
> unsigned size = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (rhs)));
> - gen_hsa_memory_set (hbb, addr_lhs, 0, size);
> + gen_hsa_memory_set (hbb, addr_lhs, 0, size, align);
> }
>
> /* Generate HSA instructions for a single assignment of RHS to LHS.
> @@ -2879,7 +2909,7 @@ gen_hsa_insns_for_single_assignment (tree lhs, tree
> rhs, hsa_bb *hbb)
> &lhs_align);
>
> if (TREE_CODE (rhs) == CONSTRUCTOR)
> - gen_hsa_ctor_assignment (addr_lhs, rhs, hbb);
> + gen_hsa_ctor_assignment (addr_lhs, rhs, hbb, lhs_align);
> else
> {
> BrigAlignment8_t rhs_align;
> @@ -3523,10 +3553,13 @@ get_format_argument_type (tree formal_arg_type,
> BrigType16_t actual_arg_type)
>
> /* Generate HSA instructions for a direct call instruction.
> Instructions will be appended to HBB, which also needs to be the
> - corresponding structure to the basic_block of STMT. */
> + corresponding structure to the basic_block of STMT.
> + If ASSIGN_LHS is set to true, assignment to a LHS of the called function
> + is not processed. */
I'm not sure the last sentence is really helpful becasue the word
"processed" is not very informative and also because I think it means
the opposite of what the code actually does (it depends on what being
processed means, I suppose). Can you perhaps replace it with "If
ASSIGN_LHS is false, do not copy HSA function result argument into the
corresponding HSA representation of the gimple statement LHS."
Otherwise, the patch is fine, thanks.
Martin
