Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARITH] Constraint-aware ConstIntBound, Enhance CanonicalSimplify #3132

Merged
merged 2 commits into from
May 4, 2019

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented May 2, 2019

Enhance constraints for const-int-bound, so that we can do the following simplification, that considers the constraints in the if_then_else condition. Enhance CanonicalSimplify to try to simplify base during mod.

if_then_else(x >= 10, (x - 10) %2, x) =>if_then_else(x >= 10, x %2, x)

This PR will likely improve or resolve condition brought in #3097.

@tqchen
Copy link
Member Author

tqchen commented May 2, 2019

@merrymercy @yzhliu @sgrechanik-h @kazum @kevinthesun please help review this PR

auto ret2 = DetectBoundInfo(y.Eval());
ret1.insert(ret1.end(), ret2.begin(), ret2.end());
return ret1;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need the x == c (and c == x) case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very sure if this is a case that we will need as frequently, so I decided to leave it out after some thought. We can revisit later. This might be better suited for a condition in a simplifier. Most conditions like x % 3== 1 is detected by modular analysis.

src/arithmetic/rewrite_simplify.cc Show resolved Hide resolved
@tqchen tqchen merged commit 48c9237 into apache:master May 4, 2019
@tqchen
Copy link
Member Author

tqchen commented May 4, 2019

Thanks, @sgrechanik-h @yzhliu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants