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

[sil-generic-specializer] Don't specialize types which are too wide or too deep #7994

Merged
merged 1 commit into from Mar 9, 2017

Conversation

swiftix
Copy link
Contributor

@swiftix swiftix commented Mar 9, 2017

This improves the existing logic which is used to stop specialization for types that are too big to handle. It catches some pathological cases which hang the compiler.

Fixes rdar://30938882

@swiftix
Copy link
Contributor Author

swiftix commented Mar 9, 2017

@swift-ci please smoke test

return std::make_pair(Depth, Width);
}

if (auto FnTy = t->getAs<SILFunctionType>()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to handle metatypes;

func foo<T>(t: T) {
  foo(t: T.self)
}

This produces T.Type.Type.Type...

Also, function types can appear in unlowered position:

func foo<T>(t: [T]) {
  foo(t: [{ t[0] }])
}

This produces Array<() -> () -> () -> ... -> T>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points! I'll handle those cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this works just as well:

unsigned complexity = 0;
type.visit([&](Type t) { complexity++; });

if (complexity < 1000) ...

Then you don't have to special-case anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting idea. If it would work, it would simplify a lot the code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slavapestov I tried out this idea. It work nicely on all the examples you proposed. But it hangs the compiler on the computeNat test in specialize_deep_generics.swift. This is because in this example, the complexity of the type increases vertically very slowly. And since complexity does not distinguish between the growth of the type depth and the growth of the width, there is no easy way to bail earlier.

static const unsigned BoundGenericDepthThreshold = 50;
// a bound generic type with the depth higher than this threshold
static const unsigned TypeDepthThreshold = 50;
// Set the width threshold rather high, because some projects uses very wide
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think checking the width is necessary, we have no way to make a type 'wider' by your definition without also making it 'deeper'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the test-case added in this PR results in very wide tuples, which are not very deep ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the reason why I added the width checks. This way we bail faster if the width of the type grows much faster than depth.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. This test case has O(2^n) width and O(n) height. :-)

@slavapestov
Copy link
Member

I'm waiting for the first "compile-time computation using Swift's generic specializer" blog post. :)

@swiftix swiftix force-pushed the generic-specialization-fixes branch from ff91ece to 93a954f Compare March 9, 2017 20:09
…r too deep

This improves the existing logic which is used to stop specialization for types that are too big to handle. It catches some pathological cases which hang the compiler.

Fixes rdar://30938882
@swiftix swiftix force-pushed the generic-specialization-fixes branch from 93a954f to f07743b Compare March 9, 2017 20:10
@swiftix
Copy link
Contributor Author

swiftix commented Mar 9, 2017

@swift-ci please smoke test

1 similar comment
@swiftix
Copy link
Contributor Author

swiftix commented Mar 9, 2017

@swift-ci please smoke test

@swiftix swiftix merged commit 467c8af into apple:master Mar 9, 2017
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.

None yet

2 participants