[BOLT] Do not use HLT as split point when build the CFG #150963

Jianghibo · 2025-07-28T14:45:20Z

For x86, the halt instruction is defined as a terminator instruction. When building the CFG, the instruction sequence following the hlt instruction is treated as an independent MBB. Since there is no jump information, the predecessor of this MBB cannot be identified, and it is considered an unreachable MBB that will be removed.

Using this fix, the instruction sequences before and after hlt are refused to be placed in different blocks.

github-actions · 2025-07-28T14:45:40Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-07-28T14:46:15Z

@llvm/pr-subscribers-bolt

Author: Haibo Jiang (Jianghibo)

Changes

For x86, the halt instruction is defined as a terminator instruction. When building the CFG, the instruction sequence following the hlt instruction is treated as an independent MBB. Since there is no jump information, the predecessor of this MBB cannot be identified, and it is considered an unreachable MBB that will be removed.

Using this fix, the instruction sequences before and after hlt are refused to be placed in different blocks.

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

4 Files Affected:

(modified) bolt/include/bolt/Core/MCPlusBuilder.h (+3)
(modified) bolt/lib/Core/BinaryFunction.cpp (+3-2)
(modified) bolt/lib/Target/X86/X86MCPlusBuilder.cpp (+4)
(modified) bolt/test/X86/static-exe.test (+3)

diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index f902a8c43cd1d..b698ce54d6899 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -740,6 +740,9 @@ class MCPlusBuilder {
     return false;
   }
 
