fix a few bugs revealed by ProGuard optimizations, including too-early constant propagation during array loads and stores

2025-01-03 19:54:13 +00:00 · 2008-06-10 08:49:13 -06:00 · 2008-06-10 08:49:13 -06:00 · fc8c5a2ea9
commit fc8c5a2ea9
parent 064ba4519a
4 changed files with 81 additions and 143 deletions
--- a/src/compile.cpp
+++ b/src/compile.cpp
@ -1834,67 +1834,34 @@ compile(MyThread* t, Frame* initialFrame, unsigned ip,
                       (&singletonValue(t, aioobThunk(t), 0)));
      }

-      if (c->isConstant(index)) {
-        unsigned i = c->constantValue(index);
-        switch (instruction) {
-        case aaload:
-          frame->pushObject
-            (c->load
-             (BytesPerWord, c->memory(array, ArrayBody + (i * BytesPerWord))));
-          break;
+      switch (instruction) {
+      case aaload:
+        frame->pushObject
+          (c->load
+           (BytesPerWord, c->memory(array, ArrayBody, index, BytesPerWord)));
+        break;

-        case faload:
-        case iaload:
-          frame->pushInt(c->load(4, c->memory(array, ArrayBody + (i * 4))));
-          break;
+      case faload:
+      case iaload:
+        frame->pushInt(c->load(4, c->memory(array, ArrayBody, index, 4)));
+        break;

-        case baload:
-          frame->pushInt(c->load(1, c->memory(array, ArrayBody + i)));
-          break;
+      case baload:
+        frame->pushInt(c->load(1, c->memory(array, ArrayBody, index, 1)));
+        break;

-        case caload:
-          frame->pushInt(c->loadz(2, c->memory(array, ArrayBody + (i * 2))));
-          break;
+      case caload:
+        frame->pushInt(c->loadz(2, c->memory(array, ArrayBody, index, 2)));
+        break;

-        case daload:
-        case laload:
-          frame->pushLong(c->load(8, c->memory(array, ArrayBody + (i * 8))));
-          break;
+      case daload:
+      case laload:
+        frame->pushLong(c->load(8, c->memory(array, ArrayBody, index, 8)));
+        break;

-        case saload:
-          frame->pushInt(c->load(2, c->memory(array, ArrayBody + (i * 2))));
-          break;
-        }        
-      } else {
-        switch (instruction) {
-        case aaload:
-          frame->pushObject
-            (c->load
-             (BytesPerWord, c->memory(array, ArrayBody, index, BytesPerWord)));
-          break;
-
-        case faload:
-        case iaload:
-          frame->pushInt(c->load(4, c->memory(array, ArrayBody, index, 4)));
-          break;
-
-        case baload:
-          frame->pushInt(c->load(1, c->memory(array, ArrayBody, index, 1)));
-          break;
-
-        case caload:
-          frame->pushInt(c->loadz(2, c->memory(array, ArrayBody, index, 2)));
-          break;
-
-        case daload:
-        case laload:
-          frame->pushLong(c->load(8, c->memory(array, ArrayBody, index, 8)));
-          break;
-
-        case saload:
-          frame->pushInt(c->load(2, c->memory(array, ArrayBody, index, 2)));
-          break;
-        }
+      case saload:
+        frame->pushInt(c->load(2, c->memory(array, ArrayBody, index, 2)));
+        break;
      }
    } break;

@ -1923,72 +1890,37 @@ compile(MyThread* t, Frame* initialFrame, unsigned ip,
                       (&singletonValue(t, aioobThunk(t), 0)));
      }

-      if (c->isConstant(index)) {
-        unsigned i = c->constantValue(index);
-        switch (instruction) {
-        case aastore: {
-          c->call
-            (c->constant(getThunk(t, setMaybeNullThunk)),
-             0,
-             frame->trace(0, false),
-             0,
-             4, c->thread(), array,
-             c->constant(ArrayBody + (i * BytesPerWord)),
-             value);
-        } break;
+      switch (instruction) {
+      case aastore: {
+        c->call
+          (c->constant(getThunk(t, setMaybeNullThunk)),
+           0,
+           frame->trace(0, false),
+           0,
+           4, c->thread(), array,
+           c->add(4, c->constant(ArrayBody),
+                  c->shl(4, c->constant(log(BytesPerWord)), index)),
+           value);
+      } break;

-        case fastore:
-        case iastore:
-          c->store(4, value, c->memory(array, ArrayBody + (i * 4)));
-          break;
+      case fastore:
+      case iastore:
+        c->store(4, value, c->memory(array, ArrayBody, index, 4));
+        break;

-        case bastore:
-          c->store(1, value, c->memory(array, ArrayBody + i));
-          break;
+      case bastore:
+        c->store(1, value, c->memory(array, ArrayBody, index, 1));
+        break;

-        case castore:
-        case sastore:
-          c->store(2, value, c->memory(array, ArrayBody + (i * 2)));
-          break;
+      case castore:
+      case sastore:
+        c->store(2, value, c->memory(array, ArrayBody, index, 2));
+        break;

-        case dastore:
-        case lastore:
-          c->store(8, value, c->memory(array, ArrayBody + (i * 8)));
-          break;
-        }
-      } else  {
-        switch (instruction) {
-        case aastore: {
-          c->call
-            (c->constant(getThunk(t, setMaybeNullThunk)),
-             0,
-             frame->trace(0, false),
-             0,
-             4, c->thread(), array,
-             c->add(4, c->constant(ArrayBody),
-                    c->shl(4, c->constant(log(BytesPerWord)), index)),
-             value);
-        } break;
-
-        case fastore:
-        case iastore:
-          c->store(4, value, c->memory(array, ArrayBody, index, 4));
-          break;
-
-        case bastore:
-          c->store(1, value, c->memory(array, ArrayBody, index, 1));
-          break;
-
-        case castore:
-        case sastore:
-          c->store(2, value, c->memory(array, ArrayBody, index, 2));
-          break;
-
-        case dastore:
-        case lastore:
-          c->store(8, value, c->memory(array, ArrayBody, index, 8));
-          break;
-        }
+      case dastore:
+      case lastore:
+        c->store(8, value, c->memory(array, ArrayBody, index, 8));
+        break;
      }
    } break;

