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

[Duncan Sands]

I think Anton commented on your first patch too, but quite differently
to me.  Anton, are you OK with this?

Ciao, Duncan.

On 23/11/11 19:00, Stepan Dyatkovskiy wrote:
> ping.
>
> -Stepan.
>
> Stepan Dyatkovskiy wrote:
>> Hi Duncan. You are right. Please find the fixed patch.
>> It is a pretty difficult 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
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>