This is an archive of the discontinued LLVM Phabricator instance.

[X86] Avoid SFB - Fix inconsistent codegen with/without debug info(2)
ClosedPublic

Authored by cdawson on May 24 2019, 9:55 AM.

Details

Summary

The function findPotentialBlockers may consider debug info instructions as potential blockers
and may stop searching for a store-load pair prematurely.

This patch corrects this and tests the cases where the store is separated from the
load by more than InspectionLimit debug instructions.

See D61680 for a previous simmilar fix in this area.

Diff Detail

Repository
rL LLVM

Event Timeline

cdawson created this revision.May 24 2019, 9:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2019, 9:55 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
aprantl added inline comments.May 24 2019, 11:09 AM
llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
345 ↗(On Diff #201277)

Do you perhapswant to use isMetaInstruction() here? There are other non-debug instructions (CFI for example) that you might want to ignore.

cdawson marked an inline comment as done.May 28 2019, 7:04 AM
cdawson added inline comments.
llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
345 ↗(On Diff #201277)

This seems like a good idea, though it would be inconsistent with D61680 where skipDebugInstructionsBackward() is used. I think there's a benefit to checking for meta instrs in both these cases, but as far as I can see there isn't an equivalent skipMetaInstructionsBackward(). Do you think it would be worth adding skipMetaInstructionsBackward/Forward() and amending D61680? Or should I follow your proposed change only in this case.

cdawson marked an inline comment as not done.May 28 2019, 7:04 AM

Do you think it would be worth adding skipMetaInstructionsBackward/Forward() and amending D61680?

Yes, that sounds like a good idea!

cdawson updated this revision to Diff 201858.May 29 2019, 4:13 AM

Addressed review comments

cdawson marked an inline comment as done.May 29 2019, 4:14 AM
cdawson updated this revision to Diff 201906.May 29 2019, 7:07 AM

Updated tests to include debug and meta instructions.

cdawson updated this revision to Diff 201912.May 29 2019, 7:35 AM

Reduce test cases. Since the debug/nodebug consistency check occurs in avoid-sfb-g-no-change.mir, we can just check that debug/meta instructions are skipped in the debug case.

aprantl accepted this revision.May 29 2019, 4:14 PM
This revision is now accepted and ready to land.May 29 2019, 4:14 PM
This revision was automatically updated to reflect the committed changes.