This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [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, 31 Mar 2016 19:28:44 +0200
- Subject: Re: [PATCH, HSA]: Fix PR hsa/70399
- Authentication-results: sourceware.org; auth=none
- References: <56F400C6 dot 9040809 at suse dot cz>
On 03/24/2016 03:59 PM, Martin LiÅka wrote:
> 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
Hi.
I've prepared v2 of the patch, where I postpone emission of binary representation of an immediate
value.
The patch is pre-approved by Martin Jambor.
Installed as r234647.
Thanks,
Martin
>From 1b77faf3ae850b2620b88fb656c372e7736540bd 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>
PR hsa/70399
* 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-dump.c (dump_hsa_immed): Remove checking assert.
* hsa-gen.c (hsa_op_immed::hsa_op_immed): Remove initialization
of class' fields that are removed.
(hsa_op_immed::~hsa_op_immed): Remove deinitialization.
* hsa.h (class hsa_op_immed): Remove m_brig_repr and
m_brig_repr_size fields.
---
gcc/hsa-brig.c | 133 ++++++++++++++++++++++++++++++++++++++-------------------
gcc/hsa-dump.c | 2 -
gcc/hsa-gen.c | 68 +++++++----------------------
gcc/hsa.h | 12 +++---
4 files changed, 109 insertions(+), 106 deletions(-)
diff --git a/gcc/hsa-brig.c b/gcc/hsa-brig.c
index 9b6c0b8..71d4f66 100644
--- a/gcc/hsa-brig.c
+++ b/gcc/hsa-brig.c
@@ -932,62 +932,101 @@ emit_immediate_scalar_to_buffer (tree value, char *data, unsigned need_len)
return len;
}
-void
-hsa_op_immed::emit_to_buffer (tree value)
+char *
+hsa_op_immed::emit_to_buffer (unsigned *brig_repr_size)
{
- 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;
+ char *brig_repr;
+ *brig_repr_size = hsa_get_imm_brig_type_len (m_type);
- if (TREE_CODE (value) == VECTOR_CST)
+ if (m_tree_value != NULL_TREE)
{
- int i, num = VECTOR_CST_NELTS (value);
- for (i = 0; i < num; i++)
+ /* Update brig_repr_size for special tree values. */
+ if (TREE_CODE (m_tree_value) == STRING_CST)
+ *brig_repr_size = TREE_STRING_LENGTH (m_tree_value);
+ else if (TREE_CODE (m_tree_value) == CONSTRUCTOR)
+ *brig_repr_size
+ = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (m_tree_value)));
+
+ unsigned total_len = *brig_repr_size;
+
+ /* As we can have a constructor with fewer elements, fill the memory
+ with zeros. */
+ brig_repr = XCNEWVEC (char, total_len);
+ char *p = brig_repr;
+
+ 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 (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 (*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 ();
}
+
+ brig_repr = XNEWVEC (char, *brig_repr_size);
+ memcpy (brig_repr, &bytes, *brig_repr_size);
}
- else
- emit_immediate_scalar_to_buffer (value, p, total_len);
+
+ return brig_repr;
}
/* Emit an immediate BRIG operand IMM. The BRIG type of the immediate might
@@ -999,17 +1038,21 @@ hsa_op_immed::emit_to_buffer (tree value)
static void
emit_immediate_operand (hsa_op_immed *imm)
{
+ unsigned brig_repr_size;
+ char *brig_repr = imm->emit_to_buffer (&brig_repr_size);
struct BrigOperandConstantBytes out;
memset (&out, 0, sizeof (out));
out.base.byteCount = lendian16 (sizeof (out));
out.base.kind = lendian16 (BRIG_KIND_OPERAND_CONSTANT_BYTES);
- uint32_t byteCount = lendian32 (imm->m_brig_repr_size);
+ uint32_t byteCount = lendian32 (brig_repr_size);
out.type = lendian16 (imm->m_type);
out.bytes = lendian32 (brig_data.add (&byteCount, sizeof (byteCount)));
brig_operand.add (&out, sizeof (out));
- brig_data.add (imm->m_brig_repr, imm->m_brig_repr_size);
+ brig_data.add (brig_repr, brig_repr_size);
brig_data.round_size_up (4);
+
+ free (brig_repr);
}
/* Emit a register BRIG operand REG. */
diff --git a/gcc/hsa-dump.c b/gcc/hsa-dump.c
index b69b34d..c8c2523 100644
--- a/gcc/hsa-dump.c
+++ b/gcc/hsa-dump.c
@@ -657,8 +657,6 @@ dump_hsa_immed (FILE *f, hsa_op_immed *imm)
print_generic_expr (f, imm->m_tree_value, 0);
else
{
- gcc_checking_assert (imm->m_brig_repr_size <= 8);
-
if (unsigned_int_type)
fprintf (f, HOST_WIDE_INT_PRINT_DEC, imm->m_int_value);
else
diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c
index 72eecf9..4a7d8fb 100644
--- a/gcc/hsa-gen.c
+++ b/gcc/hsa-gen.c
@@ -1054,8 +1054,7 @@ hsa_op_with_type::get_in_type (BrigType16_t dtype, hsa_bb *hbb)
hsa_op_immed::hsa_op_immed (tree tree_val, bool min32int)
: hsa_op_with_type (BRIG_KIND_OPERAND_CONSTANT_BYTES,
hsa_type_for_tree_type (TREE_TYPE (tree_val), NULL,
- min32int)),
- m_brig_repr (NULL)
+ min32int))
{
if (hsa_seen_error ())
return;
@@ -1065,30 +1064,20 @@ hsa_op_immed::hsa_op_immed (tree tree_val, bool min32int)
|| TREE_CODE (tree_val) == INTEGER_CST))
|| TREE_CODE (tree_val) == CONSTRUCTOR);
m_tree_value = tree_val;
- m_brig_repr_size = hsa_get_imm_brig_type_len (m_type);
- if (TREE_CODE (m_tree_value) == STRING_CST)
- m_brig_repr_size = TREE_STRING_LENGTH (m_tree_value);
- else if (TREE_CODE (m_tree_value) == CONSTRUCTOR)
- {
- m_brig_repr_size
- = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (m_tree_value)));
-
- /* Verify that all elements of a constructor are constants. */
- for (unsigned i = 0;
- i < vec_safe_length (CONSTRUCTOR_ELTS (m_tree_value)); i++)
- {
- tree v = CONSTRUCTOR_ELT (m_tree_value, i)->value;
- if (!CONSTANT_CLASS_P (v))
- {
- HSA_SORRY_AT (EXPR_LOCATION (tree_val),
- "HSA ctor should have only constants");
- return;
- }
- }
- }
-
- emit_to_buffer (m_tree_value);
+ /* Verify that all elements of a constructor are constants. */
+ if (TREE_CODE (m_tree_value) == CONSTRUCTOR)
+ for (unsigned i = 0;
+ i < vec_safe_length (CONSTRUCTOR_ELTS (m_tree_value)); i++)
+ {
+ tree v = CONSTRUCTOR_ELT (m_tree_value, i)->value;
+ if (!CONSTANT_CLASS_P (v))
+ {
+ HSA_SORRY_AT (EXPR_LOCATION (tree_val),
+ "HSA ctor should have only constants");
+ return;
+ }
+ }
}
/* Constructor of class representing HSA immediate values. INTEGER_VALUE is the
@@ -1096,38 +1085,14 @@ hsa_op_immed::hsa_op_immed (tree tree_val, bool min32int)
hsa_op_immed::hsa_op_immed (HOST_WIDE_INT integer_value, BrigType16_t type)
: hsa_op_with_type (BRIG_KIND_OPERAND_CONSTANT_BYTES, type),
- m_tree_value (NULL), m_brig_repr (NULL)
+ m_tree_value (NULL)
{
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 ()
- : hsa_op_with_type (BRIG_KIND_NONE, BRIG_TYPE_NONE), m_brig_repr (NULL)
+ : hsa_op_with_type (BRIG_KIND_NONE, BRIG_TYPE_NONE)
{
}
@@ -1143,7 +1108,6 @@ hsa_op_immed::operator new (size_t)
hsa_op_immed::~hsa_op_immed ()
{
- free (m_brig_repr);
}
/* Change type of the immediate value to T. */
diff --git a/gcc/hsa.h b/gcc/hsa.h
index 1d6baab..59bec04 100644
--- a/gcc/hsa.h
+++ b/gcc/hsa.h
@@ -171,25 +171,23 @@ public:
~hsa_op_immed ();
void set_type (BrigKind16_t t);
+ /* Function returns pointer to a buffer that contains binary representation
+ of the immeadiate value. The buffer has length of BRIG_SIZE and
+ a caller is responsible for deallocation of the buffer. */
+ char *emit_to_buffer (unsigned *brig_size);
+
/* Value as represented by middle end. */
tree m_tree_value;
/* Integer value representation. */
HOST_WIDE_INT m_int_value;
- /* Brig data representation. */
- char *m_brig_repr;
-
- /* Brig data representation size in bytes. */
- unsigned m_brig_repr_size;
-
private:
/* Make the default constructor inaccessible. */
hsa_op_immed ();
/* 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