This is an archive of the discontinued LLVM Phabricator instance.

Add ISD::EH_DWARF_CFA, simplify @llvm.eh.dwarf.cfa on Mips, fix on PowerPC
ClosedPublic

Authored by hfinkel on Aug 30 2016, 9:20 AM.

Details

Summary

LLVM has an @llvm.eh.dwarf.cfa intrinsic, used to lower the GCC-compatible __builtin_dwarf_cfa() builtin. As pointed out in PR26761, this is currently broken on PowerPC (and likely on ARM as well). Currently, @llvm.eh.dwarf.cfa is lowered using:

ADD(FRAMEADDR, FRAME_TO_ARGS_OFFSET)

where FRAME_TO_ARGS_OFFSET defaults to the constant zero. On x86, FRAME_TO_ARGS_OFFSET is lowered to 2*SlotSize. This setup, however, does not work for PowerPC. Because of the way that the stack layout works, the canonical frame address is not exactly (FRAMEADDR + FRAME_TO_ARGS_OFFSET) on PowerPC (there is a lower save area offset as well), so it is not just a matter of implementing FRAME_TO_ARGS_OFFSET for PowerPC (unless we redefine its semantics). We can do that, since it is currently used only for @llvm.eh.dwarf.cfa lowering, but the better to directly lower the CFA construct itself (since it can be easily represented as a fixed-offset FrameIndex). Mips currently does this, but by using a custom lowering for ADD that specifically recognizes the (FRAMEADDR, FRAME_TO_ARGS_OFFSET) pattern.

This patch introduces a ISD::EH_DWARF_CFA node, which by default expands using the existing logic, but can be directly lowered by the target. It Mips to use this method (which simplifies its implementation, and I suspect makes it more robust), and updates PowerPC to do the same.

Fixes PR26761.

Diff Detail

Event Timeline

hfinkel updated this revision to Diff 69705.Aug 30 2016, 9:20 AM
hfinkel retitled this revision from to Add ISD::EH_DWARF_CFA, simplify @llvm.eh.dwarf.cfa on Mips, fix on PowerPC.
hfinkel updated this object.
hfinkel added reviewers: dsanders, echristo, jmolloy.
hfinkel added a subscriber: llvm-commits.
emaste added a subscriber: emaste.Aug 30 2016, 10:39 AM
sdardis accepted this revision.Sep 1 2016, 3:17 AM
sdardis edited edge metadata.

LGTM. Minor formatting nit inlined.

lib/Target/PowerPC/PPCISelLowering.cpp
6200

Nit: space between "isPP64" and the ?.

This revision is now accepted and ready to land.Sep 1 2016, 3:17 AM
This revision was automatically updated to reflect the committed changes.
echristo edited edge metadata.Sep 1 2016, 11:26 AM

Couple of nits. Looks fine otherwise.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
5021

Weird formatting?

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
2788 ↗(On Diff #69976)

Ditto.