Merge pull request #2329 from Kobzol/codegen-backend-validation

Validate codegen backends in bot commands and mention them in help

Author: Kobzol

database/src/lib.rs

@@ -406,6 +406,10 @@ impl CodegenBackend {
             CodegenBackend::Cranelift => "cranelift",
         }
     }
+
+    pub fn all_values() -> &'static [Self] {
+        &[Self::Llvm, Self::Cranelift]
+    }
 }
 
 impl FromStr for CodegenBackend {
@@ -1009,12 +1013,7 @@ impl BenchmarkRequest {
             return Ok(vec![CodegenBackend::Llvm]);
         }
 
-        self.backends
-            .split(',')
-            .map(|s| {
-                CodegenBackend::from_str(s).map_err(|_| anyhow::anyhow!("Invalid backend: {s}"))
-            })
-            .collect()
+        parse_backends(&self.backends).map_err(|e| anyhow::anyhow!("{e}"))
     }
 
     /// Get the profiles for the request
@@ -1040,6 +1039,13 @@ impl BenchmarkRequest {
     }
 }
 
+pub fn parse_backends(backends: &str) -> Result<Vec<CodegenBackend>, String> {
+    backends
+        .split(',')
+        .map(|s| CodegenBackend::from_str(s).map_err(|_| format!("Invalid backend: {s}")))
+        .collect()
+}
+
 /// Cached information about benchmark requests in the DB
 pub struct BenchmarkRequestIndex {
     /// Tags (SHA or release name) of all known benchmark requests

site/frontend/templates/pages/help.html

@@ -1,48 +1,61 @@
 {% extends "layout.html" %}
 {% block head %}
 <style>
-    .help-content {
-        font-family: Helvetica, Arial, sans-serif;
-        line-height: 140%;
-        font-size: 16px;
-        max-width: 50em;
-    }
+  .help-content {
+    font-family: Helvetica, Arial, sans-serif;
+    line-height: 140%;
+    font-size: 16px;
+    max-width: 50em;
+  }
 
-    .help-content code {
-        background: #eee;
-        border-radius: 5px;
-        padding: 2px;
-    }
+  .help-content code {
+    background: #eee;
+    border-radius: 5px;
+    padding: 2px;
+  }
 </style>
 {% endblock %}
 
 {% block content %}
 <div class="help-content">
   <h3><b><code>@rust-timer</code> commands</b></h3>
   <p><code>@rust-timer</code> supports several commands, the most common (and simple) being
-    <code>@rust-timer queue</code>. This command is usually invoked as <code>@bors try @rust-timer queue</code>,
-    which starts a bors "try" run (not a merge). <code>@rust-timer</code> will wait for the try run to finish,
+    <code>@rust-timer queue</code>. This command is usually invoked as <code>@bors try @rust-timer
+      queue</code>,
+    which starts a bors "try" run (not a merge). <code>@rust-timer</code> will wait for the try run
+    to finish,
     and if it succeeds will then queue a perf run.
   </p>
   <p><code>@rust-timer queue</code> has a few extra options that can be useful:</p>
   <ul>
-    <li><code>include=&lt;INCLUDE&gt;</code> is a comma-separated list of benchmark prefixes. A benchmark is included in
+    <li><code>include=&lt;INCLUDE&gt;</code> is a comma-separated list of benchmark prefixes. A
+      benchmark is included in
       the run only if its name matches one of the given prefixes.
     </li>
-    <li><code>exclude=&lt;EXCLUDE&gt;</code> is a comma-separated list of benchmark prefixes, and the inverse of <code>include=</code>.
-      A benchmark is excluded from the run if its name matches one of the given prefixes.</li>
+    <li><code>exclude=&lt;EXCLUDE&gt;</code> is a comma-separated list of benchmark prefixes, and
+      the inverse of <code>include=</code>.
+      A benchmark is excluded from the run if its name matches one of the given prefixes.
+    </li>
     <li><code>runs=&lt;RUNS&gt;</code> configures how many times the benchmark is run. <code>&lt;RUNS&gt;</code>
-      is an integer. All benchmarks run at least once by default, but some run more than one time. You can use
-      the <code>runs</code> option to override the default run count and make every benchmark run for
+      is an integer. All benchmarks run at least once by default, but some run more than one time.
+      You can use
+      the <code>runs</code> option to override the default run count and make every benchmark run
+      for
       <code>&lt;RUNS&gt;</code> times.
     </li>
+    <li><code>backends=&lt;BACKENDS&gt;</code> configures which codegen backends should be
+      benchmarked.
+      By default, only the LLVM backend is benchmarked. If you select a non-default codegen backend,
+      rustc-perf will also gather data for this backend for the parent/baseline commit, so that we
+      have something to compare to.
+    </li>
   </ul>
-  <p><code>@rust-timer build $commit</code> will queue a perf run for the given commit <code>$commit</code>.
+  <p><code>@rust-timer build $commit</code> will queue a perf run for the given commit
+    <code>$commit</code>.
     It is usually invoked with the commit from a successful "try" run. (The
     <code>queue</code> command can be seen as a shortcut that automatically selects the
     "try" run's commit for the <code>build</code> command)
-    This command also supports the same <code>include</code>, <code>exclude</code>, and <code>runs</code> options
-    as <code>@rust-timer queue</code>.
+    This command also supports the same options as <code>@rust-timer queue</code>.
   </p>
 </div>
 {% endblock %}

site/src/request_handlers/github.rs

@@ -6,7 +6,7 @@ use crate::github::{
 use crate::job_queue::should_use_job_queue;
 use crate::load::SiteCtxt;
 
-use database::BenchmarkRequest;
+use database::{parse_backends, BenchmarkRequest, CodegenBackend};
 use hashbrown::HashMap;
 use std::sync::Arc;
 
@@ -265,6 +265,19 @@ fn parse_benchmark_parameters<'a>(
         };
         params.runs = Some(runs as i32);
     }
+    if let Some(backends) = &params.backends {
+        // Make sure that the backends are correct
+        parse_backends(backends).map_err(|e| {
+            format!(
+                "Cannot parse backends: {e}. Valid values are: {}",
+                CodegenBackend::all_values()
+                    .iter()
+                    .map(|b| b.as_str())
+                    .collect::<Vec<_>>()
+                    .join(", ")
+            )
+        })?;
+    }
 
     if !args.is_empty() {
         Err(format!(