[PATCH] D55147: Exclude non-integral pointers in isBytewiseValue

Mon Dec 3 12:57:24 PST 2018

cherry marked an inline comment as done.
cherry added inline comments.

================
Comment at: test/Transforms/MemCpyOpt/non-integral-ptr.ll:19
+  %2 = bitcast i8 addrspace(1)** @const0 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %1, i8* align 8 %2, i64 8, i1 false)
+  ret void
----------------
efriedma wrote:
> It's not clear to me why a memset is "wrong" here; either way, you're setting all the bits to zero.  Is the issue that const0 is not actually an all-zero constant?
> 
> The LLVM null pointer constant is treated as an all-zero value in LLVM, even on targets where a null pointer constant in C would have some non-zero value.  If you think that isn't appropriate, please start a thread on llvmdev; I haven't really thought it through completely, but it would probably get complicated due to the impact on various transforms like constant folding.  If you need an "opaque" null pointer constant, there's probably a simpler solution: define a special global @llvm.opaque.nullptr or something like that, and make your frontend use it instead of ConstantPointerNull.
Thanks for the comment.

Generating memset isn't immediately wrong, but other parts of the optimizer, like store-to-load forwarding, may try to do further optimizations and bad things could happen. For example, it may construct an integer 0 and convert to non-integral pointer type, which fails, or do other things that invalidate the non-integral pointer semantics.

I'm not suggesting that it should have a non-zero value. It is a zero value, just cannot be converted to/from integer zero value.

Also, there is a similar test test/Transforms/LoopIdiom/non-integral-pointers.ll that checks memset is not used for non-integral pointers.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55147/new/

https://reviews.llvm.org/D55147