[PATCH] D156136: [BPF] Clean up SelLowering

Tamir Duberstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 11:58:22 PDT 2023


tamird added a comment.

In D156136#4529021 <https://reviews.llvm.org/D156136#4529021>, @ast wrote:

> I don't see a value in this "cleanup". Looks like code churn to me.

The value is that the current error messages are often misleading or anemic. When working on this code it's extremely painful when the only way to understand what's going on is to step through a debugger because the error messages are just "condition not met" without indicating any of the inputs.



================
Comment at: llvm/lib/Target/BPF/BPFISelLowering.cpp:261
   switch (Opcode) {
-  default:
-    report_fatal_error("Unhandled custom legalization");
+  default: {
+    report_fatal_error("unhandled custom legalization: " + Twine(Opcode));
----------------
ast wrote:
> and here?
Done.


================
Comment at: llvm/lib/Target/BPF/BPFISelLowering.cpp:381
 
-const unsigned BPFTargetLowering::MaxArgs = 5;
+const size_t BPFTargetLowering::MaxArgs = 5;
 
----------------
ast wrote:
> pls drop
Why? This results in a `static_cast` because the type is not correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156136



More information about the llvm-commits mailing list