[cfe-commits] TableGen backend API refactoring review request

Sean Silva silvas at purdue.edu
Sun Jun 10 16:12:02 PDT 2012


Ok, this patch set addresses the things you brought up. Sorry about
forgetting about the namespace/static thing.

Also, I have attached a diff of the .inc files before and after the patch
set. Most of the differences are places where I either added a call to
`emitSourceFileHeader`, or changed an `OS << "TableGen'erated file"` to a
call to emitSourceFileHeader. These changes are benign. There is a peculiar
difference in the ARMGenAsmMatcher.inc, where some table entries and `case`
labels change order (but are otherwise identical); after a little bit of
investigation, I found that the ordering is determined by the iteration
order of std::map's with pointers as keys, and hence the orders are
inherently unstable, so this should not be problematic (although it may be
desirable in the future to make these orders deterministic).

attachments:
1-llvm.patch: migrate LLVM tablegen to new API
2-clang.patch: migrate Clang tablegen to new API
3-llvm.patch: remove struct TableGenBackend and all remnants of the old API
incfiles.diff: the diff of the .inc files before and after the patchset is
applied

--Sean Silva

On Sun, Jun 10, 2012 at 8:45 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

> On Jun 8, 2012, at 10:33 PM, Sean Silva <silvas at purdue.edu> wrote:
>
> > Ok, these patches address all of your points.
> >
> > 1-llvm.patch is the initial changes to the llvm tree. It changes the
> backend api for llvm/utils/TableGen/* and it makes some very minor
> non-breaking changes to include/llvm/TableGen/TableGenBackend.h and
> lib/TableGen/TableGenBackend.h be make emitSourceFileHeader available as a
> free function (the member function still exists to not break Clang's
> TableGen)
> > 2-clang.patch changes Clang's TableGen to the new API
> > 3-llvm.patch removes struct TableGenBackend and all remnants of the old
> API.
>
> Almost there. Please make sure all classes are in anonymous namespaces,
> and read this:
>
> http://llvm.org/docs/CodingStandards.html#micro_anonns
>
> > I built `check` and `clang-test` after applying each one against ToT of
> the respective repos; all green.
>
> Please verify that no .inc files change content wih these patches.
>
> /jakob
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/ce887cfc/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-llvm.patch
Type: application/octet-stream
Size: 92167 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/ce887cfc/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-clang.patch
Type: application/octet-stream
Size: 89063 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/ce887cfc/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3-llvm.patch
Type: application/octet-stream
Size: 2863 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/ce887cfc/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: incfiles.diff
Type: application/octet-stream
Size: 29611 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/ce887cfc/attachment-0003.obj>


More information about the cfe-commits mailing list