This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] For pow(x, +/-0.5), stop falling into pow(x, 1.5), etc. case
ClosedPublic

Authored by hubert.reinterpretcast on Sep 21 2020, 9:06 PM.

Details

Summary

The current code for handling pow(x, y) where y is an integer plus 0.5 is not explicitly guarded against attempting to transform the case where abs(y) is exactly 0.5.

The latter case is meant to be handled by replacePowWithSqrt. Indeed, if the pow(x, integer+0.5) case proceeds past a certain point, it will hit an assertion by attempting to form pow(x, 0) using getPow.

This patch adds an explicit check to prevent attempting the pow(x, integer+0.5) transformation on pow(x, +/-0.5) as suggested during the review of D87877. This has the effect of retaining the shrinking of pow to powf when the sqrt libcall cannot be formed.

Diff Detail

Unit TestsFailed

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2020, 9:06 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
hubert.reinterpretcast requested review of this revision.Sep 21 2020, 9:06 PM
spatel accepted this revision.Sep 22 2020, 6:15 AM

LGTM

llvm/test/Transforms/InstCombine/pow-4.ll
240–241

That looks like another bug. We formed an extra sqrtf() call from a not-quite-dead pow() call?

This revision is now accepted and ready to land.Sep 22 2020, 6:15 AM
llvm/test/Transforms/InstCombine/pow-4.ll
240–241

I'm not sure I'll be able to chase this one. Do you have a recommendation on how to mark/track it?

spatel added inline comments.Sep 22 2020, 6:46 AM
llvm/test/Transforms/InstCombine/pow-4.ll
240–241

A 'FIXME' comment in the test file should be ok. Bonus points for filing a bugzilla. :)

This revision was landed with ongoing or failed builds.Sep 22 2020, 11:23 AM
This revision was automatically updated to reflect the committed changes.