This is an archive of the discontinued LLVM Phabricator instance.

[mips] For indirect calls we don't need $gp to point to .got.
ClosedPublic

Authored by sstankovic on Aug 26 2014, 8:49 AM.

Details

Summary

Mips linker doesn't generate lazy binding stub for a function whose address is taken in the program.

Diff Detail

Repository
rL LLVM

Event Timeline

sstankovic updated this revision to Diff 12950.Aug 26 2014, 8:49 AM
sstankovic retitled this revision from to [mips] For indirect calls we don't need $gp to point to .got..
sstankovic updated this object.
sstankovic edited the test plan for this revision. (Show Details)
sstankovic added a reviewer: dsanders.
sstankovic added a subscriber: petarj.
sstankovic added a subscriber: Unknown Object (MLST).
dsanders added inline comments.Sep 23 2014, 7:50 AM
lib/Target/Mips/MipsISelLowering.cpp
2421 ↗(On Diff #12950)

I agree that this is equivalent, but is it possible to test for MO_GOT_CALL instead of the conditions that lead to choosing MO_GOT_CALL?

test/CodeGen/Mips/prevent-hoisting.ll
13–26 ↗(On Diff #12950)

This is just fixing up a test where the instructions changed order isn't it?

If so, I feel that there ought to be a more robust way of testing for this. This is close:

CHECK-NEXT: {{BB[0-9_#]+}}:
CHECK-DAG: sll $1, $[[R0:[0-9]+]], 4
CHECK: {{BB[0-9_#]+}}:
CHECK: [[BB0]]:

but there's no guarantee that there will be a second basic block between the two.

If neither of us can think of anything better, then we should go ahead with this change as-is and maintain it when necessary.

sstankovic updated this revision to Diff 14033.Sep 24 2014, 5:18 AM

The patch is updated.

sstankovic added inline comments.Sep 24 2014, 5:23 AM
lib/Target/Mips/MipsISelLowering.cpp
2421 ↗(On Diff #12950)

Done.

test/CodeGen/Mips/prevent-hoisting.ll
13–26 ↗(On Diff #12950)

This is just fixing up a test where the instructions changed order isn't it?

Yes. Currently the following sequence is generated at the start of two blocks:

sll     $1, $2, 4
sll     $2, $2, 6
addu    $1, $2, $1
lw      $2, %got(assignSE2partition)($16)

CHECK-NEXT checks only for the first instruction (it checks for instructions up to the first one that changes $at, which in this case is the first instruction). After the patch is applied, there are the same 4 instructions at the start of two blocks, but reordered:

lw      $2, %got(assignSE2partition)($2)
sll     $1, $3, 4
sll     $3, $3, 6
addu    $1, $3, $1

CHECK-NEXTs in the patch now check for first two instructions. Adding CHECK-DAG for all 4 instructions would protect from reordering, but the test must also make sure that all 4 instructions are at the start of the block, and I think that's not possible to do when DAG is used (that is, to combine DAG and NEXT).

dsanders accepted this revision.Sep 24 2014, 9:27 AM
dsanders edited edge metadata.

Thanks. I still can't think of a better way to update prevent-hoisting.ll so: LGTM

This revision is now accepted and ready to land.Sep 24 2014, 9:27 AM
sstankovic closed this revision.Oct 1 2014, 1:32 AM
sstankovic updated this revision to Diff 14265.

Closed by commit rL218744 (authored by @sstankovic).