[PATCH] D30044: [ARM] Enable SMLAL[B|T] instruction selection

Sam Parker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 22 08:53:23 PST 2017


samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMPatternHelpers.cpp:45-60
+bool isBottomS16(const SDValue &Op, SelectionDAG *CurDAG) {
+  if (isSRA16(Op))
+    return isSHL16(Op.getOperand(0));
+
+  return CurDAG->ComputeNumSignBits(Op) == 17;
+}
+
----------------
efriedma wrote:
> samparker wrote:
> > t.p.northover wrote:
> > > This combination looks extremely dodgy to me.  ComputeNumSignBits makes no promises about how it's going to find those bits, and in fact traverses nodes down to a depth of 6.
> > > 
> > > You've already found two cases where the final SDNode's operand(0) isn't actually the unextended node, I see no reason to think that's all there is.
> > Ah yes... Do you think it's worth adding a function to SelectionDAG that returns the SDValue that ComputeNumSignBits finds and uses for its calculation?
> The function you want already exists; it's called SimplifyDemandedBits.
Great, I'll look into it. Thanks


https://reviews.llvm.org/D30044





More information about the llvm-commits mailing list