This is an archive of the discontinued LLVM Phabricator instance.

[X86] Fix some vector cmp builtins - TRUE/FALSE predicates
ClosedPublic

Authored by GBuella on Jun 28 2018, 2:34 AM.

Details

Summary

This patch removes on optimization used with the TRUE/FALSE
predicates, as was suggested in https://reviews.llvm.org/D45616
for r335339.
The optimization was buggy, since r335339 used it also
for *_mask builtins, without actually applying the mask -- the
mask argument was just ignored.

Diff Detail

Repository
rC Clang

Event Timeline

GBuella created this revision.Jun 28 2018, 2:34 AM
spatel added inline comments.Jun 28 2018, 7:33 AM
test/CodeGen/avx-builtins.c
2181

Why are we deleting tests instead of replacing the CHECK lines with the new output?

GBuella added inline comments.Jun 28 2018, 7:47 AM
test/CodeGen/avx-builtins.c
2181

These cases were only added, when the TRUE/FALSE related special cases were added in CGBuiltin.cpp. Now that the special cases are removed from CGBuiltin.cpp, it seemed consistent to also remove the related special cases from the tests, as these are not special anymore.
Should we have tests with all predicates?

spatel added inline comments.Jun 28 2018, 8:02 AM
test/CodeGen/avx-builtins.c
2181

Yes. IIUC, we would have caught the bug before it was committed if we had proper tests for each predicate and C intrinsic.

GBuella added inline comments.Jun 28 2018, 11:36 AM
test/CodeGen/avx-builtins.c
2181

That is true!
I'll try to write a script that generates all cases, but not today.

GBuella updated this revision to Diff 154216.Jul 5 2018, 6:09 AM

As suggested, I added test cases with all predicates (in r336346).

spatel accepted this revision.Jul 5 2018, 7:00 AM

LGTM.

This revision is now accepted and ready to land.Jul 5 2018, 7:00 AM
This revision was automatically updated to reflect the committed changes.