[PATCH] D16875: MemorySSA Optimizations: Patch 1 of N

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 8 13:40:14 PDT 2016


george.burgess.iv updated this revision to Diff 53082.
george.burgess.iv added a comment.

So, I bugged Philip at the social, and he said the patch (as it stood) looks good, with a few comments:

1. The LLVM spec is ambiguous about whether we can hoist a non-volatile load above a volatile load when the loads alias. It's probably best not to exploit this ambiguity at the moment by unconditionally allowing the motion of nonvolatile loads above volatile loads (and vice versa).

2. It may be good to make pointsToConstantMemory bit check not succeed if the operation is a volatile access

3. A few style things

Items #3 and #1 have been addressed. #1 required a bit of refactoring of the newly-named `getLoadReorderability` function, because we now have to care about aliasing in some cases.

Item #2 I've thought about, and I'm no longer sure that I agree. Specifically, I believe that MemorySSA's job is to reason about whether one memory operation can cause another memory operation to produce a different result. If MemorySSA determines that memop A doesn't interfere with memop B, it's the *user's* job to determine where it's actually safe to put memop B. For example, consider:

  define void @foo(i8* %a) {
  	; 1 = MemoryDef(liveOnEntry)
  	store volatile i8 0, i8* %a
  	br i1 undef, label %if.then, label %if.end
  
  if.then:
  	; 2 = MemoryDef(1)
  	load volatile i8, i8* %a
  	br i1 label %if.end
  
  if.end:
  	ret void
  }

...MemorySSA will happily (and correctly) say that the volatile load is clobbered by the volatile store. However, it's clearly incorrect to hoist the volatile load into the entry block here, so passes that use MemorySSA will need to exercise some amount of caution in cases involving volatile ops anyway. For this reason, I think it's fine if we say that loads of constant memory are always live on entry, regardless of whether the load is volatile or not.


http://reviews.llvm.org/D16875

Files:
  lib/Transforms/Utils/MemorySSA.cpp
  test/Transforms/Util/MemorySSA/atomic-clobber.ll
  test/Transforms/Util/MemorySSA/constant-memory.ll
  test/Transforms/Util/MemorySSA/invariant-groups.ll
  test/Transforms/Util/MemorySSA/load-invariant.ll
  test/Transforms/Util/MemorySSA/volatile-clobber.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16875.53082.patch
Type: text/x-patch
Size: 13513 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160408/9e47a94f/attachment.bin>


More information about the llvm-commits mailing list