[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
ChenZheng via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 24 18:09:23 PDT 2022
shchenz added a comment.
Thanks for the explanation. I am more clear now about the background.
================
Comment at: clang/include/clang/Driver/Options.td:3611
HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">;
+def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">,
+ Group<m_Group>, Flags<[CC1Option]>,
----------------
lkail wrote:
> shchenz wrote:
> > amyk wrote:
> > > Would it be better if we called this `maix64-quadword-atomics` instead?
> > Do we need to change the backend check below too?
> > ```
> > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const {
> > // TODO: 16-byte atomic type support for AIX is in progress; we should be able
> > // to inline 16-byte atomic ops on AIX too in the future.
> > return Subtarget.isPPC64() &&
> > (EnableQuadwordAtomics || !Subtarget.getTargetTriple().isOSAIX()) &&
> > Subtarget.hasQuadwordAtomics();
> > }
> > ```
> We don't need to change this yet. When we are compiling a quadword lock free libatomic, we use options `-mabi=quadword-atomics -mllvm -ppc-quadword-atomics` to enforce generating quadword lock-free code on AIX.
This makes me confuse. We need to two different options to control the frontend and backend behavior?
Is it the final usage? Or we will add a follow up patch to map the backend one to the FE one? IMO finally we only need the driver option `-mabi=quadword-atomics` to control the final code generation for 128 bit atomic operations, right?
================
Comment at: clang/lib/Basic/Targets/PPC.cpp:854
+ HasQuadwordAtomics)
+ MaxAtomicInlineWidth = 128;
}
----------------
lkail wrote:
> shchenz wrote:
> > Can we set `MaxAtomicInlineWidth` in `PPC64TargetInfo::setMaxAtomicWidth()`? There is a `TODO` there
> The `TODO` marks our roadmap towards enabling quardword lock free atomics on AIX too. Putting adjustment here is implementation reason: we don't context of `LanguageOptions` in `PPC64TargetInfo::PPC64TargetInfo`.
OK, fair enough.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127189/new/
https://reviews.llvm.org/D127189
More information about the cfe-commits
mailing list