Skip to content

Commit fa26420

Browse files
committed
[SE-0206][Sema] Add separate hash(into:) impl for enums with no associated values
For enums with no associated values, it is better to move the hasher.combine call out of the switch statement, like this: func hash(into hasher: inout Hasher) { let discriminator: Int switch self { case a: discriminator = 0 case b: discriminator = 1 case c: discriminator = 2 } hasher.combine(discriminator) } This enables the optimizer to replace the switch statement with a simple integer promotion, restoring earlier behavior.
1 parent 130bcfd commit fa26420

File tree

2 files changed

+79
-38
lines changed

2 files changed

+79
-38
lines changed

lib/Sema/DerivedConformanceEquatableHashable.cpp

+63-33
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,20 @@ enumElementPayloadSubpattern(EnumElementDecl *enumElementDecl,
193193
return pat;
194194
}
195195

196+
/// Returns a new integer literal expression with the given value.
197+
/// \p C The AST context.
198+
/// \p value The integer value.
199+
/// \return The integer literal expression.
200+
static Expr *integerLiteralExpr(ASTContext &C, int64_t value) {
201+
llvm::SmallString<8> integerVal;
202+
APInt(32, value).toString(integerVal, 10, /*signed*/ false);
203+
auto integerStr = C.AllocateCopy(integerVal);
204+
auto integerExpr = new (C) IntegerLiteralExpr(
205+
StringRef(integerStr.data(), integerStr.size()), SourceLoc(),
206+
/*implicit*/ true);
207+
return integerExpr;
208+
}
209+
196210
/// Create AST statements which convert from an enum to an Int with a switch.
197211
/// \p stmts The generated statements are appended to this vector.
198212
/// \p parentDC Either an extension or the enum itself.
@@ -240,13 +254,7 @@ static DeclRefExpr *convertEnumToIndex(SmallVectorImpl<ASTNode> &stmts,
240254
auto labelItem = CaseLabelItem(pat);
241255

242256
// generate: indexVar = <index>
243-
llvm::SmallString<8> indexVal;
244-
APInt(32, index++).toString(indexVal, 10, /*signed*/ false);
245-
auto indexStr = C.AllocateCopy(indexVal);
246-
247-
auto indexExpr = new (C) IntegerLiteralExpr(StringRef(indexStr.data(),
248-
indexStr.size()), SourceLoc(),
249-
/*implicit*/ true);
257+
auto indexExpr = integerLiteralExpr(C, index++);
250258
auto indexRef = new (C) DeclRefExpr(indexVar, DeclNameLoc(),
251259
/*implicit*/true);
252260
auto assignExpr = new (C) AssignExpr(indexRef, SourceLoc(),
@@ -714,20 +722,6 @@ ValueDecl *DerivedConformance::deriveEquatable(TypeChecker &tc,
714722
return nullptr;
715723
}
716724

717-
/// Returns a new integer literal expression with the given value.
718-
/// \p C The AST context.
719-
/// \p value The integer value.
720-
/// \return The integer literal expression.
721-
static Expr *integerLiteralExpr(ASTContext &C, int64_t value) {
722-
llvm::SmallString<8> integerVal;
723-
APInt(32, value).toString(integerVal, 10, /*signed*/ false);
724-
auto integerStr = C.AllocateCopy(integerVal);
725-
auto integerExpr = new (C) IntegerLiteralExpr(
726-
StringRef(integerStr.data(), integerStr.size()), SourceLoc(),
727-
/*implicit*/ true);
728-
return integerExpr;
729-
}
730-
731725
/// Returns a new \c CallExpr representing
732726
///
733727
/// hasher.combine(hashable)
@@ -853,23 +847,54 @@ deriveBodyHashable_compat_hashInto(AbstractFunctionDecl *hashIntoDecl) {
853847
hashIntoDecl->setBody(body);
854848
}
855849

856-
/// Derive the body for the 'hash(into:)' method for an enum.
850+
/// Derive the body for the 'hash(into:)' method for an enum without associated
851+
/// values.
857852
static void
858-
deriveBodyHashable_enum_hashInto(AbstractFunctionDecl *hashIntoDecl) {
853+
deriveBodyHashable_enum_noAssociatedValues_hashInto(
854+
AbstractFunctionDecl *hashIntoDecl
855+
) {
859856
// enum SomeEnum {
860857
// case A, B, C
861858
// @derived func hash(into hasher: inout Hasher) {
859+
// let discriminator: Int
862860
// switch self {
863861
// case A:
864-
// hasher.combine(0)
862+
// discriminator = 0
865863
// case B:
866-
// hasher.combine(1)
864+
// discriminator = 1
867865
// case C:
868-
// hasher.combine(2)
866+
// discriminator = 2
869867
// }
868+
// hasher.combine(discriminator)
870869
// }
871870
// }
872-
//
871+
auto parentDC = hashIntoDecl->getDeclContext();
872+
ASTContext &C = parentDC->getASTContext();
873+
874+
auto enumDecl = parentDC->getAsEnumOrEnumExtensionContext();
875+
auto selfDecl = hashIntoDecl->getImplicitSelfDecl();
876+
877+
// generate: switch self {...}
878+
SmallVector<ASTNode, 3> stmts;
879+
auto discriminatorExpr = convertEnumToIndex(stmts, parentDC, enumDecl,
880+
selfDecl, hashIntoDecl,
881+
"discriminator");
882+
// generate: hasher.combine(discriminator)
883+
auto hasherParam = hashIntoDecl->getParameterList(1)->get(0);
884+
auto combineStmt = createHasherCombineCall(C, hasherParam, discriminatorExpr);
885+
stmts.push_back(combineStmt);
886+
887+
auto body = BraceStmt::create(C, SourceLoc(), stmts, SourceLoc(),
888+
/*implicit*/ true);
889+
hashIntoDecl->setBody(body);
890+
}
891+
892+
/// Derive the body for the 'hash(into:)' method for an enum with associated
893+
/// values.
894+
static void
895+
deriveBodyHashable_enum_hasAssociatedValues_hashInto(
896+
AbstractFunctionDecl *hashIntoDecl
897+
) {
873898
// enum SomeEnumWithAssociatedValues {
874899
// case A, B(Int), C(String, Int)
875900
// @derived func hash(into hasher: inout Hasher) {
@@ -940,7 +965,8 @@ deriveBodyHashable_enum_hashInto(AbstractFunctionDecl *hashIntoDecl) {
940965
auto hasBoundDecls = !payloadVars.empty();
941966
auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());
942967
cases.push_back(CaseStmt::create(C, SourceLoc(), labelItem, hasBoundDecls,
943-
SourceLoc(), SourceLoc(), body));
968+
SourceLoc(), SourceLoc(), body,
969+
/*implicit*/ true));
944970
}
945971

946972
// generate: switch enumVar { }
@@ -1195,16 +1221,20 @@ ValueDecl *DerivedConformance::deriveHashable(TypeChecker &tc,
11951221
// special case for enums with no associated values to preserve source
11961222
// compatibility.
11971223
auto theEnum = dyn_cast<EnumDecl>(type);
1198-
if (!(theEnum && theEnum->hasOnlyCasesWithoutAssociatedValues()) &&
1199-
type != parentDecl) {
1224+
auto hasAssociatedValues =
1225+
theEnum && !theEnum->hasOnlyCasesWithoutAssociatedValues();
1226+
if ((!theEnum || hasAssociatedValues) && type != parentDecl) {
12001227
tc.diagnose(parentDecl->getLoc(), diag::cannot_synthesize_in_extension,
12011228
hashableProto->getDeclaredType());
12021229
return nullptr;
12031230
}
1204-
if (theEnum)
1231+
if (theEnum) {
1232+
auto bodySynthesizer = hasAssociatedValues
1233+
? &deriveBodyHashable_enum_hasAssociatedValues_hashInto
1234+
: &deriveBodyHashable_enum_noAssociatedValues_hashInto;
12051235
return deriveHashable_hashInto(tc, parentDecl, theEnum,
1206-
&deriveBodyHashable_enum_hashInto);
1207-
else if (auto theStruct = dyn_cast<StructDecl>(type))
1236+
bodySynthesizer);
1237+
} else if (auto theStruct = dyn_cast<StructDecl>(type))
12081238
return deriveHashable_hashInto(tc, parentDecl, theStruct,
12091239
&deriveBodyHashable_struct_hashInto);
12101240
else // This should've been caught by canDeriveHashable above.

test/IRGen/enum_derived.swift

+16-5
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
import def_enum
77

8-
// Check if the hashValue and == for an enum (without payload) are generated and
9-
// check if that functions are compiled in an optimal way.
8+
// Check if hashValue, hash(into:) and == for an enum (without payload) are
9+
// generated and check that functions are compiled in an optimal way.
1010

1111
enum E {
1212
case E0
@@ -22,12 +22,23 @@ enum E {
2222
// CHECK: %2 = icmp eq i8 %0, %1
2323
// CHECK: ret i1 %2
2424

25-
// Check if the hashValue getter can be compiled to a simple zext instruction.
25+
// Check for the presence of the hashValue getter, calling Hasher.init() and
26+
// Hasher.finalize().
2627

2728
// CHECK-NORMAL-LABEL:define hidden swiftcc i{{.*}} @"$S12enum_derived1EO9hashValueSivg"(i8)
2829
// CHECK-TESTABLE-LABEL:define{{( dllexport)?}}{{( protected)?}} swiftcc i{{.*}} @"$S12enum_derived1EO9hashValueSivg"(i8)
29-
// CHECK: [[R:%.*]] = zext i8 %0 to i{{.*}}
30-
// CHECK: ret i{{.*}} [[R]]
30+
// CHECK: call swiftcc void @"$Ss6HasherVABycfC"(%Ts6HasherV* {{.*}})
31+
// CHECK: call swiftcc i{{[0-9]+}} @"$Ss6HasherV9_finalizeSiyF"(%Ts6HasherV* {{.*}})
32+
// CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
33+
34+
// Check if the hash(into:) method can be compiled to a simple zext instruction
35+
// followed by a call to Hasher._combine(_:).
36+
37+
// CHECK-NORMAL-LABEL:define hidden swiftcc void @"$S12enum_derived1EO4hash4intoys6HasherVz_tF"
38+
// CHECK-TESTABLE-LABEL:define{{( dllexport)?}}{{( protected)?}} swiftcc void @"$S12enum_derived1EO4hash4intoys6HasherVz_tF"
39+
// CHECK: [[V:%.*]] = zext i8 %1 to i{{.*}}
40+
// CHECK: tail call swiftcc void @"$Ss6HasherV8_combineyySuF"(i{{.*}} [[V]], %Ts6HasherV*
41+
// CHECK: ret void
3142

3243
// Derived conformances from extensions
3344
// The actual enums are in Inputs/def_enum.swift

0 commit comments

Comments
 (0)