This is an archive of the discontinued LLVM Phabricator instance.

[CSSPGO] Use nested context-sensitive profile.
ClosedPublic

Authored by hoy on Dec 6 2021, 4:54 PM.

Details

Summary

CSSPGO currently employs a flat profile format for context-sensitive profiles. Such a flat profile allows for precisely manipulating contexts that is either inlined or not inlined. This is a benefit over the nested profile format used by non-CS AutoFDO. A downside of this is the longer build time due to parsing the indexing the full CS contexts.

For a CS flat profile, though only the context profiles relevant to a module are loaded when that module is compiled, the cost to figure out what profiles are relevant is noticeably high when there're many contexts, since the sample reader will need to scan all context strings anyway. On the contrary, a nested function profile has its related inline subcontexts isolated from other unrelated contexts. Therefore when compiling a set of functions, unrelated contexts will never need to be scanned.

In this change we are exploring using nested profile format for CSSPGO. This is expected to work based on an assumption that with a preinliner-computed profile all contexts are precomputed and expected to be inlined by the compiler. Contexts not expected to be inlined will be cut off and returned to corresponding base profiles (for top-level outlined functions). This naturally forms a nested profile where all nested contexts are expected to be inlined. The compiler will less likely optimize on derived contexts that are not precomputed.

A CS-nested profile will look exactly the same with regular nested profile except that each nested profile can come with an attributes. With pseudo probes, a nested profile shown as below can also have a CFG checksum.

main:1968679:12
 2: 24
 3: 28 _Z5funcAi:18
 3.1: 28 _Z5funcBi:30
 3: _Z5funcAi:1467398
  0: 10
  1: 10 _Z8funcLeafi:11
  3: 24
  1: _Z8funcLeafi:1467299
   0: 6
   1: 6
   3: 287884
   4: 287864 _Z3fibi:315608
   15: 23
    !CFGChecksum: 138828622701
   !Attributes: 2
  !CFGChecksum: 281479271677951
  !Attributes: 2

Specific work included in this change:

  • A recursive profile converter to convert CS flat profile to nested profile.
  • Extend function checksum and attribute metadata to be stored in nested way for text profile and extbinary profile.
  • Unifiy sample loader inliner path for CS and preinlined nested profile.
    • Changes in the sample loader to support probe-based nested profile.

I've seen promising results regarding build time. A nested profile can result in a 20% shorter build time than a CS flat profile while keep an on-par performance. This is with -duplicate-contexts-into-base=1.

Test Plan:

Diff Detail

Event Timeline

hoy created this revision.Dec 6 2021, 4:54 PM
hoy requested review of this revision.Dec 6 2021, 4:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2021, 4:54 PM
hoy edited the summary of this revision. (Show Details)Dec 6 2021, 4:55 PM
hoy added reviewers: wenlei, wlei.
hoy updated this revision to Diff 392239.Dec 6 2021, 5:10 PM

Updating D115205: [CSSPGO] Use nested context-sensitive profile.

hoy updated this revision to Diff 392866.Dec 8 2021, 12:11 PM

Moving profile conversion to be after CS profile merging/trimming.

wenlei added inline comments.Dec 11 2021, 10:23 PM
llvm/include/llvm/ProfileData/SampleProf.h
1182

High level comments.

1186

FunctionSamples already has tree structure, can we use FunctionSamples tree to hold the nesting profiles directly, instead of introducing an intermediate trie tree? (The intermediate trie tree also overlaps with ContextTrieNode..)

llvm/lib/ProfileData/SampleProf.cpp
39

nit: generate-merged-base-profiles - "When generating nested context-sensitive profiles, always generate extra base profile for function with all its context profiles merged into it."

534

When NodeProfile (parent profile) doesn't exist, would it be beneficial to create the NodeProfile to contain inlinee's profile? That would be a more faithful conversion from a flat profile.

537–543

IIUC, the per-context ContextShouldBeInlined now becomes a per-profile flag. However, shouldInlineCandidate used by the inliner still checks attribute ContextShouldBeInlined from sample's context. That was how we tell inliner to honor preinline decision. Now without that attribute, how does inliner know to honor preinline decision?

llvm/lib/ProfileData/SampleProfReader.cpp
249

update comment

346–347

Does this mean we only set function hash for top level profile? why skip setting hash for nested inlinees?

1091

This parameter is not used?

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

The definition for ProfileIsPreinlined isn't very clear. A CS profile can also be preinlined, but here it looks like you're using it specifically for flat profile that is preinlined. Suggest we adhere to the literal definition which would apply to both flat and nest profile for preinline.

