[turbofan] Fix calls to computed code objects.

[turbofan] Fix calls to computed code objects.

R=mstarzinger@chromium.org
BUG=

Committed: https://crrev.com/924b0ecf8556ef75d3dff960b04c2a6318345c56
Cr-Commit-Position: refs/heads/master@{#31057}

[Michael Starzinger, 2015-10-01 15:48:54]

LGTM with a nit.

https://codereview.chromium.org/1377423002/diff/1/test/cctest/compiler/codege...
File test/cctest/compiler/codegen-tester.h (right):

https://codereview.chromium.org/1377423002/diff/1/test/cctest/compiler/codege...
test/cctest/compiler/codegen-tester.h:55: Handle<Code> code() {
nit: s/code/GetCode/ because it does way more than a simple getter.

[titzer, 2015-10-01 16:47:57]

On 2015/10/01 15:48:54, Michael Starzinger wrote:
> LGTM with a nit.
> 
>
https://codereview.chromium.org/1377423002/diff/1/test/cctest/compiler/codege...
> File test/cctest/compiler/codegen-tester.h (right):
> 
>
https://codereview.chromium.org/1377423002/diff/1/test/cctest/compiler/codege...
> test/cctest/compiler/codegen-tester.h:55: Handle<Code> code() {
> nit: s/code/GetCode/ because it does way more than a simple getter.

Done.

[titzer, 2015-10-01 16:48:00]

The CQ bit was checked by titzer@chromium.org

[titzer, 2015-10-01 16:48:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1377423002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-10-01 16:48:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1377423002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1377423002/20001

[commit-bot: I haz the power, 2015-10-01 17:11:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-01 17:12:03]

Patchset 2 (id:??) landed as
https://crrev.com/924b0ecf8556ef75d3dff960b04c2a6318345c56
Cr-Commit-Position: refs/heads/master@{#31057}

[MTBrandyberry, 2015-10-05 22:17:25]

On 2015/10/01 14:58:18, titzer wrote:

I have a question whether this new test is valid.  It appears that functions a
and b are generated with C linkage conventions, but that the calling code is
'tricked' into generating a call to a code object target (kCallAddress vs
kCallCodeObject).  I guess this works fine on most platforms (for functions such
as these without parameters).

I don't think this quite works for all platforms.  On AIX, for example, calls to
function pointers are done via descriptors -- which encode a level of
indirection such that the calling code must first load the instruction address
from the descriptor prior to branching.  Thus, as in this example, forcing the
caller to generate code not suitable for the target will result in a crash.

I see there is a TODO noted prior to the CallDescriptor logic in the test code. 
Is there an intention to implement this test in a different way in the future? 
Or perhaps I'm missing some other solution to this problem that can be
implemented in the platform code?

[titzer, 2015-10-05 22:36:16]

On 2015/10/05 22:17:25, mtbrandyberry wrote:
> On 2015/10/01 14:58:18, titzer wrote:
> 
> I have a question whether this new test is valid.  It appears that functions a
> and b are generated with C linkage conventions, but that the calling code is
> 'tricked' into generating a call to a code object target (kCallAddress vs
> kCallCodeObject).  I guess this works fine on most platforms (for functions
such
> as these without parameters).

Right. The only difference here is that the incoming code object is no longer a
constant, it's a computed value.


> I don't think this quite works for all platforms.  On AIX, for example, calls
to
> function pointers are done via descriptors -- which encode a level of
> indirection such that the calling code must first load the instruction address
> from the descriptor prior to branching.  Thus, as in this example, forcing the
> caller to generate code not suitable for the target will result in a crash.

So I think you are saying that you basically need the full functionality of the
macro assembler __ CallCFunction() for any calls to C, computed or not?

In that case, we should change the test to generate code objects that are
callable through a different convention. Here we're just interested in
checking the call, so I reused the testing functionality to get some functions
callable as C.

> I see there is a TODO noted prior to the CallDescriptor logic in the test
code. 
> Is there an intention to implement this test in a different way in the future?

> Or perhaps I'm missing some other solution to this problem that can be
> implemented in the platform code?

The TODO is mostly just to note that this was kind of a nasty dance and should
be fixed.