+  /// Return true if the hlt instruction is defind as terminator
+  virtual bool isTerminatorHLT(const MCInst &Inst) const { return false; }
+
   /// Return the width, in bytes, of the memory access performed by \p Inst, if
   /// this is a pop instruction. Return zero otherwise.
   virtual int getPopSize(const MCInst &Inst) const {
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index eec68ff5a5fce..0f904b679365f 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -2329,11 +2329,12 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       assert(PrevBB && "no previous basic block for a fall through");
       MCInst *PrevInstr = PrevBB->getLastNonPseudoInstr();
       assert(PrevInstr && "no previous instruction for a fall through");
-      if (MIB->isUnconditionalBranch(Instr) &&
+      if ((MIB->isUnconditionalBranch(Instr) &&
           !MIB->isIndirectBranch(*PrevInstr) &&
           !MIB->isUnconditionalBranch(*PrevInstr) &&
           !MIB->getConditionalTailCall(*PrevInstr) &&
-          !MIB->isReturn(*PrevInstr)) {
+          !MIB->isReturn(*PrevInstr)) ||
+          MIB->isTerminatorHLT(*PrevInstr)) {
         // Temporarily restore inserter basic block.
         InsertBB = PrevBB;
       } else {
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index a60c1a6bf156e..090d933a1e66a 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -223,6 +223,10 @@ class X86MCPlusBuilder : public MCPlusBuilder {
     return Inst.getOpcode() == X86::ENDBR32 || Inst.getOpcode() == X86::ENDBR64;
   }
 
+  bool isTerminatorHLT(const MCInst &Inst) const override {
+    return Inst.getOpcode() == X86::HLT;
+  }
+
   int getPopSize(const MCInst &Inst) const override {
     switch (Inst.getOpcode()) {
     case X86::POP16r:
diff --git a/bolt/test/X86/static-exe.test b/bolt/test/X86/static-exe.test
index e288160da1521..5d9b389d6ccca 100644
--- a/bolt/test/X86/static-exe.test
+++ b/bolt/test/X86/static-exe.test
@@ -1,6 +1,9 @@
 ## Check that llvm-bolt can rewrite static executable
+## And check CFG for halt instruction
 
 RUN: %clang %cflags %S/Inputs/static_exe.s -static -o %t.exe -nostdlib
 RUN: llvm-bolt %t.exe -o %t 2>&1 | FileCheck %s
+RUN: llvm-bolt %t.exe --print-cfg -o %t 2>&1 | FileCheck --check-prefix=CHECK-CFG
 
 CHECK: 1 out of 1 functions were overwritten
+CHECK-CFG: BB Count    : 1

maksfb · 2025-07-28T17:33:56Z

Thanks for the PR. Could you please provide more context? I'm guessing the fix is for the kernel code.

For the implementation, let's change how MCPlusBuilder::isTerminator() handles HLT instruction and add a dedicated test case.

Jianghibo · 2025-07-29T01:25:33Z

Thanks for the PR. Could you please provide more context? I'm guessing the fix is for the kernel code.

For the implementation, let's change how MCPlusBuilder::isTerminator() handles HLT instruction and add a dedicated test case.

@maksfb Thanks for your review.
yeah, I am trying to optimize the Linux kernel 5.10 for openEuler using llvm-bolt, and I encountered an error caused by the hlt instruction.

I checked the definition of the hlt instruction under the x86 architecture, and it is defined as a terminator.

maksfb · 2025-07-29T03:35:12Z

I checked the definition of the hlt instruction under the x86 architecture, and it is defined as a terminator.

Right. We don't want to change LLVM's definition, but for BOLT's CFG-building purposes we should treat it as non-terminator, that's why I propose to change MCPlusBuilder::isTerminator().

Jianghibo · 2025-07-29T14:42:13Z

I checked the definition of the hlt instruction under the x86 architecture, and it is defined as a terminator.

Right. We don't want to change LLVM's definition, but for BOLT's CFG-building purposes we should treat it as non-terminator, that's why I propose to change MCPlusBuilder::isTerminator().

updated the implementation.

maksfb

Thanks for addressing the feedback and adding a new test case. Please see the comments inlined.

maksfb · 2025-07-29T19:27:16Z

bolt/include/bolt/Core/MCPlusBuilder.h

Lets rename to isX86HLT() or alternatively override isTerminator() for X86MCPlusBuilder.

maksfb · 2025-07-29T19:28:06Z

bolt/test/X86/Inputs/cfg_build_hlt.s

Since we are not interested in other instructions, just leave hlt followed by retq.

maksfb · 2025-07-29T19:29:44Z

bolt/test/X86/cfg_build_hlt.test

It's more convenient to combine *.s sources with the test in the same file. Check double-rel.s for example.

Done, thanks

maksfb

Thanks. Please address a couple of minor nits and it's good to go.

maksfb · 2025-07-31T23:34:44Z

bolt/test/X86/cfg_build_hlt.s

Let's remove these lines too.

gentle ping.

maksfb · 2025-07-31T23:35:58Z

bolt/test/X86/cfg_build_hlt.s

Add --print-only=main to make sure we are looking at main().

Jianghibo · 2025-08-05T11:55:00Z

@maksfb Hi, is there any other comment?

maksfb

Thanks!

Jianghibo · 2025-08-15T03:36:07Z

Thanks!

@maksfb Hi, could you help to approve me to run these two workflows? It seems that since this is my first time submitting code, I don't have sufficient permissions to run these two workflows and merge PR.

Thanks.

github-actions · 2025-08-15T07:30:08Z

✅ With the latest revision this PR passed the C/C++ code formatter.

For x86, the halt instruction is defined as a terminator instruction. When building the CFG, the instruction sequence following the hlt instruction is treated as an independent MBB. Since there is no jump information, the predecessor of this MBB cannot be identified, and it is considered an unreachable MBB that will be removed. Using this fix, the instruction sequences before and after hlt are refused to be placed in different blocks.

Jianghibo · 2025-08-15T09:03:22Z

@maksfb could you help to merge this pr? thanks.

github-actions · 2025-08-15T21:35:31Z

@Jianghibo Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

This is a follow-up to llvm#150963. X86 HLT instruction may appear in the user-level code, in which case we should treat it as a terminator. Handle it as a non-terminator in the Linux kernel mode.

This is a follow-up to #150963. X86 HLT instruction may appear in the user-level code, in which case we should treat it as a terminator. Handle it as a non-terminator in the Linux kernel mode.

Jianghibo requested review from aaupov, ayermolo, maksfb, paschalis-mpeis, rafaelauler and yota9 as code owners July 28, 2025 14:45

llvmbot added the BOLT label Jul 28, 2025

Jianghibo force-pushed the fix-bolt-handle-hlt branch from dce028b to 536fec3 Compare July 29, 2025 14:40

Jianghibo requested a review from yozhu as a code owner July 29, 2025 14:40

maksfb reviewed Jul 29, 2025

View reviewed changes

Jianghibo force-pushed the fix-bolt-handle-hlt branch from 536fec3 to 76b0ce7 Compare July 30, 2025 12:44

maksfb reviewed Jul 31, 2025

View reviewed changes

Jianghibo force-pushed the fix-bolt-handle-hlt branch from 76b0ce7 to 86a61ab Compare August 1, 2025 14:08

Jianghibo requested a review from maksfb August 13, 2025 02:59

maksfb approved these changes Aug 14, 2025

View reviewed changes

maksfb changed the title ~~[BOLT] Not use hlt as split point when build the CFG~~ [BOLT] Do not use HLT as split point when build the CFG Aug 14, 2025

Jianghibo force-pushed the fix-bolt-handle-hlt branch from 7f54066 to 4d80efe Compare August 15, 2025 08:17

maksfb merged commit 21a5729 into llvm:main Aug 15, 2025
9 checks passed

Jianghibo deleted the fix-bolt-handle-hlt branch August 16, 2025 01:25

maksfb mentioned this pull request Aug 19, 2025

[BOLT] Keep X86 HLT instruction as a terminator in user mode #154402

Merged

[BOLT] Do not use HLT as split point when build the CFG #150963

[BOLT] Do not use HLT as split point when build the CFG #150963

Uh oh!

Conversation

Jianghibo commented Jul 28, 2025

Uh oh!

github-actions bot commented Jul 28, 2025

Uh oh!

llvmbot commented Jul 28, 2025

Uh oh!

maksfb commented Jul 28, 2025

Uh oh!

Jianghibo commented Jul 29, 2025

Uh oh!

maksfb commented Jul 29, 2025

Uh oh!

Jianghibo commented Jul 29, 2025

Uh oh!

maksfb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maksfb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jianghibo commented Aug 5, 2025

Uh oh!

maksfb left a comment

Choose a reason for hiding this comment

Uh oh!

Jianghibo commented Aug 15, 2025

Uh oh!

github-actions bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jianghibo commented Aug 15, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Aug 15, 2025 •

edited

Loading