Hello,

Currently, when assigning types that need helper routines (records, Windows widestrings and variants), the compiler converts assignment into a helper call immediately. If this is moved from typecheckpass to pass1, certain assignments get the chance to be optimized away. See the patch attached.

I had run the testsuite with the patch, the results were the same as with unpatched compiler.

This works, but the desired goal is achieved only partially. For example, in packages/base/netdb/uriparser.pp there are 4 cases that may (and need to) be optimized:
1) In function ParseURI(const URI: String): TURI; - assignment to the function result;
2,3) In ResolveRelativeURI() - the first two statements:

   Base := ParseUri(BaseUri);
   Rel := ParseUri(RelUri);

4) In URIToFilename() - also the first statement:
   U := ParseURI(Uri);

Only the case 4 is optimized by the patch. In cases 2 and 3, I guess that compiler assumes that address of Base and Rel is taken (although actually it is taken much later). In case 1, I don't know. Maybe someone can give me a hint how these cases can be handled?

Regards,
Sergei

That's because the compiler only performs a context-insensitive escape analysis (i.e., it is as conservative as possible: when you ask whether the address of a variable is taken, it will give an answer which is correct/safe regardless of which statements have already been executed).

To reduce the conservativeness, you have to use DFA and record the individual state changes (or states) of all variables at all points where they may be needed.

In case 1, I don't know. Maybe someone can give me a hint how these cases can be handled?

In case 1, I think it's because the function result itself is already basically a var-parameter passed to ParseURI and is treated like any other var parameter (i.e., a parameter which may also be accessible in another way, e.g. because a global variable was passed as var parameter). However, I guess the function result is a special case, since either
a) the address of a temp is passed, which is by definition not accessible from elsewhere (unless you write unsupported and broken code) b) the address of a variable is passed, but this is only done if the escape analysis determines that this variable is not accessible from elsewhere

So I guess a special case for function results could be added.

Jonas

Thanks, I've applied your patch. I'm also testing a patch for the function result handling (there was another issue: the fact that the fake "result" variable is actually implemented as an absolute variable referring to the actual function result, causing an extra absolute type conversion node to appear around it), which optimizes a lot more in general (in the compiler it removes a lot of unnecessary shortstring copy operations).

Jonas

I don't know. I'm on a network home directory, so there's quite a bit of fluctuation.

Jonas

Does it have a measurable speed impact ?

Michael.

Another thought about 'result' came to my mind:
Whenever the function returns its result in parameter, this parameter is always properly allocated and finalized by the caller. Therefore, the function itself does not need to implicitly finalize its result in case of exception. Am I right? The patch for this seems trivial (see the attachment).

The next logical step would be returning all automated types in parameters, which would get rid of quite a pack of implicit finalization code.

Sergei

Possibly, yes, I'll look at it.

The next logical step would be returning all automated types in parameters, which would get rid of quite a pack of implicit finalization code.

And thoroughly break Delphi compatibility at the same time :) Especially for assembler routines this could result in quite a few problems.

Jonas

Hmm... But Delphi does the very same thing - returns strings and variants in parameters - so the compatibility is expected to increase?

Sergei

Are you sure with the strings?

I didn't know that. In that case, it may indeed increase Delphi compatibility (but decrease backwards FPC compatibility).

Jonas

Florian Klaempfl wrote

Are you sure with the strings?

Yes. Any function that returns a string is supplied with one more argument than it was declared. After calling the function, register eax (which is used to contain the non-parameter result) is not used. Here is a simple example:

function TDOMCharacterData.GetNodeValue: DOMString;
begin
   Result := FNodeValue;
end;

push ebx
push esi
mov esi, edx <- address of result
mov ebx, eax <- Self
mov eax, esi
mov edx, [ebx+14] <- FNodeValue
call @WStrAsg
pop esi
pop ebx
ret

Sergei

I'am not sure but there might be a reason why we did it this way.

For what it's worth, I noticed that difference as well. The following code snippet describes Delphi's behavior (in regards to passing results as parameters and limited to types covered by RTTI):

if Assigned(imeResult.ipeInfo) then
  if (imeResult.ipeInfo.Kind in [tkString, tkLString, tkWString, tkDynArray,
tkMethod, tkInterface, tkVariant])
    or ( (imeResult.ipeInfo.Kind in [tkArray, tkRecord, tkSet])
      and (imeResult.ipeAlignedSize > 4)
    ) then begin
      SetLength(imeParams, Succ(Length(imeParams)));
      IntfParam := @imeParams[High(imeParams)];
      IntfParam^ := imeResult;
      IntfParam.ipeFlags := [pfOut];
      FillParamInfo(IntfMethod, IntfParam);
      FillChar(imeResult, SizeOf(imeResult), 0);
      imeResultIn := riInParam;
    end;

The Delphi approach leads to faster and simpler code as the result doesn't need to be protected by an implicit try/finally.

