Page MenuHomePhabricator

Ensure that TCReturn instructions are resolved
ClosedPublic

Authored by bgamari on Nov 27 2014, 4:48 PM.

Details

Reviewers
t.p.northover
Summary

TCReturn instructions would occassionally sneak through to AsmWriter
as GHC does not use our epilogue.

Diff Detail

Event Timeline

bgamari updated this revision to Diff 16710.Nov 27 2014, 4:48 PM
bgamari retitled this revision from to Ensure that TCReturn instructions are resolved.
bgamari updated this object.
bgamari edited the test plan for this revision. (Show Details)
bgamari added a reviewer: t.p.northover.
bgamari added a subscriber: Unknown Object (MLST).Nov 27 2014, 4:49 PM
t.p.northover edited edge metadata.Nov 27 2014, 4:58 PM

Hi Ben,

The code itself looks reasonable, but it'd be good to have a test here. Probably a .ll file living in test/CodeGen/ARM that makes sure the TCRETURNs are lowered and tail calls are emitted when using the ghc calling convention.

Cheers.

Tim.

bgamari updated this revision to Diff 16711.Nov 27 2014, 9:18 PM
bgamari edited edge metadata.

Add testcase

bgamari updated this revision to Diff 16712.Nov 27 2014, 9:20 PM

Fix diff

bgamari updated this revision to Diff 16713.Nov 27 2014, 9:22 PM
  • ghc-tcreturn: Add testcase and fix branch

A little bit of an arcanist mix-up there. All is well now.

I've added a very rudimentary testcase. It makes no attempt at ensuring that the tailcalls are lowered correctly. I think I'll have another look at this in the morning and try to add some more thorough checks.

Thanks for updating the test. It's usually better to test the actual output rather than just that a function doesn't crash though.

I'd suggest something like (untested):

; RUN: llc -mtriple=thumbv7-eabi -o - %s | FileCheck %s

declare cc 10 void @g()

define cc 10 void @test_direct_tail() {
; CHECK-LABEL: test_direct_tail:
; CHECK: b g

  tail call cc10 void @g()
  ret void
}

@ind_func = global void()* zeroinitializer

define cc 10 void @test_indirect_tail() {
; CHECK-LABEL: test_indirect_tail:
; CHECK: b {{r[0-9]+}}
  %func = load void()** @ind_func
  tail call cc10 void()* @%func()
  ret void
}

This also avoids leaving a stray .s file in the source directory and tests both TCRETURNdi and TCRETURNri.

bgamari updated this revision to Diff 16740.Nov 28 2014, 7:47 AM
  • ghc-tcreturn: Add testcase
t.p.northover accepted this revision.Nov 28 2014, 8:02 AM
t.p.northover edited edge metadata.

Thanks Ben. I think this looks fine now. Do you have commit access or shall I do it?

Tim.

This revision is now accepted and ready to land.Nov 28 2014, 8:02 AM

I do not have commit access.

Would it be possible to get this merged into a 3.5 bugfix release as well?

Hi Ben,

Sorry for the delay, I've committed the original as r223055 now.

What's going on with the 3.5 merge?