[PATCH] D126644: [llvm/CodeGen] Add ExpandLargeDivRem pass

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/ExpandLargeDivRem.cpp:47
+    case Instruction::SRem: {
+      auto *IntTy = dyn_cast<IntegerType>(I.getType());
+      if (!IntTy || IntTy->getIntegerBitWidth() <= ExpandDivRemBits)
----------------
This won't handle vectors


================
Comment at: llvm/test/Transforms/ExpandLargeDivRem/urem129.ll:13
+; CHECK-NEXT:    [[A:%.*]] = load i129, i129* [[PTR:%.*]], align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = call i129 @__llvm_urem129(i129 [[A]], i129 3)
+; CHECK-NEXT:    store i129 [[TMP1]], i129* [[OUT:%.*]], align 4
----------------
This looks out of date?


================
Comment at: llvm/test/Transforms/ExpandLargeDivRem/values129.ll:6
+; constant evaluating the result.
+; RUN: opt -S -O2 < %s | FileCheck %s
+; RUN: opt -S -expandlargedivrem < %s | opt -unroll-count=129 -inline-threshold=100000 -O2 -S | FileCheck %s
----------------
Shouldn't use -O2 in a test like this


================
Comment at: llvm/test/Transforms/ExpandLargeDivRem/values129.ll:21
+;
+  %ret = call {i129, i129} @udivrem(i129 1, i129 1)
+  ret {i129, i129} %ret
----------------
I'm not sure cases that constant fold are the most helpful tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126644