From 99b2d2671467c5f1ed86263e1f7cdeeb5fcce822 Mon Sep 17 00:00:00 2001
From: Sebastian Freundt <freundt@ga-group.nl>
Date: Tue, 18 Jun 2019 10:53:00 +0000
Subject: [PATCH 1/2] proposal for #3647

Calculate `skip` condition after and separate from key counting.
---
 src/forder.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/forder.c b/src/forder.c
index c507aacc2a..28171209a1 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -902,8 +902,13 @@ void radix_r(const int from, const int to, const int radix) {
       // Always ascending sort here (even when sortType==-1) because descending was prepared in write_key
       my_counts[my_key[0]] = 1;
       int i=1;
-      while (skip && i<my_n) { my_counts[my_key[i]]++; skip=(my_key[i]>=my_key[i-1]); i++; }
       while (i<my_n)         { my_counts[my_key[i]]++; i++; }  // as soon as not-ordered is detected (likely quickly when it isn't sorted), save the >= comparison
+      for (i=1; i<my_n; i++) {
+        if (my_key[i]<my_key[i-1]) {
+          skip=false;
+          break;
+        }
+      }
       TEND(11)
     } else {
       for (int i=0; i<my_n; i++) {

From b4e0981fbdcb8e63c49696e2131a62d5451af17e Mon Sep 17 00:00:00 2001
From: mattdowle <mattjdowle@gmail.com>
Date: Wed, 19 Jun 2019 01:04:16 -0700
Subject: [PATCH 2/2] tweak to fix, and news item

---
 NEWS.md      |  3 ++-
 src/forder.c | 13 +++++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 63a1a259bf..3e46498067 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -129,7 +129,6 @@
     # TRUE
     ```
 
-
 #### BUG FIXES
 
 1. `first`, `last`, `head` and `tail` by group no longer error in some cases, [#2030](https://github.com/Rdatatable/data.table/issues/2030) [#3462](https://github.com/Rdatatable/data.table/issues/3462). Thanks to @franknarf1 for reporting.
@@ -189,6 +188,8 @@
     # 2:     2      a
     ```
 
+23. Incorrect sorting/grouping results due to a bug in Intel's `icc` compiler 2019 (Version 19.0.4.243 Build 20190416) has been worked around thanks to a report and fix by Sebastian Freundt, [#3647](https://github.com/Rdatatable/data.table/issues/3647). Please run `data.table::test.data.table()`. If that passes, your installation does not have the problem.
+
 #### NOTES
 
 1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
diff --git a/src/forder.c b/src/forder.c
index 28171209a1..6135260986 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -900,14 +900,11 @@ void radix_r(const int from, const int to, const int radix) {
     TEND(10)
     if (sortType!=0) {
       // Always ascending sort here (even when sortType==-1) because descending was prepared in write_key
-      my_counts[my_key[0]] = 1;
-      int i=1;
-      while (i<my_n)         { my_counts[my_key[i]]++; i++; }  // as soon as not-ordered is detected (likely quickly when it isn't sorted), save the >= comparison
-      for (i=1; i<my_n; i++) {
-        if (my_key[i]<my_key[i-1]) {
-          skip=false;
-          break;
-        }
+      for (int i=0; i<my_n; ++i) my_counts[my_key[i]]++;  // minimal branch-free loop first, #3647
+      for (int i=1; i<my_n; ++i) {
+        if (my_key[i]<my_key[i-1]) { skip=false; break; }
+        // stop early as soon as not-ordered is detected; likely quickly when it isn't sorted
+        // otherwise, it's worth checking if it is ordered because skip saves time later
       }
       TEND(11)
     } else {