[PATCH] D100218: Fix for "Bug 49146 - Crash with MIPS16 multiply"
Jesse DeGuire via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 13 09:26:06 PDT 2021
jdeguire marked 3 inline comments as done.
jdeguire added a comment.
Apologies for the very, very late responses to your comments. Yes, it really did take me 3 weeks to figure out that the "Submit" button is at the very bottom.
I would still appreciate any guidance on verifying direct code generation (or if that's even normally done as part of these tests). Thank you.
================
Comment at: clang/test/CodeGen/mips16-mult.c:1
+// REQUIRES: mips-registered-target
+// RUN: %clang --target=mipsel-unknown-linux -mips16 -c -o %t %s
----------------
atanasyan wrote:
> 1. You need to create a test case in the `llvm/test/CodeGen/Mips` folder. Sometimes LLVM build without Clang.
> 2. Test case should check result of code generation. Not only the fact that the program does not crash.
Mostly done, anyway. As stated my in most recent patch update, I was able to complete (1) but (2) I was able to complete for assembly output but not for direct machine code generation due to issues I'm having with llvm-objdump.
================
Comment at: llvm/lib/Target/Mips/MipsISelLowering.cpp:208
case MipsISD::Multu: return "MipsISD::Multu";
+ case MipsISD::Mult16:
+ return "MipsISD::Mult16";
----------------
atanasyan wrote:
> It's better to keep formattings the same as the code around.
Understood, reverted.
================
Comment at: llvm/lib/Target/Mips/MipsISelLowering.h:57
- enum NodeType : unsigned {
- // Start the numbering from where ISD NodeType finishes.
----------------
atanasyan wrote:
> Please revert this too big change unrelated to this patch.
Reverted.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100218/new/
https://reviews.llvm.org/D100218
More information about the llvm-commits
mailing list