When constructing branches during IRBuilder add some simple direct eq…

…uality checks to eliminate the branch instr. If we know we are comparing true to true then we also know we can just perform a jump. This has an added big potential benefit that it should eliminate the next BB from the CFG entirely since it will no longer be reachable. If two directly comparable values are never equal then the branch is replaced with a noop. This will probably not help at all once compiled down to bytecode since the JVM will figure out we have a bogus conditional but it makes me feel better (and it was driving me crazy to see 'while(true)' generate 'b_false(true)').
jruby · Dec 18, 2015 · c95f224 · c95f224
1 parent 1c0c1fd
commit c95f224
Showing 1 changed file with 37 additions and 4 deletions.
diff --git a/core/src/main/java/org/jruby/ir/instructions/BEQInstr.java b/core/src/main/java/org/jruby/ir/instructions/BEQInstr.java
@@ -12,12 +12,45 @@
 import org.jruby.runtime.builtin.IRubyObject;
 
 public class BEQInstr extends TwoOperandBranchInstr implements FixedArityInstr {
-    public static BranchInstr create(Operand v1, Operand v2, Label jmpTarget) {
+    public static Instr create(Operand v1, Operand v2, Label jmpTarget) {
         if (v2 instanceof Boolean) {
-            return ((Boolean) v2).isTrue() ? new BTrueInstr(jmpTarget, v1) : new BFalseInstr(jmpTarget, v1);
+            Boolean lhs = (Boolean) v2;
+
+            if (lhs.isTrue()) {
+                if (v1 instanceof Boolean) {
+                    if (((Boolean) v1).isTrue()) { // true == true -> just jump
+                        return new JumpInstr(jmpTarget);
+                    } else {                       // false == true (this will never jump)
+                        return NopInstr.NOP;
+                    }
+                } else {
+                    return new BTrueInstr(jmpTarget, v1);
+                }
+            } else if (lhs.isFalse()) {
+                if (v1 instanceof Boolean) {
+                    if (((Boolean) v1).isFalse()) { // false == false -> just jump
+                        return new JumpInstr(jmpTarget);
+                    } else {                        // true == false (this will never jump)
+                        return NopInstr.NOP;
+                    }
+                } else {
+                    return new BFalseInstr(jmpTarget, v1);
+                }
+            }
+        } else if (v2 instanceof Nil) {
+            if (v1 instanceof Nil) { // nil == nil -> just jump
+                return new JumpInstr(jmpTarget);
+            } else {
+                return new BNilInstr(jmpTarget, v1);
+            }
+        }
+        if (v2 == UndefinedValue.UNDEFINED) {
+            if (v1 == UndefinedValue.UNDEFINED) {
+                return new JumpInstr(jmpTarget);
+            } else {
+                return new BUndefInstr(jmpTarget, v1);
+            }
         }
-        if (v2 instanceof Nil) return new BNilInstr(jmpTarget, v1);
-        if (v2 == UndefinedValue.UNDEFINED) return new BUndefInstr(jmpTarget, v1);
 
         return new BEQInstr(jmpTarget, v1, v2);
     }