[PATCH] D71217: Fix incorrect logic in maintaining the side-effect of compiler generated outliner functions

[Jin Lin via Phabricator via llvm-commits]

jinlin added a comment.

In D71217#1875226 <https://reviews.llvm.org/D71217#1875226>, @paquette wrote:

> > The outlined function in this test case does not serve the purpose since there are no live in registers. I did check this test case before. If I change the arguments 1, 2, 3, 4 to be incoming registers, the machine outliner won't kick in.
>
> Maybe I'm misunderstanding something here?
>
> (1) Locally, with the patch, adding
>
>   liveins: $w0, $w1, $w2, $w3, $w4
>
>
> does not change the outliner's behaviour.
>
> (2) If I understand correctly, what you are trying to test here is that `implicit $xN` is added to the outlined call when `$xN` is not undefined in the outlined range.
>
> Without this patch, this testcase produces
>
>   BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $w0, implicit-def $w1, implicit-def $w2, implicit-def $w3
>
>
> With the patch, it adds `implicit $wzr, implicit $w4` at the end:
>
>   BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $w3, implicit-def $w2, implicit-def $w1, implicit-def $w0, implicit-def $lr, implicit-def $w3, implicit-def $w2, implicit-def $w1, implicit-def $w0, implicit $sp, implicit $wzr, implicit $w4
>
>
> It's kind of weird that the implicit defs are duplicated though. (`$lr` appears twice in both cases). I'd expect it to be
>
>   BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit-def $w0, implicit-def $w1, implicit-def $w2, implicit-def $w3, implicit $sp, implicit $wzr, implicit $w4
>
>
> Also, I guess it would also be good to add a testcase that ensures that a register is //not// added as implicit when it's undefined in the range.


Hi Jessica. 
My current test case does not have undefined use register issue. It is very difficult to come up another test case to show the undefined register issue. Is it all right to have one test case for this change?
Thanks,
--Jin


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

https://reviews.llvm.org/D71217