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,
ConvertType(DestTy));
}
----------------
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(),
NULL).
================
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.
https://reviews.llvm.org/D26196
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
