Skip to content

[Cross-platform] Statement pruning after exit node etc.

Summary

This merge request adds a node optimisation that erases statements in a block that follow a node that breaks normal program flow, specifically those that correspond to raise, exit, break, continue and goto.

However, care is taken to not delete statements that correspond to labels, temp creation and deletion and nested block nodes, since if they are still present, there's probably a good reason as to why they weren't stripped out (that and it's a little more complicated to also analyse that block's statements to see if there's anything that must be preserved).

This is primarily for pure function analysis (!645) so it can properly remove nodes if the analysed flow hits an Exit statement (usually because it was within an if-block whose condition became deterministic), but some examples of compiled code improvement appear in the packages... one appears to be vestigial code and another appears to be a minor bug.

System

  • Processor architecture: Cross-platform

What is the current bug behavior?

N/A

What is the behavior after applying this patch?

Nodes corresponding to code that will never be executed due to following instructions that break program flow are now pruned.

Relevant logs and/or screenshots

Under x86_64-win64, -O4, there are a couple of notable changes... one in itolitlsreader and one in fpmkunit.

In the former, two non-volatile registers are no longer required and hence aren't pushed to the stack (the peephole optimizer mostly optimises out their use before, but it cannot remove the push/pop pairs). The Pascal code in question:

function TStreamChunk.Seek(Offset: Longint; Origin: Word): Longint;
var
  NewPosition: LongInt;
begin
  Case Origin of
    soFromBeginning : NewPosition:=Offset;
    soFromEnd       : NewPosition:=FSize+Offset;
    soFromCurrent   : NewPosition:=NewPosition+Offset;
  end;
  {$IFDEF DEBUG_HELP2}
  //WriteLn('WantSeek = ', Offset,' Size = ', FSize);
  {$ENDIF}
  FPos:=NewPosition;
  Exit(NewPosition);
  if NewPosition < 0 then NewPosition := 0;
  if NewPosition >= FSize then NewPosition := FSize-1;
  FStream.Position := FBasePos+NewPosition;
  Result := FStream.Position - FBasePos;
  FPos := Result;
  {$IFDEF DEBUG_HELP2}
  //WriteLn('Pos = ', fpos);
  {$ENDIF}
end;

In this case, everything following Exit(NewPosition); is stripped out (it seems to be vestigial code from a previous implementation, since the internal stream is repositioned whenever Read is called... although the clamping of the new position isn't applied in this case).

In the fpmkunit, the reserved stack space is 16 bytes smaller. The Pascal code in question:

function TBuildEngine.NeedsCompile(APackage: TPackage): Boolean;
Var
  I : Integer;
  P : TPackage;
  D : TDependency;
  CompileReason: string;
begin
  Result:=False;

  // Forced recompile?
  if FForceCompile then
    begin
    Result:=true;
    CompileReason:=SDbgForcedCompile;
    end;

  // Recompile because of Package Dependencies?
  if not Result then
    begin
       I:=0;
       For I:=0 to APackage.Dependencies.Count-1 do
         begin
           D:=APackage.Dependencies[i];
           if (D.DependencyType=depPackage) and
              (Defaults.CPU in D.CPUs) and (Defaults.OS in D.OSes) then
             begin
               P:=TPackage(D.Target);
               if Assigned(P) then
                 begin
                   Result:=(P.State=tsCompiled);
                   if Result then
                     begin
                     break;
                     CompileReason:=Format(SDbgPackageDepRecompiled,[P.Name]);
                     end;
                 end;
             end;
         end;
    end;

  // Recompile a Target of this package?
  If Not Result then
    begin
      GPathPrefix := APackage.Directory;
      try
        for i:=0 to APackage.Targets.Count-1 do
          begin
            Result:=NeedsCompile(APackage,APackage.Targets.TargetItems[i]);
            if Result then
              begin
              break;
              CompileReason:=Format(SDbgTargetHasToBeCompiled,[APackage.Targets.TargetItems[i].Name]);
              end;
          end;
      Finally
        GPathPrefix := '';
      end;
    end;

  if result then
    Log(vlDebug,SDbgMustCompile,[APackage.Name, CompileReason]);
end;

In this instance, the instructions after the Break statements are removed. I will question as to if this is actually a bug though because if FForceCompile is False but the specified unit needs to be recomplied, CompileReason is left undefined.

Edited by J. Gareth "Kit" Moreton

Merge request reports