-Onone @inline(__always)?


(Andrew Trick) #1

Responding on swift-dev to what turned into an interesting discussion…

We definitely want an inline-at-Onone, but I assumed we were treating "@inline(__always)" as a normal heuristic because:
- @transparent already does inline-at-Onone.
- @inline(__always) without @transparent negatively affects the debug experience.
- @inline(__always) isn't useful if no optimizations are run after inlining.

That said, if the common expectation is for @inline(__always) to kick in independent of optimization, then we should honor that.

It's easy for me to add an inlining pass to runSILPassesForOnone. I'm just not sure that will have the effect people are looking for. It's unlikely to help performance since nothing is going to run after the inlining (no capture promotion or alloc box-to-stack). The only optimization we currently run at Onone after diagnostics is prespecialization.

Actually, the inliner specializes as it goes, so inlining after mandatory SIL passes would still be quite useful.

My concern is really that the user may expect inline(__always) to be as effective a performance tool as @transparent, while preserving debug line locations.
If it runs late, it doesn’t expose capture promotion, box-to-stack, guaranteed ARC optimization, etc.

If someone is using inline(__always) for -Onone performance reasons, I think they really need to use @transparent.

An alternative is to have @inline(__always) be another way to spell @transparent. People would need to implicitly know that this would affect diagnostics of the inlined method.

Any thoughts?

`@transparent` doesn't just mean "inline-at-Onone", it also allows diagnostic passes to constant-fold through the implementation and actively eliminates debug information. It was never intended to be used for performance, but only for the limited set of things that need it. I imagine that the specialization you get by inlining some things is pretty important by itself. I suspect closure inlining would also be helpful for a lot of low-level "with*" combinators; doesn't transparent inlining do that, and could inline-always-inlining do the same?

For reference: https://github.com/apple/swift/blob/master/docs/TransparentAttr.rst

The only remaining question: is it ok for a non-transparent function to be inlined prior to running some dataflow diagnostics?

Doing so changes the set of programs that are accepted, since it may expose more code to diagnostic passes. We don't want diagnostics to depend on optimizations.

Perhaps we could avoid this by changing the diagnostic passes to intentionally ignore instructions with inlined source locations, though that seems brittle.

-Joe

Well, inline(__always) would be mandatory, not an optimization. But I think your point is that the user does not expect that attribute to affect program legality. Having data-flow diagnostics “ignore” certain instructions sounds horrible. Is there an obvious problem, or is this hypothetical? i.e. is there some diagnostic that is currently enforced at the function boundary but would be considered legal code after inlining?

-Andy

···

On Dec 1, 2016, at 11:19 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:18 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:16 AM, Andrew Trick <atrick@apple.com> wrote:

On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Nov 30, 2016, at 18:16, Joe Groff <jgroff@apple.com> wrote:

On Nov 30, 2016, at 6:10 PM, Andrew Trick <atrick@apple.com> wrote:


(Andrew Trick) #2

I guess initialization of @inout arguments would be the obvious problem.
-Andy

···

On Dec 1, 2016, at 11:48 AM, Andrew Trick <atrick@apple.com> wrote:

Responding on swift-dev to what turned into an interesting discussion…

On Dec 1, 2016, at 11:19 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

On Dec 1, 2016, at 11:18 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

On Dec 1, 2016, at 11:16 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Nov 30, 2016, at 18:16, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

On Nov 30, 2016, at 6:10 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

We definitely want an inline-at-Onone, but I assumed we were treating "@inline(__always)" as a normal heuristic because:
- @transparent already does inline-at-Onone.
- @inline(__always) without @transparent negatively affects the debug experience.
- @inline(__always) isn't useful if no optimizations are run after inlining.

That said, if the common expectation is for @inline(__always) to kick in independent of optimization, then we should honor that.

It's easy for me to add an inlining pass to runSILPassesForOnone. I'm just not sure that will have the effect people are looking for. It's unlikely to help performance since nothing is going to run after the inlining (no capture promotion or alloc box-to-stack). The only optimization we currently run at Onone after diagnostics is prespecialization.

Actually, the inliner specializes as it goes, so inlining after mandatory SIL passes would still be quite useful.

My concern is really that the user may expect inline(__always) to be as effective a performance tool as @transparent, while preserving debug line locations.
If it runs late, it doesn’t expose capture promotion, box-to-stack, guaranteed ARC optimization, etc.

If someone is using inline(__always) for -Onone performance reasons, I think they really need to use @transparent.

