Skip to content

Commit 3fb1461

Browse files
dmillerdmiller
committed
CLJCLR-176: Fix code gen for case exression in sparse mode.
1 parent 612bc0c commit 3fb1461

File tree

2 files changed

+149
-36
lines changed

2 files changed

+149
-36
lines changed

‎Clojure/Clojure.Tests/clojure/test_clojure/control.clj‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,18 @@
358358
:else))
359359
:small 1
360360
:big 1073741824
361-
:else 2))
361+
:else 2)
362+
;; DM: Added for CLJCLR-176 -- testing sparse mode
363+
(testing "CLJCLR-176: Sparse mode for case"
364+
(are [result input] (= result (case input
365+
3.0 :double-result
366+
true :true-result
367+
false :false-result
368+
:else))
369+
:double-result 3.0
370+
:true-result true
371+
:false-result false
372+
:else 2)))
362373
(testing "test emits return types"
363374
(should-not-reflect (clojure.lang.BigDecimal/Create (case 1 1 1))) ;;; (Long. (case 1 1 1)) ; new Long(long)
364375
(should-not-reflect (clojure.lang.BigDecimal/Create (case 1 1 "1")))) ;;; (Long. (case 1 1 "1")) ; new Long(String)

‎Clojure/Clojure/CljCompiler/Ast/CaseExpr.cs‎

Lines changed: 137 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class CaseExpr : Expr, MaybePrimitiveExpr
3232
public int High { get { return _high; } }
3333

3434
readonly Expr _defaultExpr;
35-
public Expr DefaultExpr { get { return _defaultExpr; } }
35+
public Expr DefaultExpr { get { return _defaultExpr; } }
3636

3737
readonly SortedDictionary<int, Expr> _tests;
3838
public SortedDictionary<int, Expr> Tests { get { return _tests; } }
@@ -69,8 +69,8 @@ public class CaseExpr : Expr, MaybePrimitiveExpr
6969

7070
#region C-tors
7171

72-
public CaseExpr( IPersistentMap sourceSpan, LocalBindingExpr expr, int shift, int mask, int low, int high, Expr defaultExpr,
73-
SortedDictionary<int, Expr> tests, Dictionary<int, Expr> thens, Keyword switchType, Keyword testType, IPersistentSet skipCheck )
72+
public CaseExpr(IPersistentMap sourceSpan, LocalBindingExpr expr, int shift, int mask, int low, int high, Expr defaultExpr,
73+
SortedDictionary<int, Expr> tests, Dictionary<int, Expr> thens, Keyword switchType, Keyword testType, IPersistentSet skipCheck)
7474
{
7575
_sourceSpan = sourceSpan;
7676
_expr = expr;
@@ -96,10 +96,10 @@ public CaseExpr( IPersistentMap sourceSpan, LocalBindingExpr expr, int shift, in
9696
if (RT.count(skipCheck) > 0 && RT.booleanCast(RT.WarnOnReflectionVar.deref()))
9797
{
9898
RT.errPrintWriter().WriteLine("Performance warning, {0}:{1}:{2} - hash collision of some case test constants; if selected, those entries will be tested sequentially.",
99-
Compiler.SourcePathVar.deref(),Compiler.GetLineFromSpanMap(sourceSpan),Compiler.GetColumnFromSpanMap(sourceSpan));
99+
Compiler.SourcePathVar.deref(), Compiler.GetLineFromSpanMap(sourceSpan), Compiler.GetColumnFromSpanMap(sourceSpan));
100100
RT.errPrintWriter().Flush();
101101
}
102-
102+
103103
}
104104

105105
#endregion
@@ -117,7 +117,7 @@ public Type ClrType
117117
}
118118

119119
#endregion
120-
120+
121121
#region Parsing
122122

123123
public sealed class Parser : IParser
@@ -150,8 +150,8 @@ public Expr Parse(ParserContext pcon, object frm)
150150
LocalBindingExpr testexpr = (LocalBindingExpr)Compiler.Analyze(pcon.SetRhc(RHC.Expression), exprForm);
151151

152152

153-
SortedDictionary<int, Expr> tests = new SortedDictionary<int, Expr>();
154-
Dictionary<int, Expr> thens = new Dictionary<int, Expr>();
153+
SortedDictionary<int, Expr> tests = new();
154+
Dictionary<int, Expr> thens = new();
155155

156156
foreach (IMapEntry me in caseMap)
157157
{
@@ -186,7 +186,7 @@ public Expr Parse(ParserContext pcon, object frm)
186186
skipCheck);
187187
}
188188
}
189-
189+
190190
#endregion
191191

192192
#region eval
@@ -201,7 +201,7 @@ public object Eval()
201201
#region Code generation
202202

203203
// Equivalent to :
204-
// switch (hashed _expr)
204+
// switch (hash _expr)
205205
//
206206
// case i: if _expr == _test_i
207207
// goto end with _then_i
@@ -226,9 +226,9 @@ public void DoEmit(RHC rhc, ObjExpr objx, CljILGen ilg, bool emitUnboxed)
226226
Label defaultLabel = ilg.DefineLabel();
227227
Label endLabel = ilg.DefineLabel();
228228

229-
SortedDictionary<int, Label> labels = new SortedDictionary<int, Label>();
229+
SortedDictionary<int, Label> thenLabels = new();
230230
foreach (int i in _tests.Keys)
231-
labels[i] = ilg.DefineLabel();
231+
thenLabels[i] = ilg.DefineLabel();
232232

233233
Type primExprType = Compiler.MaybePrimitiveType(_expr);
234234

@@ -239,41 +239,143 @@ public void DoEmit(RHC rhc, ObjExpr objx, CljILGen ilg, bool emitUnboxed)
239239

240240
if (_switchType == _sparseKey)
241241
{
242-
Label[] la = labels.Values.ToArray<Label>();
243-
ilg.Emit(OpCodes.Switch, la);
244-
ilg.Emit(OpCodes.Br, defaultLabel);
242+
Label[] testLabels = new Label[thenLabels.Count];
243+
for (int i = 0; i < thenLabels.Count; i++)
244+
testLabels[i] = ilg.DefineLabel();
245+
246+
// The hash value to test is sitting on top of the stack.
247+
// We need to store it so that each test generated by EmitSparseCaseTests can load it.
248+
249+
LocalBuilder hashLoc = ilg.DeclareLocal(typeof(int));
250+
GenContext.SetLocalName(hashLoc, "test");
251+
ilg.Emit(OpCodes.Stloc, hashLoc);
252+
253+
EmitSparseCaseTests(
254+
rhc, objx, ilg,
255+
hashLoc,
256+
0, thenLabels.Count - 1,
257+
_tests.Keys.ToArray<int>(),
258+
testLabels,
259+
thenLabels.Values.ToArray<Label>(),
260+
defaultLabel);
245261
}
246262
else
247263
{
248264
Label[] la = new Label[(_high - _low) + 1];
249265
for (int i = _low; i <= _high; i++)
250-
la[i - _low] = labels.ContainsKey(i) ? labels[i] : defaultLabel;
266+
la[i - _low] = thenLabels.ContainsKey(i) ? thenLabels[i] : defaultLabel;
251267
ilg.EmitInt(_low);
252268
ilg.Emit(OpCodes.Sub);
253269
ilg.Emit(OpCodes.Switch, la);
254270
ilg.Emit(OpCodes.Br, defaultLabel);
255-
}
271+
}
256272

257-
foreach (int i in labels.Keys)
273+
foreach (int i in thenLabels.Keys)
258274
{
259-
ilg.MarkLabel(labels[i]);
275+
ilg.MarkLabel(thenLabels[i]);
276+
260277
if (_testType == _intKey)
261278
EmitThenForInts(objx, ilg, primExprType, _tests[i], _thens[i], defaultLabel, emitUnboxed);
262279
else if ((bool)RT.contains(_skipCheck, i))
263280
EmitExpr(objx, ilg, _thens[i], emitUnboxed);
264281
else
265282
EmitThenForHashes(objx, ilg, _tests[i], _thens[i], defaultLabel, emitUnboxed);
266-
if ( _thens[i].HasNormalExit() )
283+
if (_thens[i].HasNormalExit())
267284
ilg.Emit(OpCodes.Br, endLabel);
268285
}
269286
ilg.MarkLabel(defaultLabel);
287+
270288
EmitExpr(objx, ilg, _defaultExpr, emitUnboxed);
271289
ilg.MarkLabel(endLabel);
272290
if (rhc == RHC.Statement)
273291
ilg.Emit(OpCodes.Pop);
274292
}
275293

276294

