[PATCH] D22598: [CFLAA] Add support for field offset in CFLGraph

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

Thanks for the patch!


================
Comment at: lib/Analysis/CFLGraph.h:31
@@ +30,3 @@
+// because we require a signed value.
+enum : int64_t { UnknownOffset = std::numeric_limits<int64_t>::max() };
+
----------------
IIRC, some MSVC versions we support don't have a constexpr `max()` function, so this will break those bots. Please either use a static `LLVM_CONSTEXPR` variable, or wrap this in a function. :)

================
Comment at: lib/Analysis/CFLGraph.h:312
@@ +311,3 @@
+      GEPOperator *GEPOp = dyn_cast<GEPOperator>(&Inst);
+      assert(GEPOp != nullptr);
+      visitGEP(*GEPOp);
----------------
Please use `cast<GEPOperator>` instead of `dyn_cast + assert`.

================
Comment at: lib/Analysis/CFLGraph.h:499
@@ +498,3 @@
+      case Instruction::GetElementPtr: {
+        auto GEPOp = dyn_cast<GEPOperator>(CE);
+        assert(GEPOp != nullptr);
----------------
Same "please use `cast`" comment


https://reviews.llvm.org/D22598