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

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek marked 3 inline comments as done.
Prazek 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());
----------------
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?




Repository:
  rL LLVM

https://reviews.llvm.org/D47423