[PATCH] D60056: Hoist/sink malloc/free's in LICM.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I've only skimmed through the high level comments so far, so if if any of this is addressed inline, just say so.

I was wondering why you phrased this as a combination of hoisting and sinking as opposed to promotion.  We have the scalar promotion path, and the basic transform you're doing feels a lot like promotion.  I suspect that many of the legality aspects will be common.



================
Comment at: llvm/include/llvm/IR/Instruction.h:539
 
+  /// Return true if this instruction may allocate heap memory.
+  bool mayAllocateMemory() const;
----------------
These feel like they need a bit more specification.  In particular, are there any expectations around how new memory is returned?  Is a deallocation routine allowed to free a random pointer read from a global?

(i.e. which set of locations are we talking about freeing and allocating?)


================
Comment at: llvm/lib/IR/Instruction.cpp:562
+    switch (II->getIntrinsicID()) {
+    default:
+      return false;
----------------
Err, using a blacklist here feels *really* dangerous.  


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

https://reviews.llvm.org/D60056