lkail added inline comments.

================
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]>,
----------------
shchenz wrote:
> 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?
> This makes me confuse. We need to two different options to control the 
> frontend and backend behavior?

This is multi-lang support consideration. clang is not the only frontend we 
have using LLVM as backend on AIX. If other language frontend generates `store 
atomic i128, ...`, the backend is supposed to generate libcalls into libatomic 
currently.

> Is it the final usage?
No. We finally want to achieve `-mabi=quadword-atomics` by default and generate 
inline atomic code for cpu above pwr7 by default(no need to take OS into 
consideration).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127189

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to