Page MenuHomePhabricator

[LoongArch] Set correct encodings for DWARF exception handling
ClosedPublic

Authored by wangleiat on Sep 27 2022, 12:53 AM.

Details

Summary

This patch sets correct encodings for DWARF exception handling for
LoongArch.

Depends on D134709

Diff Detail

Event Timeline

wangleiat created this revision.Sep 27 2022, 12:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2022, 12:53 AM
wangleiat requested review of this revision.Sep 27 2022, 12:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2022, 12:53 AM
xen0n added a comment.Sep 27 2022, 9:31 PM

LGTM but I haven't cross-checked with gcc.

llvm/test/CodeGen/LoongArch/dwarf-eh.ll
5

nit: one single space

LGTM but I haven't cross-checked with gcc.

https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/config/loongarch/loongarch.h#l1133:

#ifdef HAVE_AS_EH_FRAME_PCREL_ENCODING_SUPPORT
#define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
  (((GLOBAL) ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | DW_EH_PE_sdata4)
#else
#define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
  (((GLOBAL) ? DW_EH_PE_indirect : 0) | DW_EH_PE_absptr)
#endif

The ifdef is just for old GNU assemblers, we don't need to care about it because normally LLVM doesn't use an external assembler. I'm not sure about the difference with DW_EH_PE_indirect though (I know literally, nothing about debug symbols).

LGTM but I haven't cross-checked with gcc.

https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/config/loongarch/loongarch.h#l1133:

#ifdef HAVE_AS_EH_FRAME_PCREL_ENCODING_SUPPORT
#define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
  (((GLOBAL) ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | DW_EH_PE_sdata4)
#else
#define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
  (((GLOBAL) ? DW_EH_PE_indirect : 0) | DW_EH_PE_absptr)
#endif

The ifdef is just for old GNU assemblers, we don't need to care about it because normally LLVM doesn't use an external assembler.

Considering that the old binutils might be used, gcc keeps this definition:

#define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
  (((GLOBAL) ? DW_EH_PE_indirect : 0) | DW_EH_PE_absptr)

I'm not sure about the difference with DW_EH_PE_indirect though (I know literally, nothing about debug symbols).

The definition here is simply to keep consistent with gcc. As for DW_EH_PE_indirect, it seems to be for the read-only attribute of .eh_frame section.

llvm/test/CodeGen/LoongArch/dwarf-eh.ll
5

nit: one single space

Thanks, I will remove the extra spaces.

wangleiat updated this revision to Diff 463740.Sep 28 2022, 6:58 PM

Address @xen0n's comments

xen0n accepted this revision.Sep 29 2022, 8:19 PM

Cross-checked with some gcc code and it looks okay. I don't think we should care for old binutils, the new-world userbase is small enough it'd be easier to tell everyone to upgrade than maintaining such a compatibility hack indefinitely.

This revision is now accepted and ready to land.Sep 29 2022, 8:19 PM
MaskRay accepted this revision.Oct 1 2022, 12:35 AM

One nit

llvm/test/CodeGen/LoongArch/dwarf-eh.ll
2

Better to set --relocation-model=static instead of relying on the default.

wangleiat added inline comments.Oct 2 2022, 4:25 AM
llvm/test/CodeGen/LoongArch/dwarf-eh.ll
2

Better to set --relocation-model=static instead of relying on the default.

Thanks, I will modify it.

wangleiat updated this revision to Diff 464548.Oct 2 2022, 4:38 AM

Address @MaskRay's comment.