[PATCH, i386, middle-end, tessuite] Intel TSX's HLE.

Andrew MacLeod amacleod@redhat.com
Wed Apr 18 13:33:00 GMT 2012


Stupid mailer.. sigh.  trying again:

On 04/18/2012 05:36 AM, Kirill Yukhin wrote:

>    op = expand_normal (exp);
> -  if (INTVAL (op) < 0 || INTVAL (op) >= MEMMODEL_LAST)
> +  if (INTVAL (op) < 0)
>      {
>        warning (OPT_Winvalid_memory_model,
>             "invalid memory model argument to builtin");
>        return MEMMODEL_SEQ_CST;
>      }
> +
>    return (enum memmodel) INTVAL (op);
>  }

I think this could do better.

It suppose to check that the memory model is one of MEMMODEL_ enum's, 
and if it isnt' report the error and change it to SEQ_CST.   This is to 
prevent people from accidentally  passing invalid numbers in, like say '23'
I think this ought to check that
   (INTVAL(op) & memmodel_mask)  is within the proper range.

  It would be better if we could check that only other valid recognized 
bits are set in the other word, but Im guessing thats not really 
possible since thats really target specific at the moment.

In any case, I think it ought to check that the 16 reserved bits for 
memory model is correct (like it use to for the whole enum), and if it 
isn't, issue the warning and mask in SEQ_CST for the memory model portion.

  I also think that it should no longer return an 'enum memmodel' since 
the value can now be out of range.   Maybe thats more a nit...  how much 
do we care if we use an enum value as an int?  ie, do we need to worry 
about passing this value around as an 'enum memodel'  when we know its 
more than 16 bits sometimes now?

Andrew







More information about the Gcc-patches mailing list