Teach EagerSpecializer how to use the new form of the @_specialize attribute #7277

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.

Sign up for GitHub

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

Jump to bottom

Merged

swiftix merged 7 commits into swiftlang:master from swiftix:wip-partial-pre-specializations-wip-specialize-attribute-2

Feb 9, 2017

Contributor

swiftix commented Feb 6, 2017 •

edited

Loading

Teach EagerSpecializer how to use the new form of the @_specialize attribute

In addition to supporting the creation of full specializations, the EagerSpecializer changes contain some code for generating the layout-constrained partial specializations as well.

swiftix changed the title ~~Wip partial pre specializations wip specialize attribute 2~~ Teach EagerSpecializer how to use the new form of the @_specialize attribute

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from 1f063d4 to 5365bda Compare

February 6, 2017 21:27

Contributor Author

swiftix commented Feb 6, 2017

@swift-ci please smoke test

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/IPO/EagerSpecializer.cpp Outdated

    
                assert(!CanSILFuncTy->isPolymorphic());

                auto CalleeSubstFnTy = CanSILFuncTy;

                ArrayRef<Substitution> Subs;

                //assert(!CanSILFuncTy->isPolymorphic());

Contributor

slavapestov Feb 6, 2017

Remove this line?

lib/SILOptimizer/IPO/EagerSpecializer.cpp Outdated

    
                  assert(!CanSILFuncTy->isPolymorphic());

                  return Builder.createApply(CalleeFunc->getLocation(), FuncRefInst, CallArgs,

                                             false);

                  //assert(!CanSILFuncTy->isPolymorphic());

Contributor

slavapestov Feb 6, 2017

Ditto

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from 5365bda to 1683f05 Compare

February 7, 2017 03:06

Contributor Author

swiftix commented Feb 7, 2017

@swift-ci please smoke test

1 similar comment

Contributor Author

swiftix commented Feb 7, 2017

@swift-ci please smoke test

Contributor

slavapestov commented Feb 7, 2017

Sorry, the merge conflicts are from the ArrayRef<Substitution> => SubstitutionList renaming. They should be completely mechanical to resolve.

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from 1683f05 to fe4a98f Compare

February 7, 2017 15:20

Contributor Author

swiftix commented Feb 7, 2017

@swift-ci please smoke test

1 similar comment

Contributor Author

swiftix commented Feb 7, 2017

@swift-ci please smoke test

swiftix added 4 commits

February 7, 2017 23:16


          Generic specializer API changes to prepare for the new @_specialize a…

26c963f

…ttribute and partial specializations.


          Add mangling support for partial specializations.

2b9444e


          Teach EagerSpecializer how to use the new form of the @_specialize at…

ec4f28e

…tribute

In addition to supporting the creation of full specializations, the EagerSpecializer changes contain some code for generating the layout-constrained partial specializations as well.


          Teach SIL type lowering about trivial, but address only types.

6f76726

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from fe4a98f to a8ae2bd Compare

February 8, 2017 09:08

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test

Contributor Author

swiftix commented Feb 8, 2017

@slavapestov OK, so this is supposed to be a working version of the @_specialize. I implemented the code generation for the dynamic dispatch and IRGen changes. Tests are updated to test these new features.

The function ReabstractionInfo::ReabstractionInfo(SILFunction *OrigF, ArrayRef<Requirement> Requirements) in Generics.cpp may need to be improved later to use your conformance lookup fixes.


          Teach IRGen how to work with archetypes of fixed sizes.

cbe4b0d

This allows IRGen to use direct loads and stores instead of calling value witness tables.

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from a8ae2bd to cbe4b0d Compare

February 8, 2017 15:31

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from d521d0a to 072ed25 Compare

February 8, 2017 16:24

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test Linux

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test Linux

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from 072ed25 to d48d171 Compare

