This is an archive of the discontinued LLVM Phabricator instance.

push LR before mcount on ARM
AbandonedPublic

Authored by jcai19 on Jul 20 2019, 12:54 AM.

Event Timeline

jcai19 created this revision.Jul 20 2019, 12:54 AM

What a horrible function. AAPCS? Who cares about that?

llvm/lib/Target/ARM/CMakeLists.txt
54 ↗(On Diff #210949)

This filename is too generic. It handles a single GNU-specific profiling function.

llvm/lib/Target/ARM/CustomCallLoweringPass.cpp
1 ↗(On Diff #210949)

This needs a comment header, with the usual licensing info and a brief description of what this pass does.

33 ↗(On Diff #210949)

Can this be initialized from the call instruction?

45 ↗(On Diff #210949)

What happens if a function calling the counter gets inlined?

For pure stack correctness you probably need to ensure all calls to mcount get a push before them; the counts may be screwy but that's probably better than a segfault.

What a horrible function. AAPCS? Who cares about that?

Thanks for the comments. I think I accidentally created two redundant code reviews. Please take a look at https://reviews.llvm.org/D65019. There has already been suggestion for a more formal solution by introducing an ARM intinriscs that hopefully should address these concerns.

jcai19 abandoned this revision.Jul 20 2019, 11:33 PM

A front test of mcount was broken since we renamed gnu_mcount_nc to an intrinsic. Needs to fix the test before reapplying the changes.

jcai19 reclaimed this revision.Aug 16 2019, 4:00 PM
jcai19 updated this revision to Diff 215703.Aug 16 2019, 4:04 PM

Fix frontend unit tests for __gnu_mcount_nc.

Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2019, 4:04 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jcai19 abandoned this revision.Aug 16 2019, 4:23 PM