Inlined HardwareSerial calls to RX ISR.

Moreover, declaring pointers-to-registers as const and using initializer
list in class constructor allows the compiler to further improve inlining
performance.

This change recovers about 50 bytes of program space on single-UART devices.

See #1711

7 files changed, 108 insertions, 98 deletions

diff --git a/cores/arduino/HardwareSerial.cpp b/cores/arduino/HardwareSerial.cpp
index d7f1fca..5d62273 100644
--- a/cores/arduino/HardwareSerial.cpp
+++ b/cores/arduino/HardwareSerial.cpp
@@ -26,64 +26,14 @@
 #include <string.h>
 #include <inttypes.h>
 #include "Arduino.h"
-#include "wiring_private.h"
 
 #include "HardwareSerial.h"
+#include "HardwareSerial_private.h"
 
 // this next line disables the entire HardwareSerial.cpp, 
 // this is so I can support Attiny series and any other chip without a uart
 #if defined(HAVE_HWSERIAL0) || defined(HAVE_HWSERIAL1) || defined(HAVE_HWSERIAL2) || defined(HAVE_HWSERIAL3)
 
-// Ensure that the various bit positions we use are available with a 0
-// postfix, so we can always use the values for UART0 for all UARTs. The
-// alternative, passing the various values for each UART to the
-// HardwareSerial constructor also works, but makes the code bigger and
-// slower.
-#if !defined(TXC0)
-#if defined(TXC)
-// On ATmega8, the uart and its bits are not numbered, so there is no TXC0 etc.
-#define TXC0 TXC
-#define RXEN0 RXEN
-#define TXEN0 TXEN
-#define RXCIE0 RXCIE
-#define UDRIE0 UDRIE
-#define U2X0 U2X
-#define UPE0 UPE
-#define UDRE0 UDRE
-#elif defined(TXC1)
-// Some devices have uart1 but no uart0
-#define TXC0 TXC1
-#define RXEN0 RXEN1
-#define TXEN0 TXEN1
-#define RXCIE0 RXCIE1
-#define UDRIE0 UDRIE1
-#define U2X0 U2X1
-#define UPE0 UPE1
-#define UDRE0 UDRE1
-#else
-#error No UART found in HardwareSerial.cpp
-#endif
-#endif // !defined TXC0
-
-// Check at compiletime that it is really ok to use the bit positions of
-// UART0 for the other UARTs as well, in case these values ever get
-// changed for future hardware.
-#if defined(TXC1) && (TXC1 != TXC0 || RXEN1 != RXEN0 || RXCIE1 != RXCIE0 || \
-		      UDRIE1 != UDRIE0 || U2X1 != U2X0 || UPE1 != UPE0 || \
-		      UDRE1 != UDRE0)
-#error "Not all bit positions for UART1 are the same as for UART0"
-#endif
-#if defined(TXC2) && (TXC2 != TXC0 || RXEN2 != RXEN0 || RXCIE2 != RXCIE0 || \
-		      UDRIE2 != UDRIE0 || U2X2 != U2X0 || UPE2 != UPE0 || \
-		      UDRE2 != UDRE0)
-#error "Not all bit positions for UART2 are the same as for UART0"
-#endif
-#if defined(TXC3) && (TXC3 != TXC0 || RXEN3 != RXEN0 || RXCIE3 != RXCIE0 || \
-		      UDRIE3 != UDRIE0 || U3X3 != U3X0 || UPE3 != UPE0 || \
-		      UDRE3 != UDRE0)
-#error "Not all bit positions for UART3 are the same as for UART0"
-#endif
-
 // SerialEvent functions are weak, so when the user doesn't define them,
 // the linker just sets their address to 0 (which is checked below).
 // The Serialx_available is just a wrapper around Serialx.available(),
@@ -127,28 +77,6 @@ void serialEventRun(void)
 
 // Actual interrupt handlers //////////////////////////////////////////////////////////////
 
-void HardwareSerial::_rx_complete_irq(void)
-{
-  if (bit_is_clear(*_ucsra, UPE0)) {
-    // No Parity error, read byte and store it in the buffer if there is
-    // room
-    unsigned char c = *_udr;
-    int i = (unsigned int)(_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE;
-
-    // if we should be storing the received character into the location
-    // just before the tail (meaning that the head would advance to the
-    // current location of the tail), we're about to overflow the buffer
-    // and so we don't write the character or advance the head.
-    if (i != _rx_buffer_tail) {
-      _rx_buffer[_rx_buffer_head] = c;
-      _rx_buffer_head = i;
-    }
-  } else {
-    // Parity error, read byte but discard it
-    unsigned char c = *_udr;
-  };
-}
-
 void HardwareSerial::_tx_udr_empty_irq(void)
 {
   // If interrupts are enabled, there must be more data in the output
@@ -169,23 +97,6 @@ void HardwareSerial::_tx_udr_empty_irq(void)
   }
 }
 