An alternative is to have @inline(__always) be another way to spell @transparent. People would need to implicitly know that this would affect diagnostics of the inlined method.

Any thoughts?

`@transparent` doesn't just mean "inline-at-Onone", it also allows diagnostic passes to constant-fold through the implementation and actively eliminates debug information. It was never intended to be used for performance, but only for the limited set of things that need it. I imagine that the specialization you get by inlining some things is pretty important by itself. I suspect closure inlining would also be helpful for a lot of low-level "with*" combinators; doesn't transparent inlining do that, and could inline-always-inlining do the same?

For reference: https://github.com/apple/swift/blob/master/docs/TransparentAttr.rst

The only remaining question: is it ok for a non-transparent function to be inlined prior to running some dataflow diagnostics?

Doing so changes the set of programs that are accepted, since it may expose more code to diagnostic passes. We don't want diagnostics to depend on optimizations.

Perhaps we could avoid this by changing the diagnostic passes to intentionally ignore instructions with inlined source locations, though that seems brittle.

-Joe

Well, inline(__always) would be mandatory, not an optimization. But I think your point is that the user does not expect that attribute to affect program legality. Having data-flow diagnostics “ignore” certain instructions sounds horrible. Is there an obvious problem, or is this hypothetical? i.e. is there some diagnostic that is currently enforced at the function boundary but would be considered legal code after inlining?

-Andy


(Joe Groff) #3

An example of something that depends on dataflow diagnostics would be something that's constant-foldable causing static overflow diagnostics:

func double(_ x: UInt8) -> UInt8 {
  return x + x
}

_ = double(128)

Making `double` transparent causes the compiler to statically reject the call:

/Users/jgroff/bar.swift:6:5: error: arithmetic operation '128 + 128' (on unsigned 8-bit integer type) results in an overflow
_ = double(128)
    ^

-Joe

···

On Dec 1, 2016, at 11:48 AM, Andrew Trick <atrick@apple.com> wrote:

Responding on swift-dev to what turned into an interesting discussion…

On Dec 1, 2016, at 11:19 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:18 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:16 AM, Andrew Trick <atrick@apple.com> wrote:

On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Nov 30, 2016, at 18:16, Joe Groff <jgroff@apple.com> wrote:

On Nov 30, 2016, at 6:10 PM, Andrew Trick <atrick@apple.com> wrote:

We definitely want an inline-at-Onone, but I assumed we were treating "@inline(__always)" as a normal heuristic because:
- @transparent already does inline-at-Onone.
- @inline(__always) without @transparent negatively affects the debug experience.
- @inline(__always) isn't useful if no optimizations are run after inlining.

That said, if the common expectation is for @inline(__always) to kick in independent of optimization, then we should honor that.

It's easy for me to add an inlining pass to runSILPassesForOnone. I'm just not sure that will have the effect people are looking for. It's unlikely to help performance since nothing is going to run after the inlining (no capture promotion or alloc box-to-stack). The only optimization we currently run at Onone after diagnostics is prespecialization.

Actually, the inliner specializes as it goes, so inlining after mandatory SIL passes would still be quite useful.

My concern is really that the user may expect inline(__always) to be as effective a performance tool as @transparent, while preserving debug line locations.
If it runs late, it doesn’t expose capture promotion, box-to-stack, guaranteed ARC optimization, etc.

If someone is using inline(__always) for -Onone performance reasons, I think they really need to use @transparent.

An alternative is to have @inline(__always) be another way to spell @transparent. People would need to implicitly know that this would affect diagnostics of the inlined method.

Any thoughts?

`@transparent` doesn't just mean "inline-at-Onone", it also allows diagnostic passes to constant-fold through the implementation and actively eliminates debug information. It was never intended to be used for performance, but only for the limited set of things that need it. I imagine that the specialization you get by inlining some things is pretty important by itself. I suspect closure inlining would also be helpful for a lot of low-level "with*" combinators; doesn't transparent inlining do that, and could inline-always-inlining do the same?

For reference: https://github.com/apple/swift/blob/master/docs/TransparentAttr.rst

The only remaining question: is it ok for a non-transparent function to be inlined prior to running some dataflow diagnostics?

Doing so changes the set of programs that are accepted, since it may expose more code to diagnostic passes. We don't want diagnostics to depend on optimizations.

Perhaps we could avoid this by changing the diagnostic passes to intentionally ignore instructions with inlined source locations, though that seems brittle.

-Joe

