Skip to content

Conversation

@yingopq
Copy link
Contributor

@yingopq yingopq commented May 14, 2025

When -triple is windows, SelectionDAGLegalize process Legalizing br_jt, would generate ISD::JUMP_TABLE_DEBUG_INFO.
Then Mips process emitInstruction, would think JUMP_TABLE_DEBUG_INFO is a pseudo instruction and generate an error Pseudo opcode found in emitInstruction().
This instruction TargetOpcode::JUMP_TABLE_DEBUG_INFO is only used to note jump table debug info, so we can ignore it when Mips emit instruction.

Fix #134916.


// This instruction is only used to note jump table debug info, we
// did not need to emit it.
if (I->getOpcode() == TargetOpcode::JUMP_TABLE_DEBUG_INFO)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it emit some debug info?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I would add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied.

@yingopq yingopq force-pushed the Fix_bug_issue_134916 branch from 2268229 to ca5ea1b Compare May 14, 2025 08:55
@brad0
Copy link
Contributor

brad0 commented May 14, 2025

cc @MaskRay @topperc


// This instruction is only used to note jump table debug info, we
// did not need to emit it.
if (I->getOpcode() == TargetOpcode::JUMP_TABLE_DEBUG_INFO) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean emit a comment line into the asm output with some useful information.

@s-barannikov
Copy link
Contributor

This pseudo instruction is already skipped by the generic code in AsmPrinter.cpp:

case TargetOpcode::JUMP_TABLE_DEBUG_INFO:

The reason it got to MipsAsmPrinter is because it was bundled with PseudoIndirectBranch:

  PseudoIndirectBranch killed renamable $at {
    JUMP_TABLE_DEBUG_INFO 0
  }

I don't think it is intentional.

@s-barannikov
Copy link
Contributor

I think a better fix would be to add || CurrI->isJumpTableDebugInfo() here:

if (CurrI->isDebugInstr()) {

@@ -0,0 +1,119 @@
; RUN: llc -mtriple=mipsel-windows-gnu < %s | FileCheck %s -check-prefix=MIPSEL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use an existing jumptable*.ll test and add the windows-gnu triple there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing file named jumptable_labels.ll, I thought this file mainly focused on testing labels, so I choose to add a new file.
Since you think this is OK, I will add a -mtriple=mips-windows-gnu there.
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, applied.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mips is under-maintained and I find many tests need refactoring. It's great to think about whether tests can be enhanced before adding a new one:) https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-too-little#i-dont-know-an-existing-test-can-be-enhanced

@yingopq
Copy link
Contributor Author

yingopq commented May 15, 2025

I think a better fix would be to add || CurrI->isJumpTableDebugInfo() here:

if (CurrI->isDebugInstr()) {

Good idea, thanks!

@yingopq yingopq force-pushed the Fix_bug_issue_134916 branch 2 times, most recently from ff7287b to 873188e Compare May 16, 2025 08:00
@brad0 brad0 requested review from MaskRay and wzssyqa May 18, 2025 05:47
@yingopq yingopq force-pushed the Fix_bug_issue_134916 branch 3 times, most recently from e021e15 to f96fcca Compare May 21, 2025 02:36
@brad0
Copy link
Contributor

brad0 commented May 24, 2025

@wzssyqa @s-barannikov Ping.

1 similar comment
@brad0
Copy link
Contributor

brad0 commented Jun 4, 2025

@wzssyqa @s-barannikov Ping.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last part of the comment is misleading, otherwise LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I delete that part.

When -triple is windows, SelectionDAGLegalize process Legalizing
br_jt, would generate ISD::JUMP_TABLE_DEBUG_INFO.
Then Mips process emitInstruction, would think JUMP_TABLE_DEBUG_INFO
is a pseudo instruction and generate an error `Pseudo opcode found
in emitInstruction()`.
This instruction `TargetOpcode::JUMP_TABLE_DEBUG_INFO` is only used
to note jump table debug info, so we can ignore it when Mips emit
instruction.

Fix llvm#134916.
@yingopq yingopq force-pushed the Fix_bug_issue_134916 branch from f96fcca to 6ca09bc Compare June 4, 2025 07:01
@yingopq yingopq merged commit bbe5ceb into llvm:main Jun 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MIPS] Compiler crash when using -O3

5 participants