295+
private int ComputeMidIndex(int leftIndex, int rightIndex) => leftIndex + (rightIndex - leftIndex) / 2;
296+
297+
private void EmitSparseCaseTests(
298+
RHC rhc,
299+
ObjExpr objx,
300+
CljILGen ilg,
301+
LocalBuilder hashLoc,
302+
int leftIndex,
303+
int rightIndex,
304+
int[] thenValues,
305+
Label[] testLabels,
306+
Label[] thenLabels,
307+
Label defaultLabel)
308+
{
309+
if (leftIndex > rightIndex)
310+
{
311+
// We should not get here.
312+
ilg.Emit(OpCodes.Br, defaultLabel);
313+
return;
314+
}
315+
316+
int midIndex = ComputeMidIndex(leftIndex, rightIndex);
317+
318+
ilg.MarkLabel(testLabels[midIndex]);
319+
320+
// Test against the current key
321+
ilg.Emit(OpCodes.Ldloc, hashLoc); // load the hash value of the test expression
322+
ilg.EmitInt(thenValues[midIndex]);
323+
ilg.Emit(OpCodes.Beq, thenLabels[midIndex]);
324+
325+
// Determine if we need to branch left or right
326+
if (leftIndex == rightIndex)
327+
{
328+
// No tests below us
329+
ilg.Emit(OpCodes.Br, defaultLabel);
330+
return;
331+
}
332+
333+
int newRight = midIndex - 1;
334+
int newLeft = midIndex + 1;
335+
336+
bool leftExists = newRight >= leftIndex;
337+
bool rightExists = newLeft <= rightIndex;
338+
339+
if (leftExists && rightExists)
340+
{
341+
// Test again
342+
ilg.Emit(OpCodes.Ldloc, hashLoc); // load the hash value of the test expression
343+
ilg.EmitInt(thenValues[midIndex]);
344+
345+
ilg.Emit(OpCodes.Bgt, testLabels[ComputeMidIndex(newLeft, rightIndex)]); // branch right
346+
ilg.Emit(OpCodes.Br, testLabels[ComputeMidIndex(leftIndex, newRight)]); // branch left
347+
348+
EmitSparseCaseTests(rhc, objx, ilg, hashLoc, leftIndex, newRight, thenValues, testLabels, thenLabels, defaultLabel);
349+
EmitSparseCaseTests(rhc, objx, ilg, hashLoc, newLeft, rightIndex, thenValues, testLabels, thenLabels, defaultLabel);
350+
}
351+
else if (leftExists)
352+
{
353+
// No branch to right
354+
ilg.Emit(OpCodes.Ldloc, hashLoc); // load the hash value of the test expression
355+
ilg.EmitInt(thenValues[midIndex]);
356+
ilg.Emit(OpCodes.Blt, testLabels[ComputeMidIndex(leftIndex, newRight)]); // branch left
357+
ilg.Emit(OpCodes.Br, defaultLabel); // no right, so default
358+
359+
EmitSparseCaseTests(rhc, objx, ilg, hashLoc, leftIndex, newRight, thenValues, testLabels, thenLabels, defaultLabel);
360+
}
361+
else if (rightExists)
362+
{
363+
// No branch to left
364+
ilg.Emit(OpCodes.Ldloc, hashLoc); // load the hash value of the test expression
365+
ilg.EmitInt(thenValues[midIndex]);
366+
ilg.Emit(OpCodes.Bgt, testLabels[ComputeMidIndex(newLeft, rightIndex)]); // branch right
367+
ilg.Emit(OpCodes.Br, defaultLabel); // no left, so default
368+
369+
EmitSparseCaseTests(rhc, objx, ilg, hashLoc, newLeft, rightIndex, thenValues, testLabels, thenLabels, defaultLabel);
370+
}
371+
else
372+
{
373+
// no branches at all
374+
ilg.Emit(OpCodes.Br, defaultLabel); // no left, so default
375+
376+
}
377+
}
378+
277379
bool IsShiftMasked { get { return _mask != 0; } }
278380

