Page MenuHomePhabricator

[ARM][FIX] Ran out of registers due tail recursion
ClosedPublic

Authored by dnsampaio on May 30 2019, 8:30 AM.

Details

Summary
  • pr42062

When compiling for MinSize,
ARMTargetLowering::LowerCall decides to indirect
multiple calls to a same function. However,
it disconsiders the limitation that thumb1
indirect calls require the callee to be in a
register from r0 to r3 (llvm limiation).
If all those registers are used by arguments, the
compiler dies with "error: run out of registers
during register allocation".
This patch tells the function
IsEligibleForTailCallOptimization if we intend to
perform indirect calls, as to avoid tail call
optimization.

Diff Detail

Repository
rL LLVM

Event Timeline

dnsampaio created this revision.May 30 2019, 8:30 AM
dmgreen edited reviewers, added: efriedma; removed: eli.friedman.May 30 2019, 11:34 AM

Nice one. Please remember to clang-format the patch.

lib/Target/ARM/ARMISelLowering.cpp
1888 ↗(On Diff #202219)

This looks like an unrelated change (and a little harder to parse to me, with the comment and newly between the if and the else).

1888 ↗(On Diff #202219)

Also, is isSibCall ever different to isTailCall? Do we need both?

test/CodeGen/ARM/pr42062.ll
7 ↗(On Diff #202219)

Its probably best to test the whole output, to check it's using correct registers.

dnsampaio updated this revision to Diff 202380.May 31 2019, 1:11 AM
dnsampaio marked 4 inline comments as done.

[ARM][FIX] Ran out of registers due tail recursion

  • Removed isSibCall in favor of isTailCall
  • clang-format-diff
  • Updated test, now checking for correct registers
dnsampaio added inline comments.May 31 2019, 1:11 AM
lib/Target/ARM/ARMISelLowering.cpp
1888 ↗(On Diff #202219)

Indeed it is not used. Will do a little of clean up as well by removing it.

I think this looks OK, but is would be good to get @efriedma's opinion.

lib/Target/ARM/ARMISelLowering.cpp
2329 ↗(On Diff #202380)

Perhaps call this "IsIndirect" on this side of the call?

efriedma accepted this revision.May 31 2019, 1:04 PM

LGTM

Eventually I think the code in IsEligibleForTailCallOptimization will disappear if we make ip an allocatable register, but this seems okay for now.

This revision is now accepted and ready to land.May 31 2019, 1:04 PM
This revision was automatically updated to reflect the committed changes.