[PATCH] D31833: [x86] Relax the check in areLoadsFromSameBasePtr

[Craig Topper via llvm-commits]

Nevermind. I guess the correct answer from this function doesn't care about
that. So forget I said anything.

~Craig

On Tue, Apr 11, 2017 at 10:43 AM, Easwaran Raman <eraman at google.com> wrote:

> Why should the chain operand be equal in areLoadsFromSameBasePtr? Or do
> you want an assert in the caller?
>
> Thanks,
> Easwaran
>
>
> On Mon, Apr 10, 2017 at 6:07 PM, Craig Topper <craig.topper at gmail.com>
> wrote:
>
>> Can you leave an assert for the Chain being equal?
>>
>> On Mon, Apr 10, 2017 at 5:52 PM Easwaran Raman via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> eraman updated this revision to Diff 94762.
>>> eraman added a comment.
>>>
>>> Used getOperand(X86::AddrBaseReg) etc instead of getOperand(0). Also
>>> removed if the chain operands match. This check shouldn't be in
>>> areLoadsFromSameBasePtr and the caller already passes two loads that are
>>> uses of the same chain node.
>>>
>>>
>>> https://reviews.llvm.org/D31833
>>>
>>> Files:
>>>   lib/Target/X86/X86InstrInfo.cpp
>>>   test/CodeGen/X86/load-slice.ll
>>>
>>>
>>> Index: test/CodeGen/X86/load-slice.ll
>>> ===================================================================
>>> --- test/CodeGen/X86/load-slice.ll
>>> +++ test/CodeGen/X86/load-slice.ll
>>> @@ -19,10 +19,10 @@
>>>  ; STRESS-LABEL: t1:
>>>  ; Load out[out_start + 8].real, this is base + 8 * 8 + 0.
>>>  ; STRESS: vmovss 64([[BASE:[^(]+]]), [[OUT_Real:%xmm[0-9]+]]
>>> -; Add low slice: out[out_start].real, this is base + 0.
>>> -; STRESS-NEXT: vaddss ([[BASE]]), [[OUT_Real]], [[RES_Real:%xmm[0-9]+]]
>>>  ; Load out[out_start + 8].imm, this is base + 8 * 8 + 4.
>>>  ; STRESS-NEXT: vmovss 68([[BASE]]), [[OUT_Imm:%xmm[0-9]+]]
>>> +; Add low slice: out[out_start].real, this is base + 0.
>>> +; STRESS-NEXT: vaddss ([[BASE]]), [[OUT_Real]], [[RES_Real:%xmm[0-9]+]]
>>>  ; Add high slice: out[out_start].imm, this is base + 4.
>>>  ; STRESS-NEXT: vaddss 4([[BASE]]), [[OUT_Imm]], [[RES_Imm:%xmm[0-9]+]]
>>>  ; Swap Imm and Real.
>>> @@ -34,10 +34,10 @@
>>>  ; REGULAR-LABEL: t1:
>>>  ; Load out[out_start + 8].real, this is base + 8 * 8 + 0.
>>>  ; REGULAR: vmovss 64([[BASE:[^)]+]]), [[OUT_Real:%xmm[0-9]+]]
>>> -; Add low slice: out[out_start].real, this is base + 0.
>>> -; REGULAR-NEXT: vaddss ([[BASE]]), [[OUT_Real]], [[RES_Real:%xmm[0-9]+]]
>>>  ; Load out[out_start + 8].imm, this is base + 8 * 8 + 4.
>>>  ; REGULAR-NEXT: vmovss 68([[BASE]]), [[OUT_Imm:%xmm[0-9]+]]
>>> +; Add low slice: out[out_start].real, this is base + 0.
>>> +; REGULAR-NEXT: vaddss ([[BASE]]), [[OUT_Real]], [[RES_Real:%xmm[0-9]+]]
>>>  ; Add high slice: out[out_start].imm, this is base + 4.
>>>  ; REGULAR-NEXT: vaddss 4([[BASE]]), [[OUT_Imm]], [[RES_Imm:%xmm[0-9]+]]
>>>  ; Swap Imm and Real.
>>> Index: lib/Target/X86/X86InstrInfo.cpp
>>> ===================================================================
>>> --- lib/Target/X86/X86InstrInfo.cpp
>>> +++ lib/Target/X86/X86InstrInfo.cpp
>>> @@ -8980,28 +8980,25 @@
>>>      break;
>>>    }
>>>
>>> -  // Check if chain operands and base addresses match.
>>> -  if (Load1->getOperand(0) != Load2->getOperand(0) ||
>>> -      Load1->getOperand(5) != Load2->getOperand(5))
>>> +  // Lambda to check if both the loads have the same value for an
>>> operand index.
>>> +  auto HasSameOp = [&](int I) {
>>> +    return Load1->getOperand(I) == Load2->getOperand(I);
>>> +  };
>>> +
>>> +  // All operands except the displacement should match.
>>> +  if (!HasSameOp(X86::AddrBaseReg) || !HasSameOp(X86::AddrScaleAmt) ||
>>> +      !HasSameOp(X86::AddrIndexReg) || !HasSameOp(X86::AddrSegmentReg))
>>>      return false;
>>> -  // Segment operands should match as well.
>>> -  if (Load1->getOperand(4) != Load2->getOperand(4))
>>> +
>>> +  // Now let's examine if the displacements are constants.
>>> +  auto Disp1 = dyn_cast<ConstantSDNode>(Load1
>>> ->getOperand(X86::AddrDisp));
>>> +  auto Disp2 = dyn_cast<ConstantSDNode>(Load2
>>> ->getOperand(X86::AddrDisp));
>>> +  if (!Disp1 || !Disp2)
>>>      return false;
>>> -  // Scale should be 1, Index should be Reg0.
>>> -  if (Load1->getOperand(1) == Load2->getOperand(1) &&
>>> -      Load1->getOperand(2) == Load2->getOperand(2)) {
>>> -    if (cast<ConstantSDNode>(Load1->getOperand(1))->getZExtValue() !=
>>> 1)
>>> -      return false;
>>>
>>> -    // Now let's examine the displacements.
>>> -    if (isa<ConstantSDNode>(Load1->getOperand(3)) &&
>>> -        isa<ConstantSDNode>(Load2->getOperand(3))) {
>>> -      Offset1 = cast<ConstantSDNode>(Load1->ge
>>> tOperand(3))->getSExtValue();
>>> -      Offset2 = cast<ConstantSDNode>(Load2->ge
>>> tOperand(3))->getSExtValue();
>>> -      return true;
>>> -    }
>>> -  }
>>> -  return false;
>>> +  Offset1 = Disp1->getSExtValue();
>>> +  Offset2 = Disp2->getSExtValue();
>>> +  return true;
>>>  }
>>>
>>>  bool X86InstrInfo::shouldScheduleLoadsNear(SDNode *Load1, SDNode
>>> *Load2,
>>>
>>>
>>> --
>> ~Craig
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170411/fa9c59da/attachment.html>