Hello. Current HSA back-end wrongly handles memory stores. Although, we properly identify that an immediate operand needs to respect type of a memory store instruction it belongs to, the binary representation of the operand is not updated.
Following patch delays emission of the binary representation and updates hsa_op_immmed::m_brig_repr_size every time the m_type field of the operand is updated. I've been testing the patch, ready after it finishes? Thanks, Martin
>From 8fa067df55566d7a52ad6070a8844d434519fe46 Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz> Date: Thu, 24 Mar 2016 15:41:59 +0100 Subject: [PATCH] Fix PR hsa/70399 gcc/ChangeLog: 2016-03-24 Martin Liska <mli...@suse.cz> * hsa-brig.c (hsa_op_immed::emit_to_buffer): Emit either a tree value or an immediate integer value to a buffer that is eventually copied to a BRIG section. (emit_immediate_operand): Call the function here. * hsa-gen.c (hsa_op_immed::hsa_op_immed): Remove this early emission to buffer. (hsa_op_immed::set_type): Update size of m_brig_repr_size. (gen_hsa_insns_for_store): Use hsa_op_immed::set_type. * hsa.h (hsa_op_immed::emit_to_buffer): Update signature. --- gcc/hsa-brig.c | 112 +++++++++++++++++++++++++++++++++++---------------------- gcc/hsa-gen.c | 34 +++--------------- gcc/hsa.h | 2 +- 3 files changed, 76 insertions(+), 72 deletions(-) diff --git a/gcc/hsa-brig.c b/gcc/hsa-brig.c index 9b6c0b8..8d18b0f 100644 --- a/gcc/hsa-brig.c +++ b/gcc/hsa-brig.c @@ -933,61 +933,88 @@ emit_immediate_scalar_to_buffer (tree value, char *data, unsigned need_len) } void -hsa_op_immed::emit_to_buffer (tree value) +hsa_op_immed::emit_to_buffer () { - unsigned total_len = m_brig_repr_size; + if (m_tree_value != NULL_TREE) + { + unsigned total_len = m_brig_repr_size; - /* As we can have a constructor with fewer elements, fill the memory - with zeros. */ - m_brig_repr = XCNEWVEC (char, total_len); - char *p = m_brig_repr; + /* As we can have a constructor with fewer elements, fill the memory + with zeros. */ + m_brig_repr = XCNEWVEC (char, total_len); + char *p = m_brig_repr; - if (TREE_CODE (value) == VECTOR_CST) - { - int i, num = VECTOR_CST_NELTS (value); - for (i = 0; i < num; i++) + if (TREE_CODE (m_tree_value) == VECTOR_CST) + { + int i, num = VECTOR_CST_NELTS (m_tree_value); + for (i = 0; i < num; i++) + { + tree v = VECTOR_CST_ELT (m_tree_value, i); + unsigned actual = emit_immediate_scalar_to_buffer (v, p, 0); + total_len -= actual; + p += actual; + } + /* Vectors should have the exact size. */ + gcc_assert (total_len == 0); + } + else if (TREE_CODE (m_tree_value) == STRING_CST) + memcpy (m_brig_repr, TREE_STRING_POINTER (m_tree_value), + TREE_STRING_LENGTH (m_tree_value)); + else if (TREE_CODE (m_tree_value) == COMPLEX_CST) { + gcc_assert (total_len % 2 == 0); unsigned actual; actual - = emit_immediate_scalar_to_buffer (VECTOR_CST_ELT (value, i), p, 0); - total_len -= actual; + = emit_immediate_scalar_to_buffer (TREE_REALPART (m_tree_value), p, + total_len / 2); + + gcc_assert (actual == total_len / 2); p += actual; + + actual + = emit_immediate_scalar_to_buffer (TREE_IMAGPART (m_tree_value), p, + total_len / 2); + gcc_assert (actual == total_len / 2); } - /* Vectors should have the exact size. */ - gcc_assert (total_len == 0); - } - else if (TREE_CODE (value) == STRING_CST) - memcpy (m_brig_repr, TREE_STRING_POINTER (value), - TREE_STRING_LENGTH (value)); - else if (TREE_CODE (value) == COMPLEX_CST) - { - gcc_assert (total_len % 2 == 0); - unsigned actual; - actual - = emit_immediate_scalar_to_buffer (TREE_REALPART (value), p, - total_len / 2); - - gcc_assert (actual == total_len / 2); - p += actual; - - actual - = emit_immediate_scalar_to_buffer (TREE_IMAGPART (value), p, - total_len / 2); - gcc_assert (actual == total_len / 2); + else if (TREE_CODE (m_tree_value) == CONSTRUCTOR) + { + unsigned len = vec_safe_length (CONSTRUCTOR_ELTS (m_tree_value)); + for (unsigned i = 0; i < len; i++) + { + tree v = CONSTRUCTOR_ELT (m_tree_value, i)->value; + unsigned actual = emit_immediate_scalar_to_buffer (v, p, 0); + total_len -= actual; + p += actual; + } + } + else + emit_immediate_scalar_to_buffer (m_tree_value, p, total_len); } - else if (TREE_CODE (value) == CONSTRUCTOR) + else { - unsigned len = vec_safe_length (CONSTRUCTOR_ELTS (value)); - for (unsigned i = 0; i < len; i++) + hsa_bytes bytes; + + switch (m_brig_repr_size) { - tree v = CONSTRUCTOR_ELT (value, i)->value; - unsigned actual = emit_immediate_scalar_to_buffer (v, p, 0); - total_len -= actual; - p += actual; + case 1: + bytes.b8 = (uint8_t) m_int_value; + break; + case 2: + bytes.b16 = (uint16_t) m_int_value; + break; + case 4: + bytes.b32 = (uint32_t) m_int_value; + break; + case 8: + bytes.b64 = (uint64_t) m_int_value; + break; + default: + gcc_unreachable (); } + + m_brig_repr = XNEWVEC (char, m_brig_repr_size); + memcpy (m_brig_repr, &bytes, m_brig_repr_size); } - else - emit_immediate_scalar_to_buffer (value, p, total_len); } /* Emit an immediate BRIG operand IMM. The BRIG type of the immediate might @@ -999,6 +1026,7 @@ hsa_op_immed::emit_to_buffer (tree value) static void emit_immediate_operand (hsa_op_immed *imm) { + imm->emit_to_buffer (); struct BrigOperandConstantBytes out; memset (&out, 0, sizeof (out)); diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c index 72eecf9..5ea5ed8 100644 --- a/gcc/hsa-gen.c +++ b/gcc/hsa-gen.c @@ -1087,8 +1087,6 @@ hsa_op_immed::hsa_op_immed (tree tree_val, bool min32int) } } } - - emit_to_buffer (m_tree_value); } /* Constructor of class representing HSA immediate values. INTEGER_VALUE is the @@ -1101,29 +1099,6 @@ hsa_op_immed::hsa_op_immed (HOST_WIDE_INT integer_value, BrigType16_t type) gcc_assert (hsa_type_integer_p (type)); m_int_value = integer_value; m_brig_repr_size = hsa_type_bit_size (type) / BITS_PER_UNIT; - - hsa_bytes bytes; - - switch (m_brig_repr_size) - { - case 1: - bytes.b8 = (uint8_t) m_int_value; - break; - case 2: - bytes.b16 = (uint16_t) m_int_value; - break; - case 4: - bytes.b32 = (uint32_t) m_int_value; - break; - case 8: - bytes.b64 = (uint64_t) m_int_value; - break; - default: - gcc_unreachable (); - } - - m_brig_repr = XNEWVEC (char, m_brig_repr_size); - memcpy (m_brig_repr, &bytes, m_brig_repr_size); } hsa_op_immed::hsa_op_immed () @@ -1152,6 +1127,7 @@ void hsa_op_immed::set_type (BrigType16_t t) { m_type = t; + m_brig_repr_size = hsa_type_bit_size (t) / BITS_PER_UNIT; } /* Constructor of class representing HSA registers and pseudo-registers. T is @@ -2668,7 +2644,7 @@ gen_hsa_insns_for_store (tree lhs, hsa_op_base *src, hsa_bb *hbb) if (hsa_op_immed *imm = dyn_cast <hsa_op_immed *> (src)) { if (!hsa_type_packed_p (imm->m_type)) - imm->m_type = mem->m_type; + imm->set_type (mem->m_type); else { /* ...and all vector immediates apparently need to be vectors of @@ -2678,13 +2654,13 @@ gen_hsa_insns_for_store (tree lhs, hsa_op_base *src, hsa_bb *hbb) switch (bs) { case 32: - imm->m_type = BRIG_TYPE_U8X4; + imm->set_type (BRIG_TYPE_U8X4); break; case 64: - imm->m_type = BRIG_TYPE_U8X8; + imm->set_type (BRIG_TYPE_U8X8); break; case 128: - imm->m_type = BRIG_TYPE_U8X16; + imm->set_type (BRIG_TYPE_U8X16); break; default: gcc_unreachable (); diff --git a/gcc/hsa.h b/gcc/hsa.h index 1d6baab..2f84a7d 100644 --- a/gcc/hsa.h +++ b/gcc/hsa.h @@ -170,6 +170,7 @@ public: void *operator new (size_t); ~hsa_op_immed (); void set_type (BrigKind16_t t); + void emit_to_buffer (); /* Value as represented by middle end. */ tree m_tree_value; @@ -189,7 +190,6 @@ private: /* All objects are deallocated by destroying their pool, so make delete inaccessible too. */ void operator delete (void *) {} - void emit_to_buffer (tree value); }; /* Report whether or not P is a an immediate operand. */ -- 2.7.1