[PATCH] D31852: [PowerPC] Convert reg/reg instructions fed by constants to reg/imm instructions (pre and post RA)

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 31 02:11:21 PDT 2017


nemanjai added a comment.

In https://reviews.llvm.org/D31852#911347, @hfinkel wrote:

> This is really interesting. So we're seeing a number of cases where, after instruction selection, we're doing something that makes operations have constant operands. Do you know what that is? Maybe one thing we could do with this is have it assert, instead of transforming the code, and then reduce the test cases in order to understand why this happens.


I don't think that we can catch all of these early enough not to need something like this. I think that fundamentally, we're limited in our ability to select the right instruction when values come from other blocks. Most of the early cases are likely due to SDAG being bb-local. The late opportunities arise from late transformations that may eliminate or duplicate things. The motivating example for the late pass was a case where an input to a compare comes from a PHI. One of the PHI inputs is a load and the other is a constant. Block placement then duplicates the block and we end up with one block having

  li 3, 15
  cmpld  24, 3

which this patch transforms in the obvious way to `cmpldi 24, 15`.

Also, what initially prompted the early pass is that we are looking to commit the code that will produce GPR-only sequences for compares (https://reviews.llvm.org/D38575), which we have to do in C++ code. Rather than adding special cases for constant comparisons, we clean it up with something like this and get the benefits of cleaning up any other such code we emit now and in the future.

Finally, I am afraid that leaving such an assert in the code might make it likely to trigger due to other changes that may be made in target-independent code. This would be good to prompt us to fix yet another issue, but might possibly lead to:

1. Frustrating the community as their patches trigger this assert in the PPC back end
2. Adding special cases where we accept that we emit code like this under specific conditions
3. An outright removal of the assert by someone


Repository:
  rL LLVM

https://reviews.llvm.org/D31852





More information about the llvm-commits mailing list