Page MenuHomePhabricator

[MachinePipeliner] Fix order for nodes with Anti dependence in same cycle
ClosedPublic

Authored by jsji on Jul 3 2019, 9:01 PM.

Details

Summary

Problem exposed in PowerPC functional testing.

We did not consider Anti dependence for nodes in same cycle,
so we may end up generating bad machine code.
eg: the reduced test won't verify.

    • Bad machine code: Using an undefined physical register ***
  • function: lame_encode_buffer_interleaved
  • basic block: %bb.4 (0x4bde4e12928)
  • instruction: %29:gprc = ADDZE %27:gprc, implicit-def dead $carry,

implicit $carry

  • operand 3: implicit $carry

Diff Detail

Repository
rL LLVM

Event Timeline

jsji created this revision.Jul 3 2019, 9:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2019, 9:01 PM
jsji retitled this revision from [MachinePipeliner][PowerPC] Add a testcase for order in scheduled group. to [MachinePipeliner] Fix order for nodes with Anti dependence in same cycle.Jul 3 2019, 9:03 PM
jsji edited the summary of this revision. (Show Details)
jsji added reviewers: bcahoon, kparzysz, hfinkel.
jsji updated this revision to Diff 207967.Jul 3 2019, 9:07 PM
jsji edited the summary of this revision. (Show Details)

Remove "require asserts" in testcase.

hfinkel added inline comments.Jul 11 2019, 1:44 PM
llvm/lib/CodeGen/MachinePipeliner.cpp
3563 ↗(On Diff #207967)
= 0

(insert space)

3564 ↗(On Diff #207967)

End comments with a period.

3567 ↗(On Diff #207967)

space after if and space around ||

jsji updated this revision to Diff 209329.Jul 11 2019, 2:10 PM

Thanks Hal. Fix comments & run clang-format on it.

hfinkel accepted this revision.Jul 11 2019, 3:11 PM

LGTM.

This revision is now accepted and ready to land.Jul 11 2019, 3:11 PM
This revision was automatically updated to reflect the committed changes.