[PATCH] D150443: [KnownBits] Define and use meet and join operations

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 12 08:21:05 PDT 2023


goldstein.w.n added inline comments.


================
Comment at: llvm/include/llvm/Support/KnownBits.h:297
+  /// Returns the greatest lower bound of this (aka LHS) and RHS, i.e. the
+  /// information that is known to be true for both LHS and RHS.
+  ///
----------------
Personally the text after the 'i.e' seems more useful for understanding the code (and I think matches the motivation for the code better) so I would swap the order.

'Returns the information that is known to be true for both this (aka LHS) and RHS. Another way to look at it is the greatest lower bound of LHS and RHS.'

Likewise below.


================
Comment at: llvm/include/llvm/Support/KnownBits.h:310
+  }
+  static KnownBits meet(const KnownBits &LHS, KnownBits &&RHS) {
+    RHS.meet(LHS);
----------------
Since we already have `KnownBits::commonBits` maybe instead of adding the new essentially equivilent `meet` API, we could just add a non-static version of `commonBits` (so you could do `Known.commonBits(Known2)`). Seems like an all around simpler change and probably more familiar for people who have used the interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150443



More information about the llvm-commits mailing list