[PATCH] D149362: [SPIR-V] Rename SPIRVGlobalRegistry to SPIRVGlobalObjectRegistry

Ilia Diachkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 4 05:09:36 PDT 2023


iliya-diyachkov added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.cpp:1-2
-//===-- SPIRVGlobalRegistry.cpp - SPIR-V Global Registry --------*- C++ -*-===//
+//===-- SPIRVGlobalObjectRegistry.cpp - SPIR-V Global Object Registry --*- C++
+//-*-===//
 //
----------------
The same suggestion as for SPIRVGlobalObjectRegistry.h.


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.cpp:8
 //
-//===----------------------------------------------------------------------===//
+//===-----------------------------------------------------------------------------===//
 //
----------------
The same issue.


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.cpp:16
 //
-//===----------------------------------------------------------------------===//
+//===-----------------------------------------------------------------------------===//
 
----------------
The same issue.


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.h:1-2
-//===-- SPIRVGlobalRegistry.h - SPIR-V Global Registry ----------*- C++ -*-===//
+//===-- SPIRVGlobalObjectRegistry.h - SPIR-V Global Object Registry -*- C++
+//-*-===//
 //
----------------
This line break doesn't look very nice. There are such examples in llvm, but much more often this comment is written in one line. We could use something like this 
```
//==- SPIRVGlobalObjectRegistry.h - SPIR-V Global Object Registry -*- C++ -*-=//
```
On other hand, you are going to introduce SPIRVGlobalInstrRegistry files in the next step, reducing `Instruction` to `Instr`. So it may be a suitable solution (in the same way) to use `Obj` instead of `Object` in the file/class name. It's still meaningful, and the names won't be so long.


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.h:8
 //
-//===----------------------------------------------------------------------===//
+//===--------------------------------------------------------------------------===//
 //
----------------
The preferred string length is 80 characters. 


================
Comment at: llvm/lib/Target/SPIRV/Registries/SPIRVGlobalObjectRegistry.h:15
 //
-//===----------------------------------------------------------------------===//
+//===--------------------------------------------------------------------------===//
 
----------------
The same issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149362



More information about the llvm-commits mailing list