Skip to content

[cxx-interop] Inline constexpr vars. #35311

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

Merged
merged 1 commit into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/AST/ASTScopeCreation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ void ASTScope::
}

void ASTScope::expandFunctionBody(AbstractFunctionDecl *AFD) {
auto *const SF = AFD->getParentSourceFile();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the problem here is that we've never actually had a C++ function with a body before. And because there's no source file associated with a C++ decl context, it makes sense that this might be null.

SF->getScope().expandFunctionBodyImpl(AFD);
if (auto *const SF = AFD->getParentSourceFile())
Copy link
Member

Choose a reason for hiding this comment

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

This should be const auto * rather than auto * const which makes the pointer const. While the pointer is nice to make const, it is not as valuable as the value itself being marked const.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that doesn't work because getScope is not a const member (and can't be because it lazily creates the scope). So, I'm going to leave this as-is.

Copy link
Member

Choose a reason for hiding this comment

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

I think that this case deserves a comment to explain why/when we expect a source to be missing.

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 point. Fixed in a follow up here: #35364.

SF->getScope().expandFunctionBodyImpl(AFD);
}

void ASTScope::expandFunctionBodyImpl(AbstractFunctionDecl *AFD) {
Expand Down
46 changes: 46 additions & 0 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4094,6 +4094,47 @@ namespace {
return nullptr;
}

AccessorDecl *tryCreateConstexprAccessor(const clang::VarDecl *clangVar,
VarDecl *swiftVar) {
assert(clangVar->isConstexpr());
clangVar->evaluateValue();
auto evaluated = clangVar->getEvaluatedValue();
if (!evaluated)
return nullptr;

// If we have a constexpr var with an evaluated value, try to create an
// accessor for it. If we can remove all references to the global (which
// we should be able to do for constexprs) then we can remove the global
// entirely.
auto accessor = AccessorDecl::create(
Impl.SwiftContext, SourceLoc(), SourceLoc(), AccessorKind::Get,
swiftVar, SourceLoc(), StaticSpellingKind::KeywordStatic,
/*throws=*/false, SourceLoc(), /*genericParams*/ nullptr,
ParameterList::createEmpty(Impl.SwiftContext), swiftVar->getType(),
swiftVar->getDeclContext());
Expr *value = nullptr;
// TODO: add non-numeric types.
if (evaluated->isInt()) {
value = IntegerLiteralExpr::createFromUnsigned(
Impl.SwiftContext,
static_cast<unsigned>(
clangVar->getEvaluatedValue()->getInt().getZExtValue()));
} else if (evaluated->isFloat()) {
auto floatStr = evaluated->getAsString(Impl.getClangASTContext(),
clang::QualType());
auto floatIdent = Impl.SwiftContext.getIdentifier(floatStr);
value = new (Impl.SwiftContext)
FloatLiteralExpr(floatIdent.str(), SourceLoc());
}
if (!value)
return nullptr;
auto returnStmt = new (Impl.SwiftContext) ReturnStmt(SourceLoc(), value);
auto body = BraceStmt::create(Impl.SwiftContext, SourceLoc(),
{returnStmt}, SourceLoc());
accessor->setBodyParsed(body);
return accessor;
}

Decl *VisitVarDecl(const clang::VarDecl *decl) {
// Variables are imported as... variables.
Optional<ImportedName> correctSwiftName;
Expand Down Expand Up @@ -4161,6 +4202,11 @@ namespace {
if (correctSwiftName)
markAsVariant(result, *correctSwiftName);

// For constexpr vars, we can create an accessor with a numeric literal.
if (decl->isConstexpr())
if (auto acc = tryCreateConstexprAccessor(decl, result))
result->setAccessors(SourceLoc(), {acc}, SourceLoc());

return result;
}

Expand Down
6 changes: 6 additions & 0 deletions test/Interop/Cxx/static/Inputs/static-member-var.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ class WithConstStaticMember {
const static int definedOutOfLine;
};

constexpr float getFloatValue() { return 42; }
constexpr float getIntValue(int arg) { return 40 + arg; }

class WithConstexprStaticMember {
public:
constexpr static int definedInline = 139;
constexpr static int definedInlineWithArg = getIntValue(2);
constexpr static float definedInlineFloat = 139;
constexpr static float definedInlineFromMethod = getFloatValue();
};

class ClassA {
Expand Down
52 changes: 39 additions & 13 deletions test/Interop/Cxx/static/static-member-var-irgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
// CHECK: @{{_ZN21WithConstStaticMember7definedE|"\?defined@WithConstStaticMember@@2HB"}} = {{available_externally|linkonce_odr}} {{(dso_local )?}}constant i32 48, {{(comdat, )?}}align 4
// CHECK: @{{_ZN21WithConstStaticMember16definedOutOfLineE|"\?definedOutOfLine@WithConstStaticMember@@2HB"}} = external {{(dso_local )?}}constant i32, align 4

//TODO: This test uses only values of static const members, so it does not need
//to depend on external definitions. However, our code generation pattern loads
//the value dynamically. Instead, we should inline known constants. That would
//allow Swift code to even read the value of WithIncompleteStaticMember::notDefined.
// NOTE: we allow both available_externally and linkonce_odr as there are
// differences in between MSVC and itanium model semantics where the constexpr
// value is emitted into COMDAT.
// CHECK: @{{_ZN25WithConstexprStaticMember13definedInlineE|"\?definedInline@WithConstexprStaticMember@@2HB"}} = {{available_externally|linkonce_odr}} {{(dso_local )?}}constant i32 139, {{(comdat, )?}}align 4
// Make sure we remove constexpr globals after all uses have been inlined.
// CHECK-NOT: _ZN25WithConstexprStaticMember13definedInlineE
// CHECK-NOT: ?definedInline@WithConstexprStaticMember@@2HB
// CHECK-NOT: @_ZN25WithConstexprStaticMember20definedInlineWithArgE
// CHECK-NOT: @_ZN25WithConstexprStaticMember18definedInlineFloatE
// CHECK-NOT: @_ZN25WithConstexprStaticMember23definedInlineFromMethodE

import StaticMemberVar

Expand Down Expand Up @@ -73,10 +71,38 @@ public func readDefinedOutOfLineConstMember() -> CInt {
// CHECK: [[VALUE:%.*]] = load i32, i32* getelementptr inbounds (%Ts5Int32V, %Ts5Int32V* bitcast (i32* @{{_ZN21WithConstStaticMember16definedOutOfLineE|"\?definedOutOfLine@WithConstStaticMember@@2HB"}} to %Ts5Int32V*), i32 0, i32 0), align 4
// CHECK: ret i32 [[VALUE]]

public func readConstexprStaticMember() -> CInt {
return WithConstexprStaticMember.definedInline
public func readConstexprStaticIntMembers() {
let x = WithConstexprStaticMember.definedInline
let y = WithConstexprStaticMember.definedInlineWithArg
}

// CHECK: define {{(protected |dllexport )?}}swiftcc i32 @"$s4main25readConstexprStaticMembers5Int32VyF"() #0
// CHECK: [[VALUE:%.*]] = load i32, i32* getelementptr inbounds (%Ts5Int32V, %Ts5Int32V* bitcast (i32* @{{_ZN25WithConstexprStaticMember13definedInlineE|"\?definedInline@WithConstexprStaticMember@@2HB"}} to %Ts5Int32V*), i32 0, i32 0), align 4
// CHECK: ret i32 [[VALUE]]
// CHECK-LABEL: define {{(protected |dllexport )?}}swiftcc void @"$s4main29readConstexprStaticIntMembersyyF"()
// CHECK: call swiftcc i32 @"$sSo25WithConstexprStaticMemberV13definedInlines5Int32VvgZ"()
// CHECK: call swiftcc i32 @"$sSo25WithConstexprStaticMemberV013definedInlineA3Args5Int32VvgZ"()
// CHECK: ret void

// CHECK-LABEL: define linkonce_odr {{.*}}swiftcc i32 @"$sSo25WithConstexprStaticMemberV13definedInlines5Int32VvgZ"()
// CHECK-NEXT: entry
// CHECK-NEXT: ret i32 139

// CHECK-LABEL: define linkonce_odr {{.*}}swiftcc i32 @"$sSo25WithConstexprStaticMemberV013definedInlineA3Args5Int32VvgZ"()
// CHECK-NEXT: entry
// CHECK-NEXT: ret i32 42

public func readConstexprStaticFloatMembers() {
let x = WithConstexprStaticMember.definedInlineFloat
let y = WithConstexprStaticMember.definedInlineFromMethod
}

// CHECK-LABEL: define {{(protected |dllexport )?}}swiftcc void @"$s4main31readConstexprStaticFloatMembersyyF"()
// CHECK: call swiftcc float @"$sSo25WithConstexprStaticMemberV18definedInlineFloatSfvgZ"()
// CHECK: call swiftcc float @"$sSo25WithConstexprStaticMemberV23definedInlineFromMethodSfvgZ"()
// CHECK: ret void

// CHECK-LABEL: define linkonce_odr {{.*}}swiftcc float @"$sSo25WithConstexprStaticMemberV18definedInlineFloatSfvgZ"()
// CHECK-NEXT: entry
// CHECK-NEXT: ret float 1.390000e+02

// CHECK-LABEL: define linkonce_odr {{.*}}swiftcc float @"$sSo25WithConstexprStaticMemberV23definedInlineFromMethodSfvgZ"()
// CHECK-NEXT: entry
// CHECK-NEXT: ret float 4.200000e+01
24 changes: 13 additions & 11 deletions test/Interop/Cxx/static/static-member-var-silgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// CHECK: sil_global public_external [let] @{{_ZN21WithConstStaticMember7definedE|\?defined@WithConstStaticMember@@2HB}} : $Int32
// CHECK: // clang name: WithConstStaticMember::definedOutOfLine
// CHECK: sil_global public_external [let] @{{_ZN21WithConstStaticMember16definedOutOfLineE|\?definedOutOfLine@WithConstStaticMember@@2HB}} : $Int32
// CHECK: // clang name: WithConstexprStaticMember::definedInline
// CHECK: sil_global public_external [let] @{{_ZN25WithConstexprStaticMember13definedInlineE|\?definedInline@WithConstexprStaticMember@@2HB}} : $Int32

import StaticMemberVar

Expand Down Expand Up @@ -70,16 +68,20 @@ func readDefinedOutOfLineConstMember() -> CInt {
return WithConstStaticMember.definedOutOfLine
}

// CHECK: sil hidden @$s4main31readDefinedOutOfLineConstMembers5Int32VyF : $@convention(thin) () -> Int32
// CHECK: [[ADDR:%.*]] = global_addr @{{_ZN21WithConstStaticMember16definedOutOfLineE|\?definedOutOfLine@WithConstStaticMember@@2HB}} : $*Int32
// CHECK: [[VALUE:%.*]] = load [[ADDR]] : $*Int32
// CHECK: return [[VALUE]] : $Int32
// CHECK: sil hidden @$s4main25readConstexprStaticMembers5Int32VyF : $@convention(thin) () -> Int32
// CHECK: [[META:%.*]] = metatype $@thin WithConstexprStaticMember.Type
// CHECK: [[ACC:%.*]] = function_ref @$sSo25WithConstexprStaticMemberV13definedInlines5Int32VvgZ : $@convention(method) (@thin WithConstexprStaticMember.Type) -> Int32
// CHECK: [[OUT:%.*]] = apply [[ACC]]([[META]]) : $@convention(method) (@thin WithConstexprStaticMember.Type) -> Int32
// CHECK: return [[OUT]] : $Int32
// CHECK-LABEL: end sil function '$s4main25readConstexprStaticMembers5Int32VyF'

// Make sure we also generate the accessor with a numeric literal.
// CHECK-LABEL: sil shared [serializable] @$sSo25WithConstexprStaticMemberV13definedInlines5Int32VvgZ : $@convention(method) (@thin WithConstexprStaticMember.Type) -> Int32
// CHECK: [[IL:%.*]] = integer_literal $Builtin.Int32, 139
// CHECK: [[OUT:%.*]] = struct $Int32 ([[IL]] : $Builtin.Int32)
// CHECK: return [[OUT]] : $Int32
// CHECK-LABEL: end sil function '$sSo25WithConstexprStaticMemberV13definedInlines5Int32VvgZ'

func readConstexprStaticMember() -> CInt {
return WithConstexprStaticMember.definedInline
}

// CHECK: sil hidden @$s4main25readConstexprStaticMembers5Int32VyF : $@convention(thin) () -> Int32
// CHECK: [[ADDR:%.*]] = global_addr @{{_ZN25WithConstexprStaticMember13definedInlineE|\?definedInline@WithConstexprStaticMember@@2HB}} : $*Int32
// CHECK: [[VALUE:%.*]] = load [[ADDR]] : $*Int32
// CHECK: return [[VALUE]] : $Int32
5 changes: 4 additions & 1 deletion test/Interop/Cxx/static/static-var-irgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ public func initStaticVars() -> CInt {
+ staticConstexprNonTrivial.val
}

// Constexpr vars should be inlined and removed.
// CHECK-NOT: ?staticConstexpr
// CHECK-NOT: _ZL15staticConstexpr

// CHECK: @{{_ZL9staticVar|staticVar}} = internal global i32 2, align 4
// CHECK: @{{_ZL13staticVarInit|staticVarInit}} = internal global i32 0, align 4
// CHECK: @{{_ZL19staticVarInlineInit|staticVarInlineInit}} = internal global i32 0, align 4
// CHECK: @{{_ZL11staticConst|staticConst}} = internal constant i32 4, align 4
// CHECK: @{{_ZL15staticConstInit|staticConstInit}} = internal global i32 0, align 4
// CHECK: @{{_ZL21staticConstInlineInit|staticConstInlineInit}} = internal global i32 0, align 4
// CHECK: @{{_ZL15staticConstexpr|staticConstexpr}} = internal constant i32 32, align 4
// CHECK: @{{_ZL16staticNonTrivial|staticNonTrivial}} = internal global %class.NonTrivial zeroinitializer, align 4
// CHECK: @{{_ZL21staticConstNonTrivial|staticConstNonTrivial}} = internal global %class.NonTrivial zeroinitializer, align 4
// CHECK: @{{_ZL25staticConstexprNonTrivial|staticConstexprNonTrivial}} = internal constant %class.NonTrivial { i32 8192 }, align 4
Expand Down
5 changes: 3 additions & 2 deletions test/Interop/Cxx/static/static-var-silgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ func initStaticVars() -> CInt {
+ staticConstexprNonTrivial.val
}

// Constexpr globals should be inlined and removed.
// CHECK-NOT: sil_global public_external [let] @staticConstexpr : $Int32

// CHECK: // clang name: staticVar
// CHECK: sil_global public_external @staticVar : $Int32
// CHECK: // clang name: staticVarInit
Expand All @@ -20,8 +23,6 @@ func initStaticVars() -> CInt {
// CHECK: sil_global public_external [let] @staticConstInit : $Int32
// CHECK: // clang name: staticConstInlineInit
// CHECK: sil_global public_external [let] @staticConstInlineInit : $Int32
// CHECK: // clang name: staticConstexpr
// CHECK: sil_global public_external [let] @staticConstexpr : $Int32
// CHECK: // clang name: staticNonTrivial
// CHECK: sil_global public_external @staticNonTrivial : $NonTrivial
// CHECK: // clang name: staticConstNonTrivial
Expand Down