[PATCH] D113656: [LICM] Check a heuristic case to hoist load

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 planned changes to this revision.
jaykang10 added a comment.

Ah... `@a` could contains the address of `@a`... Thanks for pointing out it. @fhahn @nikic

It is fundamental problem with alias between different pointer types like `long **`, `long***`... I need to find the clear background reason to distinguish them...

Let me think about it more.



================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2407
+  // ; MemoryUse(2) MayAlias
+  //   %0 = load i32**, i32*** @a, align 8, !tbaa !8
+  //   %idxprom = zext i32 %i.0 to i64
----------------
fhahn wrote:
> is `!tbaa` required here?
> 
> Also, would it be possible that `@a` contains the address of `@a` before we enter the loop? Could the first store in the loop modify `@a` that way?
> is !tbaa required here?
The same `!tbaa` on load and store is intended to show that it causes MayAlias on TBAA.

>  Also, would it be possible that @a contains the address of @a before we enter the loop? Could the first store in the loop modify @a that way?
You are right! I missed it...


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2466
+  auto *PtrDiffSCEV = SE->getMinusSCEV(StorePtrSCEV, LoadSCEV);
+  if (dyn_cast<SCEVAddRecExpr>(PtrDiffSCEV))
+    return false;
----------------
fhahn wrote:
> is this sufficient? An addrec could have an unknown step I think, which could be 0.
Ah, step could be 0... Thanks for pointing out it.


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

https://reviews.llvm.org/D113656