Page MenuHomePhabricator

[X86] Make ENDBR instruction a scheduling boundary
ClosedPublic

Authored by joaomoreira on Wed, Jul 29, 9:04 AM.

Details

Summary

Instructions should not be scheduled across ENDBR instructions, as this would result in the ENDBR being displaced, breaking the parity needed for the Indirect Branch Tracking feature of CET.

Currently, the X86IndirectBranchTracking pass is later than the instruction scheduling in the pipeline, what causes the bug to be unnoticeable and very hard (if not unfeasible) to be triggered while compiling C files with the standard LLVM setup. Yet, for correctness and to prevent issues in future changes, the compiler should prevent the such scheduling.

Diff Detail

Event Timeline

joaomoreira created this revision.Wed, Jul 29, 9:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jul 29, 9:04 AM
joaomoreira requested review of this revision.Wed, Jul 29, 9:04 AM

Tried to use the inline code editor - not sure if it worked that well.....

llvm/lib/Target/X86/X86InstrInfo.cpp
6681

Could this be simplified to use the base method as the fallback?

Fixes:

  • Applied suggestions made by @RKSimon,
  • Fixed "opcode" variable name capitalization.
lebedev.ri retitled this revision from Make ENDBR instruction a scheduling boundary to [X86] Make ENDBR instruction a scheduling boundary.Fri, Jul 31, 3:54 AM

SGTM - @craig.topper are you happy with this to go in without a test case?

craig.topper accepted this revision.Sat, Aug 1, 1:29 PM

LGTM. I think I'm ok with it. I suppose we could construct some synthetic MIR test case, but not sure how complex it would have to be to make the scheduler want to move the ENDBR.

This revision is now accepted and ready to land.Sat, Aug 1, 1:29 PM
This revision was automatically updated to reflect the committed changes.