[llvm-commits] r61980 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/bitcast-gep.ll

[Duncan Sands]

Hi Chris, I just remembered that it is undefined in C as to whether
the result of % is positive or negative for a negative lhs.  I guess
that explains the comment!  However it also means that this code is
wrong:

+    FirstIdx = Offset/TySize;
+    Offset %= TySize;
+    
+    // Handle silly modulus not returning values values [0..TySize).
+    if (Offset < 0) {
+      --FirstIdx;
+      Offset += TySize;
+      assert(Offset >= 0);
+    }
+    assert((uint64_t)Offset < (uint64_t)TySize && "Out of range offset");

Suppose for example that TySize = 4 and Offset = -1.  Then FirstIdx needs
to be set to -1.  In the above code it will only be set to -1 if
Offset % TySize returns a negative quantity, and apparently it is undefined
as to whether it will be or not (what a crazy language C is!).  How about
this instead:

  FirstIdx = Offset/TySize;
  if (Offset < 0)
    --FirstIdx;
  Offset -= FirstIdx * TySize;

Of course, if C had a division that rounds towards -infinity (like gcc's
FLOOR_DIV_EXPR) then you could just use that (what a crazy language C is!).

Ciao,

Duncan.