[PATCH] D76446: [ConstantRange] Use APInt::or/APInt::and for single elements.

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 19 16:28:07 PDT 2020


nikic added a comment.

In D76446#1932076 <https://reviews.llvm.org/D76446#1932076>, @fhahn wrote:

> In D76446#1932013 <https://reviews.llvm.org/D76446#1932013>, @nikic wrote:
>
> > I'd like to see a unit test that exhaustively tests this for all binary operators (or the ones where it holds). I don't think single element ranges are handled accurately for all the ops (e.g. probably not for urem/srem?)
>
>
> Yes, ideally we would handle all single element ranges in ConstantRange::binaryOp directly, by delegating to APInt, rather than doing it separately for each one I think. Maybe we should add an APInt::binaryOp?


I don't think that's possible due to layering. APInt is in Support, so it can't depend on IR.

> Or just dispatch in ConstantRange::binaryOp?

We probably don't want to make binaryOp() more powerful than the individual methods, so I think your current approach is already fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76446/new/

https://reviews.llvm.org/D76446





More information about the llvm-commits mailing list