This is an archive of the discontinued LLVM Phabricator instance.

[MSan] [Mips64] Add tests for vararg handling.
ClosedPublic

Authored by koriakin on May 4 2016, 6:57 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

koriakin retitled this revision from to [MSan] [Mips64] Add tests for vararg handling..
koriakin updated this object.
koriakin added reviewers: eugenis, aizatsky.
koriakin set the repository for this revision to rL LLVM.
koriakin added a project: Restricted Project.
koriakin added a subscriber: llvm-commits.

Ugh... while these tests are correct, I just noticed Mips assumes there's exactly one argument before the ... in arument list, and breaks hopelessly if that's not true. x86_64 is also broken, but less so - it assumes all arguments before ... fit in registers (ie. 6 ints/pointers, plus some floats). I haven't looked into AArch64 yet, but I suspect the same issue as x86_64.

aizatsky edited edge metadata.May 4 2016, 10:35 AM

I'm not sure what does "-el" mean in "vararg-el.ll". Can you pick a more descriptive suffix? Or add a comment to the test describing the difference from "vararg.ll".

Ugh... while these tests are correct, I just noticed Mips assumes there's exactly one argument before the ... in arument list, and breaks hopelessly if that's not true. x86_64 is also broken, but less so - it assumes all arguments before ... fit in registers (ie. 6 ints/pointers, plus some floats). I haven't looked into AArch64 yet, but I suspect the same issue as x86_64.

Issue reported at https://llvm.org/bugs/show_bug.cgi?id=27646 . I'll attempt to fix it for Mips64 and PowerPC64.

It's short for little endian.

I'm not sure what does "-el" mean in "vararg-el.ll". Can you pick a more descriptive suffix? Or add a comment to the test describing the difference from "vararg.ll".

It's for mips64el (little-endian variant). I'll change the names to vararg-mips64.ll and vararg-mips64el.ll .

aizatsky accepted this revision.May 4 2016, 11:00 AM
aizatsky edited edge metadata.
This revision is now accepted and ready to land.May 4 2016, 11:00 AM
This revision was automatically updated to reflect the committed changes.