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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
