This is an archive of the discontinued LLVM Phabricator instance.

[mips] Add calling convention tests covering O32, N32, and N64.
ClosedPublic

Authored by dsanders on Apr 10 2014, 6:04 AM.

Details

Summary

I had difficulty finding tests for the N32 and N64 ABI so I've added a
collection of calling convention tests based on the document MIPS ABIs
Described (MD00305), the MIPSpro N32 Handbook, and the SYSV ABI. Where the
documents/implementations disagree, I've used GCC to resolve the conflict.

A few interesting details:

  • For N32, LLVM uses 64-bit pointers when saving $ra despite pointers being 32-bit. I've yet to find a supporting statement in the ABI documentation but the current behaviour matches GCC.
  • For O32, the non-variable portion of a varargs argument list is also subject to the rule that floating-point is passed via GPR's (on N32/N64 only the variable portion is subject to this rule). This agrees with GCC's behaviour and the SYSV ABI but contradicts part of the MIPSpro N32 Handbook which talks about O32's behaviour.
  • The N32 implementation has the wrong callee-saved register list. (I already have a fix for this but will commit it as a follow-up).

I've left RUN-TODO lines in for O32 on MIPS64. I don't plan to support this case
for now but we should revisit it.

Diff Detail

Event Timeline

Some nitpicks apart from the problem with "ALL-INV".
There are some tests (the last 5 that I just skimmed through). I'll have another go at them later tonight.

test/CodeGen/Mips/cconv/arguments-float.ll
62

Wrongly placed comment.

104

After this comment we have no instructions that show N32/N64 getting the arguments from the stack. I believe it would be easier to read this test if we moved ; NEW-DAG: ld [[R3:\$[0-9]+]], 0($sp) to after the comment.

164

I believe you =) but this part of the test doesn't appear to cover that case. Could you add one instruction that shows the load from the stack for N32/N64 ?

test/CodeGen/Mips/cconv/arguments-fp128.ll
11

As discussed, fp128 on O32 is the same as double.

46

Isn't it best to actually test it used the stack ? This only proves the first 4 arguments were passed in registers.

test/CodeGen/Mips/cconv/arguments-hard-float.ll
172

Wrong comment.

202

Wrong comment.

test/CodeGen/Mips/cconv/arguments-hard-fp128.ll
11

As discussed, same as double on O32.

44

I believe it would be best to actually check we are accessing the stack for the other arguments otherwise the test only checks the first 4 arguments were passed on the stack.

test/CodeGen/Mips/cconv/arguments.ll
125

Wrong comment ?

test/CodeGen/Mips/cconv/callee-saved-float.ll
12

Is there a bug report describing this problem ? This is so we don't completely forget about it given that the test is not marked as XFAIL.

33

Prefix not defined.

Done.

test/CodeGen/Mips/cconv/return-hard-float.ll
14

Possibly "float returns" given that this test checks the return value using FP hardware registers ?

test/CodeGen/Mips/cconv/return-hard-fp128.ll
8

Possibly "float returns" given that this test checks the return value using FP hardware registers ?

dsanders added inline comments.Apr 14 2014, 3:09 AM
test/CodeGen/Mips/cconv/callee-saved-float.ll
12

There's no need. The follow up D3340 fixes the problem and removes the -TODO's.

33

Well spotted. Fixing this also uncovers a a contradiction in the test. It's testing for the odds $f25-$f31 not being callee saved for all ABI's but testing that they are callee-saved for N64.

dsanders updated this revision to Unknown Object (????).Apr 14 2014, 3:22 AM
  • fix issues raised by review and a self-contradiction in callee-saved-float.ll for N64
dsanders updated this revision to Unknown Object (????).Apr 14 2014, 3:23 AM
  • fix issues raised by review and a self-contradiction in callee-saved-float.ll for N64

Please ignore diff 2. I accidentally generated it based on origin/master instead of HEAD^

matheusalmeida accepted this revision.Apr 16 2014, 1:29 AM

LGTM.

test/CodeGen/Mips/cconv/callee-saved-float.ll
12

Great.

dsanders closed this revision.Apr 16 2014, 3:06 AM