This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [trans-mem] rms-tm bug report
On 05/28/2010 02:53 PM, Aldy Hernandez wrote:
> First, I only used the TYPE argument for the callbacks, as the caller of
> the callback already handles the appropriate cast. See
> built_tm_{load,store}.
Nice.
> Unfortunately I needed some more callbacks to implement
> is_tm_*{load,store}.
Ok.
> I added a FIXME for adding vector logging functions later. Right now
> they'll be handled with the ubiquitous ITM_LB.
Ok.
> OK for branch?
Nearly...
> +ix86_init_tm_builtins (void)
> +{
> + enum ix86_builtin_func_type ftype;
> + const struct builtin_description *d;
> + size_t i;
> + tree decl, attrs;
> +
> + if (!flag_tm)
> + return;
> +
> + attrs = tree_cons (get_identifier ("transaction_pure"), NULL, NULL);
The attributes here aren't correct. Stores get ATTR_TM_NOTHROW_LIST;
loads get ATTR_TM_PURE_TMPURE_NOTHROW_LIST.
So you're setting transaction_pure for stores incorrectly, failing to
set nothrow on any of the builtins, and missing the REGPARM attribute
normally had via the ATTR_TM_REGPARM hack. Which means that i686 will
fail badly in testing.
> +ix86_builtin_tm_load (tree type)
> +{
> + if (TYPE_SIZE_UNIT (type) != NULL
> + && host_integerp (TYPE_SIZE_UNIT (type), 1))
> + {
This should be
if (TREE_CODE (type) == VECTOR_TYPE)
to make sure that we don't apply this to e.g. 16 and 32-byte structures.
You needn't test the host_integerp; that will always be set for vectors.
> + switch (tree_low_cst (TYPE_SIZE_UNIT (type), 1) * BITS_PER_UNIT)
switch (tree_low_cst (TYPE_SIZE (type), 1))
> +ix86_builtin_tm_store (tree type)
Likewise.
r~