search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.98k stars 11.95k forks source link

LLVM PowerPC backend assert - exposed by swift bootstrap #40522

Closed llvmbot closed 5 years ago

llvmbot commented 5 years ago
Bugzilla Link 41177
Resolution FIXED
Resolved on Apr 18, 2019 00:59
Version trunk
OS Linux
Attachments Issues at step “— Installing swiftpm —” while building Apple Swift 5 toolchain on PowerPC64LE
Reporter LLVM Bugzilla Contributor
CC @dwblaikie,@hfinkel,@nemanjai

Extended Description

Facing the following error at step "--- Installing swiftpm ---" while building Apple Swift 5 toolchain on PowerPC64LE using the master branch(detailed log attached):-

--- Installing swiftpm ---

llvmbot commented 5 years ago

https://reviews.llvm.org/D60811 Closed by commit rL358644: [PowerPC] Fix wrong ElemSIze when calling isConsecutiveLS() (Link:- https://reviews.llvm.org/rL358644)

llvmbot commented 5 years ago

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

llvmbot commented 5 years ago

Hi @​nemanjai,

I saw that this part of the code was introduced by you in commit - https://github.com/apple/swift-llvm/commit/502534bc2a23e0d8eab19ee77d7648dd65bf0667#diff-e9b0770875cf87fad4925f505fe401ae (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?

llvmbot commented 5 years ago

Narrow down the error case I have narrowed down the error case. One can use llc simplify.ll to reproduce the error.

llvmbot commented 5 years ago

Any updates on this one? Sorry for pushing this, but just wanted to mention that this is a blocker for our efforts on building Swift toolchain on ppc64le. Would highly appreciate an early resolution. Thanks in advance!

llvmbot commented 5 years ago

Recieved the below response in mail from one of the developers, please confirm.

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

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

hfinkel commented 5 years ago

This is a swift-related issue, report on their bugtrack.

This looks like an LLVM backend bug and, thus, should be tracked here.

llvmbot commented 5 years ago

Attaching the IR output of the failing command at https://bugs.swift.org/browse/SR-10176.

The failing command mentions this particular function literal in its stack dump:-

Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on function '@"$s22LanguageServerProtocol13HoverResponseV8contents5rangeAcA13MarkupContentV_SnyAA8PositionVGSgtcfC"' I could find "s22LanguageServerProtocol13HoverResponseV8contents5rangeAcA13MarkupContentV_SnyAA8PositionVGSgtcfC" function literal definition in the IR dump, however could not make much out of it.

Any help or any person you could point out to resolve this?

Please note that currently this is the only/final error we are facing now in our aim of building Apple Swift 5 toolchain on PPC64LE. Rest all issues have been resolved.

Just FYI, the toolchain builds to completion (thus confirming of no further failures) if we comment out the assert function which throws up this error - i.e. 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."'

Please suggest.

llvmbot commented 5 years ago

Please ignore the previous comment. Looking at the assert, while trying a few things initially, I think I misread the assert in this case , the assert will hit only if the first condition is false which means both variables 'InputsAreConsecutiveLoads' and 'InputsAreReverseConsecutive' are true. :(

llvmbot commented 5 years ago

Investigated more on this and found that the following assert statement which throws up this error might be wrongly computed:- assert(!(InputsAreConsecutiveLoads && InputsAreReverseConsecutive) && "The loads cannot be both consecutive and reverse consecutive.");

Both variables 'InputsAreConsecutiveLoads' and 'InputsAreReverseConsecutive' are computed based on the return value from 'isConsecutiveLS' function. The 'isConsecutiveLS' function definition mentions in comments that it is "Like SelectionDAG::isConsecutiveLoad", which "Return true if LD is loading 'Bytes' bytes from a location that is 'Dist' units away from the location that the 'Base' load is loading from."

Based on this, both variables 'InputsAreConsecutiveLoads' and 'InputsAreReverseConsecutive' which are initialised to 'true', are set to 'false' if the return value of 'isConsecutiveLS' function is false i.e. if the loads are not consecutive or reverse consecutive respectively.

Further we do "Exit early if the loads are neither consecutive nor reverse consecutive.":- if (!InputsAreConsecutiveLoads && !InputsAreReverseConsecutive) return SDValue();

To summarise:- InputsAreConsecutiveLoads == true => Loads are consecutive InputsAreReverseConsecutive == true => Loads are reverse consecutive

InputsAreConsecutiveLoads == false => Loads are NOT consecutive InputsAreReverseConsecutive == false => Loads are NOT reverse consecutive

Based on this, the assert statement in question should have been the following:- assert((InputsAreConsecutiveLoads && InputsAreReverseConsecutive) && "The loads cannot be both consecutive and reverse consecutive.");

Please correct if I am missing anything.

llvmbot commented 5 years ago

Can some one please have a look and comment on this? This is blocking building Apple Swift 5 toolchain on PPC64LE.

llvmbot commented 5 years ago

Hi Davide,

I have already raised this on swift bugtracker:- https://bugs.swift.org/browse/SR-10059

I got the response from community members - "Looks like an LLVM bug to me."

Please suggest. Would appreciate a prompt response.

llvmbot commented 5 years ago

This is a swift-related issue, report on their bugtrack.