[fpc-devel] Proposal to change GetNextInstructionUsingReg

[J. Gareth Moreton]

Hi everyone,

So during my last optimisation change, I tried changing this method in 
order to improve efficiency and functionality, unaware that other 
platforms and functions may depend on its very particular behaviour.

On x86 at least, the function takes a starting instruction and a 
register, and returns True so long as some assembler list entry was 
found.  It also has an 'out' parameter that returns the first 
instruction it finds that uses the specified register.  The full 
behaviour is as follows:

- Starting with p, call GetNextInstruction.
- If GetNextInstruction returns False - which occurs only if the 
assembler list reaches the end (i.e. p.Next = nil) - drop out and return 
False, otherwise set Result to True.
- With the next entry obtained, set the out parameter to this entry.
- If not on -O3 or above, drop out (overall, this method is rather 
expensive to call overall, so -O1 and -O2 only check the next instruction).
- If it is not an instruction (e.g. a label or a marker), drop out.
- If the instruction uses the register specified (calls 
RegInInstruction, which returns True for any instructions that have 
Ch_All set, such as CALL), drop out.
- If the instruction is a call or a jump (conditional or unconditional), 
drop out.
- Otherwise, call GetNextInstruction and repeat.

When I look at the design of this method, I feel like that perhaps it 
should return False if it fails to find an instruction that actually 
uses the specified register, as doing so will remove the need to analyse 
the out parameter for validity and to see if it's what you expected.  
Optimisations check to see if the method returned True anyway before 
proceeding, so it should not break existing code.

Additionally, and this was suggested by Florian privately, I would like 
to insert an additional Boolean parameter called IgnoreConditionalJumps, 
with a default value of False.  For some optimisations, conditional 
jumps don't have to act as instruction fences, and looking past these 
can permit more potential optimisations.  Nevertheless, ignoring 
conditional jumps by default might break optimisations that expect the 
method to drop out instead.

If GetNextInstructionUsingReg returns True only if it lands on an 
instruction that uses the specified register, it can be guaranteed that 
the out parameter is of type ait_instruction and contains some kind of 
operation that uses the register (or a 'fence' instruction like CALL).  
Returning False immediately indicates that the specified register is not 
in use within the nearby code.

Admittedly, the first problem is designing a cross-platform function.  
On x86, the method isn't even virtual.  My first version, which was 
rejected mainly due to me changing it without warning and not putting 
any comments in (I personally felt the code was readable, but this may 
not be the case):

function TX86AsmOptimizer.GetNextInstructionUsingReg(Current: tai; out 
Next: tai; reg: TRegister): Boolean;
   begin
      if not (cs_opt_level3 in current_settings.optimizerswitches) then
        Result :=
          GetNextInstruction(Next, Next) and
          (Next.typ = ait_instruction) and
          RegInInstruction(reg,Next)
      else
        begin
          Result := False;
          while GetNextInstruction(Next, Next) and (Next.typ = 
ait_instruction) do
            if RegInInstruction(reg, Next) then
              begin
                Result := True;
                Exit;
              end
            else
              if taicpu(Next).opcode = A_JMP then
                { Immediately stop upon finding an unconditional jump }
                Exit;
        end;
   end;

With an "IgnoreConditionalJumps" parameter, the check for A_JMP would 
only be this if said parameter was True, otherwise it would call 
"is_calljmp" on the opcode instead.  There's probably a good way to 
write this function in a cross-platform way, so long as I comment 
everything so the process is clear.

To conclude, I would like to change GetNextInstructionUsingReg in the 
following way:

- Add a 4th parameter named "IgnoreConditionalJumps" with a default 
value of False (so existing code doesn't break). Setting this to True 
will allow the method to look past conditional jumps for instructions 
that use the specified register.
- Return False if the method fails to find an instruction that uses the 
register (because it reached a label first, for example).

How do the developers and administrators feel?

Gareth aka. Kit