[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/AST/APValue.h:379
   void setLValue(LValueBase B, const CharUnits &O, NoLValuePath,
-                 unsigned CallIndex);
+                 unsigned CallIndex, bool IsNuppPtr);
   void setLValue(LValueBase B, const CharUnits &O,
----------------
Typo.


================
Comment at: lib/AST/ExprConstant.cpp:1153
+        IsNullPtr = false;
+    }
+    void adjustOffsetAndIndex(EvalInfo &Info, const Expr *E, uint64_t Index,
----------------
Hmm.  I think the code would be clearer overall if this were conditionalized in the callers instead of here.


================
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");
----------------
yaxunl wrote:
> rjmccall wrote:
> > Why is this necessary?  ptrtoint on the recursively-emitted null pointer should do this automatically.
> Since the target knows the value in the null pointers, it can fold a null pointer to integer literal directly.
> 
> The above code does that, e.g.
> 
> ```
> void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
>                                            size_t arg_local,
>                                            size_t arg_global,
>                                            size_t arg_constant,
>                                            size_t arg_generic);
> 
> // CHECK-LABEL: test_cast_null_pointer_to_sizet
> // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1, i64 -1, i64 0, i64 0, i64 0)
> void test_cast_null_pointer_to_sizet(void) {
>   test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
>                                         (size_t)((local char*)0),
>                                         (size_t)((global char*)0),
>                                         (size_t)((constant char*)0),
>                                         (size_t)((generic char*)0));
> }
> 
> ```
> 
> Without the above code, we only get ptrtoint instructions.
Oh, does the constant folder not know how to fold ptrtoint(inttoptr(X)) -> X?  I guess that's probably LLVM's nominal target-independence rearing its head.

Is getting a slightly LLVM constant actually important here?  I would prefer to avoid this complexity (and unnecessary recursive walk) if possible.


================
Comment at: lib/CodeGen/TargetInfo.h:236
+  virtual llvm::Value *performAddrSpaceCast(CodeGen::CodeGenFunction &CGF,
+      Expr *E, QualType DestTy) const;
+
----------------
This should just take a Value* and the source and dest types.


https://reviews.llvm.org/D26196