llvm dev - Nov 2017 - RFC: Splitting <Target>DAGISel.inc into declarations and definitions

Hi,

Currently, TableGen generates all the instruction selection functions 
(in the .inc file) as if they were top-most functions. To make them 
members of their corresponding SelectionDAGISel derivative, each target 
has to include the .inc file directly into the body of the class:

--- FooDAGISel.inc ---
void SelectCode(Node *N) {
   // 1E6 lines of pattern matching code
}
...
----------------------

--- FooISelDAGToDAG.cpp ---
class FooDAGToDAGISel : public SelectionDAGISel {
#include "FooDAGISel.inc"
};
---------------------------

If someone wanted to put the class definition in a separate header file, 
the large .inc file would need to be included in that header. If that 
someone then wanted to split their .cpp file into smaller pieces, each 
such .cpp would contain these function bodies (which would then be 
needlessly compiled multiple times).


What I propose is to have TableGen emit information that would allow 
something like this:

class FooDAGToDAGISel : public SelectionDAGISel {
#define DAGISEL_DECL
#include "FooDAGISel.inc"
#undef DAGISEL_DECL
};

#define DAGISEL_BODY(FooDAGToDAGISel)
#include "FooDAGISel.inc"
#undef DAGISEL_BODY

This would allow only putting member declarations in the class 
definition, and then include the member bodies in a single place.

What does everybody think?

-Krzysztof


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation

This seems reasonable to me.

Though I think we usually make these things self cleaning so the "#undef
DAGISEL_DECL" should be in the .inc file after its #ifdef.

~Craig

On Thu, Nov 2, 2017 at 1:12 PM, Krzysztof Parzyszek via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi,
>
> Currently, TableGen generates all the instruction selection functions (in
> the .inc file) as if they were top-most functions. To make them members of
> their corresponding SelectionDAGISel derivative, each target has to include
> the .inc file directly into the body of the class:
>
> --- FooDAGISel.inc ---
> void SelectCode(Node *N) {
>   // 1E6 lines of pattern matching code
> }
> ...
> ----------------------
>
> --- FooISelDAGToDAG.cpp ---
> class FooDAGToDAGISel : public SelectionDAGISel {
> #include "FooDAGISel.inc"
> };
> ---------------------------
>
> If someone wanted to put the class definition in a separate header file,
> the large .inc file would need to be included in that header. If that
> someone then wanted to split their .cpp file into smaller pieces, each such
> .cpp would contain these function bodies (which would then be needlessly
> compiled multiple times).
>
>
> What I propose is to have TableGen emit information that would allow
> something like this:
>
> class FooDAGToDAGISel : public SelectionDAGISel {
> #define DAGISEL_DECL
> #include "FooDAGISel.inc"
> #undef DAGISEL_DECL
> };
>
> #define DAGISEL_BODY(FooDAGToDAGISel)
> #include "FooDAGISel.inc"
> #undef DAGISEL_BODY
>
> This would allow only putting member declarations in the class definition,
> and then include the member bodies in a single place.
>
> What does everybody think?
>
> -Krzysztof
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by The Linux Foundation
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171102/f8361f42/attachment.html>

Great! I posted a review: https://reviews.llvm.org/D39596, it includes 
the self-cleanup.

-Krzysztof


On 11/3/2017 12:15 AM, Craig Topper wrote:
> This seems reasonable to me.
> 
> Though I think we usually make these things self cleaning so the
"#undef
> DAGISEL_DECL" should be in the .inc file after its #ifdef.
> 
> ~Craig
> 
> On Thu, Nov 2, 2017 at 1:12 PM, Krzysztof Parzyszek via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
> 
>     Hi,
> 
>     Currently, TableGen generates all the instruction selection
>     functions (in the .inc file) as if they were top-most functions. To
>     make them members of their corresponding SelectionDAGISel
>     derivative, each target has to include the .inc file directly into
>     the body of the class:
> 
>     --- FooDAGISel.inc ---
>     void SelectCode(Node *N) {
>        // 1E6 lines of pattern matching code
>     }
>     ...
>     ----------------------
> 
>     --- FooISelDAGToDAG.cpp ---
>     class FooDAGToDAGISel : public SelectionDAGISel {
>     #include "FooDAGISel.inc"
>     };
>     ---------------------------
> 
>     If someone wanted to put the class definition in a separate header
>     file, the large .inc file would need to be included in that header.
>     If that someone then wanted to split their .cpp file into smaller
>     pieces, each such .cpp would contain these function bodies (which
>     would then be needlessly compiled multiple times).
> 
> 
>     What I propose is to have TableGen emit information that would allow
>     something like this:
> 
>     class FooDAGToDAGISel : public SelectionDAGISel {
>     #define DAGISEL_DECL
>     #include "FooDAGISel.inc"
>     #undef DAGISEL_DECL
>     };
> 
>     #define DAGISEL_BODY(FooDAGToDAGISel)
>     #include "FooDAGISel.inc"
>     #undef DAGISEL_BODY
> 
>     This would allow only putting member declarations in the class
>     definition, and then include the member bodies in a single place.
> 
>     What does everybody think?
> 
>     -Krzysztof
> 
> 
>     -- 
>     Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>     hosted by The Linux Foundation
>     _______________________________________________
>     LLVM Developers mailing list
>     llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>     <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
> 
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation