[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 4 14:26:32 PST 2019


tra added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7688
 
+  // Coercion type builder for lower HIP pointer argument from generic pointer
+  // to global pointer.
----------------
Nit: `for lower` -> `for lowering` or `that lowers`


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7690
+  // to global pointer.
+  class CoerceGenericPointerTypeBuilder {
+    llvm::LLVMContext &Context;
----------------
I don't think you need a class here -- it just complicates calling of coerce().
I'd just make `coerce()` a member function.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7696
+  public:
+    CoerceGenericPointerTypeBuilder(llvm::LLVMContext &VMCtx, unsigned DAS,
+                                    unsigned GAS)
----------------
Nit: `VM` in `VMCtx` is not useful. `Ctx` or `LLVMCtx` would be better, IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826





More information about the cfe-commits mailing list