This is an archive of the discontinued LLVM Phabricator instance.

[MSP430] Expand Atomic nodes
Needs ReviewPublic

Authored by INdek on Mar 30 2019, 10:41 AM.

Details

Reviewers
asl
mskvortsov
Summary

Expand most atomic nodes into their respective libcalls.

Missing is the cmpxchg operation which produces ATOMIC_CMP_SWAP_WITH_SUCCESS.
A number of optimizations can still be done with these nodes.

Event Timeline

INdek created this revision.Mar 30 2019, 10:41 AM
asl added a comment.Mar 30 2019, 12:46 PM

Well, what's the overall motivation of all these changes? There are no multiple threads, etc. on msp430. I believe instead of lowering to libcalls we could simply "drop" atomic.

INdek added a comment.Mar 30 2019, 2:24 PM

Well, I've been fixing crashes with the GCC torture suite, and these are some of them, so that's why i've place this change.

When you say "drop", is there a way to tell isel to use the regular instructions for all of the atomics? Because, yeah, that would be better. Also, how do I do that?

There are no multiple threads, etc. on msp430

Threads are a property of the software, not the hardware; atomic operations can still be relevant on a single-processor systems. clang has an option -mthread-model if the user explicitly doesn't want to support multi-threaded execution; I'm not sure it's a good idea to force "-mthread-model single" on all users of msp430. (Granted, maybe msp430 has too little memory for anyone to try multi-threaded programming...)

pftbest added a subscriber: pftbest.Apr 2 2019, 1:08 PM

But MSP430 has interrupts and they are similar to threads in some ways. For example if I need to increment some shared counter, I would like it to be done in a single instruction. That way if some interrupt triggers at the same time it would not corrupt the value, because a single instruction can not be split. I had to use inline assembly to make such kind of counter, because volatile doesn't optimize to a single instruction in debug builds, only in release.

My point is that just forcing mthread-model single may lead to bugs in peoples code if someone would actually use atomics to implement a counter for example.