adds vabsd* instruction for unsigned byte/halfword/word, and matching intrinsics to expose them.
Diff Detail
Event Timeline
lib/Target/PowerPC/PPCInstrAltivec.td | ||
---|---|---|
1410–1419 | IIRC, when I was doing the patches that I sent you, there was a generic absolute value intrinsic, that I could use. That is better than the PPC intrinsics, because it is more likely that optimizations understand it. Could you use the generic one here? | |
test/CodeGen/PowerPC/vec_absd.ll | ||
1–2 | you should not need -mattr=+altivec when you have -mcpu=pwr9. Does something go wrong when you remove that? |
include/llvm/IR/IntrinsicsPowerPC.td | ||
---|---|---|
697 | Been a while since I looked at this, but do the PowerPC_Vec_BBB_Intrinsic and it's halfword and word equivalent not have the same definition as this (and therefore allow you to more concisely define these)? Basically what I'm getting at is using the most specialized class that has the right form. | |
lib/Target/PowerPC/PPCInstrAltivec.td | ||
454 | Please don't add blank lines arbitrarily. | |
1410–1419 | I don't think there is a generic one for integer types, but I might be wrong. I think there's only llvm.fabs. | |
1412 | This is a very minor nit, but the very first characters on the continuation line should line up inside the angle bracket. So the double quote character should line up with the 1 character above. | |
test/CodeGen/PowerPC/vec_absd.ll | ||
1–2 | I agree. If this test case behaves differently with and without -mattr=+altivec, we need to look into why. | |
20 | Please add the register operands here. The ABI locks all 3. | |
test/MC/Disassembler/PowerPC/ppc64-encoding-p9vector.txt | ||
1 | Please don't add this file or the one below. The assembler and disassembler do not care about predicates. If they understand an encoding, they will emit what it is. Just add these to the existing test cases. |
Updated based on the review comments:
- fixed extra blank line/spacing issues
- switched the intrinsics to use the most derived type that fits the format
- removed the new test files and put the checks into the existing files
- removed the unneeded altivec feature to the codegen test
Committed revision 285627.
Sean, please close this review if there are no buildbot failures.
Been a while since I looked at this, but do the PowerPC_Vec_BBB_Intrinsic and it's halfword and word equivalent not have the same definition as this (and therefore allow you to more concisely define these)? Basically what I'm getting at is using the most specialized class that has the right form.