T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ISLE: remove all uses of argument polarity, and remove it from the language. (#4091)

Browse Source 
This PR removes "argument polarity": the feature of ISLE extractors that lets them take
inputs aside from the value to be matched.

Cases that need this expressivity have been subsumed by #4072 with if-let clauses;
we can now finally remove this misfeature of the language, which has caused significant
confusion and has always felt like a bit of a hack.

This PR (i) removes the feature from the ISLE compiler; (ii) removes it from the reference
documentation; and (iii) refactors away all uses of the feature in our three existing
backends written in ISLE.
This commit is contained in:
Chris Fallin
2022-05-02 09:52:12 -07:00
committed by GitHub
parent c7e2c21bb2
commit 03793b71a7
21 changed files with 2123 additions and 2464 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
cranelift/isle/docs/language-reference.md
View File

@@ -1202,59 +1202,6 @@ If an extractor returns `None`, then the generated matching code
proceeds just as if an enum variant match had failed: it moves on to
try the next rule in turn.
#### Advanced Extractors: Arg-Polarity
There is one shortcoming in the extractor mechanism as defined so far:
there is no mechanism to provide additional context or "parameterize"
an external extractor; it only receives the term to deconstruct, with
no other parameters. For example, one might wish to write a
`(GetNthArg n arg_pattern)` extractor that matches on an instruction,
fetches the `n`th "arg" from it, and then returns that as the
extracted match result, allowing `arg_pattern` to match against it.
Inspired by Prolog, where argument data can flow in both directions
during "unification", we implement a limited form of bidirectionality
in ISLE for extractor arguments. Specifically, we can declare the
"polarity" of the arguments to an extractor so that some of the
arguments flow "in" rather than "out". This lets us provide data to
the extractor via expressions embedded in the pattern.
An example might make this more clear: we can define a term
```lisp
(decl GetNthArg (u32 Arg) Inst)
```
that we wish to use as in the example given above, and then we can define
an external extractor
```lisp
(extern extractor GetNthArg get_nth_arg (in out))
```
which indicates that `get_nth_arg()` will take parameters of `&Inst`
(the value being extracted) *and* `u32` (the in-arg), and return
`Option<(Arg,)>`, i.e., *just* the out-args.
In order to use this extractor, to avoid parse ambiguity (i.e., to
avoid the need for the parser to know argument polarity while it is
still parsing), we require special syntax for the in-argument so that
it is parsed as an expression: it must be prepended by `<`. So one
might use `GetNthArg` as follows:
```lisp
(rule (Lower (and
(Inst ...)
(GetNthArg <2 second_arg)))
...)
```
This functionality is meant to improve the expressive power of the
DSL, but is not intended to be commonly used outside of "binding" or
"library" ISLE code. In other words, because it is somewhat
non-intuitive, it is best to wrap it within friendlier internal
extractors.
### Mapping Type Declarations to Rust
When we declare a type like
@@ -1528,9 +1475,5 @@ newline). The grammar accepted by the parser is as follows:
<extern> ::= "constructor" <ident> <ident>
| "extractor" [ "infallible" ] <ident> <ident>
[ "(" <polarity>* ")" ]
| "const" <const-ident> <ident> <ty>
<polarity> ::= "in" | "out"
```

16
cranelift/isle/isle/isle_examples/pass/test3.isle
View File

@@ -12,8 +12,8 @@
(decl Op (Opcode) Inst)
(extern extractor infallible Op get_opcode)
(decl InstInput (InstInput u32) Inst)
(extern extractor infallible InstInput get_inst_input (out in))
(decl InstInputs2 (InstInput InstInput) Inst)
(extern extractor infallible InstInputs2 get_inst_inputs_2)
(decl Producer (Inst) InstInput)
(extern extractor Producer get_input_producer)
@@ -44,15 +44,13 @@
(extractor
(Iadd a b)
(and
(Op (Opcode.Iadd))
(InstInput a <0)
(InstInput b <1)))
(Op (Opcode.Iadd))
(InstInputs2 a b)))
(extractor
(Isub a b)
(and
(Op (Opcode.Isub))
(InstInput a <0)
(InstInput b <1)))
(Op (Opcode.Isub))
(InstInputs2 a b)))
;; Now the nice syntax-sugar that "end-user" backend authors can write:
(rule
@@ -63,4 +61,4 @@
(MachInst.Add3 (UseInput ra) (UseInput rb) (UseInput rc)))
(rule
(Lower (Isub ra rb))
(MachInst.Sub (UseInput ra) (UseInput rb)))
(MachInst.Sub (UseInput ra) (UseInput rb)))

61
cranelift/isle/isle/src/ast.rs
View File

@@ -126,7 +126,7 @@ pub enum Pattern {
/// An application of a type variant or term.
Term {
sym: Ident,
args: Vec<TermArgPattern>,
args: Vec<Pattern>,
pos: Pos,
},
/// An operator that matches anything.
@@ -152,9 +152,7 @@ impl Pattern {
Pattern::Term { sym, args, pos } => {
f(*pos, sym);
for arg in args {
if let TermArgPattern::Pattern(p) = arg {
p.terms(f);
}
arg.terms(f);
}
}
Pattern::And { subpats, .. } => {
@@ -291,41 +289,6 @@ impl Pattern {
}
}
/// A pattern in a term argument. Adds "evaluated expression" to kinds
/// of patterns in addition to all options in `Pattern`.
#[derive(Clone, PartialEq, Eq, Debug)]
pub enum TermArgPattern {
/// A regular pattern that must match the existing value in the term's argument.
Pattern(Pattern),
/// An expression that is evaluated during the match phase and can
/// be given into an extractor. This is essentially a limited form
/// of unification or bidirectional argument flow (a la Prolog):
/// we can pass an arg *into* an extractor rather than getting the
/// arg *out of* it.
Expr(Expr),
}
impl TermArgPattern {
fn make_macro_template(&self, args: &[Ident]) -> TermArgPattern {
log::trace!("repplace_macro_args: {:?} with {:?}", self, args);
match self {
&TermArgPattern::Pattern(ref pat) => {
TermArgPattern::Pattern(pat.make_macro_template(args))
}
&TermArgPattern::Expr(_) => self.clone(),
}
}
fn subst_macro_args(&self, args: &[Pattern]) -> Option<TermArgPattern> {
match self {
&TermArgPattern::Pattern(ref pat) => {
Some(TermArgPattern::Pattern(pat.subst_macro_args(args)?))
}
&TermArgPattern::Expr(_) => Some(self.clone()),
}
}
}
/// An expression: the right-hand side of a rule.
///
/// Note that this *almost* looks like a core Lisp or lambda calculus,
@@ -405,15 +368,6 @@ pub enum Extern {
func: Ident,
/// The position of this decl.
pos: Pos,
/// Poliarity of args: whether values are inputs or outputs to
/// the external extractor function. This is a sort of
/// statically-defined approximation to Prolog-style
/// unification; we allow for the same flexible directionality
/// but fix it at DSL-definition time. By default, every arg
/// is an *output* from the extractor (and the 'retval", or
/// more precisely the term value that we are extracting, is
/// an "input").
arg_polarity: Option<Vec<ArgPolarity>>,
/// Infallibility: if an external extractor returns `(T1, T2,
/// ...)` rather than `Option<(T1, T2, ...)>`, and hence can
/// never fail, it is declared as such and allows for slightly
@@ -433,17 +387,6 @@ pub enum Extern {
Const { name: Ident, ty: Ident, pos: Pos },
}
/// Whether an argument is an input or an output.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ArgPolarity {
/// An arg that must be given an Expr in the pattern and passes data *to*
/// the extractor op.
Input,
/// An arg that must be given a regular pattern (not Expr) and receives data
/// *from* the extractor op.
Output,
}
/// An implicit converter: the given term, which must have type
/// (inner_ty) -> outer_ty, is used either in extractor or constructor
/// position as appropriate when a type mismatch with the given pair

