[PATCH] D120329: [SelectionDAG] Emit calls to __divei4 and friends for division/remainder of large integers

Matthias Gehre via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 11 07:39:22 PST 2022


mgehre-amd added inline comments.


================
Comment at: llvm/include/llvm/IR/RuntimeLibcalls.def:50
 HANDLE_LIBCALL(MUL_I128, "__multi3")
+HANDLE_LIBCALL(MUL_IEXT, nullptr)
+
----------------
LuoYuanke wrote:
> Why set mul? Is there any test case for it?
see my other comment


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2110
+  default:
+    LC = Call_IEXT;
+    break;
----------------
aaron.ballman wrote:
> LuoYuanke wrote:
> > Do we support bitsize that is no power of 2 (e.g., MVT::i1)? Should we check if the bit size exceed 128?
> Yes, `_BitInt` can be of any width: https://godbolt.org/z/4zGfrosc7
It seems that SelectionDAG through some magic always first expands the integer type to a power of two before coming here and expanding into a lib call.

So i1 would be expanded into i8. For this reason, only divisions on types bigger than i128 currently crash.

For types bigger than i128, SelectionDAG also expands them to the next power of two, i.e. i129 becomes i256.
I didn't want to change this, because it makes implementing the lib function easier. 

But for efficiency, we can try to disable this and pass the original non-power-of-2 type to the libcall in a future PR.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4346
     break;
   case ISD::MUL:
+    Results.push_back(ExpandIntLibCall(
----------------
LuoYuanke wrote:
> Why touch MUL? Can't MUL be splitted by Codegen? 
Yes, MUL will be splitted by codegen and never get here afaik, but I still needed to add a MUL_IEXT constant
because this uses the same `ExpandIntLibCall` which is used for the division/remainder.
And I had to extend ExpandIntLibCall by an extra argument to handle the > 128 bit case.

I'm not really sure why this code is here at all, and whether MUL is actually expanded to a libcall on any target.

I'm open for suggestions, I don't know a lot about SelectionDAG.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:3990
+  else {
+    SDValue Result =
+        passArguments_DIVREM(TLI, RTLIB::SDIV_IEXT, DAG, N, dl, VT);
----------------
LuoYuanke wrote:
> Do we need to check type bit size exceed 128?
Yes, but there is already an assert in passArguments_DIVREM (now called ExpandExtIntRes_DIVREM).


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:3991
+    SDValue Result =
+        passArguments_DIVREM(TLI, RTLIB::SDIV_IEXT, DAG, N, dl, VT);
+    SplitInteger(Result, Lo, Hi);
----------------
LuoYuanke wrote:
> Is expandExtIntRes_DIVREM more readable as the function name?
yes, I'll rename it


================
Comment at: llvm/test/CodeGen/X86/udivmodei5.ll:9
+; X86-NEXT:    pushl %ebp
+; X86-NEXT:    .cfi_def_cfa_offset 8
+; X86-NEXT:    .cfi_offset %ebp, -8
----------------
LuoYuanke wrote:
> Add nounwind to avoid generating cfi instruction.
Great, thanks! I will add it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120329



More information about the llvm-commits mailing list