[PATCH] D93585: [AArch64] Enable out-of-line atomics by default.

Tue Jan 19 13:05:48 PST 2021

jgreenhalgh added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1087
                      FeatureNEON,
+                     FeatureOutlineAtomics,
                      FeaturePerfMon,
----------------
t.p.northover wrote:
> ilinpv wrote:
> > t.p.northover wrote:
> > > I think this still enables it more widely than we want. Clang overrides it with `-outline-atomics`, but other front-ends don't.
> > Could I ask you to clarify what front-ends you meant (to check outline atomics suport for them)?
> Any front-end that generates LLVM IR. Generally important ones are Swift and Rust, but there are many more and I kind of doubt it's feasible to ensure they all will. It's unfortunate, and maybe at some point we can put logic in LLVM to approximate Clang's decision-making and get the benefit without any opt-in, but I kind of think it's a bit early yet. We'd risk breaking too many people's builds.
> 
> Also, putting it in the generic CPU model here means that if a front-end allows something like `-mcpu=cortex-a57` it will suddenly disable outlined atomics. Whether they're good or bad, that's just plain weird behaviour. A CPU-level feature isn't the right place for this.
Not necessarily arguing in either direction for where these should be enabled, but is that behaviour really so weird?

I would expect that we want to specialise to inline atomics as soon as we're given architecture permission (-march=armv8.1-a+lse or greater) or CPU-directed specialisation. These things are only useful for the transition case where your platform mandates Armv8.0 only binaries but you're expecting to also run fast on cores with LSE support.

What we're nervous about is burying this option, which we believe to be generally useful for the Armv8.0 platforms, such that it is opt-in. It really isn't the end of the world if this is C/C++ on LInux/Android only for now, but it feels like missed opportunity. That said, it is safe, so probably makes sense to go for that now Pavel and think again about where else we want it on.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93585/new/

https://reviews.llvm.org/D93585