> On Dec 1, 2016, at 11:55 AM, Joe Groff <jgr...@apple.com> wrote:
> 
> 
>> On Dec 1, 2016, at 11:48 AM, Andrew Trick <atr...@apple.com 
>> <mailto:atr...@apple.com>> wrote:
>> 
>> Responding on swift-dev to what turned into an interesting discussion…
>> 
>>> On Dec 1, 2016, at 11:19 AM, Joe Groff <jgr...@apple.com 
>>> <mailto:jgr...@apple.com>> wrote:
>>> 
>>>> 
>>>> On Dec 1, 2016, at 11:18 AM, Joe Groff <jgr...@apple.com> wrote:
>>>> 
>>>>> 
>>>>> On Dec 1, 2016, at 11:16 AM, Andrew Trick <atr...@apple.com> wrote:
>>>>> 
>>>>> 
>>>>>> On Dec 1, 2016, at 10:34 AM, Jordan Rose <jordan_r...@apple.com> wrote:
>>>>>> 
>>>>>>> 
>>>>>>> On Nov 30, 2016, at 18:16, Joe Groff <jgr...@apple.com> wrote:
>>>>>>> 
>>>>>>>> 
>>>>>>>> On Nov 30, 2016, at 6:10 PM, Andrew Trick <atr...@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 
>>>>>> <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):

  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

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Reply via email to