Page MenuHomePhabricator

[mlir][arith] cmpi: move constant to the right side
ClosedPublic

Authored by Hardcode84 on Jul 16 2022, 3:33 AM.

Details

Summary

Convert arith.cmpi to the canonical form with constants on the right side
to simplify further optimizations and open more opportunities for CSE.

Diff Detail

Event Timeline

Hardcode84 created this revision.Jul 16 2022, 3:33 AM
Hardcode84 requested review of this revision.Jul 16 2022, 3:33 AM

Change in sccp-structured test is suspicious, I'm not sure what this test is supposed to check and why this change is needed (scf.while didn't get removed without it). Can someone help me with this?

This comment was removed by bondhugula.
mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp
1337

wil -> will

1337

Side note: The other invertPredicate should, I believe, be renamed to negatePredicate -- it's negating, not inverting. (<= would become > -- it's not just == and !=).

1345

auto -> Pred

1360

This can now be updated (simplified). Since you've moved the constant to the right, you only need to check for the lhs not being a constant! If the lhs is a constant, the rhs will guaranteed to be a constant at this stage given your change above.

mlir/test/Dialect/Arithmetic/canonicalize.mlir
130

Nit: Full stop at the end.

LGTM - this is a welcome change. You can eliminate an additional check in the folder further below given this canonicalization; please see comment below.

review comments

Hardcode84 marked 4 inline comments as done.Jul 16 2022, 10:49 AM
bondhugula accepted this revision.Jul 16 2022, 11:22 AM

LGTM - please expand the commit summary to more importantly mention that this puts the ops in a canonical form in the presence of one constant. CSE and other ease in optimizing/checking follows from that.

mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp
1363

Nit: Please punctuate appropriately. (semi-colon or full stop after side.)
https://llvm.org/docs/CodingStandards.html#commenting

This revision is now accepted and ready to land.Jul 16 2022, 11:22 AM

fix comment

Hardcode84 edited the summary of this revision. (Show Details)Jul 16 2022, 1:43 PM
bondhugula accepted this revision.Jul 17 2022, 7:21 PM
Hardcode84 edited the summary of this revision. (Show Details)

rebase, fix flang tests

Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2022, 2:42 PM
This revision was automatically updated to reflect the committed changes.