rjmccall added inline comments.

================
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:
> > 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.
> Since it's target dependent, it won't be constant folded by the target 
> agnostic passes until very late in the backend, which may lose optimization 
> opportunities. I think it's better folded by FE like other constants.
Are you sure?  All of my attempts to produce these constants in LLVM seem to 
get instantly folded, even without a target set in the IR:
  i32 ptrtoint (i8* inttoptr (i32 -1 to i8*) to i32)
  i64 ptrtoint (i8* inttoptr (i64 -1 to i8*) to i64)
This folding literally occurs within ConstantExpr::getPtrToInt, without any 
passes running, and it seems to happen regardless of the pointer having an 
addrspace.

I suspect that the problem is that you additionally have an addrspacecast 
constant in place, which almost certainly does block the constant folder.  The 
right fix for that is to ensure that your target's implementation of 
performAddrSpaceCast makes an effort to peephole casts of Constants.


================
Comment at: lib/CodeGen/TargetInfo.h:236
+  virtual llvm::Value *performAddrSpaceCast(CodeGen::CodeGenFunction &CGF,
+      Expr *E, QualType DestTy) const;
+
----------------
rjmccall wrote:
> This should just take a Value* and the source and dest types.
Please also pass the source QualType.  The QualType is necessary to determine 
the source-language address space, which is how an implementation can correctly 
determine the null-pointer representation in the source type.


https://reviews.llvm.org/D26196



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to