[PATCH] Exploit dereferenceable_or_null attribute in LICM pass

Philip Reames listmail at philipreames.com
Wed May 6 09:48:02 PDT 2015


LGTM w/final comments addressed.  Would you like me to commit this for you?


================
Comment at: include/llvm/Analysis/ValueTracking.h:244
@@ +243,3 @@
+  /// itself and its operands, so if this method returns true, it is safe to
+  /// move the instruction as long as the correct dominance relationships for
+  /// the operands and users hold.
----------------
apilipenko wrote:
> hfinkel wrote:
> > What does this comment mean? It is saying anything nonobvious/nontrivial?
> That is the comment from original function description. I left it to say that this function can perform context insensitive analysis. Any suggestions for better wording?
Given this is the original comment describing the unchanged behaviour, I think you can leave it as is.  If Hal disagrees, we can revise this in a separate change.

================
Comment at: lib/Analysis/ValueTracking.cpp:3175
@@ +3174,3 @@
+    for (auto *CmpU : Cmp->users()) {
+      if (const BranchInst *BI = dyn_cast<BranchInst>(CmpU)) {
+        assert(BI->isConditional() && "uses a comparison!");
----------------
I'd prefer you use an early continue here rather than increase the nesting depth.  

================
Comment at: test/Transforms/LICM/hoist-deref-load.ll:226
@@ +225,3 @@
+
+define i1 @test6(i32* noalias nocapture %a, i32* noalias nocapture readonly %b, i32* nocapture readonly dereferenceable_or_null(4) %c, i32 %n) #0 {
+entry:
----------------
Please remove any unnecessary attributes.  Sorry for not noticing that before.

http://reviews.llvm.org/D9253

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list