From 6b6fc9ec3e55a408663312cc68857ea1835a20e7 Mon Sep 17 00:00:00 2001
From: Trevor Elliott <telliott@fastly.com>
Date: Tue, 9 Aug 2022 12:19:32 -0700
Subject: [PATCH] ISLE: Fix a bug with extractor ordering (#4661)

https://github.com/bytecodealliance/wasmtime/pull/4661

Co-authored-by: Chris Fallin <chris@cfallin.org>
---
 .../run/extractor_ordering_bug.isle           | 19 +++++++++++++++
 .../run/extractor_ordering_bug_main.rs        | 23 +++++++++++++++++++
 cranelift/isle/isle/src/ir.rs                 |  5 ++--
 3 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 cranelift/isle/isle/isle_examples/run/extractor_ordering_bug.isle
 create mode 100644 cranelift/isle/isle/isle_examples/run/extractor_ordering_bug_main.rs

diff --git a/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug.isle b/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug.isle
new file mode 100644
index 0000000000..d0a383920e
--- /dev/null
+++ b/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug.isle
@@ -0,0 +1,19 @@
+
+(type u32 (primitive u32))
+
+(decl identity (u32) u32)
+(extern extractor infallible identity identity)
+
+(decl is_zero (u32) u32)
+(extern extractor is_zero is_zero)
+
+(decl test (u32) u32)
+
+;; This exposes a bug where infallible extractors were running before fallible
+;; ones, as the derived ordering for the `Extract` type was ordering them ahead
+;; of the fallible ones. The result is that the fallible `is_zero` extractor
+;; never runs, as the `identity` extractor will always succeed before it's
+;; called.
+(rule (test (identity x)) x)
+
+(rule (test (is_zero x)) 2)
diff --git a/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug_main.rs b/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug_main.rs
new file mode 100644
index 0000000000..70bb544bee
--- /dev/null
+++ b/cranelift/isle/isle/isle_examples/run/extractor_ordering_bug_main.rs
@@ -0,0 +1,23 @@
+mod extractor_ordering_bug;
+
+struct Context;
+impl extractor_ordering_bug::Context for Context {
+    fn is_zero(&mut self, val: u32) -> Option<u32> {
+        if val == 0 {
+            Some(val)
+        } else {
+            None
+        }
+    }
+
+    fn identity(&mut self, val: u32) -> u32 {
+        val
+    }
+}
+
+fn main() {
+    let mut ctx = Context;
+
+    assert_eq!(extractor_ordering_bug::constructor_test(&mut ctx, 0), Some(2));
+    assert_eq!(extractor_ordering_bug::constructor_test(&mut ctx, 1), Some(1));
+}
diff --git a/cranelift/isle/isle/src/ir.rs b/cranelift/isle/isle/src/ir.rs
index cb58cc1749..37665c6edf 100644
--- a/cranelift/isle/isle/src/ir.rs
+++ b/cranelift/isle/isle/src/ir.rs
@@ -94,6 +94,9 @@ pub enum PatternInst {
     /// value to extract, the other are the `Input`-polarity extractor args) and
     /// producing an output value for each `Output`-polarity extractor arg.
     Extract {
+        /// Whether this extraction is infallible or not. `false`
+        /// comes before `true`, so fallible nodes come first.
+        infallible: bool,
         /// The value to extract, followed by polarity extractor args.
         inputs: Vec<Value>,
         /// The types of the inputs.
@@ -102,8 +105,6 @@ pub enum PatternInst {
         output_tys: Vec<TypeId>,
         /// This extractor's term.
         term: TermId,
-        /// Whether this extraction is infallible or not.
-        infallible: bool,
     },
 
     // NB: This has to go last, since it is infallible, so that when we sort