Well, inline(__always) would be mandatory, not an optimization. But I think your point is that the user does not expect that attribute to affect program legality. Having data-flow diagnostics “ignore” certain instructions sounds horrible. Is there an obvious problem, or is this hypothetical? i.e. is there some diagnostic that is currently enforced at the function boundary but would be considered legal code after inlining?


(Andrew Trick) #4

Right. In fact, constant propagation appears to be the only diagnostic that needs to run after mandatory inlining (I was misremembering SILDiagnostic pipeline):

  PM.addCapturePromotion();
  PM.addAllocBoxToStack();
  PM.addNoReturnFolding();
  PM.addDefiniteInitialization();

  PM.addMandatoryInlining();
  PM.addPredictableMemoryOptimizations();
  PM.addDiagnosticConstantPropagation();
  PM.addGuaranteedARCOpts();

I think PredicatableMemoryOptimization is a "required" optimization, not a diagnostic, and GuaranteedARCOpts is an "expected" optimization that doesn't need to be in the diagnostic pipeline at all.

I'm not sure it's actually bad for @inline(__always) to strengthen diagnostics, but it isn't the pure solution. So, in the interest of purity, I'll suggest this:

Diagnostic Pipeline:

  PM.addCapturePromotion();
  PM.addAllocBoxToStack();
  PM.addNoReturnFolding();
  PM.addDefiniteInitialization();
  PM.addMandatoryInlining();
  PM.addPredictableMemoryOptimizations();
  PM.addDiagnosticConstantPropagation();

Onone Pipeline:

  PM.addGuaranteedInlining();
  PM.addGuaranteedARCOpts();

Eventually we could add another round of PredictableMemoryOptimizations to the Onone pipeline if needed.

-Andy

···

On Dec 1, 2016, at 11:55 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:48 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

Responding on swift-dev to what turned into an interesting discussion…

On Dec 1, 2016, at 11:19 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

On Dec 1, 2016, at 11:18 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:16 AM, Andrew Trick <atrick@apple.com> wrote:

On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Nov 30, 2016, at 18:16, Joe Groff <jgroff@apple.com> wrote:

On Nov 30, 2016, at 6:10 PM, Andrew Trick <atrick@apple.com> wrote:

We definitely want an inline-at-Onone, but I assumed we were treating "@inline(__always)" as a normal heuristic because:
- @transparent already does inline-at-Onone.
- @inline(__always) without @transparent negatively affects the debug experience.
- @inline(__always) isn't useful if no optimizations are run after inlining.

That said, if the common expectation is for @inline(__always) to kick in independent of optimization, then we should honor that.

It's easy for me to add an inlining pass to runSILPassesForOnone. I'm just not sure that will have the effect people are looking for. It's unlikely to help performance since nothing is going to run after the inlining (no capture promotion or alloc box-to-stack). The only optimization we currently run at Onone after diagnostics is prespecialization.

Actually, the inliner specializes as it goes, so inlining after mandatory SIL passes would still be quite useful.

My concern is really that the user may expect inline(__always) to be as effective a performance tool as @transparent, while preserving debug line locations.
If it runs late, it doesn’t expose capture promotion, box-to-stack, guaranteed ARC optimization, etc.

If someone is using inline(__always) for -Onone performance reasons, I think they really need to use @transparent.

An alternative is to have @inline(__always) be another way to spell @transparent. People would need to implicitly know that this would affect diagnostics of the inlined method.

Any thoughts?

`@transparent` doesn't just mean "inline-at-Onone", it also allows diagnostic passes to constant-fold through the implementation and actively eliminates debug information. It was never intended to be used for performance, but only for the limited set of things that need it. I imagine that the specialization you get by inlining some things is pretty important by itself. I suspect closure inlining would also be helpful for a lot of low-level "with*" combinators; doesn't transparent inlining do that, and could inline-always-inlining do the same?

For reference: https://github.com/apple/swift/blob/master/docs/TransparentAttr.rst

The only remaining question: is it ok for a non-transparent function to be inlined prior to running some dataflow diagnostics?

Doing so changes the set of programs that are accepted, since it may expose more code to diagnostic passes. We don't want diagnostics to depend on optimizations.

Perhaps we could avoid this by changing the diagnostic passes to intentionally ignore instructions with inlined source locations, though that seems brittle.

-Joe

Well, inline(__always) would be mandatory, not an optimization. But I think your point is that the user does not expect that attribute to affect program legality. Having data-flow diagnostics “ignore” certain instructions sounds horrible. Is there an obvious problem, or is this hypothetical? i.e. is there some diagnostic that is currently enforced at the function boundary but would be considered legal code after inlining?

