x64: Remove unnecessary register use when comparing against constants

[elliottt]

When lowering instructions like icmp eq (iconst 0), v2, we currently store the constant 0 in a register before performing the comparison. The result is that for inputs like the following:

  v1 = iconst.i32 1
  v2 = load.i32 v0
  v3 = icmp eq v1, v2
  brnz v3, block1

we generate this assembly:

  movl    0(%rdi), %r9d                                                              
  movl    $1, %r10d                                                                  
  cmpl    %r9d, %r10d                                                                
  jz      label1; j label2 

This PR introduces a case into emit_cmp that handles constants on the left-hand side of a comparison by swapping the order and reversing the operation. With this new rule, we emit the following assembly for the clif snippet above:

  movl    0(%rdi), %r8d                                                          
  cmpl    $1, %r8d 
  jz      label1; j label2 

[elliottt]

I'm not super happy with the need for a rule priority here, and would have expected the simm32_from_value use to render it unnecessary. Any idea why it doesn't fire without higher priority?

[elliottt]

This can go away once #4661 is merged.

[github-actions]

Subscribe to Label Action

cc @cfallin, @fitzgen

Details

This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[cfallin]

This looks good to me as-is, but I'm curious to see if the rule-priority tweak we talked about today removes the need for the priority here before we merge it...

[elliottt]

This looks good to me as-is, but I'm curious to see if the rule-priority tweak we talked about today removes the need for the priority here before we merge it...

The bugfix you linked removes the need for the explicit priority here, so I'll make a separate PR for that before merging this without the priority 👍