[PATCH] D155957: [PPC][AIX] Fix toc-data peephole bug and some related cleanup.

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

It seems to me that this section of this function has built up a significant amount of technical debt and we should really try to get it refactored so it is clearer what is happening and why. Please note that this is a statement about the existing implementation, not just about the updates in this patch.

Just some examples:

- There is a statement that these pseudo instructions are used exclusively for AIX small code model for globals defined in the TOC. Is this enforced somehow? Will it become a problem if it is used in a different context?
- The operands for these instructions are reversed. Why? If we have defined something specifically for this particular optimization and decided to define it in a way that is deliberately different from other similar artifacts, that seems misguided to me.
- It seems that the `ReplaceFlags` variable controls more than whether or not we're replacing the flags
- It should probably be clarified what this code does in different contexts (ELFv1, ELFv2, AIX, different code models - particularly pointing out the differences)

Also, we committed this as well as backported it to the release branch. This is a non-trivial patch that didn't really have time in trunk to really bake in. I think a more appropriate course of action would probably have been to turn off the optimization due to the bug, get that ported back and then work on fixing and re-enabling it in trunk in the early part of the release.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155957