[llvm] r257253 - [BranchFolding] Set correct mem refs
Michael Zolotukhin via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 10 22:13:16 PST 2016
> On Jan 10, 2016, at 9:30 PM, 박준모 <junmoz.park at samsung.com> wrote:
>
> Hi Michael.
Hi Junmo,
>
> Thanks for comments. I’ll try harder to make secure code.
>
> When we run “llc -march=aarch64 -mtriple=aarch64-none-linux-gnu -stop-after branch-folder -o /dev/null branch-folder-merge-mmos.ll”, we can get result likes below.
>
> Compiling will be stopped after “branch-folder” optimization. So there is no foo label. And “–o /dev/null”, this is just for avoiding error “LLVM ERROR: IO failure on output stream.”.
> And this option is used for several testcases.
> If we don’t use the option “-stop-after branch-folder”, we don’t need to use “-o /dev/null”. As you said, there is no error with “-o -”.
Ah, that makes sense then, sorry for the noise.
>
> I deleted test function. Are there any comments for testcase?
I don’t have any other comments regarding the test.
Thanks,
Michael
>
> -Junmo
> --- |
> ; ModuleID = 'branch-folder-merge-mmos.ll'
> target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
> target triple = "aarch64-none-linux-gnu"
>
> define void @foo(i32 %a, i32 %b, float* nocapture %foo_arr) {
> entry:
> %cmp = icmp sgt i32 %a, 0
> br i1 %cmp, label %if.then, label %if.end
>
> if.then: ; preds = %entry
> %0 = load float, float* %foo_arr, align 4
> %arrayidx1.i1 = getelementptr inbounds float, float* %foo_arr, i64 1
> %1 = load float, float* %arrayidx1.i1, align 4
> %sub.i = fsub float %0, %1
> store float %sub.i, float* %foo_arr, align 4
> br label %if.end3
>
> if.end: ; preds = %entry
> %cmp1 = icmp sgt i32 %b, 0
> br i1 %cmp1, label %if.then2, label %if.end3
>
> if.then2: ; preds = %if.end
> %2 = load float, float* %foo_arr, align 4
> %arrayidx1.i2 = getelementptr inbounds float, float* %foo_arr, i64 1
> %3 = load float, float* %arrayidx1.i2, align 4
> %sub.i3 = fsub float %2, %3
> store float %sub.i3, float* %foo_arr, align 4
> br label %if.end3
>
> if.end3: ; preds = %if.then2, %if.end, %if.then
> ret void
> }
>
> ...
> ---
> name: foo
> alignment: 2
> exposesReturnsTwice: false
> hasInlineAsm: false
> isSSA: false
> tracksRegLiveness: false
> tracksSubRegLiveness: false
> liveins:
> - { reg: '%w0' }
> - { reg: '%w1' }
> - { reg: '%x2' }
> frameInfo:
> isFrameAddressTaken: false
> isReturnAddressTaken: false
> hasStackMap: false
> hasPatchPoint: false
> stackSize: 0
> offsetAdjustment: 0
> maxAlignment: 0
> adjustsStack: false
> hasCalls: false
> maxCallFrameSize: 0
> hasOpaqueSPAdjustment: false
> hasVAStart: false
> hasMustTailInVarArgFunc: false
> body: |
> bb.0.entry:
> successors: %bb.2.if.then2(0x50000000 / 0x80000000 = 62.50%), %bb.1.if.end(0x30000000 / 0x80000000 = 37.50%)
> liveins: %w0, %w1, %x2
>
> dead %wzr = SUBSWri killed %w0, 1, 0, implicit-def %nzcv
> Bcc 10, %bb.2.if.then2, implicit %nzcv
>
> bb.1.if.end:
> successors: %bb.2.if.then2(0x50000000 / 0x80000000 = 62.50%), %bb.3.if.end3(0x30000000 / 0x80000000 = 37.50%)
> liveins: %w1, %x2
>
> dead %wzr = SUBSWri killed %w1, 1, 0, implicit-def %nzcv
> Bcc 11, %bb.3.if.end3, implicit %nzcv
>
> bb.2.if.then2:
> successors: %bb.3.if.end3(0x80000000 / 0x80000000 = 100.00%)
> liveins: %x2
>
> %s0 = LDRSui %x2, 0 :: (load 4 from %ir.foo_arr)
> %s1 = LDRSui %x2, 1 :: (load 4 from %ir.arrayidx1.i2), (load 4 from %ir.arrayidx1.i1)
> %s0 = FSUBSrr killed %s0, killed %s1
> STRSui killed %s0, killed %x2, 0 :: (store 4 into %ir.foo_arr)
>
> bb.3.if.end3:
> RET_ReallyLR
>
> ...
>
>
>
> From: mzolotukhin at apple.com [mailto:mzolotukhin at apple.com]
> Sent: Monday, January 11, 2016 12:42 PM
> To: 박준모
> Cc: Benjamin Kramer; Chandler Carruth via llvm-commits
> Subject: Re: [llvm] r257253 - [BranchFolding] Set correct mem refs
>
>
>> On Jan 10, 2016, at 6:53 PM, 박준모 <junmoz.park at samsung.com <mailto:junmoz.park at samsung.com>> wrote:
>>
>> Hi Benjamin, Michael.
> Hi Junmo,
>
>
> I couldn't expect that array's memory reference information can be changed with -O3 option.
> Why I used combination of "Opt -O3 & llc" is that I want to represent real world's problem.(Branch folding can be occurred after Function Inlining.)
> I see. LLVM exploits a modular structure, so (almost) every transformation can be tested independently on others. That doesn’t mean all transformations work only on artificial tests - all of them actually target some real world problems, but testing them in isolation is just much more stable.
>
>
> But this makes test failure on some machine. (Index of arrayidx1 can be changed, when optimization pass is changed.)
>
> So I change my test code likes below. I hope below code is ok for everyone. If there are no objections, I will recommit this.
> I haven’t looked at the code, but if you didn’t change it, I suppose it’s still considered “ok to commit”. However, I have some comments regarding the test - please see below.
>
>
> diff --git a/lib/CodeGen/BranchFolding.cpp b/lib/CodeGen/BranchFolding.cpp
> index 6080349..df5cac5 100644
> --- a/lib/CodeGen/BranchFolding.cpp
> +++ b/lib/CodeGen/BranchFolding.cpp
> @@ -780,7 +780,7 @@ removeMMOsFromMemoryOperations(MachineBasicBlock::iterator MBBIStartPos,
> assert(MBBICommon->isIdenticalTo(&*MBBI) && "Expected matching MIIs!");
>
> if (MBBICommon->mayLoad() || MBBICommon->mayStore())
> - MBBICommon->setMemRefs(MBBI->mergeMemRefsWith(*MBBI));
> + MBBICommon->setMemRefs(MBBICommon->mergeMemRefsWith(*MBBI));
>
> ++MBBI;
> ++MBBICommon;
> diff --git a/test/CodeGen/AArch64/branch-folder-merge-mmos.ll b/test/CodeGen/AArch64/branch-folder-merge-mmos.ll
> new file mode 100644
> index 0000000..1c79e60
> --- /dev/null
> +++ b/test/CodeGen/AArch64/branch-folder-merge-mmos.ll
> @@ -0,0 +1,44 @@
> +; RUN: llc -march=aarch64 -mtriple=aarch64-none-linux-gnu -stop-after branch-folder -o /dev/null < %s | FileCheck %s
> This doesn’t make much sense to me - why the output is /dev/null? I’m surprised that could work at all. I guess it should be “-o -“, so that output would be in stdout, which then will be piped to FileCheck.
>
>> +target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
>> +
>> +; Function Attrs: norecurse nounwind
>> +define void @test(float* nocapture %test_arr) #0 {
> Why does the test need this function? We don’t check anything in it, so I suppose it can be completely removed.
>
> +entry:
> + %0 = load float, float* %test_arr, align 4
> + %arrayidx1 = getelementptr inbounds float, float* %test_arr, i64 1
> + %1 = load float, float* %arrayidx1, align 4
> + %sub = fsub float %0, %1
> + store float %sub, float* %test_arr, align 4
> + ret void
> +}
> +
> +; Function Attrs: norecurse nounwind
> +define void @foo(i32 %a, i32 %b, float* nocapture %foo_arr) #0 {
> It is a good practice to use "CHECK-LABEL: @foo” in the test. This way, if the test contains several functions, CHECK directives are guaranteed to work in the intended scope of the given function (they work in between CHECK-LABEL directives).
>
> +; CHECK: (load 4 from %ir.arrayidx1.{{i[1-2]}}), (load 4 from %ir.arrayidx1.{{i[1-2]}})
> +entry:
> + %cmp = icmp sgt i32 %a, 0
> + br i1 %cmp, label %if.then, label %if.end
> +
> +if.then: ; preds = %entry
> + %0 = load float, float* %foo_arr, align 4
> + %arrayidx1.i1 = getelementptr inbounds float, float* %foo_arr, i64 1
> + %1 = load float, float* %arrayidx1.i1, align 4
> + %sub.i = fsub float %0, %1
> + store float %sub.i, float* %foo_arr, align 4
> + br label %if.end3
> +
> +if.end: ; preds = %entry
> + %cmp1 = icmp sgt i32 %b, 0
> + br i1 %cmp1, label %if.then2, label %if.end3
> +
> +if.then2: ; preds = %if.end
> + %2 = load float, float* %foo_arr, align 4
> + %arrayidx1.i2 = getelementptr inbounds float, float* %foo_arr, i64 1
>
> Michael
>
>
> Junmo.
>
> -----Original Message-----
> From: Benjamin Kramer [mailto:benny.kra at gmail.com <mailto:benny.kra at gmail.com>]
> Sent: Sunday, January 10, 2016 4:57 AM
> To: Junmo Park
> Cc: Chandler Carruth via llvm-commits
> Subject: Re: [llvm] r257253 - [BranchFolding] Set correct mem refs
>
> On Sat, Jan 9, 2016 at 8:30 AM, Junmo Park via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: flyingforyou
> Date: Sat Jan 9 01:30:13 2016
> New Revision: 257253
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257253&view=rev <http://llvm.org/viewvc/llvm-project?rev=257253&view=rev>
> Log:
> [BranchFolding] Set correct mem refs
>
> Merge MBBICommon and MBBI's MMOs.
>
> Differential Revision: http://reviews.llvm.org/D15990 <http://reviews.llvm.org/D15990>
>
> Added:
> llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll
> Modified:
> llvm/trunk/lib/CodeGen/BranchFolding.cpp
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=257253&r1=257252&r2=257253&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=257253&r1=257252&r2=257253&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Sat Jan 9 01:30:13 2016
> @@ -780,7 +780,7 @@ removeMMOsFromMemoryOperations(MachineBa
> assert(MBBICommon->isIdenticalTo(&*MBBI) && "Expected matching MIIs!");
>
> if (MBBICommon->mayLoad() || MBBICommon->mayStore())
> - MBBICommon->setMemRefs(MBBI->mergeMemRefsWith(*MBBI));
> + MBBICommon->setMemRefs(MBBICommon->mergeMemRefsWith(*MBBI));
>
> ++MBBI;
> ++MBBICommon;
>
> Added: llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll?rev=257253&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll?rev=257253&view=auto>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll (added)
> +++ llvm/trunk/test/CodeGen/AArch64/branch-folder-merge-mmos.ll Sat Jan 9 01:30:13 2016
> @@ -0,0 +1,53 @@
> +; RUN: opt < %s -O3 | llc -march=aarch64 -mtriple=aarch64-none-linux-gnu -stop-after branch-folder -o /dev/null | FileCheck %s
>
> Why are you running opt -O3 on every run instead of just committing
> the optimized IR? Running the O3 pipeline can cause the test to fail
> when IR optimization passes change.
>
> - Ben
>
>
> +target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
> +
> +; Function Attrs: nounwind
> +define void @test(float* %test_arr) #0 {
> +entry:
> + %test_arr.addr = alloca float*, align 8
> + store float* %test_arr, float** %test_arr.addr, align 8
> + %0 = load float*, float** %test_arr.addr, align 8
> + %arrayidx = getelementptr inbounds float, float* %0, i64 0
> + %1 = load float, float* %arrayidx, align 4
> + %2 = load float*, float** %test_arr.addr, align 8
> + %arrayidx1 = getelementptr inbounds float, float* %2, i64 1
> + %3 = load float, float* %arrayidx1, align 4
> + %sub = fsub float %1, %3
> + %4 = load float*, float** %test_arr.addr, align 8
> + %arrayidx2 = getelementptr inbounds float, float* %4, i64 0
> + store float %sub, float* %arrayidx2, align 4
> + ret void
> +}
> +
> +; Function Attrs: nounwind
> +define void @foo(i32 %a, i32 %b, float* %foo_arr) #0 {
> +; CHECK: (load 4 from %ir.arrayidx1.i2), (load 4 from %ir.arrayidx1.i)
> +entry:
> + %a.addr = alloca i32, align 4
> + %b.addr = alloca i32, align 4
> + %foo_arr.addr = alloca float*, align 8
> + store i32 %a, i32* %a.addr, align 4
> + store i32 %b, i32* %b.addr, align 4
> + store float* %foo_arr, float** %foo_arr.addr, align 8
> + %0 = load i32, i32* %a.addr, align 4
> + %cmp = icmp sgt i32 %0, 0
> + br i1 %cmp, label %if.then, label %if.end
> +
> +if.then: ; preds = %entry
> + %1 = load float*, float** %foo_arr.addr, align 8
> + call void @test(float* %1)
> + br label %if.end3
> +
> +if.end: ; preds = %entry
> + %2 = load i32, i32* %b.addr, align 4
> + %cmp1 = icmp sgt i32 %2, 0
> + br i1 %cmp1, label %if.then2, label %if.end3
> +
> +if.then2: ; preds = %if.end
> + %3 = load float*, float** %foo_arr.addr, align 8
> + call void @test(float* %3)
> + br label %if.end3
> +
> +if.end3: ; preds = %if.then, %if.then2, %if.end
> + ret void
> +}
>
>
> _______________________________________________
> 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>
> <diff.txt>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160110/cc646110/attachment.html>
More information about the llvm-commits
mailing list