46
cranelift/isle/isle/src/ir.rs
View File

@@ -387,12 +387,6 @@ impl PatternSequence {
let arg_tys = &termdata.arg_tys[..];
for (i, subpat) in args.iter().enumerate() {
let value = self.add_arg(i, arg_tys[i]);
let subpat = match subpat {
&TermArgPattern::Expr(..) => {
panic!("Should have been caught in typechecking")
}
&TermArgPattern::Pattern(ref pat) => pat,
};
self.gen_pattern(
ValueOrArgs::Value(value),
typeenv,
@@ -411,10 +405,6 @@ impl PatternSequence {
let arg_values =
self.add_match_variant(input, ty, arg_tys, *variant);
for (subpat, value) in args.iter().zip(arg_values.into_iter()) {
let subpat = match subpat {
&TermArgPattern::Pattern(ref pat) => pat,
_ => unreachable!("Should have been caught by sema"),
};
self.gen_pattern(
ValueOrArgs::Value(value),
typeenv,
@@ -438,11 +428,7 @@ impl PatternSequence {
}
TermKind::Decl {
extractor_kind:
Some(ExtractorKind::ExternalExtractor {
ref arg_polarity,
infallible,
..
}),
Some(ExtractorKind::ExternalExtractor { infallible, .. }),
..
} => {
// Evaluate all `input` args.
@@ -452,33 +438,9 @@ impl PatternSequence {
let mut output_pats = vec![];
inputs.push(input);
input_tys.push(termdata.ret_ty);
for (arg, pol) in args.iter().zip(arg_polarity.iter()) {
match pol {
&ArgPolarity::Input => {
let expr = match arg {
&TermArgPattern::Expr(ref expr) => expr,
_ => panic!(
"Should have been caught by typechecking"
),
};
let mut seq = ExprSequence::default();
let value = seq.gen_expr(typeenv, termenv, expr, vars);
seq.add_return(expr.ty(), value);
let value = self.add_expr_seq(seq, value, expr.ty());
inputs.push(value);
input_tys.push(expr.ty());
}
&ArgPolarity::Output => {
let pat = match arg {
&TermArgPattern::Pattern(ref pat) => pat,
_ => panic!(
"Should have been caught by typechecking"
),
};
output_tys.push(pat.ty());
output_pats.push(pat);
}
}
for arg in args {
output_tys.push(arg.ty());
output_pats.push(arg);
}
// Invoke the extractor.

10
cranelift/isle/isle/src/lexer.rs
View File

@@ -66,8 +66,6 @@ pub enum Token {
Int(i64),
/// `@`
At,
/// `<`
Lt,
}
impl<'a> Lexer<'a> {
@@ -176,7 +174,7 @@ impl<'a> Lexer<'a> {
fn next_token(&mut self) -> Result<Option<(Pos, Token)>> {
fn is_sym_first_char(c: u8) -> bool {
match c {
b'-' | b'0'..=b'9' | b'(' | b')' | b';' => false,
b'-' | b'0'..=b'9' | b'(' | b')' | b';' | b'<' | b'>' => false,
c if c.is_ascii_whitespace() => false,
_ => true,
}
@@ -222,10 +220,6 @@ impl<'a> Lexer<'a> {
self.advance_pos();
Ok(Some((char_pos, Token::At)))
}
b'<' => {
self.advance_pos();
Ok(Some((char_pos, Token::Lt)))
}
c if is_sym_first_char(c) => {
let start = self.pos.offset;
let start_pos = self.pos();
@@ -295,7 +289,7 @@ impl<'a> Lexer<'a> {
};
Ok(Some((start_pos, tok)))
}
c => panic!("Unexpected character '{}' at offset {}", c, self.pos.offset),
c => Err(self.error(self.pos, format!("Unexpected character '{}'", c))),
}
}

37
cranelift/isle/isle/src/parser.rs
View File

@@ -77,9 +77,6 @@ impl<'a> Parser<'a> {
fn is_at(&self) -> bool {
self.is(|tok| *tok == Token::At)
}
fn is_lt(&self) -> bool {
self.is(|tok| *tok == Token::Lt)
}
fn is_sym(&self) -> bool {
self.is(|tok| tok.is_sym())
}
@@ -109,9 +106,6 @@ impl<'a> Parser<'a> {
fn at(&mut self) -> Result<()> {
self.take(|tok| *tok == Token::At).map(|_| ())
}
fn lt(&mut self) -> Result<()> {
self.take(|tok| *tok == Token::Lt).map(|_| ())
}
fn symbol(&mut self) -> Result<String> {
match self.take(|tok| tok.is_sym())? {
@@ -338,30 +332,10 @@ impl<'a> Parser<'a> {
let term = self.parse_ident()?;
let func = self.parse_ident()?;
let arg_polarity = if self.is_lparen() {
let mut pol = vec![];
self.lparen()?;
while !self.is_rparen() {
if self.is_sym_str("in") {
self.symbol()?;
pol.push(ArgPolarity::Input);
} else if self.is_sym_str("out") {
self.symbol()?;
pol.push(ArgPolarity::Output);
} else {
return Err(self.error(pos, "Invalid argument polarity".to_string()));
}
}
self.rparen()?;
Some(pol)
} else {
None
};
Ok(Extern::Extractor {
term,
func,
pos,
arg_polarity,
infallible,
})
} else if self.is_sym_str("const") {
@@ -461,7 +435,7 @@ impl<'a> Parser<'a> {
let sym = self.parse_ident()?;
let mut args = vec![];
while !self.is_rparen() {
args.push(self.parse_pattern_term_arg()?);
args.push(self.parse_pattern()?);
}
self.rparen()?;
Ok(Pattern::Term { sym, args, pos })
@@ -471,15 +445,6 @@ impl<'a> Parser<'a> {
}
}
fn parse_pattern_term_arg(&mut self) -> Result<TermArgPattern> {
if self.is_lt() {
self.lt()?;
Ok(TermArgPattern::Expr(self.parse_expr()?))
} else {
Ok(TermArgPattern::Pattern(self.parse_pattern()?))
}
}
fn parse_iflet_or_expr(&mut self) -> Result<IfLetOrExpr> {
let pos = self.pos();
if self.is_lparen() {

182
cranelift/isle/isle/src/sema.rs
View File

@@ -268,8 +268,6 @@ pub enum ExtractorKind {
ExternalExtractor {
/// The external name of the extractor function.
name: Sym,
/// Which arguments of the extractor are inputs and which are outputs?
arg_polarity: Vec<ArgPolarity>,
/// Is the external extractor infallible?
infallible: bool,
/// The position where this external extractor was declared.
@@ -277,8 +275,6 @@ pub enum ExtractorKind {
},
}
pub use crate::ast::ArgPolarity;
/// An external function signature.
#[derive(Clone, Debug)]
pub struct ExternalSig {
@@ -357,34 +353,16 @@ impl Term {
TermKind::Decl {
extractor_kind:
Some(ExtractorKind::ExternalExtractor {
name,
ref arg_polarity,
infallible,
..
name, infallible, ..
}),
..
} => {
let mut arg_tys = vec![];
let mut ret_tys = vec![];
arg_tys.push(self.ret_ty);
for (&arg, polarity) in self.arg_tys.iter().zip(arg_polarity.iter()) {
match polarity {
&ArgPolarity::Input => {
arg_tys.push(arg);
}
&ArgPolarity::Output => {
ret_tys.push(arg);
}
}
}
Some(ExternalSig {
func_name: tyenv.syms[name.index()].clone(),
full_name: format!("C::{}", tyenv.syms[name.index()]),
param_tys: arg_tys,
ret_tys,
infallible: *infallible,
})
}
} => Some(ExternalSig {
func_name: tyenv.syms[name.index()].clone(),
full_name: format!("C::{}", tyenv.syms[name.index()]),
param_tys: vec![self.ret_ty],
ret_tys: self.arg_tys.clone(),
infallible: *infallible,
}),
_ => None,
}
}
@@ -477,7 +455,7 @@ pub enum Pattern {
/// Match the current value against the given extractor term with the given
/// arguments.
Term(TypeId, TermId, Vec<TermArgPattern>),
Term(TypeId, TermId, Vec<Pattern>),
/// Match anything of the given type successfully.
Wildcard(TypeId),
@@ -486,15 +464,6 @@ pub enum Pattern {
And(TypeId, Vec<Pattern>),
}
/// Arguments to a term inside a pattern (i.e. an extractor).
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum TermArgPattern {
/// A pattern to match sub-values (i.e. the extractor's results) against.
Pattern(Pattern),
/// An expression to generate a value that is passed into the extractor.
Expr(Expr),
}
/// A right-hand side expression of some rule.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Expr {
@@ -1248,7 +1217,6 @@ impl TermEnv {
ref term,
ref func,
pos,
ref arg_polarity,
infallible,
}) => {
let term_sym = tyenv.intern_mut(term);
@@ -1266,22 +1234,11 @@ impl TermEnv {
let termdata = &mut self.terms[term_id.index()];
let arg_polarity = if let Some(pol) = arg_polarity.as_ref() {
if pol.len() != termdata.arg_tys.len() {
tyenv.report_error(pos, "Incorrect number of argument-polarity directions in extractor definition".to_string());
continue;
}
pol.clone()
} else {
vec![ArgPolarity::Output; termdata.arg_tys.len()]
};
match &mut termdata.kind {
TermKind::Decl { extractor_kind, .. } => match extractor_kind {
None => {
*extractor_kind = Some(ExtractorKind::ExternalExtractor {
name: func_sym,
arg_polarity,
infallible,
pos,
});
@@ -1476,7 +1433,7 @@ impl TermEnv {
let expanded_pattern = ast::Pattern::Term {
sym: converter_term_ident,
pos: pattern.pos(),
args: vec![ast::TermArgPattern::Pattern(pattern.clone())],
args: vec![pattern.clone()],
};
return Some(expanded_pattern);
@@ -1712,63 +1669,12 @@ impl TermEnv {
TermKind::Decl {
constructor_kind: Some(ConstructorKind::InternalConstructor),
..
} if is_root && *tid == rule_term => {
// This is just the `(foo ...)` pseudo-pattern inside a
// `(rule (foo ...) ...)` form. Just keep checking the
// sub-patterns.
for arg in args {
if let ast::TermArgPattern::Expr(e) = arg {
tyenv.report_error(
e.pos(),
"cannot use output-polarity expression with top-level rules"
.to_string(),
);
}
}
}
TermKind::EnumVariant { .. } => {
for arg in args {
if let &ast::TermArgPattern::Expr(_) = arg {
tyenv.report_error(
pos,
format!(
"Term in pattern '{}' cannot have an injected expr, because \
it is an enum variant",
sym.0
)
);
}
}
}
} if is_root && *tid == rule_term => {}
TermKind::EnumVariant { .. } => {}
TermKind::Decl {
extractor_kind:
Some(ExtractorKind::ExternalExtractor {
ref arg_polarity, ..
}),
extractor_kind: Some(ExtractorKind::ExternalExtractor { .. }),
..
} => {
for (arg, pol) in args.iter().zip(arg_polarity.iter()) {
match (arg, pol) {
(&ast::TermArgPattern::Expr(..), &ArgPolarity::Input) => {}
(&ast::TermArgPattern::Expr(ref e), &ArgPolarity::Output) => {
tyenv.report_error(
e.pos(),
"Expression used for output-polarity extractor arg"
.to_string(),
);
}
(_, &ArgPolarity::Output) => {}
(&ast::TermArgPattern::Pattern(ref p), &ArgPolarity::Input) => {
tyenv.report_error(
p.pos(),
"Non-expression used in pattern but expression required for \
input-polarity extractor arg"
.to_string()
);
}
}
}
}
} => {}
TermKind::Decl {
extractor_kind: Some(ExtractorKind::InternalExtractor { ref template }),
..
@@ -1779,19 +1685,7 @@ impl TermEnv {
// substitutions.
let mut macro_args: Vec<ast::Pattern> = vec![];
for template_arg in args {
let sub_ast = match template_arg {
&ast::TermArgPattern::Pattern(ref pat) => pat.clone(),
&ast::TermArgPattern::Expr(_) => {
tyenv.report_error(
pos,
"Cannot expand an extractor macro with an expression in a \
macro argument"
.to_string(),
);
return None;
}
};
macro_args.push(sub_ast.clone());
macro_args.push(template_arg.clone());
}
log::trace!("internal extractor macro args = {:?}", args);
let pat = template.subst_macro_args(&macro_args[..])?;
@@ -1824,13 +1718,13 @@ impl TermEnv {
for (i, arg) in args.iter().enumerate() {
let term = unwrap_or_continue!(self.terms.get(tid.index()));
let arg_ty = unwrap_or_continue!(term.arg_tys.get(i).copied());
let (subpat, _) = unwrap_or_continue!(self.translate_pattern_term_arg(
let (subpat, _) = unwrap_or_continue!(self.translate_pattern(
tyenv,
rule_term,
pos,
arg,
Some(arg_ty),
bindings,
/* is_root = */ false,
));
subpats.push(subpat);
}
@@ -1841,48 +1735,6 @@ impl TermEnv {
}
}
fn translate_pattern_term_arg(
&self,
tyenv: &mut TypeEnv,
rule_term: TermId,
pos: Pos,
pat: &ast::TermArgPattern,
expected_ty: Option<TypeId>,
bindings: &mut Bindings,
) -> Option<(TermArgPattern, TypeId)> {
match pat {
&ast::TermArgPattern::Pattern(ref pat) => {
let (subpat, ty) = self.translate_pattern(
tyenv,
rule_term,
pat,
expected_ty,
bindings,
/* is_root = */ false,
)?;
Some((TermArgPattern::Pattern(subpat), ty))
}
&ast::TermArgPattern::Expr(ref expr) => {
if expected_ty.is_none() {
tyenv.report_error(
pos,
"Expression in pattern must have expected type".to_string(),
);
return None;
}
let ty = expected_ty.unwrap();
let expr = self.translate_expr(
tyenv,
expr,
expected_ty,
bindings,
/* pure = */ true,
)?;
Some((TermArgPattern::Expr(expr), ty))
}
}
}
fn maybe_implicit_convert_expr(
&self,
tyenv: &mut TypeEnv,