279381
void EmitShiftMask(CljILGen ilg)
@@ -291,17 +393,17 @@ private void EmitExprForInts(ObjExpr objx, CljILGen ilg, Type exprType, Label de
291393
{
292394
if (exprType == null)
293395
{
294-
if ( RT.booleanCast(RT.WarnOnReflectionVar.deref()))
396+
if (RT.booleanCast(RT.WarnOnReflectionVar.deref()))
295397
{
296398
RT.errPrintWriter().WriteLine("Performance warning, {0}:{1}:{2} - case has int tests, but tested expression is not primitive.",
297-
Compiler.SourcePathVar.deref(),Compiler.GetLineFromSpanMap(_sourceSpan),Compiler.GetColumnFromSpanMap(_sourceSpan));
399+
Compiler.SourcePathVar.deref(), Compiler.GetLineFromSpanMap(_sourceSpan), Compiler.GetColumnFromSpanMap(_sourceSpan));
298400
RT.errPrintWriter().Flush();
299401
}
300-
_expr.Emit(RHC.Expression,objx,ilg);
301-
ilg.Emit(OpCodes.Call,Compiler.Method_Util_IsNonCharNumeric);
302-
ilg.Emit(OpCodes.Brfalse,defaultLabel);
303-
_expr.Emit(RHC.Expression,objx,ilg);
304-
ilg.Emit(OpCodes.Call,Compiler.Method_Util_ConvertToInt);
402+
_expr.Emit(RHC.Expression, objx, ilg);
403+
ilg.Emit(OpCodes.Call, Compiler.Method_Util_IsNonCharNumeric);
404+
ilg.Emit(OpCodes.Brfalse, defaultLabel);
405+
_expr.Emit(RHC.Expression, objx, ilg);
406+
ilg.Emit(OpCodes.Call, Compiler.Method_Util_ConvertToInt);
305407
EmitShiftMask(ilg);
306408
}
307409
else if (exprType == typeof(long)
@@ -313,14 +415,14 @@ private void EmitExprForInts(ObjExpr objx, CljILGen ilg, Type exprType, Label de
313415
|| exprType == typeof(ushort)
314416
|| exprType == typeof(sbyte))
315417
{
316-
_expr.EmitUnboxed(RHC.Expression,objx,ilg);
418+
_expr.EmitUnboxed(RHC.Expression, objx, ilg);
317419
ilg.Emit(OpCodes.Conv_I4);
318420
EmitShiftMask(ilg);
319421

320422
}
321423
else
322424
{
323-
ilg.Emit(OpCodes.Br,defaultLabel);
425+
ilg.Emit(OpCodes.Br, defaultLabel);
324426
}
325427
}
326428

@@ -332,16 +434,16 @@ private void EmitThenForInts(ObjExpr objx, CljILGen ilg, Type exprType, Expr tes
332434
test.Emit(RHC.Expression, objx, ilg);
333435
ilg.Emit(OpCodes.Call, Compiler.Method_Util_equiv);
334436
ilg.Emit(OpCodes.Brfalse, defaultLabel);
335-
EmitExpr(objx, ilg, then, emitUnboxed);
437+
EmitExpr(objx, ilg, then, emitUnboxed);
336438
}
337439
else if (exprType == typeof(long))
338440
{
339441
((NumberExpr)test).EmitUnboxed(RHC.Expression, objx, ilg);
340442
_expr.EmitUnboxed(RHC.Expression, objx, ilg);
341443
ilg.Emit(OpCodes.Ceq);
342444
ilg.Emit(OpCodes.Brfalse, defaultLabel);
343-
EmitExpr(objx, ilg, then, emitUnboxed);
344-
445+
EmitExpr(objx, ilg, then, emitUnboxed);
446+
345447
}
346448
else if (exprType == typeof(int)
347449
|| exprType == typeof(short)
@@ -361,7 +463,7 @@ private void EmitThenForInts(ObjExpr objx, CljILGen ilg, Type exprType, Expr tes
361463
EmitExpr(objx, ilg, then, emitUnboxed);
362464
}
363465
// else direct match
364-
EmitExpr(objx, ilg, then, emitUnboxed);
466+
EmitExpr(objx, ilg, then, emitUnboxed);
365467
}
366468
else
367469
{
@@ -390,7 +492,7 @@ void EmitThenForHashes(ObjExpr objx, CljILGen ilg, Expr test, Expr then, Label d
390492
ilg.Emit(OpCodes.Call, Compiler.Method_Util_equiv);
391493
ilg.Emit(OpCodes.Brfalse, defaultLabel);
392494
}
393-
EmitExpr(objx, ilg, then, emitUnboxed);
495+
EmitExpr(objx, ilg, then, emitUnboxed);
394496
}
395497

396498
private static void EmitExpr(ObjExpr objx, CljILGen ilg, Expr expr, bool emitUnboxed)
@@ -401,7 +503,7 @@ private static void EmitExpr(ObjExpr objx, CljILGen ilg, Expr expr, bool emitUnb
401503
expr.Emit(RHC.Expression, objx, ilg);
402504
}
403505

404-
public bool HasNormalExit()
506+
public bool HasNormalExit()
405507
{
406508
if (_defaultExpr.HasNormalExit())
407509
return true;

0 commit comments

Comments
 (0)