[PATCH] D38486: [PPC] Implement the heuristic to choose between a X-Form VSX ld/st vs a X-Form FP ld/st.

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.

Please add test cases for the following:

- An actual use of the VSX instructions (i.e. where the register pressure is high enough that an upper VSX register will be allocated - you should be able to accomplish this with inline asm clobbers lists in a small test case)
- A Power9 test case where we will use an X-Form version (i.e. the offset is large enough not to fit in a displacement field)



================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1992
+// and using the larger register file in high register pressure situations.
+// using the larger register file in high register pressure situations.
+bool PPCInstrInfo::targetInstrHeuristic(MachineInstr &MI) const {
----------------
Redundant line.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1993
+// using the larger register file in high register pressure situations.
+bool PPCInstrInfo::targetInstrHeuristic(MachineInstr &MI) const {
     unsigned UpperOpcode, LowerOpcode;
----------------
The name is a bit too vague. Perhaps something like `expandVSXMemInstr()`, to indicate that it does the actual expansion (i.e. changes the opcode of the MI) as well as that it handles VSX memory operation instructions.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:2864
 
+let AddedComplexity = 400, Predicates = [HasP8Vector] in {
+  let isPseudo = 1 in {
----------------
I really don't think it is a particularly good idea to separate this out here. I'd keep it next to where the definitions of the actual instructions are. This will ensure that you define these in the right scope with all the properties set and will avoid having to figure out what the correct set of `let` expressions is. For example, as you've defined them here, the `mayLoad/mayStore` flags won't be set correctly here. These flags are used in various post-ISEL passes to check for side-effects, so it is important to set them correctly.


https://reviews.llvm.org/D38486