[PATCH] D110508: [SelectionDAG] Fix shift libcall ABI mismatch in shift-amount argument

[Itay Bookstein via Phabricator via llvm-commits]

nextsilicon-itay-bookstein added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4001
+        EVT::getIntegerVT(*DAG.getContext(), DAG.getLibInfo().getIntSize());
+    SDValue ShAmt = DAG.getAnyExtOrTrunc(N->getOperand(1), dl, ShAmtTy);
+    SDValue Ops[2] = {N->getOperand(0), ShAmt};
----------------
bjope wrote:
> AnyExt seem a bit weird as I assume the lib function will use all bits. I think getZExtOrTrunc would be a more correct choice here.
I agree that AnyExt is incorrect, however I'm wondering whether it should be ZExt or SExt. The source-level semantics seem to point towards SExt, because the libcalls are defined with a signed shift amount (`si_int` as opposed to `su_int`). Earlier code in this function uses ZExt (under the `LegalOrCustom && TLI.shouldExpandShift(DAG, N)` case), but it doesn't need to concern itself with the signature for `__ashlti3` and friends. Note that this actually affects the output for `llvm/test/CodeGen/X86/shift_minsize.ll` functions `shl128, ashr128, lshr128`, in whether the `movzbl` is emitted or not.


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

https://reviews.llvm.org/D110508