[PATCH] D69954: [OpenMP][Opt] Delete read-only parallel regions

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 17 07:58:56 PST 2019


jdoerfert marked an inline comment as done.
jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:127
+        return false;
+
+      LLVM_DEBUG(dbgs() << Tag << "Delete read-only parallel region in "
----------------
JonChesterfield wrote:
> I'm not totally confident that onlyReadsMemory is sufficient here (concerned about I/O, functions that do not return).
> 
> LICM hoists based on this so it's safe for I/O. Likewise malloc/free are fine.
> 
> Please could you point me to the reason this is safe for functions that don't return?
> Please could you point me to the reason this is safe for functions that don't return?

It is not. I'll add the proper checks, we have the attribute (`willreturn`) so it is not a problem to look for it.

I/O is not read only. Malloc/free are neither. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69954





More information about the llvm-commits mailing list