Index: doc/theses/thierry_delisle_PhD/thesis/fig/base.fig =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/fig/base.fig (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/fig/base.fig (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -12,4 +12,9 @@ 1 3 0 1 0 0 50 -1 20 0.000 1 0.0000 6900 4200 20 20 6900 4200 6920 4200 1 3 0 1 0 0 50 -1 20 0.000 1 0.0000 6975 4200 20 20 6975 4200 6995 4200 +-6 +6 6375 5100 6675 5250 +1 3 0 1 0 0 50 -1 20 0.000 1 0.0000 6450 5175 20 20 6450 5175 6470 5175 +1 3 0 1 0 0 50 -1 20 0.000 1 0.0000 6525 5175 20 20 6525 5175 6545 5175 +1 3 0 1 0 0 50 -1 20 0.000 1 0.0000 6600 5175 20 20 6600 5175 6620 5175 -6 1 3 0 1 0 7 50 -1 -1 0.000 1 0.0000 3900 2400 300 300 3900 2400 4200 2400 @@ -75,4 +80,13 @@ 2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 0 0 2 2400 2475 3000 2475 +2 3 0 1 0 7 50 -1 -1 0.000 0 0 0 0 0 7 + 3300 5210 3150 4950 2850 4950 2700 5210 2850 5470 3150 5470 + 3300 5210 +2 3 0 1 0 7 50 -1 -1 0.000 0 0 0 0 0 7 + 4500 5210 4350 4950 4050 4950 3900 5210 4050 5470 4350 5470 + 4500 5210 +2 3 0 1 0 7 50 -1 -1 0.000 0 0 0 0 0 7 + 5700 5210 5550 4950 5250 4950 5100 5210 5250 5470 5550 5470 + 5700 5210 4 2 -1 50 -1 0 12 0.0000 2 135 630 2100 3075 Threads\001 4 2 -1 50 -1 0 12 0.0000 2 165 450 2100 2850 Ready\001 @@ -82,2 +96,3 @@ 4 1 -1 50 -1 0 11 0.0000 2 135 180 2700 3550 TS\001 4 1 -1 50 -1 0 11 0.0000 2 135 180 2700 2650 TS\001 +4 2 -1 50 -1 0 12 0.0000 2 135 900 2100 5175 Processors\001 Index: doc/theses/thierry_delisle_PhD/thesis/glossary.tex =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/glossary.tex (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/glossary.tex (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -40,4 +40,10 @@ \textit{Synonyms : User threads, Lightweight threads, Green threads, Virtual threads, Tasks.} +} + +\longnewglossaryentry{rmr} +{name={remote memory reference}} +{ + } Index: doc/theses/thierry_delisle_PhD/thesis/text/core.tex =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/text/core.tex (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/text/core.tex (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -49,73 +49,113 @@ \section{Design} -In general, a na\"{i}ve \glsxtrshort{fifo} ready-queue does not scale with increased parallelism from \glspl{hthrd}, resulting in decreased performance. The problem is adding/removing \glspl{thrd} is a single point of contention. As shown in the evaluation sections, most production schedulers do scale when adding \glspl{hthrd}. The common solution to the single point of contention is to shard the ready-queue so each \gls{hthrd} can access the ready-queue without contention, increasing performance. +In general, a na\"{i}ve \glsxtrshort{fifo} ready-queue does not scale with increased parallelism from \glspl{hthrd}, resulting in decreased performance. The problem is adding/removing \glspl{thrd} is a single point of contention. As shown in the evaluation sections, most production schedulers do scale when adding \glspl{hthrd}. The solution to this problem is to shard the ready-queue : create multiple sub-ready-queues that multiple \glspl{hthrd} can access and modify without interfering. -\subsection{Sharding} \label{sec:sharding} -An interesting approach to sharding a queue is presented in \cit{Trevors paper}. This algorithm presents a queue with a relaxed \glsxtrshort{fifo} guarantee using an array of strictly \glsxtrshort{fifo} sublists as shown in Figure~\ref{fig:base}. Each \emph{cell} of the array has a timestamp for the last operation and a pointer to a linked-list with a lock. Each node in the list is marked with a timestamp indicating when it is added to the list. A push operation is done by picking a random cell, acquiring the list lock, and pushing to the list. If the cell is locked, the operation is simply retried on another random cell until a lock is acquired. A pop operation is done in a similar fashion except two random cells are picked. If both cells are unlocked with non-empty lists, the operation pops the node with the oldest timestamp. If one of the cells is unlocked and non-empty, the operation pops from that cell. If both cells are either locked or empty, the operation picks two new random cells and tries again. +Before going into the design of \CFA's scheduler proper, I want to discuss two sharding solutions which served as the inspiration scheduler in this thesis. +\subsection{Work-Stealing} + +As I mentioned in \ref{existing:workstealing}, a popular pattern shard the ready-queue is work-stealing. As mentionned, in this pattern each \gls{proc} has its own ready-queue and \glspl{proc} only access each other's ready-queue if they run out of work. +The interesting aspect of workstealing happen in easier scheduling cases, \ie enough work for everyone but no more and no load balancing needed. In these cases, work-stealing is close to optimal scheduling: it can achieve perfect locality and have no contention. +On the other hand, work-stealing schedulers only attempt to do load-balancing when a \gls{proc} runs out of work. +This means that the scheduler may never balance unfairness that does not result in a \gls{proc} running out of work. +Chapter~\ref{microbench} shows that in pathological cases this problem can lead to indefinite starvation. + + +Based on these observation, I conclude that \emph{perfect} scheduler should behave very similarly to work-stealing in the easy cases, but should have more proactive load-balancing if the need arises. + +\subsection{Relaxed-Fifo} +An entirely different scheme is to create a ``relaxed-FIFO'' queue as in \todo{cite Trevor's paper}. This approach forgos any ownership between \gls{proc} and ready-queue, and simply creates a pool of ready-queues from which the \glspl{proc} can pick from. +\Glspl{proc} choose ready-queus at random, but timestamps are added to all elements of the queue and dequeues are done by picking two queues and dequeing the oldest element. +The result is a queue that has both decent scalability and sufficient fairness. +The lack of ownership means that as long as one \gls{proc} is still able to repeatedly dequeue elements, it is unlikely that any element will stay on the queue for much longer than any other element. +This contrasts with work-stealing, where \emph{any} \gls{proc} busy for an extended period of time results in all the elements on its local queue to have to wait. Unless another \gls{proc} runs out of work. + +An important aspects of this scheme's fairness approach is that the timestamps make it possible to evaluate how long elements have been on the queue. +However, another major aspect is that \glspl{proc} will eagerly search for these older elements instead of focusing on specific queues. + +While the fairness, of this scheme is good, it does suffer in terms of performance. +It requires very wide sharding, \eg at least 4 queues per \gls{hthrd}, and the randomness means locality can suffer significantly and finding non-empty queues can be difficult. + +\section{\CFA} +The \CFA is effectively attempting to merge these two approaches, keeping the best of both. +It is based on the \begin{figure} \centering \input{base.pstex_t} - \caption[Relaxed FIFO list]{Relaxed FIFO list \smallskip\newline List at the base of the scheduler: an array of strictly FIFO lists. The timestamp is in all nodes and cell arrays.} + \caption[Base \CFA design]{Base \CFA design \smallskip\newline A list of sub-ready queues offers the sharding, two per \glspl{proc}. However, \glspl{proc} can access any of the sub-queues.} \label{fig:base} \end{figure} -\subsection{Finding threads} -Once threads have been distributed onto multiple queues, identifying empty queues becomes a problem. Indeed, if the number of \glspl{thrd} does not far exceed the number of queues, it is probable that several of the cell queues are empty. Figure~\ref{fig:empty} shows an example with 2 \glspl{thrd} running on 8 queues, where the chances of getting an empty queue is 75\% per pick, meaning two random picks yield a \gls{thrd} only half the time. This scenario leads to performance problems since picks that do not yield a \gls{thrd} are not useful and do not necessarily help make more informed guesses. -\begin{figure} - \centering - \input{empty.pstex_t} - \caption[``More empty'' Relaxed FIFO list]{``More empty'' Relaxed FIFO list \smallskip\newline Emptier state of the queue: the array contains many empty cells, that is strictly FIFO lists containing no elements.} - \label{fig:empty} -\end{figure} -There are several solutions to this problem, but they ultimately all have to encode if a cell has an empty list. My results show the density and locality of this encoding is generally the dominating factor in these scheme. Classic solutions to this problem use one of three techniques to encode the information: +% The common solution to the single point of contention is to shard the ready-queue so each \gls{hthrd} can access the ready-queue without contention, increasing performance. -\paragraph{Dense Information} Figure~\ref{fig:emptybit} shows a dense bitmask to identify the cell queues currently in use. This approach means processors can often find \glspl{thrd} in constant time, regardless of how many underlying queues are empty. Furthermore, modern x86 CPUs have extended bit manipulation instructions (BMI2) that allow searching the bitmask with very little overhead compared to the randomized selection approach for a filled ready queue, offering good performance even in cases with many empty inner queues. However, this technique has its limits: with a single word\footnote{Word refers here to however many bits can be written atomically.} bitmask, the total amount of ready-queue sharding is limited to the number of bits in the word. With a multi-word bitmask, this maximum limit can be increased arbitrarily, but the look-up time increases. Finally, a dense bitmap, either single or multi-word, causes additional contention problems that reduces performance because of cache misses after updates. This central update bottleneck also means the information in the bitmask is more often stale before a processor can use it to find an item, \ie mask read says there are available \glspl{thrd} but none on queue when the subsequent atomic check is done. +% \subsection{Sharding} \label{sec:sharding} +% An interesting approach to sharding a queue is presented in \cit{Trevors paper}. This algorithm presents a queue with a relaxed \glsxtrshort{fifo} guarantee using an array of strictly \glsxtrshort{fifo} sublists as shown in Figure~\ref{fig:base}. Each \emph{cell} of the array has a timestamp for the last operation and a pointer to a linked-list with a lock. Each node in the list is marked with a timestamp indicating when it is added to the list. A push operation is done by picking a random cell, acquiring the list lock, and pushing to the list. If the cell is locked, the operation is simply retried on another random cell until a lock is acquired. A pop operation is done in a similar fashion except two random cells are picked. If both cells are unlocked with non-empty lists, the operation pops the node with the oldest timestamp. If one of the cells is unlocked and non-empty, the operation pops from that cell. If both cells are either locked or empty, the operation picks two new random cells and tries again. -\begin{figure} - \centering - \vspace*{-5pt} - {\resizebox{0.75\textwidth}{!}{\input{emptybit.pstex_t}}} - \vspace*{-5pt} - \caption[Underloaded queue with bitmask]{Underloaded queue with bitmask indicating array cells with items.} - \label{fig:emptybit} +% \begin{figure} +% \centering +% \input{base.pstex_t} +% \caption[Relaxed FIFO list]{Relaxed FIFO list \smallskip\newline List at the base of the scheduler: an array of strictly FIFO lists. The timestamp is in all nodes and cell arrays.} +% \label{fig:base} +% \end{figure} - \vspace*{10pt} - {\resizebox{0.75\textwidth}{!}{\input{emptytree.pstex_t}}} - \vspace*{-5pt} - \caption[Underloaded queue with binary search-tree]{Underloaded queue with binary search-tree indicating array cells with items.} - \label{fig:emptytree} +% \subsection{Finding threads} +% Once threads have been distributed onto multiple queues, identifying empty queues becomes a problem. Indeed, if the number of \glspl{thrd} does not far exceed the number of queues, it is probable that several of the cell queues are empty. Figure~\ref{fig:empty} shows an example with 2 \glspl{thrd} running on 8 queues, where the chances of getting an empty queue is 75\% per pick, meaning two random picks yield a \gls{thrd} only half the time. This scenario leads to performance problems since picks that do not yield a \gls{thrd} are not useful and do not necessarily help make more informed guesses. - \vspace*{10pt} - {\resizebox{0.95\textwidth}{!}{\input{emptytls.pstex_t}}} - \vspace*{-5pt} - \caption[Underloaded queue with per processor bitmask]{Underloaded queue with per processor bitmask indicating array cells with items.} - \label{fig:emptytls} -\end{figure} +% \begin{figure} +% \centering +% \input{empty.pstex_t} +% \caption[``More empty'' Relaxed FIFO list]{``More empty'' Relaxed FIFO list \smallskip\newline Emptier state of the queue: the array contains many empty cells, that is strictly FIFO lists containing no elements.} +% \label{fig:empty} +% \end{figure} -\paragraph{Sparse Information} Figure~\ref{fig:emptytree} shows an approach using a hierarchical tree data-structure to reduce contention and has been shown to work in similar cases~\cite{ellen2007snzi}. However, this approach may lead to poorer performance due to the inherent pointer chasing cost while still allowing significant contention on the nodes of the tree if the tree is shallow. +% There are several solutions to this problem, but they ultimately all have to encode if a cell has an empty list. My results show the density and locality of this encoding is generally the dominating factor in these scheme. Classic solutions to this problem use one of three techniques to encode the information: -\paragraph{Local Information} Figure~\ref{fig:emptytls} shows an approach using dense information, similar to the bitmap, but each \gls{hthrd} keeps its own independent copy. While this approach can offer good scalability \emph{and} low latency, the liveliness and discovery of the information can become a problem. This case is made worst in systems with few processors where even blind random picks can find \glspl{thrd} in a few tries. +% \paragraph{Dense Information} Figure~\ref{fig:emptybit} shows a dense bitmask to identify the cell queues currently in use. This approach means processors can often find \glspl{thrd} in constant time, regardless of how many underlying queues are empty. Furthermore, modern x86 CPUs have extended bit manipulation instructions (BMI2) that allow searching the bitmask with very little overhead compared to the randomized selection approach for a filled ready queue, offering good performance even in cases with many empty inner queues. However, this technique has its limits: with a single word\footnote{Word refers here to however many bits can be written atomically.} bitmask, the total amount of ready-queue sharding is limited to the number of bits in the word. With a multi-word bitmask, this maximum limit can be increased arbitrarily, but the look-up time increases. Finally, a dense bitmap, either single or multi-word, causes additional contention problems that reduces performance because of cache misses after updates. This central update bottleneck also means the information in the bitmask is more often stale before a processor can use it to find an item, \ie mask read says there are available \glspl{thrd} but none on queue when the subsequent atomic check is done. -I built a prototype of these approaches and none of these techniques offer satisfying performance when few threads are present. All of these approach hit the same 2 problems. First, randomly picking sub-queues is very fast. That speed means any improvement to the hit rate can easily be countered by a slow-down in look-up speed, whether or not there are empty lists. Second, the array is already sharded to avoid contention bottlenecks, so any denser data structure tends to become a bottleneck. In all cases, these factors meant the best cases scenario, \ie many threads, would get worst throughput, and the worst-case scenario, few threads, would get a better hit rate, but an equivalent poor throughput. As a result I tried an entirely different approach. +% \begin{figure} +% \centering +% \vspace*{-5pt} +% {\resizebox{0.75\textwidth}{!}{\input{emptybit.pstex_t}}} +% \vspace*{-5pt} +% \caption[Underloaded queue with bitmask]{Underloaded queue with bitmask indicating array cells with items.} +% \label{fig:emptybit} -\subsection{Dynamic Entropy}\cit{https://xkcd.com/2318/} -In the worst-case scenario there are only few \glspl{thrd} ready to run, or more precisely given $P$ \glspl{proc}\footnote{For simplicity, this assumes there is a one-to-one match between \glspl{proc} and \glspl{hthrd}.}, $T$ \glspl{thrd} and $\epsilon$ a very small number, than the worst case scenario can be represented by $T = P + \epsilon$, with $\epsilon \ll P$. It is important to note in this case that fairness is effectively irrelevant. Indeed, this case is close to \emph{actually matching} the model of the ``Ideal multi-tasking CPU'' on page \pageref{q:LinuxCFS}. In this context, it is possible to use a purely internal-locality based approach and still meet the fairness requirements. This approach simply has each \gls{proc} running a single \gls{thrd} repeatedly. Or from the shared ready-queue viewpoint, each \gls{proc} pushes to a given sub-queue and then pops from the \emph{same} subqueue. The challenge is for the the scheduler to achieve good performance in both the $T = P + \epsilon$ case and the $T \gg P$ case, without affecting the fairness guarantees in the later. +% \vspace*{10pt} +% {\resizebox{0.75\textwidth}{!}{\input{emptytree.pstex_t}}} +% \vspace*{-5pt} +% \caption[Underloaded queue with binary search-tree]{Underloaded queue with binary search-tree indicating array cells with items.} +% \label{fig:emptytree} -To handle this case, I use a \glsxtrshort{prng}\todo{Fix missing long form} in a novel way. There exist \glsxtrshort{prng}s that are fast, compact and can be run forward \emph{and} backwards. Linear congruential generators~\cite{wiki:lcg} are an example of \glsxtrshort{prng}s of such \glsxtrshort{prng}s. The novel approach is to use the ability to run backwards to ``replay'' the \glsxtrshort{prng}. The scheduler uses an exclusive \glsxtrshort{prng} instance per \gls{proc}, the random-number seed effectively starts an encoding that produces a list of all accessed subqueues, from latest to oldest. Replaying the \glsxtrshort{prng} to identify cells accessed recently and which probably have data still cached. +% \vspace*{10pt} +% {\resizebox{0.95\textwidth}{!}{\input{emptytls.pstex_t}}} +% \vspace*{-5pt} +% \caption[Underloaded queue with per processor bitmask]{Underloaded queue with per processor bitmask indicating array cells with items.} +% \label{fig:emptytls} +% \end{figure} -The algorithm works as follows: -\begin{itemize} - \item Each \gls{proc} has two \glsxtrshort{prng} instances, $F$ and $B$. - \item Push and Pop operations occur as discussed in Section~\ref{sec:sharding} with the following exceptions: - \begin{itemize} - \item Push operations use $F$ going forward on each try and on success $F$ is copied into $B$. - \item Pop operations use $B$ going backwards on each try. - \end{itemize} -\end{itemize} +% \paragraph{Sparse Information} Figure~\ref{fig:emptytree} shows an approach using a hierarchical tree data-structure to reduce contention and has been shown to work in similar cases~\cite{ellen2007snzi}. However, this approach may lead to poorer performance due to the inherent pointer chasing cost while still allowing significant contention on the nodes of the tree if the tree is shallow. -The main benefit of this technique is that it basically respects the desired properties of Figure~\ref{fig:fair}. When looking for work, a \gls{proc} first looks at the last cell they pushed to, if any, and then move backwards through its accessed cells. As the \gls{proc} continues looking for work, $F$ moves backwards and $B$ stays in place. As a result, the relation between the two becomes weaker, which means that the probablisitic fairness of the algorithm reverts to normal. Chapter~\ref{proofs} discusses more formally the fairness guarantees of this algorithm. +% \paragraph{Local Information} Figure~\ref{fig:emptytls} shows an approach using dense information, similar to the bitmap, but each \gls{hthrd} keeps its own independent copy. While this approach can offer good scalability \emph{and} low latency, the liveliness and discovery of the information can become a problem. This case is made worst in systems with few processors where even blind random picks can find \glspl{thrd} in a few tries. -\section{Details} +% I built a prototype of these approaches and none of these techniques offer satisfying performance when few threads are present. All of these approach hit the same 2 problems. First, randomly picking sub-queues is very fast. That speed means any improvement to the hit rate can easily be countered by a slow-down in look-up speed, whether or not there are empty lists. Second, the array is already sharded to avoid contention bottlenecks, so any denser data structure tends to become a bottleneck. In all cases, these factors meant the best cases scenario, \ie many threads, would get worst throughput, and the worst-case scenario, few threads, would get a better hit rate, but an equivalent poor throughput. As a result I tried an entirely different approach. + +% \subsection{Dynamic Entropy}\cit{https://xkcd.com/2318/} +% In the worst-case scenario there are only few \glspl{thrd} ready to run, or more precisely given $P$ \glspl{proc}\footnote{For simplicity, this assumes there is a one-to-one match between \glspl{proc} and \glspl{hthrd}.}, $T$ \glspl{thrd} and $\epsilon$ a very small number, than the worst case scenario can be represented by $T = P + \epsilon$, with $\epsilon \ll P$. It is important to note in this case that fairness is effectively irrelevant. Indeed, this case is close to \emph{actually matching} the model of the ``Ideal multi-tasking CPU'' on page \pageref{q:LinuxCFS}. In this context, it is possible to use a purely internal-locality based approach and still meet the fairness requirements. This approach simply has each \gls{proc} running a single \gls{thrd} repeatedly. Or from the shared ready-queue viewpoint, each \gls{proc} pushes to a given sub-queue and then pops from the \emph{same} subqueue. The challenge is for the the scheduler to achieve good performance in both the $T = P + \epsilon$ case and the $T \gg P$ case, without affecting the fairness guarantees in the later. + +% To handle this case, I use a \glsxtrshort{prng}\todo{Fix missing long form} in a novel way. There exist \glsxtrshort{prng}s that are fast, compact and can be run forward \emph{and} backwards. Linear congruential generators~\cite{wiki:lcg} are an example of \glsxtrshort{prng}s of such \glsxtrshort{prng}s. The novel approach is to use the ability to run backwards to ``replay'' the \glsxtrshort{prng}. The scheduler uses an exclusive \glsxtrshort{prng} instance per \gls{proc}, the random-number seed effectively starts an encoding that produces a list of all accessed subqueues, from latest to oldest. Replaying the \glsxtrshort{prng} to identify cells accessed recently and which probably have data still cached. + +% The algorithm works as follows: +% \begin{itemize} +% \item Each \gls{proc} has two \glsxtrshort{prng} instances, $F$ and $B$. +% \item Push and Pop operations occur as discussed in Section~\ref{sec:sharding} with the following exceptions: +% \begin{itemize} +% \item Push operations use $F$ going forward on each try and on success $F$ is copied into $B$. +% \item Pop operations use $B$ going backwards on each try. +% \end{itemize} +% \end{itemize} + +% The main benefit of this technique is that it basically respects the desired properties of Figure~\ref{fig:fair}. When looking for work, a \gls{proc} first looks at the last cell they pushed to, if any, and then move backwards through its accessed cells. As the \gls{proc} continues looking for work, $F$ moves backwards and $B$ stays in place. As a result, the relation between the two becomes weaker, which means that the probablisitic fairness of the algorithm reverts to normal. Chapter~\ref{proofs} discusses more formally the fairness guarantees of this algorithm. + +% \section{Details} Index: doc/theses/thierry_delisle_PhD/thesis/text/eval_macro.tex =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/text/eval_macro.tex (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/text/eval_macro.tex (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -4,6 +4,4 @@ In Memory Plain Text - -Networked Plain Text Networked ZIPF Index: doc/theses/thierry_delisle_PhD/thesis/text/eval_micro.tex =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/text/eval_micro.tex (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/text/eval_micro.tex (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -2,26 +2,24 @@ The first step of evaluation is always to test-out small controlled cases, to ensure that the basics are working properly. -This sections presents four different experimental setup, evaluating some of the basic features of \CFA's scheduler. +This sections presents five different experimental setup, evaluating some of the basic features of \CFA's scheduler. \section{Cycling latency} The most basic evaluation of any ready queue is to evaluate the latency needed to push and pop one element from the ready-queue. -While these two operation also describe a \texttt{yield} operation, many systems use this as the most basic benchmark. -However, yielding can be treated as a special case, since it also carries the information that the length of the ready queue will not change. +Since these two operation also describe a \texttt{yield} operation, many systems use this as the most basic benchmark. +However, yielding can be treated as a special case, since it also carries the information that the number of the ready \glspl{at} will not change. Not all systems use this information, but those which do may appear to have better performance than they would for disconnected push/pop pairs. For this reason, I chose a different first benchmark, which I call the Cycle Benchmark. -This benchmark arranges many threads into multiple rings of threads. +This benchmark arranges many \glspl{at} into multiple rings of \glspl{at}. Each ring is effectively a circular singly-linked list. -At runtime, each thread unparks the next thread before parking itself. +At runtime, each \gls{at} unparks the next \gls{at} before parking itself. This corresponds to the desired pair of ready queue operations. -Unparking the next thread requires pushing that thread onto the ready queue and the ensuing park will cause the runtime to pop a thread from the ready-queue. +Unparking the next \gls{at} requires pushing that \gls{at} onto the ready queue and the ensuing park will cause the runtime to pop a \gls{at} from the ready-queue. Figure~\ref{fig:cycle} shows a visual representation of this arrangement. -The goal of this ring is that the underlying runtime cannot rely on the guarantee that the number of ready threads will stay constant over the duration of the experiment. -In fact, the total number of threads waiting on the ready is expected to vary a little because of the race between the next thread unparking and the current thread parking. -The size of the cycle is also decided based on this race: cycles that are too small may see the -chain of unparks go full circle before the first thread can park. +The goal of this ring is that the underlying runtime cannot rely on the guarantee that the number of ready \glspl{at} will stay constant over the duration of the experiment. +In fact, the total number of \glspl{at} waiting on the ready queue is expected to vary because of the race between the next \gls{at} unparking and the current \gls{at} parking. +The size of the cycle is also decided based on this race: cycles that are too small may see the chain of unparks go full circle before the first \gls{at} can park. While this would not be a correctness problem, every runtime system must handle that race, it could lead to pushes and pops being optimized away. -Since silently omitting ready-queue operations would throw off the measuring of these operations. -Therefore the ring of threads must be big enough so the threads have the time to fully park before they are unparked. +Since silently omitting ready-queue operations would throw off the measuring of these operations, the ring of \glspl{at} must be big enough so the \glspl{at} have the time to fully park before they are unparked. Note that this problem is only present on SMP machines and is significantly mitigated by the fact that there are multiple rings in the system. @@ -29,22 +27,144 @@ \centering \input{cycle.pstex_t} - \caption[Cycle benchmark]{Cycle benchmark\smallskip\newline Each thread unparks the next thread in the cycle before parking itself.} + \caption[Cycle benchmark]{Cycle benchmark\smallskip\newline Each \gls{at} unparks the next \gls{at} in the cycle before parking itself.} \label{fig:cycle} \end{figure} \todo{check term ``idle sleep handling''} -To avoid this benchmark from being dominated by the idle sleep handling, the number of rings is kept at least as high as the number of processors available. +To avoid this benchmark from being dominated by the idle sleep handling, the number of rings is kept at least as high as the number of \glspl{proc} available. Beyond this point, adding more rings serves to mitigate even more the idle sleep handling. -This is to avoid the case where one of the worker threads runs out of work because of the variation on the number of ready threads mentionned above. +This is to avoid the case where one of the worker \glspl{at} runs out of work because of the variation on the number of ready \glspl{at} mentionned above. The actual benchmark is more complicated to handle termination, but that simply requires using a binary semphore or a channel instead of raw \texttt{park}/\texttt{unpark} and carefully picking the order of the \texttt{P} and \texttt{V} with respect to the loop condition. -\todo{mention where to get the code.} +\todo{code, setup, results} +\begin{lstlisting} + Thread.main() { + count := 0 + for { + wait() + this.next.wake() + count ++ + if must_stop() { break } + } + global.count += count + } +\end{lstlisting} + \section{Yield} For completion, I also include the yield benchmark. -This benchmark is much simpler than the cycle tests, it simply creates many threads that call \texttt{yield}. +This benchmark is much simpler than the cycle tests, it simply creates many \glspl{at} that call \texttt{yield}. +As mentionned in the previous section, this benchmark may be less representative of usages that only make limited use of \texttt{yield}, due to potential shortcuts in the routine. +Its only interesting variable is the number of \glspl{at} per \glspl{proc}, where ratios close to 1 means the ready queue(s) could be empty. +This sometimes puts more strain on the idle sleep handling, compared to scenarios where there is clearly plenty of work to be done. + +\todo{code, setup, results} + +\begin{lstlisting} + Thread.main() { + count := 0 + while !stop { + yield() + count ++ + } + global.count += count + } +\end{lstlisting} + + +\section{Churn} +The Cycle and Yield benchmark represents an ``easy'' scenario for a scheduler, \eg, an embarrassingly parallel application. +In these benchmarks, \glspl{at} can be easily partitioned over the different \glspl{proc} up-front and none of the \glspl{at} communicate with each other. + +The Churn benchmark represents more chaotic usages, where there is no relation between the last \gls{proc} on which a \gls{at} ran and the \gls{proc} that unblocked it. +When a \gls{at} is unblocked from a different \gls{proc} than the one on which it last ran, the unblocking \gls{proc} must either ``steal'' the \gls{at} or place it on a remote queue. +This results can result in either contention on the remote queue or \glspl{rmr} on \gls{at} data structure. +In either case, this benchmark aims to highlight how each scheduler handles these cases, since both cases can lead to performance degradation if they are not handled correctly. + +To achieve this the benchmark uses a fixed size array of \newterm{chair}s, where a chair is a data structure that holds a single blocked \gls{at}. +When a \gls{at} attempts to block on the chair, it must first unblocked the \gls{at} currently blocked on said chair, if any. +This creates a flow where \glspl{at} push each other out of the chairs before being pushed out themselves. +For this benchmark to work however, the number of \glspl{at} must be equal or greater to the number of chairs plus the number of \glspl{proc}. + +\todo{code, setup, results} +\begin{lstlisting} + Thread.main() { + count := 0 + for { + r := random() % len(spots) + next := xchg(spots[r], this) + if next { next.wake() } + wait() + count ++ + if must_stop() { break } + } + global.count += count + } +\end{lstlisting} \section{Locality} +\todo{code, setup, results} + \section{Transfer} +The last benchmark is more exactly characterize as an experiment than a benchmark. +It tests the behavior of the schedulers for a particularly misbehaved workload. +In this workload, one of the \gls{at} is selected at random to be the leader. +The leader then spins in a tight loop until it has observed that all other \glspl{at} have acknowledged its leadership. +The leader \gls{at} then picks a new \gls{at} to be the ``spinner'' and the cycle repeats. + +The benchmark comes in two flavours for the behavior of the non-leader \glspl{at}: +once they acknowledged the leader, they either block on a semaphore or yield repeatadly. + +This experiment is designed to evaluate the short term load balancing of the scheduler. +Indeed, schedulers where the runnable \glspl{at} are partitioned on the \glspl{proc} may need to balance the \glspl{at} for this experient to terminate. +This is because the spinning \gls{at} is effectively preventing the \gls{proc} from runnning any other \glspl{thrd}. +In the semaphore flavour, the number of runnable \glspl{at} will eventually dwindle down to only the leader. +This is a simpler case to handle for schedulers since \glspl{proc} eventually run out of work. +In the yielding flavour, the number of runnable \glspl{at} stays constant. +This is a harder case to handle because corrective measures must be taken even if work is still available. +Note that languages that have mandatory preemption do circumvent this problem by forcing the spinner to yield. + +\todo{code, setup, results} +\begin{lstlisting} + Thread.lead() { + this.idx_seen = ++lead_idx + if lead_idx > stop_idx { + done := true + return + } + + // Wait for everyone to acknowledge my leadership + start: = timeNow() + for t in threads { + while t.idx_seen != lead_idx { + asm pause + if (timeNow() - start) > 5 seconds { error() } + } + } + + // pick next leader + leader := threads[ prng() % len(threads) ] + + // wake every one + if !exhaust { + for t in threads { + if t != me { t.wake() } + } + } + } + + Thread.wait() { + this.idx_seen := lead_idx + if exhaust { wait() } + else { yield() } + } + + Thread.main() { + while !done { + if leader == me { this.lead() } + else { this.wait() } + } + } +\end{lstlisting} Index: doc/theses/thierry_delisle_PhD/thesis/text/existing.tex =================================================================== --- doc/theses/thierry_delisle_PhD/thesis/text/existing.tex (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ doc/theses/thierry_delisle_PhD/thesis/text/existing.tex (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -33,5 +33,5 @@ -\section{Work Stealing} +\section{Work Stealing}\label{existing:workstealing} One of the most popular scheduling algorithm in practice (see~\ref{existing:prod}) is work-stealing. This idea, introduce by \cite{DBLP:conf/fpca/BurtonS81}, effectively has each worker work on its local tasks first, but allows the possibility for other workers to steal local tasks if they run out of tasks. \cite{DBLP:conf/focs/Blumofe94} introduced the more familiar incarnation of this, where each workers has queue of tasks to accomplish and workers without tasks steal tasks from random workers. (The Burton and Sleep algorithm had trees of tasks and stole only among neighbours). Blumofe and Leiserson also prove worst case space and time requirements for well-structured computations. Index: libcfa/src/concurrency/io.cfa =================================================================== --- libcfa/src/concurrency/io.cfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/io.cfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -306,5 +306,10 @@ ctx->proc->io.pending = true; ctx->proc->io.dirty = true; - if(sq.to_submit > 30 || !lazy) { + if(sq.to_submit > 30) { + __tls_stats()->io.flush.full++; + __cfa_io_flush( ctx->proc, 0 ); + } + if(!lazy) { + __tls_stats()->io.flush.eager++; __cfa_io_flush( ctx->proc, 0 ); } Index: libcfa/src/concurrency/kernel.cfa =================================================================== --- libcfa/src/concurrency/kernel.cfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/kernel.cfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -42,7 +42,7 @@ #if !defined(__CFA_NO_STATISTICS__) - #define __STATS( ...) __VA_ARGS__ + #define __STATS_DEF( ...) __VA_ARGS__ #else - #define __STATS( ...) + #define __STATS_DEF( ...) #endif @@ -122,4 +122,5 @@ static thread$ * __next_thread(cluster * this); static thread$ * __next_thread_slow(cluster * this); +static thread$ * __next_thread_search(cluster * this); static inline bool __must_unpark( thread$ * thrd ) __attribute((nonnull(1))); static void __run_thread(processor * this, thread$ * dst); @@ -187,6 +188,4 @@ MAIN_LOOP: for() { - #define OLD_MAIN 1 - #if OLD_MAIN // Check if there is pending io __maybe_io_drain( this ); @@ -196,7 +195,18 @@ if( !readyThread ) { + __IO_STATS__(true, io.flush.idle++; ) __cfa_io_flush( this, 0 ); + readyThread = __next_thread( this->cltr ); + } + + if( !readyThread ) for(5) { + __IO_STATS__(true, io.flush.idle++; ) + readyThread = __next_thread_slow( this->cltr ); + + if( readyThread ) break; + + __cfa_io_flush( this, 0 ); } @@ -206,20 +216,14 @@ if( __atomic_load_n(&this->do_terminate, __ATOMIC_SEQ_CST) ) break MAIN_LOOP; - #if !defined(__CFA_NO_STATISTICS__) - __tls_stats()->ready.sleep.halts++; - #endif - // Push self to idle stack if(!mark_idle(this->cltr->procs, * this)) continue MAIN_LOOP; // Confirm the ready-queue is empty - readyThread = __next_thread_slow( this->cltr ); + readyThread = __next_thread_search( this->cltr ); if( readyThread ) { // A thread was found, cancel the halt mark_awake(this->cltr->procs, * this); - #if !defined(__CFA_NO_STATISTICS__) - __tls_stats()->ready.sleep.cancels++; - #endif + __STATS__(true, ready.sleep.cancels++; ) // continue the mai loop @@ -248,122 +252,7 @@ if(this->io.pending && !this->io.dirty) { + __IO_STATS__(true, io.flush.dirty++; ) __cfa_io_flush( this, 0 ); } - - #else - #warning new kernel loop - SEARCH: { - /* paranoid */ verify( ! __preemption_enabled() ); - - // First, lock the scheduler since we are searching for a thread - ready_schedule_lock(); - - // Try to get the next thread - readyThread = pop_fast( this->cltr ); - if(readyThread) { ready_schedule_unlock(); break SEARCH; } - - // If we can't find a thread, might as well flush any outstanding I/O - if(this->io.pending) { __cfa_io_flush( this, 0 ); } - - // Spin a little on I/O, just in case - for(5) { - __maybe_io_drain( this ); - readyThread = pop_fast( this->cltr ); - if(readyThread) { ready_schedule_unlock(); break SEARCH; } - } - - // no luck, try stealing a few times - for(5) { - if( __maybe_io_drain( this ) ) { - readyThread = pop_fast( this->cltr ); - } else { - readyThread = pop_slow( this->cltr ); - } - if(readyThread) { ready_schedule_unlock(); break SEARCH; } - } - - // still no luck, search for a thread - readyThread = pop_search( this->cltr ); - if(readyThread) { ready_schedule_unlock(); break SEARCH; } - - // Don't block if we are done - if( __atomic_load_n(&this->do_terminate, __ATOMIC_SEQ_CST) ) { - ready_schedule_unlock(); - break MAIN_LOOP; - } - - __STATS( __tls_stats()->ready.sleep.halts++; ) - - // Push self to idle stack - ready_schedule_unlock(); - if(!mark_idle(this->cltr->procs, * this)) goto SEARCH; - ready_schedule_lock(); - - // Confirm the ready-queue is empty - __maybe_io_drain( this ); - readyThread = pop_search( this->cltr ); - ready_schedule_unlock(); - - if( readyThread ) { - // A thread was found, cancel the halt - mark_awake(this->cltr->procs, * this); - - __STATS( __tls_stats()->ready.sleep.cancels++; ) - - // continue the main loop - break SEARCH; - } - - __STATS( if(this->print_halts) __cfaabi_bits_print_safe( STDOUT_FILENO, "PH:%d - %lld 0\n", this->unique_id, rdtscl()); ) - __cfadbg_print_safe(runtime_core, "Kernel : core %p waiting on eventfd %d\n", this, this->idle_fd); - - { - eventfd_t val; - ssize_t ret = read( this->idle_fd, &val, sizeof(val) ); - if(ret < 0) { - switch((int)errno) { - case EAGAIN: - #if EAGAIN != EWOULDBLOCK - case EWOULDBLOCK: - #endif - case EINTR: - // No need to do anything special here, just assume it's a legitimate wake-up - break; - default: - abort( "KERNEL : internal error, read failure on idle eventfd, error(%d) %s.", (int)errno, strerror( (int)errno ) ); - } - } - } - - __STATS( if(this->print_halts) __cfaabi_bits_print_safe( STDOUT_FILENO, "PH:%d - %lld 1\n", this->unique_id, rdtscl()); ) - - // We were woken up, remove self from idle - mark_awake(this->cltr->procs, * this); - - // DON'T just proceed, start looking again - continue MAIN_LOOP; - } - - RUN_THREAD: - /* paranoid */ verify( ! __preemption_enabled() ); - /* paranoid */ verify( readyThread ); - - // Reset io dirty bit - this->io.dirty = false; - - // We found a thread run it - __run_thread(this, readyThread); - - // Are we done? - if( __atomic_load_n(&this->do_terminate, __ATOMIC_SEQ_CST) ) break MAIN_LOOP; - - if(this->io.pending && !this->io.dirty) { - __cfa_io_flush( this, 0 ); - } - - ready_schedule_lock(); - __maybe_io_drain( this ); - ready_schedule_unlock(); - #endif } @@ -476,7 +365,5 @@ break RUNNING; case TICKET_UNBLOCK: - #if !defined(__CFA_NO_STATISTICS__) - __tls_stats()->ready.threads.threads++; - #endif + __STATS__(true, ready.threads.threads++; ) // This is case 2, the racy case, someone tried to run this thread before it finished blocking // In this case, just run it again. @@ -493,7 +380,5 @@ __cfadbg_print_safe(runtime_core, "Kernel : core %p finished running thread %p\n", this, thrd_dst); - #if !defined(__CFA_NO_STATISTICS__) - __tls_stats()->ready.threads.threads--; - #endif + __STATS__(true, ready.threads.threads--; ) /* paranoid */ verify( ! __preemption_enabled() ); @@ -506,5 +391,5 @@ thread$ * thrd_src = kernelTLS().this_thread; - __STATS( thrd_src->last_proc = kernelTLS().this_processor; ) + __STATS_DEF( thrd_src->last_proc = kernelTLS().this_processor; ) // Run the thread on this processor @@ -558,5 +443,5 @@ // Dereference the thread now because once we push it, there is not guaranteed it's still valid. struct cluster * cl = thrd->curr_cluster; - __STATS(bool outside = hint == UNPARK_LOCAL && thrd->last_proc && thrd->last_proc != kernelTLS().this_processor; ) + __STATS_DEF(bool outside = hint == UNPARK_LOCAL && thrd->last_proc && thrd->last_proc != kernelTLS().this_processor; ) // push the thread to the cluster ready-queue @@ -609,12 +494,17 @@ ready_schedule_lock(); - thread$ * thrd; - for(25) { - thrd = pop_slow( this ); - if(thrd) goto RET; - } - thrd = pop_search( this ); - - RET: + thread$ * thrd = pop_slow( this ); + ready_schedule_unlock(); + + /* paranoid */ verify( ! __preemption_enabled() ); + return thrd; +} + +// KERNEL ONLY +static inline thread$ * __next_thread_search(cluster * this) with( *this ) { + /* paranoid */ verify( ! __preemption_enabled() ); + + ready_schedule_lock(); + thread$ * thrd = pop_search( this ); ready_schedule_unlock(); @@ -732,30 +622,46 @@ // Wake a thread from the front if there are any static void __wake_one(cluster * this) { + eventfd_t val; + /* paranoid */ verify( ! __preemption_enabled() ); /* paranoid */ verify( ready_schedule_islocked() ); // Check if there is a sleeping processor - // int fd = __atomic_load_n(&this->procs.fd, __ATOMIC_SEQ_CST); - int fd = 0; - if( __atomic_load_n(&this->procs.fd, __ATOMIC_SEQ_CST) != 0 ) { - fd = __atomic_exchange_n(&this->procs.fd, 0, __ATOMIC_RELAXED); - } - - // If no one is sleeping, we are done - if( fd == 0 ) return; - - // We found a processor, wake it up - eventfd_t val; - val = 1; - eventfd_write( fd, val ); - - #if !defined(__CFA_NO_STATISTICS__) - if( kernelTLS().this_stats ) { - __tls_stats()->ready.sleep.wakes++; - } - else { - __atomic_fetch_add(&this->stats->ready.sleep.wakes, 1, __ATOMIC_RELAXED); - } - #endif + struct __fd_waitctx * fdp = __atomic_load_n(&this->procs.fdw, __ATOMIC_SEQ_CST); + + // If no one is sleeping: we are done + if( fdp == 0p ) return; + + int fd = 1; + if( __atomic_load_n(&fdp->fd, __ATOMIC_SEQ_CST) != 1 ) { + fd = __atomic_exchange_n(&fdp->fd, 1, __ATOMIC_RELAXED); + } + + switch(fd) { + case 0: + // If the processor isn't ready to sleep then the exchange will already wake it up + #if !defined(__CFA_NO_STATISTICS__) + if( kernelTLS().this_stats ) { __tls_stats()->ready.sleep.early++; + } else { __atomic_fetch_add(&this->stats->ready.sleep.early, 1, __ATOMIC_RELAXED); } + #endif + break; + case 1: + // If someone else already said they will wake them: we are done + #if !defined(__CFA_NO_STATISTICS__) + if( kernelTLS().this_stats ) { __tls_stats()->ready.sleep.seen++; + } else { __atomic_fetch_add(&this->stats->ready.sleep.seen, 1, __ATOMIC_RELAXED); } + #endif + break; + default: + // If the processor was ready to sleep, we need to wake it up with an actual write + val = 1; + eventfd_write( fd, val ); + + #if !defined(__CFA_NO_STATISTICS__) + if( kernelTLS().this_stats ) { __tls_stats()->ready.sleep.wakes++; + } else { __atomic_fetch_add(&this->stats->ready.sleep.wakes, 1, __ATOMIC_RELAXED); } + #endif + break; + } /* paranoid */ verify( ready_schedule_islocked() ); @@ -770,4 +676,6 @@ __cfadbg_print_safe(runtime_core, "Kernel : waking Processor %p\n", this); + + this->idle_wctx.fd = 1; eventfd_t val; @@ -779,4 +687,19 @@ static void idle_sleep(processor * this, io_future_t & future, iovec & iov) { + // Tell everyone we are ready to go do sleep + for() { + int expected = this->idle_wctx.fd; + + // Someone already told us to wake-up! No time for a nap. + if(expected == 1) { return; } + + // Try to mark that we are going to sleep + if(__atomic_compare_exchange_n(&this->idle_wctx.fd, &expected, this->idle_fd, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ) { + // Every one agreed, taking a nap + break; + } + } + + #if !defined(CFA_WITH_IO_URING_IDLE) #if !defined(__CFA_NO_STATISTICS__) @@ -825,4 +748,8 @@ static bool mark_idle(__cluster_proc_list & this, processor & proc) { + __STATS__(true, ready.sleep.halts++; ) + + proc.idle_wctx.fd = 0; + /* paranoid */ verify( ! __preemption_enabled() ); if(!try_lock( this )) return false; @@ -832,5 +759,5 @@ insert_first(this.idles, proc); - __atomic_store_n(&this.fd, proc.idle_fd, __ATOMIC_SEQ_CST); + __atomic_store_n(&this.fdw, &proc.idle_wctx, __ATOMIC_SEQ_CST); unlock( this ); /* paranoid */ verify( ! __preemption_enabled() ); @@ -848,7 +775,7 @@ { - int fd = 0; - if(!this.idles`isEmpty) fd = this.idles`first.idle_fd; - __atomic_store_n(&this.fd, fd, __ATOMIC_SEQ_CST); + struct __fd_waitctx * wctx = 0; + if(!this.idles`isEmpty) wctx = &this.idles`first.idle_wctx; + __atomic_store_n(&this.fdw, wctx, __ATOMIC_SEQ_CST); } @@ -914,11 +841,7 @@ unsigned tail = *ctx->cq.tail; if(head == tail) return false; - #if OLD_MAIN - ready_schedule_lock(); - ret = __cfa_io_drain( proc ); - ready_schedule_unlock(); - #else - ret = __cfa_io_drain( proc ); - #endif + ready_schedule_lock(); + ret = __cfa_io_drain( proc ); + ready_schedule_unlock(); #endif return ret; Index: libcfa/src/concurrency/kernel.hfa =================================================================== --- libcfa/src/concurrency/kernel.hfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/kernel.hfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -53,4 +53,9 @@ coroutine processorCtx_t { struct processor * proc; +}; + + +struct __fd_waitctx { + volatile int fd; }; @@ -101,4 +106,7 @@ int idle_fd; + // Idle waitctx + struct __fd_waitctx idle_wctx; + // Termination synchronisation (user semaphore) oneshot terminated; @@ -207,5 +215,5 @@ // FD to use to wake a processor - volatile int fd; + struct __fd_waitctx * volatile fdw; // Total number of processors Index: libcfa/src/concurrency/kernel/fwd.hfa =================================================================== --- libcfa/src/concurrency/kernel/fwd.hfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/kernel/fwd.hfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -396,6 +396,18 @@ if( !(in_kernel) ) enable_interrupts(); \ } + #if defined(CFA_HAVE_LINUX_IO_URING_H) + #define __IO_STATS__(in_kernel, ...) { \ + if( !(in_kernel) ) disable_interrupts(); \ + with( *__tls_stats() ) { \ + __VA_ARGS__ \ + } \ + if( !(in_kernel) ) enable_interrupts(); \ + } + #else + #define __IO_STATS__(in_kernel, ...) + #endif #else #define __STATS__(in_kernel, ...) + #define __IO_STATS__(in_kernel, ...) #endif } Index: libcfa/src/concurrency/kernel/startup.cfa =================================================================== --- libcfa/src/concurrency/kernel/startup.cfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/kernel/startup.cfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -537,4 +537,11 @@ } + this.idle_wctx.fd = 0; + + // I'm assuming these two are reserved for standard input and output + // so I'm using them as sentinels with idle_wctx. + /* paranoid */ verify( this.idle_fd != 0 ); + /* paranoid */ verify( this.idle_fd != 1 ); + #if !defined(__CFA_NO_STATISTICS__) print_stats = 0; @@ -590,5 +597,5 @@ // Cluster static void ?{}(__cluster_proc_list & this) { - this.fd = 0; + this.fdw = 0p; this.idle = 0; this.total = 0; Index: libcfa/src/concurrency/mutex_stmt.hfa =================================================================== --- libcfa/src/concurrency/mutex_stmt.hfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/mutex_stmt.hfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -38,4 +38,17 @@ } + struct scoped_lock { + L * internal_lock; + }; + + static inline void ?{}( scoped_lock(L) & this, L & internal_lock ) { + this.internal_lock = &internal_lock; + lock(internal_lock); + } + + static inline void ^?{}( scoped_lock(L) & this ) with(this) { + unlock(*internal_lock); + } + static inline L * __get_ptr( L & this ) { return &this; Index: libcfa/src/concurrency/preemption.cfa =================================================================== --- libcfa/src/concurrency/preemption.cfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/preemption.cfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -251,7 +251,18 @@ bool enabled = __cfaabi_tls.preemption_state.enabled; + // Check if there is a pending preemption + processor * proc = __cfaabi_tls.this_processor; + bool pending = proc ? proc->pending_preemption : false; + if( enabled && pending ) proc->pending_preemption = false; + // create a assembler label after // marked as clobber all to avoid movement __cfaasm_label(check, after); + + // If we can preempt and there is a pending one + // this is a good time to yield + if( enabled && pending ) { + force_yield( __POLL_PREEMPTION ); + } return enabled; } @@ -282,4 +293,6 @@ // marked as clobber all to avoid movement __cfaasm_label(get, after); + + // This is used everywhere, to avoid cost, we DO NOT poll pending preemption return val; } @@ -358,5 +371,5 @@ if(!ready) { abort("Preemption should be ready"); } - __cfaasm_label(debug, before); + // __cfaasm_label(debug, before); sigset_t oldset; @@ -377,5 +390,5 @@ if(ret == 1) { abort("ERROR SIGTERM is disabled"); } - __cfaasm_label(debug, after); + // __cfaasm_label(debug, after); } @@ -548,5 +561,5 @@ __cfaasm_label( check ); __cfaasm_label( dsable ); - __cfaasm_label( debug ); + // __cfaasm_label( debug ); // Check if preemption is safe @@ -555,5 +568,5 @@ if( __cfaasm_in( ip, check ) ) { ready = false; goto EXIT; }; if( __cfaasm_in( ip, dsable ) ) { ready = false; goto EXIT; }; - if( __cfaasm_in( ip, debug ) ) { ready = false; goto EXIT; }; + // if( __cfaasm_in( ip, debug ) ) { ready = false; goto EXIT; }; if( !__cfaabi_tls.preemption_state.enabled) { ready = false; goto EXIT; }; if( __cfaabi_tls.preemption_state.in_progress ) { ready = false; goto EXIT; }; @@ -661,5 +674,10 @@ // Check if it is safe to preempt here - if( !preemption_ready( ip ) ) { return; } + if( !preemption_ready( ip ) ) { + #if !defined(__CFA_NO_STATISTICS__) + __cfaabi_tls.this_stats->ready.threads.preempt.rllfwd++; + #endif + return; + } __cfaabi_dbg_print_buffer_decl( " KERNEL: preempting core %p (%p @ %p).\n", __cfaabi_tls.this_processor, __cfaabi_tls.this_thread, (void *)(cxt->uc_mcontext.CFA_REG_IP) ); @@ -680,4 +698,8 @@ // Preemption can occur here + + #if !defined(__CFA_NO_STATISTICS__) + __cfaabi_tls.this_stats->ready.threads.preempt.yield++; + #endif force_yield( __ALARM_PREEMPTION ); // Do the actual __cfactx_switch Index: libcfa/src/concurrency/stats.cfa =================================================================== --- libcfa/src/concurrency/stats.cfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/stats.cfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -29,7 +29,11 @@ stats->ready.threads.threads = 0; stats->ready.threads.cthreads = 0; + stats->ready.threads.preempt.yield = 0; + stats->ready.threads.preempt.rllfwd = 0; stats->ready.sleep.halts = 0; stats->ready.sleep.cancels = 0; + stats->ready.sleep.early = 0; stats->ready.sleep.wakes = 0; + stats->ready.sleep.seen = 0; stats->ready.sleep.exits = 0; @@ -43,4 +47,8 @@ stats->io.submit.slow = 0; stats->io.flush.external = 0; + stats->io.flush.dirty = 0; + stats->io.flush.full = 0; + stats->io.flush.idle = 0; + stats->io.flush.eager = 0; stats->io.calls.flush = 0; stats->io.calls.submitted = 0; @@ -71,26 +79,30 @@ void __tally_stats( struct __stats_t * cltr, struct __stats_t * proc ) { - tally_one( &cltr->ready.push.local.attempt, &proc->ready.push.local.attempt ); - tally_one( &cltr->ready.push.local.success, &proc->ready.push.local.success ); - tally_one( &cltr->ready.push.share.attempt, &proc->ready.push.share.attempt ); - tally_one( &cltr->ready.push.share.success, &proc->ready.push.share.success ); - tally_one( &cltr->ready.push.extrn.attempt, &proc->ready.push.extrn.attempt ); - tally_one( &cltr->ready.push.extrn.success, &proc->ready.push.extrn.success ); - tally_one( &cltr->ready.pop.local .attempt, &proc->ready.pop.local .attempt ); - tally_one( &cltr->ready.pop.local .success, &proc->ready.pop.local .success ); - tally_one( &cltr->ready.pop.help .attempt, &proc->ready.pop.help .attempt ); - tally_one( &cltr->ready.pop.help .success, &proc->ready.pop.help .success ); - tally_one( &cltr->ready.pop.steal .attempt, &proc->ready.pop.steal .attempt ); - tally_one( &cltr->ready.pop.steal .success, &proc->ready.pop.steal .success ); - tally_one( &cltr->ready.pop.search.attempt, &proc->ready.pop.search.attempt ); - tally_one( &cltr->ready.pop.search.success, &proc->ready.pop.search.success ); - tally_one( &cltr->ready.threads.migration , &proc->ready.threads.migration ); - tally_one( &cltr->ready.threads.extunpark , &proc->ready.threads.extunpark ); - tally_one( &cltr->ready.threads.threads , &proc->ready.threads.threads ); - tally_one( &cltr->ready.threads.cthreads , &proc->ready.threads.cthreads ); - tally_one( &cltr->ready.sleep.halts , &proc->ready.sleep.halts ); - tally_one( &cltr->ready.sleep.cancels , &proc->ready.sleep.cancels ); - tally_one( &cltr->ready.sleep.wakes , &proc->ready.sleep.wakes ); - tally_one( &cltr->ready.sleep.exits , &proc->ready.sleep.exits ); + tally_one( &cltr->ready.push.local.attempt , &proc->ready.push.local.attempt ); + tally_one( &cltr->ready.push.local.success , &proc->ready.push.local.success ); + tally_one( &cltr->ready.push.share.attempt , &proc->ready.push.share.attempt ); + tally_one( &cltr->ready.push.share.success , &proc->ready.push.share.success ); + tally_one( &cltr->ready.push.extrn.attempt , &proc->ready.push.extrn.attempt ); + tally_one( &cltr->ready.push.extrn.success , &proc->ready.push.extrn.success ); + tally_one( &cltr->ready.pop.local .attempt , &proc->ready.pop.local .attempt ); + tally_one( &cltr->ready.pop.local .success , &proc->ready.pop.local .success ); + tally_one( &cltr->ready.pop.help .attempt , &proc->ready.pop.help .attempt ); + tally_one( &cltr->ready.pop.help .success , &proc->ready.pop.help .success ); + tally_one( &cltr->ready.pop.steal .attempt , &proc->ready.pop.steal .attempt ); + tally_one( &cltr->ready.pop.steal .success , &proc->ready.pop.steal .success ); + tally_one( &cltr->ready.pop.search.attempt , &proc->ready.pop.search.attempt ); + tally_one( &cltr->ready.pop.search.success , &proc->ready.pop.search.success ); + tally_one( &cltr->ready.threads.migration , &proc->ready.threads.migration ); + tally_one( &cltr->ready.threads.extunpark , &proc->ready.threads.extunpark ); + tally_one( &cltr->ready.threads.threads , &proc->ready.threads.threads ); + tally_one( &cltr->ready.threads.cthreads , &proc->ready.threads.cthreads ); + tally_one( &cltr->ready.threads.preempt.yield , &proc->ready.threads.preempt.yield ); + tally_one( &cltr->ready.threads.preempt.rllfwd, &proc->ready.threads.preempt.rllfwd ); + tally_one( &cltr->ready.sleep.halts , &proc->ready.sleep.halts ); + tally_one( &cltr->ready.sleep.cancels , &proc->ready.sleep.cancels ); + tally_one( &cltr->ready.sleep.early , &proc->ready.sleep.early ); + tally_one( &cltr->ready.sleep.wakes , &proc->ready.sleep.wakes ); + tally_one( &cltr->ready.sleep.seen , &proc->ready.sleep.wakes ); + tally_one( &cltr->ready.sleep.exits , &proc->ready.sleep.exits ); #if defined(CFA_HAVE_LINUX_IO_URING_H) @@ -103,4 +115,8 @@ tally_one( &cltr->io.submit.slow , &proc->io.submit.slow ); tally_one( &cltr->io.flush.external , &proc->io.flush.external ); + tally_one( &cltr->io.flush.dirty , &proc->io.flush.dirty ); + tally_one( &cltr->io.flush.full , &proc->io.flush.full ); + tally_one( &cltr->io.flush.idle , &proc->io.flush.idle ); + tally_one( &cltr->io.flush.eager , &proc->io.flush.eager ); tally_one( &cltr->io.calls.flush , &proc->io.calls.flush ); tally_one( &cltr->io.calls.submitted , &proc->io.calls.submitted ); @@ -153,5 +169,8 @@ | " (" | eng3(ready.pop.search.attempt) | " try)"; - sstr | "- Idle Slp : " | eng3(ready.sleep.halts) | "halt," | eng3(ready.sleep.cancels) | "cancel," | eng3(ready.sleep.wakes) | "wake," | eng3(ready.sleep.exits) | "exit"; + sstr | "- Idle Slp : " | eng3(ready.sleep.halts) | "halt," | eng3(ready.sleep.cancels) | "cancel," + | eng3(ready.sleep.wakes + ready.sleep.early) | '(' | eng3(ready.sleep.early) | ',' | eng3(ready.sleep.seen) | ')' | " wake(early, seen)," + | eng3(ready.sleep.exits) | "exit"; + sstr | "- Preemption : " | eng3(ready.threads.preempt.yield) | "yields," | eng3(ready.threads.preempt.rllfwd) | "delayed"; sstr | nl; } @@ -178,13 +197,14 @@ if(io.alloc.fail || io.alloc.revoke || io.alloc.block) sstr | "- failures : " | eng3(io.alloc.fail) | "oom, " | eng3(io.alloc.revoke) | "rvk, " | eng3(io.alloc.block) | "blk"; - if(io.flush.external) - sstr | "- flush external : " | eng3(io.flush.external); + // if(io.flush.external) + // sstr | "- flush external : " | eng3(io.flush.external); double avgsubs = ((double)io.calls.submitted) / io.calls.flush; double avgcomp = ((double)io.calls.completed) / io.calls.drain; sstr | "- syscll : " - | " sub " | eng3(io.calls.flush) | "/" | eng3(io.calls.submitted) | "(" | ws(3, 3, avgsubs) | "/flush)" - | " - cmp " | eng3(io.calls.drain) | "/" | eng3(io.calls.completed) | "(" | ws(3, 3, avgcomp) | "/drain)" + | " sub " | eng3(io.calls.submitted) | "/" | eng3(io.calls.flush) | "(" | ws(3, 3, avgsubs) | "/flush)" + | " - cmp " | eng3(io.calls.completed) | "/" | eng3(io.calls.drain) | "(" | ws(3, 3, avgcomp) | "/drain)" | " - " | eng3(io.calls.errors.busy) | " EBUSY"; + sstr | " - sub: " | eng3(io.flush.full) | "full, " | eng3(io.flush.dirty) | "drty, " | eng3(io.flush.idle) | "idle, " | eng3(io.flush.eager) | "eagr, " | eng3(io.flush.external) | "ext"; sstr | "- ops blk: " | " sk rd: " | eng3(io.ops.sockread) | "epll: " | eng3(io.ops.epllread) Index: libcfa/src/concurrency/stats.hfa =================================================================== --- libcfa/src/concurrency/stats.hfa (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/concurrency/stats.hfa (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -65,9 +65,15 @@ volatile int64_t threads; // number of threads in the system, includes only local change volatile int64_t cthreads; // number of threads in the system, includes only local change + struct { + volatile uint64_t yield; + volatile uint64_t rllfwd; + } preempt; } threads; struct { volatile uint64_t halts; volatile uint64_t cancels; + volatile uint64_t early; volatile uint64_t wakes; + volatile uint64_t seen; volatile uint64_t exits; } sleep; @@ -89,4 +95,8 @@ struct { volatile uint64_t external; + volatile uint64_t dirty; + volatile uint64_t full; + volatile uint64_t idle; + volatile uint64_t eager; } flush; struct { Index: libcfa/src/stdhdr/pthread.h =================================================================== --- libcfa/src/stdhdr/pthread.h (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/stdhdr/pthread.h (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -10,15 +10,34 @@ // Created On : Wed Jun 16 13:39:06 2021 // Last Modified By : Peter A. Buhr -// Last Modified On : Wed Jun 16 13:39:42 2021 -// Update Count : 1 +// Last Modified On : Thu Feb 3 21:53:26 2022 +// Update Count : 13 // +// pthread.h and setjmp.h cannot agree on the type of __sigsetjmp: +// +// extern int __sigsetjmp (struct __jmp_buf_tag *__env, int __savemask) __attribute__ ((__nothrow__)); +// extern int __sigsetjmp (struct __jmp_buf_tag __env[1], int __savemask) __attribute__ ((__nothrow__)); +// +// With -Wall, gcc-11 warns about the disagreement unless the CPP directive +// +// # 1 "/usr/include/pthread.h" 1 3 4 +// +// appears, which appears to be witchcraft. Unfortunately, this directive is removed by the CFA preprocessor, so the +// batchtest fails because of the spurious warning message. Hence, the warning is elided. + extern "C" { +#if defined(__GNUC__) && __GNUC__ == 11 + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Warray-parameter" +#endif // defined(__GNUC__) && __GNUC__ == 11 + #include_next // has internal check for multiple expansion + +#if defined(__GNUC__) && __GNUC__ == 11 + #pragma GCC diagnostic pop +#endif // defined(__GNUC__) && __GNUC__ == 11 } // extern "C" // Local Variables: // -// tab-width: 4 // // mode: c++ // -// compile-command: "make install" // // End: // Index: libcfa/src/stdhdr/setjmp.h =================================================================== --- libcfa/src/stdhdr/setjmp.h (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ libcfa/src/stdhdr/setjmp.h (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -10,15 +10,34 @@ // Created On : Mon Jul 4 23:25:26 2016 // Last Modified By : Peter A. Buhr -// Last Modified On : Tue Jul 5 20:38:33 2016 -// Update Count : 12 +// Last Modified On : Thu Feb 3 21:53:28 2022 +// Update Count : 18 // +// pthread.h and setjmp.h cannot agree on the type of __sigsetjmp: +// +// extern int __sigsetjmp (struct __jmp_buf_tag *__env, int __savemask) __attribute__ ((__nothrow__)); +// extern int __sigsetjmp (struct __jmp_buf_tag __env[1], int __savemask) __attribute__ ((__nothrow__)); +// +// With -Wall, gcc-11 warns about the disagreement unless the CPP directive +// +// # 1 "/usr/include/pthread.h" 1 3 4 +// +// appears, which appears to be witchcraft. Unfortunately, this directive is removed by the CFA preprocessor, so the +// batchtest fails because of the spurious warning message. Hence, the warning is elided. + extern "C" { +#if defined(__GNUC__) && __GNUC__ == 11 + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Warray-parameter" +#endif // defined(__GNUC__) && __GNUC__ == 11 + #include_next // has internal check for multiple expansion + +#if defined(__GNUC__) && __GNUC__ == 11 + #pragma GCC diagnostic pop +#endif // defined(__GNUC__) && __GNUC__ == 11 } // extern "C" // Local Variables: // -// tab-width: 4 // // mode: c++ // -// compile-command: "make install" // // End: // Index: src/AST/Convert.cpp =================================================================== --- src/AST/Convert.cpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Convert.cpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -9,7 +9,7 @@ // Author : Thierry Delisle // Created On : Thu May 09 15::37::05 2019 -// Last Modified By : Andrew Beach -// Last Modified On : Wed Jul 14 16:15:00 2021 -// Update Count : 37 +// Last Modified By : Peter A. Buhr +// Last Modified On : Wed Feb 2 13:19:22 2022 +// Update Count : 41 // @@ -393,6 +393,6 @@ auto stmt = new IfStmt( get().accept1( node->cond ), - get().accept1( node->thenPart ), - get().accept1( node->elsePart ), + get().accept1( node->then ), + get().accept1( node->else_ ), get().acceptL( node->inits ) ); @@ -419,10 +419,11 @@ } - const ast::Stmt * visit( const ast::WhileStmt * node ) override final { + const ast::Stmt * visit( const ast::WhileDoStmt * node ) override final { if ( inCache( node ) ) return nullptr; auto inits = get().acceptL( node->inits ); - auto stmt = new WhileStmt( + auto stmt = new WhileDoStmt( get().accept1( node->cond ), get().accept1( node->body ), + get().accept1( node->else_ ), inits, node->isDoWhile @@ -437,5 +438,6 @@ get().accept1( node->cond ), get().accept1( node->inc ), - get().accept1( node->body ) + get().accept1( node->body ), + get().accept1( node->else_ ) ); return stmtPostamble( stmt, node ); @@ -1872,6 +1874,6 @@ old->location, GET_ACCEPT_1(condition, Expr), - GET_ACCEPT_1(thenPart, Stmt), - GET_ACCEPT_1(elsePart, Stmt), + GET_ACCEPT_1(then, Stmt), + GET_ACCEPT_1(else_, Stmt), GET_ACCEPT_V(initialization, Stmt), GET_LABELS_V(old->labels) @@ -1902,10 +1904,11 @@ } - virtual void visit( const WhileStmt * old ) override final { + virtual void visit( const WhileDoStmt * old ) override final { if ( inCache( old ) ) return; - this->node = new ast::WhileStmt( + this->node = new ast::WhileDoStmt( old->location, GET_ACCEPT_1(condition, Expr), GET_ACCEPT_1(body, Stmt), + GET_ACCEPT_1(else_, Stmt), GET_ACCEPT_V(initialization, Stmt), old->isDoWhile, @@ -1923,4 +1926,5 @@ GET_ACCEPT_1(increment, Expr), GET_ACCEPT_1(body, Stmt), + GET_ACCEPT_1(else_, Stmt), GET_LABELS_V(old->labels) ); Index: src/AST/Copy.hpp =================================================================== --- src/AST/Copy.hpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Copy.hpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -10,6 +10,6 @@ // Created On : Wed Jul 10 16:13:00 2019 // Last Modified By : Andrew Beach -// Last Modified On : Thr Nov 11 9:22:00 2021 -// Update Count : 2 +// Last Modified On : Wed Dec 15 11:07:00 2021 +// Update Count : 3 // @@ -52,4 +52,14 @@ Node * deepCopy( const Node * localRoot ); +template +node_t * shallowCopy( const ptr_base & localRoot ) { + return shallowCopy( localRoot.get() ); +} + +template +node_t * deepCopy( const ptr_base & localRoot ) { + return deepCopy( localRoot.get() ); +} + } Index: src/AST/Fwd.hpp =================================================================== --- src/AST/Fwd.hpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Fwd.hpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -10,6 +10,6 @@ // Created On : Wed May 8 16:05:00 2019 // Last Modified By : Peter A. Buhr -// Last Modified On : Fri Mar 12 18:37:39 2021 -// Update Count : 4 +// Last Modified On : Tue Feb 1 09:08:33 2022 +// Update Count : 5 // @@ -44,5 +44,5 @@ class DirectiveStmt; class IfStmt; -class WhileStmt; +class WhileDoStmt; class ForStmt; class SwitchStmt; Index: src/AST/Node.cpp =================================================================== --- src/AST/Node.cpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Node.cpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -10,6 +10,6 @@ // Created On : Thu May 16 14:16:00 2019 // Last Modified By : Peter A. Buhr -// Last Modified On : Fri Mar 12 18:25:06 2021 -// Update Count : 2 +// Last Modified On : Tue Feb 1 09:09:39 2022 +// Update Count : 3 // @@ -146,6 +146,6 @@ template class ast::ptr_base< ast::IfStmt, ast::Node::ref_type::weak >; template class ast::ptr_base< ast::IfStmt, ast::Node::ref_type::strong >; -template class ast::ptr_base< ast::WhileStmt, ast::Node::ref_type::weak >; -template class ast::ptr_base< ast::WhileStmt, ast::Node::ref_type::strong >; +template class ast::ptr_base< ast::WhileDoStmt, ast::Node::ref_type::weak >; +template class ast::ptr_base< ast::WhileDoStmt, ast::Node::ref_type::strong >; template class ast::ptr_base< ast::ForStmt, ast::Node::ref_type::weak >; template class ast::ptr_base< ast::ForStmt, ast::Node::ref_type::strong >; Index: src/AST/Node.hpp =================================================================== --- src/AST/Node.hpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Node.hpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -188,4 +188,9 @@ } + ptr_base & operator=( const node_t * node ) { + assign( node ); + return *this; + } + template ptr_base & operator=( const o_node_t * node ) { Index: src/AST/Pass.hpp =================================================================== --- src/AST/Pass.hpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Pass.hpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -146,5 +146,5 @@ const ast::Stmt * visit( const ast::DirectiveStmt * ) override final; const ast::Stmt * visit( const ast::IfStmt * ) override final; - const ast::Stmt * visit( const ast::WhileStmt * ) override final; + const ast::Stmt * visit( const ast::WhileDoStmt * ) override final; const ast::Stmt * visit( const ast::ForStmt * ) override final; const ast::Stmt * visit( const ast::SwitchStmt * ) override final; Index: src/AST/Pass.impl.hpp =================================================================== --- src/AST/Pass.impl.hpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Pass.impl.hpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -33,11 +33,4 @@ /* call the implementation of the previsit of this pass */ \ __pass::previsit( core, node, 0 ); - -#define VISIT( code... ) \ - /* if this node should visit its children */ \ - if ( __visit_children() ) { \ - /* visit the children */ \ - code \ - } #define VISIT_END( type, node ) \ @@ -452,5 +445,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { { guard_symtab guard { *this }; @@ -460,5 +453,5 @@ maybe_accept( node, &ObjectDecl::bitfieldWidth ); maybe_accept( node, &ObjectDecl::attributes ); - ) + } __pass::symtab::addId( core, 0, node ); @@ -475,5 +468,7 @@ __pass::symtab::addId( core, 0, node ); - VISIT(maybe_accept( node, &FunctionDecl::withExprs );) + if ( __visit_children() ) { + maybe_accept( node, &FunctionDecl::withExprs ); + } { // with clause introduces a level of scope (for the with expression members). @@ -493,5 +488,5 @@ } }; __pass::symtab::addId( core, 0, func ); - VISIT( + if ( __visit_children() ) { // parameter declarations maybe_accept( node, &FunctionDecl::params ); @@ -509,5 +504,5 @@ maybe_accept( node, &FunctionDecl::stmts ); maybe_accept( node, &FunctionDecl::attributes ); - ) + } } } @@ -526,10 +521,10 @@ __pass::symtab::addStructFwd( core, 0, node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { * this }; maybe_accept( node, &StructDecl::params ); maybe_accept( node, &StructDecl::members ); maybe_accept( node, &StructDecl::attributes ); - }) + } // this addition replaces the forward declaration @@ -548,10 +543,10 @@ __pass::symtab::addUnionFwd( core, 0, node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { * this }; maybe_accept( node, &UnionDecl::params ); maybe_accept( node, &UnionDecl::members ); maybe_accept( node, &UnionDecl::attributes ); - }) + } __pass::symtab::addUnion( core, 0, node ); @@ -568,10 +563,10 @@ __pass::symtab::addEnum( core, 0, node ); - VISIT( + if ( __visit_children() ) { // unlike structs, traits, and unions, enums inject their members into the global scope maybe_accept( node, &EnumDecl::params ); maybe_accept( node, &EnumDecl::members ); maybe_accept( node, &EnumDecl::attributes ); - ) + } VISIT_END( Decl, node ); @@ -584,10 +579,10 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &TraitDecl::params ); maybe_accept( node, &TraitDecl::members ); maybe_accept( node, &TraitDecl::attributes ); - }) + } __pass::symtab::addTrait( core, 0, node ); @@ -602,8 +597,8 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &TypeDecl::base ); - }) + } // see A NOTE ON THE ORDER OF TRAVERSAL, above @@ -612,5 +607,5 @@ __pass::symtab::addType( core, 0, node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &TypeDecl::assertions ); @@ -619,5 +614,5 @@ maybe_accept( node, &TypeDecl::init ); } - ) + } VISIT_END( Decl, node ); @@ -630,12 +625,14 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &TypedefDecl::base ); - }) + } __pass::symtab::addType( core, 0, node ); - VISIT( maybe_accept( node, &TypedefDecl::assertions ); ) + if ( __visit_children() ) { + maybe_accept( node, &TypedefDecl::assertions ); + } VISIT_END( Decl, node ); @@ -648,7 +645,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &AsmDecl::stmt ); - ) + } VISIT_END( AsmDecl, node ); @@ -661,7 +658,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &DirectiveDecl::stmt ); - ) + } VISIT_END( DirectiveDecl, node ); @@ -674,8 +671,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &StaticAssertDecl::cond ); maybe_accept( node, &StaticAssertDecl::msg ); - ) + } VISIT_END( StaticAssertDecl, node ); @@ -687,5 +684,6 @@ const ast::CompoundStmt * ast::Pass< core_t >::visit( const ast::CompoundStmt * node ) { VISIT_START( node ); - VISIT( + + if ( __visit_children() ) { // Do not enter (or leave) a new scope if atFunctionTop. Remember to save the result. auto guard1 = makeFuncGuard( [this, enterScope = !this->atFunctionTop]() { @@ -704,5 +702,6 @@ guard_scope guard3 { *this }; maybe_accept( node, &CompoundStmt::kids ); - ) + } + VISIT_END( CompoundStmt, node ); } @@ -714,7 +713,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ExprStmt::expr ); - ) + } VISIT_END( Stmt, node ); @@ -727,10 +726,10 @@ VISIT_START( node ) - VISIT( + if ( __visit_children() ) { maybe_accept( node, &AsmStmt::instruction ); maybe_accept( node, &AsmStmt::output ); maybe_accept( node, &AsmStmt::input ); maybe_accept( node, &AsmStmt::clobber ); - ) + } VISIT_END( Stmt, node ); @@ -752,12 +751,12 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { // if statements introduce a level of scope (for the initialization) guard_symtab guard { *this }; maybe_accept( node, &IfStmt::inits ); maybe_accept( node, &IfStmt::cond ); - maybe_accept_as_compound( node, &IfStmt::thenPart ); - maybe_accept_as_compound( node, &IfStmt::elsePart ); - }) + maybe_accept_as_compound( node, &IfStmt::then ); + maybe_accept_as_compound( node, &IfStmt::else_ ); + } VISIT_END( Stmt, node ); @@ -765,16 +764,16 @@ //-------------------------------------------------------------------------- -// WhileStmt -template< typename core_t > -const ast::Stmt * ast::Pass< core_t >::visit( const ast::WhileStmt * node ) { - VISIT_START( node ); - - VISIT({ +// WhileDoStmt +template< typename core_t > +const ast::Stmt * ast::Pass< core_t >::visit( const ast::WhileDoStmt * node ) { + VISIT_START( node ); + + if ( __visit_children() ) { // while statements introduce a level of scope (for the initialization) guard_symtab guard { *this }; - maybe_accept( node, &WhileStmt::inits ); - maybe_accept( node, &WhileStmt::cond ); - maybe_accept_as_compound( node, &WhileStmt::body ); - }) + maybe_accept( node, &WhileDoStmt::inits ); + maybe_accept( node, &WhileDoStmt::cond ); + maybe_accept_as_compound( node, &WhileDoStmt::body ); + } VISIT_END( Stmt, node ); @@ -787,5 +786,5 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { // for statements introduce a level of scope (for the initialization) guard_symtab guard { *this }; @@ -795,5 +794,5 @@ maybe_accept( node, &ForStmt::inc ); maybe_accept_as_compound( node, &ForStmt::body ); - }) + } VISIT_END( Stmt, node ); @@ -806,8 +805,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &SwitchStmt::cond ); maybe_accept( node, &SwitchStmt::stmts ); - ) + } VISIT_END( Stmt, node ); @@ -820,8 +819,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &CaseStmt::cond ); maybe_accept( node, &CaseStmt::stmts ); - ) + } VISIT_END( Stmt, node ); @@ -842,7 +841,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ReturnStmt::expr ); - ) + } VISIT_END( Stmt, node ); @@ -855,8 +854,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ThrowStmt::expr ); maybe_accept( node, &ThrowStmt::target ); - ) + } VISIT_END( Stmt, node ); @@ -869,9 +868,9 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &TryStmt::body ); maybe_accept( node, &TryStmt::handlers ); maybe_accept( node, &TryStmt::finally ); - ) + } VISIT_END( Stmt, node ); @@ -884,5 +883,5 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { // catch statements introduce a level of scope (for the caught exception) guard_symtab guard { *this }; @@ -890,5 +889,5 @@ maybe_accept( node, &CatchStmt::cond ); maybe_accept_as_compound( node, &CatchStmt::body ); - }) + } VISIT_END( Stmt, node ); @@ -901,7 +900,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &FinallyStmt::body ); - ) + } VISIT_END( Stmt, node ); @@ -914,7 +913,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &SuspendStmt::then ); - ) + } VISIT_END( Stmt, node ); @@ -934,5 +933,5 @@ // } - VISIT({ + if ( __visit_children() ) { std::vector new_clauses; new_clauses.reserve( node->clauses.size() ); @@ -965,5 +964,5 @@ node = n; } - }) + } #define maybe_accept(field) \ @@ -977,5 +976,5 @@ } - VISIT( + if ( __visit_children() ) { maybe_accept( timeout.time ); maybe_accept( timeout.stmt ); @@ -983,5 +982,5 @@ maybe_accept( orElse.stmt ); maybe_accept( orElse.cond ); - ) + } #undef maybe_accept @@ -996,5 +995,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &WithStmt::exprs ); { @@ -1004,5 +1003,6 @@ maybe_accept( node, &WithStmt::stmt ); } - ) + } + VISIT_END( Stmt, node ); } @@ -1022,7 +1022,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &DeclStmt::decl ); - ) + } VISIT_END( Stmt, node ); @@ -1037,7 +1037,7 @@ // For now this isn't visited, it is unclear if this causes problem // if all tests are known to pass, remove this code - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ImplicitCtorDtorStmt::callStmt ); - ) + } VISIT_END( Stmt, node ); @@ -1050,10 +1050,10 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { // mutex statements introduce a level of scope (for the initialization) guard_symtab guard { *this }; maybe_accept( node, &MutexStmt::stmt ); maybe_accept( node, &MutexStmt::mutexObjs ); - }) + } VISIT_END( Stmt, node ); @@ -1066,5 +1066,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { { guard_symtab guard { *this }; @@ -1073,5 +1073,5 @@ maybe_accept( node, &ApplicationExpr::func ); maybe_accept( node, &ApplicationExpr::args ); - ) + } VISIT_END( Expr, node ); @@ -1084,5 +1084,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { { guard_symtab guard { *this }; @@ -1091,5 +1091,5 @@ maybe_accept( node, &UntypedExpr::args ); - ) + } VISIT_END( Expr, node ); @@ -1102,8 +1102,8 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &NameExpr::result ); - }) + } VISIT_END( Expr, node ); @@ -1116,10 +1116,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &CastExpr::result ); } maybe_accept( node, &CastExpr::arg ); - ) + } VISIT_END( Expr, node ); @@ -1132,10 +1133,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &KeywordCastExpr::result ); } maybe_accept( node, &KeywordCastExpr::arg ); - ) + } VISIT_END( Expr, node ); @@ -1148,10 +1150,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &VirtualCastExpr::result ); } maybe_accept( node, &VirtualCastExpr::arg ); - ) + } VISIT_END( Expr, node ); @@ -1164,10 +1167,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &AddressExpr::result ); } maybe_accept( node, &AddressExpr::arg ); - ) + } VISIT_END( Expr, node ); @@ -1180,8 +1184,8 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &LabelAddressExpr::result ); - }) + } VISIT_END( Expr, node ); @@ -1194,5 +1198,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &UntypedMemberExpr::result ); @@ -1200,5 +1205,5 @@ maybe_accept( node, &UntypedMemberExpr::aggregate ); maybe_accept( node, &UntypedMemberExpr::member ); - ) + } VISIT_END( Expr, node ); @@ -1211,10 +1216,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &MemberExpr::result ); } maybe_accept( node, &MemberExpr::aggregate ); - ) + } VISIT_END( Expr, node ); @@ -1227,8 +1233,8 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &VariableExpr::result ); - }) + } VISIT_END( Expr, node ); @@ -1241,8 +1247,8 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &ConstantExpr::result ); - }) + } VISIT_END( Expr, node ); @@ -1255,5 +1261,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &SizeofExpr::result ); @@ -1264,5 +1271,5 @@ maybe_accept( node, &SizeofExpr::expr ); } - ) + } VISIT_END( Expr, node ); @@ -1275,5 +1282,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &AlignofExpr::result ); @@ -1284,5 +1292,5 @@ maybe_accept( node, &AlignofExpr::expr ); } - ) + } VISIT_END( Expr, node ); @@ -1295,10 +1303,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &UntypedOffsetofExpr::result ); } maybe_accept( node, &UntypedOffsetofExpr::type ); - ) + } VISIT_END( Expr, node ); @@ -1311,10 +1320,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &OffsetofExpr::result ); } maybe_accept( node, &OffsetofExpr::type ); - ) + } VISIT_END( Expr, node ); @@ -1327,10 +1337,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &OffsetPackExpr::result ); } maybe_accept( node, &OffsetPackExpr::type ); - ) + } VISIT_END( Expr, node ); @@ -1343,5 +1354,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &LogicalExpr::result ); @@ -1349,5 +1361,5 @@ maybe_accept( node, &LogicalExpr::arg1 ); maybe_accept( node, &LogicalExpr::arg2 ); - ) + } VISIT_END( Expr, node ); @@ -1360,5 +1372,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &ConditionalExpr::result ); @@ -1367,5 +1380,5 @@ maybe_accept( node, &ConditionalExpr::arg2 ); maybe_accept( node, &ConditionalExpr::arg3 ); - ) + } VISIT_END( Expr, node ); @@ -1378,5 +1391,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &CommaExpr::result ); @@ -1384,5 +1398,5 @@ maybe_accept( node, &CommaExpr::arg1 ); maybe_accept( node, &CommaExpr::arg2 ); - ) + } VISIT_END( Expr, node ); @@ -1395,10 +1409,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &TypeExpr::result ); } maybe_accept( node, &TypeExpr::type ); - ) + } VISIT_END( Expr, node ); @@ -1411,5 +1426,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &AsmExpr::result ); @@ -1417,5 +1433,5 @@ maybe_accept( node, &AsmExpr::constraint ); maybe_accept( node, &AsmExpr::operand ); - ) + } VISIT_END( Expr, node ); @@ -1428,10 +1444,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &ImplicitCopyCtorExpr::result ); } maybe_accept( node, &ImplicitCopyCtorExpr::callExpr ); - ) + } VISIT_END( Expr, node ); @@ -1444,10 +1461,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &ConstructorExpr::result ); } maybe_accept( node, &ConstructorExpr::callExpr ); - ) + } VISIT_END( Expr, node ); @@ -1460,10 +1478,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &CompoundLiteralExpr::result ); } maybe_accept( node, &CompoundLiteralExpr::init ); - ) + } VISIT_END( Expr, node ); @@ -1476,5 +1495,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &RangeExpr::result ); @@ -1482,5 +1502,5 @@ maybe_accept( node, &RangeExpr::low ); maybe_accept( node, &RangeExpr::high ); - ) + } VISIT_END( Expr, node ); @@ -1493,10 +1513,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &UntypedTupleExpr::result ); } maybe_accept( node, &UntypedTupleExpr::exprs ); - ) + } VISIT_END( Expr, node ); @@ -1509,10 +1530,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &TupleExpr::result ); } maybe_accept( node, &TupleExpr::exprs ); - ) + } VISIT_END( Expr, node ); @@ -1525,10 +1547,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &TupleIndexExpr::result ); } maybe_accept( node, &TupleIndexExpr::tuple ); - ) + } VISIT_END( Expr, node ); @@ -1541,10 +1564,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &TupleAssignExpr::result ); } maybe_accept( node, &TupleAssignExpr::stmtExpr ); - ) + } VISIT_END( Expr, node ); @@ -1557,5 +1581,6 @@ VISIT_START( node ); - VISIT(// don't want statements from outer CompoundStmts to be added to this StmtExpr + if ( __visit_children() ) { + // don't want statements from outer CompoundStmts to be added to this StmtExpr // get the stmts that will need to be spliced in auto stmts_before = __pass::stmtsToAddBefore( core, 0); @@ -1574,5 +1599,5 @@ maybe_accept( node, &StmtExpr::returnDecls ); maybe_accept( node, &StmtExpr::dtors ); - ) + } VISIT_END( Expr, node ); @@ -1585,10 +1610,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &UniqueExpr::result ); } maybe_accept( node, &UniqueExpr::expr ); - ) + } VISIT_END( Expr, node ); @@ -1601,5 +1627,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &UntypedInitExpr::result ); @@ -1607,5 +1634,5 @@ maybe_accept( node, &UntypedInitExpr::expr ); // not currently visiting initAlts, but this doesn't matter since this node is only used in the resolver. - ) + } VISIT_END( Expr, node ); @@ -1618,5 +1645,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &InitExpr::result ); @@ -1624,5 +1652,5 @@ maybe_accept( node, &InitExpr::expr ); maybe_accept( node, &InitExpr::designation ); - ) + } VISIT_END( Expr, node ); @@ -1635,5 +1663,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &DeletedExpr::result ); @@ -1641,5 +1670,5 @@ maybe_accept( node, &DeletedExpr::expr ); // don't visit deleteStmt, because it is a pointer to somewhere else in the tree. - ) + } VISIT_END( Expr, node ); @@ -1652,10 +1681,11 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &DefaultArgExpr::result ); } maybe_accept( node, &DefaultArgExpr::expr ); - ) + } VISIT_END( Expr, node ); @@ -1668,5 +1698,6 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { + { guard_symtab guard { *this }; maybe_accept( node, &GenericExpr::result ); @@ -1697,5 +1728,5 @@ node = n; } - ) + } VISIT_END( Expr, node ); @@ -1726,8 +1757,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { // xxx - should PointerType visit/mutate dimension? maybe_accept( node, &PointerType::base ); - ) + } VISIT_END( Type, node ); @@ -1740,8 +1771,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ArrayType::dimension ); maybe_accept( node, &ArrayType::base ); - ) + } VISIT_END( Type, node ); @@ -1754,7 +1785,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ReferenceType::base ); - ) + } VISIT_END( Type, node ); @@ -1767,8 +1798,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &QualifiedType::parent ); maybe_accept( node, &QualifiedType::child ); - ) + } VISIT_END( Type, node ); @@ -1781,5 +1812,5 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { // guard_forall_subs forall_guard { *this, node }; // mutate_forall( node ); @@ -1787,5 +1818,5 @@ maybe_accept( node, &FunctionType::returns ); maybe_accept( node, &FunctionType::params ); - }) + } VISIT_END( Type, node ); @@ -1800,8 +1831,8 @@ __pass::symtab::addStruct( core, 0, node->name ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &StructInstType::params ); - }) + } VISIT_END( Type, node ); @@ -1816,8 +1847,8 @@ __pass::symtab::addUnion( core, 0, node->name ); - VISIT({ + if ( __visit_children() ) { guard_symtab guard { *this }; maybe_accept( node, &UnionInstType::params ); - }) + } VISIT_END( Type, node ); @@ -1830,7 +1861,7 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { maybe_accept( node, &EnumInstType::params ); - }) + } VISIT_END( Type, node ); @@ -1843,7 +1874,7 @@ VISIT_START( node ); - VISIT({ + if ( __visit_children() ) { maybe_accept( node, &TraitInstType::params ); - }) + } VISIT_END( Type, node ); @@ -1856,5 +1887,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { { maybe_accept( node, &TypeInstType::params ); @@ -1862,5 +1893,5 @@ // ensure that base re-bound if doing substitution __pass::forall::replace( core, 0, node ); - ) + } VISIT_END( Type, node ); @@ -1873,8 +1904,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &TupleType::types ); maybe_accept( node, &TupleType::members ); - ) + } VISIT_END( Type, node ); @@ -1887,7 +1918,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &TypeofType::expr ); - ) + } VISIT_END( Type, node ); @@ -1900,7 +1931,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &VTableType::base ); - ) + } VISIT_END( Type, node ); @@ -1950,5 +1981,7 @@ VISIT_START( node ); - VISIT( maybe_accept( node, &Designation::designators ); ) + if ( __visit_children() ) { + maybe_accept( node, &Designation::designators ); + } VISIT_END( Designation, node ); @@ -1961,7 +1994,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &SingleInit::value ); - ) + } VISIT_END( Init, node ); @@ -1974,8 +2007,8 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ListInit::designations ); maybe_accept( node, &ListInit::initializers ); - ) + } VISIT_END( Init, node ); @@ -1988,9 +2021,9 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &ConstructorInit::ctor ); maybe_accept( node, &ConstructorInit::dtor ); maybe_accept( node, &ConstructorInit::init ); - ) + } VISIT_END( Init, node ); @@ -2003,7 +2036,7 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { maybe_accept( node, &Attribute::params ); - ) + } VISIT_END( Attribute, node ); @@ -2016,5 +2049,5 @@ VISIT_START( node ); - VISIT( + if ( __visit_children() ) { { bool mutated = false; @@ -2032,5 +2065,5 @@ } } - ) + } VISIT_END( TypeSubstitution, node ); @@ -2038,4 +2071,3 @@ #undef VISIT_START -#undef VISIT #undef VISIT_END Index: src/AST/Print.cpp =================================================================== --- src/AST/Print.cpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Print.cpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -511,12 +511,12 @@ ++indent; os << indent; - safe_print( node->thenPart ); - --indent; - - if ( node->elsePart != 0 ) { + safe_print( node->then ); + --indent; + + if ( node->else_ != 0 ) { os << indent << "... else:" << endl; ++indent; os << indent; - node->elsePart->accept( *this ); + node->else_->accept( *this ); --indent; } // if @@ -524,5 +524,5 @@ } - virtual const ast::Stmt * visit( const ast::WhileStmt * node ) override final { + virtual const ast::Stmt * visit( const ast::WhileDoStmt * node ) override final { if ( node->isDoWhile ) { os << "Do-"; } os << "While on condition:" << endl; Index: src/AST/Stmt.cpp =================================================================== --- src/AST/Stmt.cpp (revision 5f3ba117d89cd71b0b82121f2363d27a6752b378) +++ src/AST/Stmt.cpp (revision b56ad5e7e1abbbd650e2765fd6b0429712b6cfe6) @@ -9,7 +9,7 @@ // Author : Aaron B. Moss // Created On : Wed May 8 13:00:00 2019 -// Last Modified By : Andrew Beach -// Last Modified On : Wed May 15 15:53:00 2019 -// Update Count : 2 +// Last Modified By : Peter A. Buhr +// Last Modified On : Wed Feb 2 19:01:20 2022 +// Update Count : 3 // @@ -56,6 +56,6 @@ // --- BranchStmt -BranchStmt::BranchStmt( const CodeLocation& loc, Kind kind, Label target, std::vector