Page MenuHomePhabricator

[DAG] Fix for Bug PR34620 - Allow SimplifyDemandedBits to look through bitcasted constants

Authored by sameconrad on Dec 30 2017, 7:35 PM.



Allow SimplifyDemandedBits to use TargetLoweringOpt::computeKnownBits to look through bitcasted constants. This can help simplifying in some cases where bitcasts of constants generated during or after legalization can't be folded away, and thus didn't get picked up by SimplifyDemandedBits. This fixes PR34620, where a redundant pand created during legalization from lowering and lshr <16xi8> wasn't being simplified due to the presence of a bitcasted build_vector as an operand.

Diff Detail


Event Timeline

sameconrad created this revision.Dec 30 2017, 7:35 PM
RKSimon added inline comments.Dec 31 2017, 9:15 AM
1225 ↗(On Diff #128362)

Would this catch more if we used isConstantOrConstantVector from DAGCombiner.cpp (we'd have to move it to SelectionDAG.h or similar)?

craig.topper added inline comments.
1225 ↗(On Diff #128362)

Why do we need to the constant qualifier at all? Isn't it generally useful to get any known bits from the other side of the bitcast?

RKSimon added inline comments.Jan 1 2018, 4:01 AM
1225 ↗(On Diff #128362)

+1 Removing the isConstOrConstSplat condition entirely unearths a number of other improvements to various tests.

sameconrad updated this revision to Diff 128395.Jan 1 2018, 2:39 PM

Updated to remove isConstOrConstSplat condition

sameconrad added inline comments.Jan 1 2018, 2:45 PM
1225 ↗(On Diff #128362)

I've updated to remove the isConstOrConstSplat condition. I originally added that because I was worried about hurting build times too much (since computeKnownBits can get pretty expensive), but if it unearths improvements to a lot of tests then it makes sense to remove the check. I'll work on updating the tests that are affected.

sameconrad marked 2 inline comments as done.Jan 1 2018, 2:45 PM

@sameconrad Any update on the test fixes? I have a local version of the patch if its useful

sameconrad updated this revision to Diff 128862.Jan 6 2018, 6:09 PM

Fix tests that failed after removing isConstOrConstSplat condition

@sameconrad Any update on the test fixes? I have a local version of the patch if its useful

Was a bit busy this week, just uploaded the fixes.

sameconrad updated this revision to Diff 128865.Jan 6 2018, 7:13 PM

Remove unnecessary whitespace changes

RKSimon accepted this revision.Jan 7 2018, 2:26 AM

LGTM - thanks

This revision is now accepted and ready to land.Jan 7 2018, 2:26 AM

Thanks. I don't have commit access, so someone will have to commit this.

This revision was automatically updated to reflect the committed changes.