Merge branch 'master' into software-tracer

tests/c/simple.newcg.c

@@ -1,4 +1,6 @@
-// ignore-if: test YK_JIT_COMPILER != "yk"
+// # Currently this test breaks CI entirely, so we temporarily ignore it
+// # completely.
+// ignore-if: test $YK_JIT_COMPILER != "yk"
 // Run-time:
 //   env-var: YKD_PRINT_IR=aot
 //   env-var: YKD_SERIALISE_COMPILATION=1

ykrt/src/compile/jitc_llvm.rs

@@ -5,7 +5,7 @@ use crate::{
     compile::{CompilationError, CompiledTrace, Compiler},
     location::HotLocation,
     mt::{SideTraceInfo, MT},
-    trace::TracedAOTBlock,
+    trace::{AOTTraceIterator, TracedAOTBlock},
 };
 use object::{Object, ObjectSection};
 use parking_lot::Mutex;
@@ -32,10 +32,11 @@ impl Compiler for JITCLLVM {
     fn compile(
         &self,
         mt: Arc<MT>,
-        irtrace: Vec<TracedAOTBlock>,
+        aottrace_iter: Box<dyn AOTTraceIterator>,
         sti: Option<SideTraceInfo>,
         hl: Arc<Mutex<HotLocation>>,
     ) -> Result<CompiledTrace, CompilationError> {
+        let irtrace = aottrace_iter.collect::<Vec<_>>();
         let (func_names, bbs, trace_len) = self.encode_trace(&irtrace);
 
         let llvmbc = llvmbc_section();

ykrt/src/compile/jitc_yk/aot_ir.rs

@@ -111,7 +111,8 @@ pub(crate) trait IRDisplay {
 
     /// Print myself to stderr in human-readable form.
     ///
-    /// This is provided as a debugging convenience.
+    /// This isn't used during normal operation of the system: it is provided as a debugging aid.
+    #[allow(dead_code)]
     fn dump(&self, m: &Module) {
         eprintln!("{}", self.to_str(m));
     }

ykrt/src/compile/jitc_yk/mod.rs

@@ -5,7 +5,7 @@ use crate::{
     compile::{CompiledTrace, Compiler},
     location::HotLocation,
     mt::{SideTraceInfo, MT},
-    trace::TracedAOTBlock,
+    trace::AOTTraceIterator,
 };
 use parking_lot::Mutex;
 use std::{
@@ -60,7 +60,7 @@ impl Compiler for JITCYk {
     fn compile(
         &self,
         _mt: Arc<MT>,
-        mtrace: Vec<TracedAOTBlock>,
+        aottrace_iter: Box<dyn AOTTraceIterator>,
         sti: Option<SideTraceInfo>,
         _hl: Arc<Mutex<HotLocation>>,
     ) -> Result<CompiledTrace, CompilationError> {
@@ -77,6 +77,7 @@ impl Compiler for JITCYk {
             eprintln!("--- End aot ---");
         }
 
+        let mtrace = aottrace_iter.collect::<Vec<_>>();
         let jit_mod = trace_builder::build(&aot_mod, &mtrace)?;
 
         if PHASES_TO_PRINT.contains(&IRPhase::PreOpt) {

ykrt/src/compile/jitc_yk/trace_builder.rs

@@ -171,7 +171,22 @@ impl<'a> TraceBuilder<'a> {
             Some(b) => Ok(b),
             None => Err(CompilationError::Unrecoverable("empty trace".into())),
         }?;
-        let firstblk = self.lookup_aot_block(first_blk);
+
+        // Find the block containing the control point call. This is the (sole) predecessor of the
+        // first (guaranteed mappable) block in the trace.
+        let prev = match first_blk {
+            TracedAOTBlock::Mapped { func_name, bb } => {
+                debug_assert!(*bb > 0);
+                // It's `- 1` due to the way the ykllvm block splitting pass works.
+                TracedAOTBlock::Mapped {
+                    func_name: func_name.clone(),
+                    bb: bb - 1,
+                }
+            }
+            TracedAOTBlock::Unmappable => panic!(),
+        };
+
+        let firstblk = self.lookup_aot_block(&prev);
         // FIXME: This unwrap assumes the first block is mappable, but Laurie just merged a change
         // that strips the initial block (the block we return to from the control point), so I
         // don't think this assumption necessarily holds any more. Investigate.

ykrt/src/compile/mod.rs

@@ -1,7 +1,7 @@
 use crate::{
     location::HotLocation,
     mt::{SideTraceInfo, MT},
-    trace::TracedAOTBlock,
+    trace::AOTTraceIterator,
 };
 use libc::c_void;
 use parking_lot::Mutex;
@@ -41,7 +41,7 @@ pub(crate) trait Compiler: Send + Sync {
     fn compile(
         &self,
         mt: Arc<MT>,
-        irtrace: Vec<TracedAOTBlock>,
+        aottrace_iter: Box<dyn AOTTraceIterator>,
         sti: Option<SideTraceInfo>,
         hl: Arc<Mutex<HotLocation>>,
     ) -> Result<CompiledTrace, CompilationError>;

ykrt/src/mt.rs

@@ -27,7 +27,7 @@ use crate::print_jit_state;
 use crate::{
     compile::{default_compiler, CompilationError, CompiledTrace, Compiler, GuardId},
     location::{HotLocation, HotLocationKind, Location, TraceFailed},
-    trace::{default_tracer, TraceCollector, TraceIterator, Tracer},
+    trace::{default_tracer, AOTTraceIterator, TraceCollector, Tracer},
     ykstats::{TimingState, YkStats},
 };
 use yktracec::promote;
@@ -483,15 +483,14 @@ impl MT {
     ///     in the `hl_arc`.
     fn queue_compile_job(
         self: &Arc<Self>,
-        trace_iter: Box<dyn TraceIterator>,
+        trace_iter: Box<dyn AOTTraceIterator>,
         hl_arc: Arc<Mutex<HotLocation>>,
         sidetrace: Option<(SideTraceInfo, Arc<CompiledTrace>)>,
     ) {
         self.stats.trace_collected_ok();
         let mt = Arc::clone(self);
         let do_compile = move || {
             mt.stats.timing_state(TimingState::TraceMapping);
-            let irtrace = trace_iter.collect::<Vec<_>>();
             debug_assert!(
                 sidetrace.is_none() || matches!(hl_arc.lock().kind, HotLocationKind::Compiled(_))
             );
@@ -504,7 +503,7 @@ impl MT {
             let guardid = sidetrace.as_ref().map(|x| x.0.guardid);
             match compiler.compile(
                 Arc::clone(&mt),
-                irtrace,
+                trace_iter,
                 sidetrace.as_ref().map(|x| x.0),
                 Arc::clone(&hl_arc),
             ) {

ykrt/src/trace/hwt/mapper.rs

@@ -196,6 +196,16 @@ impl HWTMapper {
                 }
             }
         }
+
+        // The last block contains pointless unmappable code (the stop tracing call).
+        match ret.pop() {
+            Some(x) => {
+                // This is a rough proxy for "check that we removed only the thing we want to
+                // remove".
+                assert!(matches!(x, TracedAOTBlock::Unmappable));
+            }
+            _ => unreachable!(),
+        }
         Ok(ret)
     }
 }

ykrt/src/trace/hwt/mod.rs

@@ -1,6 +1,6 @@
 //! Hardware tracing via hwtracer.
 
-use super::{errors::InvalidTraceError, TraceCollector, TraceIterator, TracedAOTBlock};
+use super::{errors::InvalidTraceError, AOTTraceIterator, TraceCollector, TracedAOTBlock};
 use std::{error::Error, sync::Arc};
 
 pub(crate) mod mapper;
@@ -33,7 +33,7 @@ struct HWTTraceCollector {
 }
 
 impl TraceCollector for HWTTraceCollector {
-    fn stop_collector(self: Box<Self>) -> Result<Box<dyn TraceIterator>, InvalidTraceError> {
+    fn stop_collector(self: Box<Self>) -> Result<Box<dyn AOTTraceIterator>, InvalidTraceError> {
         let tr = self.thread_tracer.stop_collector().unwrap();
         let mut mt = HWTMapper::new();
         let mapped = mt
@@ -53,7 +53,7 @@ struct HWTTraceIterator {
     trace: std::vec::IntoIter<TracedAOTBlock>,
 }
 
-impl TraceIterator for HWTTraceIterator {}
+impl AOTTraceIterator for HWTTraceIterator {}
 
 impl Iterator for HWTTraceIterator {
     type Item = TracedAOTBlock;

ykrt/src/trace/mod.rs

@@ -46,11 +46,11 @@ pub(crate) fn default_tracer() -> Result<Arc<dyn Tracer>, Box<dyn Error>> {
 pub(crate) trait TraceCollector {
     /// Stop collecting a trace of the current thread and return an iterator which successively
     /// produces the traced blocks.
-    fn stop_collector(self: Box<Self>) -> Result<Box<dyn TraceIterator>, InvalidTraceError>;
+    fn stop_collector(self: Box<Self>) -> Result<Box<dyn AOTTraceIterator>, InvalidTraceError>;
 }
 
 /// An iterator which takes an underlying raw trace and successively produces [TracedAOTBlock]s.
-pub(crate) trait TraceIterator: Iterator<Item = TracedAOTBlock> + Send {}
+pub(crate) trait AOTTraceIterator: Iterator<Item = TracedAOTBlock> + Send {}
 
 /// An AOT LLVM IR block that has been traced at JIT time.
 #[derive(Debug, Eq, PartialEq)]

yktracec/src/jitmodbuilder.cc

@@ -1322,13 +1322,7 @@ class JITModBuilder {
           continue;
         }
 
-        size_t LastBlockIndex;
-        if (IsSWTrace) {
-          LastBlockIndex = InpTrace.Length();
-        } else {
-          LastBlockIndex = InpTrace.Length() - 1;
-        }
-        if (Idx > 0 && Idx < LastBlockIndex) {
+        if (Idx < InpTrace.Length()) {
           // Stores into YkCtrlPointVars only need to be copied if they appear
           // at the beginning or end of the trace. Any YkCtrlPointVars stores
           // inbetween come from tracing over the control point and aren't
@@ -1349,8 +1343,7 @@ class JITModBuilder {
                 I++;
                 CurInstrIdx++;
               }
-              assert(InpTrace.Length() > 2);
-              if (Idx == LastBlockIndex - 1) {
+              if (Idx == InpTrace.Length() - 1) {
                 // Once we reached the YkCtrlPointVars stores at the end of the
                 // trace, we're done. We don't need to copy those instructions
                 // over, since all YkCtrlPointVars are stored on the shadow