Fixes untainted branch that is immediately after relaxed loads
authorPeizhao Ou <peizhaoo@uci.edu>
Thu, 16 Nov 2017 00:00:46 +0000 (16:00 -0800)
committerPeizhao Ou <peizhaoo@uci.edu>
Thu, 16 Nov 2017 00:00:46 +0000 (16:00 -0800)
lib/CodeGen/AtomicExpandPass.cpp
lib/CodeGen/SelectionDAG/DAGCombiner.cpp

index c4eff88..faf9519 100644 (file)
@@ -504,13 +504,17 @@ bool taintConditionalBranch(BranchInst* BI, Value* DepVal) {
       Builder.CreateTrunc(AndDep, IntegerType::get(DepVal->getContext(), 1));
   auto* OrCond = Builder.CreateOr(TruncAndDep, Cond);
   BI->setOperand(0, OrCond);
+
+  // Debug output.
+  DEBUG(dbgs() << "\tTainted branch condition:\n" << *BI->getParent());
+
   return true;
 }
 
 // XXX-update: For a relaxed load 'LI', find the first immediate atomic store or
-// the first conditional branch. Returns itself if 'LI' can be left as is;
-// returns nullptr if there's no such immediately following store/branch
-// instructions, which we can only enforce the load with 'acquire'.
+// the first conditional branch. Returns nullptr if there's no such immediately
+// following store/branch instructions, which we can only enforce the load with
+// 'acquire'.
 Instruction* findFirstStoreCondBranchInst(LoadInst* LI) {
   // In some situations, relaxed loads can be left as is:
   // 1. The relaxed load is used to calculate the address of the immediate
@@ -522,6 +526,8 @@ Instruction* findFirstStoreCondBranchInst(LoadInst* LI) {
   // if (r1 != 0) {
   //   y.store(1, relaxed);
   // }
+  // However, in this function, we don't deal with them directly. Instead, we
+  // just find the immediate following store/condition branch and return it.
 
   auto* BB = LI->getParent();
   auto BE = BB->end();
@@ -550,35 +556,37 @@ Instruction* findFirstStoreCondBranchInst(LoadInst* LI) {
       }
     }
     if (BBI == BE) {
-      return LI;
+      return nullptr;
     }
   }
 }
 
-void taintMonotonicLoads(const SmallVector<LoadInst*, 1>& MonotonicLoadInsts) {
+// XXX-comment: Returns whether the code has been changed.
+bool taintMonotonicLoads(const SmallVector<LoadInst*, 1>& MonotonicLoadInsts) {
+  bool Changed = false;
   for (auto* LI : MonotonicLoadInsts) {
     auto* FirstInst = findFirstStoreCondBranchInst(LI);
     if (FirstInst == nullptr) {
-      // No need to worry about the relaxed load.
-      continue;
-    }
-    if (FirstInst == LI) {
       // We don't seem to be able to taint a following store/conditional branch
       // instruction. Simply make it acquire.
+      DEBUG(dbgs() << "[RelaxedLoad]: Transformed to acquire load\n"
+                   << *LI << "\n");
       LI->setOrdering(Acquire);
+      Changed = true;
       continue;
     }
     // Taint 'FirstInst', which could be a store or a condition branch
     // instruction.
     if (FirstInst->getOpcode() == Instruction::Store) {
-      taintStoreAddress(dyn_cast<StoreInst>(FirstInst), LI);
+      Changed |= taintStoreAddress(dyn_cast<StoreInst>(FirstInst), LI);
     } else if (FirstInst->getOpcode() == Instruction::Br) {
-      taintConditionalBranch(dyn_cast<BranchInst>(FirstInst), LI);
+      Changed |= taintConditionalBranch(dyn_cast<BranchInst>(FirstInst), LI);
     } else {
       assert(false && "findFirstStoreCondBranchInst() should return a "
                     "store/condition branch instruction");
     }
   }
+  return Changed;
 }
 
 /**** Implementations of public methods for dependence tainting ****/
@@ -977,7 +985,7 @@ bool AtomicExpand::runOnFunction(Function &F) {
     }
   }
 
-  taintMonotonicLoads(MonotonicLoadInsts);
+  MadeChange |= taintMonotonicLoads(MonotonicLoadInsts);
 
   return MadeChange;
 }
index 86732bb..bdcf5a2 100644 (file)
@@ -3100,9 +3100,19 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
 
   // XXX-disabled: (and x, 0) should not be folded.
+  // (and (and x, 0), y) shouldn't either.
   if (!N0C && N1C->isNullValue()) {
     return SDValue();
   }
+  if (!N0C) {
+    if (N0.getOpcode() == ISD::AND) {
+      auto* N01 = N0.getOperand(1).getNode();
+      auto* N01C = dyn_cast<ConstantSDNode>(N01);
+      if (N01C && N01C->isNullValue()) {
+        return SDValue();
+      }
+    }
+  }
 
   if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::AND, SDLoc(N), VT, N0C, N1C);