[llvm] r219223 - Two case switch to select optimization

[Hal Finkel]

Marcello,

Multiple incoming edges are allowed, so long as the corresponding value is the same. Different values from the same predecessor are not permitted. If there is some code that does not handle this correctly, it will need to be fixed.

 -Hal

----- Original Message -----
> From: "Marcello Maggioni" <hayarms at gmail.com>
> To: "Marcello Maggioni" <hayarms at gmail.com>, llvm-commits at cs.uiuc.edu
> Sent: Sunday, October 12, 2014 6:44:54 PM
> Subject: Re: [llvm] r219223 - Two case switch to select optimization
> 
> 
> 
> I did a fast research into the LLVM base code and seems like, even if
> having multiple incoming edges for the same block is forbidden by
> the verifier through a check at line Verifier.cpp:1146, it is a
> common place assumption in SimplifyCFG that if a block branches to
> the same target multiple times the PHI should have multiple incoming
> edges from the same block.
> 
> 
> I find this pretty strange ... I tried to remove the source of double
> incoming edges for the specific case in the example posted by Joerg,
> but doing so breaks other code that EXPECTS multiple incoming edges
> for the same block in certain conditions (like a conditional branch
> targeting the same block two times with both the True and False
> branches).
> 
> 
> If this is the direction we want to go (continuing to support
> multiple incoming edges from the same block) tomorrow I will post a
> patch for switch to select that takes into consideration this when
> multiple cases of a switch target the same block and I will add Hans
> and Jin as reviewers.
> 
> 
> Cheers,
> Marcello
> 
> 
> 
> 
> 
> 2014-10-12 14:40 GMT-07:00 Marcello Maggioni < hayarms at gmail.com > :
> 
> 
> 
> Hmm,
> 
> 
> compiling your test I've noticed that even before entering the
> SwitchToSelect optimization the PHI that breaks the module already
> has two incoming edges for the same predecessor:
> 
> 
> This is the IR just entering SwitchToSelect:
> 
> 
> 
> 
> ; Function Attrs: nounwind ssp uwtable
> 
> define i32 @fn1() #0 {
> 
> entry:
> 
> %0 = load i32* @b, align 4
> 
> %tobool = icmp eq i32 %0, 0
> 
> br i1 %tobool, label %if.end3, label %if.then
> 
> 
> 
> 
> if.then: ; preds = %entry
> 
> %1 = load i32* @a, align 4
> 
> switch i32 %1, label %if.end3 [
> 
> i32 5, label %return
> 
> i32 0, label %return
> 
> ]
> 
> 
> 
> 
> if.end3: ; preds = %if.then, %entry
> 
> br label %return
> 
> 
> 
> 
> return: ; preds = %if.then, %if.then, %if.end3
> 
> %retval.0 = phi i32 [ 0, %if.end3 ], [ %1, %if.then ], [ %1, %if.then
> ]
> 
> ret i32 %retval.0
> 
> }
> 
> 
> Is this condition expected to be handled by the switch lowering? Or
> is the switch formation that probably introduced this malformed?
> Is pretty easily to workaround this in SwitchToSelect to reiterate
> Incoming Edge deletion when the switch is removed until no more
> edges from the same predecessor are found, but I guess if we have
> some other issue more up in the pipeline.
> 
> 
> 2014-10-12 14:35 GMT-07:00 Marcello Maggioni < hayarms at gmail.com > :
> 
> 
> 
> 
> 
> Thanks again Joerg, I missed the attachment!
> 
> 
> Marcello
> 
> 
> 2014-10-12 13:50 GMT-07:00 Joerg Sonnenberger <
> joerg at britannica.bec.de > :
> 
> 
> 
> 
> On Sun, Oct 12, 2014 at 01:09:25PM -0700, Marcello Maggioni wrote:
> > Thanks Joerg for reverting and thanks Jiangning for reporting the
> > problem.
> > 
> > Would it be possible to have the snippet of the code that is
> > causing the
> > issue or is it not publicly available? In order to be able to debug
> > this
> > more effectively and safely.
> 
> See my earlier post in this thread for a test case.
> 
> Joerg
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory