[llvm-commits] [llvm] r135760 - in /llvm/trunk: cmake/modules/LLVMLibDeps.cmake lib/Support/CMakeLists.txt lib/Support/TargetRegistry.cpp lib/Target/CMakeLists.txt lib/Target/TargetRegistry.cpp

[Evan Cheng]

On Aug 1, 2011, at 3:01 PM, Chandler Carruth wrote:

> On Mon, Aug 1, 2011 at 12:44 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> 
> On Aug 1, 2011, at 12:36 PM, Chandler Carruth wrote:
> 
>> On Mon, Aug 1, 2011 at 11:47 AM, Evan Cheng <evan.cheng at apple.com> wrote:
>> Hi Chandler,
>> 
>> Are you planning to fix this?
>> 
>> Yes, I just got pre-empted by really really bad CMake issues. If you get time to hack on it first, that's cool. Otherwise I should get to it in the next couple of days.
> 
> Thanks, please do. I've verified this bug has increased size of release build of llvm-mc by about 900k.
> 
> I understand that llvm-mc has grown because of this, but previously it only worked by happenstance. Everything related to TargetMachine was confined to inline functions, and all of the classes managed to be incomplete, and so things "just worked", but that doesn't seem realistically tenable.

No, I don't think it worked by luck. TargetRegistry is designed so it doesn't need the details of the class implementations.

> 
> I started looking at how this could be fixed without the size increase to llvm-mc, but moving everything to Support really doesn't feel like the right solution.
> 
> Fundamentally, I feel like there are several different registries intermingled into one file here. Were we to separate them, they would each have a more logical home:
> 
> 1) Abstract Target / Triple registration. Essentially this is only concerned with mapping particular triples to some abstract target. This seems to make a lot of sense for Support. It also wouldn't (by default) register anything, simply exposing the fundamental APIs and the *means* of registering.
> 
> 2) MC-subsystem registration. This is the registration of various MC subsystems for a particular abstract target. This makes the most sense to live in the MC layer. The clients of this layer are already MC clients (Info, Desc, Disassembler ...). They could specialize the abstract target as an MCTarget (or some better name), and register it with the appropriate facilities.
> 
> 3) TargetMachine registration. This is the registration of the concrete target machine representation used by the codegen layer to model a particular target. This makes sense to live in the Target layer, and could either specialize MCTarget with any further information needed or (in the case of say CBackend) directly specialize the abstract target, etc.
> 
> 
> The reason I feel this shouldn't be sunk into Support is that it leaks all of these concepts (MC, TargetMachine, MCStreamer, InstPrinter, Disassembler) into the Support library. Clients of it might easily fail to depend on the appropriate libraries to actually cause entities to be registered that they want to query, etc etc. 

I think you are over-thinking this a bit. TargetRegistry is just a collection of routines for creating target specific implementation of Target and MC classes. It's a bunch of "support" routines for clients, nothing more. Sure, they can be organized into Target / MC parts or that "TargetInfo" can be moved to a separate library. But I think there is limited value there.

> 
> That said, the above is a lot more work, and not likely something I can tackle this week. What are your thoughts here? Is the current state one we can live with for a few weeks?

I am not comfortable with a few weeks. Is it not possible to just move TargetRegistry.cpp back to Support?

Evan

> 
> -Chandler

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110801/5b2a4f34/attachment.html>