[llvm-commits] Tblgen refactoring

Malul, Elior elior.malul at intel.com
Mon Dec 31 05:20:28 PST 2012


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.





More information about the llvm-commits mailing list