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