View Issue Details

IDProjectCategoryView StatusLast Update
0038113FPCCompilerpublic2020-11-24 13:55
ReporterOkobaPatino Assigned ToJonas Maebe  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Summary0038113: Reason for not inlining
DescriptionCan FPC report why this code is not inlined?
I May think it is because the implementation of Test1 came after Test2, and I guess somewhere in the compiler, it knows this, so why not report it back? I only think this as I read it somewhere, and no documentation I can link to, so a reason for not inlining a function can be useful. Is it feasible?


unit Unit1;

{$mode objfpc}{$H+}

interface

function Test2(const A: PChar): PChar; inline;
function Test1(const A: PChar): PChar; inline;

implementation

function Test2(const A: PChar): PChar;
begin
  Result := Test1(A);
end;

function Test1(const A: PChar): PChar;
begin
  Result := A + 1;
end;

end.


Another case is this:

program Project1;
 
  function Test1(A: PChar): PChar; inline;
  begin
    Result := A + 1;
  end;
 
var
  S: String;
  P: PChar;
begin
  S := 'Test';
  P := Pointer(S); //Inlined
  P := Test1(P);
  P := Test1(Pointer(S)); //Not inlined
end.

I think it is not inlined because I am passing unmanaged type of a managed type, and again I only think this because I read this here:
http://free-pascal-general.1045716.n5.nabble.com/inlining-functions-depending-on-implementation-only-functions-td5732866.html#a5732888
So again, a reason along the hint can be helpful.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget-
Attached Files

Activities

Jonas Maebe

2020-11-22 13:12

manager   ~0127094

Trunk already does this. Please always mention the version you are using.

OkobaPatino

2020-11-22 13:24

reporter   ~0127097

I am using the trunk; sorry for not mentioning that. Here is the hint compiler (latest trunk) says to me for both cases:
Note: Call to subroutine "function Test1(const A:PChar):^Char;" marked as inline is not inlined
If I get the hint of this message, it is:
The directive inline is only a hint to the compiler. Sometimes the compiler ignores this hint, a subroutine marked as inline is not inlined. In this case, this hint is given. Compiling with -vd might result in more information why the directive inline is ignored.
And I tried passing -vd to the custom options of the project and the same message will be visible, no explanation about the reason.

Jonas Maebe

2020-11-22 13:29

manager   ~0127099

The reason is mentioned as a note (-vn)

OkobaPatino

2020-11-22 13:34

reporter   ~0127100

I tried -vn too, no specific reason as a note, just saying that it is not inlined. Can I ask what is the note you are getting?

Sven Barth

2020-11-22 13:46

manager   ~0127101

