The following code:

---
char Global[100];
char *test1(unsigned i) {
  return &Global[i];
}
---

Turns into the following LLVM code:
---
        %tmp.1 = cast uint %i.1 to [100 x sbyte]*
        %tmp.4 = cast [100 x sbyte]* %tmp.1 to long
        %tmp.5 = add long %tmp.4, cast ([100 x sbyte]* %Global to long)
        %tmp.6 = cast long %tmp.5 to sbyte*
        ret sbyte* %tmp.6
---

This is disgusting, it should use a getelementptr instruction.  Testcase is:
test/Regression/CFrontend/2003-11-03-AddrArrayElement.c.tr


-Chris

This turned out to be easier than I thought.  I wish there was some way to find
ALL of them though.  :(

$ diff -u c-typeck.c~ c-typeck.c
--- c-typeck.c~ 2003-08-19 17:22:52.000000000 -0500
+++ c-typeck.c  2003-11-07 00:11:04.000000000 -0600
@@ -2379,7 +2379,7 @@
        }
 
       /* For &x[y], return x+y */
-      if (TREE_CODE (arg) == ARRAY_REF)
+      if (TREE_CODE (arg) == ARRAY_REF && !EMIT_LLVM)
        {
          if (!c_mark_addressable (TREE_OPERAND (arg, 0)))
            return error_mark_node;

---

The testcase now produces:
sbyte* %test1(uint %i.1) {
        %tmp.1 = cast uint %i.1 to long        
        %tmp.2 = getelementptr [100 x sbyte]* %Global, long 0, long %tmp.1     
        
        ret sbyte* %tmp.2
}

... as expected.  Long live uncollapsed DSNodes!

-Chris

Here's another horrible case, this one found in the pool allocator:

struct foo {
  int array[100];
  void *getAddr(unsigned i);
};

void *foo::getAddr(unsigned i) { 
  return &array[i];
}

This becomes:
sbyte* %_ZN3foo7getAddrEj(%struct.foo* %this.1, uint %i.1) {
entry:          ; No predecessors!
        %tmp.4 = shl uint %i.1, ubyte 2         ; <uint> [#uses=1]
        %tmp.5 = cast uint %tmp.4 to int*               ; <int*> [#uses=1]
        %tmp.6 = cast %struct.foo* %this.1 to long              ; <long> [#uses=1]
        %tmp.7 = cast int* %tmp.5 to long               ; <long> [#uses=1]
        %tmp.8 = add long %tmp.7, %tmp.6                ; <long> [#uses=1]
        %tmp.9 = cast long %tmp.8 to sbyte*             ; <sbyte*> [#uses=1]
        ret sbyte* %tmp.9
}

Testcase here: test/Regression/C++Frontend/2003-11-08-ArrayAddress.cpp.tr

Ugh.

-Chris

Wow, this is certainly QUITE amazing.  The reason that the problem resurfaced in
the C++ front-end is that it contains a COPY of a huge amount of code from the C
frontend, including this hunk of code.  Nasty nasty nasty.  At least this one
was easy to fix.

Here's the patch:

$ diff -u cp/typeck.c~ cp/typeck.c
--- typeck.c~   2003-08-27 16:50:48.000000000 -0500
+++ typeck.c    2003-11-08 17:05:24.000000000 -0600
@@ -3882,7 +3882,7 @@
        }
 
       /* For &x[y], return x+y */
-      if (TREE_CODE (arg) == ARRAY_REF)
+      if (TREE_CODE (arg) == ARRAY_REF && !EMIT_LLVM)
        {
          if (!cxx_mark_addressable (TREE_OPERAND (arg, 0)))
            return error_mark_node;


The second testcase now generates the following code:
sbyte* %_ZN3foo7getAddrEj(%struct.foo* %this.1, uint %i.1) {
entry:          ; No predecessors!
        %tmp.3 = cast uint %i.1 to long         ; <long> [#uses=1]
        %tmp.4 = getelementptr %struct.foo* %this.1, long 0, ubyte 0, long
%tmp.3               ; <int*> [#uses=1]
        %tmp.5 = cast int* %tmp.4 to sbyte*             ; <sbyte*> [#uses=1]
        ret sbyte* %tmp.5
}

Here's another testcase:
char *test(char* C) {
  return C-1;   // Should turn into a GEP
}

This turns into:

sbyte* %test(sbyte* %C.1) {
entry:          ; No predecessors!
        %tmp.4 = cast sbyte* %C.1 to long               ; <long> [#uses=1]
        %tmp.6 = add long %tmp.4, -1            ; <long> [#uses=1]
        %tmp.7 = cast long %tmp.6 to sbyte*             ; <sbyte*> [#uses=1]
        ret sbyte* %tmp.7
}

This testcase is: CFrontend/2003-11-08-PointerSubNotGetelementptr.c.tr

-Chris

Fixing this one was more challenging:

$ diff -u llvm-expand.c~ llvm-expand.c
--- llvm-expand.c~      2003-11-06 10:21:33.000000000 -0600
+++ llvm-expand.c       2003-11-08 18:21:08.000000000 -0600
@@ -2902,13 +2902,58 @@
   /*return 0;    // Used to disabled addsub transformation */
 
   assert(DestTy->ID == PointerTyID && op0->Ty->ID == PointerTyID);
-  if (!isAdd) return 0;   /* Only handle add so far */
 
   /* Dig through constant-expr casts. */
   if (op1->VTy == ConstantExpr &&
       ((llvm_constant_expr*)op1)->Inst->Opcode == O_Cast)
     op1 = ((llvm_constant_expr*)op1)->Inst->Operands[0];
 
+  /* Handle array indexing cases... */
+  if (DestTy == op0->Ty) {
+    llvm_type *ETy = GET_POINTER_TYPE_ELEMENT(DestTy);
+    unsigned ETySize = llvm_type_get_size(ETy);
+
+    /* If we are indexing through a one byte element types, turn it directly
+     * into a getelementptr instruction.
+     */
+    if (ETySize == 1) {
+      llvm_instruction *I = llvm_instruction_new(DestTy, "tmp",
+                                                 O_GetElementPtr, 2);
+      I->Operands[0] = op0;
+      I->Operands[1] = cast_if_type_not_equal(Fn, op1, LongTy);
+      if (!isAdd) {
+        /* If this is a subtract, negate the index! */
+        I->Operands[1] = append_inst(Fn, create_binary_inst("tmp", O_Sub,
+                                    llvm_constant_long_0, I->Operands[1]));
+      }
+
+      return append_inst(Fn, I);
+    }
+
+    /* Otherwise this was not a one byte element.  See if the constant we are
+     * adding or subtracting is a multiple of the type size.
+     */
+    if (op1->VTy != Constant) return 0;  /* Only works for constants */
+    if (V2C(op1)->Repr[0] < '0' || V2C(op1)->Repr[0] > '9')
+      return 0; /* Not a positive numeric constant? */
+    Value = llvm_constant_get_integer_val(V2C(op1)); /* Get the constant value*
/
+
+    /* We can only do this if it is an exact multiple! */
+    if (Value % ETySize == 0) {
+      long long GEPIdx = Value/ETySize;
+      llvm_instruction *I = llvm_instruction_new(DestTy, "tmp",
+                                                 O_GetElementPtr, 2);
+
+      if (!isAdd) GEPIdx *= -1;
+
+      I->Operands[0] = op0;
+      I->Operands[1] = llvm_constant_new_integral(LongTy, GEPIdx);
+      return append_inst(Fn, I);
+    }
+  }
+
+  if (!isAdd) return 0;   /* Only handle add so far */
+
   if (op1->VTy != Constant) return 0;  /* Only works for constants */
   if (V2C(op1)->Repr[0] < '0' || V2C(op1)->Repr[0] > '9')
     return 0; /* Not a positive numeric constant? */

Division by zero is bad:

$ diff -u llvm-expand.c~ llvm-expand.c
--- llvm-expand.c~      2003-11-08 18:21:08.000000000 -0600
+++ llvm-expand.c       2003-11-08 22:13:27.000000000 -0600
@@ -2939,7 +2939,7 @@
     Value = llvm_constant_get_integer_val(V2C(op1)); /* Get the constant value*/
 
     /* We can only do this if it is an exact multiple! */
-    if (Value % ETySize == 0) {
+    if (ETySize && Value % ETySize == 0) {
       long long GEPIdx = Value/ETySize;
       llvm_instruction *I = llvm_instruction_new(DestTy, "tmp",
                                                  O_GetElementPtr, 2);

Here's another patch.  The corresponding fix was already in the C frontend. 
Ugh.

* Fix address of structure element:

$ diff -u typeck.c~ typeck.c
--- typeck.c~   2003-11-08 17:05:24.000000000 -0600
+++ typeck.c    2003-11-09 11:34:20.000000000 -0600
@@ -4000,7 +4000,8 @@
          }
        else
          {
-           /* Unfortunately we cannot just build an address
+            tree addrLLVMTmp;
+           /* Unfortunately we cxannot just build an address
               expression here, because we would not handle
               address-constant-expressions or offsetof correctly.  */
            tree field = TREE_OPERAND (arg, 1);
@@ -4013,6 +4014,15 @@
            rval = build_nop (argtype, rval);
            addr = fold (build (PLUS_EXPR, argtype, rval,
                                cp_convert (argtype, byte_position (field))));
+
+            addrLLVMTmp = fold(build1(ADDR_EXPR, argtype, arg));
+            if (EMIT_LLVM &&
+                (TREE_CONSTANT(addrLLVMTmp) || !TREE_CONSTANT(addr))) {
+              /* For LLVM, don't fold pointer arithmetic unless doing so
+               * produces a constant!
+               */
+              addr = addrLLVMTmp;
+            }
          }

        if (TREE_CODE (argtype) == POINTER_TYPE

Here's yet another testcase which is causing bad things to happen.  :(  This
equally affects the C and C++ front-ends.

#include <stdlib.h>

int *test(int Y) {
  int *X = (int*)malloc(4);

  return X + Y;
}
---
Generates:

int* %_Z4testi(int %Y.1) {
entry:          ; No predecessors!
        %tmp.0 = malloc int             ; <int*> [#uses=1]
        %tmp.4 = cast int %Y.1 to uint          ; <uint> [#uses=1]
        %tmp.5 = shl uint %tmp.4, ubyte 2               ; <uint> [#uses=1]
        %tmp.6 = cast uint %tmp.5 to int*               ; <int*> [#uses=1]
        %tmp.7 = cast int* %tmp.0 to long               ; <long> [#uses=1]
        %tmp.8 = cast int* %tmp.6 to long               ; <long> [#uses=1]
        %tmp.9 = add long %tmp.7, %tmp.8                ; <long> [#uses=1]
        %tmp.10 = cast long %tmp.9 to int*              ; <int*> [#uses=1]
        ret int* %tmp.10
}

Testcase here:
llvm/test/Regression/CFrontend/2003-11-13-TypeSafety.c.tr

Created an attachment (id=21) [details]
Patch to fix latest problem.

This should fix the (ptr) + (intval) problem.

-Chris