[llvm-commits] Sink improvements
Duncan Sands
baldrick at free.fr
Wed May 23 04:49:10 PDT 2012
Hi Carlo,
> Index: lib/Transforms/Scalar/Sink.cpp
> ===================================================================
> --- lib/Transforms/Scalar/Sink.cpp (revisione 157154)
> +++ lib/Transforms/Scalar/Sink.cpp (copia locale)
> @@ -174,6 +175,41 @@
> return true;
> }
>
> +bool Sinking::IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo) const {
this should have a comment describing what it does.
> + if (SuccToSinkTo == NULL || Inst == NULL)
> + return false;
It looks like neither of these occurs, so how about getting rid of these or
turning it into an assertion instead.
> +
> + // It is not possible to sink an instruction into its own block. This can
> + // happen with loops.
> + if (Inst->getParent() == SuccToSinkTo)
> + return false;
> +
> + // If the block has multiple predecessors, this would introduce computation on
> + // a path that it doesn't already exist. We could split the critical edge,
that it doesn't already exist -> doesn't already exist
> + // but for now we just punt.
> + // FIXME: Split critical edges if not backedges.
> + if (SuccToSinkTo->getUniquePredecessor() != Inst->getParent()) {
> + // We cannot sink a load across a critical edge - there may be stores in
> + // other code paths.
> + if (!isSafeToSpeculativelyExecute(Inst))
> + return false;
> +
> + // We don't want to sink across a critical edge if we don't dominate the
> + // successor. We could be introducing calculations to new code paths.
> + if (!DT->dominates(Inst->getParent(), SuccToSinkTo))
> + return false;
> +
> + // Don't sink instructions into a loop.
> + Loop *succ = LI->getLoopFor(SuccToSinkTo), *cur = LI->getLoopFor(Inst->getParent());
> + if (succ != NULL && cur != NULL && succ != cur)
The LLVM style is to use 0 rather than NULL. Likewise for all the other
instances of NULL you added elsewhere in the patch.
> + return false;
> + }
> +
> + // Finally, check that we all the uses of the instruction are actually
that we all the -> that all the
> + // dominated by the candidate
> + return AllUsesDominatedByBlock(Inst, SuccToSinkTo);
> +}
> +
> /// SinkInstruction - Determine whether it is safe to sink the specified machine
> /// instruction out of its current block into a successor.
> bool Sinking::SinkInstruction(Instruction *Inst,
The rest looks OK to me (except for the use of NULL).
Ciao, Duncan.
More information about the llvm-commits
mailing list