dragonegg svn still broken

Despite the commit of...

Looks like that break came from http://llvm.org/viewvc/llvm-project?view=rev&revision=135154

Judging by this ArrayRef ctor you should be able to simplify those calls by changing the last two arguments from “&foo, 1” to, simply, “foo”.

Chris made major changes to the LLVM type system, which requires major changes
to dragonegg (c.f. the patch Chris applied to llvm-gcc). I'm on holiday which
is why I haven't taken care of it yet.

Ciao, Duncan.

I looked into this. The posted errors in Convert.cpp can be easily fixed as mentioned. However, compiling the Types.cpp file (the type stuff in dragonegg), yields to more complex errors, that are introduced e.g. by the removal of the Opaque type. So unfortunately it seems, we need to wait until someone spends a couple of hours/days on this.

@Duncan: Enjoy your vacations, such that you are full of energy to fix these bugs when you are back. :wink:

Cheers
Tobi

Yeah, sorry about that Duncan. I can't directly hack on the code, but I'm happy to answer questions when you get back.

-Chris

I've done this in r135472.

Jay.

Jay,
  I don't know if this is from a new commit but at r135473, dragonegg svn
fails to compile against FSF gcc 4.5.3 with the error...

Compiling DefaultABI.cpp
/sw/src/fink.build/dragonegg-gcc45-3.0-1/dragonegg-3.0/src/DefaultABI.cpp:194:18: error: use of undeclared identifier 'OpaqueType'
    Type *OpTy = OpaqueType::get(getGlobalContext());
                 ^
          Jack

I don't know if this is from a new commit but at r135473, dragonegg svn
fails to compile against FSF gcc 4.5.3 with the error...

Compiling DefaultABI.cpp
/sw/src/fink.build/dragonegg-gcc45-3.0-1/dragonegg-3.0/src/DefaultABI.cpp:194:18: error: use of undeclared identifier 'OpaqueType'
Type *OpTy = OpaqueType::get(getGlobalContext());
^

Nope, anything to do with OpaqueType or PATypeHolder is a result of
the type-system-rewrite merge.

Jay.

Compiling DefaultABI.cpp
/sw/src/fink.build/dragonegg-gcc45-3.0-1/dragonegg-3.0/src/DefaultABI.cpp:194:18: error: use of undeclared identifier 'OpaqueType'
     Type *OpTy = OpaqueType::get(getGlobalContext());
                  ^
           Jack

Dragonegg is broken until Duncan is back.

Hi Chris,

Chris made major changes to the LLVM type system, which requires major changes
to dragonegg (c.f. the patch Chris applied to llvm-gcc). I'm on holiday which
is why I haven't taken care of it yet.

Yeah, sorry about that Duncan. I can't directly hack on the code, but I'm happy to answer questions when you get back.

going on holiday is a dangerous business! I've resurrected, but will probably
have some questions since I'm seeing lots of testsuite failures.

Ciao, Duncan.

Hi Chris,

Chris made major changes to the LLVM type system, which requires major changes
to dragonegg (c.f. the patch Chris applied to llvm-gcc). I'm on holiday which
is why I haven't taken care of it yet.

Yeah, sorry about that Duncan. I can't directly hack on the code, but I'm happy to answer questions when you get back.

there seems to be a nasty flaw in the scheme you devised for llvm-gcc, involving
arrays. Consider the following mutually recursive arrays of structs:

   struct S has a field which is a pointer to array type A.
   The element type of array type A is struct T.
   struct T has a field which is a pointer to array type B.
   The element type of array type B is struct S.

When trying to convert one of the structs or arrays, with the current scheme you
inevitably end up forming an array of opaque struct types. This immediately
blows up dragonegg because it wants to compute the array size to see if it needs
to append padding. It can blow up llvm-gcc too, because it also wants to get
the size of arrays in more exotic circumstances.

Do you have any suggestions on the best way to handle this?

Here are some cheating approaches:

(1) when converting a pointer, if it is a pointer to an array type with
(recursively) a struct element type, then form a pointer to the struct
rather than a pointer to the array. This is probably fairly harmless as
far as optimization of code is concerned.

