-
Notifications
You must be signed in to change notification settings - Fork 12.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Mips] Fix unable to handle inline assembly ends with compat-branch o… #77291
Conversation
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-mc Author: None (yingopq) Changes…n MIPS Modify: Fix #61045. Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not ending, so we commit pull request again. Full diff: https://github.com/llvm/llvm-project/pull/77291.diff 6 Files Affected:
diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
index 3c673ae938fdec..9725c6f9f6183a 100644
--- a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -150,6 +150,7 @@ class MipsAsmParser : public MCTargetAsmParser {
bool IsLittleEndian;
bool IsPicEnabled;
bool IsCpRestoreSet;
+ bool CurForbiddenSlotAttr;
int CpRestoreOffset;
unsigned GPReg;
unsigned CpSaveLocation;
@@ -552,6 +553,7 @@ class MipsAsmParser : public MCTargetAsmParser {
CurrentFn = nullptr;
+ CurForbiddenSlotAttr = false;
IsPicEnabled = getContext().getObjectFileInfo()->isPositionIndependent();
IsCpRestoreSet = false;
@@ -723,6 +725,16 @@ class MipsAsmParser : public MCTargetAsmParser {
return getSTI().hasFeature(Mips::FeatureGINV);
}
+ bool hasForbiddenSlot(const MCInstrDesc &MCID) const {
+ return !inMicroMipsMode() && (MCID.TSFlags & MipsII::HasForbiddenSlot);
+ }
+
+ bool SafeInForbiddenSlot(const MCInstrDesc &MCID) const {
+ return !(MCID.TSFlags & MipsII::IsCTI);
+ }
+
+ void onEndOfFile() override;
+
/// Warn if RegIndex is the same as the current AT.
void warnIfRegIndexIsAT(unsigned RegIndex, SMLoc Loc);
@@ -2307,7 +2319,42 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
bool FillDelaySlot =
MCID.hasDelaySlot() && AssemblerOptions.back()->isReorder();
- if (FillDelaySlot)
+
+ // Get previous instruction`s forbidden slot attribute and
+ // whether set reorder.
+ // This 'CurForbiddenSlotAttr' is a global variable.
+ bool PrevForbiddenSlotAttr = CurForbiddenSlotAttr;
+
+ // Flag represents we set reorder after nop.
+ bool SetReorderAfterNop = false;
+
+ // If previous instruction has forbidden slot and .set reorder
+ // is active and current instruction is CTI.
+ // Then emit a NOP after it.
+ if (PrevForbiddenSlotAttr && !SafeInForbiddenSlot(MCID)) {
+ TOut.emitEmptyDelaySlot(false, IDLoc, STI);
+ // When 'FillDelaySlot' is true, the existing logic will add
+ // noreorder before instruction and reorder after it. So there
+ // need exclude this case avoiding two '.set reorder'.
+ // The format of the first case is:
+ // .set noreorder
+ // bnezc
+ // nop
+ // .set reorder
+ if (AssemblerOptions.back()->isReorder() && !FillDelaySlot) {
+ SetReorderAfterNop = true;
+ TOut.emitDirectiveSetReorder();
+ }
+ }
+
+ // Save current instruction`s forbidden slot and whether set reorder.
+ // This is the judgment condition for whether to add nop.
+ // We would add a couple of '.set noreorder' and '.set reorder' to
+ // wrap the current instruction and the next instruction.
+ CurForbiddenSlotAttr =
+ hasForbiddenSlot(MCID) && AssemblerOptions.back()->isReorder();
+
+ if (FillDelaySlot || CurForbiddenSlotAttr)
TOut.emitDirectiveSetNoReorder();
MacroExpanderResultTy ExpandResult =
@@ -2322,6 +2369,17 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
return true;
}
+ // When current instruction was not CTI, recover reorder state.
+ // The format of the second case is:
+ // .set noreoder
+ // bnezc
+ // add
+ // .set reorder
+ if (PrevForbiddenSlotAttr && !SetReorderAfterNop && !FillDelaySlot &&
+ AssemblerOptions.back()->isReorder()) {
+ TOut.emitDirectiveSetReorder();
+ }
+
// We know we emitted an instruction on the MER_NotAMacro or MER_Success path.
// If we're in microMIPS mode then we must also set EF_MIPS_MICROMIPS.
if (inMicroMipsMode()) {
@@ -2331,6 +2389,14 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
// If this instruction has a delay slot and .set reorder is active,
// emit a NOP after it.
+ // The format of the third case is:
+ // .set noreorder
+ // bnezc
+ // nop
+ // .set noreorder
+ // j
+ // nop
+ // .set reorder
if (FillDelaySlot) {
TOut.emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc, STI);
TOut.emitDirectiveSetReorder();
@@ -2356,6 +2422,17 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
return false;
}
+void MipsAsmParser::onEndOfFile() {
+ MipsTargetStreamer &TOut = getTargetStreamer();
+ SMLoc IDLoc = SMLoc();
+ // If has pending forbidden slot, fill nop and recover reorder.
+ if (CurForbiddenSlotAttr) {
+ TOut.emitEmptyDelaySlot(false, IDLoc, STI);
+ if (AssemblerOptions.back()->isReorder())
+ TOut.emitDirectiveSetReorder();
+ }
+}
+
MipsAsmParser::MacroExpanderResultTy
MipsAsmParser::tryExpandInstruction(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out,
const MCSubtargetInfo *STI) {
diff --git a/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll b/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll
new file mode 100644
index 00000000000000..13cfe6b3126f3b
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll
@@ -0,0 +1,46 @@
+target triple = "mipsisa32r6el-unknown-linux-gnu"
+
+; RUN: llc -filetype=asm %s -o - | FileCheck %s --check-prefix=MIPSELR6
+; Function Attrs: noinline nounwind optnone uwtable
+define i1 @foo0() nounwind {
+; MIPSELR6: bnezc $1, $BB0_2
+; MIPSELR6-NEXT: nop
+; MIPSELR6: jr $ra
+entry:
+ %0 = icmp eq i32 0, 1
+ br i1 %0, label %2, label %3
+ ret i1 %0
+2:
+ ret i1 %0
+3:
+ ret i1 %0
+}
+
+define i32 @foo1() nounwind {
+; MIPSELR6: beqzc $2, $tmp0
+; MIPSELR6-NEXT: nop
+; MIPSELR6: jrc $ra
+entry:
+ %0 = tail call i32 asm "1: addiu $0, $0, 1; beqzc $0, 1b", "=r"() nounwind
+ ret i32 %0
+}
+
+define i32 @foo2() nounwind {
+; MIPSELR6: beqzc $9, End
+; MIPSELR6-NEXT: nop
+; MIPSELR6: addiu $9, $9, 1
+entry:
+ %0 = tail call i32 asm "beqzc $$t1, End", "=r"() nounwind
+ %1 = tail call i32 asm "addiu $$t1, $$t1, 1", "=r"() nounwind
+ %2 = add nsw i32 %1, %0
+ ret i32 %2
+}
+
+define i32 @foo3() nounwind {
+; MIPSELR6: beqzc $2, $tmp1
+; MIPSELR6-NEXT: nop
+; MIPSELR6: j End
+entry:
+ %0 = tail call i32 asm "1: addiu $0, $0, 1; beqzc $0, 1b; j End", "=r"() nounwind
+ ret i32 %0
+}
diff --git a/llvm/test/MC/Mips/forbidden-slot.s b/llvm/test/MC/Mips/forbidden-slot.s
new file mode 100644
index 00000000000000..161c07b9eba54d
--- /dev/null
+++ b/llvm/test/MC/Mips/forbidden-slot.s
@@ -0,0 +1,16 @@
+# RUN: llvm-mc -assemble -mcpu=mips64r6 -arch=mips64el -filetype=obj %s -o tmp.o
+# RUN: llvm-objdump -d tmp.o | FileCheck %s --check-prefix=MIPSELR6
+
+# MIPSELR6: beqzc $13, 0x0 <aaa>
+# MIPSELR6-NEXT: b 0x0 <aaa>
+# MIPSELR6: beqzc $13, 0x8 <bbb>
+# MIPSELR6-NEXT: nop <aaa>
+# MIPSELR6: b 0x8 <bbb>
+ .set noreorder
+aaa:
+ beqzc $t1, aaa
+ b aaa
+ .set reorder
+bbb:
+ beqzc $t1, bbb
+ b bbb
diff --git a/llvm/test/MC/Mips/mips32r6/relocations.s b/llvm/test/MC/Mips/mips32r6/relocations.s
index dfd75e633bc2f7..8d4464bbbed77a 100644
--- a/llvm/test/MC/Mips/mips32r6/relocations.s
+++ b/llvm/test/MC/Mips/mips32r6/relocations.s
@@ -52,17 +52,17 @@
# CHECK-ELF: Relocations [
# CHECK-ELF: 0x0 R_MIPS_PC19_S2 bar
# CHECK-ELF: 0x4 R_MIPS_PC16 bar
-# CHECK-ELF: 0x8 R_MIPS_PC16 bar
-# CHECK-ELF: 0xC R_MIPS_PC21_S2 bar
-# CHECK-ELF: 0x10 R_MIPS_PC21_S2 bar
-# CHECK-ELF: 0x14 R_MIPS_PC26_S2 bar
-# CHECK-ELF: 0x18 R_MIPS_PC26_S2 bar
-# CHECK-ELF: 0x1C R_MIPS_PCHI16 bar
-# CHECK-ELF: 0x20 R_MIPS_PCLO16 bar
-# CHECK-ELF: 0x24 R_MIPS_PC19_S2 bar
-# CHECK-ELF: 0x28 R_MIPS_PC19_S2 bar
-# CHECK-ELF: 0x2C R_MIPS_LO16 bar
-# CHECK-ELF: 0x30 R_MIPS_LO16 bar
+# CHECK-ELF: 0xC R_MIPS_PC16 bar
+# CHECK-ELF: 0x14 R_MIPS_PC21_S2 bar
+# CHECK-ELF: 0x1C R_MIPS_PC21_S2 bar
+# CHECK-ELF: 0x24 R_MIPS_PC26_S2 bar
+# CHECK-ELF: 0x28 R_MIPS_PC26_S2 bar
+# CHECK-ELF: 0x2C R_MIPS_PCHI16 bar
+# CHECK-ELF: 0x30 R_MIPS_PCLO16 bar
+# CHECK-ELF: 0x34 R_MIPS_PC19_S2 bar
+# CHECK-ELF: 0x38 R_MIPS_PC19_S2 bar
+# CHECK-ELF: 0x3C R_MIPS_LO16 bar
+# CHECK-ELF: 0x40 R_MIPS_LO16 bar
# CHECK-ELF: ]
addiupc $2,bar
diff --git a/llvm/test/MC/Mips/mips64r6/relocations.s b/llvm/test/MC/Mips/mips64r6/relocations.s
index 8353ec019a3ca8..8b02be37284aaa 100644
--- a/llvm/test/MC/Mips/mips64r6/relocations.s
+++ b/llvm/test/MC/Mips/mips64r6/relocations.s
@@ -59,19 +59,19 @@
# CHECK-ELF: Relocations [
# CHECK-ELF: 0x0 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
# CHECK-ELF: 0x4 R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0x8 R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0xC R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0x10 R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0x14 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0x18 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF: 0x1C R_MIPS_PCHI16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x20 R_MIPS_PCLO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x24 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x28 R_MIPS_PC18_S3/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x2C R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x30 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x34 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF: 0x38 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0xC R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF: 0x14 R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF: 0x1C R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF: 0x24 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF: 0x28 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF: 0x2C R_MIPS_PCHI16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x30 R_MIPS_PCLO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x34 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x38 R_MIPS_PC18_S3/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x3C R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x40 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x44 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF: 0x48 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
# CHECK-ELF: ]
addiupc $2,bar
diff --git a/llvm/test/MC/Mips/relocation.s b/llvm/test/MC/Mips/relocation.s
index 9c8bb657ea68c1..a92c62744fcaa5 100644
--- a/llvm/test/MC/Mips/relocation.s
+++ b/llvm/test/MC/Mips/relocation.s
@@ -237,7 +237,7 @@ baz: .long foo // RELOC: R_MIPS_32 foo
// ENCLE: addiu $2, $3, %tprel_lo(foo) # encoding: [A,A,0x62,0x24]
// FIXUP: # fixup A - offset: 0, value: %tprel_lo(foo), kind: fixup_Mips_TPREL_LO
-// DATA-NEXT: 00C0: D85FFFFF CBFFFFFF EC580000 EC480000
+// DATA-NEXT: 00C0: D85FFFFF 00000000 CBFFFFFF EC580000
// ?????: R_MIPS_GLOB_DAT foo
.set mips32r6
beqzc $2, foo // RELOC: R_MIPS_PC21_S2 foo
@@ -262,7 +262,7 @@ baz: .long foo // RELOC: R_MIPS_32 foo
// ENCLE: lwpc $2, foo # encoding: [A,A,0b01001AAA,0xec]
// FIXUP: # fixup A - offset: 0, value: foo, kind: fixup_MIPS_PC19_S2
-// DATA-NEXT: 00D0: 24620000 24620000 00000000
+// DATA-NEXT: 00D0: EC480000 24620000 24620000 00000000
addiu $2, $3, %pcrel_hi(foo) // RELOC: R_MIPS_PCHI16 foo
// ENCBE: addiu $2, $3, %pcrel_hi(foo) # encoding: [0x24,0x62,A,A]
// ENCLE: addiu $2, $3, %pcrel_hi(foo) # encoding: [A,A,0x62,0x24]
|
@topperc Could you help review this patch? Thanks. |
@topperc Hello, could you help review this patch at your convenience? Thanks. |
@MaskRay |
What assembly does GCC emit, and how does GNU as deal with that assembly? That is, how do those two tools interact in the GNU world when dealing with forbidden slots? |
All results were same: add nop after beqzc.
and
|
1 similar comment
All results were same: add nop after beqzc.
and
|
@wzssyqa |
…n MIPS Modify: Add a new class variable 'CurForbiddenSlotAttr' to save current instruction`s forbidden slot and whether set reorder. This is the judgment condition for whether to add nop. We would add a couple of '.set noreorder' and '.set reorder' to wrap the current instruction and the next instruction. Then we can get previous instruction`s forbidden slot attribute and whether set reorder by 'CurForbiddenSlotAttr'. If previous instruction has forbidden slot and .set reorder is active and current instruction is CTI. Then emit a NOP after it. Fix llvm#61045. Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not ending, so we commit pull request again.
9f46c06
to
0489df8
Compare
llvm#77291) …n MIPS Modify: Add a global variable 'CurForbiddenSlotAttr' to save current instruction's forbidden slot and whether set reorder. This is the judgment condition for whether to add nop. We would add a couple of '.set noreorder' and '.set reorder' to wrap the current instruction and the next instruction. Then we can get previous instruction`s forbidden slot attribute and whether set reorder by 'CurForbiddenSlotAttr'. If previous instruction has forbidden slot and .set reorder is active and current instruction is CTI. Then emit a NOP after it. Fix llvm#61045. Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not ending, so we commit pull request again. (cherry picked from commit 96abee5)
llvm#77291) …n MIPS Modify: Add a global variable 'CurForbiddenSlotAttr' to save current instruction's forbidden slot and whether set reorder. This is the judgment condition for whether to add nop. We would add a couple of '.set noreorder' and '.set reorder' to wrap the current instruction and the next instruction. Then we can get previous instruction`s forbidden slot attribute and whether set reorder by 'CurForbiddenSlotAttr'. If previous instruction has forbidden slot and .set reorder is active and current instruction is CTI. Then emit a NOP after it. Fix llvm#61045. Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not ending, so we commit pull request again. (cherry picked from commit 96abee5)
…n MIPS
Modify:
Add a global variable 'CurForbiddenSlotAttr' to save current instruction's forbidden slot and whether set reorder. This is the judgment condition for whether to add nop. We would add a couple of '.set noreorder' and '.set reorder' to wrap the current instruction and the next instruction.
Then we can get previous instruction`s forbidden slot attribute and whether set reorder by 'CurForbiddenSlotAttr'.
If previous instruction has forbidden slot and .set reorder is active and current instruction is CTI. Then emit a NOP after it.
Fix #61045.
Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not ending, so we commit pull request again.