[PATCH] D105382: [GlobalISel] Tail call memcpy/memmove/memset even in the presence of copies

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs marked an inline comment as done and an inline comment as not done.
jroelofs added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:513
+
+  // We can also turn it into a tail call if we have a `thisreturn` callee, and
+  // a sequence like:
----------------
paquette wrote:
> Would be nice if this could be done for `thisreturn` callees in general, not just the memcpy/memmove/memset calls? Maybe a TODO?
The only other case is G_BZERO. Updated to give the default case an unreachable to make this clear.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:548
+    Next = next_nodbg(Next, MBB.instr_end());
+  }
+
----------------
paquette wrote:
> jroelofs wrote:
> > hm. This isn't quite right. It shouldn't be allowed to fire on this case:
> > 
> > ```
> >     %0:_(p0) = COPY $x1
> >     %1:_(p0) = COPY $x0
> >     %2:_(s64) = COPY $x2
> >     G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
> >     $x1 = COPY %0(p0)
> >     RET_ReallyLR implicit $x0
> > ```
> I guess you need to check that the register is implicit on the return? So, you probably also have to check that there //is// a return in the block as well?
> 
> Also, what happens if there's more than one copy after the call?
> 
> e.g. I think this would do the wrong thing here too:
> 
> ```
> %0:_(p0) = COPY $x1
> %1:_(p0) = COPY $x0
> %2:_(s64) = COPY $x2
> G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
> $x1 = COPY %0(p0)
> $x1 = COPY %1(p0)
> RET_ReallyLR implicit $x1
> ```
> I guess you need to check that the register is implicit on the return? 

oh, right, thanks!


I believe the `tailcall; copy; whatever; ret` case is covered in the next line:

```
 if (Next == MBB.instr_end() || TII.isTailCall(*Next) || !Next->isReturn())
    return false;
```

... since the preceding block skips at most a single copy instruction (+ debug insts) after the call. Added this as a test case for good measure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105382