On Wed, Apr 18, 2012 at 5:32 PM, Andrew MacLeod <amacl...@redhat.com> wrote:
> 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.
Good point. I'll check for overflow masked value there.
>
> 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?
Frankly speaking, I have no idea about casting of such things. I may remove it,
if it is needed.
K
