[PATCH] D56214: AggressiveInstCombine: Fold full mul i64 x i64 -> i128

[Arthur O'Dwyer via Phabricator via llvm-commits]

Quuxplusone added inline comments.


================
Comment at: lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:391
+  const auto LowMask =
+      m_SpecificInt(~uint64_t{0} >> ((MaxSizeInBits / 2) - HalfSizeInBits));
+
----------------
Nit: This expression isn't intuitively clear to me. Also, I would write `uint64_t{0}` as `uint64_t(0)`.

I think you're getting really lucky here that `MaxSizeInBits / 2` just happens to be the same number of bits (64) as the width of `uint64_t{0}`; otherwise this math would be wrong.

How about

    assert(HalfSizeInBits <= 64);
    const auto LowMask = m_SpecificInt((uint64_t(1) << (HalfSizeInBits-1) << 1) - 1);

or if there's an existing utility function to compute `uint64_t(1) << HalfSizeInBits` directly.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56214