[llvm-commits] GC patches again

[Gordon Henriksen]

On Sep 28, 2007, at 2:04 PM, Chris Lattner <clattner at apple.com> wrote:

> BTW, thank you for splitting up this patch!  It makes it much easier
> to review.

No problem!

>> //===-- gc-3-collector.patch (+531) ---------------------------===//
>>
>>  include/llvm/CodeGen/Collector.h (+134)
>>  lib/CodeGen/Collector.cpp (+359)
>>  lib/CodeGen/README.txt (+38)
>>
>> Collector is the base class for garbage collector code generators.
>> This version enhances the previous patch to add root initialization
>> as discussed here:
>
> +#ifndef LLVM_CODEGEN_GC_H
> +#define LLVM_CODEGEN_GC_H
>
> Should probably be LLVM_CODEGEN_COLLECTOR_H
>
> +
> +#include "llvm/CodeGen/CollectorMetadata.h"
> +#include <ostream>
>
> plz use <iosfwd> instead of <ostream> or neither if you don't need  
> them.
>
> +namespace llvm {
> +
> +  class AsmPrinter;
> +  class FunctionPassManager;
> +  class Pass;
> +  class PassManager;
> +  class TargetAsmInfo;
>
> You can probably remove some of these.
>
> In Collector.cpp, it looks like you have several redundant #include's.
>
> Otherwise, looks great, plz commit.
>
>> //===-- gc-4-integration.patch (+116 -17) ---------------------===//
>>
>> In this patch, Collector winds its tendrils throughout the compiler.
>> Overhead should be minimal when disabled.
>>
>> I would particularly appreciate any feedback on this interface.
>> The primary item of concern to me is that I exposed the desired
>> collector to the compiler using a global. I have not decided on a
>> better approach. In the meantime, it works and is simple.
>
> I agree, this is not the right way to go.  There are two problems:
>
> 1. introducing global data means that two instances of the codegen
> can't be made at the same time.  This is not acceptable.  Fixing this
> should be relatively straight forward: hang any mutable data off
> MachineFunction or MachineModuleInfo as appropriate, and any
> immutable target info off TargetMachine.

Okay.

> 2. Adding a -gc option to llc isn't what we really want.

Right. I think it should be attached to the module (perhaps via the  
[sub-]target) so that the module is self-contained and magic command- 
line options are not needed.

I toyed with the idea of attaching it to the function to allow linking  
modules with differing collectors. That might be a good idea.

I do not think attaching it to the operations (ala EH) is wise;  
different collectors could have differing requirements of the stack  
layout. This precludes inlining Java into Ocaml, but the null  
collector doesn't count.

> Ideally, the code generator would generate gc info for a function  
> iff there is a gcroot in it.  This would allow the codegen to emit  
> GC code code that obviously uses it.  Is there any case where the  
> codegen has to do something for a function with no gcroots (or  
> declared safepoints
> etc) in it?

There is.

- Ocaml needs to know the frame size at each safe point, regardless of  
whether there are roots in the function.

- Sun's JVM does something analogous in that it enforces a stack  
layout so that its stack walker (and HotSpot) can work.

So, generally, safe points are safe points regardless of whether the  
function has roots.

-- Gordon
sent from my iPhone