[PATCH] D35027: [PowerPC] Reduce register pressure by not materializing a constant just for use as an index register for X-Form loads/stores

Lei Huang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 6 12:29:36 PDT 2017


lei marked an inline comment as done.
lei added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:2246
+  if (N.getOpcode() == ISD::ADD &&
+      (!(isIntS16Immediate(N.getOperand(1), imm) &&
+         N.getOperand(1).hasOneUse() && N.getOperand(0).hasOneUse()))) {
----------------
jtony wrote:
> It would be nice to break the complex condition into several parts and give an meaningful name for each. It increase the code readability maintainability. Maybe something like this:
> bool AAAA = N.getOpcode() == ISD::ADD;
> bool BBBB = isIntS16Immediate(N.getOperand(1), imm) && N.getOperand(1).hasOneUse() && N.getOperand(0).hasOneUse(), then we can use
> if (AAAA && !BBBB ) {...} instead. As an example name for AAAA, we can use IsADD, and for BBBB: CanGetRidOfAdd. I am not good at naming, you can figure out some better name than I. :-)
Can't break this up.  `N.getOperand(1)` isn’t guaranteed to succeed, we **have to** rely on checking whether it’s an `ISD::ADD` **first**.  Sometimes the address is going to be something like `ISD::TargetConstant`/`ISD::TargetGlobalAddress`, etc. which actually doesn’t have operand 1.


https://reviews.llvm.org/D35027





More information about the llvm-commits mailing list