Page MenuHomePhabricator

[LoongArch] Add support for llvm.eh.dwarf.cfa
ClosedPublic

Authored by xen0n on Aug 16 2022, 10:27 PM.

Diff Detail

Event Timeline

xen0n created this revision.Aug 16 2022, 10:27 PM
xen0n requested review of this revision.Aug 16 2022, 10:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2022, 10:27 PM
xen0n updated this revision to Diff 453202.Aug 16 2022, 10:44 PM

use opaque pointers

Looks good to me except some nits. @wangleiat Do you have any comments?

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

Use double dashes.

2

Ditto.

xen0n updated this revision to Diff 453209.Aug 17 2022, 12:20 AM

Sigh. I always forget to update the invocation lines when I copy things...

xen0n marked 2 inline comments as done.Aug 17 2022, 12:21 AM
wangleiat added inline comments.Aug 17 2022, 12:51 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
181

Incorrect type

xen0n updated this revision to Diff 453223.Aug 17 2022, 1:06 AM

fix types

xen0n marked an inline comment as done.Aug 17 2022, 1:07 AM
xen0n added inline comments.
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
181

Oh my. I was pretty sure I used auto before, not sure how I managed to keep the wrong "bool" type (of IsRISCV64) in the submission... Fixed though.

SixWeining accepted this revision.Aug 17 2022, 4:11 AM
SixWeining added inline comments.
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
181

Seems this is only used one time. Better
to inline it? But I can’t find this rule in llvm coding standard.

This revision is now accepted and ready to land.Aug 17 2022, 4:11 AM
xen0n marked an inline comment as done.Aug 17 2022, 4:33 AM
xen0n added inline comments.
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
181

In this particular case, if all "values used only once" were to be inlined into the use site, no local values would remain. You would be left with a giant return which probably won't help with readability. (And I already slightly tweaked the code compared to the RISCV version.)

So I don't feel very inclined to inline more...

This revision was automatically updated to reflect the committed changes.