[llvm-commits] Tblgen refactoring

[Sean Silva]

Jakob, could you take a look at this?

-- Sean Silva

On Tue, Jan 1, 2013 at 2:06 AM, Malul, Elior <elior.malul at intel.com> wrote:
> Attached is a revised patch.
> Thx, Elior
>
> -----Original Message-----
> From: Sean Silva [mailto:silvas at purdue.edu]
> Sent: Tuesday, January 01, 2013 02:31
> To: Malul, Elior
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] Tblgen refactoring
>
> Please submit a revised patch for review.
>
> -- Sean Silva
>
> On Mon, Dec 31, 2012 at 6:20 AM, Malul, Elior <elior.malul at intel.com> wrote:
>> Hi Sean.
>> As you mentioned, the main idea behind the patch is to eliminate the (what seems to me as) excessive overloading of 'subClassOf' for every derived type.
>> Instead, I have changed it so there will be only one virtual method with the prototype 'virtual void baseClassOf(const RecTy*)'.
>> Then, the dynamic type to the parameter is done (when needed) by querying the 'getRecTyKind()' method.
>> It does add some branches to the code, but I think all together, it conforms better to the DRY principle.
>>
>> Thx for your comment. See rest of my comment in the text below:
>>
>> -----Original Message-----
>> From: Sean Silva [mailto:silvas at purdue.edu]
>> Sent: Monday, December 31, 2012 13:09
>> To: Malul, Elior
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] Tblgen refactoring
>>
>> This is something that has been bugging me too!
>>
>> Maybe you could explain a bit better the idea behind the patch a bit more? I think that it's the right direction. My understanding is that what you are doing is removing all the different overloads and having a single baseClassOf which dynamically looks at the records it is interested in? If that's correct then that sounds good to me.
>>
>> A few comments:
>>
>> -//===----------------------------------------------------------------------===//
>> -//    std::string wrapper for DenseMap purposes
>> -//===----------------------------------------------------------------
>> ------===//
>> -
>>
>> --Elior:
>> OK, I will separate for a different patch.
>>
>> This is unrelated to the current patch. I think it's a good change, but please separate it out into a different patch.
>>
>> +//
>> +//RecTy
>> +//
>> +
>>
>> --Elior:
>> Same goes here.
>>
>> Same for these "headers" too. I think they are a worthwhile change, but keep them in a separate patch.
>>
>> -    return true;
>> +   return true;
>>
>> What's the point of dedenting this by 1 space?
>>
>> --Elior:
>> Accidental space deletion.
>>
>> -- Sean Silva
>>
>> On Mon, Dec 31, 2012 at 2:25 AM, Malul, Elior <elior.malul at intel.com> wrote:
>>> Hi all, attached is a small patch that I think makes tblgen somewhat
>>> more maintainable.
>>>
>>>
>>>
>>> The way code is at the moment, if one want's to add a new tblgen type
>>> (that is to subclass 'RecTy'),
>>>
>>> Not only he/she compelled to add implementation to all variants of
>>> 'baseClassOf',
>>>
>>> one also needs to add a new 'baseClassOf' method for the newly added
>>> types to *all* the types in the hierarchy.
>>>
>>> The attached patch tries to deal with that phenomena.
>>>
>>>
>>>
>>> I will appreciate if those who care about tblgen will review it.
>>>
>>> Thx, Elior
>>>
>>> ---------------------------------------------------------------------
>>> Intel Israel (74) Limited
>>>
>>> This e-mail and any attachments may contain confidential material for
>>> the sole use of the intended recipient(s). Any review or distribution
>>> by others is strictly prohibited. If you are not the intended
>>> recipient, please contact the sender and delete all copies.
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>> ---------------------------------------------------------------------
>> Intel Israel (74) Limited
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
>>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.