[PATCH] D156497: [BPF] report fatal error rather than miscompile

Eduard Zingerman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 2 11:42:09 PDT 2023


eddyz87 added a comment.

In D156497#4554932 <https://reviews.llvm.org/D156497#4554932>, @tamird wrote:

>> Note also that 8e6aab3 (D21368 <https://reviews.llvm.org/D21368>) removed the exit-on-error flag which is necessary for the unit tests with this change.
>
> I think that even if everyone agreed that crashing is acceptable we'd have to find a solution that would allow the tests to pass. Any ideas?

Whats the problem with tests?
The following snippet works on Linux (tried on a single test, should apply to the rest):

  diff --git a/llvm/lib/Target/BPF/BPFISelLowering.cpp b/llvm/lib/Target/BPF/BPFISelLowering.cpp
  index ffeef14d413b..857b729b0217 100644
  --- a/llvm/lib/Target/BPF/BPFISelLowering.cpp
  +++ b/llvm/lib/Target/BPF/BPFISelLowering.cpp
  @@ -48,6 +48,7 @@ static void fail(const SDLoc &DL, SelectionDAG &DAG, const Twine &Msg,
     MachineFunction &MF = DAG.getMachineFunction();
     DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
         MF.getFunction(), Twine(Str).concat(Msg), DL.getDebugLoc()));
  +  llvm::report_fatal_error("Fatal error in BPFISelLowering, can't proceed");
   }
   
   BPFTargetLowering::BPFTargetLowering(const TargetMachine &TM,
  diff --git a/llvm/test/CodeGen/BPF/many_args1.ll b/llvm/test/CodeGen/BPF/many_args1.ll
  index 9e9a3fdfc953..4fb709437f91 100644
  --- a/llvm/test/CodeGen/BPF/many_args1.ll
  +++ b/llvm/test/CodeGen/BPF/many_args1.ll
  @@ -1,5 +1,4 @@
  -; RUN: not llc -march=bpf < %s 2> %t1
  -; RUN: FileCheck %s < %t1
  +; RUN: (llc -march=bpf < %s 2>&1 || exit 0) | FileCheck %s
   ; CHECK: error: <unknown>:0:0: in function foo i32 (i32, i32, i32): t10: i64 = GlobalAddress<ptr @bar> 0 too many arguments
   
   ; Function Attrs: nounwind uwtable

But CI is also run on Windows, so probably a more portable incantation is needed (don't have Windows at hand to check).
Separate question is whether to use `report_fatal_error` or something that does not generate stack trace as error messages have the necessary information after recent changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156497/new/

https://reviews.llvm.org/D156497



More information about the llvm-commits mailing list