An example of something that depends on dataflow diagnostics would be something that's constant-foldable causing static overflow diagnostics:

func double(_ x: UInt8) -> UInt8 {
  return x + x
}

_ = double(128)

Making `double` transparent causes the compiler to statically reject the call:

/Users/jgroff/bar.swift:6:5: error: arithmetic operation '128 + 128' (on unsigned 8-bit integer type) results in an overflow
_ = double(128)
    ^

-Joe


(Joe Groff) #5

Unreachable code analysis is also affected. For instance:

import Swift

func die() {
  Builtin.unreachable()
}

func foo() -> Int {
  die()
  // should get an error about missing return
}

If 'die()' is made transparent, then the "missing return" in foo() will be unexpectedly suppressed.

-Joe

···

On Dec 1, 2016, at 12:11 PM, Andrew Trick <atrick@apple.com> wrote:

On Dec 1, 2016, at 11:55 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:48 AM, Andrew Trick <atrick@apple.com> wrote:

Responding on swift-dev to what turned into an interesting discussion…

On Dec 1, 2016, at 11:19 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:18 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 1, 2016, at 11:16 AM, Andrew Trick <atrick@apple.com> wrote:

On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Nov 30, 2016, at 18:16, Joe Groff <jgroff@apple.com> wrote:

On Nov 30, 2016, at 6:10 PM, Andrew Trick <atrick@apple.com> wrote:

We definitely want an inline-at-Onone, but I assumed we were treating "@inline(__always)" as a normal heuristic because:
- @transparent already does inline-at-Onone.
- @inline(__always) without @transparent negatively affects the debug experience.
- @inline(__always) isn't useful if no optimizations are run after inlining.

That said, if the common expectation is for @inline(__always) to kick in independent of optimization, then we should honor that.

It's easy for me to add an inlining pass to runSILPassesForOnone. I'm just not sure that will have the effect people are looking for. It's unlikely to help performance since nothing is going to run after the inlining (no capture promotion or alloc box-to-stack). The only optimization we currently run at Onone after diagnostics is prespecialization.

Actually, the inliner specializes as it goes, so inlining after mandatory SIL passes would still be quite useful.

My concern is really that the user may expect inline(__always) to be as effective a performance tool as @transparent, while preserving debug line locations.
If it runs late, it doesn’t expose capture promotion, box-to-stack, guaranteed ARC optimization, etc.

If someone is using inline(__always) for -Onone performance reasons, I think they really need to use @transparent.

An alternative is to have @inline(__always) be another way to spell @transparent. People would need to implicitly know that this would affect diagnostics of the inlined method.

Any thoughts?

`@transparent` doesn't just mean "inline-at-Onone", it also allows diagnostic passes to constant-fold through the implementation and actively eliminates debug information. It was never intended to be used for performance, but only for the limited set of things that need it. I imagine that the specialization you get by inlining some things is pretty important by itself. I suspect closure inlining would also be helpful for a lot of low-level "with*" combinators; doesn't transparent inlining do that, and could inline-always-inlining do the same?

For reference: https://github.com/apple/swift/blob/master/docs/TransparentAttr.rst

The only remaining question: is it ok for a non-transparent function to be inlined prior to running some dataflow diagnostics?

Doing so changes the set of programs that are accepted, since it may expose more code to diagnostic passes. We don't want diagnostics to depend on optimizations.

Perhaps we could avoid this by changing the diagnostic passes to intentionally ignore instructions with inlined source locations, though that seems brittle.

-Joe

Well, inline(__always) would be mandatory, not an optimization. But I think your point is that the user does not expect that attribute to affect program legality. Having data-flow diagnostics “ignore” certain instructions sounds horrible. Is there an obvious problem, or is this hypothetical? i.e. is there some diagnostic that is currently enforced at the function boundary but would be considered legal code after inlining?

An example of something that depends on dataflow diagnostics would be something that's constant-foldable causing static overflow diagnostics:

func double(_ x: UInt8) -> UInt8 {
  return x + x
}

_ = double(128)

Making `double` transparent causes the compiler to statically reject the call:

/Users/jgroff/bar.swift:6:5: error: arithmetic operation '128 + 128' (on unsigned 8-bit integer type) results in an overflow
_ = double(128)
    ^

-Joe

Right. In fact, constant propagation appears to be the only diagnostic that needs to run after mandatory inlining (I was misremembering SILDiagnostic pipeline):