[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Tim Northover via Phabricator via llvm-commits]

t.p.northover added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6366
+      // Option -moutline-atomics supported for AArch64 target only.
+      if (Triple.getArch() != llvm::Triple::aarch64) {
+        D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
----------------
This excludes `aarch64_be`, which is a bit weird. Might be best to check `Triple.isAArch64()`.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1818
 
-  if (SrcSize > SlotSize) 
+  if (SrcSize > SlotSize)
     Store = DAG.getTruncStore(Chain, dl, SrcOp, FIPtr, PtrInfo,
----------------
Could you do the whitespace changes separately (if at all)?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4064
+      // If outline atomic available
+      // prepare it's arguments and expand.
+      Ops.append(Node->op_begin() + 2, Node->op_end());
----------------
"its"


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2170
+  SmallVector<SDValue, 4> Ops;
+  if (TLI.getLibcallName(LC)) {
+    Ops.append(Node->op_begin() + 2, Node->op_end());
----------------
I think this is a bit of an abuse of the `LibcallName` mechanism. A separate function in `TargetLowering` would probably be better.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:476
+  bool outlineAtomics() const {
+    // Don't outline atomics if
+    // subtarget has LSE
----------------
I think something is a bit weird with how your clang-format handles comments. Here and earlier line lengths are about half as long as I'd expect.


================
Comment at: llvm/test/CodeGen/AArch64/atomic-ops.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -disable-post-ra -verify-machineinstrs < %s | FileCheck %s
----------------
I'd prefer not to overwrite existing CHECKS that have generic dataflow with ones produced by that tool hardcoding a particular register allocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157