[llvm] 8fffa40 - [GC] Remove redundant entiries in stackmap section (and test it this time)
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 12 08:36:33 PDT 2020
Should be fixed in f1bb955c573].
Philip
On 3/11/20 11:57 PM, Mikael Holmén wrote:
> Hi Philip,
>
> clang warns on new code in this patch:
>
> ../lib/CodeGen/SelectionDAG/StatepointLowering.cpp:843:13: error:
> unused variable 'BaseSD' [-Werror,-Wunused-variable]
> SDValue BaseSD = getValue(Relocate->getBasePtr());
> ^
> 1 error generated.
>
> Can we just remove the line completely or is there some side effect in
> getValue that is needed?
>
> Regards,
> Mikael
>
> On Wed, 2020-03-11 at 21:24 -0700, Philip Reames via llvm-commits
> wrote:
>> Author: Philip Reames
>> Date: 2020-03-11T21:24:48-07:00
>> New Revision: 8fffa40400e8719222e7f67152c12738521fa9fb
>>
>> URL:
>> https://protect2.fireeye.com/v1/url?k=3ded4adc-6167680a-3ded0a47-0cc47ad93dcc-77b6f9a7f7f52e82&q=1&e=b4cd1a3e-fe77-479c-a47b-f22079f5281f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2F8fffa40400e8719222e7f67152c12738521fa9fb
>> DIFF:
>> https://protect2.fireeye.com/v1/url?k=c655e929-9adfcbff-c655a9b2-0cc47ad93dcc-8edef7b6eb27e30f&q=1&e=b4cd1a3e-fe77-479c-a47b-f22079f5281f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2F8fffa40400e8719222e7f67152c12738521fa9fb.diff
>>
>> LOG: [GC] Remove redundant entiries in stackmap section (and test it
>> this time)
>>
>> This is a reimplementation of the optimization removed in D75964. The
>> actual spill/fill optimization is handled by D76013, this one just
>> worries about reducing the stackmap section size itself by
>> eliminating redundant entries. As noted in the comments, we could go
>> a lot further here, but avoiding the degenerate invoke case as we did
>> before is probably "enough" in practice.
>>
>> Differential Revision:
>> https://protect2.fireeye.com/v1/url?k=0b7c0ec1-57f62c17-0b7c4e5a-0cc47ad93dcc-9eb52bdd595d688c&q=1&e=b4cd1a3e-fe77-479c-a47b-f22079f5281f&u=https%3A%2F%2Freviews.llvm.org%2FD76021
>>
>> Added:
>> llvm/test/CodeGen/X86/statepoint-stackmap-size.ll
>>
>> Modified:
>> llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>>
>> Removed:
>>
>>
>>
>> #####################################################################
>> ###########
>> diff --git a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>> b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>> index 40033db4e26d..e65b59416085 100644
>> --- a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>> +++ b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>> @@ -603,7 +603,7 @@ SDValue SelectionDAGBuilder::LowerAsSTATEPOINT(
>> // Clear state
>> StatepointLowering.startNewStatepoint(*this);
>> assert(SI.Bases.size() == SI.Ptrs.size() &&
>> - SI.Ptrs.size() == SI.GCRelocates.size());
>> + SI.Ptrs.size() <= SI.GCRelocates.size());
>>
>> #ifndef NDEBUG
>> for (auto *Reloc : SI.GCRelocates)
>> @@ -823,10 +823,29 @@
>> SelectionDAGBuilder::LowerStatepoint(ImmutableStatepoint ISP,
>> ISP.getNumCallArgs(), ActualCallee,
>> ISP.getActualReturnType(), false /*
>> IsPatchPoint */);
>>
>> + // There may be duplication in the gc.relocate list; such as two
>> copies of
>> + // each relocation on normal and exceptional path for an
>> invoke. We only
>> + // need to spill once and record one copy in the stackmap, but we
>> need to
>> + // reload once per gc.relocate. (Dedupping gc.relocates is
>> trickier and best
>> + // handled as a CSE problem elsewhere.)
>> + // TODO: There a couple of major stackmap size optimizations we
>> could do
>> + // here if we wished.
>> + // 1) If we've encountered a derived pair {B, D}, we don't need to
>> actually
>> + // record {B,B} if it's seen later.
>> + // 2) Due to rematerialization, actual derived pointers are
>> somewhat rare;
>> + // given that, we could change the format to record base pointer
>> relocations
>> + // separately with half the space. This would require a format rev
>> and a
>> + // fairly major rework of the STATEPOINT node though.
>> + SmallSet<SDValue, 8> Seen;
>> for (const GCRelocateInst *Relocate : ISP.getRelocates()) {
>> SI.GCRelocates.push_back(Relocate);
>> - SI.Bases.push_back(Relocate->getBasePtr());
>> - SI.Ptrs.push_back(Relocate->getDerivedPtr());
>> +
>> + SDValue BaseSD = getValue(Relocate->getBasePtr());
>> + SDValue DerivedSD = getValue(Relocate->getDerivedPtr());
>> + if (Seen.insert(DerivedSD).second) {
>> + SI.Bases.push_back(Relocate->getBasePtr());
>> + SI.Ptrs.push_back(Relocate->getDerivedPtr());
>> + }
>> }
>>
>> SI.GCArgs = ArrayRef<const Use>(ISP.gc_args_begin(),
>> ISP.gc_args_end());
>>
>> diff --git a/llvm/test/CodeGen/X86/statepoint-stackmap-size.ll
>> b/llvm/test/CodeGen/X86/statepoint-stackmap-size.ll
>> new file mode 100644
>> index 000000000000..d4a8a04c644e
>> --- /dev/null
>> +++ b/llvm/test/CodeGen/X86/statepoint-stackmap-size.ll
>> @@ -0,0 +1,22 @@
>> +; RUN: llc -verify-machineinstrs < %s | fgrep -A 10000
>> .llvm_stackmaps | wc -l | FileCheck %s
>> +
>> +; Without removal of duplicate entries, the size is 62 lines
>> +; CHECK: 50
>> +
>> +target triple = "x86_64-pc-linux-gnu"
>> +
>> +declare void @func()
>> +
>> +define i1 @test1(i32 addrspace(1)* %arg) gc "statepoint-example" {
>> +entry:
>> + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...)
>> @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()*
>> @func, i32 0, i32 0, i32 0, i32 0, i32 addrspace(1)* %arg)
>> + %reloc1 = call i32 addrspace(1)*
>> @llvm.experimental.gc.relocate.p1i32(token %safepoint_token, i32 7,
>> i32 7)
>> + %reloc2 = call i32 addrspace(1)*
>> @llvm.experimental.gc.relocate.p1i32(token %safepoint_token, i32 7,
>> i32 7)
>> + %cmp1 = icmp eq i32 addrspace(1)* %reloc1, null
>> + %cmp2 = icmp eq i32 addrspace(1)* %reloc2, null
>> + %cmp = and i1 %cmp1, %cmp2
>> + ret i1 %cmp
>> +}
>> +
>> +declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32,
>> void ()*, i32, i32, ...)
>> +declare i32 addrspace(1)*
>> @llvm.experimental.gc.relocate.p1i32(token, i32, i32)
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>>
> https://protect2.fireeye.com/v1/url?k=fbd6a13a-a75c83ec-fbd6e1a1-0cc47ad93dcc-627a9a11a4cea8d2&q=1&e=b4cd1a3e-fe77-479c-a47b-f22079f5281f&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits
More information about the llvm-commits
mailing list