[PATCH] D67233: InstCombine: Fix crash on icmp of gep with addrspacecasted null

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: test/Transforms/InstCombine/gep-inbounds-null.ll:223
+  ret i1 %tmp2
+}
----------------
arsenm wrote:
> jdoerfert wrote:
> > I somehow feel we should transform this into `ret i1 false` instead of something we cannot lower into a constant anymore.
> > 
> > However, I see the problem and the fix seems fine. Could you add the following test as well and a fixme above to denote this should be `ret i1 false`?
> > 
> > 
> > ```
> > define i1 @invalid_bitcast_icmp_addrspacecast_as0_null_var(i32 addrspace(5)* %ptr, i32 %idx) {
> > ; CHECK-LABEL: @invalid_bitcast_icmp_addrspacecast_as0_null(
> > ; CHECK-NEXT:  bb:
> > ; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i32 addrspace(5)* [[PTR:%.*]], addrspacecast (i32* null to i32 addrspace(5)*)
> > ; CHECK-NEXT:    ret i1 [[TMP2]]
> > ;
> > bb:
> >   %tmp1 = getelementptr inbounds i32, i32 addrspace(5)* %ptr, i32 %idx
> >   %tmp2 = icmp eq i32 addrspace(5)* %tmp1, addrspacecast (i32* null to i32 addrspace(5)*)
> >   ret i1 %tmp2
> > }
> > ```
> Actually, this did fold to false in my original test. I might have broken something when I copied it into the preexisting test
My original test was comparing to an alloca derived pointer, so I believe it was deleted for other reasons 


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

https://reviews.llvm.org/D67233