Examples of bad code

[apt1002]

This issue collects examples of bad code generated by Mijit, now or in the past. Each example has an explanation of how it arises. Keep this issue open as long as it is useful.

Discussion of how to fix the examples doesn't go here. Make a separate issue per example, add a reference to it here, and tick the box.

• [ ] Variable shifts on x86:

  0x7ffff7fb55ab:	mov    r12,rcx
  0x7ffff7fb55ae:	mov    rcx,r15
  0x7ffff7fb55b1:	mov    r15,r9
  0x7ffff7fb55b4:	sar    r15d,cl
  0x7ffff7fb55b7:	mov    rcx,r12
  

  x86 requires the shift amount to be in c. If the optimizer doesn't put it there, you get a sequence like this, whose intended effect is r15 = r9 >> r15. The Lowerer doesn't know whether rcx is dead, so it conservatively moves it to r12 and back again.
• [ ] (#15) Not using immediate constants:

  0x7ffff7fba1e8:	mov    r14d,0x4
  0x7ffff7fba1ee:	add    r14d,edi
  

  Mijit code cannot express that an operand is a constant.
• [ ] Awkward register allocation for asymmetric binary operations:

  0x7ffff7fb5a05:	mov    r12,r14
  0x7ffff7fb5a08:	mov    r14,r10
  0x7ffff7fb5a0b:	sub    r14d,r12d
  

  The Lowerer generates this sequence to achieve r14 = r12 - r14. It would generate better code if the destination and the second operand were in different registers.
• [ ] Jump to following instruction:

  0x7ffff7fb55c0:	rex jmp 0x7ffff7fb55c6
  

  Every specialization has at its fetch_label a jump to its first fetch child. A greatest specialization has no fetch children, so the jump goes to its retire_label, which is the following instruction. The jump is there so it can be patched to jump to a greater specialization.
• [ ] Jump to a jump:

  0x7ffff7fb55db:	rex jmp 0x7ffff7fb545a
  
  0x7ffff7fb545a:	rex jmp 0x7ffff7fb587d
  

  Every retire transition has such a jump, and it targets the fetch_label of its retire parent, where there is a jump to its first fetch child.
• [ ] Unnecessary comparisons:

  0x7ffff7fb587d:	mov    r12d,0xff
  0x7ffff7fb5883:	and    r12d,r14d
  0x7ffff7fb5886:	cmp    r12d,0x0
  0x7ffff7fb588d:	jne    0x7ffff7fb58b4
  

  This arises from a TestOp::Bits with a pattern of zero.
• [ ] Not using register + register addressing mode:

  0x7ffff7fba1e2:	mov    r15,r13
  0x7ffff7fba1e5:	add    r15,rdi
  ...
  0x7ffff7fba1f1:	mov    r15d,DWORD PTR [r15+0x0]
  

  Mijit code doesn't yet allow an address to be the sum of two registers.
• [ ] Not using register + constant addressing mode:
• [ ] Short-sighted register allocation:

  0x7ffff7fba1ee:	add    r14d,edi
  0x7ffff7fba1f1:	mov    r15d,DWORD PTR [r15+0x0]
  0x7ffff7fba1f8:	mov    rdi,r14
  0x7ffff7fba1fb:	mov    r9,r15
  0x7ffff7fba1fe:	rex jmp 0x7ffff7fba204
  

  Every block ends with Move instructions to match the register allocation at the jump target. Often these could be merged with previous instructions.
• [ ] Verbose switches:

  0x7ffff7fb58b4:	mov    r12d,0xff
  0x7ffff7fb58ba:	and    r12d,r14d
  0x7ffff7fb58bd:	cmp    r12d,0x1
  0x7ffff7fb58c4:	jne    0x7ffff7fb5917
  
  0x7ffff7fb5917:	mov    r12d,0xff
  0x7ffff7fb591d:	and    r12d,r14d
  0x7ffff7fb5920:	cmp    r12d,0x2
  0x7ffff7fb5927:	jne    0x7ffff7fb595a
  
  etc.
  

  This chain of tests arises because the TestOps are compiled incrementally. Each one patches itself into the previous one.
• [ ] Multiplication by constant powers of two:

  0x7ffff7fba785:	mov    r15d,0x4
  0x7ffff7fba78b:	imul   r15d,r9d
  

  Mijit doesn't know this can be replaced by a shift.