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

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:508
 
   // Only tail call if the following instruction is a standard return.
   auto Next = next_nodbg(MI.getIterator(), MBB.instr_end());
----------------
This comment is out of date with this change.


================
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:
----------------
Would be nice if this could be done for `thisreturn` callees in general, not just the memcpy/memmove/memset calls? Maybe a TODO?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:548
+    Next = next_nodbg(Next, MBB.instr_end());
+  }
+
----------------
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
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105382