Add andm, orm, xorm, eqvm, nndm, negm, pcvm, lzvm, and tovm intrinsic
instructions, a few pseudo instructions to expand logical intrinsic
using VM512, a mechnism to expand such pseudo instructions, and
regression tests. Also, assign vector mask types and vector mask
register classes correctly. This is required to use VM512 registers
as function arguments.
Details
- Reviewers
simoll k-ishizaka - Commits
- rGaefedb170734: [VE] Add logical mask intrinsic instructions
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
50 ms | x64 windows > LLVM.CodeGen/XCore::threads.ll |
Event Timeline
Just this one thing, else, LGTM.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
87–89 | Can we make this a separate patch? |
Hmm, not sure why it needs. These new tests shows the correctness of code modifications enough, IMO.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
87–89 | I condider that, but it's difficult to do so. In order to do so, a regression test is required. Making such a regression test requires additional implementation of vector mask register copy function. I'm not sure what is required to implement that. Probablly, I should ask them when you add above code. I asked tests for V64 registers but forgot to ask tests of VM registers. Anywa, this time I add new intrinsic instructions supporting vm512 and hit this bug. You can see crashes by disabling above modifications and run new regression tests that this patch adds. Then, you can see no crash after enabling above modifications and run the identical tests again. Isn't that enough? |
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
87–89 | You can fix this in a separate patch without additional tests since it is obviously incorrect. I am asking you to do this because this patch adds new functionality first - we shouldn't bundle it up with bug fixes. |
Separate this bug fixes to https://reviews.llvm.org/D93212.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
87–89 | Ok. If you don't require additional test for bug fix, I can do it. |
Can we make this a separate patch?