[PATCH] D60564: Changes for LLVM PPCISelLowering function combineBVOfConsecutiveLoads

[Sarvesh Tamba via Phabricator via llvm-commits]

sarveshtamba created this revision.
sarveshtamba added a project: LLVM.
Herald added subscribers: llvm-commits, jsji, kbarton, nemanjai.

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: shkzhang at cn.ibm.com                            Phone: 86-21-60928940
---------------------------------------------------------------------------

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


Repository:
  rL LLVM

https://reviews.llvm.org/D60564

Files:
  lib/Target/PowerPC/PPCISelLowering.cpp


Index: lib/Target/PowerPC/PPCISelLowering.cpp
===================================================================
--- lib/Target/PowerPC/PPCISelLowering.cpp
+++ lib/Target/PowerPC/PPCISelLowering.cpp
@@ -12107,7 +12107,7 @@
       return SDValue();
   }
 
-  assert(!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
+  assert((InputsAreConsecutiveLoads && InputsAreReverseConsecutive) &&
          "The loads cannot be both consecutive and reverse consecutive.");
 
   SDValue FirstLoadOp =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60564.194663.patch
Type: text/x-patch
Size: 504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190411/672ac40a/attachment.bin>