Well this one is interesting.
I think I see what you're getting at here; if we can ASC away all uses of a generic pointer except the icmp, then keeping around the generic pointers in icmp is just unnecessary register pressure.
But at least on NVPTX, I think this is only safe if we can transform *both* pointers in the icmp, and moreover only if both pointers are transformed to the same AS.
I don't see where we check that NewV and OtherNewV have the same AS. Shouldn't it be right here?
Sorry if it's obvious; I'm breaking my rule against reviewing patches late at night, and may be embarrassed in the morning.
Just so I know how to read these tests: AS(4) is the flat AS?
Can we have a test for icmp x, undef and icmp undef, x?
If KOtherSrc->AS() == NewAS, then isn't this ASC redundant? It seems we should only need it if KOtherSrc is undef, and in that case, can we just substitute a new undef value, instead of ASC'ing the old undef?
I can add another test for undef, x, but there's no point in trying to handle these in the code since these are canonicalized to constants in the RHS
I made this too conservative by adding the new check here, so this is backwards. The addrspace check is redundant with the cast. We need to cast the constants, otherwise it's making an assumption about the bit values of the pointer. For undef I think replacing the type directly would be OK, but the ConstantExpr cast should do the right thing (and I think this will be necessary in the future with freeze anyway)
Since this is now generic, we probably shouldn't say "shared" here.
Is this comment now out of date? I'm confused about "instead of currently".
Would also prefer for the comment to start with prose. For example:
// If we can infer that both pointers are in the same addrspace, transform e.g. // %cmp = icmp eq float*%p, %q // into // %cmp = icmp eq float addrspace(3)* %new_p, %new_q
Is that all we need to say here?