This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Arith] Pass to switch signed ops for equivalent unsigned ones

Authored by krzysz00 on Apr 19 2022, 11:01 AM.



If all the arguments to and results of an operation are known to be
non-negative when interpreted as signed (which also implies that all
computations producing those values did not experience signed
overflow), we can replace that operation with an equivalent one that
operates on unsigned values.

Such a replacement, when it is possible, can provide useful hints to
backends, such as by allowing LLVM to replace remainder with bitwise
operations in more cases.

Depends on D124022

Depends on D124023

Diff Detail

Event Timeline

krzysz00 created this revision.Apr 19 2022, 11:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2022, 11:01 AM
krzysz00 requested review of this revision.Apr 19 2022, 11:01 AM
krzysz00 updated this revision to Diff 424584.Apr 22 2022, 12:42 PM
krzysz00 edited the summary of this revision. (Show Details)

Adjust for new range struct

krzysz00 updated this revision to Diff 426782.May 3 2022, 11:01 AM

Update for changes to previous revisions

krzysz00 updated this revision to Diff 428163.May 9 2022, 12:29 PM

Update for interface movement

krzysz00 updated this revision to Diff 428190.May 9 2022, 1:34 PM

Update to new IntRangeAnalysis API

Mogball accepted this revision.May 11 2022, 9:01 PM
Mogball added a subscriber: Mogball.
Mogball added inline comments.

can you add a description and add an example?


can you stick this description on the pass class or runOnOperation?


brief comments on this helpers?

This revision is now accepted and ready to land.May 11 2022, 9:01 PM
krzysz00 updated this revision to Diff 429034.May 12 2022, 11:36 AM
krzysz00 marked 2 inline comments as done.

Address review feedback, add longer description.

krzysz00 updated this revision to Diff 430448.May 18 2022, 10:40 AM

Update for previous changes.

Mogball accepted this revision.Jun 14 2022, 11:54 AM
This revision was landed with ongoing or failed builds.Jun 14 2022, 2:18 PM
This revision was automatically updated to reflect the committed changes.
bkramer added inline comments.

I wonder if it would've been easier to express this using the DialectConversion infrastructure. Mark signed ops dynamically illegal and do the rewriting with actual patterns.

krzysz00 added inline comments.Jun 15 2022, 8:28 AM

That is a good point, yeah. Dialect conversion gives you access to the original op, let me rewrite

Looking at this again, I'm not sure why I hit the approve button so fast. There's a bunch of little things and also that this couldve been done with pattern matching. I see the review date as may 11 and I think I was 30 hours without sleep at the time....

Should cleaning this up be a new revision or is there a way to re-open this one?

Just clean up the existing code and send a new patch