[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:05:31 PDT 2023

eddyz87 added a comment.

I grepped the code base and see that finalizeLowering() logic is customized only in a handful of cases:

- llvm/lib/Target/ARM/ARMISelLowering.cpp
- llvm/lib/Target/AMDGPU/SIISelLowering.cpp
- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

In all these cases there is no similar logic with regards to error handling.

We have the following "fail" messages in the BPFISelLowering:

- "stack arguments are not supported"
- "variadic functions are not supported"
- "aggregate returns are not supported"
- "too many arguments"
- "pass by value not supported"
- "A call to built-in function '$func' is not supported." (this one is for ExternalSymbolSDNode)
- "aggregate returns are not supported"
- "only small returns supported"

There are no true warnings here, every case is actually an error. (Maybe with exception for ExternalSymbolSDNode which just should be allowed). From my practice I rarely hit even one instance of such errors => it might be the case that capability to report multiple errors at once is not that important. Maybe just update `fail` function to call `report_fatal_error()` internally?

BPFISelLowering is a field in BPFSubtarget, as far as I understand sub-target is a singleton => lowering instance is also a singleton, thus having a global state there is not ideal.

@yonghong-song , what do you think?

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list