rjmccall added inline comments.

Comment at: include/clang/Basic/TargetInfo.h:306
+  /// \param AddrSpace address space of pointee in source language.
+  virtual uint64_t getNullPtrValue(unsigned AddrSpace) const {
+    return 0;
We normally spell out "Pointer", I think.

Comment at: lib/AST/APValue.cpp:585
+bool APValue::isNullPtr() const {
+  return isLValue() && ((const LV*)(const char*)Data.buffer)->IsNullPtr;
Seems reasonable to assert isLValue() here.

Comment at: lib/CodeGen/CGExprAgg.cpp:1054
+    return ICE->getCastKind() == CK_NullToPointer &&
+        CGF.getTypes().isZeroInitializable(E->getType());
   // '\0'
This is probably a common enough case that it's worth "inlining", i.e. adding a 
isPointerZeroInitializable() method that asserts that it's been given a pointer 
type and does the right thing with it.

Comment at: lib/CodeGen/CGExprScalar.cpp:1407
     return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
As discussed, please also delegate addrspace conversions into 
TargetCodeGenInfo.  You don't have to implement them for your target if you 
don't want, but please do the delegation correctly.

Also, even if you decide not to implement the actual dynamic comparison logic, 
you should at least teach your target's implementation to recognize when the 
original pointer is a constant null pointer and just produce the correct new 
constant.  That should eliminate the need for this special case here, as well 
as fixing the case where E is a null pointer with side effects like (foo(), 

Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+      return llvm::ConstantInt::get(ConvertType(DestTy),
+          CGF.getContext().getTargetNullPtrValue(E->getType()));
     assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
Why is this necessary?  ptrtoint on the recursively-emitted null pointer should 
do this automatically.

Comment at: lib/CodeGen/CodeGenModule.h:1156
+  /// Get target specific null pointer.
+  llvm::Constant *getNullPtr(llvm::PointerType *T, QualType QT);
Like above, this should be getNullPointer.  Also, please document which type QT 
is: the pointer type or the pointee type.

Comment at: lib/CodeGen/TargetInfo.h:228
+  virtual llvm::Constant *getNullPtr(const CodeGen::CodeGenModule &CGM,
+      llvm::PointerType *T, QualType QT) const;
Same as the comment on getNullPtr above.


cfe-commits mailing list

Reply via email to