[PATCH] D64203: [OpaquePtr] add Type parameter to Loads.h analysis API.

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good :)



================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:599-603
+  Type *BaseTy = nullptr;
+  if (ByValTy) {
     SafeToUnconditionallyLoad.insert(IndicesVector(1, 0));
+    BaseTy = ByValTy;
+  }
----------------
I'd probably simplify this, eg:

  Type *BaseTy = ByValTy;
  if (BaseTy)
    SafeToUnconditionallyLoad.insert(...);

Or:

  if (ByValTy)
    SafeToUnconditionallyLoad...

  Type *BaseTy = ByValTy;

(I'd potentially declare BaseTy right before UpdateBaseTy - after the comment, since it's so closely connected to that lambda)


================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:611-612
+    if (BaseTy && NewBaseTy != BaseTy)
+      return false;
+    else if (!BaseTy) {
+      BaseTy = NewBaseTy;
----------------
Drop the 'else' after return?

(& potentially invert the second condition to use an early return & reduce indentation:  

  if (BaseTy)
    return true;

  BaseTy = NewBaseTy;
  if (allCallers(...)) {
    ...
  }

  return true;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64203/new/

https://reviews.llvm.org/D64203