Note: the reason that the first one is not inlined is that the body of Test1 has not yet been parsed when you call it inside Test2. FPC does a single parsing pass through the code, not multiple like a C or C++ compiler usually does (that's one of the the main points of Pascal after all), thus if the body is not yet available the compiler can't inline it. So swap the order of Test1 and Test2 and the compiler will happily inline it.

Jonas Maebe

2020-11-22 13:50

manager   ~0127103

Last edited: 2020-11-22 13:51

View 2 revisions

Ah, I only looked at the second program (where the reason is indeed reported by -vd, not -vn; -vn reports other kinds of reasons that are detected elsewhere in the compiler).

I think the first case should actually not be reported at all, or only with -vd. The reason is that if the helper unit itself depends on other units, or calls the routine in its implementation before the implementation has been encountered, you get a bunch of notes which may be impossible (and/or uninteresting) to "fix".

In general, I don't like these inlining hints at all because they cause people to spend a lot of time reorganising their code for little or no gains. Especially with LTO, where inlining happens transparently in the background (regardless of inlining modifiers), this is a huge waste of programmer time and detracts from actually important aspects of the code. Especially since they're indeed only compiler hints and since what a compiler can/will inline or not under which circumstances can change across versions, you may spend time reorganising your code in one version and then have to part of it again for another version.

Edit: note that LTO-style inlining is currently only supported when using the LLVM backend. There's also experimental auto-inlining support in the compiler itself, but it's not yet mature enough to use.

OkobaPatino

2020-11-22 14:12

reporter   ~0127105

@PascalDragon, I am not getting the hint you mentioned. I attached the project, and I am compiling it with FPC and Lazarus's latest trunk version. Probably I am missing something.
@Jonas, probably you are saying that as you knew how the compiler works. As a user, I do not understand what this hint is and what I should do if I like to improve the code. Even all I thought was because I read your notes in the mail list, and I guess many ones will be confused why FPC shows that hint. The hints are handy when you are sensitive about a method's performance, and reporting a hint with an apparent reason makes development much more effortless.
InlineHint.zip (1,765 bytes)

Sven Barth

2020-11-22 16:43

manager   ~0127108

@OkobaPatino: I didn't say anything about a hint. Aside from the "is not inlined" hint the compiler does not print anything more for this. I only explained to you why it happened.

@Jonas Maebe: I don't necessarily agree that these hints are bad. Especially this one that occurs when the body is not yet parsed is important in my opinion as most people are simply not aware that the order is important and then wonder why it's not inlined (and it also helped me correct some code in e.g. the Rtti unit). And it also points out locations where we might want to improve the inlining.

Jonas Maebe

2020-11-22 17:01

manager   ~0127111

> As a user, I do not understand what this hint is

That's why I think this shouldn't be shown as a hint at all. At most, it should be something shown when using -vd.

> The hints are handy when you are sensitive about a method's performance

With modern processors smaller code size is often much more important than getting rid of a call/returns to increase performance (cache is everything). That is why "inline" is almost completely ignored by C/C++ compilers nowadays, and they purely use heuristics. At some point, the same will happen in FPC.

Benito van der Zander

2020-11-22 17:26

reporter   ~0127112

What about this in r47006:

function strbeginswith(const strToBeExaminated,expectedStart: string): boolean;
var
  strLength, expectedLength: SizeInt;
begin
  strLength := length(strToBeExaminated);
  expectedLength := length(expectedStart);
  result:= (strLength >= expectedLength) and ( (expectedStart='') or (CompareByte(PByte(strToBeExaminated)^, pbyte(expectedStart)^, expectedLength ) = 0));
end;

var s, t: string;
  writeln(strBeginsWith('xy','x'));
  writeln(strBeginsWith(s, t));
  writeln(strBeginsWith('y','x'));

None are inlined, but the first two calls produce NO warning.

Sven Barth

2020-11-22 18:41

manager   ~0127117

@Jonas Maebe: Using heuristics would mean that the node tree of every function would need to be put into the PPU. I personally don't see that happening which is why I'm rather sceptical of the AutoInline optimization as well.

@Benito van der Zander: Your function is not declared as inline, so of course the compiler does not inline it. If you mark it as inline then it will inline it (the AutoInline optimization won't inline it either).

OkobaPatino

2020-11-22 18:43

reporter   ~0127118

@PascalDragon, I thought you are answering my question about compiler showing the reason.
@Jonas So to be clear, does compiler show the actual reason on why this it can not inline? And if yes, can you please check my sample?

Jonas Maebe

2020-11-22 19:16

manager   ~0127120

> Using heuristics would mean that the node tree of every function would need to be put into the PPU.
Only for potential inlining candidates. That's something that would be part of the heuristics.

> So to be clear, does compiler show the actual reason on why this it can not inline?
The compiler shows the reason except if it's because the implementation not yet available.

OkobaPatino

2020-11-22 19:21

reporter   ~0127121

So can I ask what is message that FPC shows you in both reported case? As I can not see the reason, just the note that it can not be inlined.

Jonas Maebe

2020-11-22 21:29

manager   ~0127123

With -vhd, I get this message:

InlineHint.lpr(17,8) Not inlining "Test1", invocation parameter contains an unsafe/unsupported construct

There's only one message, because the other one is a case of "the implementation not yet available", where no message is shown (as mentioned in my previous reply).

Benito van der Zander

2020-11-22 21:54

reporter   ~0127124

> @Benito van der Zander: Your function is not declared as inline, so of course the compiler does not inline it. If you mark it as inline then it will inline it (the AutoInline optimization won't inline it either).

It is not? But the third call gives a warning

Right. I copied the wrong function

It is this one:

function strlnsequal(p1,p2:pansichar;l2: SizeInt):boolean;
var i:SizeInt;
begin
  for i:=0 to l2-1 do begin
    if p1[i]<>p2[i] then
      begin result := false; exit; end;
    if p1[i]=#0 then
      begin result := i = l2-1; exit; end
  end;
  result:=true;
end;

function strbeginswith(const p: pansichar; const expectedStart: string): boolean; inline;
var
  q: PAnsiChar;
begin
  q := pansichar(pointer(expectedStart));
  result:=(expectedStart='') or (strlnsequal(p, q, length(expectedStart)));
end;


Is it treats 'a' as char rather than string? And then it prefers to convert char to pchar over converting it to string, and then it cannot inline the conversion, because?

OkobaPatino

2020-11-23 09:24

reporter   ~0127130

@Jonas
I'm afraid that I did not know how you get this message; I ran the project I attached with FPC and Lazarus Trunk (of two days ago) with custom options of -vd -vn and -vhd, and the only messages I get are:
Unit1.pas(14,13) Note: Call to subroutine "function Test1(const A:PChar):^Char;" marked as inline is not inlined
InlineHint.lpr(17,8) Note: Call to subroutine "function Test1(A:PChar):^Char;" marked as inline is not inlined
Hint of both is:
The directive inline is only a hint to the compiler. Sometimes the compiler ignores this hint, a subroutine marked as inline is not inlined. In this case, this hint is given. Compiling with -vd might result in more information why the directive inline is ignored.
I did try with only -vd, only -vn too and no luck.

OkobaPatino

2020-11-23 09:25

reporter   ~0127131

@BeniBela I guess the problem in your case is that in strbeginswith you are calling strlnsequal, and it is not inline, so the compiler can not inline strbeginswith.

Jonas Maebe

2020-11-23 22:02

manager   ~0127146

I don't use Lazarus, I compile on the command line:

$ ppcx64 -l -Mobjfpc -B -Sh -vhd InlineHint.lpr
...
Found source file name "InlineHint.lpr"
Free Pascal Compiler version 3.3.1 [2020/11/22] for x86_64
Copyright (c) 1993-2020 by Florian Klaempfl and others
Compiler OS: Darwin for x86_64
Target OS: Darwin for x86_64
Compiling InlineHint.lpr
Compiling unit1.pas
unit1.pas(3,2) Handling switch "$MODE"
unit1.pas(3,16) Handling switch "$H+"
unit1.pas(13,1) procedure/function Test2(const PChar):^Char;
unit1.pas(18,1) procedure/function Test1(const PChar):^Char;
Assembling unit1
InlineHint.lpr(6,3) procedure/function Test1(PChar):^Char;
Parsing internally generated code: procedure main(const ARGC:LongInt;const ARGV:^^Char;const ARGP:^^Char); CDecl;begin __FPC_IMPL_EXTERNAL_REDIRECT__FPC_SYSTEMMAIN(&ARGC,&ARGV,&ARGP) end;
procedure/function $main(const LongInt;const ^^Char;const ^^Char); CDecl;
>>>>>> InlineHint.lpr(17,8) Not inlining "Test1", invocation parameter contains an unsafe/unsupported construct <<<<<<
InlineHint.lpr(3,6) Hint: Unit "Unit1" not used in InlineHint
Assembling inlinehint
Linking InlineHint
40 lines compiled, 0.1 sec
3 hint(s) issued

OkobaPatino

2020-11-24 08:25

reporter   ~0127150

I attached a screenshot of the messages. It is from FPC and Lazarus Trunk in Windows. Can anyone else check the project I attached before with Lazarus?
Screenshot.png (14,023 bytes)   
Screenshot.png (14,023 bytes)   

Sven Barth

2020-11-24 13:55

manager   ~0127157

@Benito van der Zander: if I put the functions you provided into a program and use your previous example (with String replaced by PAnsiChar) then the compiler inlines the first two calls, but not the third and for the third it provides a note. So please provide a full example (attached here or maybe a new bug report) that shows the problem.

Issue History

Date Modified Username Field Change
2020-11-22 10:16 OkobaPatino New Issue
2020-11-22 13:12 Jonas Maebe Assigned To => Jonas Maebe
2020-11-22 13:12 Jonas Maebe Status new => resolved
2020-11-22 13:12 Jonas Maebe Resolution open => fixed
2020-11-22 13:12 Jonas Maebe FPCTarget => -
2020-11-22 13:12 Jonas Maebe Note Added: 0127094
2020-11-22 13:24 OkobaPatino Note Added: 0127097
2020-11-22 13:29 Jonas Maebe Note Added: 0127099
2020-11-22 13:34 OkobaPatino Note Added: 0127100
2020-11-22 13:46 Sven Barth Note Added: 0127101
2020-11-22 13:50 Jonas Maebe Note Added: 0127103
2020-11-22 13:51 Jonas Maebe Note Edited: 0127103 View Revisions
2020-11-22 14:12 OkobaPatino Note Added: 0127105
2020-11-22 14:12 OkobaPatino File Added: InlineHint.zip
2020-11-22 16:43 Sven Barth Note Added: 0127108
2020-11-22 17:01 Jonas Maebe Note Added: 0127111
2020-11-22 17:26 Benito van der Zander Note Added: 0127112
2020-11-22 18:41 Sven Barth Note Added: 0127117
2020-11-22 18:43 OkobaPatino Note Added: 0127118
2020-11-22 19:16 Jonas Maebe Note Added: 0127120
2020-11-22 19:21 OkobaPatino Note Added: 0127121
2020-11-22 21:29 Jonas Maebe Note Added: 0127123
2020-11-22 21:54 Benito van der Zander Note Added: 0127124
2020-11-23 09:24 OkobaPatino Note Added: 0127130
2020-11-23 09:25 OkobaPatino Note Added: 0127131
2020-11-23 22:02 Jonas Maebe Note Added: 0127146
2020-11-24 08:25 OkobaPatino Note Added: 0127150
2020-11-24 08:25 OkobaPatino File Added: Screenshot.png
2020-11-24 13:55 Sven Barth Note Added: 0127157