[PATCH] D24565: [InstCombine] PR30366 : Teach the udiv folding logic how to handle constant expressions.

[Andrea Di Biagio via llvm-commits]

andreadb added a comment.

Thanks for the feedback David!


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1002
@@ +1001,3 @@
+  Value *N;
+  match(ShiftLeft, m_Shl(m_APInt(CI), m_Value(N)));
+  if (*CI != 1)
----------------
majnemer wrote:
> andreadb wrote:
> > majnemer wrote:
> > > You should check if match fails.
> > In theory, this check should never fail because visitUDivOperand (see lines 1036:1040) always checks that Op1 is a left shift of a power_of_2 before adding 'foldUDivShl' to the set of folding actions.
> > 
> > Would it be okay if I add an assertion failure if the call to match returns false?
> Yes, something like:
>   if (!match(...))
>     llvm_unreachable("match shouldn't fail here!");
I will add that check.

================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1004-1005
@@ +1003,4 @@
+  if (*CI != 1)
+    N = IC.Builder->CreateAdd(N, ConstantInt::get(N->getType(),
+                              CI->logBase2()));
+  if (Op1 != ShiftLeft)
----------------
majnemer wrote:
> This looks strangely formatted.
Sorry, I forgot to clang-format this patch. I will fix it.


https://reviews.llvm.org/D24565