This is an archive of the discontinued LLVM Phabricator instance.

[X86] Accept SELECT op code for x86-64 fp128 type
ClosedPublic

Authored by chh on Jun 27 2016, 11:39 AM.

Details

Summary

DAGTypeLegalizer::CanSkipSoftenFloatOperand should allow SELECT op code for x86_64 fp128 type.
Without this patch, the test case fp128-select.ll fails in SoftenFloatOperand because the new
ISD::SELECT generated by llvm is not skipped in CanSkipSoftenFloatOperand.

This bug was found when compiling some compiler-rt code for Android x86-64 target.

Diff Detail

Event Timeline

chh updated this revision to Diff 61988.Jun 27 2016, 11:39 AM
chh retitled this revision from to [X86] Accept SELECT op code for x86-64 fp128 type.
chh updated this object.
chh added a subscriber: srhines.
chh removed a reviewer: llvm-commits.
chh added a subscriber: llvm-commits.
chh removed a subscriber: srhines.
RKSimon added inline comments.
test/CodeGen/X86/fp128-select.ll
18

Please can you regenerate this with utils/update_llc_test_checks.py or at least improve the checks so that the branch is more obvious.

chh updated this revision to Diff 62959.Jul 6 2016, 2:01 PM

Regenerated test file with utils/update_llc_test_checks.py.

chh marked an inline comment as done.Jul 6 2016, 2:03 PM

Regenerated fp128-select.ll.

Please can you test on at least one other target where fp128 is not legal?

test/CodeGen/X86/fp128-select.ll
3

You shouldn't need to specify mmx here

chh updated this revision to Diff 64183.Jul 15 2016, 12:43 PM

Add test cases for both MMX and non-MMX targets.

chh added a comment.Jul 15 2016, 12:46 PM

I added test cases for non-MMX cases.
I think non-fp128 types should already be covered in other tests.
So far this patch is to fix the only problem I know with fp128 type on x86-64 targets with MMX.

test/CodeGen/X86/fp128-select.ll
3

The problem only happens on x86-64 targets that use 128 bits for long double and with mmx.
Now I added both mmx and non-mmx tests.

RKSimon accepted this revision.Jul 16 2016, 5:17 AM
RKSimon edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jul 16 2016, 5:17 AM
This revision was automatically updated to reflect the committed changes.