Page MenuHomePhabricator

[SCEV] Infer known bits from known sign bits
ClosedPublic

Authored by reames on Feb 19 2021, 9:45 AM.

Details

Summary

This was suggested by lebedev.ri over on D96534.

This seems like a reasonable change, but I haven't been able to find a test case which actually changes output. All of the obvious ones are caught through the existing ashr constant range trick for signed ranges. Despite this, I think this is reasonable to land if reviewers agree.

Diff Detail

Event Timeline

reames created this revision.Feb 19 2021, 9:45 AM
reames requested review of this revision.Feb 19 2021, 9:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 9:45 AM

I'm not really surprised that it doesn't affect any tests, but does it affect any code out there?
If you can pinpoint some original source file that produces different final .ll/.s, a test case can be reduced trivially.

reames added a comment.EditedMar 3 2021, 9:29 AM

I'm not really surprised that it doesn't affect any tests, but does it affect any code out there?
If you can pinpoint some original source file that produces different final .ll/.s, a test case can be reduced trivially.

I'm not motivated to go run a broad set of benchmarks here. I'm doing it only to address your review comment. If you don't have a motivating case (or are okay landing without one), I'd prefer to abandon the patch.

(reworded comment - realized my original wording was poor)

lebedev.ri accepted this revision.Mar 9 2021, 10:28 AM

So, i've looked, and this does not cause any binary differences as of plain vanilla test-suite+RawSpeed.
That being said, the change is rather straight-forward, and is practically free,
so i think we could just land this, even that isn't great..

This revision is now accepted and ready to land.Mar 9 2021, 10:28 AM
This revision was landed with ongoing or failed builds.Mar 9 2021, 12:37 PM
This revision was automatically updated to reflect the committed changes.