[erlang-bugs] Possible bug in HiPE pattern matching

[Mikael Pettersson]

Jon Meredith writes:
 > Hi all,
 > 
 > I think I've discovered an issue with pattern matching under HiPE  
 > going back on at least 5.6.3 under Debian/lenny and 5.7.2 under OS X
 > 
 > Here is a module that demonstrates the issue
 > 
 > -module(bug).
 > -export([f/1,g/1]).
 > f(0) -> zero;
 > f(-10) -> one;
 > f(-20) -> two;
 > f(-30) -> three;
 > f(-40) -> four;
 > f(-50) -> five;
 > f(-60) -> six;
 > f(-70) -> seven;
 > f(-80) -> eight;  %% Uncomment this line to make it work with HiPE
 > f(X) -> {error, X}.
 > 
 > g(X) ->
 >      case X of
 >          0 -> zero;
 >          -10 -> one;
 >          -20 -> two;
 >          -30 -> three;
 >          -40 -> four;
 >          -50 -> five;
 >          -60 -> six;
 >          -70 -> seven;
 >          -80 -> eight;  %% Uncomment this line to make it work with HiPE
 >          X -> {error, X}
 >      end.
 > 
 > Running normally without HiPE it does what I expect
 > 
 >    jmeredith@REDACTED:~$ /usr/bin/erlc bug.erl
 >    jmeredith@REDACTED:~$ /usr/bin/erl
 >    Erlang (BEAM) emulator version 5.6.3 [source] [64-bit] [smp:2]  
 > [async-threads:0] [hipe] [kernel-poll:false]
 > 
 >    Eshell V5.6.3  (abort with ^G)
 >    1> bug:f(0).
 >    zero
 >    2> bug:g(0).
 >    zero
 > 
 > If I compile with +native, the pattern doesn't match the first case as  
 > it should
 > 
 >    jmeredith@REDACTED:~$ /usr/bin/erlc +native bug.erl
 >    jmeredith@REDACTED:~$ /usr/bin/erl
 >    Erlang (BEAM) emulator version 5.6.3 [source] [64-bit] [smp:2]  
 > [async-threads:0] [hipe] [kernel-poll:false]
 > 
 >    Eshell V5.6.3  (abort with ^G)
 >    1> bug:f(0).
 >    {error,0}
 >    2> bug:g(0).
 >    {error,0}

This turned out to be a long-standing bug that only triggers for
largish but sparse cases on negative small integers, where the
generated search code did not account for key table transformations
done at load-time (to facilitate binary searches on atoms).

The fix is included below. Thanks for reporting the problem and
for providing such a good (small) test case.

/Mikael

--- otp-0710/lib/hipe/rtl/hipe_rtl_mk_switch.erl.~1~	2009-03-18 21:56:34.000000000 +0100
+++ otp-0710/lib/hipe/rtl/hipe_rtl_mk_switch.erl	2009-07-10 13:49:11.000000000 +0200
@@ -880,7 +880,7 @@ tab(KeyList, LabelList, KeyReg, TablePnt
   [
    hipe_rtl:mk_move(IndexReg,hipe_rtl:mk_imm(0)),
    hipe_rtl:mk_load(Temp,TablePntrReg,hipe_rtl:mk_imm(Init)),
-   hipe_rtl:mk_branch(Temp, ge, KeyReg,
+   hipe_rtl:mk_branch(Temp, geu, KeyReg,
 		      hipe_rtl:label_name(Lab2), 
 		      hipe_rtl:label_name(Lab1), 0.5),
    Lab1,
@@ -911,7 +911,7 @@ step(I,TablePntrReg,IndexReg,KeyReg) ->
   Lab2 = hipe_rtl:mk_new_label(),
   [hipe_rtl:mk_alu(TempIndex, IndexReg, add, hipe_rtl:mk_imm(I)),
    hipe_rtl:mk_load(Temp,TablePntrReg,TempIndex),
-   hipe_rtl:mk_branch(Temp, gt, KeyReg,
+   hipe_rtl:mk_branch(Temp, gtu, KeyReg,
                       hipe_rtl:label_name(Lab2), 
                       hipe_rtl:label_name(Lab1) , 0.5),
    Lab1] ++