[PATCH] D65144: [InstCombine] Fold '((%x * %y) u/ %x) != %y' to '@llvm.umul.with.overflow' + overflow bit extraction
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 23 10:27:40 PDT 2019
lebedev.ri added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3378
+ if (Mul)
+ Builder.SetInsertPoint(Mul);
+
----------------
lebedev.ri wrote:
> xbolva00 wrote:
> > Also add multi use check?
> >
> > bool HasMultiUseMul = Mul && !Mul->hasOneUse();
> > if (..)
> >
> >
> If we insert in place of old `mul` only if `mul` had extra uses,
> then maybe this intrinsic will still be hoisted back into that
> other basic block because it's loop-invariant.
> Or maybe it we always place insert in place of old `mul`
> it will get sinked into some other only basic block where it's used.
>
> I'm honestly not sure what the best solution here is,
> we just don't have enough information to make the final
> (the one that won't be altered by later passes) choice here.
> Either choice that passes `-verify` is good.
>
> Consistently placing it in place of the old `mul` //seemed// least arbitrary to me.
>
> Maybe others have some other ideas?
> Maybe others have some other ideas?
Err, what i meant to say is, maybe there is some consistent justification that points
to some particular location being the correct one to insert to?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65144/new/
https://reviews.llvm.org/D65144
More information about the llvm-commits
mailing list