[llvm-commits] [LLVM, loop-unswitch] Potential bug in RewriteLoopBodyWithConditionConstant

[Stepan Dyatkovskiy]

Hi Duncan. You are right. Please find the fixed patch.
It is a pretty diffucult to create a test case. By now I can only say 
that after replaceUsesOfWith was invoked we implicitly proceed to 
enumerate Replacement's UseList instead of LIC's. But we try to unswitch 
instructions that uses both LIC and Replacement. That why all looks fine.
I also attached dumped instructions enumerated before and after fix (for 
first invocation of LoopUnswitch::RewriteLoopBodyWithConditionConstant 
with IsEqual = true).
.ll file that was proceed is also attached.

-Stepan.

Duncan Sands wrote:
> Hi Stepan,
>
>> It seems that the code inside the
>> LoopUnswitch::RewriteLoopBodyWithConditionConstant contains potential bugs.
>> UseList is changed inside the loops that are goes through its items:
>>
>> LoopUnswitch.cpp, string #905: for (Value::use_iterator UI = LIC->use_begin(), E
>> = LIC->use_end(); UI != E; ++UI) {
>>
>> ...and then inside the loop body UseList is changed implicitly (
>> LoopUnswitch.cpp, string #910):
>> U->replaceUsesOfWith(LIC, Replacement);
>>
>> It seems that after editing UseList loop may produce unpreditable results. But
>> we are lucky by now though :-)
>>
>> I propose to collect all to be changed and then do some changes?
>> If so, please find the patch attached for review.
>
> can you just do something like this instead?  Also, do you have a testcase?
>
>          Replacement = ConstantInt::get(Type::getInt1Ty(Val->getContext()),
>                                         !cast<ConstantInt>(Val)->getZExtValue());
>
>        for (Value::use_iterator UI = LIC->use_begin(), E = LIC->use_end();
> -         UI != E; ++UI) {
> -      Instruction *U = dyn_cast<Instruction>(*UI);
> +         UI != E; ) {
> +      Instruction *U = dyn_cast<Instruction>(*UI++);
>          if (!U || !L->contains(U))
>            continue;
>          U->replaceUsesOfWith(LIC, Replacement);
>          Worklist.push_back(U);
>        }
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: loop-unswitch-uselist.patch
Type: text/x-patch
Size: 667 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111122/33c59d6f/attachment.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: uselist-afterfix.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111122/33c59d6f/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: loop-case-3.ll
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111122/33c59d6f/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: uselist-beforefix.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111122/33c59d6f/attachment-0001.txt>