[PATCH] D50501: [LICM] Hoist guards with invariant conditions
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 14 02:45:56 PDT 2018
mkazantsev added inline comments.
================
Comment at: lib/Transforms/Scalar/LICM.cpp:523
+ CurLoop->hasLoopInvariantOperands(&I)) {
+ bool Hoisted = hoist(I, DT, CurLoop, SafetyInfo, ORE);
+ Changed |= Hoisted;
----------------
reames wrote:
> mkazantsev wrote:
> > reames wrote:
> > > hoist always returns true. Separately, change it to return void and update other caller.
> > Not sure if it's a good idea... What if it changes in the future? Besides, `sink` can actually return `false`, and it would be strange to have different signatures for these two.
> This was not a suggestion. Please make the change. It can be done post commit, but I want this cleanup done. At the moment, you have control flow in this patch which is never executed and introduces non-trivial reasoning complexity for no value.
>
> In general, the person who changes code in the future has to think about the implications of their changes. Future proofing never works and just adds complexity now.
https://reviews.llvm.org/D50696
https://reviews.llvm.org/D50501
More information about the llvm-commits
mailing list