Skip to content
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

@_noLocks doesn't work with Builtin.atomicload_{} #74407

Closed
jcavar opened this issue Jun 13, 2024 · 5 comments · Fixed by #75615
Closed

@_noLocks doesn't work with Builtin.atomicload_{} #74407

jcavar opened this issue Jun 13, 2024 · 5 comments · Fixed by #75615
Labels
attributes Feature: Declaration and type attributes bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself @_noLocks Feature → attributes: The @_noLocks attribute performance SIL swift 6.0 unexpected error Bug: Unexpected error

Comments

@jcavar
Copy link

jcavar commented Jun 13, 2024

Description

The new Atomic type cannot be used with @_noLocks since a simple usage reports a violation. It seems that the issue is in the implementation of Atomic load and the use Builtin.atomicload_{} family of functions.

A simplified code sample is available below. Required settings:

swiftSettings: [
    .unsafeFlags(["-disable-availability-checking"]),
    .enableExperimentalFeature("StaticExclusiveOnly"),
    .enableExperimentalFeature("RawLayout"),
    .enableExperimentalFeature("BuiltinModule")
]

Reproduction

import Builtin

public struct MyAtomic: ~Copyable {
  var rawAddress: Builtin.RawPointer {
    Builtin.unprotectedAddressOfBorrow(self)
  }
}

extension MyAtomic {
  @_noLocks
  public func load(ordering: AtomicLoadOrdering) -> Int {
    let result = Builtin.atomicload_monotonic_Int64(rawAddress) // error: this code pattern can cause locking
    return 1
  }
}

Expected behavior

Atomic load doesn't cause locking and performance annotation doesn't report violation.

Environment

Apple Swift version 6.0-dev (LLVM 57177aa1b91540b, Swift 3889ede)
Target: arm64-apple-macosx14.0

Additional information

cc @eeckstein @Azoy

@jcavar jcavar added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jun 13, 2024
@Azoy
Copy link
Contributor

Azoy commented Jun 20, 2024

Looks like we classify atomic instructions as locking:

case BuiltinValueKind::Fence:
case BuiltinValueKind::CmpXChg:
case BuiltinValueKind::AtomicLoad:
case BuiltinValueKind::AtomicStore:
case BuiltinValueKind::AtomicRMW:
  return RuntimeEffect::Locking;

@jcavar
Copy link
Author

jcavar commented Jun 25, 2024

Is there a reason for that? Can we mark them as NoEffect?

@hborla hborla added performance @_noLocks Feature → attributes: The @_noLocks attribute and removed triage needed This issue needs more specific labels labels Jul 14, 2024
@jcavar
Copy link
Author

jcavar commented Jul 30, 2024

Hey @Azoy and @eeckstein, just wanted to check in and see if there have been any updates on this issue?

@AnthonyLatsis AnthonyLatsis added attributes Feature: Declaration and type attributes compiler The Swift compiler itself unexpected error Bug: Unexpected error SIL swift 6.0 labels Jul 31, 2024
eeckstein added a commit to eeckstein/swift that referenced this issue Aug 1, 2024
@eeckstein
Copy link
Contributor

Let's just fix this: #75615

@jcavar
Copy link
Author

jcavar commented Sep 25, 2024

Thank you! Works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attributes Feature: Declaration and type attributes bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself @_noLocks Feature → attributes: The @_noLocks attribute performance SIL swift 6.0 unexpected error Bug: Unexpected error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants