This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, Apr 18, 2012 at 5:32 PM, Andrew MacLeod <amacleod@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]