[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 09:53:06 PDT 2017


lei added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:2239-2243
+  // If the address is the result of an add, we will utilize the fact that the
+  // address calculation includes an implicit add. However we don't want to
+  // materialize a constant just for use as the index register. Unless this is
+  // an add of a value and a 16-bit signed constant and both have a single use,
+  // we get rid of the add.
----------------
echristo wrote:
> Between this and the comment underneath the if conditional it's a little hard to understand the tradeoffs involved. Also Stefan's comment.
change: 
    However we don't want to materialize a constant just for use as the index register.
to:
    However, we can reduce register pressure if we do not materialize a constant just for use as the index register.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:2247
+      (!(isIntS16Immediate(N.getOperand(1), imm) &&
+         N.getOperand(1).hasOneUse() && N.getOperand(0).hasOneUse()))) {
     Base = N.getOperand(0);
----------------
stefanp wrote:
> stefanp wrote:
> > Do we need `N.getOperand(1)` to have only one use?
> > For example: 
> > ```
> > r10 = (generate r10 somehow)
> > ADD r4 = r3 + r10  (Is ADD, is not immed S16, r3 has one use, BUT r10 has two uses)
> > LD r5 = (0, r4)
> > ADD r7 = r6 + r10 
> > LD r8 = (0, r7)
> > ```
> > Ideally I think we can get rid of those ADDs from above. 
> > There may be a case I'm not thinking of here. 
> Sorry... I made a mistake with that last comment.
> Ok, so I mixed up the location of the NOT in that if statement.
> 
> It may be clearer if instead of `! ( COND && COND && COND)` it's `!COND || !COND || !COND`. 
> At least it may be clearer in my head...
> 
> Either way, I'll have to go back and look at this again. 
> 
Will update the if condition accordingly


https://reviews.llvm.org/D35027





More information about the llvm-commits mailing list