[PATCH] D85544: [OpenMPOpt] ICV tracking for calls

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

We need a test with more basic blocks between the setter and getter.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1183
+  virtual Optional<Value *> getUniqueValue(InternalControlVar ICV,
+                                           Attributor &A) const = 0;
 
----------------
The comment of `getUniqueValue` is not helpful. It looks like this will compute the value of `ICV` after the function was called, correct? Whatever it does, write it here ;)


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1381
+    const auto *CB = dyn_cast<CallBase>(I);
+    assert(CB && "Instruction must be a call.");
+
----------------
If this is only for calls, why not take a `CallBase &CB` instead?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1395-1397
+
+    UniqueReplVal = nullptr;
+    return UniqueReplVal;
----------------
`UniqueReplVal` should not be needed anywhere here.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1476
       }
     }
 
----------------
We should have a worklist of instructions we need to look at. Initially it is the argument, if we go through the prdedecessor blocks we add their terminator to the worklist. I'm not sure we need the explorer at all in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85544