[PATCH] D47423: Simplify recursive launder.invariant.group and strip

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1294-1295
+        "simplifyInvariantGroupIntrinsic only handles launder and strip");
+  if (Result->getType()->getPointerAddressSpace() !=
+      II.getType()->getPointerAddressSpace())
+    Result = IC.Builder.CreateAddrSpaceCast(Result, II.getType());
----------------
Prazek wrote:
> rsmith wrote:
> > I think this is still not right from the `bitcast` / `addrspacecast` perspective: if we previously had a `bitcast` from one address space to another, this will convert it into an `addrspacecast`, which may mean something else. As a pathological example, if we had:
> > 
> > ```
> > %a = call launder(...)   ; i8 addrspace(0)*
> > %b = bitcast %a to i8 addrspace(1)*
> > %c = addrspacecast %b to i8 addrspace(2)*
> > %d = bitcast %c to i8 addrspacecast(3)*
> > %e = call launder(%d)
> > ```
> > 
> > ... then you need to rebuild an addrspacecast from address space 1 to address space 2, even though your source is in address space 0 and your destination is in address space 3. (And in general, you may need to rebuild more than one address space cast.)
> AFAIK the addrspace casts are just like bitcasts, so code like this:
>   define i8 addrspace(3)* @foo(i8* %a) {
> 
>     %b = addrspacecast i8* %a to i8 addrspace(1)*
>     %c = addrspacecast i8 addrspace(1)* %b to i8 addrspace(2)*
>     %d = addrspacecast i8 addrspace(2)* %c to i8 addrspace(3)*
>     
>     ret i8 addrspace(3)* %d
>   }
> Is optimized by opt to:
>   define i8 addrspace(3)* @foo(i8* readnone %a) local_unnamed_addr #0 {
>     %d = addrspacecast i8* %a to i8 addrspace(3)*
>     ret i8 addrspace(3)* %d
>   }
> 
> I didn't use bitcasts because if I am not wrong, you cant cast between addrspace with bitcast. Do you think that with this assumption the transformation is correct?
> 
> 
I can't find any restriction that would prevent bitcasting between pointers to different address spaces; the only restriction on pointer `bitcast` I can find in the LangRef is that the source and destination pointer types be the same size. In practice, LLVM appears to produce inttoptr(ptrtoint(p)) instead of a cross-address-space bitcast, though, so maybe it's not actually permitted.

Feedback from someone who knows the IR rules better than I do would be useful :)


Repository:
  rL LLVM

https://reviews.llvm.org/D47423