This is an archive of the discontinued LLVM Phabricator instance.

DAGCombiner: fix combine of trunc and select
ClosedPublic

Authored by AsafBadouh on Nov 9 2016, 6:25 AM.

Details

Summary

the pattern catch the following:
// trunc (select c, a, b) -> select c, (trunc a), (trunc b)

in case select has more than one use the exist pattern will add extra select.

i16 t1 = select c, a , b
i16 t2 = srl t1, 3
i8 t3 = trunc i16 t1 to i8
-->
i8 a1 = trunc i16 a to i8
i8 b1 = trunc i16 a to i8
i8 t3 = select c, a1 , b1
i16 t1 = select c, a , b
i16 t2 = srl t1, 3

Diff Detail

Repository
rL LLVM

Event Timeline

AsafBadouh updated this revision to Diff 77341.Nov 9 2016, 6:25 AM
AsafBadouh retitled this revision from to DAGCombiner: fix combine of trunc and select.
AsafBadouh updated this object.
AsafBadouh set the repository for this revision to rL LLVM.
AsafBadouh added a subscriber: llvm-commits.
AsafBadouh updated this revision to Diff 77345.Nov 9 2016, 6:59 AM
zvi added inline comments.Nov 9 2016, 8:05 AM
test/CodeGen/X86/select.ll
519 ↗(On Diff #77345)

Can this test be reduced to a smaller example?
Please add a comment explaining what this test checks.

AsafBadouh updated this revision to Diff 77475.Nov 10 2016, 6:06 AM
AsafBadouh added a reviewer: spatel.

+ Sanjay
fixes pr29002
update the test case using the example in pr29002

https://llvm.org/bugs/show_bug.cgi?id=29002

spatel edited edge metadata.Nov 10 2016, 6:33 AM

Thanks for tracking this down!

  1. Please use the regression test from:

https://llvm.org/bugs/show_bug.cgi?id=29002#c1

I think that's the minimal case. If you want to include the original test in this patch, I suppose that's ok, but I don't think it actually adds value. Zvi's example in:
https://llvm.org/bugs/show_bug.cgi?id=29002#c2
probably is interesting as it shows a different output?

  1. Please use utils/update_llc_test_checks.py to auto-generate complete checks (and then remove any lines that you think are irrelevant).
AsafBadouh updated this revision to Diff 77497.Nov 10 2016, 8:47 AM
AsafBadouh edited edge metadata.
  1. update the regression tests.
  2. used the utils/update_llc_test_checks.py to auto-generate complete checks.
AsafBadouh marked an inline comment as done.Nov 10 2016, 8:48 AM
zvi edited edge metadata.Nov 10 2016, 9:53 AM

Thanks, Asaf. LGTM. Please make sure you are running the tests on all targets.

This revision was automatically updated to reflect the committed changes.