[PATCH] Fix crash in MachineLICM.cpp
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 8 16:41:58 PDT 2016
Could you make a patch out of the test case + the code change?
A couple more comment on the test case:
1. Use 'opt -instnamer' to get rid of the %[0-9]+ variables (you could do that manually since you only have one %0).
2. Add a comment explaining what this test is checking.
3. Add more check lines to make sure we generate something sane. (Like add a check for load and add, or whatever was touched by the LICM pass.)
4. You may want to use a triple instead of march for stability accross bot.
Cheers,
Q.
> On Jun 8, 2016, at 4:34 PM, zan jyu Wong <zyfwong at gmail.com> wrote:
>
> Hi,
>
> The following is the test:
>
> ; RUN: llc -march=thumb -sink-insts-to-avoid-spills -o - %s \
> ; RUN: | FileCheck %s
> ; CHECK-LABEL: sum
>
> ; Function Attrs: nounwind readonly
> define arm_aapcscc i32 @sum(i32* noalias nocapture readonly %arr, i32 %c) {
> entry:
> %cmp7 = icmp sgt i32 %c, 0
> br i1 %cmp7, label %for.body.preheader, label %for.cond.cleanup
>
> for.body.preheader: ; preds = %entry
> %sp = tail call i32 asm "mov $0, sp", "=r"()
> br label %for.body
>
> for.cond.cleanup.loopexit: ; preds = %for.body
> br label %for.cond.cleanup
>
> for.cond.cleanup: ; preds = %for.cond.cleanup.loopexit, %entry
> %sum.0.lcssa = phi i32 [ 0, %entry ], [ %add, %for.cond.cleanup.loopexit ]
> ret i32 %sum.0.lcssa
>
> for.body: ; preds = %for.body, %for.body.preheader
> %i.09 = phi i32 [ %inc, %for.body ], [ 0, %for.body.preheader ]
> %sum.08 = phi i32 [ %add, %for.body ], [ 0, %for.body.preheader ]
> %arrayidx = getelementptr inbounds i32, i32* %arr, i32 %i.09
> %0 = load i32, i32* %arrayidx, align 4
> %add = add nsw i32 %0, %sp
> %inc = add nuw nsw i32 %i.09, 1
> %exitcond = icmp eq i32 %inc, %c
> br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body
> }
>
> On Thu, Jun 9, 2016 at 6:06 AM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
> Hi Zan Jyu,
>
> Great!
>
> Add a few CHECK lines and get rid of the unused metadata and you should be good to go!
>
> Cheers,
> -Quentin
>
>> On Jun 7, 2016, at 9:06 PM, zan jyu Wong <zyfwong at gmail.com <mailto:zyfwong at gmail.com>> wrote:
>>
>> I've tried the following example on Thumb backend with the flags, it's ok?
>>
>> RUN: llc licm.ll -sink-insts-to-avoid-spills
>>
>> ========== example ===========
>> ; ModuleID = 'licm.c'
>> source_filename = "licm.c"
>> target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
>> target triple = "armv4t--"
>>
>> ; Function Attrs: nounwind readonly
>> define arm_aapcscc i32 @sum(i32* noalias nocapture readonly %arr, i32 %c) #0 {
>> entry:
>> %cmp7 = icmp sgt i32 %c, 0
>> br i1 %cmp7, label %for.body.preheader, label %for.cond.cleanup
>>
>> for.body.preheader: ; preds = %entry
>> %sp = tail call i32 asm "mov $0, sp", "=r"() #1, !srcloc !3
>> br label %for.body
>>
>> for.cond.cleanup.loopexit: ; preds = %for.body
>> br label %for.cond.cleanup
>>
>> for.cond.cleanup: ; preds = %for.cond.cleanup.loopexit, %entry
>> %sum.0.lcssa = phi i32 [ 0, %entry ], [ %add, %for.cond.cleanup.loopexit ]
>> ret i32 %sum.0.lcssa
>>
>> for.body: ; preds = %for.body.preheader, %for.body
>> %i.09 = phi i32 [ %inc, %for.body ], [ 0, %for.body.preheader ]
>> %sum.08 = phi i32 [ %add, %for.body ], [ 0, %for.body.preheader ]
>> %arrayidx = getelementptr inbounds i32, i32* %arr, i32 %i.09
>> %0 = load i32, i32* %arrayidx, align 4, !tbaa !4
>> %mul = mul nsw i32 %0, %sp
>> %add = add nsw i32 %mul, %sum.08
>> %inc = add nuw nsw i32 %i.09, 1
>> %exitcond = icmp eq i32 %inc, %c
>> br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body
>> }
>>
>> attributes #0 = { nounwind readonly "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="arm7tdmi" "target-features"="+soft-float,+strict-align,-crypto,-neon" "unsafe-fp-math"="false" "use-soft-float"="true" }
>> attributes #1 = { nounwind readnone }
>>
>> !llvm.module.flags = !{!0, !1}
>> !llvm.ident = !{!2}
>>
>> !0 = !{i32 1, !"wchar_size", i32 4}
>> !1 = !{i32 1, !"min_enum_size", i32 4}
>> !2 = !{!"clang version 3.9.0 (http://llvm.org/git/clang.git <http://llvm.org/git/clang.git> e8204bf8264d6d52aab4e8ac84328654d3ef28bd) (http://llvm.org/git/llvm.git <http://llvm.org/git/llvm.git> d75f887d2fcccc1f0dbf7809c2638e2ff3448462)"}
>> !3 = !{i32 76}
>> !4 = !{!5, !5, i64 0}
>> !5 = !{!"long", !6, i64 0}
>> !6 = !{!"omnipotent char", !7, i64 0}
>> !7 = !{!"Simple C/C++ TBAA"}
>>
>> On Wed, Jun 8, 2016 at 8:50 AM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>> Could you try a simple example with that flag on x86_64 to produce a test case?
>>
>> Thanks,
>> Q.
>>
>>> On Jun 7, 2016, at 5:38 PM, zan jyu Wong <zyfwong at gmail.com <mailto:zyfwong at gmail.com>> wrote:
>>>
>>> No. Since I'm working on a custom backend, unless I upload the backend.
>>> But there is an assertion in isDef(), so I think it can be reproduced on other backend when compile with -sink-insts-to-avoid-spills=true.
>>>
>>> On Wed, Jun 8, 2016 at 12:03 AM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>> Hi Zan Jyu,
>>>
>>> Yep.
>>>
>>> Do you have a test case to put with that?
>>>
>>> Thanks,
>>> -Quentin
>>>> On Jun 7, 2016, at 12:25 AM, zan jyu Wong via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>
>>>> We should test isReg() before isDef()
>>>>
>>>> diff --git a/lib/CodeGen/MachineLICM.cpp b/lib/CodeGen/MachineLICM.cpp
>>>> index 00dec82..4ea0c56 100644
>>>> --- a/lib/CodeGen/MachineLICM.cpp
>>>> +++ b/lib/CodeGen/MachineLICM.cpp
>>>> @@ -716,7 +716,7 @@ void MachineLICM::SinkIntoLoop() {
>>>>
>>>> for (MachineInstr *I : Candidates) {
>>>> const MachineOperand &MO = I->getOperand(0);
>>>> - if (!MO.isDef() || !MO.isReg() || !MO.getReg())
>>>> + if (!MO.isReg() || !MO.isDef() || !MO.getReg())
>>>> continue;
>>>> if (!MRI->hasOneDef(MO.getReg()))
>>>> continue;
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160608/fc50b2dc/attachment.html>
More information about the llvm-commits
mailing list