or(zext(load8(base)), zext(load8(base+1)) -> zext(load16 base)
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 41287 Build 41481: arc lint + arc unit
Event Timeline
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
639 | Yes, that's something I meant to highlight in thsi patch, but forgot to send the comment: I've seen in other tests for ARM we're OK paying quite a large number of instructions to avoid loads, e.g. define i32 @load_i32_by_i8_bswap(i32* %arg) { ; BSWAP is not supported by 32 bit target ; CHECK-LABEL: load_i32_by_i8_bswap: ; CHECK: @ %bb.0: ; CHECK-NEXT: ldr r0, [r0] ; CHECK-NEXT: mov r1, #65280 ; CHECK-NEXT: mov r2, #16711680 ; CHECK-NEXT: and r1, r1, r0, lsr #8 ; CHECK-NEXT: and r2, r2, r0, lsl #8 ; CHECK-NEXT: orr r1, r1, r0, lsr #24 ; CHECK-NEXT: orr r0, r2, r0, lsl #24 ; CHECK-NEXT: orr r0, r0, r1 ; CHECK-NEXT: mov pc, lr I'm not very familiar with these targets, where do we draw the line ? |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
639 | This appears to be running under a very old architecture where bswap is not legal. Any idea why it is trying to perform this optimisation when there is no bswap to make it efficient? |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
639 | // Before legalize we can introduce illegal bswaps which will be later // converted to an explicit bswap sequence. This way we end up with a single // load and byte shuffling instead of several loads and byte shuffling. if (NeedsBswap && LegalOperations && !TLI.isOperationLegal(ISD::BSWAP, VT)) return SDValue(); I can always bail out on zext: if (NeedsBswap && (LegalOperations || NeedsZext) && !TLI.isOperationLegal(ISD::BSWAP, VT)) return SDValue(); But it's not clear to me what's worth between a load and 4 arithmetic instructions on ARM. |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
639 | I think:
Can you add these two targets to the two test files: If they are OK (along with the armv6 case) then I, from an ARM perspective, am happy with this. |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
639 |
OK, so it looks like this was a real regression. So I've disabled illegal bswaps when zext-ending. Thanks for the information !
Done. |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
3–4 | Ah, yeah. Try thumbv6meb-none-eabi and thumbv7meb-none-eabi for big endian. |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
3–4 | D'oh. Sorry about that. |
llvm/test/CodeGen/ARM/load-combine-big-endian.ll | ||
---|---|---|
3–4 | Nice one. I'm happy, if RKSimon is happy. |
NFC fix?