Page MenuHomePhabricator

Restore "[ARM, Fix] Fix emitLeading/TrailingFence on old ARM processors"

Authored by morisset on Sep 17 2014, 12:43 PM.



This patch was originally in D5304 (I could not find a way to reopen that revision).
It was accepted, commited and broke the build bots because the overloading of
the constructor of ArrayRef for braced initializer lists is not supported by all
toolchains. I then reverted it, and propose this fixed version that uses a plain
C array instead in makeDMB (that array is then converted implicitly to an
ArrayRef, but that is not behind an ifdef). Could someone confirm me whether
initialization lists for plain C arrays are supported by every toolchain used
to build llvm ? Otherwise I can just initialize the array in the old way:
args[0] = ...; .. ; args[5] = ...;

Below is the description of the original patch:

I had only tested this code for ARMv7 and ARMv8. This patch adds several
fallback paths if the processor does not support dmb ish:
- dmb sy if a cortex-M with support for dmb
- mcr p15, #0, r0, c7, c10, #5 for ARMv6 (special instruction equivalent to a DMB)
These fallback paths were chosen based on the code for fence seq_cst.

Thanks to luqmana for having noticed this bug.

Diff Detail


Event Timeline

morisset updated this revision to Diff 13796.Sep 17 2014, 12:43 PM
morisset retitled this revision from to Restore "[ARM, Fix] Fix emitLeading/TrailingFence on old ARM processors".
morisset updated this object.
morisset edited the test plan for this revision. (Show Details)
morisset added reviewers: jfb, t.p.northover, luqmana.
morisset added subscribers: aemerson, Unknown Object (MLST).
jfb accepted this revision.Sep 17 2014, 1:00 PM
jfb edited edge metadata.

Yes, the C array init should work. You can even remove the 6 from [6] if you want.

This revision is now accepted and ready to land.Sep 17 2014, 1:00 PM
dpeixott accepted this revision.Sep 17 2014, 2:23 PM
dpeixott added a reviewer: dpeixott.
luqmana accepted this revision.Sep 17 2014, 2:57 PM
luqmana edited edge metadata.


morisset closed this revision.Sep 18 2014, 12:06 PM
morisset updated this revision to Diff 13841.

Closed by commit rL218066 (authored by @morisset).