[llvm] r353038 - [NFC] Make a check in GuardWidening more obvious

Maxim Kazantsev via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 4 20:20:36 PST 2019


Hi Philip,

I am not entirely getting your point. After we have implemented elimination of non-guard branches by widening their conditions into dominating guards some time ago, Instr here is not not always a guard. Therefore, it is not present in GuardsInCurBB. And we cannot take an iterator on it.

I think it is logical to do this query iff instr is a guard, because otherwise, we will find nothing. We know that Instr can only be a terminator when it's not a guard, and therefore, we don't need any adjustment of E because all other instructions dominate the terminator.

--Max

-----Original Message-----
From: Philip Reames <listmail at philipreames.com> 
Sent: Tuesday, February 5, 2019 2:22 AM
To: Maxim Kazantsev <max.kazantsev at azul.com>; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r353038 - [NFC] Make a check in GuardWidening more obvious

Max,

I don't think this has exactly the effect you indended.  It doesn't break anything, but it does slow down the code.  The purpose of the original terminator check was to avoid doing the extra work when the guard was the terminator.

In fact, the whole find is overkill.  You should be able to use getIterator on the Instruction.

Would you mind making that simplification, and removing the isGuard check entirely?

Philip

On 2/4/19 2:41 AM, Max Kazantsev via llvm-commits wrote:
> Author: mkazantsev
> Date: Mon Feb  4 02:41:17 2019
> New Revision: 353038
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353038&view=rev
> Log:
> [NFC] Make a check in GuardWidening more obvious
>
> Modified:
>      llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/G
> uardWidening.cpp?rev=353038&r1=353037&r2=353038&view=diff
> ======================================================================
> ========
> --- llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp Mon Feb  4 
> +++ 02:41:17 2019
> @@ -375,7 +375,7 @@ bool GuardWideningImpl::eliminateInstrVi
>   
>       assert((i == (e - 1)) == (Instr->getParent() == CurBB) && "Bad 
> DFS?");
>   
> -    if (Instr->getParent() == CurBB && CurBB->getTerminator() != Instr) {
> +    if (Instr->getParent() == CurBB && isGuard(Instr)) {
>         // Corner case: make sure we're only looking at guards strictly dominating
>         // GuardInst when visiting GuardInst->getParent().
>         auto NewEnd = std::find(I, E, Instr);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list