[x86] Bug fix for "J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret" optimisation

Summary

This merge request fixes #39922 (closed) where a mov $0,%reg instruction was inserted on the wrong side of a SETcc instruction as part of an optimisation.

System

• Processor architecture: i386, x86_64

What is the current bug behavior?

In situations where the "J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret" optimisation is applied to a destination register that is larger than 8 bits in size, a mov $0,%reg instruction is generated to zero-extend the result set by the SETcc instruction. However, due to some faulty logic, this instruction was inserted after the SETcc instruction rather than before, thereby nullifying its effect and causing the result to always be zero.

What is the behavior after applying this patch?

• The mov $0,%reg instruction is now in the correct place.
• Additionally, under -Os, a MOVZX instruction is favoured instead.

Relevant logs and/or screenshots

For the new tw39922.pp test - before:

	...
# [13] ((AnsiChar(c) >= 'a') and (AnsiChar(c) <= 'z'))
	leal	-97(%ecx),%eax
	# Register ecx released
	# Register rflags allocated
	cmpb	$25,%al
	# Register al released
# Peephole Optimization: J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret
	setbeb	%al
	movl	$0,%eax ; <-- Instruction essentially nullifies the setbeb instruction.
	ret
	# Register rflags released
.Lj5:
	# Register eax allocated
# [15] Result := 1
	movl	$1,%eax
# Peephole Optimization: Converted JMP to RET as part of SETcc optimisation (1st jump)
	ret
	...

After:

	...
# [13] ((AnsiChar(c) >= 'a') and (AnsiChar(c) <= 'z'))
	leal	-97(%ecx),%eax
	# Register ecx released
	# Register rflags allocated
	cmpb	$25,%al
	# Register al released
# Peephole Optimization: J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret
	# Register rax allocated
	movl	$0,%eax
	setbeb	%al
	ret
	# Register rflags,rax released
.Lj5:
	# Register eax allocated
# [15] Result := 1
	movl	$1,%eax
# Peephole Optimization: Converted JMP to RET as part of SETcc optimisation (1st jump)
	ret
	...

For the new tw39922a.pp test that tests the -Os version - before:

	...
# [13] ((AnsiChar(c) >= 'a') and (AnsiChar(c) <= 'z'))
	leal	-97(%ecx),%eax
	# Register ecx released
	# Register rflags allocated
	cmpb	$25,%al
	# Register al released
# Peephole Optimization: J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret
	setbeb	%al
	movl	$0,%eax ; <- MOV instruction in the wrong place.
	ret
	# Register rflags released
.Lj5:
	# Register eax allocated
# [15] Result := 1
	movl	$1,%eax
# Peephole Optimization: Converted JMP to RET as part of SETcc optimisation (1st jump)
	ret
	...

After:

	...
# [13] ((AnsiChar(c) >= 'a') and (AnsiChar(c) <= 'z'))
	leal	-97(%ecx),%eax
	# Register ecx released
	# Register rflags allocated
	cmpb	$25,%al
	# Register al released
# Peephole Optimization: J(c)Mov1Jmp/RetMov0Jmp/Ret -> Set(~c)Jmp/Ret
	# Register rax allocated
	setbeb	%al
# Peephole Optimization: var9
	andl	$255,%eax ; <- MOVZX instruction gets inserted instead, and is then converted into an AND instruction.
	ret
	# Register rflags,rax released
.Lj5:
	# Register eax allocated
# [15] Result := 1
	movl	$1,%eax
# Peephole Optimization: Converted JMP to RET as part of SETcc optimisation (1st jump)
	ret
	...

Additional Notes

There was an oversight where a conditional jump was converted into a RET instruction but its is_jmp property was not reset, which caused the andl $255,%eax instruction to be converted into a testl $255,%eax instruction due to this issue and the flags register still being in use (its deallocation could potentially be moved to right after the SETcc instruction). This was corrected by changing the code to use ConvertJumpToRET like the other jumps in the routine, but a minor addition was also added to this utility method, which was to ensure the condition is set to C_None... this means the method now works correctly for both conditional and unconditional jumps.

Register allocation was also corrected - the target register was not always allocated across the SETcc and supporting instructions.

The -Os optimisation is not as good as it could be due to an inefficiency that is fixed in !296 (merged).