Page MenuHomePhabricator

[CSInfo][MIPS] Update CSInfo during MipsDelaySlotFiller
ClosedPublic

Authored by ntesic on Jan 14 2021, 6:26 AM.

Details

Summary

In MipsDelaySlotFiller, when replacing old call-branch with the compact branch
instruction, an assertion is caused by erasing the old call with unhandled CSInfo.
The problem was reported in Bug 48695.
This patch fixes it, by moving call site info from the old call instruction to its replace.

Diff Detail

Event Timeline

ntesic created this revision.Jan 14 2021, 6:26 AM
ntesic requested review of this revision.Jan 14 2021, 6:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2021, 6:26 AM

Patch fixes the reported bug; added test cases produces a crash before patch is applied. LGTM with minor nit.

llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
568–572

Do you want to use

ToErase->eraseFromParent();

here?

atanasyan accepted this revision.Jan 14 2021, 10:43 AM
atanasyan added a reviewer: atanasyan.

LGTM

This revision is now accepted and ready to land.Jan 14 2021, 10:44 AM

This fixes 32r6{,el}_defconfig and does not regress any of my other MIPS kernel builds.

some comments inline, lgtm otherwise

llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
568–572

+1

570

Should we use the shouldUpdateCallSiteInfo() predicate instead?

llvm/test/DebugInfo/MIR/Mips/call-site-info-update-delay-slot-filler.mir
34

Do we need all these attributes?

ntesic updated this revision to Diff 317317.Jan 18 2021, 3:15 AM

Addressed comments:

  • Use shouldUpdateCallSiteInfo() predicate
  • Use ToErase->eraseFromParent()
  • Reduce function attributes in test

@nickdesaulniers @djtodoro I agree with your suggestions.
@atanasyan @nathanchance Thank you all for your comments!

ntesic marked 4 inline comments as done.Jan 18 2021, 3:16 AM
djtodoro accepted this revision.Jan 18 2021, 3:19 AM

@ntesic Do you have commit access?

@ntesic Do you have commit access?

My colleague will commit this. Thanks!

This revision was automatically updated to reflect the committed changes.