[PATCH] D151312: [PowerPC][AIX] Enable quadword atomics by default for AIX

Kai Luo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 3 23:06:31 PDT 2023


lkail added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:18411
 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()) &&
+  return Subtarget.isPPC64() && EnableQuadwordAtomics &&
          Subtarget.hasQuadwordAtomics();
----------------
hubert.reinterpretcast wrote:
> lkail wrote:
> > hubert.reinterpretcast wrote:
> > > With this, turning off `EnableQuadwordAtomics` will have an effect for non-AIX platforms. I think this function should be left alone (except for updating the comment to say that the ability to turn off quadword atomics for AIX is a historical artifact).
> > That's a good point. Will update the comment.
> I meant that perhaps the functionality should be left the same as before? Otherwise, the scope of the patch is bigger than necessary (and tests may need expansion).
Is it still necessary to keep `EnableQuadwordAtomics`(and defaults to `true`) for AIX? Since the updated libatomic is already delivered, I don't see the essentiality to keep it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151312



More information about the llvm-commits mailing list