<div dir="ltr">I see, thanks for the specification Hal!<div><br></div><div>I was confused because the verifier tests it in this way:</div><div><br></div><div><div>      Assert1(PN->getNumIncomingValues() == Preds.size(),</div><div>              "PHINode should have one entry for each predecessor of its "</div><div>              "parent basic block!", PN);</div></div><div><br></div><div>where Preds are the predecessors of the block. Because getNumIncomingValues() returns the number of operands, which should be equal to the number of incoming edges , I thought that if multiple edges are specified for the same block this check would fail.</div><div>Apparently then also the predecessors are stored multiple times.</div><div><br></div><div>Marcello</div></div><div class="gmail_extra"><br><div class="gmail_quote">2014-10-12 18:06 GMT-07:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Marcello,<br>
<br>
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.<br>
<br>
 -Hal<br>
<div class="HOEnZb"><div class="h5"><br>
----- Original Message -----<br>
> From: "Marcello Maggioni" <<a href="mailto:hayarms@gmail.com">hayarms@gmail.com</a>><br>
> To: "Marcello Maggioni" <<a href="mailto:hayarms@gmail.com">hayarms@gmail.com</a>>, <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Sent: Sunday, October 12, 2014 6:44:54 PM<br>
> Subject: Re: [llvm] r219223 - Two case switch to select optimization<br>
><br>
><br>
><br>
> I did a fast research into the LLVM base code and seems like, even if<br>
> having multiple incoming edges for the same block is forbidden by<br>
> the verifier through a check at line Verifier.cpp:1146, it is a<br>
> common place assumption in SimplifyCFG that if a block branches to<br>
> the same target multiple times the PHI should have multiple incoming<br>
> edges from the same block.<br>
><br>
><br>
> I find this pretty strange ... I tried to remove the source of double<br>
> incoming edges for the specific case in the example posted by Joerg,<br>
> but doing so breaks other code that EXPECTS multiple incoming edges<br>
> for the same block in certain conditions (like a conditional branch<br>
> targeting the same block two times with both the True and False<br>
> branches).<br>
><br>
><br>
> If this is the direction we want to go (continuing to support<br>
> multiple incoming edges from the same block) tomorrow I will post a<br>
> patch for switch to select that takes into consideration this when<br>
> multiple cases of a switch target the same block and I will add Hans<br>
> and Jin as reviewers.<br>
><br>
><br>
> Cheers,<br>
> Marcello<br>
><br>
><br>
><br>
><br>
><br>
> 2014-10-12 14:40 GMT-07:00 Marcello Maggioni < <a href="mailto:hayarms@gmail.com">hayarms@gmail.com</a> > :<br>
><br>
><br>
><br>
> Hmm,<br>
><br>
><br>
> compiling your test I've noticed that even before entering the<br>
> SwitchToSelect optimization the PHI that breaks the module already<br>
> has two incoming edges for the same predecessor:<br>
><br>
><br>
> This is the IR just entering SwitchToSelect:<br>
><br>
><br>
><br>
><br>
> ; Function Attrs: nounwind ssp uwtable<br>
><br>
> define i32 @fn1() #0 {<br>
><br>
> entry:<br>
><br>
> %0 = load i32* @b, align 4<br>
><br>
> %tobool = icmp eq i32 %0, 0<br>
><br>
> br i1 %tobool, label %if.end3, label %if.then<br>
><br>
><br>
><br>
><br>
> if.then: ; preds = %entry<br>
><br>
> %1 = load i32* @a, align 4<br>
><br>
> switch i32 %1, label %if.end3 [<br>
><br>
> i32 5, label %return<br>
><br>
> i32 0, label %return<br>
><br>
> ]<br>
><br>
><br>
><br>
><br>
> if.end3: ; preds = %if.then, %entry<br>
><br>
> br label %return<br>
><br>
><br>
><br>
><br>
> return: ; preds = %if.then, %if.then, %if.end3<br>
><br>
> %retval.0 = phi i32 [ 0, %if.end3 ], [ %1, %if.then ], [ %1, %if.then<br>
> ]<br>
><br>
> ret i32 %retval.0<br>
><br>
> }<br>
><br>
><br>
> Is this condition expected to be handled by the switch lowering? Or<br>
> is the switch formation that probably introduced this malformed?<br>
> Is pretty easily to workaround this in SwitchToSelect to reiterate<br>
> Incoming Edge deletion when the switch is removed until no more<br>
> edges from the same predecessor are found, but I guess if we have<br>
> some other issue more up in the pipeline.<br>
><br>
><br>
> 2014-10-12 14:35 GMT-07:00 Marcello Maggioni < <a href="mailto:hayarms@gmail.com">hayarms@gmail.com</a> > :<br>
><br>
><br>
><br>
><br>
><br>
> Thanks again Joerg, I missed the attachment!<br>
><br>
><br>
> Marcello<br>
><br>
><br>
> 2014-10-12 13:50 GMT-07:00 Joerg Sonnenberger <<br>
> <a href="mailto:joerg@britannica.bec.de">joerg@britannica.bec.de</a> > :<br>
><br>
><br>
><br>
><br>
> On Sun, Oct 12, 2014 at 01:09:25PM -0700, Marcello Maggioni wrote:<br>
> > Thanks Joerg for reverting and thanks Jiangning for reporting the<br>
> > problem.<br>
> ><br>
> > Would it be possible to have the snippet of the code that is<br>
> > causing the<br>
> > issue or is it not publicly available? In order to be able to debug<br>
> > this<br>
> > more effectively and safely.<br>
><br>
> See my earlier post in this thread for a test case.<br>
><br>
> Joerg<br>
><br>
><br>
><br>
><br>
</div></div><div class="HOEnZb"><div class="h5">> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>