[BOLT] Skip optimization of functions with alt instructions

[maksfb]

Alternative instructions in the Linux kernel may modify control flow in a function. As such, it is unsafe to optimize functions with alternative instructions until we properly support CFG alternatives.

Previously, we marked functions with alt instructions before the emission, but that could be too late if we remove or replace instructions with alternatives. We could have marked functions as non-simple immediately after reading .altinstructions, but it's nice to be able to view functions after CFG is built. Thus assign the non-simple status after building CFG.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Maksim Panchenko (maksfb)

Changes

Alternative instructions in the Linux kernel may modify control flow in a function. As such, it is unsafe to optimize functions with alternative instructions until we properly support CFG alternatives.

Previously, we marked functions with alt instructions before the emission, but that could be too late if we remove or replace instructions with alternatives. We could have marked functions as non-simple immediately after reading .altinstructions, but it's nice to be able to view functions after CFG is built. Thus assign the non-simple status after building CFG.

---

Full diff: https://github.com/llvm/llvm-project/pull/95172.diff

2 Files Affected:

• (modified) bolt/lib/Rewrite/LinuxKernelRewriter.cpp (+7-9)
• (modified) bolt/test/X86/linux-alt-instruction.s (+6-6)

diff --git a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
index 6b3f5bce9f0f5..7849e6b244f63 100644
--- a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
+++ b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
@@ -273,9 +273,9 @@ class LinuxKernelRewriter final : public MetadataRewriter {
 
   /// Handle alternative instruction info from .altinstructions.
   Error readAltInstructions();
+  void processAltInstructionsPostCFG();
   Error tryReadAltInstructions(uint32_t AltInstFeatureSize,
                                bool AltInstHasPadLen, bool ParseOnly);
-  Error rewriteAltInstructions();
 
   /// Read .pci_fixup
   Error readPCIFixupTable();
@@ -326,6 +326,8 @@ class LinuxKernelRewriter final : public MetadataRewriter {
     if (Error E = processORCPostCFG())
       return E;
 
+    processAltInstructionsPostCFG();
+
     return Error::success();
   }
 
@@ -335,9 +337,6 @@ class LinuxKernelRewriter final : public MetadataRewriter {
     if (Error E = rewriteExceptionTable())
       return E;
 
-    if (Error E = rewriteAltInstructions())
-      return E;
-
     if (Error E = rewriteParaInstructions())
       return E;
 
@@ -1485,12 +1484,11 @@ Error LinuxKernelRewriter::tryReadAltInstructions(uint32_t AltInstFeatureSize,
   return Error::success();
 }
 
-Error LinuxKernelRewriter::rewriteAltInstructions() {
-  // Disable output of functions with alt instructions before the rewrite
-  // support is complete.
+void LinuxKernelRewriter::processAltInstructionsPostCFG() {
+  // Disable optimization and output of functions with alt instructions before
+  // the rewrite support is complete. Alt instructions can modify the control
+  // flow, hence we may end up deleting seemingly unreachable code.
   skipFunctionsWithAnnotation("AltInst");
-
-  return Error::success();
 }
 
 /// When the Linux kernel needs to handle an error associated with a given PCI
diff --git a/bolt/test/X86/linux-alt-instruction.s b/bolt/test/X86/linux-alt-instruction.s
index 66cd33a711b89..dc1b12a277785 100644
--- a/bolt/test/X86/linux-alt-instruction.s
+++ b/bolt/test/X86/linux-alt-instruction.s
@@ -6,7 +6,7 @@
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
 # RUN: %clang %cflags -nostdlib %t.o -o %t.exe \
 # RUN:   -Wl,--image-base=0xffffffff80000000,--no-dynamic-linker,--no-eh-frame-hdr,--no-pie
-# RUN: llvm-bolt %t.exe --print-normalized --alt-inst-feature-size=2 -o %t.out \
+# RUN: llvm-bolt %t.exe --print-cfg --alt-inst-feature-size=2 -o %t.out \
 # RUN:   | FileCheck %s
 
 ## Older kernels used to have padlen field in alt_instr. Check compatibility.
@@ -15,7 +15,7 @@
 # RUN:   %s -o %t.padlen.o
 # RUN: %clang %cflags -nostdlib %t.padlen.o -o %t.padlen.exe \
 # RUN:   -Wl,--image-base=0xffffffff80000000,--no-dynamic-linker,--no-eh-frame-hdr,--no-pie
-# RUN: llvm-bolt %t.padlen.exe --print-normalized --alt-inst-has-padlen -o %t.padlen.out \
+# RUN: llvm-bolt %t.padlen.exe --print-cfg --alt-inst-has-padlen -o %t.padlen.out \
 # RUN:   | FileCheck %s
 
 ## Check with a larger size of "feature" field in alt_instr.
@@ -24,7 +24,7 @@
 # RUN:   --defsym FEATURE_SIZE_4=1 %s -o %t.fs4.o
 # RUN: %clang %cflags -nostdlib %t.fs4.o -o %t.fs4.exe \
 # RUN:   -Wl,--image-base=0xffffffff80000000,--no-dynamic-linker,--no-eh-frame-hdr,--no-pie
-# RUN: llvm-bolt %t.fs4.exe --print-normalized --alt-inst-feature-size=4 -o %t.fs4.out \
+# RUN: llvm-bolt %t.fs4.exe --print-cfg --alt-inst-feature-size=4 -o %t.fs4.out \
 # RUN:   | FileCheck %s
 
 ## Check that out-of-bounds read is handled properly.
@@ -33,9 +33,9 @@
 
 ## Check that BOLT automatically detects structure fields in .altinstructions.
 
-# RUN: llvm-bolt %t.exe --print-normalized -o %t.out | FileCheck %s
-# RUN: llvm-bolt %t.exe --print-normalized -o %t.padlen.out | FileCheck %s
-# RUN: llvm-bolt %t.exe --print-normalized -o %t.fs4.out | FileCheck %s
+# RUN: llvm-bolt %t.exe --print-cfg -o %t.out | FileCheck %s
+# RUN: llvm-bolt %t.exe --print-cfg -o %t.padlen.out | FileCheck %s
+# RUN: llvm-bolt %t.exe --print-cfg -o %t.fs4.out | FileCheck %s
 
 # CHECK:      BOLT-INFO: Linux kernel binary detected
 # CHECK:      BOLT-INFO: parsed 2 alternative instruction entries

[maksfb]

Ping.

[dcci]

Sorry, this one got stuck in my inbox. LG.