[llvm-commits] [llvm] r168931 - in /llvm/trunk: include/llvm/Target/TargetTransformImpl.h include/llvm/TargetTransformInfo.h lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h lib/Target/X86/X86TargetMachine.h lib/Transforms/Scalar/LoopIdiomRecognize.cpp test/Transforms/LoopIdiom/popcnt.ll

[Duncan Sands]

Hi Shuxin,

>    Thank you for pointing out the problem. Yes, the "&&" should be replaced with
> "||".
> I will commit the fix after some tests.  I don't feel like to include a testing
> case for
> such inadvertent bug.

this sounds backwards to me.  Doesn't the fact your testing didn't catch this
prove the need for a test case?  But maybe I misunderstood you, and you do plan
to add a test for this?

Ciao, Duncan.

>
>     Following is the contrived *.ll to reproduce the problem.
>
>
>    /the/path/to/opt  -loop-idiom -mtriple=x86_64-apple-darwin -mcpu=corei7 a.ll
> -S -o -
>
> cat a.ll
>
> target triple = "x86_64-apple-macosx10.8.0"
>
> define i32 @PopCnt(i64 %a) nounwind uwtable readnone ssp {
> entry:
>    %tobool3 = icmp eq i64 %a, 0
>    br i1 %tobool3, label %while.end, label %while.body
>
> while.body:                                       ; preds = %entry, %while.body
>    %c.05 = phi i32 [ %inc, %while.body ], [ 0, %entry ]
>    %a.addr.04 = phi i64 [ %and, %while.body ], [ %a, %entry ]
>    %t = add i32 %c.05, %c.05
>    %inc = add nsw i32 %t, 1
>    %sub = add i64 %a.addr.04, -1
>    %and = and i64 %sub, %a.addr.04
>    %tobool = icmp eq i64 %and, 0
>    br i1 %tobool, label %while.end, label %while.body
>
> while.end:                                        ; preds = %while.body, %entry
>    %c.0.lcssa = phi i32 [ 0, %entry ], [ %inc, %while.body ]
>    ret i32 %c.0.lcssa
> }
>
>
>
>
>
>> This is incorrect, and will crash LLVM on certain inputs: if InitConst is null
>> here, we pass the test and pass it to CreateAdd below that eventually
>> segfaults. Please fix, and please add a test case for this case. I spotted it
>> by inspection (after some code in the wild crashed). I can try to help you
>> with a testcase if you need, but it should be quite straight forward to
>> synthesize one.
>> -------
>>
>> Here is the snippet regarding the other one I spotted by inspection:
>> ----
>>
>>     +      PHINode *Phi = dyn_cast<PHINode>(Inst->getOperand(0));
>>     +      if (!Phi && Phi->getParent() != LoopEntry)
>>
>>
>>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>