[PATCH] D19381: Extend load/store type canonicalization to handle unordered operations

[JF Bastien via llvm-commits]

jfb added a comment.

I don't think it matters but would rather ask: there's no case where a less-than-naturally-aligned atomic would matter? I'm specifically thinking of an example such as:

  %v = load atomic i32, i32* %p unordered, align 2
  %c = bitcast i32 to float
  %s = add float %c, 1.0

Could this new code be leading backends to generate bad code? I think not, LMK if you think otherwise.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:330
@@ -329,2 +329,3 @@
       LI.getAlignment(), LI.getName() + Suffix);
+  NewLoad->setAtomic(LI.getOrdering(), LI.getSynchScope());
   MDBuilder MDB(NewLoad->getContext());
----------------
Add `assert(!LI.isVolatile() && "volatile unhandled here")` here and below, makes it less likely to break in the future.

================
Comment at: test/Transforms/InstCombine/atomic.ll:207
@@ +206,3 @@
+
+define i32 @test21(i32** %p, i8* %v) {
+; CHECK-LABEL: define i32 @test21(
----------------
`; TODO: probably also legal in this case`


http://reviews.llvm.org/D19381