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
