This is an archive of the discontinued LLVM Phabricator instance.

Add a test case showing the instcombine fail from D52548
ClosedPublic

Authored by sheredom on Sep 26 2018, 8:19 AM.

Details

Summary

This is a splice out from D52548 to add the test case that causes the instcombine fail, with the expected output pre-fixing it (notice the 0.0f's suddenly appearing because instcombine thinks it can optimize the div/rems away).

Diff Detail

Repository
rL LLVM

Event Timeline

sheredom created this revision.Sep 26 2018, 8:19 AM
spatel added inline comments.Sep 26 2018, 9:17 AM
test/Transforms/InstCombine/stop_bad_undef_propagation.ll
122–123 ↗(On Diff #167144)

Here and below: there are spacing glitches, so this doesn't compile.

126 ↗(On Diff #167144)

Here and in all tests: do we need fast-math-flags?

130 ↗(On Diff #167144)

It's not clear what different problems each of the 3 tests per opcode are demonstrating. Please add a comment at the top of the file or give the tests a meaningful name that describes what is changing between the tests.

sheredom updated this revision to Diff 167162.Sep 26 2018, 10:42 AM
  • Added a comment explaining each of the 3 variants for each of sdiv/srem/udiv/urem
  • Fixed the whitespace issue
  • Removed the FMF flags that were a legacy from the original shader this was reduced form
sheredom marked 3 inline comments as done.Sep 26 2018, 10:43 AM
spatel accepted this revision.Sep 26 2018, 1:22 PM

LGTM - I'll remove the block label to shrink this a bit, give the values real names (makes it easier to mix and match instructions if needed), and commit on your behalf. Thanks!

This revision is now accepted and ready to land.Sep 26 2018, 1:22 PM
spatel added inline comments.Sep 26 2018, 1:26 PM
test/Transforms/InstCombine/stop_bad_undef_propagation.ll
147 ↗(On Diff #167162)

Not sure how you're generating this, but it still has at least 3 typos that prevent compilation.

This revision was automatically updated to reflect the committed changes.