[llvm] [X86] Resolve FIXME: Create cld only when needed (PR #82415)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 21 09:15:01 PST 2024


================
@@ -2194,13 +2194,35 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF,
   // flag (DF in EFLAGS register). Clear this flag by creating "cld" instruction
   // in each prologue of interrupt handler function.
   //
-  // FIXME: Create "cld" instruction only in these cases:
+  // Create "cld" instruction only in these cases:
   // 1. The interrupt handling function uses any of the "rep" instructions.
   // 2. Interrupt handling function calls another function.
   //
-  if (Fn.getCallingConv() == CallingConv::X86_INTR)
-    BuildMI(MBB, MBBI, DL, TII.get(X86::CLD))
-        .setMIFlag(MachineInstr::FrameSetup);
+  if (Fn.getCallingConv() == CallingConv::X86_INTR) {
+    bool NeedsCLD = false;
+
+    // Check if the function calls another function.
+    for (const BasicBlock &BB : Fn) {
+      for (const Instruction &I : BB) {
+        if (isa<CallInst>(I)) {
+          NeedsCLD = true;
+          break;
+        }
+
+        // Check for rep opcode.
+        unsigned Opcode = I.getOpcode();
+        if (X86::REP_MOVSB_32 <= Opcode && Opcode <= X86::REP_STOSW_64) {
----------------
topperc wrote:

If someone adds an instruction in the future like `REP_NEW`, it will end up in the middle of this range. I want this code to be audited if that happens. So please add a static_assert that we currently expect 16 instructions between REP_MOVSB_32 and REP_STOSW_64. That way the static_assert will fail if a new instruction is added the breaks the range. The person making the change will see the error and update the code if the new instruction should not or should not be included.

https://github.com/llvm/llvm-project/pull/82415


More information about the llvm-commits mailing list