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

Adrien Guinet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 15 23:38:01 PDT 2020


aguinet added inline comments.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:2147
+    CC = CallingConv::Tail;
+    break;
   case lltok::kw_cc: {
----------------
Again here this "big" diff is a result of clang-format. We can see that "kw_aarch64_sve_vector_pcs" has been "clang-formated" but not the rest. I would prefer just add a one-line diff and maybe also add annotations?


================
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;
----------------
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?


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