MC: Fix NOP insertion between fused instructions that breaks macro fusion #155784
+29
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-x86 Author: Aleksandr Popov (aleks-tmb) ChangesFixes #155045 Full diff: https://github.com/llvm/llvm-project/pull/155784.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index 56a4cc3d65c2e..865fc0ce8101b 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -485,7 +485,16 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
if (!CanPadInst)
return;
- if (PendingBA && PendingBA->getNext() == OS.getCurrentFragment()) {
+ if (PendingBA) {
+ auto *NextFragment = PendingBA->getNext();
+ assert(NextFragment && "NextFragment should not be null");
+ if (NextFragment == OS.getCurrentFragment())
+ return;
+ // We eagerly create an empty fragment when inserting a fragment
+ // with a variable-size tail.
+ if (NextFragment->getNext() == OS.getCurrentFragment())
+ return;
+
// Macro fusion actually happens and there is no other fragment inserted
// after the previous instruction.
//
diff --git a/llvm/test/MC/X86/align-branch-fused.s b/llvm/test/MC/X86/align-branch-fused.s
index 7530967a890c6..5b4a144e30921 100644
--- a/llvm/test/MC/X86/align-branch-fused.s
+++ b/llvm/test/MC/X86/align-branch-fused.s
@@ -1,7 +1,10 @@
-# RUN: llvm-mc -filetype=obj -triple x86_64 --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc %s | llvm-objdump -d --no-show-raw-insn - | FileCheck %s
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple x86_64 --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc foo.s | llvm-objdump -d --no-show-raw-insn - | FileCheck foo.s
+# RUN: llvm-mc -filetype=obj -triple x86_64 --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc -x86-pad-max-prefix-size=1 bar.s | llvm-objdump -d --no-show-raw-insn - | FileCheck bar.s
# Exercise cases where fused instructions need to be aligned.
+#--- foo.s
.text
.globl foo
foo:
@@ -40,3 +43,18 @@ foo:
cmp %rax, %rbp
jne foo
int3
+
+# Exercise the case where fused instructions need to be aligned,
+# ensuring fusion is not broken by a NOP
+
+#--- bar.s
+ .text
+ .globl bar
+bar:
+ .nops 27
+# CHECK: 20: testq %rcx, %rcx
+# CHECK: 23: je
+ testq %rcx, %rcx
+ je .EXIT
+.EXIT:
+ ret
|
|
Reland reviewed PR #155316 |
…s macro fusion Fixes llvm#155045
5e62070 to
516b347
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the 39c8cfb patch, getOrCreateDataFragment was optimized by eagerly allocating an empty fragment when adding a fragment with a variable-size tail. This means that in this case the current MC fragment is no longer the one where the instruction was inserted, and the check
PendingBA && PendingBA->getNext() == OS.getCurrentFragment()fails, since CurrentFragment is now the empty fragment instead of the fragment containing the instruction.PendingBA -> Fragment with a variable-size tail (contains previous instruction) -> CurrentFragment (newly allocated empty fragment)This breaks the macro-fusion logic because it incorrectly assumes another fragment has been inserted between the fused instructions.
Fixes #155045
#155316 Reland