Skip to content

Commit cf962e0

Browse files
committed
[SILVerifier] Disallow address-type block arguments in raw SIL.
As a structural SIL property, we disallow address-type block arguments. Supporting them would prevent reliably reasoning about the underlying storage of memory access. This reasoning is important for diagnosing violations of memory access rules and supporting future optimizations such as bitfield packing. Address-type block arguments also create unnecessary complexity for SIL optimization passes that need to reason about memory aliasing. This must be enforced in RAW SIL for diagnosing exclusive memory access. The optimizer currently breaks the invariant in three places: 1. Normal Simplify CFG during conditional branch simplification (sneaky jump threading). 2. Simplify CFG via Jump Threading. 3. Loop Rotation.
1 parent 5f2f440 commit cf962e0

File tree

10 files changed

+91
-11
lines changed

10 files changed

+91
-11
lines changed

lib/SIL/SILVerifier.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,58 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
329329
delete Dominance;
330330
}
331331

332+
// Address-type block args must be prohibited whenever access markers are
333+
// present. Access markers are always present in raw SIL to diagnose exclusive
334+
// memory access. In canonical SIL, access markers are only present with
335+
// EnforceExclusivityDynamic.
336+
//
337+
// FIXME: For sanity, address-type block args should be prohibited at all SIL
338+
// stages. However, the optimizer currently breaks the invariant in three
339+
// places:
340+
// 1. Normal Simplify CFG during conditional branch simplification
341+
// (sneaky jump threading).
342+
// 2. Simplify CFG via Jump Threading.
343+
// 3. Loop Rotation.
344+
// Once EnforceExclusivityDynamic is performant enough to be enabled by
345+
// default at -O, address-type blocks args can be prohibited unconditionally.
346+
bool prohibitAddressBlockArgs() {
347+
// If this function was deserialized from canonical SIL, this invariant may
348+
// already have been violated regardless of this module's SIL stage or
349+
// exclusivity enforcement level. Presumably, access markers were already
350+
// removed prior to serialization.
351+
if (F.wasDeserializedCanonical())
352+
return false;
353+
354+
SILModule &M = F.getModule();
355+
if (M.getStage() == SILStage::Raw)
356+
return true;
357+
358+
// If dynamic enforcement is enabled, markers are present at -O so we
359+
// prohibit address block args.
360+
return M.getOptions().EnforceExclusivityDynamic;
361+
}
362+
332363
void visitSILArgument(SILArgument *arg) {
333364
checkLegalType(arg->getFunction(), arg, nullptr);
334365
checkValueBaseOwnership(arg);
366+
367+
if (isa<SILPHIArgument>(arg) && prohibitAddressBlockArgs()) {
368+
// As a structural SIL property, we disallow address-type block
369+
// arguments. Supporting them would prevent reliably reasoning about the
370+
// underlying storage of memory access. This reasoning is important for
371+
// diagnosing violations of memory access rules and supporting future
372+
// optimizations such as bitfield packing. Address-type block arguments
373+
// also create unnecessary complexity for SIL optimization passes that
374+
// need to reason about memory aliasing.
375+
//
376+
// Note: We could allow non-phi block arguments to be addresses, because
377+
// the address source would still be uniquely recoverable. But then
378+
// we would need to separately ensure that something like begin_access is
379+
// never passed as a block argument before being used by end_access. For
380+
// now, it simpler to have a strict prohibition.
381+
require(!arg->getType().isAddress(),
382+
"Block arguments cannot be addresses");
383+
}
335384
}
336385

337386
void visitSILInstruction(SILInstruction *I) {

test/SILOptimizer/basic-aa.sil

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -module-name Swift %s -aa-kind=basic-aa -aa-dump -o /dev/null | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -module-name Swift %s -aa-kind=basic-aa -aa-dump -o /dev/null | %FileCheck %s
22

33
// REQUIRES: asserts
44

5+
// Declare this SIL to be canonical because some tests break raw SIL
6+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
7+
// required to allow address-type block args in canonical SIL.
8+
sil_stage canonical
9+
510
import Builtin
611

712
struct Int {

test/SILOptimizer/copyforward.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -copy-forwarding -enable-copyforwarding -enable-destroyhoisting | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all %s -copy-forwarding -enable-copyforwarding -enable-destroyhoisting | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

test/SILOptimizer/cowarray_opt.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all -cowarray-opt %s | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all -cowarray-opt %s | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

test/SILOptimizer/licm.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -licm | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all %s -licm | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

test/SILOptimizer/looprotate.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all -loop-rotate %s | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all -loop-rotate %s | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

test/SILOptimizer/redundant_load_elim.sil

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -redundant-load-elim | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all %s -redundant-load-elim | %FileCheck %s
2+
3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
6+
sil_stage canonical
27

38
import Builtin
49
import Swift

test/SILOptimizer/sil_combine.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -sil-combine -verify-skip-unreachable-must-be-last | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all %s -sil-combine -verify-skip-unreachable-must-be-last | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

test/SILOptimizer/simplify_cfg.sil

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -jumpthread-simplify-cfg | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all %s -jumpthread-simplify-cfg | %FileCheck %s
22
// FIXME: Update for select_enum change.
33

4+
// Declare this SIL to be canonical because some tests break raw SIL
5+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
6+
// required to allow address-type block args in canonical SIL.
7+
sil_stage canonical
8+
49
import Builtin
510
import Swift
611

7-
sil_stage canonical
8-
912
///////////////////////
1013
// Type Declarations //
1114
///////////////////////

test/SILOptimizer/sroa_bbargs.sil

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all -sroa-bb-args %s | %FileCheck %s
1+
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enforce-exclusivity=none -enable-sil-verify-all -sroa-bb-args %s | %FileCheck %s
22

3+
// Declare this SIL to be canonical because some tests break raw SIL
4+
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
5+
// required to allow address-type block args in canonical SIL.
36
sil_stage canonical
47

58
import Builtin

0 commit comments

Comments
 (0)