(2) never create array types: instead of [1000 x {i32}] create a struct with
1000 {i32} fields. Then in the above situation you can start off with an
opaque "array" (in fact struct) type and fill in fields later. This would
use up vast amounts of memory for big array types (like Ada's "array that
spans all of memory" types).

(3) convert all pointers to i8*. I'm not sure how much this would get in
the way of the optimizers. Note that dragonegg distinguishes between
register types and in-memory types (I think clang does the same), and pointer
types used for registers would still be sensible (struct S* rather than i8*).

A less cheating approach would be to not actually create LLVM types when
converting things, but instead some kind of front-end fake types for which
you can do type refinement on "pointers"; refine "pointers" when everything
has been converted; and only then generate LLVM types out of the resulting
front-end types.

Ciao, Duncan.

Hi Chris,

Chris made major changes to the LLVM type system, which requires major changes
to dragonegg (c.f. the patch Chris applied to llvm-gcc). I'm on holiday which
is why I haven't taken care of it yet.

Yeah, sorry about that Duncan. I can't directly hack on the code, but I'm happy to answer questions when you get back.

there seems to be a nasty flaw in the scheme you devised for llvm-gcc, involving
arrays.

I believe it. I just did the minimal possible thing to get llvm-gcc to bootstrap for me, because some people have trouble letting go of the past, I'm sorry for the impact that had on dragonegg. Clang is at least working now :slight_smile:

Consider the following mutually recursive arrays of structs:

struct S has a field which is a pointer to array type A.
The element type of array type A is struct T.
struct T has a field which is a pointer to array type B.
The element type of array type B is struct S.

When trying to convert one of the structs or arrays, with the current scheme you
inevitably end up forming an array of opaque struct types. This immediately
blows up dragonegg because it wants to compute the array size to see if it needs
to append padding. It can blow up llvm-gcc too, because it also wants to get
the size of arrays in more exotic circumstances.

Do you have any suggestions on the best way to handle this?

Right, there are similar cases involving function pointers etc. For example, here's an evil case where converting a struct converts a function pointer that wants the struct being converted (kudos to Eli):

struct foo {
  void (*FP)(struct foo);
} g;

If you run this through clang, you get:

%struct.foo = type { {}* }
@g = common global %struct.foo zeroinitializer, align 8

and any uses of "FP" do a bitcast to the right type. The way this works in clang (and the attempt in llvm-gcc which probably is pretty close just in need of debugging) is that when a struct is being converted, if we go to convert a pointer underneath it, we disable converting the pointee (just returning {}). At one stage in its development, this was pretty hardcore: we were lowering stuff like:

struct x { int a; };
struct y {
  struct x *P;
} h;

to y = "{ {}* }".

This is effectively your #3 below.

(3) convert all pointers to i8*. I'm not sure how much this would get in
the way of the optimizers. Note that dragonegg distinguishes between
register types and in-memory types (I think clang does the same), and pointer
types used for registers would still be sensible (struct S* rather than i8*).

Yep, I did this to beat out all of the assumptions in the clang IRGen that was assuming that the type return by a field access load was sane (they'd blow up when they got an unexpected {}* instead of a function pointer or whatever).

Once everything worked, I added the 'isSafeToConvert' logic in clang/lib/CodeGen/CodeGenTypes.cpp. The idea is that when converting the first "g" example, there is no way we can do better than {}*. OTOH, in the second "h" example there is no problem recursively converting X to a nice type, and isSafeToConvert returns true for it. Because we can recurse into it, we get pretty IR for that case (and this handles almost every other common case):

%struct.y = type { %struct.x* }
%struct.x = type { i32 }

@h = common global %struct.y zeroinitializer, align 8

This gets clang the benefit of getting pretty types that are maximally friendly to the optimizer (and the compiler hacking reading IR!) while still being able to fall back to an ugly type when it is required by conversion.

-Chris

Hi Chris, thanks for the pointers.

Right, there are similar cases involving function pointers etc. For example, here's an evil case where converting a struct converts a function pointer that wants the struct being converted (kudos to Eli):

struct foo {
   void (*FP)(struct foo);
} g;

I don't understand this - are function types with an opaque argument type not
allowed? If so, why aren't they allowed?

Ciao, Duncan.

As I understand it, conversion from a C function type to an LLVM
function type depends on target ABI details like whether a struct is
passed in multiple integer registers, and you can only work this out
properly if the parameter types and return type are complete.

Jay.

Hi Jay,