[PATCH] D127126: [SelectionDAG] Remove invalid TypeSize conversion from PromoteIntRes_BITCAST.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:378
+  /// will result in a value whose size matches our own.
+  bool hasKnownScalarFactor(const LinearPolySize &RHS) const {
+    return isScalable() == RHS.isScalable() &&
----------------
This seems to be roughly doing the same thing as `isKnownMultipleOf` from `LinearPolySize`. Should these interfaces be merged? If so, I prefer `isKnownMultipleOf` because it's a more intuitive name.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:385
+  /// value whose size matches our own.
+  ScalarTy getKnownScalarFactor(const LinearPolySize &RHS) const {
+    assert(hasKnownScalarFactor(RHS) && "Expected RHS to be a known factor!");
----------------
It  might actually be nice if this could return an Optional, so that you could write:

  if (Optional<unsigned> F = EC.getCommonFactor(OtherEC)) {
    // do something with F
  }

bikeshedding: `getCommonFactor` would have been a better name IMO, even if it's perhaps less accurate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127126