On Sep 28, 2007, at 2:04 PM, Chris Lattner <[EMAIL PROTECTED]> 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
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
