[PATCH] D96448: BPF: disable SpeculateAroundPHIsPass with TTI IntImmCost TCC_Free

[Yonghong Song via Phabricator via llvm-commits]

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

Several bpf selftests failed with latest llvm trunk, e.g.,

  #10/16 strobemeta.o:FAIL
  #10/17 strobemeta_nounroll1.o:FAIL
  #10/18 strobemeta_nounroll2.o:FAIL
  #10/19 strobemeta_subprogs.o:FAIL
  #96 snprintf_btf:FAIL

I did some study on #10/18 and found the reason of failure is due
to additional branches which make kernel verifier less effective.
For example,

  for (i = 0; i < 25; i++) {
    int len = 0;
    location = ...
    if (!location) continue;
    len = ...
    if (len >= 2) continue;
    payload += len;
  }
  // using payload

LLVM 12 generate loop body like

   int i = 0;
  begin:
   int len = 0;
   location = ...
   if (!location) goto end;
   len = ...
   if (len >= 2) goto end;
   payload += len;
  end:
   i++;
   goto begin;

This way, verifier will be able to verifier the loop
first before any backtracking:

  int len = 0;
  location = ...;
  len = ...; // assume !!locatoin
  payload += len; // assume 0 <= len < 1
  ...

This way, verifier will get a state and later backtrack
can skip traversing the code after the "i" loop as
it can be proven that it is safe to skip.

LLVM 13 generate loop body like

   int i = 0;
  begin:
   int len = 0;
   location = ...
   if (location) goto next1;
   goto end;
  next1:
   len = ...
   if (0 <= len < 2) goto next2;
   goto end;
  next2:
   payload += len;
  end:
   i++;
   goto begin;

This way, verifier will traverse

  int len = 0;
  location = ... // assume location == 0
  i++;

for the whole "i" loop and then codes after "i" loop.
When backtracking in "i" happens, the codes after "i"
loop needs to be traversed again as it is not deemed safe.

The above additional control flow changes in llvm13
comes from SpeculateAroundPHIsPass. Disabling
this phase fixed all above failures including test #96
which complains complicated control flow with verifier.
In llvm12, SpeculateAroundPHIsPass is not called.

Verifier might have room to further improve but
this patch disabled SpeculateAroundPHIsPass with
backend reimplemented TTI.getIntImmCost() as there
is no need to make control flow more complex.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96448

Files:
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BPFTargetMachine.h
  llvm/lib/Target/BPF/BPFTargetTransformInfo.h


Index: llvm/lib/Target/BPF/BPFTargetTransformInfo.h
===================================================================
--- /dev/null
+++ llvm/lib/Target/BPF/BPFTargetTransformInfo.h
@@ -0,0 +1,46 @@
+//===------ BPFTargetTransformInfo.h - BPF specific TTI ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file uses the target's specific information to
+// provide more precise answers to certain TTI queries, while letting the
+// target independent and default TTI implementations handle the rest.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_BPF_BPFTARGETTRANSFORMINFO_H
+#define LLVM_LIB_TARGET_BPF_BPFTARGETTRANSFORMINFO_H
+
+#include "BPFTargetMachine.h"
+#include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/CodeGen/BasicTTIImpl.h"
+
+namespace llvm {
+class BPFTTIImpl : public BasicTTIImplBase<BPFTTIImpl> {
+  typedef BasicTTIImplBase<BPFTTIImpl> BaseT;
+  typedef TargetTransformInfo TTI;
+  friend BaseT;
+
+  const BPFSubtarget *ST;
+  const BPFTargetLowering *TLI;
+
+  const BPFSubtarget *getST() const { return ST; }
+  const BPFTargetLowering *getTLI() const { return TLI; }
+
+public:
+  explicit BPFTTIImpl(const BPFTargetMachine *TM, const Function &F)
+      : BaseT(TM, F.getParent()->getDataLayout()), ST(TM->getSubtargetImpl(F)),
+        TLI(ST->getTargetLowering()) {}
+
+  int getIntImmCost(const APInt &Imm, Type *Ty, TTI::TargetCostKind CostKind) {
+    return TTI::TCC_Free;
+  }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_LIB_TARGET_BPF_BPFTARGETTRANSFORMINFO_H
Index: llvm/lib/Target/BPF/BPFTargetMachine.h
===================================================================
--- llvm/lib/Target/BPF/BPFTargetMachine.h
+++ llvm/lib/Target/BPF/BPFTargetMachine.h
@@ -34,6 +34,8 @@
 
   TargetPassConfig *createPassConfig(PassManagerBase &PM) override;
 
+  TargetTransformInfo getTargetTransformInfo(const Function &F) override;
+
   TargetLoweringObjectFile *getObjFileLowering() const override {
     return TLOF.get();
   }
Index: llvm/lib/Target/BPF/BPFTargetMachine.cpp
===================================================================
--- llvm/lib/Target/BPF/BPFTargetMachine.cpp
+++ llvm/lib/Target/BPF/BPFTargetMachine.cpp
@@ -12,6 +12,7 @@
 
 #include "BPFTargetMachine.h"
 #include "BPF.h"
+#include "BPFTargetTransformInfo.h"
 #include "MCTargetDesc/BPFMCAsmInfo.h"
 #include "TargetInfo/BPFTargetInfo.h"
 #include "llvm/CodeGen/Passes.h"
@@ -145,6 +146,11 @@
   TargetPassConfig::addIRPasses();
 }
 
+TargetTransformInfo
+BPFTargetMachine::getTargetTransformInfo(const Function &F) {
+  return TargetTransformInfo(BPFTTIImpl(this, F));
+}
+
 // Install an instruction selector pass using
 // the ISelDag to gen BPF code.
 bool BPFPassConfig::addInstSelector() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96448.322805.patch
Type: text/x-patch
Size: 3073 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210210/d4ff4dc9/attachment.bin>