[PATCH] D83283: [Attributor] AAPotentialValues Interface

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 30 17:19:09 PDT 2020


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

Don't worry about the other test, master is sometimes broken as well. I left one comment for a follow up patch and one comment for a cleanup to avoid code duplication we have now.
Assuming that can be done without problems prior to committing this, the patch looks good to me :)



================
Comment at: llvm/include/llvm/ADT/APInt.h:102
 
+  friend struct DenseMapInfo<APInt>;
+
----------------
This is the same thing as `DenseMapAPIntKeyInfo` right? We should for sure avoid duplication. Let's just move the implementation into this header, call it `DenseMapInfo<APInt>`, and use it instead of `DenseMapAPIntKeyInfo` in `LLVMContextImpl.h`. That way everyone else can put an APInt into a DenseMap as well.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:7168
+      return;
+    }
+
----------------
In a follow up we should add a flag to the state to indicate it is not empty but contains `undef`. That would allow us to merge such a state with for example `7` and get `7`.


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

https://reviews.llvm.org/D83283



More information about the llvm-commits mailing list