[BOLT][RISCV] Handle CIE's produced by GNU as #69578
Merged
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.
On RISC-V, GNU as produces the following initial instruction in CIE's:
While I believe it is technically illegal to use this instruction without first using a
DW_CFA_def_cfa
(since the offset is undefined), bothreadelf
andllvm-dwarfdump
accept this and implicitly set the offset to 0.In BOLT, however, this triggers an assert (in
CFISnapshot::advanceTo
) as it (correctly) believes the offset is not set. This patch fixes this by setting the offset to 0 whenever executingDW_CFA_def_cfa_register
while the offset is undefined.Note that this is probably the simplest workaround but it has a downside: while emitting CFI start, we check if the initial instructions are contained within
MCAsmInfo::getInitialFrameState
and omit them if they are. This will not be true for GNU CIE's (since they differ from LLVM's) which causes an unnecessaryDW_CFA_def_cfa_register
to be emitted.While technically correct, it would probably be better to replace the GNU CIE with the one used by LLVM to avoid this situation. This would solve the problem this patch solves while also preventing unnecessary CFI instructions. However, this is a bit trickier to implement correctly so I propose to keep this for a later time.
Note on testing: the test creates a simple function with three basic blocks and forces the CFI state of the last one to be different from the others using an arbitrary CFI instruction. Then,
--reorder-blocks=reverse
is used to forceCFISnapshot::advanceTo
to be called. This causes an assert on the current main branch.