February 8, 2017 17:03

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test Linux

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test Linux

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please smoke test OS X

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 5365bda86db14e7b727000d94f76729157f01296
Test requested by - @swiftix

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - d48d171e125191a25066e9bd3825fe83e4dadb75
Test requested by - @swiftix

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - d48d171e125191a25066e9bd3825fe83e4dadb75
Test requested by - @swiftix

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from d48d171 to 7a0013a Compare

February 8, 2017 21:33

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 7a0013ac903bf759f76f992574f6df99dcb72ffd
Test requested by - @swiftix

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 7a0013ac903bf759f76f992574f6df99dcb72ffd
Test requested by - @swiftix


          Fix the test to work on objc-interop enabled and disabled platforms.

e05dee1

swiftix force-pushed the wip-partial-pre-specializations-wip-specialize-attribute-2 branch from 7a0013a to e05dee1 Compare

February 8, 2017 22:54

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp Outdated

+                    FnTy->getOptionalErrorResult(), M.getASTContext());
+                // This is an interface type. It should not have any archetypes.
+                if (NewFnTy->hasArchetype()) {

Contributor

slavapestov Feb 8, 2017

Remove these three lines

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp Outdated

+                    assert(FirstType && SecondType);
+                    bool isFirstTypeNonConcrete =
+                        FirstType->hasArchetype() || FirstType->hasTypeParameter();

Contributor

slavapestov Feb 8, 2017

hasArchetype should always be false -- can you assert?

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp Outdated

+                    bool isSecondTypeNonConcrete =
+                        SecondType->hasArchetype() || SecondType->hasTypeParameter();
+                    // Only one of the types should be concrete.
+                    assert((isFirstTypeNonConcrete ^ isSecondTypeNonConcrete) &&

Contributor

slavapestov Feb 8, 2017

I would find != more readable than ^ here

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp Outdated

+                for (auto &Req : Requirements) {
+                  if (Req.getKind() == RequirementKind::SameType) {
+                    auto CallerArchetype = dyn_cast<SubstitutableType>(

Contributor

slavapestov Feb 8, 2017

dyn_cast can return null; either use cast or handle the null case

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp

+                            ->getCanonicalType());
+                    // Remember that a given generic parameter is mapped
+                    // to a concrete type.
+                    CallerArchetypeToInterfaceMap.addSubstitution(

Contributor

slavapestov Feb 8, 2017

addSubstitution() is going to be private. I just refactored a dozen usages of it, please don't add new ones. Substitution maps should be constructed by using GenericSignature::getSubstitutionMap() or GenericEnvironment::getSubstitutionMap()

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp Outdated

+                for (auto Sub : ForwardingSubs) {
+                  auto ClonerSub = Sub.subst(
+                      SM, QuerySubstitutionMap{ClonerArchetypeToConcreteMap},
+                      LookUpConformanceInModule(SM));

Contributor

slavapestov Feb 8, 2017

Add a FIXME comment to clean up the module conformance lookup (for me :))

slavapestov reviewed

View reviewed changes

lib/SILOptimizer/Utils/Generics.cpp

+                // Substitute into forwarding substitutions to get the cloner substitutions
+                // and substitutions to be used by the caller for calling the specialized
+                // function.
+                SmallVector<Substitution, 4> ClonerSubsList;

Contributor

slavapestov Feb 8, 2017

Instead, construct these arrays by calling GenericSignature::getSubstitutions() or GenericEnvironment::getSubstitutions(). Otherwise you're relying on them having entries in the same order as the forwarding substitutions.


          Code cleanups based @slavapestov's review

c3bbe64

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - e05dee1
Test requested by - @swiftix

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

1 similar comment

Contributor Author

swiftix commented Feb 8, 2017

@swift-ci please test

Contributor

slavapestov commented Feb 8, 2017

Ok, I think this is good to go now.

Contributor

swift-ci commented Feb 8, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - c3bbe64
Test requested by - @swiftix

Contributor Author

swiftix commented Feb 9, 2017

@swift-ci please smoke test

1 similar comment

Contributor Author

swiftix commented Feb 9, 2017

@swift-ci please smoke test

swiftix merged commit 163f58e into swiftlang:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet