Changes for LLVM PPCISelLowering function combineBVOfConsecutiveLoads

[sarveshtamba]

Summary

Seeing the following issue while building swift tool chain on ppc64le:-

swift: /home/sar/swift-source/llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12111: llvm::SDValue combineBVOfConsecutiveLoads(llvm::SDNode *, llvm::SelectionDAG &): Assertion `!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) && "The loads cannot be both consecutive and reverse consecutive."' failed.

More details in https://bugs.llvm.org/show_bug.cgi?id=41177

Making the changes based on the below suggestion which fixes the issue and the tool chain builds successfully:-

Hi, Sarvesh,
I think there is a bug in below code.

for (int i = 1, e = N->getNumOperands(); i < e; ++i) {
  // If any inputs are fp_round(extload), they all must be.
  if (IsRoundOfExtLoad && N->getOperand(i).getOpcode() != ISD::FP_ROUND)
    return SDValue();
  SDValue NextInput = IsRoundOfExtLoad ? N->getOperand(i).getOperand(0) :
    N->getOperand(i);
  if (NextInput.getOpcode() != ISD::LOAD)
    return SDValue();
  SDValue PreviousInput =
    IsRoundOfExtLoad ? N->getOperand(i-1).getOperand(0) : N->getOperand(i-1);
  LoadSDNode *LD1 = dyn_cast<LoadSDNode>(PreviousInput);
  LoadSDNode *LD2 = dyn_cast<LoadSDNode>(NextInput);
  // If any inputs are fp_round(extload), they all must be.
  if (IsRoundOfExtLoad && LD2->getExtensionType() != ISD::EXTLOAD)
    return SDValue();
  if (!isConsecutiveLS(LD2, LD1, ElemSize, 1, DAG))
    InputsAreConsecutiveLoads = false;
  if (!isConsecutiveLS(LD1, LD2, ElemSize, 1, DAG))
    InputsAreReverseConsecutive = false;
  // Exit early if the loads are neither consecutive nor reverse consecutive.
  if (!InputsAreConsecutiveLoads && !InputsAreReverseConsecutive)
    return SDValue();
}
assert(!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
       "The loads cannot be both consecutive and reverse consecutive.");

For below if statement, if InputsAreConsecutiveLoads = 0 and InputsAreReverseConsecutive = 0, it will return SDValue().
if (!InputsAreConsecutiveLoads && !InputsAreReverseConsecutive)

return SDValue();

For below 3 situations, if-statement will not return SDValue():
InputsAreConsecutiveLoads = 0 and InputsAreReverseConsecutive = 1,
InputsAreConsecutiveLoads = 1 and InputsAreReverseConsecutive = 0,
InputsAreConsecutiveLoads = 1 and InputsAreReverseConsecutive = 1,

For below assert-statement:
assert(!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
"The loads cannot be both consecutive and reverse consecutive.");

If InputsAreConsecutiveLoads = 1 and InputsAreReverseConsecutive = 1,
this assert condtion '!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive)' will get false.

So I think the right assert statement should be:
assert((InputsAreConsecutiveLoads || InputsAreReverseConsecutive) &&
"The loads cannot be both consecutive and reverse consecutive.");

Thanks,
Zhang Kang (张抗)
XL Compiler Backend(TOBEY) developer, CDL Shanghai

Email: [e-mail] Phone: [phone]

关注IBM中国编译器开发团队 - 新浪微博: http://www.weibo.com/ibmcompiler | developerWorks: http://ibm.co/HK0GCx

[hfinkel]

lib/Target/PowerPC/PPCISelLowering.cpp

12110

You put || in the description, but this still says &&. Did you mean ||?

12111

This text needs to be updated too?

[jsji]

@ZhangKang Looks like the patch is suggested by you? Do you have any testcases that can validate this?
And also are you able to reproduce (and narrow down) the problem before suggesting the patch?

[ZhangKang]

@jsji , I will talk with @sarveshtamba to check and add the test case.

[sarveshtamba]

Hi @nemanjai,
I saw that this part of the code was introduced by you in commit - https://reviews.llvm.org/D26023
Can you please tell what this piece of code is doing and if at all the assert statement needs to be revisited?

@ZhangKang and me are trying to fix this issue on ppc64le.

Thanks in advance!

[ZhangKang]

This path has been abandoned, the new patch for this bug is in https://reviews.llvm.org/D60811.

[sarveshtamba]

The new patch for this bug is in https://reviews.llvm.org/D60811.

lib/Target/PowerPC/PPCISelLowering.cpp

for (int i = 1, e = N->getNumOperands(); i < e; ++i) {
     if (!isConsecutiveLS(LD1, LD2, ElemSize, 1, DAG))
       InputsAreReverseConsecutive = false;
 
     // Exit early if the loads are neither consecutive nor reverse consecutive.
     if (!InputsAreConsecutiveLoads && !InputsAreReverseConsecutive)
       return SDValue();
   }
 
-  assert(!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
+  assert((InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
          "The loads cannot be both consecutive and reverse consecutive.");
 
   SDValue FirstLoadOp =
     IsRoundOfExtLoad ? FirstInput.getOperand(0) : FirstInput;
   SDValue LastLoadOp =
     IsRoundOfExtLoad ? N->getOperand(N->getNumOperands()-1).getOperand(0) :
                        N->getOperand(N->getNumOperands()-1);
 
   LoadSDNode *LD1 = dyn_cast<LoadSDNode>(FirstLoadOp);