This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH, HSA]: Fix PR hsa/70399
- From: Martin LiÅka <mliska at suse dot cz>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Martin Jambor <mjambor at suse dot cz>
- Date: Thu, 24 Mar 2016 15:59:18 +0100
- Subject: [PATCH, HSA]: Fix PR hsa/70399
- Authentication-results: sourceware.org; auth=none
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 <mliska@suse.cz>
Date: Thu, 24 Mar 2016 15:41:59 +0100
Subject: [PATCH] Fix PR hsa/70399
gcc/ChangeLog:
2016-03-24 Martin Liska <mliska@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