[llvm-commits] [PATCH] PR889: devirtualize Value::printCustom

[Chris Lattner]

On Nov 14, 2011, at 8:07 AM, Jay Foad wrote:

> On 14 November 2011 09:53, Chandler Carruth <chandlerc at google.com> wrote:
>> I don't think this is an acceptable way to approach this. You're still
>> adding a dependency on CodeGen to VMCore. You've just wrapped it up into a
>> header file dependency instead of a link-time dependency. I think this is
>> likely to make our already troublesome layering story significantly worse.
>> VMCore should not know of or need anything CodeGen.
> 
> VMCore defines a class hierarchy rooted at Value. CodeGen wants to
> extend that hierarchy with a new subclass, PseudoSourceValue. All
> these classes of value want to be able print themselves (for use in
> debugging dumps if nothing else). If this is to be done without VMCore
> knowing anything about CodeGen then a virtual method seems like a very
> good way of doing it.
> 
> The point of the PR is that we want to do away with virtual methods to
> save space, because we won't need a vtable pointer in each object.
> Instead we can do manual dispatch on the object's ValueID. But I can't
> think of a good way for independent libraries (like CodeGen) to hook
> into this dispatch mechanism. Maybe they could register themselves
> with VMCore, saying "I want to extend the Value hierarchy with a new
> class with this ID"; but that doesn't sound like it would be very
> clean or efficient.
> 
> So unless anyone has any bright ideas, it seems to me that this PR is
> fundamentally at odds with LLVM's "already troublesome layering
> story".

The "right" way to fix this is to eliminate PseudoSourceValue, which has always been a disgusting hack.  Places in codegen that can uses PseudoSourceValue (e.g. the SDLoadNode and memoperands) should use a discriminated union of "Value* and SomeMachineLocationType".  PseudoSourceValue never should have happened in the first place.

-Chris