[llvm] r294922 - NewGVN: Apply the fast math flags fix in r267113 to NewGVN as well.

[Daniel Berlin via llvm-commits]

Author: dannyb
Date: Sun Feb 12 16:25:20 2017
New Revision: 294922

URL: http://llvm.org/viewvc/llvm-project?rev=294922&view=rev
Log:
NewGVN: Apply the fast math flags fix in r267113 to NewGVN as well.

Modified:
    llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
    llvm/trunk/test/Transforms/NewGVN/flags.ll

Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=294922&r1=294921&r2=294922&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Sun Feb 12 16:25:20 2017
@@ -2059,31 +2059,34 @@ void NewGVN::convertDenseToLoadsAndStore
 }
 
 static void patchReplacementInstruction(Instruction *I, Value *Repl) {
+    auto *ReplInst = dyn_cast<Instruction>(Repl);
+  if (!ReplInst)
+    return;
+
   // Patch the replacement so that it is not more restrictive than the value
   // being replaced.
-  auto *Op = dyn_cast<BinaryOperator>(I);
-  auto *ReplOp = dyn_cast<BinaryOperator>(Repl);
-
-  if (Op && ReplOp)
-    ReplOp->andIRFlags(Op);
+  // Note that if 'I' is a load being replaced by some operation,
+  // for example, by an arithmetic operation, then andIRFlags()
+  // would just erase all math flags from the original arithmetic
+  // operation, which is clearly not wanted and not needed.
+  if (!isa<LoadInst>(I))
+    ReplInst->andIRFlags(I);
 
-  if (auto *ReplInst = dyn_cast<Instruction>(Repl)) {
-    // FIXME: If both the original and replacement value are part of the
-    // same control-flow region (meaning that the execution of one
-    // guarentees the executation of the other), then we can combine the
-    // noalias scopes here and do better than the general conservative
-    // answer used in combineMetadata().
+  // FIXME: If both the original and replacement value are part of the
+  // same control-flow region (meaning that the execution of one
+  // guarantees the execution of the other), then we can combine the
+  // noalias scopes here and do better than the general conservative
+  // answer used in combineMetadata().
 
-    // In general, GVN unifies expressions over different control-flow
-    // regions, and so we need a conservative combination of the noalias
-    // scopes.
-    unsigned KnownIDs[] = {
-        LLVMContext::MD_tbaa,           LLVMContext::MD_alias_scope,
-        LLVMContext::MD_noalias,        LLVMContext::MD_range,
-        LLVMContext::MD_fpmath,         LLVMContext::MD_invariant_load,
-        LLVMContext::MD_invariant_group};
-    combineMetadata(ReplInst, I, KnownIDs);
-  }
+  // In general, GVN unifies expressions over different control-flow
+  // regions, and so we need a conservative combination of the noalias
+  // scopes.
+  static const unsigned KnownIDs[] = {
+      LLVMContext::MD_tbaa,           LLVMContext::MD_alias_scope,
+      LLVMContext::MD_noalias,        LLVMContext::MD_range,
+      LLVMContext::MD_fpmath,         LLVMContext::MD_invariant_load,
+      LLVMContext::MD_invariant_group};
+  combineMetadata(ReplInst, I, KnownIDs);
 }
 
 static void patchAndReplaceAllUsesWith(Instruction *I, Value *Repl) {

Modified: llvm/trunk/test/Transforms/NewGVN/flags.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/flags.ll?rev=294922&r1=294921&r2=294922&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/flags.ll (original)
+++ llvm/trunk/test/Transforms/NewGVN/flags.ll Sun Feb 12 16:25:20 2017
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; RUN: opt -newgvn -S < %s | FileCheck %s
 
 declare void @use(i1)