From b4426be0729cbc372288dfabac4f11f3506472d8 Mon Sep 17 00:00:00 2001
From: Chris Fallin <chris@cfallin.org>
Date: Fri, 8 Jan 2021 17:07:51 -0800
Subject: [PATCH] machinst lowering: update inst color when scanning across
 branch to allow more load-op merging.

A branch is considered side-effecting and so updates the instruction
color (which is our way of computing how far instructions can sink).
However, in the lowering loop, we did not update current instruction
color when scanning backward across branches, which are side-effecting.
As a result, the color was stale and fewer load-op merges were permitted
than are actually possible.

Note that this would not have resulted in any correctness issues, as the
stale color is too high (so no merges are permitted that should have
been disallowed).

Fixes #2562.
---
 cranelift/codegen/src/machinst/lower.rs            | 10 ++++++----
 cranelift/filetests/filetests/isa/x64/load-op.clif | 11 +++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs
index 46b5fc1685..28e4edd0c7 100644
--- a/cranelift/codegen/src/machinst/lower.rs
+++ b/cranelift/codegen/src/machinst/lower.rs
@@ -683,10 +683,6 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
                 has_side_effect,
                 value_needed,
             );
-            // Skip lowering branches; these are handled separately.
-            if self.f.dfg[inst].opcode().is_branch() {
-                continue;
-            }
 
             // Update scan state to color prior to this inst (as we are scanning
             // backward).
@@ -699,6 +695,12 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
                 self.cur_scan_entry_color = Some(entry_color);
             }
 
+            // Skip lowering branches; these are handled separately
+            // (see `lower_clif_branches()` below).
+            if self.f.dfg[inst].opcode().is_branch() {
+                continue;
+            }
+
             // Normal instruction: codegen if the instruction is side-effecting
             // or any of its outputs its used.
             if has_side_effect || value_needed {
diff --git a/cranelift/filetests/filetests/isa/x64/load-op.clif b/cranelift/filetests/filetests/isa/x64/load-op.clif
index 6570a798c3..8fefaf6d42 100644
--- a/cranelift/filetests/filetests/isa/x64/load-op.clif
+++ b/cranelift/filetests/filetests/isa/x64/load-op.clif
@@ -59,3 +59,14 @@ block0(v0: i64, v1: i64):
   ; nextln: movq    0(%rax,%rdi,1), %rsi
   ; nextln: movq    %rsi, %rax
 }
+
+function %merge_scalar_to_vector(i64) -> i32x4 {
+block0(v0: i64):
+  v1 = load.i32 v0
+  v2 = scalar_to_vector.i32x4 v1
+  ; check: movss   0(%rdi), %xmm0
+
+  jump block1
+block1:
+  return v2
+}