Cheers,
Thorsten

Ok, so I guess we can change this. Anybody volunteer to implement it :)?

Florian Klaempfl wrote

Ok, so I guess we can change this. Anybody volunteer to implement it :)?

Well, if there is no hard time schedule imposed on this, I can try. I've started all this, after all :)

Sergei

I had investigated the issue. Surprisingly enough, changing string return convention does not cause any test failures on both Linux and Windows. The only issue is that statements like 's := Copy(s, ...)' start leaking memory, but that is easily fixed.

However, it seems that I understood reasons for the current implementation. It is exactly the same reason for which I've started this topic - until assignments of function return value can be properly optimized, the current implementation is more effective than Delphi's. Changing the convention right now will replace implicit exception frame in callee by a temp variable (with the same implicit frame) in caller, resulting in no performance I guess that I should turn straight to DFA. However, some questions arise even before the start:

1. I see no place in compiler source where cs_opt_nodedfa flag is being set. How is it supposed to work?

2. DFA is currently disabled for functions with exception frames, which makes it useless for resolving the issue with automated types assignments. Any hints how to work around this?

Sergei

Help me, why do we need dfa :)?

1. I see no place in compiler source where cs_opt_nodedfa flag is being set. How is it supposed to work?

It is only set together with -O2

2. DFA is currently disabled for functions with exception frames, which makes it useless for resolving the issue with automated types assignments. Any hints how to work around this?

Well, when having exception frames, dfa gets rather useless:

try
  a:=1;
  b:=c;
  d:=e;
  ...
except/finally
  ...
end;

when entering the except block, all variables being assigned in the try block live because from every instruction in the try block a execution path goes to the except/finally block.

But turning it on and creating one is no real problem, it only pretty useless ;)

Florian Klaempfl wrote

Help me, why do we need dfa :)?

I am not sure that we need the particular DFA from optdfa.pas, but something to reduce conservativeness of escape analysis is definitely needed. Otherwise, temps that are needed for just one particular case of x:=f(x) are being created almost everywhere.
DFA comes from Jonas's advice from one of the first messages:

--------------------

If I understand right, it means that except block cannot be optimized, but inside the 'try' part normal rules still apply? E.g. in "a:=1" line, we know that "d" is not assigned yet?
If a variable is not referenced within except block, it may be treated as if except block does not exist?

But turning it on and creating one is no real problem, it only pretty useless ;)

Sergei

When Jonas posted this message first, I didn't understand because I thought I read it too quick, but after carefull reading, I still don't understand it :) The first paragraph talks about alias analysis while the second talks about dfa.

Let it consider from a practical point :) Which piece of code makes too conservative assumptions? The big if in ncgld.maybechangetemp?

You need the latter to perform the former in a context-sensitive way. Context-sensitive means that the outcome of your analysis is tailored to a particular point in the procedure, rather than valid in any context/at any point in the procedure.

However, you can only do this if you know what has been executed before you reached that particular point, and that is where DFA comes in.

Let it consider from a practical point :) Which piece of code makes too conservative assumptions? The big if in ncgld.maybechangetemp?

No, it's the big if in tcallnode.maybe_create_funcret_node. The conservative/context-insensitive parts are the ti_addr_taken and addr_taken checks. These are computed globally for the entire current procedure, rather than statement per statement based on which statements can have been executed before the current statement is executed.

Jonas

So indeed an alias analysis is required. Doing a simple one shouldn't be too hard but I fear it will be too conservative as well ...

What's a typical example of code which is compiled too conservative? Maybe we can reduce the setting of ti_addr_taken?

I read too quickly over your message, what I wanted to say was that your need dfa to perform *escape analysis* in a context-sensitive way. Alias analysis can be used to further refine the results (namely set the ti_addr_taken/addr_taken stuff in less cases, or replace them with another less global mechanism), but is not required.

function f: trecord; forward;
procedure t(var r: trecord); forward;
procedure test;
var
   r: trecord;
begin
   r:=f; {1}
   t(r); {2}
end;

{1} cannot be optimized because the address of r is taken in {2}.

Alias analysis could moreover allow optimization of cases like this:

function f(const rec: trecord): trecord; forward;
procedure t(var r: trecord); forward;
procedure test;
var
   r1, r2: trecord;
   p: precord;
begin
   r2.field1:=3;
   p^.field1:=2;
   r1:=f(r2);   {1}
   ;
   r1:=f(p^);   {2}
end;

{1} could be optimized because we would know that while p^ aliases r1, it has not escaped. {2} could be optimized because we would know that p aliases r2 and not r1.

There's of course also the issue of dynamic arrays, where you can have aliasing without taking the address of a variable (and in principle also with ansi/widestrings due to pchar/pwidechar casts which avoid the uniquestring calls).

Maybe we can reduce the setting of ti_addr_taken?

Not without either alias analysis or compromising correctness in certain (valid) special cases.

Jonas