joaomoreira created this revision.
joaomoreira added reviewers: craig.topper, chandlerc, erichkeane, oren_ben_simhon, xiangzhangllvm, pengfei, hjl.tools, LuoYuanke, vzakhari, annita.zhang, smaslov, espindola, ruiu, grimar, wxiao3, peter.smith.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
joaomoreira requested review of this revision.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84862
Files:
llvm/lib/Target/X86/X86InstrInfo.cpp
llvm/lib/Target/X86/X86InstrInfo.h
Index: llvm/lib/Target/X86/X86InstrInfo.h
===================================================================
--- llvm/lib/Target/X86/X86InstrInfo.h
+++ llvm/lib/Target/X86/X86InstrInfo.h
@@ -409,6 +409,13 @@
bool areLoadsFromSameBasePtr(SDNode *Load1, SDNode *Load2, int64_t &Offset1,
int64_t &Offset2) const override;
+ /// isSchedulingBoundary - Overrides the isSchedulingBoundary from
+ /// Codegen/TargetInstrInfo.cpp to make it capable of identifying ENDBR
+ /// intructions and prevent it from being re-scheduled.
+ bool isSchedulingBoundary(const MachineInstr &MI,
+ const MachineBasicBlock *MBB,
+ const MachineFunction &MF) const override;
+
/// shouldScheduleLoadsNear - This is a used by the pre-regalloc scheduler to
/// determine (in conjunction with areLoadsFromSameBasePtr) if two loads
/// should be scheduled togther. On some targets if two loads are loading from
Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===================================================================
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -6675,6 +6675,20 @@
return true;
}
+bool X86InstrInfo::isSchedulingBoundary(const MachineInstr &MI,
+ const MachineBasicBlock *MBB,
+ const MachineFunction &MF) const {
+ if (TargetInstrInfo::isSchedulingBoundary(MI, MBB, MF))
+ return true;
+
+ // ENDBR instructions should not be scheduled around.
+ unsigned opcode = MI.getOpcode();
+ if (opcode == X86::ENDBR64 || opcode == X86::ENDBR32)
+ return true;
+
+ return false;
+}
+
bool X86InstrInfo::
reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const {
assert(Cond.size() == 1 && "Invalid X86 branch condition!");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84862.281617.patch
Type: text/x-patch
Size: 1864 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200729/a916c00f/attachment-0001.bin>