[PATCH] D65506: [MachineScheduler] improve reuse of 'releaseNode'method

Lorenzo Casalino via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 15:07:29 PDT 2019


DoktorC added a comment.

In D65506#1639640 <https://reviews.llvm.org/D65506#1639640>, @fhahn wrote:

> This change causes a crash, because MinReadyCycle is not properly initialised when going through `pickOnlyChoice`. I think we should address that separately and I'll commit the version with setting MinReadyCycle at the original position.


I beg your pardon, I ran only the unit-tests and forgot the regression ones.
Yes, I agree with you.

In D65506#1639686 <https://reviews.llvm.org/D65506#1639686>, @fhahn wrote:

> Even with moving setting the MinReadyCycle back, it fails the following tests. Could you take a look?
>
> Failing Tests (3):
>
>   LLVM :: CodeGen/PowerPC/build-vector-tests.ll
>   LLVM :: CodeGen/PowerPC/vec_conv_fp32_to_i64_elts.ll
>   LLVM :: CodeGen/PowerPC/vec_conv_i16_to_fp64_elts.ll


Checked. By my side, regression tests (I didn't forget them this time) and unit tests are all passed.

Stats running `check-llvm`:

> Expected Passes: 22977
>  Expected Failures: 115
>  Unsupported Tests: 10030

Stats running `check-llvm-unit`:

> Expected Passes: 4037

LLVM repository cloned from official github repo.
Patch applied on master branch, last revision: llvm-svn: 369577

Maybe, this is a really silly question: are you sure to have moved MinReadyCycle setting in the previous
location? (in `SchedBoundary::releaseNode()`, just after `MaxObservedStall`)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65506/new/

https://reviews.llvm.org/D65506





More information about the llvm-commits mailing list