[PATCH] D50786: [AST] Remove notion of volatile from alias sets

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added inline comments.
This revision now requires changes to proceed.


================
Comment at: test/Transforms/LICM/hoisting.ll:153
 
+; Can neither sink nor hoist
+define i32 @test_volatile(i1 %c) {
----------------
Please add `CHECK-LABEL: Out` after the load, otherwise we cannot be sure that we didn't sink.



================
Comment at: test/Transforms/LICM/scalar-promote.ll:80
+; CHECK: Loop:
+; CHECK: store volatile
+  br label %Loop
----------------
Two concerns here:
1) If you want to make sure that we don't sink, please add `CHECK-LABEL: Out` after the check on store.
2) In this particular case, it would be correct to either hoist or sink, just because we *never* go to the 2nd iteration and this code is in fact linear. The optimizer could've recognized this pattern and sunk or hoisted the instruction, and it would not be a bug. Do you mind making a similar test but with a different loop exit condition in which that would be incorrect?


Repository:
  rL LLVM

https://reviews.llvm.org/D50786