[MC][AsmPrinter] Introduce llvm_reg_offset pseudo cfi instruction. #125104
Conversation
| Expr.push_back(dwarf::DW_OP_consts); | ||
| Expr.append(Buffer, Buffer + encodeSLEB128(Offset, Buffer)); | ||
| Expr.push_back((uint8_t)dwarf::DW_OP_bregx); | ||
| Expr.append(Buffer, Buffer + encodeULEB128(FrameReg, Buffer)); |
There was a problem hiding this comment.
Is this using llvm's internal numbering for registers? That's not stable across releases.
There was a problem hiding this comment.
No, it's using DwarfEH register numbers like all other CFIs
There was a problem hiding this comment.
I see other code does stuff like this:
if (!IsEH)
Reg = MRI->getDwarfRegNumFromDwarfEHRegNum(Reg);
Do we need to do something similar?
There was a problem hiding this comment.
I didn't find a good way to pass IsEH. However currently, when we are emitting the same escape elsewhere we don't care about IsEH, so if in this patch if we ignore IsEH things should work the way they are now.
There was a problem hiding this comment.
Do we need MRI->getDwarfRegNumFromDwarfEHRegNum(FrameReg);`?
mgudim
force-pushed
the
cfi_reg_offset
branch
from
5c7f4e1 to
c37f01c
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mgudim
force-pushed
the
cfi_reg_offset
branch
2 times, most recently
from
3405801 to
6dd7dd8
Compare
| #CHECK-LABEL: func: | ||
| #CHECK: .cfi_startproc | ||
| #CHECK: .cfi_escape 0x10, 0x01, 0x06, 0x11, 0x56, 0x92, 0x02, 0x00, 0x22 | ||
| #COM: CHECK: ret |
There was a problem hiding this comment.
Why are these COM? If not needed should they be removed?
| @@ -265,6 +265,8 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) { | |||
| case MCCFIInstruction::OpLabel: | |||
| case MCCFIInstruction::OpValOffset: | |||
| break; | |||
| case MCCFIInstruction::OpLLVMRegOffset: | |||
| llvm_unreachable("Can't handle llvm_reg_offset yet!"); | |||
There was a problem hiding this comment.
What is the impact of not handling this yet?
There was a problem hiding this comment.
Nothing emits OpLLVMRegOffset yet, I just added this to avoid a warning.
mgudim
force-pushed
the
cfi_reg_offset
branch
from
6dd7dd8 to
bb72c5f
Compare
|
Can you please post the shrink wrapping PRs that stack on this one? It is difficult to understand why this change is needed without the additional context. |
@michaelmaitland |
Some targets which have scalable vectors (AArch64, RISCV) emit cfi escape expression of the form "deref(FrameReg + Offset)". Now instead of explicitly building up the expression, we can just emit the llvm_reg_offset. Also, we will need to handle such escape expressions in CFIInstrInserter. Without this pseudo, we would have to try to decode the escape expression inside the CFIInstrInserter. Now, when we have this pseudo, we can understand such escape expressions without decoding.
Some targets which have scalable vectors (AArch64, RISCV) emit cfi escape expression of the form "deref(FrameReg + Offset)". Now instead of explicitly building up the expression, we can just emit the llvm_reg_offset.
Also, we will need to handle such escape expressions in CFIInstrInserter. Without this pseudo, we would have to try to decode the escape expression inside the CFIInstrInserter. Now, when we have this pseudo, we can understand such escape expressions without decoding.