This is an archive of the discontinued LLVM Phabricator instance.

[mips] Fix select patterns for MIPS64
ClosedPublic

Authored by sdardis on Apr 21 2016, 11:10 AM.

Details

Summary

When targetting MIPS64R6 some of the patterns for select were guarded by a
broken predicate. The predicate was supposed to test if a constant value
could fit in a 16 bit zero-extended field. Instead the value was tested to
fit in a 16 bit sign-extended field. For negative constants of native word
width this resulted in wrong code generation.

Diff Detail

Event Timeline

sdardis updated this revision to Diff 54547.Apr 21 2016, 11:10 AM
sdardis retitled this revision from to [mips] Fix select patterns for MIPS64.
sdardis updated this object.
sdardis added reviewers: dsanders, vkalintiris.
sdardis set the repository for this revision to rL LLVM.
sdardis added subscribers: petarj, llvm-commits.

Forgot llvm-commits the last time.

Daniel, Vasileios, if this can be merged today (assuming it's an obvious fix), we could push Google for getting it into next clang binary (later today or Monday). Otherwise, we will likely have to wait another month or so.

vkalintiris accepted this revision.Apr 22 2016, 5:46 AM
vkalintiris edited edge metadata.

LGTM with the addition of one test for the M3 prefix.

test/CodeGen/Mips/llvm-ir/select-int.ll
165

This label is different from the function's name.

This revision is now accepted and ready to land.Apr 22 2016, 5:46 AM
dsanders accepted this revision.Apr 22 2016, 5:57 AM
dsanders edited edge metadata.

Well spotted. LGTM with the changes Vasileios requested and an indentation nit.

Forgot llvm-commits the last time.

Normally you should abandon the patch and re-submit with llvm-commits CC'd. This is because the commit message and patch aren't sent to the mailing list unless it's CC'd from the start.

test/CodeGen/Mips/llvm-ir/select-int.ll
196–197

Indentation

sdardis updated this revision to Diff 54638.Apr 22 2016, 6:21 AM
sdardis edited edge metadata.
sdardis removed rL LLVM as the repository for this revision.
sdardis marked 2 inline comments as done.

Addressed review comments.

sdardis closed this revision.Apr 22 2016, 6:26 AM

Thanks, committed as r267151.