This is my first attempt to contribute to llvm. I'm trying to implement this https://bugs.llvm.org/show_bug.cgi?id=30633. MUL+ADDSUB to FMADDSUB was implemented here https://reviews.llvm.org/D28087. I'm looking for help now.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
7620 ↗ | (On Diff #123884) | Rename to isFMAddSubOrFMSubAdd? |
7628 ↗ | (On Diff #123884) | Should just return SDValue() here no matter the type. We can't create a X86ISD::SUBADD, there's no such instruction. |
29708 ↗ | (On Diff #123884) | Maybe merge this with isAddSub by passing a bool value to indicate which one you're looking for. Then you just need to factor that bool into the code that detects the FSUB/FADD |
lib/Target/X86/X86ISelLowering.h | ||
204 ↗ | (On Diff #123884) | There's no such instruction as SUBADD that doesn't include the multiply. So we shouldn't be adding this opcode. |
Updated diff according to code review. I need some help with documenting updated functions
Is this really testing both ways of doing this? There's one from shuffles and one from build_vector.
According to the coverage build bot, the build_vector path of fmaddsub isn't tested today.
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
7330 ↗ | (On Diff #124128) | Rename to isAddSubOrSubAdd? |
7333 ↗ | (On Diff #124128) | Call this matchSubAdd instead of isSubAdd. |
29730 ↗ | (On Diff #124128) | This should probably be "ADDSUB, FMADDSUB, or FMSUBADD" |
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
7862 ↗ | (On Diff #124128) | I added that if statement, but I have problems testing this, specifically I don't know how to test this |
Lol, I added inline comment to that code, but didn't submit comment. Indeed, this isn't tested and I don't know how to do it.
Can you just remove the untested part? I'll add a TODO to the existing untested code indicating that it isn't tested.