The check here could be using whether context tracker is available, basically we're try to track context without context tracker (which is tied to flat CS profile).

1523

nit: promoteMergeNotInlinedContextSamples. this is effectively a "context tracker" for nest profile, so we can use similar name as context tracker.

1775

This also opens up priority inliner for non-CS (not even preinlined) profile. Some (manual) testing as sanity check would be good.

llvm/test/tools/llvm-profdata/Inputs/cs-sample-preinline.proftext
16

what triggered these attribute value changes?

llvm/test/tools/llvm-profdata/cs-sample-nested-profile.test
2

have a probe version test case too to cover read/write per-inlinee function hash?

llvm/tools/llvm-profgen/ProfileGenerator.cpp
757

The name ProfileIsPreinlined suggest it should be set for flat profile too when preinliner is on, but this seems to be only used for nested profile.

hoy added inline comments.Dec 13 2021, 11:43 AM
llvm/include/llvm/ProfileData/SampleProf.h
1186

Nodes in a context tree may not have a function profile. Right, the struct here basically does what ContextTrieNode do. I was thinking about making ContextTrieNode shared between ProfilingTools and IPO, and that would require moving the definition into SampleProf.h, which may not be what we want.

llvm/lib/ProfileData/SampleProf.cpp
39

Sounds good.

534

When NodeProfile doesn't exist, it means the parent function is cold, and its profile also doesn't exist in the flat profile. So we are honoring that by not creating such profile here.

537–543

The per-profile flag is only used to tell the compiler how to set up the inliner (priority vs non-prioirty) and mcf extra. When it comes to inlining a particular callsite, the inliner would not honor the inline decision based on the per-context attribute. We always encode the per-context attribute in the profile.

llvm/lib/ProfileData/SampleProfReader.cpp
249

Good catch, done.

346–347

We set function hash for all profiles. We just count top-level profiles by ProbeProfileCount for sanity check in the assert several lines below. I just renamed the variable to TopLevelProbeProfileCount.

1091

Good catch.

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

Yeah, ProfileIsNestedCS sounds better?

Makes sense to check against ContextTracker.

1523

Sounds good.

1775

Test added to llvm-project/llvm/test/Transforms/SampleProfile/inline-mergeprof.ll and inline.ll

llvm/test/tools/llvm-profdata/Inputs/cs-sample-preinline.proftext
16

It's due to the following change in SampleProfWriter.cpp:

