This is an archive of the discontinued LLVM Phabricator instance.

[OMPT] Drop LoongArch support
AbandonedPublic

Authored by Ami-zhang on Nov 7 2022, 4:17 AM.

Details

Summary

Currently, __builtin_frame_address() is required for OMPT. And
LoongArch only supports the current frame (i.e. depth = 0). So
this will fail if the depth != 0.

Diff Detail

Event Timeline

Ami-zhang created this revision.Nov 7 2022, 4:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 4:17 AM
Ami-zhang requested review of this revision.Nov 7 2022, 4:17 AM
xry111 added a comment.Nov 7 2022, 5:16 AM

Is it possible to implement __builtin_frame_address for LoongArch then?

MaskRay requested changes to this revision.Nov 7 2022, 9:41 AM

This change is only necessary and will be unneeded when __builtin_frame_address is implemented. OpenMP support is bringing up. LoongArch should not cause unnecessary churn to the upstream.
LoongArch users building openmp can disable LIBOMP_OMPT_SUPPORT for now.

This revision now requires changes to proceed.Nov 7 2022, 9:41 AM

Is it possible to implement __builtin_frame_address for LoongArch then?

For __builtin_frame_address(depth) where depth > 0, can we return 0 directly if frame pointer elimination is enabled to avoid the issue mentioned here.
I wonder how gcc handle case?

GCC documentation claims:

On some machines it may be impossible to determine the frame
address of any function other than the current one; in such cases,
or when the top of the stack has been reached, this function
returns '0' if the first frame pointer is properly initialized by
the startup code.

Calling this function with a nonzero argument can have
unpredictable effects, including crashing the calling program.  As
a result, calls that are considered unsafe are diagnosed when the
'-Wframe-address' option is in effect.  Such calls should only be
made in debugging situations.
Ami-zhang abandoned this revision.Nov 11 2022, 6:09 PM

This change is only necessary and will be unneeded when __builtin_frame_address is implemented. OpenMP support is bringing up. LoongArch should not cause unnecessary churn to the upstream.
LoongArch users building openmp can disable LIBOMP_OMPT_SUPPORT for now.

I have abandoned this diff.
We'll try to support that condition where depth != 0 later .

If I read the code correctly, calling __builtin_frame_address with a depth > 0 is only used in tests. Can we disable those tests if __builtin_frame_address cannot handle depth > 0?

If I read the code correctly, calling __builtin_frame_address with a depth > 0 is only used in tests. Can we disable those tests if __builtin_frame_address cannot handle depth > 0?

Good idea.