Page MenuHomePhabricator

[mips] Don't use odd-numbered single precision registers for fastcc calling convention if -mno-odd-spreg is used.
ClosedPublic

Authored by sstankovic on Jul 26 2014, 2:52 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

sstankovic updated this revision to Diff 11914.Jul 26 2014, 2:52 AM
sstankovic retitled this revision from to [mips] Don't use odd-numbered single precision registers for fastcc calling convention if -mno-odd-spreg is used..
sstankovic updated this object.
sstankovic edited the test plan for this revision. (Show Details)
sstankovic added a reviewer: dsanders.
sstankovic added a project: deleted.
sstankovic added a subscriber: Unknown Object (MLST).
dsanders accepted this revision.Jul 26 2014, 7:31 AM
dsanders edited edge metadata.

LGTM, with a nit

I'll merge it to the branch once it's committed since it fixes the last of the MIPS32r6/MIPS64r6 test-suite failures.

test/CodeGen/Mips/fastcc.ll
300–302 ↗(On Diff #11914)

It's because FileCheck scans ahead for ']]' which leaves the pattern as 'f[0-9]*[02468'. I usually work around it using a '+' like so:

$[[F0:f[0-9]*[02468]+]]

I can't explain why the other one works though. Triple ']' has never worked correctly for me.

357 ↗(On Diff #11914)

Nit: Change the ']]]' to ']+]]' since we don't understand why this particular case works when all other cases of ']]]' we've encountered result in invalid regexes

This revision is now accepted and ready to land.Jul 26 2014, 7:31 AM
sstankovic updated this revision to Diff 11941.Jul 28 2014, 4:21 AM
sstankovic edited edge metadata.

I tried $[[F0:f[0-9]*[02468]+]] but it also doesn't work. It seems that problem is related to using DAG in check lines. I removed DAG from the last 3 lines, used $[[F0:f[0-9]*[02468]]] pattern and the test passes.

In D4682#8, @sstankovic wrote:

I tried $[[F0:f[0-9]*[02468]+]] but it also doesn't work. It seems that problem is related to using DAG in check lines. I removed DAG from the last 3 lines, used $[[F0:f[0-9]*[02468]]] pattern and the test passes.

I've explained the current problem below. To be clear, I think you are ok to commit as-is. If it's possible to follow-up to make the test more robust then we should do so. Unique-ing the output instructions using different offsets is a good way to do this but I've found cases before where it's not sufficient (one of the calling convention tests has the same problem).

test/CodeGen/Mips/fastcc.ll
300–302 ↗(On Diff #11914)

I think I see why this didn't work. It seems that the directive on line 303 is matching the first lwc1 in the code below:

lw      $1, %got(gfa8)($gp)
lwc1    $f16, 0($1)
...
lw      $1, %got(gfa10)($gp)
lwc1    $f20, 0($1)

The reason I think this happens is that IIRC, all the *-DAG directives start their search from the same point (the directive on line 273). There's nothing making the lines unique so line 295 and line 303 match in the same place. This leaves F0 set to the wrong register, line 304 fails to match, and the test is reported to fail.

There are two solutions to this. The first is make each line unique using different offsets into an array instead of using multiple global variables. The second is the way you've done it: Use something other than *-DAG to advance the current position past the unintended match.

If the above is correct, then I'm inclined to think that FileCheck ought to be accounting for the topography earlier. In particular, variable uses ought to start matching from the end of the directive containing the definition (instead of from the end of the previous non-*-DAG directive). However, changing that is likely to be hard and it's difficult to say whether there are tests depending on the current behaviour or not.

I replaced global variables with the array. This actually makes CHECKS much simpler.

LGTM, Thanks.

sstankovic closed this revision.Jul 29 2014, 7:48 AM
sstankovic updated this revision to Diff 11980.

Closed by commit rL214180 (authored by @sstankovic).