search

ARMmbed / ATParser

Parser for AT commands and similar protocols.
36 stars 27 forks source link

Insert compiler optimization barriers #16

Closed betzw closed 7 years ago

betzw commented 7 years ago

Insert compiler optimization barriers into methods MyBuffer::put() & MyBuffer::get()

This modification is required to force the compiler to increment the read/write location pointers _rloc and _wloc only after having read/written the data from/to the buffer. This happened on my side on a NUCLEO_F401RE + X-NUCLEO-IDW01M1 using GCC_ARM toolchain (version 6.3.1 20170620 (release)).

cc @geky

geky commented 7 years ago

@c1728p9, oh hey, is this the issue you noted a while back?

geky commented 7 years ago

Other question out of curiousity, is marking _wloc/_rloc as volatile also a valid solution?

betzw commented 7 years ago

_wloc/_rloc are already volatile and this seems not to be sufficient for not making the compiler reorder the instructions.

geky commented 7 years ago

Ignore me, just triggering CI :)

c1728p9 commented 7 years ago

@c1728p9, oh hey, is this the issue you noted a while back?

Yep. I should have just fixed this when I saw it rather than just reporting it.

Other question out of curiousity, is marking _wloc/_rloc as volatile also a valid solution?

The volatile statement only prevents reordering relative to other volatile statements. Since _buf is not pointing to a volatile type the access can be re-ordered before or after the increment (as long as the original index value is used). Changing the element type of _buf to be volatile (and moving the increment out of indexing) would fix this problem, but could degrade performance.

betzw commented 7 years ago

Just to in form you that I have integrated @c1728p9's suggestion.

geky commented 7 years ago

Thanks! Looks good to me, time for merge