Hi Eli,

Thanks for your comment.

I and my colleague tried to cut the cycle at struct type without returning null 
on ConvertType. This patch cuts cycle on just struct type with checking 
functions being processed. This method defers these struct types. I also send 
test case for this situation. I didn't have experience to make test case. 
Please check this test case and reivew the patch file. If there are something 
wrong, please let me know about that.

Thanks,
Jin-Gu Kang
________________________________________
From: Eli Friedman [[email protected]]
Sent: Thursday, November 08, 2012 4:47 PM
To: Jin Gu Kang
Cc: [email protected]
Subject: Re: [cfe-commits] [cfe-dev][patch] Strange LLVM IR result on recursive 
type

On Wed, Nov 7, 2012 at 6:02 PM, Jin Gu Kang <[email protected]> wrote:
> Hi Eli,
>
> Thank you for your reviewing.
>
>> Your patch is missing a testcase.
>> Please submit patches based on trunk
>
> I attach testcase file "recursive_struct.c". Before making patch file based 
> on trunk, I would like to explain the reason why I modified ConvertType not 
> FuctionType code. After reading the Clang code, I thought current Clang 
> generates struct type as type holder to cut the cycle of types on struct type 
> and function type. Type holder of struct type can be changed to correct 
> struct type later but one of function type can't be changed to correct 
> function type becase type holder is struct type. So I removed type holder 
> generation on function type and made only struct type cut the cycle of types. 
> There is one example to explain this situation.
>
> struct foo1 {
>   struct foo2* a;
>   struct foo3* b;
> };
> struct foo2 {
>   void (*func1)(struct foo1*);
> };
> struct foo3 {
>   void (*func1)(struct foo1*);
> };
> struct foo2 e;
>
> Cycle of types can be generated by struct type, function type and pointer 
> type. Type dependence graph of above code is as following.
>
>        ---> struct foo2
>        |           |
>        |          V
>        |  void (*func1)(struct foo1*) <----
> 1     |           |                                       |
>        |          V                                      |
>        |  void (func1)(struct foo1 *)        |
>        |           |                                      |
>        |          V                                     |
>        |  struct foo1*                               |
>        |           |                                       | 2
>        |          V                                      |
>        |  struct foo1                                 |
>        |____ _|                                       |
>                    |                                      |
>           struct foo3*                             |
>                    |                                      |
>           struct foo                                 |
>                    |___________________|
>
> Current Clang generates type holder at "struct foo2" to cut cycle 1 and  at 
> "void (func1)(struct foo1 *)" to cut cycle 2. My patch is to use type holder 
> of struct type to remove cycle 2 not type holder of function type. In other 
> words, function type and pointer type return null when they have cycle and 
> only struct type generates type holder. For example, on above graph, "void 
> (func1)(struct foo1)" know that it has cycle so returns null and defers its 
> type translation, and "void (func1)(struct foo1)" also returns null and 
> finally "struct foo" generates type holder. To implement this concept on 
> current Clang code, I modifed ConvertType function. What do you think about 
> this? I couldn't find other solutions without modifying llvm vmcore to 
> support type holder for function type. If modifying of ConvertType function 
> is not problem, I will make patch base on trunk.

By "testcase", I mean a patch to clang/test/.

Modifying ConvertType is fine.  I'm still not happy with making it
return null in some cases, particularly since there are obvious places
where ConvertType itself doesn't handle a null return from ConvertType
correctly.

There is another way to cut cycles: ConvertType already has a
mechanism to delay the conversion of a struct type (look for
"DeferredRecords" in CodeGenTypes.cpp).  It just isn't triggering for
your testcase.

-Eli

Attachment: RecursiveType.patch
Description: RecursiveType.patch

// RUN: %clang_cc1 -emit-llvm %s -o - | grep "{}*"

// This test case checks strange type conversion of cycled types.
// Strange result of this test case has "{}*"
struct foo1 {
  struct foo2* a;
  struct foo3* b;
};

struct foo2 {
  void (*func1)(struct foo1*);
};

struct foo3 {
  void (*func1)(struct foo1*);
};

struct foo2 e;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to