cortex-m: fix warning about volatile keyword in incorrect position

[hudson-ayers]

Pull Request Overview

#2002 added a volatile keyword in the position intended for clobbers in the assembly responsible for resetting the stack pointer.
This surfaced a warning whenever compiling a cortex-m4 architecture, but this warning did not cause CI to fail because cd arch/ && RUSTFLAGS="-D warnings" cargo test ignores code that can't be compiled on the host platform, and we do not deny warnings when compiling all boards.

Testing Strategy

This pull request was tested by compiling Imix and seeing that the warning is gone.

TODO or Help Wanted

Ideally we would deny warnings in ci-job-compilation so mistakes like this couldn't slip past CI, but passing RUSTFLAGS="-D warnings" to the call to make allboards within ci-job-compilation did not seem to work, and I did not try anything further.

Documentation Updated

• No updates are required.

Formatting

• Ran make prepush.

[ppannuto]

There are a few other issues here too, namely that not all of the assembly blocks properly specify their clobbers (#2037) -- it would be good to fix all of this while we're here if we can.

[hudson-ayers]

There are a few other issues here too, namely that not all of the assembly blocks properly specify their clobbers (#2037) -- it would be good to fix all of this while we're here if we can.

Okay, I pushed a commit that I think fixes a number of issues with the cortex-m clobbers. I could have made some mistakes though, as I am hardly an inline assembly expert. The fixes can be summarized as follows:

1. anywhere there was a clobber of a register that is also an input or output register, I removed it as a listed clobber (see https://doc.rust-lang.org/unstable-book/library-features/llvm-asm.html#clobbers).

2. anywhere that used instructions which at a glance I knew affected the APSR register (i.e. cmp and similar) I added the "cc" clobber indicating that condition codes are modified by the assembly therein.

3. Added a bunch of registers to the clobbers list where I could see instructions in the assembly modifying those registers despite them not being listed as inputs, outputs, or clobbers.

I tried to match the changes to cortex-m4 and cortex-m3. I did not touch cortex-m0. Someone checking my work would be appreciated.

[bradjc]

Arg can we merge this into #2027? I don't know how we're going to keep all of this straight.

[hudson-ayers]

yeah I can port these changes over to #2027, but I would maybe like if they could be reviewed here first. I think that #2027 will be reviewed as though it is just reorganizing existing code, and its probably good to have at least one more set of eyes on these changes.

[bradjc]

This looks fine to me (but I'm no expert), but I would rather see a more comprehensive review happen with a switch to llvm_asm to asm.

[ppannuto]

Huh, TIL about the cc and memory clobbers; neat.

I think we need memory clobbers wherever we're doing str's IIUC the documentation correctly.

[hudson-ayers]

Huh, TIL about the cc and memory clobbers; neat.

I think we need memory clobbers wherever we're doing str's IIUC the documentation correctly.

Yeah, agreed about the memory clobbers. Slightly better documentation is available here: https://llvm.org/docs/LangRef.html#clobber-constraints , and in general the llvm syntax seems based off of the gcc syntax, which has even better documentation here: https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Clobbers-and-Scratch-Registers

The "memory" clobber tells the compiler that the assembly code performs memory reads or writes to items other than those listed in the input and output operands (for example, accessing the memory pointed to by one of the input parameters). To ensure memory contains correct values, GCC may need to flush specific register values to memory before executing the asm. Further, the compiler does not assume that any values read from memory before an asm remain unchanged after that asm; it reloads them as needed. Using the "memory" clobber effectively forms a read/write memory barrier for the compiler.

[ppannuto]

This looks good to me now

[bradjc]

bors r+

[bors]

Build succeeded:

• ci-build (macos-latest)
• ci-build (ubuntu-latest)
• ci-format (macos-latest)
• ci-format (ubuntu-latest)
• ci-qemu
• ci-tests (macos-latest)
• ci-tests (ubuntu-latest)