Stop using the _branchHint function #23375
Conversation
LLVM r355981 changed various intrinsic functions, including expect, to require immediate arguments. Swift's _branchHint function has an expected value that is passed in as an argument, so that it cannot use LLVM's expect intrinsic. The good news is that _branchHint is only ever used with immediate arguments, so we can just move the intrinsic into _fastPath and _slowPath and use those instead of _branchHint. As was noted in the documentation, the _fastPath and _slowPath names are confusing but we have passed the point where we can simply rename them. We could add new names but would still need to keep the old ones around for binary compatibility, and it is not clear that it is worth the trouble. I have removed that note from the documentation.
|
@swift-ci please test |
| // never be called in cases where performance matters, so just return the | ||
| // value without any branch hint. | ||
| return actual | ||
| } |
There was a problem hiding this comment.
I think it would be nice to introduce a Swift50ABICompatibility.swift and move this there so that we know what version we are maintaining compatibility with. But, that is beyond the scope of this change.
There was a problem hiding this comment.
+1. My other PRs pending SE introduce a LegacyABI.swift file. It doesn't matter to me what's it's called, but splitting them out improves maintainability.
| // never be called in cases where performance matters, so just return the | ||
| // value without any branch hint. | ||
| return actual | ||
| } |
There was a problem hiding this comment.
+1. My other PRs pending SE introduce a LegacyABI.swift file. It doesn't matter to me what's it's called, but splitting them out improves maintainability.
|
@milseman - the reason that I was suggesting the versioning is because it can allow us to track the set that was changed between [base]-5.1, [base]-5.2, [base]-6.0, etc. It prevents a single file from growing unbounded. But, yes, that can be handled later as well. |
|
OK. I'll wait until you add that LegacyABI.swift file (or a versioned file) and then move the _branchHint function over there. |
|
@compnerd which version, the version it was added to the ABI or it was moved into "legacy"? The version it was introduced to the ABI is part of its |
|
Follow-up PR to move _branchHint to LegacyABI.swift is in #23896 |
LLVM r355981 changed various intrinsic functions, including expect,
to require immediate arguments. Swift's _branchHint function has an
expected value that is passed in as an argument, so that it cannot
use LLVM's expect intrinsic. The good news is that _branchHint is only
ever used with immediate arguments, so we can just move the intrinsic
into _fastPath and _slowPath and use those instead of _branchHint.
As was noted in the documentation, the _fastPath and _slowPath names are
confusing but we have passed the point where we can simply rename them.
We could add new names but would still need to keep the old ones around
for binary compatibility, and it is not clear that it is worth the
trouble. I have removed that note from the documentation.