[PATCH] D83640: [PredicateInfo] Add a common method to interpret predicate as cmp constraint

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 19 05:17:58 PDT 2020


fhahn added a comment.

In D83640#2159284 <https://reviews.llvm.org/D83640#2159284>, @nikic wrote:

> Return Optional<PredicateConstraint>.
>
> What do you think about this variant? Something I have in mind here as a possible followup is to change the way we represent predicates away from storing the raw condition, which has renaming issues we only partially mitigated with RenamedOp, towards storing the PredicateConstraint introduced here directly. This makes sure that we cannot go out of sync between the PredicateInfo construction logic and the retrieval of the condition. (For now all conditions have the form of "cmp", but if needed this could be made polymorphic in the future.)


Thanks, that looks good.

Just referring to a condition in the IR makes things more flexible in theory I think, e.g. if the OtherOp part of the condition is modified after PredicateInfo construction. The current users (NewGVN, SCCP) only do modifications after all analysis is finished, so that would not  matter for them. If there are clients in the future that really need this, it's probably fine to think about it then.


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

https://reviews.llvm.org/D83640





More information about the llvm-commits mailing list