[PATCH] D71657: [SDAG] Handle BUILD_PAIR in ComputeNumSignBits

Mon Dec 23 08:02:56 PST 2019

danilaml marked an inline comment as done.
danilaml added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3461
+  case ISD::BUILD_PAIR:
+    Tmp = ComputeNumSignBits(Op.getOperand(1), Depth + 1);
+    if (Op.getOperand(1).getValueSizeInBits() == Tmp)
----------------
craig.topper wrote:
> I don't think this is correct. If the first call returns "all sign bits" then we check operand(0) that call will always return at least 1. So we increase the total by at least 1. So its impossible for this code to ever return that the number of sign bits of a build_pair is exactly half the total size of the build_pair. That seems wrong.
Good catch. Do you know of any way to recover/query whether one value is a replicated sign bit of another?
Computing the lo part sign bits is crucial to trigger proper (u)smul_hilo transformations latter on, so missing this info seems unfortunate.
It looks like the relevant info is there when copyFromRegs are generated, but I couldn't find a way to represent it in the DAG similarly to Assert*ext (other than breaking the semantics somewhat and using Assert* with Lo part node).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71657