[PATCH] D37198: [InlineCost] add visitSelectInst()

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

This is looking pretty good. Some nit-picky improvements below. I'd love for Easwaran to get another chance to look at this too though...



================
Comment at: lib/Analysis/InlineCost.cpp:1202
+
+    if (CheckSROA) {
+      std::pair<Value *, APInt> TrueBaseAndOffset =
----------------
Prefer early exit here, and below:

  if (!CheckSROA)
    return Base::visitSelectInst(SI);


================
Comment at: lib/Analysis/InlineCost.cpp:1207-1208
+          ConstantOffsetPtrs.lookup(FalseVal);
+      if (TrueBaseAndOffset.first && FalseBaseAndOffset.first &&
+          TrueBaseAndOffset == FalseBaseAndOffset) {
+        ConstantOffsetPtrs[&SI] = TrueBaseAndOffset;
----------------
Similar to the above, this can be:

  if (TrueBaseAndOffset == FalseBaseAndOffset && TrueBaseAndOffset.first)


================
Comment at: lib/Analysis/InlineCost.cpp:1245
+
+  if (CheckSROA) {
+    std::pair<Value *, APInt> BaseAndOffset =
----------------
Early return here as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D37198