[PATCH] D35701: Break up Targets.cpp into a header/impl pair per target type[NFCI]

[Craig Topper via Phabricator via cfe-commits]

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