[PATCH] D83153: [DAGCombiner] Prevent regression in isMulAddWithConstProfitable

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15750
+    // are too large.
+    if (ConstNode.getScalarValueSizeInBits() > 8 * sizeof(int64_t))
+      return true;
----------------
This should be `>=`


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15758
+        int Bits = 8 * sizeof(int64_t) - ConstNode.getScalarValueSizeInBits();
+        int64_t C1C2 = ((C1 * C2) << Bits) >> Bits;
+        // This transform will introduce regression, if c1 is legal add
----------------
C1 * C2 may trigger a signed overflow. (C1 * C2) may be negative and left shifting a signed integer is UB before C++20. 

Please use llvm::SignExtend64(uint64_t, unsigned)


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

https://reviews.llvm.org/D83153