[PATCH] D89490: Introduce __attribute__((darwin_abi))

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/IR/AsmWriter.cpp:379
+    Out << "aarch64_darwincc";
+    break;
   case CallingConv::SPIR_FUNC:     Out << "spir_func"; break;
----------------
aguinet wrote:
> mstorsjo wrote:
> > The new code here has a different indentation than the rest; the same in a couple other places throughout the patch.
> As clang-format changed that formatting for me, should I reformat the whole switch-case to be "clang-format compatible", or should we set this section as ignored?
As the manually laid out custom formatting here is kinda beneficial, I think it's preferred to keep it as is. If there are annotations to make clang-format stay off of it, that's probably good (but I don't know what our policy regarding use of such annotations is, if we have any).


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:876
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerAAPCSFromDarwin_VASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG &DAG) const;
----------------
aguinet wrote:
> Same problem as with clang-format: clang-tidy suggests "lowerAapcsFromDarwinVastart", which would mean modifying the whole file for consistency. Should we set clang-tidy to ignore this file?
I think the churn generally isn't considered worth it regarding such things; such changes can be quite disruptive to downstream users (with a lot of non-upstream code) for little benefit. Same thing here, not sure what the policy is regarding annotations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490