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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
