Fix LICM to avoid hoisting never-executed traps

[atrick]

It is legal for the optimizer to consider code after a loop always
reachable, but when a loop has no exits, or when the loops exits are
dominated by a conditional statement, we should not consider
conditional statements within the loop as dominating all possible
execution paths through the loop. At least not when there is at least
one path through the loop that contains a "synchronization point",
such as a function that may contain a memory barrier, perform I/O, or
exit the program.

Sadly, we still don't model synchronization points in the optimizer,
so we need to conservatively assume all loops have a synchronization
point and avoid hoisting conditional traps that may never be executed.

Fixes rdar://66791257 (Print statement provokes "Can't unsafeBitCast
between types of different sizes" when optimizations enabled)

Originated in 2014.

[atrick]

@swift-ci test

[atrick]

@swift-ci benchmark

[aschwaighofer]

LGTM

[swift-ci]

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	4164	6194	+48.8%	0.67x (?)
StringComparison_slowerPrenormal	1510	1660	+9.9%	0.91x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
StringComparison_slowerPrenormal	1510	1670	+10.6%	0.90x (?)

Code size: -Osize

Performance: -Onone

Improvement	OLD	NEW	DELTA	RATIO
StringBuilderLong	1780	1650	-7.3%	1.08x (?)

Code size: -swiftlibs

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

[atrick]

@shahmishal fyi, CI didn't trigger... oh, looks like CI was rebooted. never mind.

[atrick]

@swift-ci test

[shahmishal]

We had outage on ci.swift.org today, which might have caused the issue.

[shahmishal]

@swift-ci test

[shahmishal]

@swift-ci test macOS

[shahmishal]

@swift-ci test Linux

[atrick]

@mikeash Explanation of what went wrong. I'd add it to the commit message, but that would mean waiting for CI again.

The test case had an infinite loop. The optimizer generally assumes that loops eventually terminate and calls eventually return. So, if there's a condition that will trap after the loop or call, it's ok to trap before executing the loop or call. This is generally an important thing for the optimizer to do, but I think the optimizer should respect synchronization points (I/O, thread communication, program exit). In 2014 I advocated for functions defaulting to no-sync (an no globals), but lost that argument on the language side. Instead, we'll need a way to annotate functions that don't synchronize (and don't modify globals), but never implemented that. Meanwhile, the optimizer continues to assume a different model than the language 6 years later. So, when you call print in a loop, you can't actually rely on that being executed before an assert for example!

The loop invariant code pass (LICM) was interpreting "the loop must exit" as "any code that must run before the exit must execute". For infinite loops, that means that all code in the loop must execute! So, if you inline array subscript.read, and there's a check for whether the array is native, both sides of that check must execute. The non-native side has a test for the element size being object-sized. The optimizer was just assuming it could perform that element size check before executing the loop even though the check doesn't apply to your array at all.

This is a more general bug that could occur even if the loop has exits, but for example, you expect the program to terminate before reaching the exits. The general fix is that LICM should not assume that code that must run before the loop can exit always runs. I fixed LICM to only hoist operations if they occur on all paths through the loop, not just on paths through all loop exits. It doesn't really change anything in the common case, just handles loops that don't exit correctly.