[llvm-commits] [patch] add gcroot attribute, test

[Chris Lattner]

On Sep 20, 2007, at 7:46 PM, Eric Christopher wrote:

> Here are a pair of patches to add an attribute to automatically  
> gcroot things based on type. It currently works for pointers to  
> structures only because a) that's all I need, and b) I think that's  
> all that makes sense really. I could remove the restriction if  
> someone can come up with a good use case for other types. Note that  
> we're also zeroing out automatic variables as well so if we do a  
> collection before they're initialized we don't end up following  
> garbage.

Hi Eric,

The patch looks great.  The only problem I have is this:

@@ -1732,6 +1734,8 @@ struct tree_type GTY(())
    unsigned packed_flag : 1;
    unsigned restrict_flag : 1;
    unsigned contains_placeholder_bits : 2;
+  /* APPLE LOCAL LLVM */
+  unsigned gcroot_flag : 1;

    unsigned lang_flag_0 : 1;
    unsigned lang_flag_1 : 1;

GCC carefully packs these bitfields so that there are exactly 32 of  
them.  With this addition, you'll push over and require an extra word  
for every tree.  Would it be possible to record the gcroot flag in  
the attributes list somehow?  This tradeoff seems worthwhile, because  
almost no structs will be marked as GCable.

Your testcase patch looks great, one suggested tweak:

+// RUN: %llvmgxx -c -emit-llvm %s -o - | llvm-dis | grep llvm.gcroot
+// RUN: %llvmgxx -c -emit-llvm %s -o - | llvm-dis | grep llvm.gcroot  
| count 6

I'd suggest changing these into:

+// RUN: %llvmgxx -S -emit-llvm %s -o - | grep llvm.gcroot
+// RUN: %llvmgxx -S -emit-llvm %s -o - | grep llvm.gcroot | count 6

... which removes an extra command invocation.

Thanks Eric!

-Chris


> Tested on x86-darwin.
>
> -eric
>
> <gcroot.diff.txt>
> <llvm.diff.txt>