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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:7655
+    SDValue ImmOpnd =
+        Subtarget->isAIXABI() ? Base.getOperand(0) : Base.getOperand(1);
 
----------------
`ImmOpnd` and `RegOperand` should never be associated with the same `Base` operand index. The conditions should be harmonized. It seems to me that the `OpcodeIsAIXTocData` condition is the more conservative one.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:7657-7658
 
     bool UpdateHBase = false;
     SDValue HBase = Base.getOperand(0);
 
----------------
This is moved a bit too far from where `UpdateHBase` is being set (thus making the initialization of `HBase` here confusing).

Suggestion:

  - Use the default value for `HBase`.
  - Move these declarations down to immediately before the `if (ReplaceFlags)` block.
  - Move the `RegOperand` declaration to immediately before that of `ImmOpnd`.
  - Use `RegOperand` in place of `HBase` until after the `ADDIStocHA8` check.
  - Assign to `HBase` the value of `RegOperand`.





================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:7716-7717
 
     // We found an opportunity.  Reverse the operands from the add
     // immediate and substitute them into the load or store.  If
     // needed, update the target flags for the immediate operand to
----------------
This comment is incorrect. Substitution does not always occur with the operands reversed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155957