Release a new version

[YuhanLiin]

The most recently released version of this crate, 0.1.4, does not build on non-MSP430 targets on the latest nightly. This issue is caused by the atomic intrinsics changing in the latest nightly. Master does not have this issue, so it should be released.

[cr1901]

Indeed this was fixed by #6. I agree a new version should be released, but I believe this crate should be deprecated in favor of using portable atomic, which is a proper superset of this crate's functionality. I have personally migrated my MSP430 code to that crate.

Keeping open until I have the bandwidth to update CHANGELOG, make a deprecation msg, and do a release.

[YuhanLiin]

My problem with portable-atomic is the lack of a trait for atomic operations, implemented across all atomic types. This makes it more difficult to write generic register manipulation code, though it's still possible.

[cr1901]

Then for now, I am willing to not deprecate the crate until portable-atomic contains such traits. Alternatively, this could become a wrapper over portable-atomic that implements the traits.

The main thing is I don't think I want to maintain two copies of the inline assembly (portable-atomic's msp430 implementation is based on this crates inline asm).

[YuhanLiin]

It's not that big of a deal (I already figured out a workaround in my own code), so no need to delay deprecation.

[YuhanLiin]

According to these lines portable-atomic doesn't use inline ASM to implement atomic ints, but uses the disable-interrupt method. If we want to replace msp430-atomic then we need to add the inline ASM atomic implementations into portable-atomic. Alternatively we can make the same PR to atomic-polyfill.

[cr1901]

Unfortunately, the inline ASM atomic implementations don't return the modified argument (except for atomic_load, which is implemented using inline ASM). AIUI, Rust atomic ops (and atomics in general) like e.g. fetch_add are expected to return the previous value stored in memory after the value was update.

It's not actually possible to load the old value in memory into a register and then update the memory in a single instruction on MSP430. And if done in multiple instructions, an interrupt could happen between the load of the old value and the update to the new one. If the value in memory was modified within an interrupt, the thread trying to do the atomic update will have loaded and returned an "outdated" rather than "the actual value before the atomic update occurred".

With this constraint, only msp430 loads and stores are truly atomic (and not susceptible to the multi core problem on ARM). So, AFAICT, the disable-interrupt method, except for loads and stores, is the correct thing to do for truly atomic operations. Realizing this was one of the main reasons I wanted to deprecate msp430-atomic.

[YuhanLiin]

I'm only interested in atomically modifying register values in PACs, in which case I don't care about the previous value in-memory. This makes the msp430-atomic semantics perfect for my usecase. On the other hand, the disable-interrupt method doesn't really fit in a PAC IMO. I think we should document the difference between msp430-atomic and portable-atomic, and not deprecate msp430-atomic.

[taiki-e]

@YuhanLiin

My problem with portable-atomic is the lack of a trait for atomic operations, implemented across all atomic types. This makes it more difficult to write generic register manipulation code, though it's still possible.

I have no plans to provide an unsafe trait like msp430-atomic in portable-atomic, but I wonder if a generic atomic type (taiki-e/portable-atomic#8) might be sufficient to solve your problem.

According to these lines portable-atomic doesn't use inline ASM to implement atomic ints, but uses the disable-interrupt method.

As cr1901 said, portable-atomic uses interrupt-disable-based atomic implementation because appropriate atomic operations are not available, except for load/store.

---

@cr1901

So, AFAICT, the disable-interrupt method,
except for loads and stores, is the correct thing to do for truly atomic operations. Realizing this was one of the main reasons I wanted to deprecate msp430-atomic.

As a side note, if MSP430 atomics are implemented in LLVM, LLVM can understand whether the return value is used or not, so they can generate code similar to msp430-atomic in the case where the return value is not used (actually, such optimizations are available on x86). Once LLVM implements MSP430 atomics, portable-atomic will use it.

[YuhanLiin]

Generic atomic type would be pretty useful for me, but it's not critical. My main problem is that I don't need fetch_and_add or fetch_and_xor. I only need add and xor, which are guaranteed to be a single instruction on MSP430.

[cr1901]

LLVM can understand whether the return value is used or not, so they can generate code similar to msp430-atomic in the case where the return value is not used (actually, such optimizations are available on x86).

This is interesting, but I don't like it applies to msp430, as msp430 doesn't have an equivalent to lock that enables removing code surrounding the actual atomic access. Since critical sections invokes assembly with side effects (i.e. volatile), the optimizer can't remove the critical sections that get called to acquire and release a portable-atomic type using a critical section.

[cr1901]

@YuhanLiin In any case, if @taiki-e doesn't want to add the add, not, etc (i.e. without the fetch) operations that become a single assembly instruction for msp430 into portable_atomic (as a potential optimization for when you don't care about the old value), then I will not deprecate this crate. Regardless, I'll do a release soon.

[cr1901]

@YuhanLiin @taiki-e I have updated this crate's README.md with information comparing and contrasting portable-atomic and atomic-polyfill. If this looks good to you, I think I can release v0.1.5.

[YuhanLiin]

Looks pretty good

[taiki-e]

I have updated this crate's README.md with information comparing and contrasting portable-atomic and atomic-polyfill.

Looks good to me!

if @taiki-e doesn't want to add the add, not, etc (i.e. without the fetch) operations that become a single assembly instruction for msp430 into portable_atomic (as a potential optimization for when you don't care about the old value)

I'm fine with adding them.

[cr1901]

Cool, thanks for the feedback both of you! I will release after I sleep.

I'm fine with adding them.

If @YuhanLiin is fine with that, we can deprecate this crate once portable-atomic has the relevant functions (add, not, etc without the fetch).

[YuhanLiin]

I'm good with that

[cr1901]

I will release after I sleep.

I apologize for taking so long; releasing this crate v0.1.5 and msp430 v0.4.1 is unlikely to happen before Dec 6th, due to personal/client matters taking priority (nothing is wrong, just many small things need to be done before Wednesday, and I won't be available until the next Monday). However, releasing is on my radar.

[cr1901]

A new release has been made (finally), along with pointing to portable-atomic as a suitable replacement going forward thanks to taiki-e/portable-atomic#47.