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

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 15 23:45:28 PDT 2020


mstorsjo added inline comments.


================
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:
> mstorsjo wrote:
> > 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.
> I do agree that a big diff just for this is counter productive. There are few places where we already have clang-format annotations, like https://github.com/llvm/llvm-project/blob/master/llvm/lib/TextAPI/MachO/TextStub.cpp#L262 . Maybe we can add one here?
Sure, I guess that can be done.

Normally, I don't strictly follow what clang-format suggests blindly, but only selectively keep the bits that look sensible and don't unnecessarily munge otherwise untouched code. But annotations to avoid manually filtering it too much probably is better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490



More information about the llvm-commits mailing list