[llvm-commits] [llvm-gcc] Arbitrary Precition Integer Support

[Reid Spencer]

On Mon, 2007-02-19 at 14:41 -0800, Chris Lattner wrote:
> On Feb 19, 2007, at 12:37 PM, Reid Spencer wrote:
> 
> > Devang / Jim / Chris,
> >
> > The attached patch has been reviewed by Devang and provides the
> > "bitwidth" attribute and 3 builtin functions for arbitrary precision
> > integers (concat, bit select, part select). This is the same as the  
> > last
> > version except it has an additional check for a null pointer that
> > produced a failure in bootstrap.  That problem is now gone and this
> > patch passes the Integer test suite as well as MultiSource/Benchmarks.
> >
> > I would appreciate it if you could commit this one or indicate why it
> > could not be committed.
> 
> Problems:
> 
> @@ -1741,6 +1756,10 @@
>     unsigned lang_flag_5 : 1;
>     unsigned lang_flag_6 : 1;
>     unsigned user_align : 1;
> +  /* APPLE LOCAL begin LLVM bit accurate integer types - ras */
> +  unsigned user_bitwidth : 1;
> +  unsigned bitwidth : 23;
> +  /* APPLE LOCAL end LLVM bit accurate integer types - ras */
> 
> This increases the size of tree_type by an extra word.  Can you find  
> a place to store this that doesn't penalize the non-bitwidth-aware  
> case (e.g. a hash table on the side)?

No. Been down that road. Every attribute uses its own field, bitwidth is
no different. Given the huge size of these tree nodes, this isn't a huge
increment.  I understand the impact, but there's just no other way to do
it.  I tried using precision at one time but that just breaks 98% of the
gcc code.

> The changes around llvm_get_pseudo_builtin_index are quite invasive,  
> disabling many places in the compiler.  I don't know that there is a  
> better solution,

I don't like it either, but its either that or the gcc code gets nearly
totally rewritten (modes are useless, many assumptions about sizes of
things are useless, etc. etc.)

> but have you tried treating the builtins as if they  
> were varargs? 

that's how they are being treated.  I originally thought I could get
away with a varargs solution using regular builtins. Unfortunately,
there's nothing in C or GCC that allows the result of the function to be
"any bit width". So, the result type is i32 through gcc processing and
then fixed up by llvm-convert.cpp when it emits. Not great, but I can't
find another way to do this without re-writting 1/2 of gcc.

>  Alternatively, why not lazily create specific builtins  
> as you hit them.  For example if the user writes:
> 
> myint13 A, B;
> ...
>    __builtin_bit_concat(A, B);
> 
> the CFE would synthesize "__builtin_bit_concat13", with i13  
> arguments.  This seems like a simpler and more-local change than the  
> various pieces you have now.

Because gcc just can't do that. You can't define builtin functions "on
the fly". They have to be in the builtins.def file, for a variety of
reasons. If I've missed something, please enlighten me, but I went down
this road too while trying to come up with a solution.

Furthermore, for the intended use case (chip design), there could be a
lot of different prototypes create (100s to 1000s).

> 
> BTW, bsearch is totally overkill (i.e. much slower than a linear  
> search) if you have an array of 3 entries to search.

There were originally 9, but we got rid of 6. There could be more in the
future. I'll know more about this in 2-3 weeks. I left it in as bsearch
to facilitate easy extension. If there isn't any more in the future,
I'll turn this into linear scan, but not yet.

> 
> +  if (Callee) {
> +    // Handle arbitrary precision integer bit manipulation builtins
> +    Value* Res = 0;
> +    Function* Func = 0;
> +    if (ConstantExpr* CE = dyn_cast<ConstantExpr>(Callee)) {
> +      if (CE->getOpcode() == Instruction::BitCast)
> +        Func = dyn_cast<Function>(CE->getOperand(0));
> +    } else
> +      Func = dyn_cast<Function>(Callee);
> +    if (Func) {
> +      switch (llvm_get_pseudo_builtin_index(Func->getName().c_str())) {
> +        default: assert(0 && "Unexpected pseudo-builtin index");
> 
> This significantly slows down codegen (by calling  
> llvm_get_pseudo_builtin_index, which is implemented in terms of  
> string compares) of every single direct call compiled.  This isn't  
> acceptable.  Instead, do by-pointer comparisons (of decls), and only  
> after you first see one of the builtins being used.

There's no "decl" that corresponds to these pseudo-builtin functions,
just an identifier node.

> 
> +  // Set the result type on the expression since it might be != i32.
> +  llvm_set_type(TREE_TYPE(exp), Result->getType());
> 
> Why are you doing this?  Setting the type of an expression needs to  
> be done in the FE, now at expansion to llvm time.

How do you say in GCC tree parlance, "this type is any bit width and use
the actual bit width returned". Unfortunately, you can't. I don't have a
good solution for this. The problem is that the result gets
truncated/extended to what gcc thinks the expression is and gcc has no
way of dealing with integers > 64-bits (no applicable mode). Attempting
to make gcc deal with them is many man years of work in my opinion and
affects nearly everyting in GCC (I started down this road and quickly
found out the task was "huge").

> 
> 
> +  // FIXME: For some reason if we set the type of this expression,  
> the type of
> +  // "main" and all its variables become i1 too. This is a complete  
> mystery. In
> +  // the bit_concat.c test case, this function is called from a  
> function that
> +  // main calls. There's no way that "exp" refers to "main". How  
> could it?
> +  // ***BOGGLE***
> 
> I believe this is an artifact of the above: you're modifying the  
> GLOBAL, SHARED, INT TYPE!  Don't do this.
> Please figure out the right solution in the F.E.

The right solution is to re-write the F.E. I don't have that luxury in
the time allotment given to me.

> 
> 
> 
> +  /* The tree_type structure (tree.h) reserves 23 bits for the  
> bitwidth field.
> +   * Consequently we must ensure that the value provided fits within  
> 23 bits.
> +   * The number 23 is an artifact of LLVM's implementation of arbitrary
> +   * precision integer types as there were 23 bits left over in the  
> Type class.
> +   * (see include/llvm/Type.h). 2^23 > 8 million which should be  
> sufficient
> +   * for any foreseeable application.
> +   */
> 
> The GCC F.E. should allow any bitwidth (i.e. up to 2^32), the GCC- 
>  >LLVM converter should check for and reject anything greater than  
> 2^23 bits.

This feature is only enabled for LLVM anyway, so what's the diff?

> 
> nitpick:
> 
> * markers like "- ras" (which I assume is you) shouldn't exist.

Just following the pattern used elsewhere in llvm-gcc.

> 
> -Chris