if (S.getContext().getAllAttributes()) {
    OS.indent(Indent + 1);
    OS << "!Attributes: " << S.getContext().getAllAttributes() << "\n";
llvm/test/tools/llvm-profdata/cs-sample-nested-profile.test
2

test added.

llvm/tools/llvm-profgen/ProfileGenerator.cpp
757

The flag is supposed to be only used by the compiler to differentiate a nested CS profile from a nested non-CS profile. Maybe ProfileIsNestedCS sound better?

hoy updated this revision to Diff 393979.Dec 13 2021, 11:43 AM

Addressing Wenlei's comments.

wenlei added inline comments.Dec 14 2021, 12:40 AM
llvm/lib/ProfileData/SampleProf.cpp
534

If we have preinliner on, cold parent should be removed from context already. If the parent exists, that means it's somewhat relevant for inlining.

537–543

When it comes to inlining a particular callsite, the inliner would not honor the inline decision based on the per-context attribute.

Did you mean would or would not? I expect the correct behavior to be that inliner will honor per-context ContextShouldBeInlined attribute, which is what happens with flat CS profile.

We always encode the per-context attribute in the profile.

Where is the per-context attribute transferred from flat profile to nest profile? This is the only place I see we check ContextShouldBeInlined, but it's transferred to global FunctionSamples::ProfileIsPreinlined, not per-context attribute.

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

Yeah, ProfileIsNestedCS sounds better?

That works. Though it doesn't make sense for ProfileIsNestedCS to be true while ProfileIsCS is false. So if we use ProfileIsNestedCS, we need to either 1) rename ProfileIsCS to be ProfileIsFlatCS, or 2) replace check on ProfileIsCS with ProfileIsCS & !ProfileIsNestedCS

llvm/tools/llvm-profdata/llvm-profdata.cpp
952

how about gen-nested-cs-profile to make it explicit and differentiate from non-CS profile, especially if we go with ProfileIsNestedCS global flag.

wenlei added inline comments.Dec 14 2021, 12:42 AM
llvm/test/tools/llvm-profdata/Inputs/cs-sample-preinline.proftext
16

I meant the value change from 0/1 to 2, not the ones that were 0.

hoy added inline comments.Dec 14 2021, 9:56 AM
llvm/lib/ProfileData/SampleProf.cpp
534

Right. Here we are not necessarily converting a preinlined flat profile. When the flat profile is preinlined, we'll set the ProfileIsNestedCS flag, otherwise nodes in the context trie could have no profiles.

537–543

Did you mean would or would not? I expect the correct behavior to be that inliner will honor per-context ContextShouldBeInlined attribute, which is what happens with flat CS profile.

I mean would. Sorry for the confusion.

Where is the per-context attribute transferred from flat profile to nest profile? This is the only place I see we check ContextShouldBeInlined, but it's transferred to global FunctionSamples::ProfileIsPreinlined, not per-context attribute.

The attribute that comes with a context in a flat profile will be encoded with the context into the nested profile as well. Note that we only reset the name of the original context but nothing else. The original attribute is still there.

// Reset the child context to be contextless.
    ChildProfile->getContext().setName(OrigChildContext.getName());

See the test llvm/test/tools/llvm-profdata/cs-sample-nested-profile.test .

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

ProfileIsFlatCS and ProfileIsNestedCS sounds good to me.

llvm/test/tools/llvm-profdata/Inputs/cs-sample-preinline.proftext
16

Ah, this is the input profile of a test. I made it to be preinliner-generated. It was copied from cs-sample.proftext.

llvm/tools/llvm-profdata/llvm-profdata.cpp
952

Sounds good.

hoy updated this revision to Diff 394297.Dec 14 2021, 9:57 AM

Renaming ProfileIsCS to ProfileIsCSFlat, ProfileIsPreinlined to ProfileIsCSNested.

hoy updated this revision to Diff 394308.Dec 14 2021, 10:27 AM

Rebasing

wenlei added inline comments.Dec 14 2021, 10:59 AM
llvm/lib/ProfileData/SampleProf.cpp
534

So at least in preinliner case, it could be better to keep parent profile?

537–543

Now I see how the attributed is transferred, thanks for clarification.

llvm/lib/ProfileData/SampleProfReader.cpp
249

I'm still seeing SeenMetadata in the comment?

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

nit: there's NestedCS and CSFlat (as opposed to FlatCS) in switch, section flags and variable names , which is a bit inconsistent.

hoy updated this revision to Diff 394344.Dec 14 2021, 12:15 PM

Renaming gen-nested-cs-profile to gen-cs-nested-profile

llvm/lib/ProfileData/SampleProf.cpp
534

Yes, that's currently it is. With preinliner, every node in the context trie will come with a profile so NodeProfile will never be empty. Without preinliner, NodeProfile could be empty.

llvm/lib/ProfileData/SampleProfReader.cpp
249

Good catch, changed to DepthMetadata.

llvm/lib/Transforms/IPO/SampleProfile.cpp
1484

Yeah, we still use the switch gen-nested-cs-profile. Let me change it to gen-cs-nested-profile

wenlei accepted this revision.Dec 14 2021, 12:33 PM

lgtm, thanks.

llvm/lib/ProfileData/SampleProf.cpp
534

ok, sounds good.

This revision is now accepted and ready to land.Dec 14 2021, 12:33 PM
This revision was landed with ongoing or failed builds.Dec 14 2021, 2:41 PM
This revision was automatically updated to reflect the committed changes.

Hi,

Is anyone else having problems with this commit?
We get:

/proj/flexasic/app/llvm/8.0/bin/clang++  -march=corei7  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/ProfileData -I../lib/ProfileData -Iinclude -I../include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT lib/ProfileData/CMakeFiles/LLVMProfileData.dir/SampleProf.cpp.o -MF lib/ProfileData/CMakeFiles/LLVMProfileData.dir/SampleProf.cpp.o.d -o lib/ProfileData/CMakeFiles/LLVMProfileData.dir/SampleProf.cpp.o -c ../lib/ProfileData/SampleProf.cpp
In file included from ../lib/ProfileData/SampleProf.cpp:14:
In file included from ../include/llvm/ProfileData/SampleProf.h:17:
In file included from ../include/llvm/ADT/DenseSet.h:16:
In file included from ../include/llvm/ADT/DenseMap.h:16:
In file included from ../include/llvm/ADT/DenseMapInfo.h:19:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/tuple:39:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/array:38:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/stdexcept:39:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/string:41:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/allocator.h:46:
In file included from /proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/x86_64-unknown-linux-gnu/bits/c++allocator.h:33:
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/ext/new_allocator.h:120:23: error: no matching constructor for initialization of 'std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>'
        { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
                             ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/alloc_traits.h:530:8: note: in instantiation of function template specialization '__gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > >::construct<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>, llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
        { __a.construct(__p, std::forward<_Args>(__args)...); }
              ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_tree.h:529:23: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > > >::construct<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>, llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
              _Alloc_traits::construct(_M_get_Node_allocator(),
                             ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_tree.h:546:4: note: in instantiation of function template specialization 'std::_Rb_tree<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>, std::_Select1st<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> >, std::less<void>, std::allocator<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > >::_M_construct_node<llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
          _M_construct_node(__tmp, std::forward<_Args>(__args)...);
          ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_tree.h:2123:19: note: in instantiation of function template specialization 'std::_Rb_tree<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>, std::_Select1st<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> >, std::less<void>, std::allocator<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > >::_M_create_node<llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
        _Link_type __z = _M_create_node(std::forward<_Args>(__args)...);
                         ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_map.h:559:16: note: in instantiation of function template specialization 'std::_Rb_tree<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples>, std::_Select1st<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> >, std::less<void>, std::allocator<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > >::_M_emplace_unique<llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
        { return _M_t._M_emplace_unique(std::forward<_Args>(__args)...); }
                      ^
../lib/ProfileData/SampleProf.cpp:524:18: note: in instantiation of function template specialization 'std::map<std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples, std::less<void>, std::allocator<std::pair<const std::__cxx11::basic_string<char>, llvm::sampleprof::FunctionSamples> > >::emplace<llvm::StringRef, llvm::sampleprof::FunctionSamples &>' requested here
      SamplesMap.emplace(OrigChildContext.getName(), *ChildProfile);
                 ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:112:26: note: candidate constructor not viable: no known conversion from 'llvm::StringRef' to 'const const std::__cxx11::basic_string<char>' for 1st argument
      _GLIBCXX_CONSTEXPR pair(const _T1& __a, const _T2& __b)
                         ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:138:12: note: candidate constructor template not viable: no known conversion from 'llvm::StringRef' to 'const const std::__cxx11::basic_string<char>' for 1st argument
        constexpr pair(const _T1& __x, _U2&& __y)
                  ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:133:12: note: candidate template ignored: requirement 'is_convertible<llvm::StringRef, const std::__cxx11::basic_string<char> >::value' was not satisfied [with _U1 = llvm::StringRef]
        constexpr pair(_U1&& __x, const _T2& __y)
                  ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:144:12: note: candidate template ignored: requirement '__and_<std::is_convertible<llvm::StringRef, const std::__cxx11::basic_string<char> >, std::is_convertible<llvm::sampleprof::FunctionSamples &, llvm::sampleprof::FunctionSamples> >::value' was not satisfied [with _U1 = llvm::StringRef, _U2 = llvm::sampleprof::FunctionSamples &]
        constexpr pair(_U1&& __x, _U2&& __y)
                  ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:124:12: note: candidate constructor template not viable: requires single argument '__p', but 2 arguments were provided
        constexpr pair(const pair<_U1, _U2>& __p)
                  ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:150:12: note: candidate constructor template not viable: requires single argument '__p', but 2 arguments were provided
        constexpr pair(pair<_U1, _U2>&& __p)
                  ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:155:9: note: candidate constructor template not viable: requires 3 arguments, but 2 were provided
        pair(piecewise_construct_t, tuple<_Args1...>, tuple<_Args2...>);
        ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:206:9: note: candidate constructor template not viable: requires 4 arguments, but 2 were provided
        pair(tuple<_Args1...>&, tuple<_Args2...>&,
        ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:127:17: note: candidate constructor not viable: requires 1 argument, but 2 were provided
      constexpr pair(const pair&) = default;
                ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:128:17: note: candidate constructor not viable: requires 1 argument, but 2 were provided
      constexpr pair(pair&&) = default;
                ^
/proj/flexasic/app/llvm/8.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:108:26: note: candidate constructor not viable: requires 0 arguments, but 2 were provided
      _GLIBCXX_CONSTEXPR pair()
                         ^
1 error generated.
foad added a subscriber: foad.Dec 15 2021, 6:30 AM

@uabelho it looks like this has been fixed by 156b1e6ba84887ba92688b3ad31e400c56f707b2.

@uabelho it looks like this has been fixed by 156b1e6ba84887ba92688b3ad31e400c56f707b2.

Oh, yes it has. Thanks!