Skip to content

SILOptimizer: always assume that inout arguments are not aliasing #27664

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
Oct 14, 2019
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
21 changes: 2 additions & 19 deletions include/swift/SIL/SILArgumentConvention.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@

namespace swift {

enum class InoutAliasingAssumption {
/// Assume that an inout indirect parameter may alias other objects.
/// This is the safe assumption an optimization should make if it may break
/// memory safety in case the inout aliasing rule is violation.
Aliasing,

/// Assume that an inout indirect parameter cannot alias other objects.
/// Optimizations should only use this if they can guarantee that they will
/// not break memory safety even if the inout aliasing rule is violated.
NotAliasing
};

/// Conventions for apply operands and function-entry arguments in SIL.
///
/// This is simply a union of ParameterConvention and ResultConvention
Expand Down Expand Up @@ -142,19 +130,14 @@ struct SILArgumentConvention {
}

/// Returns true if \p Value is a not-aliasing indirect parameter.
/// The \p isInoutAliasing specifies what to assume about the inout
/// convention.
/// See InoutAliasingAssumption.
bool isNotAliasedIndirectParameter(InoutAliasingAssumption isInoutAliasing) {
bool isNotAliasedIndirectParameter() {
switch (Value) {
case SILArgumentConvention::Indirect_In:
case SILArgumentConvention::Indirect_In_Constant:
case SILArgumentConvention::Indirect_Out:
case SILArgumentConvention::Indirect_In_Guaranteed:
return true;

case SILArgumentConvention::Indirect_Inout:
return isInoutAliasing == InoutAliasingAssumption::NotAliasing;
return true;

case SILArgumentConvention::Indirect_InoutAliasable:
case SILArgumentConvention::Direct_Unowned:
Expand Down
6 changes: 2 additions & 4 deletions include/swift/SILOptimizer/Analysis/ValueTracking.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,14 @@ namespace swift {
/// any other pointer in the function.
/// The \p assumeInoutIsNotAliasing specifies in no-aliasing is assumed for
/// the @inout convention. See swift::isNotAliasedIndirectParameter().
bool isNotAliasingArgument(SILValue V, InoutAliasingAssumption isInoutAliasing =
InoutAliasingAssumption::Aliasing);
bool isNotAliasingArgument(SILValue V);

/// Returns true if \p V is local inside its function. This means its underlying
/// object either is a non-aliasing function argument or a locally allocated
/// object.
/// The \p assumeInoutIsNotAliasing specifies in no-aliasing is assumed for
/// the @inout convention. See swift::isNotAliasedIndirectParameter().
bool pointsToLocalObject(SILValue V, InoutAliasingAssumption isInoutAliasing =
InoutAliasingAssumption::Aliasing);
bool pointsToLocalObject(SILValue V);

enum class IsZeroKind {
Zero,
Expand Down
10 changes: 4 additions & 6 deletions lib/SILOptimizer/Analysis/ValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,13 @@
using namespace swift;
using namespace swift::PatternMatch;

bool swift::isNotAliasingArgument(SILValue V,
InoutAliasingAssumption isInoutAliasing) {
bool swift::isNotAliasingArgument(SILValue V) {
auto *Arg = dyn_cast<SILFunctionArgument>(V);
if (!Arg)
return false;

SILArgumentConvention Conv = Arg->getArgumentConvention();
return Conv.isNotAliasedIndirectParameter(isInoutAliasing);
return Conv.isNotAliasedIndirectParameter();
}

/// Check if the parameter \V is based on a local object, e.g. it is an
Expand Down Expand Up @@ -81,10 +80,9 @@ static bool isLocalObject(SILValue Obj) {
return true;
}

bool swift::pointsToLocalObject(SILValue V,
InoutAliasingAssumption isInoutAliasing) {
bool swift::pointsToLocalObject(SILValue V) {
V = getUnderlyingObject(V);
return isLocalObject(V) || isNotAliasingArgument(V, isInoutAliasing);
return isLocalObject(V) || isNotAliasingArgument(V);
}

/// Check if the value \p Value is known to be zero, non-zero or unknown.
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/basic-aa.sil
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ bb1(%1 : $*Builtin.NativeObject, %2 : $*Builtin.NativeObject):
// CHECK: PAIR #1.
// CHECK-NEXT: %0 = argument of bb0
// CHECK-NEXT: %1 = argument of bb0
// CHECK-NEXT: MayAlias
// CHECK-NEXT: NoAlias
sil @inout_args_may_alias: $@convention(thin) (@inout Builtin.NativeObject, @inout Builtin.NativeObject) -> () {
bb0(%0 : $*Builtin.NativeObject, %1 : $*Builtin.NativeObject):
%2 = tuple()
Expand Down
18 changes: 18 additions & 0 deletions test/SILOptimizer/dead_store_elim.sil
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,24 @@ bb3:
return %9999 : $()
}

sil @unknown : $@convention(thin) () -> ()

// CHECK-LABEL: sil @inout_is_not_aliasing : $@convention(thin) (@inout Builtin.Int32) -> () {
// CHECK: bb0
// CHECK-NEXT: integer_literal
// CHECK-NEXT: function_ref
// CHECK: return
sil @inout_is_not_aliasing : $@convention(thin) (@inout Builtin.Int32) -> () {
bb0(%0 : $*Builtin.Int32):
%1 = integer_literal $Builtin.Int32, 0
store %1 to %0 : $*Builtin.Int32
%f = function_ref @unknown : $@convention(thin) () -> ()
%3 = apply %f() : $@convention(thin) () -> ()
store %1 to %0 : $*Builtin.Int32
%9999 = tuple()
return %9999 : $()
}

// We should be able to remove the store in bb0, but we currently
// can't due to deficiency in alias analysis.
//
Expand Down
5 changes: 3 additions & 2 deletions test/SILOptimizer/redundant_load_elim.sil
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,9 @@ bb5:
// CHECK: store
// CHECK-NOT: = load
// CHECK: bb2:
// CHECK: bb3:
// CHECK: = load
// CHECK: bb3([[A:%[0-9]+]] : $Builtin.Int32):
// CHECK-NOT: = load
// CHECK: apply %{{[0-9]+}}([[A]])
sil @load_to_load_conflicting_branches_diamond : $@convention(thin) (@inout Builtin.Int32) -> () {
// %0 // users: %1, %4, %9, %11, %16, %21
bb0(%0 : $*Builtin.Int32):
Expand Down