[PATCH] D18438: Calculate __builtin_object_size when pointer depends on a condition

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

Thanks for fixing the offset bug! Just a few rather small comments, and I think we're good.


================
Comment at: lib/Analysis/MemoryBuiltins.cpp:390
@@ -391,1 +389,3 @@
 
+APInt ObjectSizeOffsetVisitor::getSizeWithOverflow(SizeOffsetType Data) {
+  if (Data.second.slt(0) || Data.first.ult(Data.second))
----------------
Is there a reason this method needs to exist on the visitor? Either way, please take `SizeOffsetType` by const&.

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:391
@@ +390,3 @@
+APInt ObjectSizeOffsetVisitor::getSizeWithOverflow(SizeOffsetType Data) {
+  if (Data.second.slt(0) || Data.first.ult(Data.second))
+    return APInt(Data.first.getBitWidth(), 0);
----------------
Nit: `Data.second.isNegative()`

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:579
@@ +578,3 @@
+    if (Mode == ObjSizeMode::Min) {
+      if (TrueResult.slt(FalseResult)) {
+        return TrueSide;
----------------
Nit: The braces serve no purpose, and I don't think they were here before. Please remove them. :)

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:585
@@ +584,3 @@
+    if (Mode == ObjSizeMode::Max) {
+      if (TrueResult.sgt(FalseResult)) {
+        return TrueSide;
----------------
And here


Repository:
  rL LLVM

http://reviews.llvm.org/D18438