This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Avoid folding `index.remu` and `index.rems` for 0 rhs
ClosedPublic

Authored by rikhuijzer on May 25 2023, 11:21 AM.

Details

Summary

As discussed in https://github.com/llvm/llvm-project/issues/59714#issuecomment-1369518768, the folder for the remainder operations should be resillient when the rhs is 0.
The file IndexOps.cpp was already checking for multiple divisions by zero, so I tried to stick to the code style from those checks.

Fixes #59714.

As a side note, is it correct that remainder operations are never optimized away? I would expect that the following code

func.func @remu_test() -> index {
  %c3 = index.constant 2
  %c0 = index.constant 1
  %0 = index.remu %c3, %c0
  return %0 : index
}

would be optimized to

func.func @remu_test() -> index {
  return index.constant 0 : index
}

when called with mlir-opt --convert-scf-to-openmp temp.mlir, but maybe I'm misunderstanding something.

Diff Detail

Event Timeline

rikhuijzer created this revision.May 25 2023, 11:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2023, 11:21 AM
rikhuijzer requested review of this revision.May 25 2023, 11:21 AM
rikhuijzer edited the summary of this revision. (Show Details)May 25 2023, 11:22 AM
rikhuijzer edited the summary of this revision. (Show Details)May 25 2023, 11:29 AM
rikhuijzer edited the summary of this revision. (Show Details)
Mogball accepted this revision.May 31 2023, 8:06 AM
This revision is now accepted and ready to land.May 31 2023, 8:06 AM

Thanks for your review, Jeff! Could you maybe also land the patch? I don't have permissions.

Sure. I can do that.

As a side note, is it correct that remainder operations are never optimized away? I would expect that the following code

They should get folded, but not necessarily by all conversion passes.

This revision was automatically updated to reflect the committed changes.