CallSiteSplitting and musttail calls

Project Infrastructure LLVM Dev List Archives
1

Hello!

I’ve discovered that CallSiteSplitting optimization doesn’t support musttail calls. The easiest fix as it stands is disabling it for such call sites: https://reviews.llvm.org/D43729 . However, I’m not happy with such contribution.

My more sophisticated attempt has failed due to my poor understanding of llvm internals. Here is the attempted patch: https://gist.github.com/indutny/240c33522563ebd633613a903479d5e6

I’d greatly appreciate any help with it.

Just in case, there’re few questions that I’m trying to find answers for:

  • Why replacing removeFromParent() with eraseFromParent breaks the code?
  • How to properly remove predecessors from the resulting Tail block?
  • How to remove the Tail block itself when we’re done?

Thank you,
Fedor.

2

Update:

I was able to make progress on it today ( See https://reviews.llvm.org/D43729 ). Apparently my problems were:

  • Iterating through the instruction/block list after erasing block/instruction
  • Trying to split block after removing one predecessor

Regarding the latter, it appears that semantics of DuplicateInstructionsInSplitBetween change significantly in such case, and it starts to loop indefinitely. The SplitEdge function that it calls places all of the instructions into the split block, so that the original block becomes empty.

Is it expected behavior, or am I doing something wrong?

Thanks,
Fedor.

3

I think you realized this now, but to be clear:
More likely, you’ve found some bugs.
Unfortunately, not all of these utilities have good unit tests (though they should!).

This would not be the first set of bugs people have found wrt to very start/end of blocks, or bb == predbb issues.

4

Hi,

I think you realized this now, but to be clear:
More likely, you've found some bugs.
Unfortunately, not all of these utilities have good unit tests (though they should!).

This would not be the first set of bugs people have found wrt to very start/end of blocks, or bb == predbb issues.

Coincidentally I stumbled over a similar issue with bb == predbb in DuplicateInstructionsInSplitBetween too and put up a patch trying to fix it https://reviews.llvm.org/D43822

    Update:

    I was able to make progress on it today ( See
    https://reviews.llvm.org/D43729 ).
    Apparently my problems were:

Great, thanks for putting up a patch for that. It made good progress already and I left a few more comments directly over there.

Cheers,
Florian

5

Ha, that’s a good timing!

Thank you for comments, I’ll address them on phabricator.