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

[Chris Lattner]

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)?


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, but have you tried treating the builtins as if they  
were varargs?  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.

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


+  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.

+  // 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.


+  // 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 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.

nitpick:

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

-Chris