[cfe-dev] Question about 'CodeGenFunction::EmitGotoStmt'

[John McCall via cfe-dev]

On 30 Jun 2021, at 9:54, paul.robinson at sony.com wrote:
>> -----Original Message-----
>> From: jingu kang <jaykang10 at gmail.com>
>> Sent: Wednesday, June 30, 2021 9:00 AM
>> To: Robinson, Paul <paul.robinson at sony.com>; rjmccall at apple.com
>> Cc: Jingu.Kang at arm.com; cfe-dev at lists.llvm.org
>> Subject: Re: [cfe-dev] Question about 'CodeGenFunction::EmitGotoStmt'
>>
>> Additionally, I am looking at this case from perlbench of spec2017.
>>
>> 2021년 6월 30일 (수) 오후 12:49, jingu kang 
>> <jaykang10 at gmail.com>님이 작성:
>>>
>>> Hi Paul
>>>
>>> Thanks for your kind explanation.
>>>
>>> I have investigated the situation more.
>>>
>>> clang pushes cleanup for lifetime.marker of local variable. When the
>>> cleanup is popped, the fixup for the goto statement is updated with
>>> the cleanup.
>>>
>>> From a spec benchmark, it looks like it causes more instructions as
>> below.
>>> For AArch64
>>> <test+11400>     ldr    x8, [x19, #16]
>>> <test+11404>     lsl    x9, x24, #4
>>> <test+11408>     ldrh   w8, [x8, x9]
>>> <test+11412>     cbz    w8, 0x34ce94 <test+12868> --> goto cleanup1
>>>
>>> cleanup1:
>>> <test+12868>     mov    w8, #0x8
>>> <test+12872>     b      0x34d034 <test+13284>  --> goto cleanup2
>>>
>>> cleanup2:
>>> <test+13284>     cmp    w8, #0x6
>>> <test+13288>     b.eq   0x34edc8 <test+20856>
>>> <test+13292>     cmp    w8, #0x8
>>> <test+13296>     b.eq   0x34edc8 <test+20856> --> goto destination
>>>
>>> destination:
>>> <test+20856>     ldr    w8, [sp, #392]
>>> <test+20860>     tbz    w8, #0, 0x34ee28 <test+20952>
>>> <test+20864>     ldr    x9, [sp, #296]
>>> <test+20868>     cbz    x9, 0x34f4f8 <test+22696>
>>>
>>> We could disable the lifetime.marker with the cmd option "-Xclang
>>> -disable-lifetime-markers" but it looks bad for llvm passes...
>>>
>>> At this moment, I am looking at below comment and code...
>>>
>>> /// A branch fixup.  These are required when emitting a goto to a
>>> /// label which hasn't been emitted yet.  The goto is optimistically
>>> /// emitted as a branch to the basic block for the label, and (if it
>>> /// occurs in a scope with non-trivial cleanups) a fixup is added to
>>> /// the innermost cleanup.  When a (normal) cleanup is popped, any
>>> /// unresolved fixups in that scope are threaded through the 
>>> cleanup.
>>> struct BranchFixup {
>>>
>>> void CodeGenFunction::PopCleanupBlock(bool 
>>> FallthroughIsBranchThrough)
>>> ...
>>>       // IV.  Pop the cleanup and emit it.
>>> ...
>>>       // Optimistically hope that any fixups will continue falling
>> through.
>>>       for (unsigned I = FixupDepth, E = 
>>> EHStack.getNumBranchFixups();
>>>            I < E; ++I) {
>>>         BranchFixup &Fixup = EHStack.getBranchFixup(I);
>>>         if (!Fixup.Destination) continue;
>>>         if (!Fixup.OptimisticBranchBlock) {
>>>
>> createStoreInstBefore(Builder.getInt32(Fixup.DestinationIndex),
>>>                                 getNormalCleanupDestSlot(),
>>>                                 Fixup.InitialBranch);
>>>           Fixup.InitialBranch->setSuccessor(0, NormalEntry);
>>>         }
>>>         Fixup.OptimisticBranchBlock = NormalExit;
>>>       }
>>>
>>> Is it possible to avoid adding a fixup to the cleanup with
>>> lifetime.marker or something like that? If I missed something, 
>>> please
>>> let me know.

We could treat branches through lifetime scopes differently, but
then we’d emit worse lifetime annotations, which could interfere
with optimization in other ways.  I don’t know what problem you’re
seeing, but generally all the branch-fixup stuff is pretty simple
to eliminate with jump-folding.  I don’t think this is a problem we
can solve in the frontend without major rearchitecture.

John.

>
> Unfortunately I am unfamiliar with the lifetime marker mechanism,
> so I cannot help you there.
> --paulr
>
>>>
>>> Thanks
>>> JinGu Kang
>>>
>>> 2021년 6월 29일 (화) 오후 6:40, <paul.robinson at sony.com>님이 
>>> 작성:
>>>>
>>>>> From a spec benchmark, I have seen that the ‘goto’ statement 
>>>>> goes to
>>>>> its destination through the cleanup function as below.
>>>>>
>>>>> void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
>>>>>   // If this code is reachable then emit a stop point (if 
>>>>> generating
>>>>>   // debug info). We have to do this ourselves because we are on 
>>>>> the
>>>>>   // "simple" statement path.
>>>>>   if (HaveInsertPoint())
>>>>>     EmitStopPoint(&S);
>>>>>   EmitBranchThroughCleanup(getJumpDestForLabel(S.getLabel()));
>>>>> }
>>>>>
>>>>> I guess we could emit the branch for the target directly. If
>> possible,
>>>>> can someone let me know why the goto statement has to go through 
>>>>> the
>>>>> cleanup function please? If I missed something, please let me 
>>>>> know.
>>>>
>>>> I haven't looked, but one reason would be if the 'goto' transfers 
>>>> out
>>>> of a block that has a local variable with a destructor; the 
>>>> destructor
>>>> has to run before control transfers to the 'goto' label.  If there 
>>>> are
>>>> no such local variables, there is no cleanup to do, and the 'goto'
>>>> becomes a simple branch.
>>>> --paulr
>>>>