-// Constructors ////////////////////////////////////////////////////////////////
-
-HardwareSerial::HardwareSerial(
-  volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
-  volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
-  volatile uint8_t *ucsrc, volatile uint8_t *udr)
-{
-  _tx_buffer_head = _tx_buffer_tail = 0;
-  _rx_buffer_head = _rx_buffer_tail = 0;
-  _ubrrh = ubrrh;
-  _ubrrl = ubrrl;
-  _ucsra = ucsra;
-  _ucsrb = ucsrb;
-  _ucsrc = ucsrc;
-  _udr = udr;
-}
-
 // Public Methods //////////////////////////////////////////////////////////////
 
 void HardwareSerial::begin(unsigned long baud, byte config)
diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h
index da9efbc..bd3c4c4 100644
--- a/cores/arduino/HardwareSerial.h
+++ b/cores/arduino/HardwareSerial.h
@@ -66,12 +66,12 @@
 class HardwareSerial : public Stream
 {
   protected:
-    volatile uint8_t *_ubrrh;
-    volatile uint8_t *_ubrrl;
-    volatile uint8_t *_ucsra;
-    volatile uint8_t *_ucsrb;
-    volatile uint8_t *_ucsrc;
-    volatile uint8_t *_udr;
+    volatile uint8_t * const _ubrrh;
+    volatile uint8_t * const _ubrrl;
+    volatile uint8_t * const _ucsra;
+    volatile uint8_t * const _ucsrb;
+    volatile uint8_t * const _ucsrc;
+    volatile uint8_t * const _udr;
     // Has any byte been written to the UART since begin()
     bool _written;
 
@@ -87,7 +87,7 @@ class HardwareSerial : public Stream
     unsigned char _tx_buffer[SERIAL_BUFFER_SIZE];
 
   public:
-    HardwareSerial(
+    inline HardwareSerial(
       volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
       volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
       volatile uint8_t *ucsrc, volatile uint8_t *udr);
@@ -107,7 +107,7 @@ class HardwareSerial : public Stream
     operator bool() { return true; }
 
     // Interrupt handlers - Not intended to be called externally
-    void _rx_complete_irq(void);
+    inline void _rx_complete_irq(void);
     void _tx_udr_empty_irq(void);
 };
 
diff --git a/cores/arduino/HardwareSerial0.cpp b/cores/arduino/HardwareSerial0.cpp
index dd4d806..efbd527 100644
--- a/cores/arduino/HardwareSerial0.cpp
+++ b/cores/arduino/HardwareSerial0.cpp
@@ -1,5 +1,6 @@
 #include "Arduino.h"
 #include "HardwareSerial.h"
+#include "HardwareSerial_private.h"
 
 // Each HardwareSerial is defined in its own file, sine the linker pulls
 // in the entire file when any element inside is used. --gc-sections can
diff --git a/cores/arduino/HardwareSerial1.cpp b/cores/arduino/HardwareSerial1.cpp
index c04263e..089f35b 100644
--- a/cores/arduino/HardwareSerial1.cpp
+++ b/cores/arduino/HardwareSerial1.cpp
@@ -1,5 +1,6 @@
 #include "Arduino.h"
 #include "HardwareSerial.h"
+#include "HardwareSerial_private.h"
 
 // Each HardwareSerial is defined in its own file, sine the linker pulls
 // in the entire file when any element inside is used. --gc-sections can
diff --git a/cores/arduino/HardwareSerial2.cpp b/cores/arduino/HardwareSerial2.cpp
index 2c448f9..aa1397c 100644
--- a/cores/arduino/HardwareSerial2.cpp
+++ b/cores/arduino/HardwareSerial2.cpp
@@ -1,5 +1,6 @@
 #include "Arduino.h"
 #include "HardwareSerial.h"
+#include "HardwareSerial_private.h"
 
 // Each HardwareSerial is defined in its own file, sine the linker pulls
 // in the entire file when any element inside is used. --gc-sections can
diff --git a/cores/arduino/HardwareSerial3.cpp b/cores/arduino/HardwareSerial3.cpp
index 012839d..0140647 100644
--- a/cores/arduino/HardwareSerial3.cpp
+++ b/cores/arduino/HardwareSerial3.cpp
@@ -1,5 +1,6 @@
 #include "Arduino.h"
 #include "HardwareSerial.h"
+#include "HardwareSerial_private.h"
 
 // Each HardwareSerial is defined in its own file, sine the linker pulls
 // in the entire file when any element inside is used. --gc-sections can
diff --git a/cores/arduino/HardwareSerial_private.h b/cores/arduino/HardwareSerial_private.h
new file mode 100644
index 0000000..54a0c7a
--- /dev/null
+++ b/cores/arduino/HardwareSerial_private.h
@@ -0,0 +1,95 @@
+#include "wiring_private.h"
+
+// this next line disables the entire HardwareSerial.cpp, 
+// this is so I can support Attiny series and any other chip without a uart
+#if defined(HAVE_HWSERIAL0) || defined(HAVE_HWSERIAL1) || defined(HAVE_HWSERIAL2) || defined(HAVE_HWSERIAL3)
+
+// Ensure that the various bit positions we use are available with a 0
+// postfix, so we can always use the values for UART0 for all UARTs. The
+// alternative, passing the various values for each UART to the
+// HardwareSerial constructor also works, but makes the code bigger and
+// slower.
+#if !defined(TXC0)
+#if defined(TXC)
+// On ATmega8, the uart and its bits are not numbered, so there is no TXC0 etc.
+#define TXC0 TXC
+#define RXEN0 RXEN
+#define TXEN0 TXEN
+#define RXCIE0 RXCIE
+#define UDRIE0 UDRIE
+#define U2X0 U2X
+#define UPE0 UPE
+#define UDRE0 UDRE
+#elif defined(TXC1)
+// Some devices have uart1 but no uart0
+#define TXC0 TXC1
+#define RXEN0 RXEN1
+#define TXEN0 TXEN1
+#define RXCIE0 RXCIE1
+#define UDRIE0 UDRIE1
+#define U2X0 U2X1
+#define UPE0 UPE1
+#define UDRE0 UDRE1
+#else
+#error No UART found in HardwareSerial.cpp
+#endif
+#endif // !defined TXC0
+
+// Check at compiletime that it is really ok to use the bit positions of
+// UART0 for the other UARTs as well, in case these values ever get
+// changed for future hardware.
+#if defined(TXC1) && (TXC1 != TXC0 || RXEN1 != RXEN0 || RXCIE1 != RXCIE0 || \
+		      UDRIE1 != UDRIE0 || U2X1 != U2X0 || UPE1 != UPE0 || \
+		      UDRE1 != UDRE0)
+#error "Not all bit positions for UART1 are the same as for UART0"
+#endif
+#if defined(TXC2) && (TXC2 != TXC0 || RXEN2 != RXEN0 || RXCIE2 != RXCIE0 || \
+		      UDRIE2 != UDRIE0 || U2X2 != U2X0 || UPE2 != UPE0 || \
+		      UDRE2 != UDRE0)
+#error "Not all bit positions for UART2 are the same as for UART0"
+#endif
+#if defined(TXC3) && (TXC3 != TXC0 || RXEN3 != RXEN0 || RXCIE3 != RXCIE0 || \
+		      UDRIE3 != UDRIE0 || U3X3 != U3X0 || UPE3 != UPE0 || \
+		      UDRE3 != UDRE0)
+#error "Not all bit positions for UART3 are the same as for UART0"
+#endif
+
+// Constructors ////////////////////////////////////////////////////////////////
+
+HardwareSerial::HardwareSerial(
+  volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
+  volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
+  volatile uint8_t *ucsrc, volatile uint8_t *udr) :
+    _ubrrh(ubrrh), _ubrrl(ubrrl),
+    _ucsra(ucsra), _ucsrb(ucsrb), _ucsrc(ucsrc),
+    _udr(udr),
+    _tx_buffer_head(0), _tx_buffer_tail(0),
+    _rx_buffer_head(0), _rx_buffer_tail(0)
+{
+}
+
+// Actual interrupt handlers //////////////////////////////////////////////////////////////
+
+void HardwareSerial::_rx_complete_irq(void)
+{
+  if (bit_is_clear(*_ucsra, UPE0)) {
+    // No Parity error, read byte and store it in the buffer if there is
+    // room
+    unsigned char c = *_udr;
+    int i = (unsigned int)(_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE;
+
+    // if we should be storing the received character into the location
+    // just before the tail (meaning that the head would advance to the
+    // current location of the tail), we're about to overflow the buffer
+    // and so we don't write the character or advance the head.
+    if (i != _rx_buffer_tail) {
+      _rx_buffer[_rx_buffer_head] = c;
+      _rx_buffer_head = i;
+    }
+  } else {
+    // Parity error, read byte but discard it
+    unsigned char c = *_udr;
+  };
+}
+
+#endif // whole file