[PATCH] D100445: [RS4GC] Introduce intrinsics to get base ptr and offset

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:599
 
+  // This value might have been generated by findBasePointer() called when
+  // substituting gc.get.pointer.base() intrinsic.
----------------
yrouban wrote:
> reames wrote:
> > I don't think this piece of the change should be required.  Can you explain why this is needed?
> This case is tested in the new intrinsics.ll:
> 
> Here is a simplified version:
> ```
> define void @test_chained(i8 addrspace(1)* %obj1, i8 addrspace(1)* %obj2, i32 %len, i1 %c) gc "statepoint-example" {
> entry:
>   %obj1.12 = getelementptr inbounds i8, i8 addrspace(1)* %obj1, i64 12
>   %obj2.16 = getelementptr inbounds i8, i8 addrspace(1)* %obj2, i64 16
>   %obj.x = select i1 %c, i8 addrspace(1)* %obj1.12, i8 addrspace(1)* %obj2.16
> 
>   %obj.x.base = call i8 addrspace(1)* @llvm.experimental.gc.get.pointer.base.p1i8.p1i8(i8 addrspace(1)* %obj.x)
>   %obj.x.base_of_base = call i8 addrspace(1)* @llvm.experimental.gc.get.pointer.base.p1i8.p1i8(i8 addrspace(1)* %obj.x.base)
> 
>   ret void
> }
> 
> ```
> After the first //%obj.x.base = get.pointer.base(%obj.x)// is inlined by //inlineGetBaseAndOffset()// we get all //%obj.x.base// uses RAUWed with the result //%obj.x.base1// which is marked with !is_base_value:
> ```
>   %obj1.12 = getelementptr inbounds i8, i8 addrspace(1)* %obj1, i64 12
>   %obj2.16 = getelementptr inbounds i8, i8 addrspace(1)* %obj2, i64 16
>   %obj.x.base1 = select i1 %c, i8 addrspace(1)* %obj1, i8 addrspace(1)* %obj2, !is_base_value !0
>   %obj.x = select i1 %c, i8 addrspace(1)* %obj1.12, i8 addrspace(1)* %obj2.16
>   %obj.x.base_of_base = call i8 addrspace(1)* @llvm.experimental.gc.get.pointer.base.p1i8.p1i8(i8 addrspace(1)* %obj.x.base1)
> ```
> Then //inlineGetBaseAndOffset()// calls //findBasePointer(%obj.x.base1)//, which ends up in instantiating //BaseDefiningValueResult(%obj.x.base1, IsKnownBase)//.
I think you might be thinking about an older implementation of RS4GC, we recently improved the find base pointer algorithm to detect obj.x.base as a base pointer without the hint, and the metadata check should no longer be needed.  If not, I'd like to see the reduced test case which still requires it because it's probably a lurking inference bug.  

Can I ask you to run that test?

To be clear, I don't really care about this.  It doesn't do any major harm to have the redundant check.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100445