[PATCH] Exploit dereferenceable_or_null attribute in LICM pass

Philip Reames listmail at philipreames.com
Tue May 5 16:15:25 PDT 2015


I'm not particularly worried about compile time w.r.t. this patch.  The much more invasive version of this in ValueTracking appears to be a non-issue.  (Which reminds me, I need to actually enable that!)

With the current round of comments addressed (two comments, one bug fix), I'm willing to sign off on this.  We've had plenty of time for interested reviewers to take a look and three different people have looked at it.  Not sure what else we could wait for.  :)


================
Comment at: lib/Analysis/ValueTracking.cpp:3175
@@ +3174,3 @@
+    for (auto *CmpU : Cmp->users()) {
+      const BranchInst *BI = dyn_cast<BranchInst>(CmpU);
+      assert(BI->isConditional() && "uses a comparison!");
----------------
Er, the use can definitely be not a branch instruction.  I think you're missing:
if (!BI) continue;

Add a test case which covers this please.

================
Comment at: lib/Transforms/Scalar/LICM.cpp:645
@@ -642,2 +644,3 @@
                                            LICMSafetyInfo *SafetyInfo) {
   // If it is not a trapping instruction, it is always safe to hoist.
+  const Instruction *CtxI = CurLoop->getLoopPreheader()->getTerminator();
----------------
This comment is now stale.  Possibly:
// If it is not a trapping instruction, it is always safe to hoist.  If the instruction is known not to trap when moved to the preheader, it is also safe to hoist.

http://reviews.llvm.org/D9253

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






More information about the llvm-commits mailing list