Comparison of object against addrspacecasted null pointer are not optimized out

[arsenm]

I recently saw a test case where a conditional branch was left behind that should be trivially optimized out. A constant addrspacecast from a generic null pointer is compared to a valid object:

@lds = linkonce_odr hidden addrspace(3) global [128 x i32] undef, align 16

define void @foo() {
bb0:
  br i1 icmp eq ([128 x i32] addrspace(3)* addrspacecast ([128 x i32]* null to [128 x i32] addrspace(3)*), [128 x i32] addrspace(3)* @lds), label %bb1, label %bb2

bb1:
  store volatile i32 0, i32 addrspace(1)* null
  ret void

bb2:
  ret void
}

opt -O3 leaves behind this branch, which should trivially fold to false

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

[nikic]

Is this a valid optimization without target-specific knowledge? We don't give null special semantics for non-default address space, so I'd expect that the @lds global can theoretically be located at the zero address.

[nhaehnle]

@lds can indeed have integer value 0, in the sense that

  %i = ptrtoint ptr addrspace(3) @lds to i32

may result in %i == 0. That's actually a pretty common case. But if you first addrspacecast to addrspace(0) and then do the ptrtoint, you'll get a non-0 integer value.

But in @arsenm's example, we started with an addrspace(0) null pointer, and that does have special null semantics.

[arsenm]

This should be valid under the current LangRef rules. null in addrspace(0) can't point to a valid object, and no amount of casting can get you to one. We have clang emit this cast from null to get a literal -1, given the lack of proper support for non-0 null pointers.

As a side note, we really should get https://reviews.llvm.org/D63525 reviewed and landed.

[arsenm]

https://reviews.llvm.org/D136882 fixes this specific case. It doesn't catch the cases where the compared global has a GEP offset applied

[nhaehnle]

For completeness: @nikic pointed out on Phabricator that icmp is defined to do only bitwise comparison, without regards for objects or provenance. As such, I agree with him that comparisons against addrspacecast ptr null to ptr addrspace(N) can't be folded away without target-specific information about the address spaces.

[arsenm]

That leaves me stuck on deciding where to fold this out. Ideally we want this folded out early, not in a backend pass

[arsenm]

That leaves me stuck on deciding where to fold this out. Ideally we want this folded out early, not in a backend pass

I guess AggressiveInstCombine is the best candidate, since it seems to have TTI available