llvm dev - Sep 2010 - [LLVMdev] Proposal: Splitting up MC/ELF + AsmPrinter Hierarchy?

Hi everyone,

I am in the process of adding some new code for th ARM/MC ELF
emission, but noticed a curious linkage between th AsmPrinter and the
MC.

It looks like the MC code (on X86 at least) calls out to the
(misnamed?) AsmPrinter to dump out ELF bits (in X86AsmPrinter.cpp) in
the same routine, using conditional branching....

As JimG and I are working both on ARM emission stuff, I want to
minimize chances of us stomping on each other - which probably means a
refactoring the AsmPrinter hierarchy to contain a common ABC.
This approach seems like the right one to take, but it itself is a
"big" nonlocal change, so I wanted to gather opinions before I send in
a patch along those lines...

Steps would be:

1. New ABC MCTargetPrinter  (similar to the current AsmPrinter.cpp class)
2. New derived ABC  ARMMCTargetjPrinter
3. New derived class ARMMCAsmPrinter (this is where JimG's new ARM MC
.s printer code would go)
4. New Derived class ARMMCObjPrinter (this is where the ELF specific
stuff would go)
4a. Add in new temp switches to configure and llc to enable this
feature for ARM only...
5. Once working, clear off ARMAsmPrinter.cppr
6. Once ported over to other architectures, clear off AsmPrinter
entirely and rejigger configure script.

I'd start off with lib/Target/ARM specifically, first, and these are
pure additions at first, so disruptions willhopefully b e minimized...
>From a cursory inspection, I haven't seen past discussions on this
topic specifically, so I was curious whether something like this was
planned/proposed but than was shelved for some reason?


Thanks,

-Jason

On Sep 27, 2010, at 10:41 AM, Jason Kim wrote:
> Hi everyone,
> 
> I am in the process of adding some new code for th ARM/MC ELF
> emission, but noticed a curious linkage between th AsmPrinter and the
> MC.
> 
> It looks like the MC code (on X86 at least) calls out to the
> (misnamed?) AsmPrinter to dump out ELF bits (in X86AsmPrinter.cpp) in
> the same routine, using conditional branching....
You're right: AsmPrinter is misnamed, a better name would be
"MCLowering".  AsmPrinter (and its subclasses) are in chart of
lowering from the "Machine" layer to the MC layer.  The flow here is
that the AsmPrinter code calls into the MC code.
> As JimG and I are working both on ARM emission stuff, I want to
> minimize chances of us stomping on each other - which probably means a
> refactoring the AsmPrinter hierarchy to contain a common ABC.
> This approach seems like the right one to take, but it itself is a
> "big" nonlocal change, so I wanted to gather opinions before I
send in
> a patch along those lines...
Please don't do this, there shouldn't be a new subclass or change in
AsmPrinter.  I recently fixed X86 to only have one AsmPrinter class instead of 3
:-)

What specific problem are you trying to solve that you think requires this?

-Chris