craig.topper added a comment. Just review blank lines between every function. I'm too lazy to keep marking them.
================ Comment at: lib/Basic/Targets/AArch64.cpp:20 +using namespace clang::targets; +const char *const AArch64TargetInfo::GCCRegNames[] = { + // 32-bit Integer registers ---------------- Blank line ================ Comment at: lib/Basic/Targets/AArch64.h:1 +//===--- x86.h - Declare AArch64 target feature support -------------------===// +// ---------------- Header says x86 ================ Comment at: lib/Basic/Targets/AArch64.h:1 +//===--- x86.h - Declare AArch64 target feature support -------------------===// +// ---------------- craig.topper wrote: > Header says x86 Also I think .h files are supposed to have 'cpp' in their top line. ================ Comment at: lib/Basic/Targets/AArch64.h:16 +#define LLVM_CLANG_LIB_BASIC_TARGETS_AARCH64_H +#include "OSTargets.h" +#include "clang/Basic/TargetBuiltins.h" ---------------- Prevailing style is blank line between the include guard and the includes i think ================ Comment at: lib/Basic/Targets/AArch64.h:19 +#include "llvm/Support/TargetParser.h" +namespace clang { +namespace targets { ---------------- Blank line ================ Comment at: lib/Basic/Targets/AArch64.h:20 +namespace clang { +namespace targets { +class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo { ---------------- blank line ================ Comment at: lib/Basic/Targets/AArch64.h:426 + : AArch64TargetInfo(Triple, Opts) {} + void getTargetDefines(const LangOptions &Opts, + MacroBuilder &Builder) const override { ---------------- Blank line ================ Comment at: lib/Basic/Targets/AArch64.h:468 +}; +// 64-bit RenderScript is aarch64 +class LLVM_LIBRARY_VISIBILITY RenderScript64TargetInfo ---------------- Blank line ================ Comment at: lib/Basic/Targets/AMDGPU.cpp:25 +using namespace llvm; +namespace clang { +namespace targets { ---------------- blank line ================ Comment at: lib/Basic/Targets/AMDGPU.cpp:85 +} // namespace clang +const Builtin::Info AMDGPUTargetInfo::BuiltinInfo[] = { +#define BUILTIN(ID, TYPE, ATTRS) \ ---------------- Blank line ================ Comment at: lib/Basic/Targets/AMDGPU.cpp:92 +}; +const char *const AMDGPUTargetInfo::GCCRegNames[] = {"v0", + "v1", ---------------- Can we line break less often here. ================ Comment at: lib/Basic/Targets/AMDGPU.cpp:545 +} +void AMDGPUTargetInfo::adjustTargetOptions(const CodeGenOptions &CGOpts, + TargetOptions &TargetOpts) const { ---------------- blank line ================ Comment at: lib/Basic/Targets/AMDGPU.cpp:566 +} +AMDGPUTargetInfo::GPUKind AMDGPUTargetInfo::parseR600Name(StringRef Name) { + return llvm::StringSwitch<GPUKind>(Name) ---------------- blank line ================ Comment at: lib/Basic/Targets/AMDGPU.h:23 +namespace targets { +class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { + static const Builtin::Info BuiltinInfo[]; ---------------- Blank line https://reviews.llvm.org/D35701 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits