This is an archive of the discontinued LLVM Phabricator instance.

Simplify Libcalls for ARM PCS
ClosedPublic

Authored by samparker on Sep 12 2016, 5:35 AM.

Details

Summary

Added a helper function for SimplifyLibCalls to allow functions annotated with APCS, AAPCS or AAPCS_VFP to be treated like normal C functions if they only have integer and/or pointer arguments and return values, and is not on iOS.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker updated this revision to Diff 70996.Sep 12 2016, 5:35 AM
samparker retitled this revision from to Simplify Libcalls for ARM PCS.
samparker updated this object.
samparker added reviewers: rengolin, t.p.northover.
samparker added a subscriber: llvm-commits.
jmolloy requested changes to this revision.Sep 12 2016, 5:48 AM
jmolloy added a reviewer: jmolloy.
jmolloy added a subscriber: jmolloy.

Hi Sam,

Thanks for doing this. The testing in particular is really nice. I have a couple of superficial comments.

Cheers,

James

lib/Transforms/Utils/SimplifyLibCalls.cpp
59 ↗(On Diff #70996)

Typo: "compatible"

59 ↗(On Diff #70996)

It'd be nice for the function to encode that it's talking about compatibility with the C calling convention. I understand that that would put a lot of C's in a row though...

isCallingConvCCompatible()
64 ↗(On Diff #70996)

I think this would look neater as a switch.

68 ↗(On Diff #70996)

Please add a comment as to why.

78 ↗(On Diff #70996)

param -> Param

79 ↗(On Diff #70996)

This could simply be:

if (!P->isPointerTy() && !P->isIntegerTy())
  return false;
This revision now requires changes to proceed.Sep 12 2016, 5:48 AM

revision to follow.

lib/Transforms/Utils/SimplifyLibCalls.cpp
59 ↗(On Diff #70996)

cheers, I forgot to change this before uploading.

64 ↗(On Diff #70996)

sure

79 ↗(On Diff #70996)

much nicer.

samparker updated this revision to Diff 71012.Sep 12 2016, 8:35 AM
samparker edited edge metadata.

corrected typos, added comment and changed to use a switch statement.

jmolloy accepted this revision.Sep 12 2016, 8:55 AM
jmolloy edited edge metadata.

LGTM with one change.

lib/Transforms/Utils/SimplifyLibCalls.cpp
86 ↗(On Diff #71012)

Just change this to default: return false; so we don't end up with "enumeration value not covered in switch" warnings.

This revision is now accepted and ready to land.Sep 12 2016, 8:55 AM
This revision was automatically updated to reflect the committed changes.