Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix detection of cyclic structs and unions #116

Merged
merged 1 commit into from
Jul 31, 2018
Merged

Conversation

kornilova203
Copy link
Member

@kornilova203 kornilova203 commented Jul 11, 2018

Closes #103
Closes #115

Removed CycleDetection because it supported only limited cases and also it could find a cycle in a unsuccessful place: in a record that uses value instead of pointer (cycles may be "broken" only in pointer fields).

So types in IR can contain cycles.
usesType method was modified such that it does not get stuck in cycles.

To do:

  • Break cycle only in one place
  • Add tests for unions and more complex tests for structs
  • Support more complex cases like fields of function pointer type

@kornilova203
Copy link
Member Author

I'll review it after #114 is merged

@kornilova203 kornilova203 changed the base branch from master to 0.3 July 22, 2018 05:13
@kornilova203
Copy link
Member Author

To break a cycle in one place:

  1. find all structs and unions in the cycle
  2. filter out records that use value type
  3. Decide where cycle will be broken. For example it can be the struct/union that is defined after all other structs/unions or struct/union name of which is alphabetically largest/lowest.

It is also a good idea to minimize amount of fields where a cycle is broken (this is relevant for situations when one or more fields belong to multiple cycles).
It looks like a problem of finding minimum dominating set:

  1. create a node for each cycle and for each cyclic field
  2. connect each node of cyclic field to nodes of corresponding cycles.
  3. connect all nodes of cyclic fields to each other.
  4. find a minimum dominating set.
  5. cut cycles in node that belong to mds.
    (it is always possible to find minimum dominating set that consist only of cyclic fields nodes)

Maybe it can be done by simply picking fields that belong to maximum number of cycles, but I cannot prove that now.
Anyway, this situation is pretty rare, so I am not going to solve that now 😄

@kornilova203
Copy link
Member Author

Just for completeness:
If we want to minimize number of "cuts" that brake cycles it should not be done by "cutting" fields that are shared by maximum number of cycles. Consider this situation:

According to greedy algorithm we would "cut" vertex B and then A and C but optimal solution is to "cut" only A and C.

@kornilova203 kornilova203 force-pushed the recursive-structs branch 4 times, most recently from 6833e33 to 2be6f62 Compare July 26, 2018 04:40
@kornilova203
Copy link
Member Author

All supported cases are listed in tests/samples/Cycles.h
I think that this commit covers all possible cases but maybe I am not aware of some of them

@kornilova203 kornilova203 changed the title [WIP] Fix detection of cyclic structs and unions Fix detection of cyclic structs and unions Jul 26, 2018
@kornilova203 kornilova203 requested a review from jonas July 26, 2018 05:41
@kornilova203 kornilova203 force-pushed the recursive-structs branch 3 times, most recently from b752706 to bc0fea7 Compare July 30, 2018 07:27
Break cycle only in one place.
Support breaking cycles on complex types.
Remove `avoid` parameter in TypeTranslator methods.
@kornilova203 kornilova203 merged commit 78584f0 into 0.3 Jul 31, 2018
@kornilova203 kornilova203 deleted the recursive-structs branch July 31, 2018 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant