Skip to content
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

Unable to handle inline assembly ends with compat-branch on MIPS #61045

Closed
FlyGoat opened this issue Feb 28, 2023 · 3 comments · Fixed by #77291
Closed

Unable to handle inline assembly ends with compat-branch on MIPS #61045

FlyGoat opened this issue Feb 28, 2023 · 3 comments · Fixed by #77291

Comments

@FlyGoat
Copy link

FlyGoat commented Feb 28, 2023

Currently LLVM unable to handle the situation that a inline Assembly ends with branch instruction and then compiler generate another CTI immediately.

It would leave compiler generated CTI in forbidden slot and crash at runtime.

Test case:

define i32 @foo0() nounwind {
entry:
  %0 = tail call i32 asm "1: addiu $0, $$0, 1; beqzc $0, 1b;", "=r"() nounwind
  ret i32 %0
}

Generate object file with:

llc -mtriple=mips64el-linux-gnuabi64 -mcpu=mips64r6 -filetype=obj ./broken-forbidden-slot.ll

Then dissemble with:

$ mips64el-linux-gnuabi64-objdump -d ./broken-forbidden-slot.o
./broken-forbidden-slot.o:     file format elf64-tradlittlemips


Disassembly of section .text:

0000000000000000 <foo0>:
   0:   24020001        li      v0,1
   4:   d85ffffe        beqzc   v0,0 <foo0>
   8:   d81f0000        jrc     ra

JRC lies in forbidden slot of BEQZC.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 28, 2023

@llvm/issue-subscribers-backend-mips

mpe pushed a commit to linuxppc/linux-ci that referenced this issue Mar 2, 2023
Clang is unable to handle the situation that a chunk of inline
assembly ends with a compat branch instruction and then compiler
generates another control transfer instruction immediately after
this compat branch. The later instruction will end up in forbidden
slot and cause exception.

Workaround by add a option to control the use of compact branch.
Currently it's selected by CC_IS_CLANG and hopefully we can change
it to a version check in future if clang manages to fix it.

Fix boot on boston board.

Link: llvm/llvm-project#61045
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Acked-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 24, 2023

Why is inline assembly relevant? GCC and Clang do the same thing for this https://godbolt.org/z/hrf5ndKhv. Is it just a case of teaching the .set reorder code to deal with forbidden slots like it does delay slots?

@FlyGoat
Copy link
Author

FlyGoat commented Aug 25, 2023

Why is inline assembly relevant? GCC and Clang do the same thing for this https://godbolt.org/z/hrf5ndKhv. Is it just a case of teaching the .set reorder code to deal with forbidden slots like it does delay slots?

Ouch yes that's probably the root cause. I was only inspecting generated binary so I was not aware of those directives.

@wzssyqa Could you please make Ying Huang aware of that?

yingopq added a commit to yingopq/llvm-project that referenced this issue Jan 9, 2024
…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.
yingopq added a commit to yingopq/llvm-project that referenced this issue Feb 22, 2024
…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.
wzssyqa pushed a commit that referenced this issue Feb 24, 2024
#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 #61045.

Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not
ending, so we commit pull request again.
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Feb 24, 2024
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)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Feb 27, 2024
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants