[PATCH] D87428: [RFC] BPF: avoid same condition evaluated in multiple branch instructions

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song created this revision.
yonghong-song added a reviewer: ast.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
yonghong-song requested review of this revision.

This patch specifically tries to address an issue reported at

  https://lore.kernel.org/bpf/20200812074826.GB754656@krava/

If one icmp condition is used in more than one places, prevent
this condition from inlining into multiple places.

For example,

  cond = a < b 
  ... 
  select cond, val1, val2
  ... 
  br cond, block1, block2

In actually lowering, cond will be put into select_cc and br_cc
so we will have proper cond in the jmp inst.
It is reported this caused the issue if cond happens to be

  pkt_ptr < pkt_end

due to particular kernel implementation.
The transformation here is to put a barrier after "cond = a < b"
so cond cannot be inlined into select/br. The pseudo generated code
looks like:

  if a < b then
    tmp = 1 
  then
    tmp = 0 
  ... 
  if (tmp == 1) val1; else val2
  ... 
  if (tmp == 1) goto block1; else goto block2;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87428

Files:
  llvm/lib/Target/BPF/BPFCheckAndAdjustIR.cpp


Index: llvm/lib/Target/BPF/BPFCheckAndAdjustIR.cpp
===================================================================
--- llvm/lib/Target/BPF/BPFCheckAndAdjustIR.cpp
+++ llvm/lib/Target/BPF/BPFCheckAndAdjustIR.cpp
@@ -61,6 +61,7 @@
   void checkIR(Module &M);
   bool adjustIR(Module &M);
   bool removePassThroughBuiltin(Module &M);
+  bool serializeCond(Module &M);
   bool optimizeReloc(Module &M);
   void findOptRelocCand(Instruction &I);
   void checkBitCast(BitCastInst *BI, GetElementPtrInst *GEP1, GlobalVariable *GV1);
@@ -144,6 +145,61 @@
   return Changed;
 }
 
+bool BPFCheckAndAdjustIR::serializeCond(Module &M) {
+  // Forcing to lower an icmp condition if it is used
+  // in multiple places.
+  // For:
+  //   icmp1 = ...
+  //   ...
+  //   a = select icmp1, val1, val2
+  //   ...
+  //   ... icmp1 ...
+  // Change to:
+  //   icmp1 = ...
+  //   icmp1 = user_asm_barrier(icmp1)
+  //   ...
+  //   a = select icmp1, val1, val2
+  //   ...
+  //   ... icmp1 ...
+  SmallVector<ICmpInst *, 16> Candidates;
+  for (Function &F : M)
+    for (auto &BB : F)
+      for (auto &I : BB) {
+        auto *Cond = dyn_cast<ICmpInst>(&I);
+        if (!Cond || Cond->use_empty() || Cond->hasOneUse())
+          continue;
+        Candidates.push_back(Cond);
+      }
+
+  if (Candidates.empty())
+    return false;
+
+  bool Changed = false;
+  for (auto *Cond : Candidates) {
+    SmallVector<Instruction *, 16> Uses;
+    for (User *U : Cond->users()) {
+      Instruction *Inst = dyn_cast<Instruction>(U);
+      if (!Inst || (!isa<SelectInst>(Inst) && !isa<BranchInst>(Inst)))
+        continue;
+      Uses.push_back(Inst);
+    }
+    if (Uses.size() < 2)
+      continue;
+
+    Changed = true;
+    Instruction *NextI = Cond->getNextNonDebugInstruction();
+    assert(NextI);
+    FunctionType *AsmTy = FunctionType::get(Cond->getType(), {Cond->getType()}, false);
+    InlineAsm *Asm = InlineAsm::get(AsmTy, StringRef(""), StringRef("=r,0"), true);
+    auto *CI = CallInst::Create(Asm, {Cond}, "", NextI);
+    for (Instruction *Inst : Uses) {
+      Inst->setOperand(0, CI);
+    }
+  }
+
+  return Changed;
+}
+
 // Adjust/Optimize the following pattern
 //    %0 = load i64, i64* @"llvm.bpf_htab:0:0$0:0", align 8
 //    %1 = getelementptr i8, i8* bitcast (%struct.anon.2* @m_hash to i8*),
@@ -304,6 +360,7 @@
   bool Changed = false;
 
   Changed = removePassThroughBuiltin(M);
+  Changed = serializeCond(M);
   Changed = optimizeReloc(M) || Changed;
 
   return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87428.290862.patch
Type: text/x-patch
Size: 2496 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200910/53c4953a/attachment.bin>