llc c backend can produce code that doesn't compile on gcc 4.x

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

llc c backend can produce code that doesn't compile on gcc 4.x

Eric van Riet Paap-2
Hello,

I would like to ask the llvm developers to have a look at http://llvm.org/bugs/show_bug.cgi?id=918 .
This bug has been reported 4 month ago but is none the less a somewhat serious one.

Below I have pasted the test case and output of the issue running on my ppc machine.

thank you
Eric


pb:~ eric$ cat testme.ll;llvm-as -f testme.ll;llc -march=c -f testme.bc;gcc -c testme.cbe.c ;gcc --version;llc --version
%structtype_s = type { i32 }
%fixarray_array3 = type [3 x %structtype_s]

define i32 %witness(%fixarray_array3* %p) {
    %q = getelementptr %fixarray_array3* %p, i32 0, i32 0, i32 0
    %v = load i32* %q
    ret i32 %v
}
testme.cbe.c:106: error: array type has incomplete element type
testme.cbe.c:119: warning: conflicting types for built-in function 'malloc'
powerpc-apple-darwin8-gcc-4.0.0 (GCC) 4.0.0 (Apple Computer, Inc. build 5026)
Copyright (C) 2005 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Low Level Virtual Machine (http://llvm.org/):
  llvm version 2.0cvs
  Optimized build with assertions.
pb:~ eric$ 

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: llc c backend can produce code that doesn't compile on gcc 4.x

Nick Lewycky
Eric van Riet Paap wrote:
> *testme.cbe.c:106: error: array type has incomplete element type*

The problem code boils down to:

  /* Structure forward decls */
  struct l_structtype_s;

  /* Typedefs */
  typedef struct l_structtype_s l_fixarray_array3[3];

which is illegal C, but perfectly valid C++, and g++ accepts it.

The structure contents are defined right afterwards, but I assume that
the typedefs are used when emitting the structure contents? We may have
to put fully defined structures first and typedefs second.

Nick Lewycky
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: llc c backend can produce code that doesn't compile on gcc 4.x

Chris Lattner
On Mon, 15 Jan 2007, Nick Lewycky wrote:

> Eric van Riet Paap wrote:
>> *testme.cbe.c:106: error: array type has incomplete element type*
>
> The problem code boils down to:
>
>  /* Structure forward decls */
>  struct l_structtype_s;
>
>  /* Typedefs */
>  typedef struct l_structtype_s l_fixarray_array3[3];
>
> which is illegal C, but perfectly valid C++, and g++ accepts it.
>
> The structure contents are defined right afterwards, but I assume that
> the typedefs are used when emitting the structure contents? We may have
> to put fully defined structures first and typedefs second.

Looks like it.  It sounds like the CBE should build an ordering of types,
based on their nesting properties, then emit them in nesting order.
Because all recursive types have to go through a pointer in C, we should
get a dag of types, which is easy to emit.

Anyone want to take a crack at it?

-Chris

--
http://nondot.org/sabre/
http://llvm.org/
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: llc c backend can produce code that doesn't compile on gcc 4.x

Gordon Henriksen-3
On 2007-01-15, at 11:48, Chris Lattner wrote:

> On Mon, 15 Jan 2007, Nick Lewycky wrote:
>
>> The structure contents are defined right afterwards, but I assume  
>> that the typedefs are used when emitting the structure contents?  
>> We may have to put fully defined structures first and typedefs  
>> second.
>
> Looks like it.  It sounds like the CBE should build an ordering of  
> types, based on their nesting properties, then emit them in nesting  
> order. Because all recursive types have to go through a pointer in  
> C, we should get a dag of types, which is easy to emit.

A DAG traversal is already performed, but only for struct types. It  
is implemented in CWriter::printContainedStructs (lib/Target/CBackend/
CBackend.cpp:1722).

The bug is in the calling function, printModuleTypes, which prints  
typedefs for all named types before performing the dependency-
sensitive traversal. For array typedefs, this ordering is incorrect;  
the element type must be defined first. But consider something like  
[2 x { [2 x {int}] }]; these two stages must be interleaved.

The simplest solution is to avoid typedefs for array types. With  
names of array types removed from the symbol table, llc will simply  
output (for instance) 'l_structtype_s[3]' instead of the equivalent  
'l_fixarray_array3'.

A more ambitious fix would be to merge the "typedef" and "structure  
contents" loops into a single, substantially more complex dependency-
ordered traversal. This would preserve type names from the bytecode  
file—but to what end?

Here's the patch and test:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of- 
Mon-20070115/042760.html

A workaround for this bug was to avoid naming array types in .ll files.

— Gordon


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: llc c backend can produce code that doesn't compile on gcc 4.x

Chris Lattner
On Tue, 16 Jan 2007, Gordon Henriksen wrote:
> The simplest solution is to avoid typedefs for array types. With
> names of array types removed from the symbol table, llc will simply
> output (for instance) 'l_structtype_s[3]' instead of the equivalent
> 'l_fixarray_array3'.

Very nice catch and approach.

> A more ambitious fix would be to merge the "typedef" and "structure
> contents" loops into a single, substantially more complex dependency-
> ordered traversal. This would preserve type names from the bytecode
> file—but to what end?

Good question, I can't think of a reason!  I applied your patch but
generalized it slightly.  The CBE now strips off any non-struct type
names, to avoid issues with vectors (if the CBE starts supporting them in
the future).

Thanks Gordon!

-Chris

--
http://nondot.org/sabre/
http://llvm.org/
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: llc c backend can produce code that doesn't compile on gcc 4.x

Gordon Henriksen-3
On 2007-01-16, at 02:47, Chris Lattner wrote:

On Tue, 16 Jan 2007, Gordon Henriksen wrote:

The simplest solution is to avoid typedefs for array types. With names of array types removed from the symbol table, llc will simply output (for instance) 'l_structtype_s[3]' instead of the equivalent 'l_fixarray_array3'.

Very nice catch and approach.

Thanks!

A more ambitious fix would be to merge the "typedef" and "structure contents" loops into a single, substantially more complex dependency-ordered traversal. This would preserve type names from the bytecode file—but to what end?

Good question, I can't think of a reason!  I applied your patch but generalized it slightly.  The CBE now strips off any non-struct type names, to avoid issues with vectors (if the CBE starts supporting them in the future).

Excellent. I was going to suggest just that generalization, but didn't feel comfortable enough with the Type hierarchy to be sure it was safe.

— Gordon


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev