Implicitly capture a mutating self


(Richard Wei) #1

Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self), .value(value), .value(Int64(count))
        ]
    }

-Richard


(Joe Groff) #2

Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self), .value(value), .value(Int64(count))
        ]
    }

This looks like a compiler bug. Assuming `sync`'s closure is not @escaping, it should be allowed to capture self. If you haven't yet, would you be able to file a bug at bugs.swift.org? Does making a shadow copy work as a workaround, something like:

var tmpSelf = self
device.sync {... &tmpSelf ...}
self = tmpSelf

Note that, independently, this part looks fishy:

try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self)

UnsafePointers formed by passing an argument inout are not valid after the called function returns, so if this function is forming and returning a pointer, it will likely result in undefined behavior. You should use `withUnsafePointer(&self) { p in ... }` to get a usable pointer.

-Joe

···

On Dec 15, 2016, at 11:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:


(Zhao Xin) #3

I did not test the code. But if you cannot implicitly capture a mutating
self, you should do it explicitly, right?

let blockSize = min(512, count)
let blockCount = (count+blockSize-1)/blockSize
device.sync { *[self] () -> () in* // Launch CUDA kernel
    try! fill<<<(blockSize, blockCount)>>>[
        .pointer(to: &self), .value(value), .value(Int64(count))
    ]
}

Hope above code works.

Zhaoxin

···

On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users < swift-users@swift.org> wrote:

Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But
I have to pass inout self to the closure. Any workaround?

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self), .value(value), .value(Int64(count))
        ]
    }

-Richard

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


(Richard Wei) #4

Capturing makes it immutable, which unfortunately can't solve this problem.

-Richard

···

On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:

I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?

let blockSize = min(512, count)
let blockCount = (count+blockSize-1)/blockSize
device.sync { [self] () -> () in // Launch CUDA kernel
    try! fill<<<(blockSize, blockCount)>>>[
        .pointer(to: &self), .value(value), .value(Int64(count))
    ]
}

Hope above code works.

Zhaoxin

On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:
Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self), .value(value), .value(Int64(count))
        ]
    }

<PastedGraphic-1.png>

-Richard

_______________________________________________
swift-users mailing list
swift-users@swift.org <mailto:swift-users@swift.org>
https://lists.swift.org/mailman/listinfo/swift-users


(Rien) #5

How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

···

On 16 Dec 2016, at 09:10, Richard Wei via swift-users <swift-users@swift.org> wrote:

Capturing makes it immutable, which unfortunately can't solve this problem.

-Richard

On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:

I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?

let blockSize = min(512, count)
let blockCount = (count+blockSize-1)/blockSize
device.sync { [self] () -> () in // Launch CUDA kernel
    try! fill<<<(blockSize, blockCount)>>>[
        .pointer(to: &self), .value(value), .value(Int64(count))
    ]
}

Hope above code works.

Zhaoxin

On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:
Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: &self), .value(value), .value(Int64(count))
        ]
    }

<PastedGraphic-1.png>

-Richard

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

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


(Richard Wei) #6

`sync` is not escaping. Shadow copy causes the device memory to make a copy, which can’t be a solution either. I’ll file a radar.

Note that, independently, this part looks fishy:

try! fill<<<(blockSize, blockCount)>>>[
           .pointer(to: &self)

UnsafePointers formed by passing an argument inout are not valid after the called function returns, so if this function is forming and returning a pointer, it will likely result in undefined behavior. You should use `withUnsafePointer(&self) { p in ... }` to get a usable pointer.

This part is a semi-"type safe” wrapper to CUDA kernel launcher. The purpose is to make explicit which argument gets passed to the kernel as a pointer; in this case, self is a `DeviceArray`. `.pointer` is a factory method under `KernelArgument`. Since most arguments to the kernel are passed in by pointers, IMO using a bunch of `withUnsafe...` clauses would only make it look unnecessarily clumsy.

-Richard

···

On Dec 16, 2016, at 12:41, Joe Groff <jgroff@apple.com> wrote:

On Dec 15, 2016, at 11:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:

Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

   let blockSize = min(512, count)
   let blockCount = (count+blockSize-1)/blockSize
   device.sync { // Launch CUDA kernel
       try! fill<<<(blockSize, blockCount)>>>[
           .pointer(to: &self), .value(value), .value(Int64(count))
       ]
   }

This looks like a compiler bug. Assuming `sync`'s closure is not @escaping, it should be allowed to capture self. If you haven't yet, would you be able to file a bug at bugs.swift.org? Does making a shadow copy work as a workaround, something like:

var tmpSelf = self
device.sync {... &tmpSelf ...}
self = tmpSelf

Note that, independently, this part looks fishy:

try! fill<<<(blockSize, blockCount)>>>[
           .pointer(to: &self)

UnsafePointers formed by passing an argument inout are not valid after the called function returns, so if this function is forming and returning a pointer, it will likely result in undefined behavior. You should use `withUnsafePointer(&self) { p in ... }` to get a usable pointer.

-Joe


(Zhao Xin) #7

What is the type of self? If it is a class, try [unowned self].

Zhaoxin

···

On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users <swift-users@swift.org > wrote:

How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

> On 16 Dec 2016, at 09:10, Richard Wei via swift-users < > swift-users@swift.org> wrote:
>
> Capturing makes it immutable, which unfortunately can't solve this
problem.
>
> -Richard
>
>> On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:
>>
>> I did not test the code. But if you cannot implicitly capture a
mutating self, you should do it explicitly, right?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { [self] () -> () in // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> Hope above code works.
>>
>> Zhaoxin
>>
>> On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users < > swift-users@swift.org> wrote:
>> Hi,
>>
>> Swift 3.0.2 seems to have broken my code due to mutating self capture.
But I have to pass inout self to the closure. Any workaround?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> <PastedGraphic-1.png>
>>
>> -Richard
>>
>> _______________________________________________
>> swift-users mailing list
>> swift-users@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-users
>>
>>
>
> _______________________________________________
> swift-users mailing list
> swift-users@swift.org
> https://lists.swift.org/mailman/listinfo/swift-users

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


(Richard Wei) #8

Zhao, it’s not a class.

Rien, unsafe pointers is the last thing I would ever want to introduce in my library…

Capturing self was clean, working perfectly, until Swift 3.0.2.

-Richard

···

On Dec 16, 2016, at 02:49, Zhao Xin <owenzx@gmail.com> wrote:

What is the type of self? If it is a class, try [unowned self].

Zhaoxin

On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:
How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl <http://balancingrock.nl/>
Blog: http://swiftrien.blogspot.com <http://swiftrien.blogspot.com/>
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl <http://swiftfire.nl/>

> On 16 Dec 2016, at 09:10, Richard Wei via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:
>
> Capturing makes it immutable, which unfortunately can't solve this problem.
>
> -Richard
>
>> On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com <mailto:owenzx@gmail.com>> wrote:
>>
>> I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { [self] () -> () in // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> Hope above code works.
>>
>> Zhaoxin
>>
>> On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:
>> Hi,
>>
>> Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> <PastedGraphic-1.png>
>>
>> -Richard
>>
>> _______________________________________________
>> swift-users mailing list
>> swift-users@swift.org <mailto:swift-users@swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-users
>>
>>
>
> _______________________________________________
> swift-users mailing list
> swift-users@swift.org <mailto:swift-users@swift.org>
> https://lists.swift.org/mailman/listinfo/swift-users

_______________________________________________
swift-users mailing list
swift-users@swift.org <mailto:swift-users@swift.org>
https://lists.swift.org/mailman/listinfo/swift-users


(Richard Wei) #9

It’s not the correct choice here :slight_smile:

···

On Dec 16, 2016, at 03:04, Rien <Rien@Balancingrock.nl> wrote:

Just because it is called “Unsafe” does not mean it is unsafe :slight_smile:
It all depends on how you use it.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 16 Dec 2016, at 09:52, Richard Wei <rxrwei@gmail.com> wrote:

Zhao, it’s not a class.

Rien, unsafe pointers is the last thing I would ever want to introduce in my library…

Capturing self was clean, working perfectly, until Swift 3.0.2.

-Richard

On Dec 16, 2016, at 02:49, Zhao Xin <owenzx@gmail.com> wrote:

What is the type of self? If it is a class, try [unowned self].

Zhaoxin

On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users <swift-users@swift.org> wrote:
How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 16 Dec 2016, at 09:10, Richard Wei via swift-users <swift-users@swift.org> wrote:

Capturing makes it immutable, which unfortunately can't solve this problem.

-Richard

On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:

I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?

let blockSize = min(512, count)
let blockCount = (count+blockSize-1)/blockSize
device.sync { [self] () -> () in // Launch CUDA kernel
   try! fill<<<(blockSize, blockCount)>>>[
       .pointer(to: &self), .value(value), .value(Int64(count))
   ]
}

Hope above code works.

Zhaoxin

On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:
Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

   let blockSize = min(512, count)
   let blockCount = (count+blockSize-1)/blockSize
   device.sync { // Launch CUDA kernel
       try! fill<<<(blockSize, blockCount)>>>[
           .pointer(to: &self), .value(value), .value(Int64(count))
       ]
   }

<PastedGraphic-1.png>

-Richard

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

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

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


(Richard Wei) #10

The original file’s here:
https://github.com/rxwei/cuda-swift/blob/master/Sources/Warp/CollectionOperations.swift

I’m assuming this change has affected Dispatch users as well. It's not a good idea for such high-level APIs to rely on explicit pointers to pass mutating self for struct types.

-Richard

···

On Dec 16, 2016, at 03:05, Richard Wei <rxrwei@gmail.com> wrote:

It’s not the correct choice here :slight_smile:

On Dec 16, 2016, at 03:04, Rien <Rien@Balancingrock.nl> wrote:

Just because it is called “Unsafe” does not mean it is unsafe :slight_smile:
It all depends on how you use it.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 16 Dec 2016, at 09:52, Richard Wei <rxrwei@gmail.com> wrote:

Zhao, it’s not a class.

Rien, unsafe pointers is the last thing I would ever want to introduce in my library…

Capturing self was clean, working perfectly, until Swift 3.0.2.

-Richard

On Dec 16, 2016, at 02:49, Zhao Xin <owenzx@gmail.com> wrote:

What is the type of self? If it is a class, try [unowned self].

Zhaoxin

On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users <swift-users@swift.org> wrote:
How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 16 Dec 2016, at 09:10, Richard Wei via swift-users <swift-users@swift.org> wrote:

Capturing makes it immutable, which unfortunately can't solve this problem.

-Richard

On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:

I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?

let blockSize = min(512, count)
let blockCount = (count+blockSize-1)/blockSize
device.sync { [self] () -> () in // Launch CUDA kernel
  try! fill<<<(blockSize, blockCount)>>>[
      .pointer(to: &self), .value(value), .value(Int64(count))
  ]
}

Hope above code works.

Zhaoxin

On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:
Hi,

Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?

  let blockSize = min(512, count)
  let blockCount = (count+blockSize-1)/blockSize
  device.sync { // Launch CUDA kernel
      try! fill<<<(blockSize, blockCount)>>>[
          .pointer(to: &self), .value(value), .value(Int64(count))
      ]
  }

<PastedGraphic-1.png>

-Richard

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

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

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


(Rien) #11

Just because it is called “Unsafe” does not mean it is unsafe :slight_smile:
It all depends on how you use it.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

···

On 16 Dec 2016, at 09:52, Richard Wei <rxrwei@gmail.com> wrote:

Zhao, it’s not a class.

Rien, unsafe pointers is the last thing I would ever want to introduce in my library…

Capturing self was clean, working perfectly, until Swift 3.0.2.

-Richard

On Dec 16, 2016, at 02:49, Zhao Xin <owenzx@gmail.com> wrote:

What is the type of self? If it is a class, try [unowned self].

Zhaoxin

On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users <swift-users@swift.org> wrote:
How about using:

UnsafeMutablePointer<TypeOfSelf>

?

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

> On 16 Dec 2016, at 09:10, Richard Wei via swift-users <swift-users@swift.org> wrote:
>
> Capturing makes it immutable, which unfortunately can't solve this problem.
>
> -Richard
>
>> On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:
>>
>> I did not test the code. But if you cannot implicitly capture a mutating self, you should do it explicitly, right?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { [self] () -> () in // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> Hope above code works.
>>
>> Zhaoxin
>>
>> On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users <swift-users@swift.org> wrote:
>> Hi,
>>
>> Swift 3.0.2 seems to have broken my code due to mutating self capture. But I have to pass inout self to the closure. Any workaround?
>>
>> let blockSize = min(512, count)
>> let blockCount = (count+blockSize-1)/blockSize
>> device.sync { // Launch CUDA kernel
>> try! fill<<<(blockSize, blockCount)>>>[
>> .pointer(to: &self), .value(value), .value(Int64(count))
>> ]
>> }
>>
>> <PastedGraphic-1.png>
>>
>> -Richard
>>
>> _______________________________________________
>> swift-users mailing list
>> swift-users@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-users
>>
>>
>
> _______________________________________________
> swift-users mailing list
> swift-users@swift.org
> https://lists.swift.org/mailman/listinfo/swift-users

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


(Joe Groff) #12

Unfortunately, that's the only defined way to write this code. The pointer you get as an argument from `&self` is only good until pointer(to:) returns, so it won't be guaranteed to be valid by the time you use it, and the compiler will assume that `self` doesn't change afterward, so any mutation done to `self` through that pointer will lead to undefined behavior. Rewriting this code to use withUnsafePointer should also work around the capture bug without requiring a shadow copy:

   let blockSize = min(512, count)
   let blockCount = (count+blockSize-1)/blockSize
   withUnsafePointer(to: &self) { selfPtr in
     device.sync { // Launch CUDA kernel
         try! fill<<<(blockSize, blockCount)>>>[
             .pointer(to: selfPtr), .value(value), .value(Int64(count))
         ]
     }
  }

-Joe

···

On Dec 16, 2016, at 12:10 PM, Richard Wei <rxrwei@gmail.com> wrote:

`sync` is not escaping. Shadow copy causes the device memory to make a copy, which can’t be a solution either. I’ll file a radar.

Note that, independently, this part looks fishy:

try! fill<<<(blockSize, blockCount)>>>[
          .pointer(to: &self)

UnsafePointers formed by passing an argument inout are not valid after the called function returns, so if this function is forming and returning a pointer, it will likely result in undefined behavior. You should use `withUnsafePointer(&self) { p in ... }` to get a usable pointer.

This part is a semi-"type safe” wrapper to CUDA kernel launcher. The purpose is to make explicit which argument gets passed to the kernel as a pointer; in this case, self is a `DeviceArray`. `.pointer` is a factory method under `KernelArgument`. Since most arguments to the kernel are passed in by pointers, IMO using a bunch of `withUnsafe...` clauses would only make it look unnecessarily clumsy.


(Zhao Xin) #13

Below code should work:

    let blockSize = min(512, count)
    let blockCount = (count+blockSize-1)/blockSize

    let boxedClosure = { (foo:inout MutableDeviceCollection) -> () in
        device.sync { // Launch CUDA kernel
            try! fill<<<(blockSize, blockCount)>>>[
                .pointer(to: foo), .value(value), .value(Int64(count))
            ]
        }
    }

    boxedClosure(&self)

Zhaoxin

···

On Fri, Dec 16, 2016 at 5:12 PM, Richard Wei <rxrwei@gmail.com> wrote:

The original file’s here:
https://github.com/rxwei/cuda-swift/blob/master/Sources/
Warp/CollectionOperations.swift

I’m assuming this change has affected Dispatch users as well. It's not a
good idea for such high-level APIs to rely on explicit pointers to pass
mutating self for struct types.

-Richard

> On Dec 16, 2016, at 03:05, Richard Wei <rxrwei@gmail.com> wrote:
>
> It’s not the correct choice here :slight_smile:
>
>> On Dec 16, 2016, at 03:04, Rien <Rien@Balancingrock.nl> wrote:
>>
>> Just because it is called “Unsafe” does not mean it is unsafe :slight_smile:
>> It all depends on how you use it.
>>
>> Regards,
>> Rien
>>
>> Site: http://balancingrock.nl
>> Blog: http://swiftrien.blogspot.com
>> Github: http://github.com/Swiftrien
>> Project: http://swiftfire.nl
>>
>>
>>
>>
>>> On 16 Dec 2016, at 09:52, Richard Wei <rxrwei@gmail.com> wrote:
>>>
>>> Zhao, it’s not a class.
>>>
>>> Rien, unsafe pointers is the last thing I would ever want to introduce
in my library…
>>>
>>> Capturing self was clean, working perfectly, until Swift 3.0.2.
>>>
>>> -Richard
>>>
>>>> On Dec 16, 2016, at 02:49, Zhao Xin <owenzx@gmail.com> wrote:
>>>>
>>>> What is the type of self? If it is a class, try [unowned self].
>>>>
>>>> Zhaoxin
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Fri, Dec 16, 2016 at 4:33 PM, Rien via swift-users < > swift-users@swift.org> wrote:
>>>> How about using:
>>>>
>>>> UnsafeMutablePointer<TypeOfSelf>
>>>>
>>>> ?
>>>>
>>>> Regards,
>>>> Rien
>>>>
>>>> Site: http://balancingrock.nl
>>>> Blog: http://swiftrien.blogspot.com
>>>> Github: http://github.com/Swiftrien
>>>> Project: http://swiftfire.nl
>>>>
>>>>
>>>>
>>>>
>>>>> On 16 Dec 2016, at 09:10, Richard Wei via swift-users < > swift-users@swift.org> wrote:
>>>>>
>>>>> Capturing makes it immutable, which unfortunately can't solve this
problem.
>>>>>
>>>>> -Richard
>>>>>
>>>>>> On Dec 16, 2016, at 02:05, Zhao Xin <owenzx@gmail.com> wrote:
>>>>>>
>>>>>> I did not test the code. But if you cannot implicitly capture a
mutating self, you should do it explicitly, right?
>>>>>>
>>>>>> let blockSize = min(512, count)
>>>>>> let blockCount = (count+blockSize-1)/blockSize
>>>>>> device.sync { [self] () -> () in // Launch CUDA kernel
>>>>>> try! fill<<<(blockSize, blockCount)>>>[
>>>>>> .pointer(to: &self), .value(value), .value(Int64(count))
>>>>>> ]
>>>>>> }
>>>>>>
>>>>>> Hope above code works.
>>>>>>
>>>>>> Zhaoxin
>>>>>>
>>>>>> On Fri, Dec 16, 2016 at 3:46 PM, Richard Wei via swift-users < > swift-users@swift.org> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Swift 3.0.2 seems to have broken my code due to mutating self
capture. But I have to pass inout self to the closure. Any workaround?
>>>>>>
>>>>>> let blockSize = min(512, count)
>>>>>> let blockCount = (count+blockSize-1)/blockSize
>>>>>> device.sync { // Launch CUDA kernel
>>>>>> try! fill<<<(blockSize, blockCount)>>>[
>>>>>> .pointer(to: &self), .value(value), .value(Int64(count))
>>>>>> ]
>>>>>> }
>>>>>>
>>>>>> <PastedGraphic-1.png>
>>>>>>
>>>>>> -Richard
>>>>>>
>>>>>> _______________________________________________
>>>>>> swift-users mailing list
>>>>>> swift-users@swift.org
>>>>>> https://lists.swift.org/mailman/listinfo/swift-users
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> swift-users mailing list
>>>>> swift-users@swift.org
>>>>> https://lists.swift.org/mailman/listinfo/swift-users
>>>>
>>>> _______________________________________________
>>>> swift-users mailing list
>>>> swift-users@swift.org
>>>> https://lists.swift.org/mailman/listinfo/swift-users
>>>>
>>>
>>
>


(Richard Wei) #14

Thanks. That’s true, but there are cases (like making BLAS calls) where I have to nest more than 4 `withUnsafeMutable…` closures. It’s safe by really clumsy. I just wish there were a cleaner way that looks like the do-notation in Haskell or for-notation in Scala.

-Richard

···

On Dec 16, 2016, at 14:16, Joe Groff <jgroff@apple.com> wrote:

On Dec 16, 2016, at 12:10 PM, Richard Wei <rxrwei@gmail.com> wrote:

`sync` is not escaping. Shadow copy causes the device memory to make a copy, which can’t be a solution either. I’ll file a radar.

Note that, independently, this part looks fishy:

try! fill<<<(blockSize, blockCount)>>>[
         .pointer(to: &self)

UnsafePointers formed by passing an argument inout are not valid after the called function returns, so if this function is forming and returning a pointer, it will likely result in undefined behavior. You should use `withUnsafePointer(&self) { p in ... }` to get a usable pointer.

This part is a semi-"type safe” wrapper to CUDA kernel launcher. The purpose is to make explicit which argument gets passed to the kernel as a pointer; in this case, self is a `DeviceArray`. `.pointer` is a factory method under `KernelArgument`. Since most arguments to the kernel are passed in by pointers, IMO using a bunch of `withUnsafe...` clauses would only make it look unnecessarily clumsy.

Unfortunately, that's the only defined way to write this code. The pointer you get as an argument from `&self` is only good until pointer(to:) returns, so it won't be guaranteed to be valid by the time you use it, and the compiler will assume that `self` doesn't change afterward, so any mutation done to `self` through that pointer will lead to undefined behavior. Rewriting this code to use withUnsafePointer should also work around the capture bug without requiring a shadow copy:

  let blockSize = min(512, count)
  let blockCount = (count+blockSize-1)/blockSize
  withUnsafePointer(to: &self) { selfPtr in
    device.sync { // Launch CUDA kernel
        try! fill<<<(blockSize, blockCount)>>>[
            .pointer(to: selfPtr), .value(value), .value(Int64(count))
        ]
    }
}

-Joe


(David Sweeris) #15

You might be able to do something with wrapper functions or operators. As long as you tell the compiler to always inline them, I don't think that'd cause any performance issues.

- Dave Sweeris

···

On Dec 16, 2016, at 15:54, Richard Wei via swift-users <swift-users@swift.org> wrote:

Thanks. That’s true, but there are cases (like making BLAS calls) where I have to nest more than 4 `withUnsafeMutable…` closures. It’s safe by really clumsy. I just wish there were a cleaner way that looks like the do-notation in Haskell or for-notation in Scala.


(Joe Groff) #16

It can help to define a pointer-taking function at the outer level of the scope where you need the pointers, so that instead of:

withUnsafeMutablePointer(&a) { p in
  withUnsafeMutablePointer(&b) { q in
    withUnsafeMutablePointer(&c) { r in
      doThing(p, q, r)
    }
  }
}

You could write:

func doThingWithPointers(_ p: UnsafeMutablePointer<A>, _ q: UnsafeMutablePointer<B>, _ r: UnsafeMutablePointer<C>) {
  doThing(p, q, r)
}

doThingWithPointers(&a, &b, &c)

-Joe

···

On Dec 16, 2016, at 3:54 PM, Richard Wei <rxrwei@gmail.com> wrote:

Thanks. That’s true, but there are cases (like making BLAS calls) where I have to nest more than 4 `withUnsafeMutable…` closures. It’s safe by really clumsy. I just wish there were a cleaner way that looks like the do-notation in Haskell or for-notation in Scala.