[llvm-commits] Tblgen refactoring

[Sean Silva]

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.
>