[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: lib/CodeGen/CGBlocks.cpp:723
+                            ? CGM.getNSConcreteStackBlock()
+                            : llvm::Constant::getNullValue(
+                                  CGM.getNSConcreteStackBlock()->getType());
----------------
Anastasia wrote:
> yaxunl wrote:
> > should use CGM.getNullPointer to create a null pointer.
> Btw, does it mean we can no longer use generic llvm::Constant::getNullValue helper for PointerTypes? This feels wrong! Is it possible to extend the helper?
> 
> Also I find it a bit counter intuitive to use getNullPointer with the second argument QualType for the case like this where we don't have an actual AST type. Why is it needed? Some documentation might be helpful here. :) Could we extend this helper to use default second argument or an overload with one argument only. 
The LLVM type may not have sufficient information, so in general situation QualType is needed. The comment before getNullPointer declaration explains the meaning of the parameters:

```
  /// Get target specific null pointer.
  /// \param T is the LLVM type of the null pointer.
  /// \param QT is the clang QualType of the null pointer.
  /// \return ConstantPointerNull with the given type \p T.
  /// Each target can override it to return its own desired constant value.
  virtual llvm::Constant *getNullPointer(const CodeGen::CodeGenModule &CGM,
      llvm::PointerType *T, QualType QT) const;
```



https://reviews.llvm.org/D28814