This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Don't subject ISD:Constant to the depth limit in TargetLowering::SimplifyDemandedBits.
ClosedPublic

Authored by craig.topper on Oct 16 2017, 11:55 AM.

Details

Summary

We shouldn't recurse any further but it doesn't mean we shouldn't be able to give the known bits for a constant. The caller would probably like that we always return the right answer for a constant RHS. This matches what InstCombine does in this case.

I don't have a test case because this showed up while trying to revive D31724.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Oct 16 2017, 11:55 AM
arsenm added a subscriber: arsenm.Oct 16 2017, 11:59 AM
arsenm added inline comments.
lib/CodeGen/SelectionDAG/TargetLowering.cpp
519 ↗(On Diff #119189)

Should also apply to ConstantFP?

Doesn't look like SimplifyDemandedBits or computeKnownBits currently handling ConstantFP. We probably don't cross any fp->integer boundaries when recursing. I definitely see an early out in ISD::BITCAST handling if its a cast from FP.

RKSimon edited edge metadata.Oct 16 2017, 2:19 PM

Doesn't look like SimplifyDemandedBits or computeKnownBits currently handling ConstantFP. We probably don't cross any fp->integer boundaries when recursing. I definitely see an early out in ISD::BITCAST handling if its a cast from FP.

I think it's mostly as we don't have a use case for handling known bits of floats, so we just early out.

RKSimon accepted this revision.Oct 20 2017, 7:14 AM

LGTM - worth doing the same for SelectionDAG::ComputeNumSignBits and SelectionDAG::computeKnownBits ?

This revision is now accepted and ready to land.Oct 20 2017, 7:14 AM
This revision was automatically updated to reflect the committed changes.