@ -2602,7 +2534,7 @@ compile(MyThread* t, Frame* initialFrame, unsigned ip,
        c->jle(target);
        break;
      }
-      
+
      compile(t, frame, newIp);
      if (UNLIKELY(t->exception)) return;
    } break;
--- a/src/compiler.cpp
+++ b/src/compiler.cpp
@ -1826,7 +1826,7 @@ class MemoryEvent: public Event {
    scale(scale), result(result)
  {
    addRead(c, base, BytesPerWord, anyRegisterSite(c));
-    if (index) addRead(c, index, BytesPerWord, anyRegisterSite(c));
+    if (index) addRead(c, index, BytesPerWord, registerOrConstantSite(c));
  }

  virtual void compile(Context* c) {
@ -1835,9 +1835,26 @@ class MemoryEvent: public Event {
    }
    
    int indexRegister;
+    int displacement = this->displacement;
+    unsigned scale = this->scale;
    if (index) {
-      assert(c, index->source->type(c) == RegisterOperand);
-      indexRegister = static_cast<RegisterSite*>(index->source)->register_.low;
+      ConstantSite* constant = 0;
+      for (Site* s = index->sites; s; s = s->next) {
+        if (s->type(c) == ConstantOperand) {
+          constant = static_cast<ConstantSite*>(s);
+          break;
+        }
+      }
+
+      if (constant) {
+        indexRegister = NoRegister;
+        displacement += (constant->value.value->value() * scale);
+        scale = 1;
+      } else {
+        assert(c, index->source->type(c) == RegisterOperand);
+        indexRegister = static_cast<RegisterSite*>
+          (index->source)->register_.low;
+      }
    } else {
      indexRegister = NoRegister;
    }
@ -1846,7 +1863,7 @@ class MemoryEvent: public Event {

    nextRead(c, base);
    if (index) {
-      if (BytesPerWord == 8) {
+      if (BytesPerWord == 8 and indexRegister != NoRegister) {
        apply(c, Move4To8, 8, index->source, index->source);
      }

@ -2641,22 +2658,6 @@ class MyCompiler: public Compiler {
    return value(&c, s, s);
  }

-  virtual bool isConstant(Operand* a) {
-    for (Site* s = static_cast<Value*>(a)->sites; s; s = s->next) {
-      if (s->type(&c) == ConstantOperand) return true;
-    }
-    return false;
-  }
-
-  virtual int64_t constantValue(Operand* a) {
-    for (Site* s = static_cast<Value*>(a)->sites; s; s = s->next) {
-      if (s->type(&c) == ConstantOperand) {
-        return static_cast<ConstantSite*>(s)->value.value->value();
-      }
-    }
-    abort(&c);
-  }
-
  virtual Operand* label() {
    return value(&c, ::constantSite(&c, static_cast<Promise*>(0)));
  }
@ -2698,7 +2699,8 @@ class MyCompiler: public Compiler {
      Value* v = value(&c);
      c.state->stack = ::stack(&c, v, 1, c.state->stack);
      c.state->stack->pushed = true;
-//       v->sites = pushSite(&c, c.state->stack->index);
+      c.state->stack->pushSite
+        = v->sites = pushSite(&c, c.state->stack->index);
    }
  }

--- a/src/compiler.h
+++ b/src/compiler.h
@ -62,9 +62,6 @@ class Compiler {
  virtual Operand* base() = 0;
  virtual Operand* thread() = 0;

-  virtual bool isConstant(Operand* value) = 0;
-  virtual int64_t constantValue(Operand* value) = 0;
-
  virtual Operand* label() = 0;
  virtual void mark(Operand* label) = 0;

--- a/test/Misc.java
+++ b/test/Misc.java
@ -95,6 +95,13 @@ public class Misc {
  }

  public static void main(String[] args) {
+    { boolean p = true;
+      int[] array = new int[] { 1, 2 };
+      expect(array[0] == array[p ? 0 : 1]);
+      p = false;
+      expect(array[1] == array[p ? 0 : 1]);
+    }
+
    expect(roundUp(156, 2) == 156);

    { Foo foo = new Foo();