Add support for translating llvm::SwitchInst.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Please add tests for user-visible error messages.
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | ||
---|---|---|
866–867 | The (unwritten) MLIR convention is to have a test for every user-visible error message. Here, I suspect we will see two error messages in a row because whatever failed in processValue should have reported the error as well. Callers usually just propagate the error state. | |
mlir/test/Target/LLVMIR/Import/switch.ll | ||
6 ↗ | (On Diff #425904) | Nit: can't this just live with the rest of ops in import.ll? |
- Integrate test cases into test/Target/LLVMIR/Import/basic.ll
- Add a new test case to test llvm.switch with arguments
- Remove error message printing while translating predicate values and block arguments to avoid showing (similar) error messages twice.
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | ||
---|---|---|
866–867 |
I feel like a better solution will be showing "failed to process the condition value of...", in addition to the error message from processValue, as a note (similar to what clang does) so that user will have a quicker grasp on the problem. But that's a different story I guess. I will remove the emitError here, the one at line 821 and 836 to avoid confusion (i.e. two error messages). But I'll still add a test case that contains block arguments |
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | ||
---|---|---|
866–867 | Yes, it would be better to have the general capability of spreading the construction of a diagnostic across multiple calls, but we don't have it now. |
add braces